Merge remote-tracking branch 'upstream/master' into ping/map-view

getodk · Dec 2, 2019 · 25f5840 · 25f5840
2 parents 0a8fbef + 49494e3
commit 25f5840
Show file tree

Hide file tree

Showing 15 changed files with 204 additions and 76 deletions.
diff --git a/collect_app/src/androidTest/assets/forms/external_data_questions.xml b/collect_app/src/androidTest/assets/forms/external_data_questions.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+<h:html
+    xmlns="http://www.w3.org/2002/xforms"
+    xmlns:ev="http://www.w3.org/2001/xml-events"
+    xmlns:h="http://www.w3.org/1999/xhtml"
+    xmlns:jr="http://openrosa.org/javarosa"
+    xmlns:odk="http://www.opendatakit.org/xforms"
+    xmlns:orx="http://openrosa.org/xforms"
+    xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    <h:head>
+        <h:title>externalDataQuestions</h:title>
+        <model>
+            <instance>
+                <externalDataQuestions id="externalDataQuestions">
+                    <q1/>
+                    <q2/>
+                    <meta>
+                        <instanceID/>
+                    </meta>
+                </externalDataQuestions>
+            </instance>
+            <bind nodeset="/externalDataQuestions/q1" type="select1"/>
+            <bind nodeset="/externalDataQuestions/q2" type="string"/>
+            <bind jr:preload="uid" nodeset="/externalDataQuestions/meta/instanceID" readonly="true()" type="string"/>
+        </model>
+    </h:head>
+    <h:body>
+        <select1 appearance="search('fruits')" ref="/externalDataQuestions/q1">
+            <label>Search func</label>
+            <item>
+                <label>name</label>
+                <value>name_key</value>
+            </item>
+        </select1>
+        <input query="instance('counties')/root/item[county= &quot;King&quot;]" ref="/externalDataQuestions/q2">
+            <label>External csv</label>
+        </input>
+    </h:body>
+</h:html>
diff --git a/.../src/androidTest/java/org/odk/collect/android/formentry/ExternalDataFileNotFoundTest.java b/.../src/androidTest/java/org/odk/collect/android/formentry/ExternalDataFileNotFoundTest.java
@@ -0,0 +1,40 @@
+package org.odk.collect.android.formentry;
+
+import android.Manifest;
+
+import androidx.test.espresso.intent.rule.IntentsTestRule;
+import androidx.test.rule.GrantPermissionRule;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.odk.collect.android.R;
+import org.odk.collect.android.activities.FormEntryActivity;
+import org.odk.collect.android.espressoutils.pages.FormEntryPage;
+import org.odk.collect.android.support.CopyFormRule;
+import org.odk.collect.android.support.ResetStateRule;
+import org.odk.collect.android.test.FormLoadingUtils;
+
+public class ExternalDataFileNotFoundTest {
+    private static final String EXTERNAL_DATA_QUESTIONS = "external_data_questions.xml";
+
+    @Rule
+    public IntentsTestRule<FormEntryActivity> activityTestRule = FormLoadingUtils.getFormActivityTestRuleFor(EXTERNAL_DATA_QUESTIONS);
+
+    @Rule
+    public RuleChain copyFormChain = RuleChain
+            .outerRule(GrantPermissionRule.grant(
+                    Manifest.permission.READ_EXTERNAL_STORAGE,
+                    Manifest.permission.WRITE_EXTERNAL_STORAGE)
+            )
+            .around(new ResetStateRule())
+            .around(new CopyFormRule(EXTERNAL_DATA_QUESTIONS));
+
+    @Test
+    public void questionsThatUseExternalFiles_ShouldDisplayFriendlyMessageWhenFilesAreMissing() {
+        new FormEntryPage("externalDataQuestions", activityTestRule)
+                .checkIsTextDisplayed(activityTestRule.getActivity().getString(R.string.file_missing, "/storage/emulated/0/odk/forms/external_data_questions-media/fruits.csv"))
+                .swipeToNextQuestion()
+                .checkIsTextDisplayed(activityTestRule.getActivity().getString(R.string.file_missing, "/storage/emulated/0/odk/forms/external_data_questions-media/itemsets.csv"));
+    }
+}
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/test/FormLoadingUtils.java b/collect_app/src/androidTest/java/org/odk/collect/android/test/FormLoadingUtils.java
@@ -20,6 +20,7 @@
 
 import androidx.test.espresso.intent.rule.IntentsTestRule;
 
+import org.javarosa.core.reference.ReferenceManager;
 import org.odk.collect.android.activities.FormEntryActivity;
 import org.odk.collect.android.application.Collect;
 import org.odk.collect.android.dao.FormsDao;
