Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[disk-buffering] Migrate StorageFile to FileOperations #986

Merged
merged 2 commits into from
Jul 31, 2023
+66 −53
Merged

[disk-buffering] Migrate StorageFile to FileOperations #986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea224af
migrate StorageFile to FileOperations
breedx-splk Jul 28, 2023
890318c
sniffed it
breedx-splk Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...src/main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private File findReadableFile() throws IOException {
// Checking if the oldest available file is currently the writable file.
if (oldestFileAvailable != null
&& currentWritableFile != null
&& oldestFileAvailable.equals(currentWritableFile.file)) {
&& oldestFileAvailable.equals(currentWritableFile.getFile())) {
currentWritableFile.close();
}
return oldestFileAvailable;
Expand All @@ -88,7 +88,7 @@ private int purgeExpiredFilesIfAny(File[] existingFiles, long currentTimeMillis)
int filesDeleted = 0;
for (File existingFile : existingFiles) {
if (hasExpiredForReading(currentTimeMillis, Long.parseLong(existingFile.getName()))) {
if (currentReadableFile != null && existingFile.equals(currentReadableFile.file)) {
if (currentReadableFile != null && existingFile.equals(currentReadableFile.getFile())) {
currentReadableFile.close();
}
if (existingFile.delete()) {
Expand All @@ -103,7 +103,7 @@ private void removeOldestFileIfSpaceIsNeeded(File[] existingFiles) throws IOExce
if (existingFiles.length > 0) {
if (isNeededToClearSpaceForNewFile(existingFiles)) {
File oldest = getOldest(existingFiles);
if (currentReadableFile != null && oldest.equals(currentReadableFile.file)) {
if (currentReadableFile != null && oldest.equals(currentReadableFile.getFile())) {
currentReadableFile.close();
}
if (!oldest.delete()) {
Expand Down
23 changes: 23 additions & 0 deletions ...n/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/FileOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.contrib.disk.buffering.internal.storage.files;

import java.io.Closeable;
import java.io.File;

public interface FileOperations extends Closeable {
long getSize();

boolean hasExpired();

boolean isClosed();

File getFile();

default String getFileName() {
return getFile().getName();
}
}
10 changes: 8 additions & 2 deletions ...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/ReadableFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
*
* <p>More information on the overall storage process in the CONTRIBUTING.md file.
*/
public final class ReadableFile extends StorageFile {
public final class ReadableFile implements FileOperations {
private final File file;
private final int originalFileSize;
private final StreamReader reader;
private final FileTransferUtil fileTransferUtil;
Expand Down Expand Up @@ -62,7 +63,7 @@ public ReadableFile(
StorageConfiguration configuration,
StreamReader.Factory readerFactory)
throws IOException {
super(file);
this.file = file;
this.clock = clock;
expireTimeMillis = createdTimeMillis + configuration.getMaxFileAgeForReadMillis();
originalFileSize = (int) file.length();
Expand Down Expand Up @@ -139,6 +140,11 @@ public synchronized boolean isClosed() {
return isClosed.get();
}

@Override
public File getFile() {
return file;
}

@Override
public synchronized void close() throws IOException {
if (isClosed.compareAndSet(false, true)) {
Expand Down
23 changes: 0 additions & 23 deletions ...main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/StorageFile.java
View file
Open in desktop

This file was deleted.

12 changes: 10 additions & 2 deletions ...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
import java.io.OutputStream;
import java.util.concurrent.atomic.AtomicBoolean;

public final class WritableFile extends StorageFile {
public final class WritableFile implements FileOperations {

private final File file;

private final StorageConfiguration configuration;
private final Clock clock;
private final long expireTimeMillis;
Expand All @@ -27,7 +30,7 @@ public final class WritableFile extends StorageFile {
public WritableFile(
File file, long createdTimeMillis, StorageConfiguration configuration, Clock clock)
throws IOException {
super(file);
this.file = file;
this.configuration = configuration;
this.clock = clock;
expireTimeMillis = createdTimeMillis + configuration.getMaxFileAgeForWriteMillis();
Expand Down Expand Up @@ -75,6 +78,11 @@ public synchronized boolean isClosed() {
return isClosed.get();
}

@Override
public File getFile() {
return file;
}

@Override
public synchronized void close() throws IOException {
if (isClosed.compareAndSet(false, true)) {
Expand Down
43 changes: 21 additions & 22 deletions ...test/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.mockito.Mockito.mock;

import io.opentelemetry.contrib.disk.buffering.internal.storage.files.ReadableFile;
import io.opentelemetry.contrib.disk.buffering.internal.storage.files.StorageFile;
import io.opentelemetry.contrib.disk.buffering.internal.storage.files.WritableFile;
import io.opentelemetry.sdk.common.Clock;
import java.io.File;
Expand All @@ -45,9 +44,9 @@ void setUp() {
void createWritableFile_withTimeMillisAsName() throws IOException {
doReturn(MILLISECONDS.toNanos(1000L)).when(clock).now();

StorageFile file = folderManager.createWritableFile();
WritableFile file = folderManager.createWritableFile();

assertEquals("1000", file.file.getName());
assertEquals("1000", file.getFileName());
}

@Test
Expand All @@ -62,11 +61,11 @@ void createWritableFile_andRemoveOldestOne_whenTheAvailableFolderSpaceIsNotEnoug
fillWithBytes(existingFile3, MAX_FILE_SIZE);
doReturn(1500L).when(clock).now();

StorageFile file = folderManager.createWritableFile();
WritableFile file = folderManager.createWritableFile();

assertNotEquals(existingFile1, file.file);
assertNotEquals(existingFile2, file.file);
assertNotEquals(existingFile3, file.file);
assertNotEquals(existingFile1, file.getFile());
assertNotEquals(existingFile2, file.getFile());
assertNotEquals(existingFile3, file.getFile());
assertTrue(existingFile2.exists());
assertTrue(existingFile3.exists());
assertFalse(existingFile1.exists());
Expand All @@ -87,7 +86,7 @@ void closeCurrentlyWritableFile_whenItIsReadyToBeRead_anNoOtherReadableFilesAreA

ReadableFile readableFile = folderManager.getReadableFile();

assertEquals(writableFile.file, readableFile.file);
assertEquals(writableFile.getFile(), readableFile.getFile());
assertTrue(writableFile.isClosed());
}

Expand All @@ -105,7 +104,7 @@ void closeCurrentlyWritableFile_whenItIsReadyToBeRead_anNoOtherReadableFilesAreA
doReturn(MILLISECONDS.toNanos(1000L + MIN_FILE_AGE_FOR_READ_MILLIS)).when(clock).now();

ReadableFile readableFile = folderManager.getReadableFile();
assertEquals(existingFile1, readableFile.file);
assertEquals(existingFile1, readableFile.getFile());

folderManager.createWritableFile();

Expand All @@ -127,11 +126,11 @@ void createWritableFile_andDoNotRemoveOldestOne_ifAtLeastOneExpiredFileIsPurged(
fillWithBytes(existingFile3, MAX_FILE_SIZE);
doReturn(MILLISECONDS.toNanos(11_000L)).when(clock).now();

StorageFile file = folderManager.createWritableFile();
WritableFile file = folderManager.createWritableFile();

assertNotEquals(existingFile1, file.file);
assertNotEquals(existingFile2, file.file);
assertNotEquals(existingFile3, file.file);
assertNotEquals(existingFile1, file.getFile());
assertNotEquals(existingFile2, file.getFile());
assertNotEquals(existingFile3, file.getFile());
assertTrue(existingFile2.exists());
assertTrue(existingFile1.exists());
assertFalse(existingFile3.exists());
Expand All @@ -146,11 +145,11 @@ void purgeExpiredForReadFiles_whenCreatingNewOne() throws IOException {
createFiles(expiredReadableFile, expiredWritableFile);
doReturn(MILLISECONDS.toNanos(11_500L)).when(clock).now();

StorageFile file = folderManager.createWritableFile();
WritableFile file = folderManager.createWritableFile();

assertFalse(expiredReadableFile.exists());
assertTrue(expiredWritableFile.exists());
assertNotEquals(expiredWritableFile, file.file);
assertNotEquals(expiredWritableFile, file.getFile());
}

@Test
Expand All @@ -163,16 +162,16 @@ void closeExpiredReadableFileInUseIfAny_whenPurgingExpiredForReadFiles_whenCreat

doReturn(MILLISECONDS.toNanos(900 + MIN_FILE_AGE_FOR_READ_MILLIS)).when(clock).now();
ReadableFile readableFile = folderManager.getReadableFile();
assertEquals(expiredReadableFileBeingRead, readableFile.file);
assertEquals(expiredReadableFileBeingRead, readableFile.getFile());

doReturn(MILLISECONDS.toNanos(11_500L)).when(clock).now();

StorageFile file = folderManager.createWritableFile();
WritableFile file = folderManager.createWritableFile();

assertFalse(expiredReadableFile.exists());
assertFalse(expiredReadableFileBeingRead.exists());
assertTrue(expiredWritableFile.exists());
assertNotEquals(expiredWritableFile, file.file);
assertNotEquals(expiredWritableFile, file.getFile());
assertTrue(readableFile.isClosed());
}

Expand All @@ -186,9 +185,9 @@ void provideFileForRead_afterItsMinFileAgeForReadTimePassed() throws IOException
File readableFile = new File(rootDir, String.valueOf(readableFileCreationTime));
createFiles(writableFile, readableFile);

StorageFile file = folderManager.getReadableFile();
ReadableFile file = folderManager.getReadableFile();

assertEquals(readableFile, file.file);
assertEquals(readableFile, file.getFile());
}

@Test
Expand All @@ -203,9 +202,9 @@ void provideOldestFileForRead_whenMultipleReadableFilesAreAvailable() throws IOE
File readableFileNewer = new File(rootDir, String.valueOf(newerReadableFileCreationTime));
createFiles(writableFile, readableFileNewer, readableFileOlder);

StorageFile file = folderManager.getReadableFile();
ReadableFile file = folderManager.getReadableFile();

assertEquals(readableFileOlder, file.file);
assertEquals(readableFileOlder, file.getFile());
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,6 @@ private static byte[] getByteArrayLine(String line) {
}

private List<String> getWrittenLines() throws IOException {
return Files.readAllLines(writableFile.file.toPath());
return Files.readAllLines(writableFile.getFile().toPath());
}
}
Loading