RUM-6307 Make sure ConsentAwareFileOrchestrator is thread safe

DataDog · Oct 11, 2024 · 428bee9 · 428bee9
1 parent 7ebe827
commit 428bee9
Show file tree

Hide file tree

Showing 10 changed files with 712 additions and 66 deletions.
diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt
@@ -351,7 +351,9 @@ internal class SdkFeature(
             fileMover = FileMover(internalLogger),
             internalLogger = internalLogger,
             filePersistenceConfig = filePersistenceConfig,
-            metricsDispatcher = metricsDispatcher
+            metricsDispatcher = metricsDispatcher,
+            coreFeature.trackingConsentProvider,
+            featureName
         )
     }
 

diff --git a/...oid-core/src/main/kotlin/com/datadog/android/core/internal/persistence/AbstractStorage.kt b/...oid-core/src/main/kotlin/com/datadog/android/core/internal/persistence/AbstractStorage.kt
@@ -30,7 +30,7 @@ internal class AbstractStorage(
     private val executorService: ExecutorService,
     private val internalLogger: InternalLogger,
     internal val storageConfiguration: FeatureStorageConfiguration,
-    consentProvider: ConsentProvider
+    private val consentProvider: ConsentProvider
 ) : Storage, TrackingConsentProviderCallback {
 
     private val grantedPersistenceStrategy: PersistenceStrategy by lazy {
@@ -64,13 +64,8 @@ internal class AbstractStorage(
         forceNewBatch: Boolean,
         callback: (EventBatchWriter) -> Unit
     ) {
-        val strategy = when (datadogContext.trackingConsent) {
-            TrackingConsent.GRANTED -> grantedPersistenceStrategy
-            TrackingConsent.PENDING -> pendingPersistenceStrategy
-            TrackingConsent.NOT_GRANTED -> notGrantedPersistenceStrategy
-        }
-
         executorService.submitSafe("Data write", internalLogger) {
+            val strategy = resolvePersistenceStrategy()
             val writer = object : EventBatchWriter {
                 @WorkerThread
                 override fun currentMetadata(): ByteArray? {
@@ -86,6 +81,14 @@ internal class AbstractStorage(
         }
     }
 
+    @WorkerThread
+    private fun resolvePersistenceStrategy() =
+        when (consentProvider.getConsent()) {
+            TrackingConsent.GRANTED -> grantedPersistenceStrategy
+            TrackingConsent.PENDING -> pendingPersistenceStrategy
+            TrackingConsent.NOT_GRANTED -> notGrantedPersistenceStrategy
+        }
+
     @WorkerThread
     override fun readNextBatch(): BatchData? {
         return grantedPersistenceStrategy.lockAndReadNext()?.let {

diff --git a/...core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt b/...core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt
@@ -19,6 +19,7 @@ import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig
 import com.datadog.android.core.internal.persistence.file.FileReaderWriter
 import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderWriter
 import com.datadog.android.core.internal.persistence.file.existsSafe
+import com.datadog.android.core.internal.privacy.ConsentProvider
 import com.datadog.android.core.internal.utils.submitSafe
 import com.datadog.android.core.metrics.MethodCallSamplingRate
 import com.datadog.android.core.metrics.TelemetryMetricType
@@ -36,7 +37,9 @@ internal class ConsentAwareStorage(
     private val fileMover: FileMover,
     private val internalLogger: InternalLogger,
     internal val filePersistenceConfig: FilePersistenceConfig,
-    private val metricsDispatcher: MetricsDispatcher
+    private val metricsDispatcher: MetricsDispatcher,
+    private val consentProvider: ConsentProvider,
+    private val featureName: String
 ) : Storage {
 
     /**
@@ -53,20 +56,14 @@ internal class ConsentAwareStorage(
         forceNewBatch: Boolean,
         callback: (EventBatchWriter) -> Unit
     ) {
-        val orchestrator = when (datadogContext.trackingConsent) {
-            TrackingConsent.GRANTED -> grantedOrchestrator
-            TrackingConsent.PENDING -> pendingOrchestrator
-            TrackingConsent.NOT_GRANTED -> null
-        }
-
         val metric = internalLogger.startPerformanceMeasure(
             callerClass = ConsentAwareStorage::class.java.name,
             metric = TelemetryMetricType.MethodCalled,
             samplingRate = MethodCallSamplingRate.RARE.rate,
-            operationName = "writeCurrentBatch[${orchestrator?.getRootDirName()}]"
+            operationName = "writeCurrentBatch[$featureName]"
         )
-
         executorService.submitSafe("Data write", internalLogger) {
+            val orchestrator = resolveOrchestrator()
             synchronized(writeLock) {
                 val batchFile = orchestrator?.getWritableFile(forceNewBatch)
                 val metadataFile = if (batchFile != null) {
@@ -153,6 +150,16 @@ internal class ConsentAwareStorage(
         }
     }
 
+    @WorkerThread
+    private fun resolveOrchestrator(): FileOrchestrator? {
+        val consent = consentProvider.getConsent()
+        return when (consent) {
+            TrackingConsent.GRANTED -> grantedOrchestrator
+            TrackingConsent.PENDING -> pendingOrchestrator
+            TrackingConsent.NOT_GRANTED -> null
+        }
+    }
+
     @WorkerThread
     private fun deleteBatch(batch: Batch, reason: RemovalReason) {
         deleteBatch(batch.file, batch.metaFile, reason)

diff --git a/...m/datadog/android/core/internal/persistence/file/advanced/ConsentAwareFileOrchestrator.kt b/...m/datadog/android/core/internal/persistence/file/advanced/ConsentAwareFileOrchestrator.kt
@@ -27,6 +27,7 @@ internal open class ConsentAwareFileOrchestrator(
     internal val internalLogger: InternalLogger
 ) : FileOrchestrator, TrackingConsentProviderCallback {
 
+    @Volatile
     private lateinit var delegateOrchestrator: FileOrchestrator
 
     init {

diff --git a/...core/src/test/kotlin/com/datadog/android/core/internal/persistence/AbstractStorageTest.kt b/...core/src/test/kotlin/com/datadog/android/core/internal/persistence/AbstractStorageTest.kt
@@ -109,7 +109,7 @@ internal class AbstractStorageTest {
         @Forgery fakeBatchEvent: RawBatchEvent
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.GRANTED
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         whenever(mockGrantedPersistenceStrategy.write(any(), anyOrNull(), any())) doReturn fakeResult
         var result: Boolean? = null
@@ -118,7 +118,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(result).isEqualTo(fakeResult)
@@ -138,7 +138,7 @@ internal class AbstractStorageTest {
         @StringForgery fakeBatchMetadata: String
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.GRANTED
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         val batchMetadata = fakeBatchMetadata.toByteArray()
         whenever(mockGrantedPersistenceStrategy.write(any(), anyOrNull(), any())) doReturn fakeResult
@@ -148,7 +148,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(result).isEqualTo(fakeResult)
@@ -165,7 +165,8 @@ internal class AbstractStorageTest {
         @BoolForgery forceNewBatch: Boolean
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.GRANTED
+        whenever(mockGrantedPersistenceStrategy.currentMetadata()) doReturn null
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         whenever(mockGrantedPersistenceStrategy.currentMetadata()) doReturn null
         var resultMetadata: ByteArray? = null
@@ -174,7 +175,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(resultMetadata).isNull()
@@ -192,7 +193,7 @@ internal class AbstractStorageTest {
         @StringForgery fakeBatchMetadata: String
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.GRANTED
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         val batchMetadata = fakeBatchMetadata.toByteArray()
         whenever(mockGrantedPersistenceStrategy.currentMetadata()) doReturn batchMetadata
@@ -202,7 +203,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(resultMetadata).isEqualTo(batchMetadata)
@@ -221,7 +222,7 @@ internal class AbstractStorageTest {
         @Forgery fakeBatchEvent: RawBatchEvent
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.PENDING)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.PENDING
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         whenever(mockPendingPersistenceStrategy.write(any(), anyOrNull(), any())) doReturn fakeResult
         var result: Boolean? = null
@@ -230,7 +231,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(result).isEqualTo(fakeResult)
@@ -250,7 +251,7 @@ internal class AbstractStorageTest {
         @StringForgery fakeBatchMetadata: String
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.PENDING)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.PENDING
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         val batchMetadata = fakeBatchMetadata.toByteArray()
         whenever(mockPendingPersistenceStrategy.write(any(), anyOrNull(), any())) doReturn fakeResult
@@ -260,7 +261,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(result).isEqualTo(fakeResult)
@@ -277,7 +278,7 @@ internal class AbstractStorageTest {
         @BoolForgery forceNewBatch: Boolean
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.PENDING)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.PENDING
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         var resultMetadata: ByteArray? = null
         whenever(mockWriteCallback.invoke(any())) doAnswer {
@@ -286,7 +287,7 @@ internal class AbstractStorageTest {
         whenever(mockPendingPersistenceStrategy.currentMetadata()) doReturn null
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(resultMetadata).isNull()
@@ -304,7 +305,7 @@ internal class AbstractStorageTest {
         @StringForgery fakeBatchMetadata: String
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.PENDING)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.PENDING
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         val batchMetadata = fakeBatchMetadata.toByteArray()
         var resultMetadata: ByteArray? = null
@@ -314,7 +315,7 @@ internal class AbstractStorageTest {
         whenever(mockPendingPersistenceStrategy.currentMetadata()) doReturn batchMetadata
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(resultMetadata).isEqualTo(batchMetadata)
@@ -333,7 +334,7 @@ internal class AbstractStorageTest {
         @StringForgery fakeBatchMetadata: String
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.NOT_GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.NOT_GRANTED
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         val batchMetadata = fakeBatchMetadata.toByteArray()
         var result: Boolean? = null
@@ -342,7 +343,7 @@ internal class AbstractStorageTest {
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(result).isFalse()
@@ -358,15 +359,15 @@ internal class AbstractStorageTest {
         @BoolForgery forceNewBatch: Boolean
     ) {
         // Given
-        val sdkContext = fakeDatadogContext.copy(trackingConsent = TrackingConsent.NOT_GRANTED)
+        whenever(mockConsentProvider.getConsent()) doReturn TrackingConsent.NOT_GRANTED
         val mockWriteCallback = mock<(EventBatchWriter) -> Unit>()
         var resultMetadata: ByteArray? = null
         whenever(mockWriteCallback.invoke(any())) doAnswer {
             resultMetadata = (it.getArgument(0) as? EventBatchWriter)?.currentMetadata()
         }
 
         // When
-        testedStorage.writeCurrentBatch(sdkContext, forceNewBatch, mockWriteCallback)
+        testedStorage.writeCurrentBatch(fakeDatadogContext, forceNewBatch, mockWriteCallback)
 
         // Then
         assertThat(resultMetadata).isNull()