Merge pull request #5546 from seadowg/predicate-caching

Introduce predicate caching as an opt-in setting

buildSrc/src/main/java/dependencies/Dependencies.kt

@@ -43,7 +43,7 @@ object Dependencies {
     const val rarepebble_colorpicker = "com.github.martin-stone:hsv-alpha-color-picker-android:3.0.1"
     const val commons_io = "commons-io:commons-io:2.5" // Commons 2.6+ introduce java.nio usage that we can't access until our minSdkVersion >= 26 (https://developer.android.com/reference/java/io/File#toPath())
     const val opencsv = "com.opencsv:opencsv:5.7.1"
-    const val javarosa = "org.getodk:javarosa:4.1.0"
+    const val javarosa = "org.getodk:javarosa:4.2.0-SNAPSHOT"
     const val javarosa_local = "org.getodk:javarosa:local"
     const val karumi_dexter = "com.karumi:dexter:6.2.3"
     const val zxing_android_embedded = "com.journeyapps:zxing-android-embedded:4.3.0"

collect_app/src/androidTest/java/org/odk/collect/android/instrumented/forms/FormNavigationTest.java

@@ -16,11 +16,15 @@
 
 package org.odk.collect.android.instrumented.forms;
 
+import static junit.framework.Assert.assertEquals;
+
 import android.app.Application;
 
 import androidx.test.core.app.ApplicationProvider;
 
 import org.javarosa.core.model.FormDef;
+import org.javarosa.form.api.FormEntryController;
+import org.javarosa.form.api.FormEntryModel;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
@@ -46,8 +50,6 @@
 
 import timber.log.Timber;
 
-import static junit.framework.Assert.assertEquals;
-
 /**
  * This test has been created in order to check indices while navigating through a form.
  * It's especially important while navigating through a form that contains nested groups and if we
@@ -59,6 +61,13 @@
 @RunWith(Parameterized.class)
 public class FormNavigationTest {
 
+    private final FormLoaderTask.FormEntryControllerFactory formEntryControllerFactory = new FormLoaderTask.FormEntryControllerFactory() {
+        @Override
+        public FormEntryController create(FormDef formDef) {
+            return new FormEntryController(new FormEntryModel(formDef));
+        }
+    };
+
     @Rule
     public RuleChain copyFormChain = TestRuleChain.chain()
             .around(new RunnableRule(() -> {
@@ -120,7 +129,7 @@ private void testIndices(String formName, String[] expectedIndices) throws Execu
             Timber.i(e);
         }
 
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath(formName), null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath(formName), null, null, formEntryControllerFactory);
         formLoaderTask.setFormLoaderListener(new FormLoaderListener() {
             @Override
             public void loadingComplete(FormLoaderTask task, FormDef fd, String warningMsg) {

collect_app/src/androidTest/java/org/odk/collect/android/instrumented/forms/FormUtilsTest.java

@@ -1,18 +1,21 @@
 package org.odk.collect.android.instrumented.forms;
 
+import org.javarosa.core.model.FormDef;
 import org.javarosa.core.reference.RootTranslator;
+import org.javarosa.form.api.FormEntryController;
+import org.javarosa.form.api.FormEntryModel;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
-import org.odk.collect.android.support.rules.CollectTestRule;
-import org.odk.collect.android.support.rules.TestRuleChain;
-import org.odk.collect.android.utilities.FormUtils;
 import org.odk.collect.android.storage.StoragePathProvider;
 import org.odk.collect.android.storage.StorageSubdirectory;
+import org.odk.collect.android.support.rules.CollectTestRule;
+import org.odk.collect.android.support.rules.TestRuleChain;
 import org.odk.collect.android.tasks.FormLoaderTask;
 import org.odk.collect.android.utilities.FileUtils;
+import org.odk.collect.android.utilities.FormUtils;
 
 import java.io.File;
 import java.util.List;
@@ -21,6 +24,13 @@ public class FormUtilsTest {
     private static final String BASIC_FORM = "basic.xml";
     private final CollectTestRule rule = new CollectTestRule();
 
+    private final FormLoaderTask.FormEntryControllerFactory formEntryControllerFactory = new FormLoaderTask.FormEntryControllerFactory() {
+        @Override
+        public FormEntryController create(FormDef formDef) {
+            return new FormEntryController(new FormEntryModel(formDef));
+        }
+    };
+
     @Rule
     public RuleChain copyFormChain = TestRuleChain.chain()
             .around(rule);
@@ -38,7 +48,7 @@ public void setUp() {
     public void sessionRootTranslatorOrderDoesNotMatter() throws Exception {
         final String formPath = new StoragePathProvider().getOdkDirPath(StorageSubdirectory.FORMS) + File.separator + BASIC_FORM;
         // Load the form in order to populate the ReferenceManager
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         formLoaderTask.execute(formPath).get();
 
         final File formXml = new File(formPath);

collect_app/src/androidTest/java/org/odk/collect/android/instrumented/tasks/FormLoaderTaskTest.java

@@ -1,5 +1,15 @@
 package org.odk.collect.android.instrumented.tasks;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.notNullValue;
+
+import android.app.Application;
+
+import androidx.test.core.app.ApplicationProvider;
+
+import org.javarosa.core.model.FormDef;
+import org.javarosa.form.api.FormEntryController;
+import org.javarosa.form.api.FormEntryModel;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
@@ -12,20 +22,14 @@
 import org.odk.collect.android.support.rules.RunnableRule;
 import org.odk.collect.android.support.rules.TestRuleChain;
 import org.odk.collect.android.tasks.FormLoaderTask;
+import org.odk.collect.android.tasks.FormLoaderTask.FormEntryControllerFactory;
 import org.odk.collect.projects.Project;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.notNullValue;
-
-import android.app.Application;
-
-import androidx.test.core.app.ApplicationProvider;
-
 public class FormLoaderTaskTest {
 
     private final StoragePathProvider storagePathProvider = new StoragePathProvider();
@@ -35,6 +39,13 @@ public class FormLoaderTaskTest {
     private static final String SIMPLE_SEARCH_EXTERNAL_CSV_FILE = "simple-search-external-csv-fruits.csv";
     private static final String SIMPLE_SEARCH_EXTERNAL_DB_FILE = "simple-search-external-csv-fruits.db";
 
+    private final FormEntryControllerFactory formEntryControllerFactory = new FormEntryControllerFactory() {
+        @Override
+        public FormEntryController create(FormDef formDef) {
+            return new FormEntryController(new FormEntryModel(formDef));
+        }
+    };
+
     @Rule
     public RuleChain copyFormChain = TestRuleChain.chain()
             .around(new RunnableRule(() -> {
@@ -55,7 +66,7 @@ public class FormLoaderTaskTest {
     @Test
     public void loadFormWithSecondaryCSV() throws Exception {
         final String formPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS) + File.separator + SECONDARY_INSTANCE_EXTERNAL_CSV_FORM;
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         FormLoaderTask.FECWrapper wrapper = formLoaderTask.execute(formPath).get();
         Assert.assertNotNull(wrapper);
     }
@@ -64,15 +75,15 @@ public void loadFormWithSecondaryCSV() throws Exception {
     @Test
     public void loadSearchFromExternalCSV() throws Exception {
         final String formPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS) + File.separator + SIMPLE_SEARCH_EXTERNAL_CSV_FORM;
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         FormLoaderTask.FECWrapper wrapper = formLoaderTask.execute(formPath).get();
         assertThat(wrapper, notNullValue());
     }
 
     @Test
     public void loadSearchFromexternalCsvLeavesFileUnchanged() throws Exception {
         final String formPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS) + File.separator + SIMPLE_SEARCH_EXTERNAL_CSV_FORM;
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         FormLoaderTask.FECWrapper wrapper = formLoaderTask.execute(formPath).get();
         Assert.assertNotNull(wrapper);
         Assert.assertNotNull(wrapper.getController());
@@ -87,7 +98,7 @@ public void loadSearchFromexternalCsvLeavesFileUnchanged() throws Exception {
     public void loadSearchFromExternalCSVmultipleTimes() throws Exception {
         final String formPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS) + File.separator + SIMPLE_SEARCH_EXTERNAL_CSV_FORM;
         // initial load with side effects
-        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null);
+        FormLoaderTask formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         FormLoaderTask.FECWrapper wrapper = formLoaderTask.execute(formPath).get();
         Assert.assertNotNull(wrapper);
         Assert.assertNotNull(wrapper.getController());
@@ -98,7 +109,7 @@ public void loadSearchFromExternalCSVmultipleTimes() throws Exception {
         long dbLastModified = dbFile.lastModified();
 
         // subsequent load should succeed despite side effects from import
-        formLoaderTask = new FormLoaderTask(formPath, null, null);
+        formLoaderTask = new FormLoaderTask(formPath, null, null, formEntryControllerFactory);
         wrapper = formLoaderTask.execute(formPath).get();
         Assert.assertNotNull(wrapper);
         Assert.assertNotNull(wrapper.getController());

collect_app/src/main/java/org/odk/collect/android/activities/FormEntryActivity.java

@@ -359,6 +359,9 @@ enum AnimationType {
     @Inject
     public AudioHelperFactory audioHelperFactory;
 
+    @Inject
+    public FormLoaderTask.FormEntryControllerFactory formEntryControllerFactory;
+
     private final LocationProvidersReceiver locationProvidersReceiver = new LocationProvidersReceiver();
 
     private SwipeHandler swipeHandler;
@@ -634,7 +637,7 @@ private void loadForm() {
                     formEntryViewModel.refresh();
                 } else {
                     Timber.w("Reloading form and restoring state.");
-                    formLoaderTask = new FormLoaderTask(instancePath, startingXPath, waitingXPath);
+                    formLoaderTask = new FormLoaderTask(instancePath, startingXPath, waitingXPath, formEntryControllerFactory);
                     showIfNotShowing(FormLoadingDialogFragment.class, getSupportFragmentManager());
                     formLoaderTask.execute(formPath);
                 }
@@ -713,7 +716,7 @@ private void loadFromIntent(Intent intent) {
             return;
         }
 
-        formLoaderTask = new FormLoaderTask(instancePath, null, null);
+        formLoaderTask = new FormLoaderTask(instancePath, null, null, formEntryControllerFactory);
         formLoaderTask.setFormLoaderListener(this);
         showIfNotShowing(FormLoadingDialogFragment.class, getSupportFragmentManager());
         formLoaderTask.execute(formPath);

collect_app/src/main/java/org/odk/collect/android/formmanagement/CollectFormEntryControllerFactory.kt

@@ -0,0 +1,22 @@
+package org.odk.collect.android.formmanagement
+
+import org.javarosa.core.model.FormDef
+import org.javarosa.entities.EntityFormFinalizationProcessor
+import org.javarosa.form.api.FormEntryController
+import org.javarosa.form.api.FormEntryModel
+import org.odk.collect.android.tasks.FormLoaderTask.FormEntryControllerFactory
+import org.odk.collect.settings.keys.ProjectKeys
+import org.odk.collect.shared.settings.Settings
+
+class CollectFormEntryControllerFactory constructor(private val settings: Settings) :
+    FormEntryControllerFactory {
+    override fun create(formDef: FormDef): FormEntryController {
+        return FormEntryController(FormEntryModel(formDef)).also {
+            it.addPostProcessor(EntityFormFinalizationProcessor())
+
+            if (!settings.getBoolean(ProjectKeys.KEY_PREDICATE_CACHING)) {
+                it.disablePredicateCaching()
+            }
+        }
+    }
+}

collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java

@@ -50,6 +50,7 @@
 import org.odk.collect.android.formentry.media.AudioHelperFactory;
 import org.odk.collect.android.formentry.media.ScreenContextAudioHelperFactory;
 import org.odk.collect.android.formlists.blankformlist.BlankFormListViewModel;
+import org.odk.collect.android.formmanagement.CollectFormEntryControllerFactory;
 import org.odk.collect.android.formmanagement.FormDownloader;
 import org.odk.collect.android.formmanagement.FormMetadataParser;
 import org.odk.collect.android.formmanagement.FormSourceProvider;
@@ -84,6 +85,7 @@
 import org.odk.collect.android.projects.ProjectDependencyProviderFactory;
 import org.odk.collect.android.storage.StoragePathProvider;
 import org.odk.collect.android.storage.StorageSubdirectory;
+import org.odk.collect.android.tasks.FormLoaderTask;
 import org.odk.collect.android.utilities.AdminPasswordProvider;
 import org.odk.collect.android.utilities.AndroidUserAgent;
 import org.odk.collect.android.utilities.ChangeLockProvider;
@@ -633,4 +635,9 @@ public BlankFormListViewModel.Factory providesBlankFormListViewModel(FormsReposi
     public ImageCompressionController providesImageCompressorManager() {
         return new ImageCompressionController(ImageCompressor.INSTANCE);
     }
+
+    @Provides
+    public FormLoaderTask.FormEntryControllerFactory formEntryControllerFactory(SettingsProvider settingsProvider) {
+        return new CollectFormEntryControllerFactory(settingsProvider.getUnprotectedSettings());
+    }
 }

collect_app/src/main/java/org/odk/collect/android/preferences/Defaults.kt

@@ -55,6 +55,8 @@ object Defaults {
             hashMap[ProjectKeys.KEY_USGS_MAP_STYLE] = "topographic"
             hashMap[ProjectKeys.KEY_GOOGLE_MAP_STYLE] = GoogleMap.MAP_TYPE_NORMAL.toString()
             hashMap[ProjectKeys.KEY_MAPBOX_MAP_STYLE] = "mapbox://styles/mapbox/streets-v11"
+            // experimental_preferences.xml
+            hashMap[ProjectKeys.KEY_PREDICATE_CACHING] = false
             return hashMap
         }
 

collect_app/src/main/java/org/odk/collect/android/tasks/FormLoaderTask.java

@@ -22,6 +22,8 @@
 import android.database.SQLException;
 import android.os.AsyncTask;
 
+import androidx.annotation.NonNull;
+
 import com.opencsv.CSVReader;
 import com.opencsv.exceptions.CsvValidationException;
 
@@ -32,9 +34,7 @@
 import org.javarosa.core.model.instance.TreeReference;
 import org.javarosa.core.model.instance.utils.DefaultAnswerResolver;
 import org.javarosa.core.reference.ReferenceManager;
-import org.javarosa.entities.EntityFormFinalizationProcessor;
 import org.javarosa.form.api.FormEntryController;
-import org.javarosa.form.api.FormEntryModel;
 import org.javarosa.xform.parse.XFormParser;
 import org.javarosa.xform.util.XFormUtils;
 import org.javarosa.xpath.XPathTypeMismatchException;
@@ -81,6 +81,7 @@ public class FormLoaderTask extends AsyncTask<String, String, FormLoaderTask.FEC
     private String instancePath;
     private final String xpath;
     private final String waitingXPath;
+    private FormEntryControllerFactory formEntryControllerFactory;
     private boolean pendingActivityResult;
     private int requestCode;
     private int resultCode;
@@ -112,10 +113,11 @@ protected void free() {
 
     FECWrapper data;
 
-    public FormLoaderTask(String instancePath, String xpath, String waitingXPath) {
+    public FormLoaderTask(String instancePath, String xpath, String waitingXPath, FormEntryControllerFactory formEntryControllerFactory) {
         this.instancePath = instancePath;
         this.xpath = xpath;
         this.waitingXPath = waitingXPath;
+        this.formEntryControllerFactory = formEntryControllerFactory;
     }
 
     /**
@@ -176,9 +178,7 @@ protected FECWrapper doInBackground(String... path) {
         }
 
         // create FormEntryController from formdef
-        final FormEntryModel fem = new FormEntryModel(formDef);
-        final FormEntryController fec = new FormEntryController(fem);
-        fec.addPostProcessor(new EntityFormFinalizationProcessor());
+        final FormEntryController fec = formEntryControllerFactory.create(formDef);
 
         boolean usedSavepoint = false;
 
@@ -574,4 +574,8 @@ private void readCSV(File csv, String formHash, String pathHash) {
     public FormDef getFormDef() {
         return formDef;
     }
+
+    public interface FormEntryControllerFactory {
+        FormEntryController create(@NonNull FormDef formDef);
+    }
 }

collect_app/src/main/res/xml/experimental_preferences.xml

@@ -2,6 +2,10 @@
     xmlns:app="http://schemas.android.com/apk/res-auto"
     android:title="@string/experimental">
 
+    <SwitchPreferenceCompat
+        android:title="@string/predicate_caching"
+        android:key="predicate_caching" />
+
     <Preference
         android:icon="@drawable/ic_person_black_24dp"
         android:title="@string/entities_title"

settings/src/main/java/org/odk/collect/settings/keys/ProjectKeys.kt

@@ -55,6 +55,9 @@ object ProjectKeys {
     const val KEY_BACKGROUND_LOCATION = "background_location"
     const val KEY_BACKGROUND_RECORDING = "background_recording"
 
+    // experimental_preferences.xml
+    const val KEY_PREDICATE_CACHING = "predicate_caching"
+
     // values
     const val PROTOCOL_SERVER = "odk_default"
     const val PROTOCOL_GOOGLE_SHEETS = "google_sheets"

strings/src/main/res/values/strings.xml

@@ -1196,5 +1196,6 @@
     <string name="crash_app">Crash app</string>
     <!-- An uncaught exception is an exception that is not handled by ODK Collect and so will force the application to close -->
     <string name="crash_app_summary">Force an uncaught exception causing the app to crash</string>
+    <string name="predicate_caching">Predicate caching</string>
 
 </resources>