Merge pull request #6448 from grzesiek2010/COLLECT-6447

Include DynamicPreloadExtra in formDef only if a form contains search or pulldata

collect_app/src/main/java/org/odk/collect/android/dynamicpreload/DynamicPreloadXFormParserFactory.kt

@@ -2,7 +2,6 @@ package org.odk.collect.android.dynamicpreload
 
 import org.javarosa.core.model.FormDef
 import org.javarosa.core.model.QuestionDef
-import org.javarosa.core.util.externalizable.ExtUtil
 import org.javarosa.core.util.externalizable.Externalizable
 import org.javarosa.core.util.externalizable.PrototypeFactory
 import org.javarosa.xform.parse.IXFormParserFactory
@@ -50,22 +49,14 @@ class DynamicPreloadParseProcessor :
     }
 
     override fun processFormDef(formDef: FormDef) {
-        formDef.extras.put(DynamicPreloadExtra(containsPullData || containsSearch))
+        if (containsPullData || containsSearch) {
+            formDef.extras.put(DynamicPreloadExtra())
+        }
     }
 }
 
-class DynamicPreloadExtra(usesDynamicPreload: Boolean) : Externalizable {
-
-    constructor() : this(false)
-
-    var usesDynamicPreload = usesDynamicPreload
-        private set
+class DynamicPreloadExtra : Externalizable {
+    override fun readExternal(`in`: DataInputStream?, pf: PrototypeFactory?) = Unit
 
-    override fun readExternal(`in`: DataInputStream?, pf: PrototypeFactory?) {
-        usesDynamicPreload = ExtUtil.read(`in`, Boolean::class.javaObjectType) as Boolean
-    }
-
-    override fun writeExternal(out: DataOutputStream?) {
-        ExtUtil.write(out, usesDynamicPreload)
-    }
+    override fun writeExternal(out: DataOutputStream?) = Unit
 }

collect_app/src/main/java/org/odk/collect/android/dynamicpreload/ExternalDataUseCases.kt

@@ -16,7 +16,7 @@ object ExternalDataUseCases {
         isCancelled: Supplier<Boolean>,
         progressReporter: Consumer<Function<Resources, String>>
     ) {
-        if (form.extras.get(DynamicPreloadExtra::class.java)?.usesDynamicPreload == false) {
+        if (form.extras.get(DynamicPreloadExtra::class.java) == null) {
             return
         }
 

collect_app/src/test/java/org/odk/collect/android/dynamicpreload/DynamicPreloadExtraTest.kt

@@ -1,20 +1,20 @@
 package org.odk.collect.android.dynamicpreload
 
 import org.hamcrest.MatcherAssert.assertThat
-import org.hamcrest.Matchers.equalTo
+import org.hamcrest.Matchers.instanceOf
 import org.javarosa.core.util.externalizable.ExtUtil
 import org.junit.Test
 
 class DynamicPreloadExtraTest {
 
     @Test
     fun `can be externalized`() {
-        val extra = DynamicPreloadExtra(true)
+        val extra = DynamicPreloadExtra()
 
         val external = ExtUtil.serialize(extra)
         val deserialized =
             ExtUtil.deserialize(external, DynamicPreloadExtra::class.java) as DynamicPreloadExtra
 
-        assertThat(deserialized.usesDynamicPreload, equalTo(true))
+        assertThat(deserialized, instanceOf(DynamicPreloadExtra::class.java))
     }
 }

collect_app/src/test/java/org/odk/collect/android/dynamicpreload/DynamicPreloadParseProcessorTest.kt

@@ -2,6 +2,7 @@ package org.odk.collect.android.dynamicpreload
 
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.equalTo
+import org.hamcrest.Matchers.instanceOf
 import org.javarosa.core.model.FormDef
 import org.javarosa.core.model.QuestionDef
 import org.javarosa.xpath.expr.XPathExpression
@@ -14,50 +15,50 @@ class DynamicPreloadParseProcessorTest {
     private val processor = DynamicPreloadParseProcessor()
 
     @Test
-    fun `usesDynamicPreload is false when xpath does not contain pulldata`() {
+    fun `DynamicPreloadExtra is null when xpath does not contain pulldata`() {
         val formDef = FormDef()
 
         processor.processXPath(createNonPullDataExpression())
         processor.processFormDef(formDef)
         assertThat(
-            formDef.extras.get(DynamicPreloadExtra::class.java).usesDynamicPreload,
-            equalTo(false)
+            formDef.extras.get(DynamicPreloadExtra::class.java),
+            equalTo(null)
         )
     }
 
     @Test
-    fun `usesDynamicPreload is true when xpath does contain pulldata`() {
+    fun `DynamicPreloadExtra is not null when xpath does contain pulldata`() {
         val formDef = FormDef()
 
         processor.processXPath(createPullDataExpression())
         processor.processFormDef(formDef)
         assertThat(
-            formDef.extras.get(DynamicPreloadExtra::class.java).usesDynamicPreload,
-            equalTo(true)
+            formDef.extras.get(DynamicPreloadExtra::class.java),
+            instanceOf(DynamicPreloadExtra::class.java)
         )
     }
 
     @Test
-    fun `usesDynamicPreload is false when question appearance does not contain search`() {
+    fun `DynamicPreloadExtra is null when question appearance does not contain search`() {
         val formDef = FormDef()
 
         processor.processQuestion(createQuestion(appearance = "minimal"))
         processor.processFormDef(formDef)
         assertThat(
-            formDef.extras.get(DynamicPreloadExtra::class.java).usesDynamicPreload,
-            equalTo(false)
+            formDef.extras.get(DynamicPreloadExtra::class.java),
+            equalTo(null)
         )
     }
 
     @Test
-    fun `usesDynamicPreload is true when question appearance does contain search`() {
+    fun `DynamicPreloadExtra is not null when question appearance does contain search`() {
         val formDef = FormDef()
 
         processor.processQuestion(createQuestion(appearance = "search('fruits')"))
         processor.processFormDef(formDef)
         assertThat(
-            formDef.extras.get(DynamicPreloadExtra::class.java).usesDynamicPreload,
-            equalTo(true)
+            formDef.extras.get(DynamicPreloadExtra::class.java),
+            instanceOf(DynamicPreloadExtra::class.java)
         )
     }
 

collect_app/src/test/java/org/odk/collect/android/dynamicpreload/ExternalDataUseCasesTest.kt

@@ -14,9 +14,7 @@ class ExternalDataUseCasesTest {
 
     @Test
     fun `create() does nothing if the form doesn't use dynamic preload`() {
-        val form = FormDef().also {
-            it.extras.put(DynamicPreloadExtra(false))
-        }
+        val form = FormDef()
 
         val mediaDir = TempFiles.createTempDir().also {
             File(it, "items.csv").writeText("name_key,name\nmango,Mango")
@@ -27,8 +25,23 @@ class ExternalDataUseCasesTest {
     }
 
     @Test
-    fun `create() works if the FormDef does not have a DynamicPreloadExtra()`() {
+    fun `create() does not create a db file if the FormDef does not have a DynamicPreloadExtra`() {
         val form = FormDef()
+
+        val mediaDir = TempFiles.createTempDir().also {
+            File(it, "items.csv").writeText("name_key,name\nmango,Mango")
+        }
+
+        ExternalDataUseCases.create(form, mediaDir, { false }, {})
+        assertThat(mediaDir.listFiles().size, equalTo(1))
+    }
+
+    @Test
+    fun `create() creates a db file if the FormDef has a DynamicPreloadExtra`() {
+        val form = FormDef().also {
+            it.extras.put(DynamicPreloadExtra())
+        }
+
         val mediaDir = TempFiles.createTempDir().also {
             File(it, "items.csv").writeText("name_key,name\nmango,Mango")
         }
@@ -40,7 +53,7 @@ class ExternalDataUseCasesTest {
     @Test
     fun `create() leaves original CSV in place`() {
         val form = FormDef().also {
-            it.extras.put(DynamicPreloadExtra(true))
+            it.extras.put(DynamicPreloadExtra())
         }
 
         val mediaDir = TempFiles.createTempDir()