BatchOperation: set "yield allowed" after every 500 operations

bitfireAT · Nov 22, 2024 · de5368d · de5368d
1 parent 1accbfd
commit de5368d
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 20 deletions.
diff --git a/...bitfire/ical4android/AbstractTasksTest.kt → ...cal4android/DmfsStyleProvidersTaskTest.kt b/...bitfire/ical4android/AbstractTasksTest.kt → ...cal4android/DmfsStyleProvidersTaskTest.kt
@@ -16,7 +16,7 @@ import java.util.logging.Logger
 
 @RunWith(Parameterized::class)
 
-abstract class AbstractTasksTest(
+abstract class DmfsStyleProvidersTaskTest(
     val providerName: TaskProvider.ProviderName
 ) {
 

diff --git a/lib/src/androidTest/kotlin/at/bitfire/ical4android/DmfsTaskListTest.kt b/lib/src/androidTest/kotlin/at/bitfire/ical4android/DmfsTaskListTest.kt
@@ -21,7 +21,7 @@ import org.junit.Assert.assertTrue
 import org.junit.Test
 
 class DmfsTaskListTest(providerName: TaskProvider.ProviderName):
-    AbstractTasksTest(providerName) {
+    DmfsStyleProvidersTaskTest(providerName) {
 
     private val testAccount = Account("AndroidTaskListTest", TaskContract.LOCAL_ACCOUNT_TYPE)
 

diff --git a/lib/src/androidTest/kotlin/at/bitfire/ical4android/DmfsTaskTest.kt b/lib/src/androidTest/kotlin/at/bitfire/ical4android/DmfsTaskTest.kt
@@ -9,13 +9,13 @@ import android.content.ContentUris
 import android.content.ContentValues
 import android.database.DatabaseUtils
 import android.net.Uri
-import androidx.test.filters.MediumTest
 import at.bitfire.ical4android.impl.TestTask
 import at.bitfire.ical4android.impl.TestTaskList
 import at.bitfire.ical4android.util.DateUtils
 import net.fortuna.ical4j.model.Date
 import net.fortuna.ical4j.model.DateList
 import net.fortuna.ical4j.model.DateTime
+import net.fortuna.ical4j.model.component.VAlarm
 import net.fortuna.ical4j.model.parameter.Email
 import net.fortuna.ical4j.model.parameter.RelType
 import net.fortuna.ical4j.model.parameter.TzId
@@ -52,8 +52,8 @@ import org.junit.Test
 import java.time.ZoneId
 
 class DmfsTaskTest(
-        providerName: TaskProvider.ProviderName
-): AbstractTasksTest(providerName) {
+    providerName: TaskProvider.ProviderName
+): DmfsStyleProvidersTaskTest(providerName) {
 
     private val tzVienna = DateUtils.ical4jTimeZone("Europe/Vienna")!!
     private val tzChicago = DateUtils.ical4jTimeZone("America/Chicago")!!
@@ -643,7 +643,6 @@ class DmfsTaskTest(
     }
 
 
-    @MediumTest
     @Test
     fun testAddTask() {
         // build and write event to calendar provider
@@ -693,7 +692,6 @@ class DmfsTaskTest(
         }
     }
 
-    @MediumTest
     @Test(expected = CalendarStorageException::class)
     fun testAddTaskWithInvalidDue() {
         val task = Task()
@@ -705,7 +703,21 @@ class DmfsTaskTest(
         TestTask(taskList!!, task).add()
     }
 
-    @MediumTest
+    @Test
+    fun testAddTaskWithManyAlarms() {
+        val task = Task()
+        task.uid = "TaskWithManyAlarms"
+        task.summary = "Task with many alarms"
+        task.dtStart = DtStart(Date("20150102"))
+
+        for (i in 1..1050)
+            task.alarms += VAlarm(java.time.Duration.ofMinutes(i.toLong()))
+
+        val uri = TestTask(taskList!!, task).add()
+        val task2 = taskList!!.findById(ContentUris.parseId(uri))
+        assertEquals(1050, task2.task?.alarms?.size)
+    }
+
     @Test
     fun testUpdateTask() {
         // add test event without reminder
@@ -739,7 +751,6 @@ class DmfsTaskTest(
         }
     }
 
-    @MediumTest
     @Test
     fun testBuildAllDayTask() {
         // add all-day event to calendar provider

diff --git a/lib/src/main/kotlin/at/bitfire/ical4android/BatchOperation.kt b/lib/src/main/kotlin/at/bitfire/ical4android/BatchOperation.kt
@@ -19,7 +19,16 @@ import java.util.logging.Logger
 class BatchOperation(
     private val providerClient: ContentProviderClient
 ) {
-
+
+    companion object {
+
+        /** Maximum number of operations per yield point. SQLiteContentProvider, which most content providers
+         * are based on, indicates a value of 500. However to correct value to avoid the [OperationApplicationException]
+         * seems to be 499. */
+        const val MAX_OPERATIONS_PER_YIELD_POINT = 499
+
+    }
+
     private val logger = Logger.getLogger(javaClass.name)
 
     private val queue = LinkedList<CpoBuilder>()
@@ -96,7 +105,7 @@ class BatchOperation(
 
         try {
             val ops = toCPO(start, end)
-            logger.fine("Running ${ops.size} operations ($start .. ${end - 1})")
+            logger.log(Level.FINEST, "Running ${ops.size} operations ($start .. ${end - 1})", ops)
             val partResults = providerClient.applyBatch(ops)
 
             val n = end - start
@@ -133,6 +142,7 @@ class BatchOperation(
          * 2. If a back reference points to a row outside of start/end,
          *    replace it by the actual result, which has already been calculated. */
 
+        var currentIdx = 1
         for (cpoBuilder in queue.subList(start, end)) {
             for ((backrefKey, backref) in cpoBuilder.valueBackrefs) {
                 val originalIdx = backref.originalIndex
@@ -148,17 +158,21 @@ class BatchOperation(
                     backref.setIndex(originalIdx - start)
             }
 
+            // Set a possible yield point every MAX_OPERATIONS_PER_YIELD_POINT operations for SQLiteContentProvider
+            if ((++currentIdx).mod(MAX_OPERATIONS_PER_YIELD_POINT) == 0)
+                cpoBuilder.withYieldAllowed()
+
             cpo += cpoBuilder.build()
         }
         return cpo
     }
 
 
     class BackReference(
-            /** index of the referenced row in the original, nonsplitted transaction */
-            val originalIndex: Int
+        /** index of the referenced row in the original, non-splitted transaction */
+        val originalIndex: Int
     ) {
-        /** overriden index, i.e. index within the splitted transaction */
+        /** overridden index, i.e. index within the splitted transaction */
         private var index: Int? = null
 
         /**
@@ -182,8 +196,8 @@ class BatchOperation(
      * value back references.
      */
     class CpoBuilder private constructor(
-            val uri: Uri,
-            val type: Type
+        val uri: Uri,
+        val type: Type
     ) {
 
         enum class Type { INSERT, UPDATE, DELETE }
@@ -197,11 +211,13 @@ class BatchOperation(
         }
 
 
-        var selection: String? = null
-        var selectionArguments: Array<String>? = null
+        private var selection: String? = null
+        private var selectionArguments: Array<String>? = null
+
+        internal val values = mutableMapOf<String, Any?>()
+        internal val valueBackrefs = mutableMapOf<String, BackReference>()
 
-        val values = mutableMapOf<String, Any?>()
-        val valueBackrefs = mutableMapOf<String, BackReference>()
+        private var yieldAllowed = false
 
 
         fun withSelection(select: String, args: Array<String>): CpoBuilder {
@@ -226,6 +242,10 @@ class BatchOperation(
             return this
         }
 
+        fun withYieldAllowed() {
+            yieldAllowed = true
+        }
+
 
         fun build(): ContentProviderOperation {
             val builder = when (type) {
@@ -242,6 +262,9 @@ class BatchOperation(
             for ((key, backref) in valueBackrefs)
                 builder.withValueBackReference(key, backref.getIndex())
 
+            if (yieldAllowed)
+                builder.withYieldAllowed(true)
+
             return builder.build()
         }