RUM-16305: Refactor checking how much space is left in the current batch

Author: aleksandr-gringauz

dd-sdk-android-core/api/apiSurface

@@ -200,7 +200,6 @@ interface com.datadog.android.api.net.RequestFactory
 interface com.datadog.android.api.storage.DataWriter<T>
   fun write(EventBatchWriter, T, EventType): Boolean
 interface com.datadog.android.api.storage.EventBatchWriter
-  fun currentMetadata(): ByteArray?
   fun write(RawBatchEvent, ByteArray?, EventType): Boolean
 enum com.datadog.android.api.storage.EventType
   - DEFAULT

dd-sdk-android-core/api/dd-sdk-android-core.api

@@ -577,7 +577,6 @@ public abstract interface class com/datadog/android/api/storage/DataWriter {
 }
 
 public abstract interface class com/datadog/android/api/storage/EventBatchWriter {
-	public abstract fun currentMetadata ()[B
 	public abstract fun write (Lcom/datadog/android/api/storage/RawBatchEvent;[BLcom/datadog/android/api/storage/EventType;)Z
 }
 

dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/storage/EventBatchWriter.kt

@@ -9,16 +9,10 @@ package com.datadog.android.api.storage
 import androidx.annotation.WorkerThread
 
 /**
- * Writer allowing [FeatureScope] to write events in the storage exposing current batch metadata.
+ * Writer allowing [FeatureScope] to write events in the storage.
  */
 interface EventBatchWriter {
 
-    /**
-     * @return the metadata of the current writeable batch
-     */
-    @WorkerThread
-    fun currentMetadata(): ByteArray?
-
     /**
      * Writes the content of the event to the current available batch.
      * @param event the event to write (content + metadata)

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt

@@ -40,7 +40,6 @@ import com.datadog.android.core.internal.net.info.NetworkInfoProvider
 import com.datadog.android.core.internal.net.info.NoOpNetworkInfoProvider
 import com.datadog.android.core.internal.persistence.JsonObjectDeserializer
 import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig
-import com.datadog.android.core.internal.persistence.file.FileWriter
 import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderWriter
 import com.datadog.android.core.internal.persistence.file.deleteSafe
 import com.datadog.android.core.internal.persistence.file.existsSafe
@@ -231,7 +230,7 @@ internal class CoreFeature(
 
     @get:WorkerThread
     private val lastViewEventFile: File by lazy { File(storageDir, LAST_RUM_VIEW_EVENT_FILE_NAME) }
-    private val lastViewEventFileWriter: FileWriter<RawBatchEvent> by lazy {
+    private val lastViewEventFileWriter: BatchFileReaderWriter by lazy {
         BatchFileReaderWriter.create(
             internalLogger = internalLogger,
             encryption = localDataEncryption
@@ -387,7 +386,8 @@ internal class CoreFeature(
 
     @WorkerThread
     internal fun writeLastViewEvent(data: ByteArray) {
-        lastViewEventFileWriter.writeData(lastViewEventFile, RawBatchEvent(data), false)
+        val serialized = lastViewEventFileWriter.serializeToBytes(RawBatchEvent(data = data)) ?: return
+        lastViewEventFileWriter.writeBinaryData(lastViewEventFile, serialized, false)
     }
 
     @WorkerThread

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/AbstractStorage.kt

@@ -67,11 +67,6 @@ internal class AbstractStorage(
     ): EventWriteScope {
         val strategy = resolvePersistenceStrategy(datadogContext)
         val writer = object : EventBatchWriter {
-            @WorkerThread
-            override fun currentMetadata(): ByteArray? {
-                return strategy.currentMetadata()
-            }
-
             @WorkerThread
             override fun write(event: RawBatchEvent, batchMetadata: ByteArray?, eventType: EventType): Boolean {
                 return strategy.write(event, batchMetadata, eventType)

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt

@@ -14,51 +14,38 @@ import com.datadog.android.api.storage.RawBatchEvent
 import com.datadog.android.core.internal.persistence.file.FileOrchestrator
 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.FileWriter
-import com.datadog.android.core.internal.persistence.file.existsSafe
+import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderWriter
 import java.io.File
 import java.util.Locale
 
 internal class FileEventBatchWriter(
     private val fileOrchestrator: FileOrchestrator,
-    private val eventsWriter: FileWriter<RawBatchEvent>,
+    private val eventsWriter: BatchFileReaderWriter,
     private val metadataReaderWriter: FileReaderWriter,
     private val filePersistenceConfig: FilePersistenceConfig,
     private val batchWriteEventListener: BatchWriteEventListener,
     private val internalLogger: InternalLogger
 ) : EventBatchWriter {
 
-    @get:WorkerThread
-    private val batchFile: File? by lazy {
-        @Suppress("ThreadSafety") // called in the worker context
-        fileOrchestrator.getWritableFile()
-    }
-
-    @get:WorkerThread
-    private val metadataFile: File?
-        get() = batchFile?.let {
-            @Suppress("ThreadSafety") // called in the worker context
-            fileOrchestrator.getMetadataFile(it)
-        }
-
-    @WorkerThread
-    override fun currentMetadata(): ByteArray? {
-        return with(metadataFile) {
-            if (this == null || !existsSafe(internalLogger)) {
-                null
-            } else {
-                metadataReaderWriter.readData(this)
-            }
-        }
-    }
-
     @WorkerThread
     override fun write(
         event: RawBatchEvent,
         batchMetadata: ByteArray?,
         eventType: EventType
     ): Boolean {
-        val (batchFile, metadataFile) = batchFile to metadataFile
+        // prevent useless operation for empty event
+        if (event.data.isEmpty()) {
+            return true
+        }
+        if (!checkEventSize(event.data.size)) {
+            return false
+        }
+
+        // Serialize once (TLV-wrapped, encrypted if configured) so the exact on-disk size is known
+        // before asking the orchestrator for a file with enough room to hold it.
+        val serializedEvent = eventsWriter.serializeToBytes(event) ?: return false
+
+        val batchFile = fileOrchestrator.getWritableFile(serializedEvent.size.toLong())
         if (batchFile == null) {
             internalLogger.log(
                 InternalLogger.Level.ERROR,
@@ -68,12 +55,9 @@ internal class FileEventBatchWriter(
             return false
         }
 
-        // prevent useless operation for empty event
-        return if (event.data.isEmpty()) {
-            true
-        } else if (!checkEventSize(event.data.size)) {
-            false
-        } else if (eventsWriter.writeData(batchFile, event, true)) {
+        val metadataFile = fileOrchestrator.getMetadataFile(batchFile)
+
+        return if (eventsWriter.writeBinaryData(batchFile, serializedEvent, true)) {
             batchWriteEventListener.onWriteEvent(event.data.size.toLong())
             if (batchMetadata?.isNotEmpty() == true && metadataFile != null) {
                 writeBatchMetadata(metadataFile, batchMetadata)

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/NoOpEventBatchWriter.kt

@@ -12,10 +12,6 @@ import com.datadog.android.api.storage.RawBatchEvent
 
 internal class NoOpEventBatchWriter : EventBatchWriter {
 
-    override fun currentMetadata(): ByteArray? {
-        return null
-    }
-
     override fun write(
         event: RawBatchEvent,
         batchMetadata: ByteArray?,

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FileOrchestrator.kt

@@ -21,11 +21,14 @@ import java.io.File
 internal interface FileOrchestrator {
 
     /**
+     * @param eventSize the size (in bytes) of the event about to be written, used to decide
+     * whether an existing batch file has enough room to accommodate it without exceeding the
+     * maximum batch size.
      * @return a File with enough space to write data, or null if no space is available
      * or the disk can't be written to.
      */
     @WorkerThread
-    fun getWritableFile(): File?
+    fun getWritableFile(eventSize: Long): File?
 
     /**
      * @param excludeFiles a set of files to exclude from the readable files

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/ConsentAwareFileOrchestrator.kt

@@ -39,8 +39,8 @@ internal open class ConsentAwareFileOrchestrator(
     // region FileOrchestrator
 
     @WorkerThread
-    override fun getWritableFile(): File? {
-        return delegateOrchestrator.getWritableFile()
+    override fun getWritableFile(eventSize: Long): File? {
+        return delegateOrchestrator.getWritableFile(eventSize)
     }
 
     @WorkerThread

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileOrchestrator.kt

@@ -57,7 +57,7 @@ internal class BatchFileOrchestrator(
     // region FileOrchestrator
 
     @WorkerThread
-    override fun getWritableFile(): File? {
+    override fun getWritableFile(eventSize: Long): File? {
         if (!isRootDirValid()) {
             return null
         }
@@ -69,7 +69,7 @@ internal class BatchFileOrchestrator(
             lastCleanupTimestamp = timeProvider.getDeviceTimestampMillis()
         }
 
-        return getReusableWritableFile() ?: createNewFile()
+        return getReusableWritableFile(eventSize) ?: createNewFile()
     }
 
     @WorkerThread
@@ -221,7 +221,7 @@ internal class BatchFileOrchestrator(
     }
 
     @Suppress("ReturnCount")
-    private fun getReusableWritableFile(): File? {
+    private fun getReusableWritableFile(eventSize: Long): File? {
         val files = listBatchFiles()
         val lastFile = files.latestBatchFile ?: return null
 
@@ -237,7 +237,7 @@ internal class BatchFileOrchestrator(
         }
 
         val isRecentEnough = isFileRecent(lastFile, recentWriteDelayMs)
-        val hasRoomForMore = lastFile.lengthSafe(internalLogger) < config.maxBatchSize - config.maxItemSize
+        val hasRoomForMore = lastFile.lengthSafe(internalLogger) + eventSize <= config.maxBatchSize
         val hasSlotForMore = (lastKnownFileItemCount < config.maxItemsPerBatch)
 
         return if (isRecentEnough && hasRoomForMore && hasSlotForMore) {