Skip to content

Commit

Permalink
refactor: extract location of storage directory to one place
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed Nov 29, 2023
1 parent 928a6d0 commit 15643da
Show file tree
Hide file tree
Showing 15 changed files with 77 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,7 @@ private void startImpl(@NonNull Context context,
nativeModule,
sessionModule,
anrModule,
dataContainerModule,
coreModule
dataContainerModule
);

loadCrashVerifier(crashModule, nonNullWorkerThreadModule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import java.net.URI
*/
internal class ApiResponseCache @JvmOverloads constructor(
private val serializer: EmbraceSerializer,
cacheDirProvider: () -> File,
storageDirProvider: () -> File,
private val logger: InternalEmbraceLogger = InternalStaticEmbraceLogger.logger
) : Closeable {

Expand All @@ -35,15 +35,15 @@ internal class ApiResponseCache @JvmOverloads constructor(

@Volatile
private var cache: HttpResponseCache? = null
private val cacheDir: File by lazy { cacheDirProvider() }
private val storageDir: File by lazy { storageDirProvider() }
private val lock = Object()

private fun initializeIfNeeded() {
if (cache == null) {
synchronized(lock) {
if (cache == null) {
cache = try {
HttpResponseCache.install(cacheDir, MAX_CACHE_SIZE_BYTES)
HttpResponseCache.install(storageDir, MAX_CACHE_SIZE_BYTES)
} catch (exc: IOException) {
logger.logWarning("Failed to initialize HTTP cache.", exc)
null
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.embrace.android.embracesdk.comms.delivery

import android.content.Context
import com.google.gson.stream.JsonReader
import io.embrace.android.embracesdk.internal.EmbraceSerializer
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
Expand All @@ -12,18 +11,17 @@ import java.util.regex.Pattern
* Handles the reading and writing of objects from the app's cache.
*/
internal class EmbraceCacheService(
context: Context,
fileProvider: Lazy<File>,
private val serializer: EmbraceSerializer,
logger: InternalEmbraceLogger
private val logger: InternalEmbraceLogger
) : CacheService {

private val cacheDir: Lazy<File>
private val logger: InternalEmbraceLogger
private val storageDir: File by fileProvider

override fun cacheBytes(name: String, bytes: ByteArray?) {
logger.logDeveloper(TAG, "Attempting to write bytes to $name")
if (bytes != null) {
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)
try {
file.writeBytes(bytes)
logger.logDeveloper(TAG, "Bytes cached")
Expand All @@ -37,7 +35,7 @@ internal class EmbraceCacheService(

override fun loadBytes(name: String): ByteArray? {
logger.logDeveloper(TAG, "Attempting to read bytes from $name")
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)
try {
return file.readBytes()
} catch (ex: FileNotFoundException) {
Expand All @@ -61,7 +59,7 @@ internal class EmbraceCacheService(
*/
override fun <T> cacheObject(name: String, objectToCache: T, clazz: Class<T>) {
logger.logDeveloper(TAG, "Attempting to cache object: $name")
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)
try {
file.bufferedWriter().use {
serializer.writeToFile(objectToCache, clazz, it)
Expand All @@ -72,7 +70,7 @@ internal class EmbraceCacheService(
}

override fun <T> loadObject(name: String, clazz: Class<T>): T? {
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)
try {
file.bufferedReader().use { bufferedReader ->
JsonReader(bufferedReader).use { jsonreader ->
Expand All @@ -95,7 +93,7 @@ internal class EmbraceCacheService(

override fun deleteFile(name: String): Boolean {
logger.logDeveloper("EmbraceCacheService", "Attempting to delete file from cache: $name")
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)

Check warning on line 96 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceCacheService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceCacheService.kt#L96

Added line #L96 was not covered by tests
try {
return file.delete()
} catch (ex: Exception) {
Expand All @@ -106,7 +104,7 @@ internal class EmbraceCacheService(

override fun deleteObject(name: String): Boolean {
logger.logDeveloper("EmbraceCacheService", "Attempting to delete: $name")
val file = File(cacheDir.value, EMBRACE_PREFIX + name)
val file = File(storageDir, EMBRACE_PREFIX + name)
try {
return file.delete()
} catch (ex: Exception) {
Expand All @@ -119,7 +117,7 @@ internal class EmbraceCacheService(
logger.logDeveloper("EmbraceCacheService", "Attempting to delete objects by regex: $regex")
val pattern = Pattern.compile(regex)
var result = false
val filesInCache = cacheDir.value.listFiles()
val filesInCache = storageDir.listFiles()
if (filesInCache != null) {
for (cache in filesInCache) {
if (pattern.matcher(cache.name).find()) {
Expand All @@ -139,20 +137,18 @@ internal class EmbraceCacheService(
}

override fun moveObject(src: String, dst: String): Boolean {
val cacheDir = cacheDir.value
val srcFile = File(cacheDir, EMBRACE_PREFIX + src)
val srcFile = File(storageDir, EMBRACE_PREFIX + src)
if (!srcFile.exists()) {
logger.logDeveloper("EmbraceCacheService", "Source file doesn't exist: $src")
return false
}
val dstFile = File(cacheDir, EMBRACE_PREFIX + dst)
val dstFile = File(storageDir, EMBRACE_PREFIX + dst)
logger.logDeveloper("EmbraceCacheService", "Object moved from $src to $dst")
return srcFile.renameTo(dstFile)
}

override fun listFilenamesByPrefix(prefix: String): List<String>? {
val cacheDir = cacheDir.value
return cacheDir.listFiles { file ->
return storageDir.listFiles { file ->
file.name.startsWith(EMBRACE_PREFIX + prefix)
}?.map { file -> file.name.substring(EMBRACE_PREFIX.length) }
}
Expand All @@ -161,9 +157,4 @@ internal class EmbraceCacheService(
private const val EMBRACE_PREFIX = "emb_"
private const val TAG = "EmbraceCacheService"
}

init {
this.logger = logger
cacheDir = lazy { context.cacheDir }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ internal class CrashModuleImpl(
nativeModule: NativeModule,
sessionModule: SessionModule,
anrModule: AnrModule,
dataContainerModule: DataContainerModule,
coreModule: CoreModule
dataContainerModule: DataContainerModule
) : CrashModule {

private val crashMarker: CrashFileMarker by singleton {
val markerFile = lazy { File(coreModule.context.cacheDir.path, CrashFileMarker.CRASH_MARKER_FILE_NAME) }
val markerFile = lazy {
val dir = essentialServiceModule.storageDirectory.value
File(dir, CrashFileMarker.CRASH_MARKER_FILE_NAME)
}
CrashFileMarker(markerFile)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ internal interface EssentialServiceModule {
val cacheService: CacheService
val deliveryCacheManager: DeliveryCacheManager
val deliveryRetryManager: DeliveryRetryManager
val storageDirectory: Lazy<File>
}

internal class EssentialServiceModuleImpl(
Expand Down Expand Up @@ -229,7 +230,7 @@ internal class EssentialServiceModuleImpl(
override val cache by singleton {
ApiResponseCache(
coreModule.jsonSerializer,
{ File(coreModule.context.cacheDir, "emb_config_cache") }
{ File(storageDirectory.value, "emb_config_cache") }
)
}

Expand Down Expand Up @@ -261,7 +262,7 @@ internal class EssentialServiceModuleImpl(
}

override val cacheService: CacheService by singleton {
EmbraceCacheService(coreModule.context, coreModule.jsonSerializer, coreModule.logger)
EmbraceCacheService(storageDirectory, coreModule.jsonSerializer, coreModule.logger)
}

override val deliveryCacheManager: DeliveryCacheManager by singleton {
Expand Down Expand Up @@ -304,6 +305,10 @@ internal class EssentialServiceModuleImpl(
coreModule.logger
)
}

override val storageDirectory: Lazy<File> = lazy {
coreModule.context.cacheDir
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
Expand Down Expand Up @@ -118,7 +117,7 @@ class EmbraceNdkService implements NdkService, ProcessStateListener {

private String unityCrashId;

private final Lazy<File> cacheDir;
private final Lazy<File> storageDir;

private final ExecutorService cleanCacheExecutorService;
private final ExecutorService ndkStartupExecutorService;
Expand All @@ -132,6 +131,7 @@ class EmbraceNdkService implements NdkService, ProcessStateListener {

EmbraceNdkService(
@NonNull Context context,
@NonNull Lazy<File> storageDir,
@NonNull MetadataService metadataService,
@NonNull ProcessStateService processStateService,
@NonNull ConfigService configService,
Expand All @@ -148,6 +148,7 @@ class EmbraceNdkService implements NdkService, ProcessStateListener {
@NonNull DeviceArchitecture deviceArchitecture) {

this.context = context;
this.storageDir = storageDir;
this.metadataService = metadataService;
this.configService = configService;
this.deliveryService = deliveryService;
Expand All @@ -168,7 +169,6 @@ class EmbraceNdkService implements NdkService, ProcessStateListener {
return null;
});

this.cacheDir = LazyKt.lazy(context::getCacheDir);
this.cleanCacheExecutorService = cleanCacheExecutorService;
this.ndkStartupExecutorService = ndkStartupExecutorService;

Expand Down Expand Up @@ -305,7 +305,7 @@ private boolean shouldIgnoreOverriddenHandler(@NonNull String culprit) {
}

protected void createCrashReportDirectory() {
String directory = cacheDir.getValue() + "/" + NATIVE_CRASH_FILE_FOLDER;
String directory = storageDir.getValue() + "/" + NATIVE_CRASH_FILE_FOLDER;

Check warning on line 308 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.java#L308

Added line #L308 was not covered by tests
File directoryFile = new File(directory);

if (directoryFile.exists()) {
Expand All @@ -318,8 +318,8 @@ protected void createCrashReportDirectory() {
}

protected void installSignals() {
String reportBasePath = cacheDir.getValue().getAbsolutePath() + "/" + NATIVE_CRASH_FILE_FOLDER;
String markerFilePath = cacheDir.getValue().getAbsolutePath() + "/" + CrashFileMarker.CRASH_MARKER_FILE_NAME;
String reportBasePath = storageDir.getValue().getAbsolutePath() + "/" + NATIVE_CRASH_FILE_FOLDER;
String markerFilePath = storageDir.getValue().getAbsolutePath() + "/" + CrashFileMarker.CRASH_MARKER_FILE_NAME;
logger.logDeveloper("EmbraceNDKService", "Creating report path at " + reportBasePath);

String nativeCrashId;
Expand Down Expand Up @@ -516,7 +516,7 @@ private NativeSymbols getNativeSymbols() {

private File[] getNativeFiles(FilenameFilter filter) {
File[] matchingFiles = null;
final File[] files = cacheDir.getValue().listFiles();
final File[] files = storageDir.getValue().listFiles();

Check warning on line 519 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.java#L519

Added line #L519 was not covered by tests

if (files == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.embrace.android.embracesdk.ndk

import android.content.Context
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
import io.embrace.android.embracesdk.payload.NativeCrashData
import java.io.File
Expand All @@ -16,10 +15,12 @@ private const val NATIVE_CRASH_MAP_FILE_SUFFIX = ".map"
* Encapsulates the logic of managing Files to get, sort and or delete them
*/
internal class EmbraceNdkServiceRepository(
private val context: Context,
fileProvider: Lazy<File>,
private val logger: InternalEmbraceLogger
) {

private val storageDir by fileProvider

fun sortNativeCrashes(byOldest: Boolean): List<File> {
val nativeCrashFiles: Array<File>? = getNativeCrashFiles()
val nativeCrashList: MutableList<File> = mutableListOf()
Expand Down Expand Up @@ -58,7 +59,7 @@ internal class EmbraceNdkServiceRepository(

private fun getNativeFiles(filter: FilenameFilter): Array<File>? {
var matchingFiles: Array<File>? = null
val files: Array<File> = context.cacheDir.listFiles() ?: return null
val files: Array<File> = storageDir.listFiles() ?: return null
for (cached in files) {
if (cached.isDirectory && cached.name == NATIVE_CRASH_FILE_FOLDER) {
matchingFiles = cached.listFiles(filter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal class NativeModuleImpl(
override val ndkService: NdkService by singleton {
EmbraceNdkService(
coreModule.context,
essentialServiceModule.storageDirectory,
essentialServiceModule.metadataService,
essentialServiceModule.processStateService,
essentialServiceModule.configService,
Expand Down Expand Up @@ -73,7 +74,7 @@ internal class NativeModuleImpl(

private val embraceNdkServiceRepository by singleton {
EmbraceNdkServiceRepository(
coreModule.context,
essentialServiceModule.storageDirectory,
coreModule.logger
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.embrace.android.embracesdk

import android.content.Context
import io.embrace.android.embracesdk.comms.api.ApiRequest
import io.embrace.android.embracesdk.comms.api.EmbraceUrl
import io.embrace.android.embracesdk.comms.delivery.CacheService
Expand All @@ -10,8 +9,6 @@ import io.embrace.android.embracesdk.comms.delivery.EmbraceCacheService
import io.embrace.android.embracesdk.internal.EmbraceSerializer
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
import io.embrace.android.embracesdk.network.http.HttpMethod
import io.mockk.every
import io.mockk.mockk
import org.junit.Assert.assertArrayEquals
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
Expand All @@ -25,18 +22,15 @@ import java.nio.file.Files

internal class EmbraceCacheServiceTest {

private lateinit var context: Context
private lateinit var service: CacheService
private lateinit var dir: File

@Before
fun setUp() {
context = mockk()

dir = Files.createTempDirectory("tmpDirPrefix").toFile()
every { context.cacheDir } returns dir
service = EmbraceCacheService(
context,
lazy { dir },
EmbraceSerializer(),
InternalEmbraceLogger()
)
Expand Down Expand Up @@ -151,7 +145,11 @@ internal class EmbraceCacheServiceTest {
// In order to force File.listFiles() to return null, we make the File not to be a directory
val myDir = File("no_directory_file")
myDir.createNewFile()
every { context.cacheDir } returns myDir
service = EmbraceCacheService(
lazy { myDir },
EmbraceSerializer(),
InternalEmbraceLogger()
)

val deleted = service.deleteObjectsByRegex(".*object.*")
assertFalse(deleted)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.embrace.android.embracesdk.worker.WorkerThreadModuleImpl
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkStatic
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
Expand Down Expand Up @@ -55,6 +56,7 @@ internal class EssentialServiceModuleImplTest {
assertNotNull(module.activityLifecycleTracker)
assertTrue(module.configService is EmbraceConfigService)
assertTrue(module.gatingService is EmbraceGatingService)
assertEquals(module.storageDirectory.value, coreModule.context.cacheDir)
}

@Test
Expand Down
Loading

0 comments on commit 15643da

Please sign in to comment.