[disk-buffering] Migrate StorageFile to FileOperations (#986)

open-telemetry · Jul 31, 2023 · d023614 · d023614
1 parent 35b4e7a
commit d023614
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 53 deletions.
diff --git a/...src/main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManager.java b/...src/main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManager.java
@@ -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;
@@ -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()) {
@@ -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()) {

diff --git a/...n/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/FileOperations.java b/...n/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/FileOperations.java
@@ -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();
+  }
+}
diff --git a/...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/ReadableFile.java b/...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/ReadableFile.java
@@ -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;
@@ -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();
@@ -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)) {

diff --git a/...main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/StorageFile.java b/...main/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/StorageFile.java
diff --git a/...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFile.java b/...ain/java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFile.java
@@ -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;
@@ -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();
@@ -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)) {

diff --git a/...test/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManagerTest.java b/...test/java/io/opentelemetry/contrib/disk/buffering/internal/storage/FolderManagerTest.java
@@ -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;
@@ -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
@@ -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());
@@ -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());
   }
 
@@ -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();
 
@@ -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());
@@ -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
@@ -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());
   }
 
@@ -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
@@ -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

diff --git a/...java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFileTest.java b/...java/io/opentelemetry/contrib/disk/buffering/internal/storage/files/WritableFileTest.java
@@ -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());
   }
 }