@@ -32,6 +33,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.odk.collect.android.forms.FormUtils.setupReferenceManagerForForm;
 import static org.odk.collect.android.test.FileUtils.copyFileFromAssets;
 
 public class FormLoadingUtils {
@@ -48,11 +50,14 @@ private FormLoadingUtils() {
      */
     public static void copyFormToSdCard(String formFilename, List<String> mediaFilenames) throws IOException {
         Collect.createODKDirs();
-        copyForm(formFilename);
 
+        String pathname = copyForm(formFilename);
         if (mediaFilenames != null) {
             copyFormMediaFiles(formFilename, mediaFilenames);
         }
+
+        setupReferenceManagerForForm(ReferenceManager.instance(), FileUtils.getFormMediaDir(new File(pathname)));
+        saveFormToDatabase(new File(pathname));
     }
 
     /**
@@ -84,10 +89,10 @@ public static IntentsTestRule<FormEntryActivity> getFormActivityTestRuleFor(Stri
         return new FormActivityTestRule(formFilename);
     }
 
-    private static void copyForm(String formFilename) throws IOException {
+    private static String copyForm(String formFilename) throws IOException {
         String pathname = Collect.FORMS_PATH + "/" + formFilename;
         copyFileFromAssets("forms/" + formFilename, pathname);
-        saveFormToDatabase(new File(pathname));
+        return pathname;
     }
 
     private static void copyFormMediaFiles(String formFilename, List<String> mediaFilenames) throws IOException {

diff --git a/collect_app/src/main/java/org/odk/collect/android/external/ExternalDataUtil.java b/collect_app/src/main/java/org/odk/collect/android/external/ExternalDataUtil.java
@@ -33,7 +33,10 @@
 import org.odk.collect.android.application.Collect;
 import org.odk.collect.android.exception.ExternalDataException;
 import org.odk.collect.android.external.handler.ExternalDataHandlerSearch;
+import org.odk.collect.android.logic.FormController;
 
+import java.io.File;
+import java.io.FileNotFoundException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -156,7 +159,7 @@ public static XPathFuncExpr getSearchXPathExpression(String appearance) {
     }
 
     public static ArrayList<SelectChoice> populateExternalChoices(FormEntryPrompt formEntryPrompt,
-            XPathFuncExpr xpathfuncexpr) {
+            XPathFuncExpr xpathfuncexpr) throws FileNotFoundException {
         try {
             List<SelectChoice> selectChoices = formEntryPrompt.getSelectChoices();
             ArrayList<SelectChoice> returnedChoices = new ArrayList<>();
@@ -205,6 +208,19 @@ public static ArrayList<SelectChoice> populateExternalChoices(FormEntryPrompt fo
             }
             return returnedChoices;
         } catch (Exception e) {
+            String fileName = String.valueOf(xpathfuncexpr.args[0].eval(null, null));
+            if (!fileName.endsWith(".csv")) {
+                fileName = fileName + ".csv";
+            }
+            FormController formController = Collect.getInstance().getFormController();
+            String filePath = fileName;
+            if (formController != null) {
+                filePath = Collect.getInstance().getFormController().getMediaFolder() + File.separator + fileName;
+            }
+            if (!new File(filePath).exists()) {
+                throw new FileNotFoundException(filePath);
+            }
+
             throw new ExternalDataException(e.getMessage(), e);
         }
     }

diff --git a/collect_app/src/main/java/org/odk/collect/android/forms/FormUtils.java b/collect_app/src/main/java/org/odk/collect/android/forms/FormUtils.java
@@ -0,0 +1,38 @@
+package org.odk.collect.android.forms;
+
+import org.javarosa.core.reference.ReferenceManager;
+import org.javarosa.core.reference.RootTranslator;
+import org.odk.collect.android.application.Collect;
+import org.odk.collect.android.logic.FileReferenceFactory;
+
+import java.io.File;
+
+public class FormUtils {
+
+    private FormUtils() {
+
+    }
+
+    public static void setupReferenceManagerForForm(ReferenceManager referenceManager, File formMediaDir) {
+
+        // Remove previous forms
+        referenceManager.clearSession();
+
+        // This should get moved to the Application Class
+        if (referenceManager.getFactories().length == 0) {
+            // this is /sdcard/odk
+            referenceManager.addReferenceFactory(new FileReferenceFactory(Collect.ODK_ROOT));
+        }
+
+        addSessionRootTranslators(formMediaDir.getName(), referenceManager,
+                "images", "image", "audio", "video", "file");
+    }
+
+    private static void addSessionRootTranslators(String formMediaDir, ReferenceManager referenceManager, String... hostStrings) {
+        // Set jr://... to point to /sdcard/odk/forms/formBasename-media/
+        final String translatedPrefix = String.format("jr://file/forms/" + formMediaDir + "/");
+        for (String t : hostStrings) {
+            referenceManager.addSessionRootTranslator(new RootTranslator(String.format("jr://%s/", t), translatedPrefix));
+        }
+    }
+}
diff --git a/collect_app/src/main/java/org/odk/collect/android/geo/MapboxMapFragment.java b/collect_app/src/main/java/org/odk/collect/android/geo/MapboxMapFragment.java
@@ -106,7 +106,6 @@ public class MapboxMapFragment extends org.odk.collect.android.geo.mapboxsdk.Map
     private boolean isDragging;
     private String styleUrl = Style.MAPBOX_STREETS;
     private File referenceLayerFile;
-    private final List<Layer> overlayLayers = new ArrayList<>();
     private final List<Source> overlaySources = new ArrayList<>();
     private static String lastLocationProvider;
 
@@ -514,7 +513,6 @@ private Style.Builder getBasemapStyleBuilder() {
      * is fully loaded, in setStyle()'s OnStyleLoaded callback.
      */
     private void loadReferenceOverlay() {
-        clearOverlays();
         if (referenceLayerFile != null) {
             addMbtiles(referenceLayerFile.getName(), referenceLayerFile);
         }
@@ -593,28 +591,8 @@ private TileSet createTileSet(MbtilesFile mbtiles, String urlTemplate) {
 
         return tileSet;
     }
-
-    private void clearOverlays() {
-        Style style = map.getStyle();
-        for (Layer layer : overlayLayers) {
-            style.removeLayer(layer);
-        }
-        overlayLayers.clear();
-        // MAPBOX ISSUE: https://github.com/mapbox/mapbox-gl-native/issues/15182
-        // Having removed the overlay layers, this would be the appropriate time
-        // to remove their sources from the map as well.  However, removing the
-        // overlay sources from the map can lead to a SEGV in libmapbox-gl.so.
-        /*
-        for (Source source : overlaySources) {
-            style.removeSource(source);
-        }
-        overlaySources.clear();
-        */
-    }
-
+
     private void addOverlayLayer(Layer layer) {
-        overlayLayers.add(layer);
-
         // If there is a LocationComponent, it will have added some layers to the
         // style.  The SymbolManager and LineManager also add their own layers
         // where they place their symbols and lines.  We need to insert the new

diff --git a/...java/org/odk/collect/android/http/openrosa/okhttp/OkHttpOpenRosaServerClientProvider.java b/...java/org/odk/collect/android/http/openrosa/okhttp/OkHttpOpenRosaServerClientProvider.java
@@ -56,8 +56,9 @@ public OpenRosaServerClient get(String scheme, String userAgent, @Nullable HttpC
         return client;
     }
 
-    private boolean credentialsHaveChanged(@Nullable HttpCredentialsInterface credentials) {
-        return lastCredentials != null && !lastCredentials.equals(credentials);
+    public boolean credentialsHaveChanged(@Nullable HttpCredentialsInterface credentials) {
+        return lastCredentials != null && !lastCredentials.equals(credentials)
+                || lastCredentials == null && credentials != null;
     }
 
     @NonNull

diff --git a/collect_app/src/main/java/org/odk/collect/android/preferences/ResetDialogPreference.java b/collect_app/src/main/java/org/odk/collect/android/preferences/ResetDialogPreference.java
@@ -25,9 +25,10 @@
 import android.preference.DialogPreference;
 import android.util.AttributeSet;
 import android.view.View;
-import android.widget.CheckBox;
 import android.widget.CompoundButton;
 
+import androidx.appcompat.widget.AppCompatCheckBox;
+
 import org.odk.collect.android.R;
 import org.odk.collect.android.fragments.dialogs.ResetSettingsResultDialog;
 import org.odk.collect.android.utilities.ResetUtility;
@@ -41,12 +42,12 @@
 import static org.odk.collect.android.utilities.ResetUtility.ResetAction.RESET_PREFERENCES;
 
 public class ResetDialogPreference extends DialogPreference implements CompoundButton.OnCheckedChangeListener {
-    private CheckBox preferences;
-    private CheckBox instances;
-    private CheckBox forms;
-    private CheckBox layers;
-    private CheckBox cache;
-    private CheckBox osmDroid;
+    private AppCompatCheckBox preferences;
+    private AppCompatCheckBox instances;
+    private AppCompatCheckBox forms;
+    private AppCompatCheckBox layers;
+    private AppCompatCheckBox cache;
+    private AppCompatCheckBox osmDroid;
     private ProgressDialog progressDialog;
 
     public ResetDialogPreference(Context context, AttributeSet attrs) {

diff --git a/collect_app/src/main/java/org/odk/collect/android/tasks/FormLoaderTask.java b/collect_app/src/main/java/org/odk/collect/android/tasks/FormLoaderTask.java
@@ -26,7 +26,6 @@
 import org.javarosa.core.model.instance.TreeReference;
 import org.javarosa.core.model.instance.utils.DefaultAnswerResolver;
 import org.javarosa.core.reference.ReferenceManager;
-import org.javarosa.core.reference.RootTranslator;
 import org.javarosa.form.api.FormEntryController;
 import org.javarosa.form.api.FormEntryModel;
 import org.javarosa.xform.parse.XFormParser;
@@ -43,7 +42,6 @@
 import org.odk.collect.android.external.ExternalDataReaderImpl;
 import org.odk.collect.android.external.handler.ExternalDataHandlerPull;
 import org.odk.collect.android.listeners.FormLoaderListener;
-import org.odk.collect.android.logic.FileReferenceFactory;
 import org.odk.collect.android.logic.FormController;
 import org.odk.collect.android.utilities.FileUtils;
 import org.odk.collect.android.utilities.FormDefCache;
@@ -60,6 +58,8 @@
 import au.com.bytecode.opencsv.CSVReader;
 import timber.log.Timber;
 
+import static org.odk.collect.android.forms.FormUtils.setupReferenceManagerForForm;
+
 /**
  * Background task for loading a form.
  *
@@ -130,19 +130,7 @@ protected FECWrapper doInBackground(String... path) {
         final File formXml = new File(formPath);
         final File formMediaDir = FileUtils.getFormMediaDir(formXml);
 
-        final ReferenceManager referenceManager = ReferenceManager.instance();
-
-        // Remove previous forms
-        referenceManager.clearSession();
-
-        // This should get moved to the Application Class
-        if (referenceManager.getFactories().length == 0) {
-            // this is /sdcard/odk
-            referenceManager.addReferenceFactory(new FileReferenceFactory(Collect.ODK_ROOT));
-        }
-
-        addSessionRootTranslators(formMediaDir.getName(), referenceManager,
-                "images", "image", "audio", "video", "file");
+        setupReferenceManagerForForm(ReferenceManager.instance(), formMediaDir);
 
         FormDef formDef = null;
         try {
@@ -225,14 +213,6 @@ protected FECWrapper doInBackground(String... path) {
         return data;
     }
 
-    private void addSessionRootTranslators(String formMediaDir, ReferenceManager referenceManager, String... hostStrings) {
-        // Set jr://... to point to /sdcard/odk/forms/formBasename-media/
-        final String translatedPrefix = String.format("jr://file/forms/" + formMediaDir + "/");
-        for (String t : hostStrings) {
-            referenceManager.addSessionRootTranslator(new RootTranslator(String.format("jr://%s/", t), translatedPrefix));
-        }
-    }
-
     private FormDef createFormDefFromCacheOrXml(String formPath, File formXml) {
         publishProgress(
                 Collect.getInstance().getString(R.string.survey_loading_reading_form_message));

diff --git a/collect_app/src/main/java/org/odk/collect/android/tasks/InstanceSyncTask.java b/collect_app/src/main/java/org/odk/collect/android/tasks/InstanceSyncTask.java
@@ -35,7 +35,6 @@
 import org.odk.collect.android.utilities.EncryptionUtils;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
-import org.xml.sax.SAXException;
 
 import java.io.File;
 import java.io.IOException;
@@ -46,7 +45,6 @@
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 
 import timber.log.Timber;
 
@@ -213,7 +211,7 @@ private String getFormIdFromInstance(final String instancePath) {
             Document document = builder.parse(new File(instancePath));
             Element element = document.getDocumentElement();
             instanceFormId = element.getAttribute("id");
-        } catch (IOException | ParserConfigurationException | SAXException e) {
+        } catch (Exception | Error e) {
             Timber.w("Unable to read form id from %s", instancePath);
         }
         return instanceFormId;
@@ -227,7 +225,7 @@ private String getInstanceIdFromInstance(final String instancePath) {
             Document document = builder.parse(new File(instancePath));
             Element element = document.getDocumentElement();
             instanceId = element.getAttribute("instanceID");
-        } catch (IOException | ParserConfigurationException | SAXException e) {
+        } catch (Exception | Error e) {
             Timber.w("Unable to read form instanceID from %s", instancePath);
         }
         return instanceId;