Skip to content

Commit

Permalink
Remove containing dir when empty (#676)
Browse files Browse the repository at this point in the history
* remove containing dir when empty

* spotless
  • Loading branch information
breedx-splk authored Oct 23, 2023
1 parent 85aef21 commit 2d493f6
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public DefaultSpanStorage(FileUtils fileUtils, File rootDir) {
}

@Override
public File provideSpanFile() {
public File provideSpansDirectory() {
File spansPath = fileUtils.getSpansDirectory(rootDir);
if (spansPath.exists() || spansPath.mkdirs()) {
return spansPath;
Expand All @@ -45,16 +45,16 @@ public File provideSpanFile() {

@Override
public Stream<File> getAllSpanFiles() {
return fileUtils.listSpanFiles(provideSpanFile());
return fileUtils.listSpanFiles(provideSpansDirectory());
}

@Override
public long getTotalFileSizeInBytes() {
return fileUtils.getTotalFileSizeInBytes(provideSpanFile());
return fileUtils.getTotalFileSizeInBytes(provideSpansDirectory());
}

@Override
public Stream<File> getPendingFiles() {
return fileUtils.listSpanFiles(provideSpanFile());
return fileUtils.listSpanFiles(provideSpansDirectory());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ private DeviceSpanStorageLimiter(Builder builder) {
boolean ensureFreeSpace() {
tryFreeingSpace();
// play nice if disk is getting full
return fileProvider.provideSpanFile().getFreeSpace() > limitInBytes();
return fileProvider.provideSpansDirectory().getFreeSpace() > limitInBytes();
}

private void tryFreeingSpace() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
interface SpanStorage {

/***
* Provide location of storing spans
* @return
* Returns the location where spans are buffered.
*/
File provideSpanFile();
File provideSpansDirectory();

/***
* @return all spans including those that can be sent or not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.splunk.rum.SplunkRum.LOG_TAG;

import android.util.Log;
import androidx.annotation.VisibleForTesting;
import io.opentelemetry.android.instrumentation.activity.VisibleScreenTracker;
import java.io.File;
import java.util.UUID;
Expand All @@ -29,7 +30,7 @@
* If the app is brought to foreground and the same session ID still in use, the background spans are moved to /span for eventual sending.
* If the app still in the background until process-kill, the background span files will eventually be deleted by the @DeviceSpanStorageLimiter.
*/
public class StartTypeAwareSpanStorage implements SpanStorage {
class StartTypeAwareSpanStorage implements SpanStorage {

private final VisibleScreenTracker visibleScreenTracker;
private final FileUtils fileUtils;
Expand All @@ -39,19 +40,37 @@ public class StartTypeAwareSpanStorage implements SpanStorage {

static StartTypeAwareSpanStorage create(
VisibleScreenTracker visibleScreenTracker, FileUtils fileUtils, File rootDir) {
File spansDir = fileUtils.getSpansDirectory(rootDir);
String uniqueId = UUID.randomUUID().toString();
return create(visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId);
}

@VisibleForTesting
static StartTypeAwareSpanStorage create(
VisibleScreenTracker visibleScreenTracker,
FileUtils fileUtils,
File rootDir,
File spansDir,
String uniqueId) {
StartTypeAwareSpanStorage startTypeAwareSpanStorage =
new StartTypeAwareSpanStorage(visibleScreenTracker, fileUtils, rootDir);
new StartTypeAwareSpanStorage(
visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId);
startTypeAwareSpanStorage.cleanupUnsentBackgroundSpans();
return startTypeAwareSpanStorage;
}

private StartTypeAwareSpanStorage(
VisibleScreenTracker visibleScreenTracker, FileUtils fileUtils, File rootDir) {
@VisibleForTesting
StartTypeAwareSpanStorage(
VisibleScreenTracker visibleScreenTracker,
FileUtils fileUtils,
File rootDir,
File spansDir,
String uniqueId) {
this.visibleScreenTracker = visibleScreenTracker;
this.fileUtils = fileUtils;
this.rootDir = rootDir;
this.spanDir = fileUtils.getSpansDirectory(rootDir);
this.uniqueId = UUID.randomUUID().toString();
this.spanDir = spansDir;
this.uniqueId = uniqueId;
}

@Override
Expand Down Expand Up @@ -80,7 +99,7 @@ private boolean isAppForeground() {

private void moveBackgroundSpanToPendingSpan() {
fileUtils
.listSpanFiles(getCurrentSessionBackgroundFile())
.listSpanFiles(getCurrentSessionBackgroundDirectory())
.forEach(
file -> {
File destinationFile = new File(spanDir, file.getName());
Expand All @@ -96,19 +115,19 @@ private void moveBackgroundSpanToPendingSpan() {
cleanupUnsentBackgroundSpans();
}

private File getCurrentSessionBackgroundFile() {
private File getCurrentSessionBackgroundDirectory() {
return new File(spanDir, "background/" + uniqueId);
}

@Override
public File provideSpanFile() {
return ensureDirExist(getSpanFile());
public File provideSpansDirectory() {
return ensureDirExist(getSpansDirectory());
}

private File getSpanFile() {
private File getSpansDirectory() {
if (!isAppForeground()) {
Log.d(LOG_TAG, "Creating background span " + uniqueId);
return getCurrentSessionBackgroundFile();
return getCurrentSessionBackgroundDirectory();
}
Log.d(LOG_TAG, "Creating foreground span " + uniqueId);
return spanDir;
Expand Down Expand Up @@ -139,5 +158,9 @@ private void cleanupUnsentBackgroundSpans() {
fileUtils.listFilesRecursively(dir).forEach(fileUtils::safeDelete);
fileUtils.safeDelete(dir);
});
File backgroundDir = getCurrentSessionBackgroundDirectory();
if (!fileUtils.listFilesRecursively(backgroundDir).findAny().isPresent()) {
fileUtils.safeDelete(backgroundDir);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public Call<Void> sendSpans(List<byte[]> encodedSpans) {
}

private File createFilename(long now) {
return new File(spanStorage.provideSpanFile(), now + ".spans");
return new File(spanStorage.provideSpansDirectory(), now + ".spans");
}

static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void setup() {
void ensureFreeSpace_littleUsageEnoughFreeSpace() {
File mockFile = mock(File.class);
when(spanStorage.getTotalFileSizeInBytes()).thenReturn(10 * 1024L);
when(spanStorage.provideSpanFile()).thenReturn(mockFile);
when(spanStorage.provideSpansDirectory()).thenReturn(mockFile);
when(mockFile.getFreeSpace()).thenReturn(99L); // Disk is very full
assertFalse(limiter.ensureFreeSpace());
verify(fileUtils, never()).safeDelete(any());
Expand All @@ -66,7 +66,7 @@ void ensureFreeSpace_littleUsageEnoughFreeSpace() {
void ensureFreeSpace_littleUsageButNotEnoughFreeSpace() {
File mockFile = mock(File.class);
when(spanStorage.getTotalFileSizeInBytes()).thenReturn(10 * 1024L);
when(spanStorage.provideSpanFile()).thenReturn(mockFile);
when(spanStorage.provideSpansDirectory()).thenReturn(mockFile);
when(mockFile.getFreeSpace()).thenReturn(MAX_STORAGE_USE_BYTES * 99); // lots of room
assertTrue(limiter.ensureFreeSpace());
verify(fileUtils, never()).safeDelete(any());
Expand All @@ -75,7 +75,7 @@ void ensureFreeSpace_littleUsageButNotEnoughFreeSpace() {
@Test
void ensureFreeSpace_underLimit() {
File mockFile = mock(File.class);
when(spanStorage.provideSpanFile()).thenReturn(mockFile);
when(spanStorage.provideSpansDirectory()).thenReturn(mockFile);

when(spanStorage.getTotalFileSizeInBytes()).thenReturn(MAX_STORAGE_USE_BYTES - 1);
when(mockFile.getFreeSpace()).thenReturn(MAX_STORAGE_USE_BYTES + 1);
Expand All @@ -91,7 +91,7 @@ void ensureFreeSpace_overLimitHappyDeletion() {
File file3 = new File("newest");

File mockFile = mock(File.class);
when(spanStorage.provideSpanFile()).thenReturn(mockFile);
when(spanStorage.provideSpansDirectory()).thenReturn(mockFile);
when(spanStorage.getTotalFileSizeInBytes()).thenReturn(MAX_STORAGE_USE_BYTES + 1);
when(fileUtils.getModificationTime(file1)).thenReturn(1000L);
when(fileUtils.getModificationTime(file2)).thenReturn(1001L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ class DiskToZipkinExporterTest {
@BeforeEach
void setup() throws Exception {
Mockito.reset(SPAN_STORAGE);
when(SPAN_STORAGE.provideSpanFile()).thenReturn(spanFilesPath);
file1 = new File(SPAN_STORAGE.provideSpanFile() + File.separator + "file1.spans");
file2 = new File(SPAN_STORAGE.provideSpanFile() + File.separator + "file2.spans");
when(SPAN_STORAGE.provideSpansDirectory()).thenReturn(spanFilesPath);
file1 = new File(SPAN_STORAGE.provideSpansDirectory() + File.separator + "file1.spans");
file2 = new File(SPAN_STORAGE.provideSpansDirectory() + File.separator + "file2.spans");
imposter =
new File(SPAN_STORAGE.provideSpanFile() + File.separator + "someImposterFile.dll");
new File(
SPAN_STORAGE.provideSpansDirectory()
+ File.separator
+ "someImposterFile.dll");

when(currentNetworkProvider.refreshNetworkStatus()).thenReturn(currentNetwork);
when(currentNetwork.isOnline()).thenReturn(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,49 +21,82 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import io.opentelemetry.android.instrumentation.activity.VisibleScreenTracker;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;

public class StartTypeAwareSpanStorageTest {
class StartTypeAwareSpanStorageTest {

private final FileUtils fileUtils = mock();

private final File rootDir = new File("files/");
private final VisibleScreenTracker visibleScreenTracker = mock();
final File rootDir = new File("files/");
VisibleScreenTracker visibleScreenTracker;
FileUtils fileUtils;

private StartTypeAwareSpanStorage fileProvider;

@BeforeEach
void setup() {
visibleScreenTracker = mock();
fileUtils = mock();
when(fileUtils.getSpansDirectory(rootDir)).thenReturn(new File(rootDir, "spans"));
fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir);
}

@Test
void create_onNewId_shouldCleanOldBackgroundFiles() {
File file = mock();
ArgumentCaptor<File> fileArgumentCaptor = ArgumentCaptor.forClass(File.class);
when(file.getPath()).thenReturn("files/spans/background/123");

fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir);
fileUtils = mock(FileUtils.class);
when(fileUtils.listDirectories(any())).thenReturn(Stream.of(file));
ArgumentCaptor<File> fileArgumentCaptor = ArgumentCaptor.forClass(File.class);
when(fileUtils.listFiles(file)).thenReturn(Stream.of(file));

fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir);

verify(fileUtils, times(2)).safeDelete(fileArgumentCaptor.capture());
assertEquals(file, fileArgumentCaptor.getAllValues().get(0));
assertEquals(
fileProvider.provideSpansDirectory(), fileArgumentCaptor.getAllValues().get(1));
}

@Test
void create_cleanDoesntRemoveDirIfNotEmpty() {
String uniqueId = UUID.randomUUID().toString();
File spansDir = new File("files/spans");
File file = mock();
File file2 = mock();
fileUtils = mock(FileUtils.class);

ArgumentCaptor<File> fileArgumentCaptor = ArgumentCaptor.forClass(File.class);
when(file.getPath()).thenReturn("files/spans/background/123");

when(fileUtils.listDirectories(any())).thenReturn(Stream.of(file));
when(fileUtils.listFiles(file)).thenReturn(Stream.of(file));
when(fileUtils.listFilesRecursively(new File(spansDir, "background/" + uniqueId)))
.thenReturn(Stream.of(file2));

fileProvider =
StartTypeAwareSpanStorage.create(
visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId);

verify(fileUtils).safeDelete(fileArgumentCaptor.capture());
assertEquals(file.getPath(), fileArgumentCaptor.getValue().getPath());
assertEquals(file, fileArgumentCaptor.getAllValues().get(0));
}

@Test
void getPendingFiles_givenInBackground_shouldReturnForegoundOnlySpan() {
void getPendingFiles_givenInBackground_shouldReturnForegroundOnlySpan() {
when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn(null);
when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("unknown");
List<File> spans = fileProvider.getPendingFiles().collect(Collectors.toList());
Expand All @@ -72,7 +105,7 @@ void getPendingFiles_givenInBackground_shouldReturnForegoundOnlySpan() {

@Test
void
getPendingFiles_givenPrevouslyInBackground_shouldMoveBackgroundSpanToForegroundSpanForSending() {
getPendingFiles_givenPreviouslyInBackground_shouldMoveBackgroundSpanToForegroundSpanForSending() {
when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn("LauncherActivity");
when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("MainActivity");

Expand Down Expand Up @@ -104,7 +137,7 @@ void getSpanPath_givenInBackground_shouldReturnBackgroundSpanPath() {
when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn(null);
when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("unknown");

File path = fileProvider.provideSpanFile();
File path = fileProvider.provideSpansDirectory();

assertTrue(path.getPath().startsWith("files/spans/background/"));
}
Expand All @@ -114,7 +147,7 @@ void getSpanPath_givenInForeground_shouldReturnForegroundSpanPath() {
when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn("LauncherActivity");
when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("MainActivity");

File path = fileProvider.provideSpanFile();
File path = fileProvider.provideSpansDirectory();

assertFalse(path.getPath().startsWith("files/spans/background/"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void setup() {

@Test
void testHappyPath() throws Exception {
when(spanStorage.provideSpanFile()).thenReturn(path);
when(spanStorage.provideSpansDirectory()).thenReturn(path);

ZipkinToDiskSender sender =
ZipkinToDiskSender.builder()
Expand Down Expand Up @@ -91,7 +91,7 @@ void testEmptyListDoesNotWriteFile() {

@Test
void testWriteFails() throws Exception {
when(spanStorage.provideSpanFile()).thenReturn(path);
when(spanStorage.provideSpansDirectory()).thenReturn(path);
doThrow(new IOException("boom")).when(fileUtils).writeAsLines(finalPath, spans);

ZipkinToDiskSender sender =
Expand Down

0 comments on commit 2d493f6

Please sign in to comment.