Skip to content

Commit

Permalink
RUMM-2767 Add the 'forceNewBatch' option into the FeatureScope
Browse files Browse the repository at this point in the history
  • Loading branch information
mariusc83 committed Dec 6, 2022
1 parent d451d7e commit 404b92a
Show file tree
Hide file tree
Showing 27 changed files with 583 additions and 66 deletions.
2 changes: 1 addition & 1 deletion dd-sdk-android/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -1832,7 +1832,7 @@ interface com.datadog.android.v2.api.FeatureConfiguration
interface com.datadog.android.v2.api.FeatureEventReceiver
fun onReceive(Any)
interface com.datadog.android.v2.api.FeatureScope
fun withWriteContext((com.datadog.android.v2.api.context.DatadogContext, EventBatchWriter) -> Unit)
fun withWriteContext(Boolean = false, (com.datadog.android.v2.api.context.DatadogContext, EventBatchWriter) -> Unit)
fun sendEvent(Any)
data class com.datadog.android.v2.api.FeatureStorageConfiguration
constructor(Long, Int, Long, Long)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,18 @@ internal class SdkFeature(

// region FeatureScope

override fun withWriteContext(callback: (DatadogContext, EventBatchWriter) -> Unit) {
override fun withWriteContext(
forceNewBoolean: Boolean,
callback: (DatadogContext, EventBatchWriter) -> Unit
) {
// TODO RUMM-0000 thread safety. Thread switch happens in Storage right now. Open questions:
// * what if caller wants to have a sync operation, without thread switch
// * should context read and write be on the dedicated thread? risk - time gap between
// caller and context
val contextProvider = coreFeature.contextProvider
if (contextProvider is NoOpContextProvider) return
val context = contextProvider.context
storage.writeCurrentBatch(context) {
callback(context, it)
}
storage.writeCurrentBatch(context, forceNewBoolean) { callback(context, it) }
}

override fun sendEvent(event: Any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ internal interface FileOrchestrator {
@WorkerThread
fun getWritableFile(): File?

/**
* @return a new File with enough space to write data, or null if no space is available
* or the disk can't be written to.
*/
fun getNewWritableFile(): File?

/**
* @param excludeFiles a set of files to exclude from the readable files
* @return a File that can be read from, or null is no file is available yet.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ internal open class ConsentAwareFileOrchestrator(
return delegateOrchestrator.getWritableFile()
}

override fun getNewWritableFile(): File? {
return delegateOrchestrator.getNewWritableFile()
}

@WorkerThread
override fun getReadableFile(excludeFiles: Set<File>): File? {
return grantedOrchestrator.getReadableFile(excludeFiles)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ internal class BatchFileOrchestrator(
return reusableFile ?: createNewFile()
}

override fun getNewWritableFile(): File? {
if (!isRootDirValid()) {
return null
}

deleteObsoleteFiles()
freeSpaceIfNeeded()

return createNewFile()
}

@WorkerThread
override fun getReadableFile(excludeFiles: Set<File>): File? {
if (!isRootDirValid()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ internal class SingleFileOrchestrator(
return file
}

@WorkerThread
override fun getNewWritableFile(): File? {
return getWritableFile()
}

@WorkerThread
override fun getReadableFile(excludeFiles: Set<File>): File? {
file.parentFile?.mkdirsSafe()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ import com.datadog.android.v2.api.context.DatadogContext
interface FeatureScope {
/**
* Utility to write an event, asynchronously.
* @param forceNewBoolean if `true` forces the [EventBatchWriter] to write in a new file and
* not reuse the already existing pending data persistence file. By default it is `false`.
* @param callback an operation called with an up-to-date [DatadogContext]
* and an [EventBatchWriter]. Callback will be executed on a worker thread from I/O pool
*/
fun withWriteContext(callback: (DatadogContext, EventBatchWriter) -> Unit)
fun withWriteContext(
forceNewBoolean: Boolean = false,
callback: (DatadogContext, EventBatchWriter) -> Unit
)

/**
* Send event to a given feature. It will be sent in a synchronous way.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ internal class ConsentAwareStorage(
@WorkerThread
override fun writeCurrentBatch(
datadogContext: DatadogContext,
forceNewBatch: Boolean,
callback: (EventBatchWriter) -> Unit
) {
val orchestrator = when (datadogContext.trackingConsent) {
Expand All @@ -53,9 +54,13 @@ internal class ConsentAwareStorage(
try {
@Suppress("UnsafeThirdPartyFunctionCall") // command is never null
executorService.submit {
val batchFile = orchestrator?.getWritableFile()
val batchFile = if (forceNewBatch) {
orchestrator?.getNewWritableFile()
} else {
orchestrator?.getWritableFile()
}
val metadataFile = if (batchFile != null) {
orchestrator.getMetadataFile(batchFile)
orchestrator?.getMetadataFile(batchFile)
} else {
null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ internal interface Storage {
// request transformation).
/**
* Utility to write data, asynchronously.
* @param forceNewBatch if `true` will force the writer to start a new batch before storing the
* data. By default this flag is `false`.
* @param callback an operation to perform with a [EventBatchWriter] that will target the current
* writeable Batch
*/
@AnyThread
fun writeCurrentBatch(datadogContext: DatadogContext, callback: (EventBatchWriter) -> Unit)
fun writeCurrentBatch(
datadogContext: DatadogContext,
forceNewBatch: Boolean = false,
callback: (EventBatchWriter) -> Unit
)

/**
* Utility to read a batch, asynchronously.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions
import com.nhaarman.mockitokotlin2.verifyZeroInteractions
import com.nhaarman.mockitokotlin2.whenever
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.BoolForgery
import fr.xgouchet.elmyr.annotation.Forgery
import fr.xgouchet.elmyr.annotation.StringForgery
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
Expand Down Expand Up @@ -267,7 +268,7 @@ internal class SdkFeatureTest {
// region FeatureScope

@Test
fun `𝕄 provide write context 𝕎 withWriteContext(callback)`(
fun `𝕄 provide write context 𝕎 withWriteContext(callback) {forceNewBatch default}`(
@Forgery fakeContext: DatadogContext,
@Mock mockBatchWriter: EventBatchWriter
) {
Expand All @@ -276,13 +277,45 @@ internal class SdkFeatureTest {
val callback = mock<(DatadogContext, EventBatchWriter) -> Unit>()
whenever(coreFeature.mockInstance.contextProvider.context) doReturn fakeContext

whenever(mockStorage.writeCurrentBatch(eq(fakeContext), any())) doAnswer {
val storageCallback = it.getArgument<(EventBatchWriter) -> Unit>(1)
whenever(mockStorage.writeCurrentBatch(eq(fakeContext), eq(false), any())) doAnswer {
val storageCallback = it.getArgument<(EventBatchWriter) -> Unit>(2)
storageCallback.invoke(mockBatchWriter)
}

// When
testedFeature.withWriteContext(callback)
testedFeature.withWriteContext(callback = callback)

// Then
verify(callback).invoke(
fakeContext,
mockBatchWriter
)
}

@Test
fun `𝕄 provide write context 𝕎 withWriteContext(callback) {forceNewBatch provided}`(
@Forgery fakeContext: DatadogContext,
@Mock mockBatchWriter: EventBatchWriter,
@BoolForgery fakeForceNewBatch: Boolean
) {
// Given
testedFeature.storage = mockStorage
val callback = mock<(DatadogContext, EventBatchWriter) -> Unit>()
whenever(coreFeature.mockInstance.contextProvider.context) doReturn fakeContext

whenever(
mockStorage.writeCurrentBatch(
eq(fakeContext),
eq(fakeForceNewBatch),
any()
)
) doAnswer {
val storageCallback = it.getArgument<(EventBatchWriter) -> Unit>(2)
storageCallback.invoke(mockBatchWriter)
}

// When
testedFeature.withWriteContext(fakeForceNewBatch, callback)

// Then
verify(callback).invoke(
Expand All @@ -300,7 +333,7 @@ internal class SdkFeatureTest {
whenever(coreFeature.mockInstance.contextProvider) doReturn NoOpContextProvider()

// When
testedFeature.withWriteContext(callback)
testedFeature.withWriteContext(callback = callback)

// Then
verifyZeroInteractions(mockStorage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ internal class UploadWorkerTest {
) : Storage {
override fun writeCurrentBatch(
datadogContext: DatadogContext,
forceNewBatch: Boolean,
callback: (EventBatchWriter) -> Unit
) {
fail("we don't expect this one to be called")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,150 @@ internal class ConsentAwareFileOrchestratorTest {

// endregion

// region getNewWritableFile

@Test
fun `𝕄 return new writable file 𝕎 getWritableFile() {consent=PENDING}`(
@Forgery file: File
) {
// Given
whenever(mockPendingOrchestrator.getNewWritableFile()) doReturn file

// When
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockGrantedOrchestrator)
}

@Test
fun `𝕄 return new writable file 𝕎 getWritableFile() {consent=GRANTED then PENDING}`(
@Forgery file: File
) {
// Given
instantiateTestedOrchestrator(TrackingConsent.GRANTED)
whenever(mockPendingOrchestrator.getNewWritableFile()) doReturn file

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.GRANTED, TrackingConsent.PENDING)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockGrantedOrchestrator)
}

@Test
fun `𝕄 return new writable file 𝕎 getWritableFile() {consent=NOT_GRANTED then PENDING}`(
@Forgery file: File
) {
// Given
instantiateTestedOrchestrator(TrackingConsent.NOT_GRANTED)
whenever(mockPendingOrchestrator.getNewWritableFile()) doReturn file

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.NOT_GRANTED, TrackingConsent.PENDING)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockGrantedOrchestrator)
}

@Test
fun `𝕄 return granted writable file 𝕎 getNewWritableFile() {consent=GRANTED}`(
@Forgery file: File
) {
// Given
instantiateTestedOrchestrator(TrackingConsent.GRANTED)
whenever(mockGrantedOrchestrator.getNewWritableFile()) doReturn file

// When
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockPendingOrchestrator)
}

@Test
fun `𝕄 return granted new writable file 𝕎 getNewWritableFile(){NOT_GRANTED then GRANTED}`(
@Forgery file: File
) {
// Given
instantiateTestedOrchestrator(TrackingConsent.NOT_GRANTED)
whenever(mockGrantedOrchestrator.getNewWritableFile()) doReturn file

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.NOT_GRANTED, TrackingConsent.GRANTED)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockPendingOrchestrator)
}

@Test
fun `𝕄 return granted new writable file 𝕎 getWritableFile() {consent=PENDING then GRANTED}`(
@Forgery file: File
) {
// Given
instantiateTestedOrchestrator(TrackingConsent.PENDING)
whenever(mockGrantedOrchestrator.getNewWritableFile()) doReturn file

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.PENDING, TrackingConsent.GRANTED)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isSameAs(file)
verifyZeroInteractions(mockPendingOrchestrator)
}

@Test
fun `𝕄 return null file 𝕎 getNewWritableFile() {consent=NOT_GRANTED}`() {
// Given
instantiateTestedOrchestrator(TrackingConsent.NOT_GRANTED)

// When
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isNull()
verifyZeroInteractions(mockPendingOrchestrator, mockGrantedOrchestrator)
}

@Test
fun `𝕄 return null file 𝕎 getNewWritableFile() {consent=GRANTED then NOT_GRANTED}`() {
// Given
instantiateTestedOrchestrator(TrackingConsent.GRANTED)

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.GRANTED, TrackingConsent.NOT_GRANTED)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isNull()
verifyZeroInteractions(mockPendingOrchestrator, mockGrantedOrchestrator)
}

@Test
fun `𝕄 return null file 𝕎 getNewWritableFile() {consent=PENDING then NOT_GRANTED}`() {
// Given
instantiateTestedOrchestrator(TrackingConsent.PENDING)

// When
testedOrchestrator.onConsentUpdated(TrackingConsent.PENDING, TrackingConsent.NOT_GRANTED)
val result = testedOrchestrator.getNewWritableFile()

// Then
assertThat(result).isNull()
verifyZeroInteractions(mockPendingOrchestrator, mockGrantedOrchestrator)
}

// endregion

// region getAllFiles

@Test
Expand Down
Loading

0 comments on commit 404b92a

Please sign in to comment.