Merge pull request #6428 from grzesiek2010/formparser

Allow forms with parse errors to be downloaded, show errors at form open time
getodk · Oct 1, 2024 · 719e214 · 719e214
2 parents 4a70306 + df9d70e
commit 719e214
Show file tree

Hide file tree

Showing 28 changed files with 638 additions and 775 deletions.
diff --git a/...droidTest/java/org/odk/collect/android/feature/formentry/CatchFormDesignExceptionsTest.kt b/...droidTest/java/org/odk/collect/android/feature/formentry/CatchFormDesignExceptionsTest.kt
@@ -43,17 +43,15 @@ class CatchFormDesignExceptionsTest {
     fun whenFormHasNonFatalErrors_explanationDialogShouldBeDisplayedAndTheFormShouldNotBeClosedAfterClickingOK() {
         rule.startAtMainMenu()
             .copyForm("g6Error.xml")
-            .startBlankFormWithError("g6Error")
-            .assertText(org.odk.collect.strings.R.string.error_occured)
+            .startBlankFormWithError("g6Error", false)
             .clickOK(FormEntryPage("g6Error"))
     }
 
     @Test
     fun whenFormHasNonFatalErrors_explanationDialogShouldNotSurviveActivityRecreation() {
         rule.startAtMainMenu()
             .copyForm("g6Error.xml")
-            .startBlankFormWithError("g6Error")
-            .assertText(org.odk.collect.strings.R.string.error_occured)
+            .startBlankFormWithError("g6Error", false)
             .clickOK(FormEntryPage("g6Error"))
             .rotateToLandscape(FormEntryPage("g6Error"))
             .assertTextDoesNotExist(org.odk.collect.strings.R.string.error_occured)

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/EntityFormTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/EntityFormTest.kt
@@ -11,6 +11,7 @@ import org.odk.collect.android.support.pages.FormEntryPage
 import org.odk.collect.android.support.pages.MainMenuPage
 import org.odk.collect.android.support.rules.CollectTestRule
 import org.odk.collect.android.support.rules.TestRuleChain
+import org.odk.collect.strings.R
 
 @RunWith(AndroidJUnit4::class)
 class EntityFormTest {
@@ -190,4 +191,39 @@ class EntityFormTest {
             .assertText("Roman Roy")
             .assertTextDoesNotExist("Logan Roy")
     }
+
+    @Test
+    fun manualEntityFormDownload_withUnsupportedSpecVersion_completesSuccessfully_butThrowsAnErrorAfterOpeningIt() {
+        testDependencies.server.addForm("one-question-entity-registration-v2020.1.xml")
+
+        rule.withProject(testDependencies.server)
+            .clickGetBlankForm()
+            .clickClearAll()
+            .clickForm("One Question Entity Registration")
+            .clickGetSelected()
+            .clickOK(MainMenuPage())
+            .startBlankFormWithError("One Question Entity Registration", true)
+            .assertTextInDialog(R.string.unrecognized_entity_version, "2020.1.0")
+            .clickOKOnDialog(MainMenuPage())
+    }
+
+    @Test
+    fun automaticEntityFormDownload_withUnsupportedSpecVersion_completesSuccessfully_butThrowsAnErrorAfterOpeningIt() {
+        testDependencies.server.addForm("one-question-entity-registration-v2020.1.xml")
+
+        rule.withMatchExactlyProject(testDependencies.server.url)
+            .startBlankFormWithError("One Question Entity Registration", true)
+            .assertTextInDialog(R.string.unrecognized_entity_version, "2020.1.0")
+            .clickOKOnDialog(MainMenuPage())
+    }
+
+    @Test
+    fun syncEntityFormFromDisc_withUnsupportedSpecVersion_completesSuccessfully_butThrowsAnErrorAfterOpeningIt() {
+        rule.startAtFirstLaunch()
+            .clickTryCollect()
+            .copyForm("one-question-entity-registration-v2020.1.xml")
+            .startBlankFormWithError("One Question Entity Registration", true)
+            .assertTextInDialog(R.string.unrecognized_entity_version, "2020.1.0")
+            .clickOKOnDialog(MainMenuPage())
+    }
 }
diff --git a/...ect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/InvalidFormTest.kt b/...ect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/InvalidFormTest.kt
@@ -19,12 +19,16 @@ class InvalidFormTest {
     var copyFormChain: RuleChain = chain().around(rule)
 
     @Test
-    fun brokenForm_shouldNotBeVisibleOnFormList() {
+    fun brokenForm_isVisibleOnFormList_butThrowsAnErrorAfterOpeningIt() {
         rule.startAtMainMenu()
             .copyForm("invalid-form.xml")
-            .clickFillBlankForm()
-            .checkIsSnackbarErrorVisible("org.javarosa.xform.parse.XFormParseException: Cycle detected in form's relevant and calculation logic!")
-            .assertTextDoesNotExist("invalid-form")
+            .startBlankFormWithError("invalid-form", true)
+            .assertTextInDialog("An unknown error has occurred. Please ask your project leadership to email support@getodk.org with information about this form.\n" +
+                "\n" +
+                "Cycle detected in form's relevant and calculation logic!\n" +
+                "The following nodes are likely involved in the loop:\n" +
+                "/asasas/q2")
+            .clickOKOnDialog(MainMenuPage())
     }
 
     @Test
@@ -41,7 +45,7 @@ class InvalidFormTest {
     fun app_ShouldNotCrash_whenFillingFormsWithRepeatInFieldList() {
         rule.startAtMainMenu()
             .copyForm("repeat_in_field_list.xml")
-            .startBlankFormWithError("repeat_in_field_list")
+            .startBlankFormWithError("repeat_in_field_list", false)
             .clickOK(FormEntryPage("repeat_in_field_list"))
             .swipeToEndScreen()
             .clickFinalize()

diff --git a/...app/src/androidTest/java/org/odk/collect/android/feature/formmanagement/FormUpdateTest.kt b/...app/src/androidTest/java/org/odk/collect/android/feature/formmanagement/FormUpdateTest.kt
@@ -162,8 +162,7 @@ class FormUpdateTest {
             .clickGetBlankForm()
             .clickGetSelected()
             .clickOKOnDialog(MainMenuPage())
-            .startBlankFormWithError("external select")
-            .assertText(org.odk.collect.strings.R.string.error_occured)
+            .startBlankFormWithError("external select", true)
             .clickOKOnDialog(MainMenuPage())
 
         testDependencies.server.addForm(

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/regression/FillBlankFormTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/regression/FillBlankFormTest.java
@@ -324,7 +324,7 @@ public void typeMismatchErrorMessage_shouldBeDisplayed() {
                 .startBlankForm("validate")
                 .longPressOnQuestion("year")
                 .removeResponse()
-                .swipeToNextQuestionWithError()
+                .swipeToNextQuestionWithError(false)
                 .checkIsTextDisplayedOnDialog("The value \"-01-01\" can't be converted to a date.");
     }
 

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/StubOpenRosaServer.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/StubOpenRosaServer.java
@@ -7,10 +7,9 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
-import org.javarosa.xform.parse.XFormParser;
 import org.jetbrains.annotations.NotNull;
-import org.kxml2.kdom.Document;
-import org.kxml2.kdom.Element;
+import org.odk.collect.android.formmanagement.metadata.FormMetadata;
+import org.odk.collect.android.formmanagement.metadata.FormMetadataParser;
 import org.odk.collect.android.openrosa.CaseInsensitiveEmptyHeaders;
 import org.odk.collect.android.openrosa.CaseInsensitiveHeaders;
 import org.odk.collect.android.openrosa.HttpCredentialsInterface;
@@ -26,7 +25,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -317,33 +315,8 @@ private InputStream getMediaFile(URI uri) throws IOException {
     }
 
     private void addFormFromInputStream(String formXML, List<MediaFileItem> mediaFiles, InputStream formDefStream) {
-        try {
-            Document doc = XFormParser.getXMLDocument(new InputStreamReader(formDefStream));
-            Element head = doc.getRootElement().getElement(null, "head");
-            String title = (String) head.getElement(null, "title").getChild(0);
-
-            Element model = head.getElement(null, "model");
-
-            Element mainInstance = null;
-            for (int i = 0; i < model.getChildCount(); i++) {
-                Element child = model.getElement(i);
-                if (child == null) {
-                    continue;
-                }
-
-                if (child.getName().equals("instance") && child.getAttributeCount() == 0) {
-                    mainInstance = child;
-                }
-            }
-
-            Element mainInstanceRoot = mainInstance.getElement(0);
-            String id = mainInstanceRoot.getAttributeValue(null, "id");
-            String version = mainInstanceRoot.getAttributeValue(null, "version");
-
-            forms.add(new XFormItem(title, formXML, id, version, mediaFiles));
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
+        FormMetadata formMetadata = FormMetadataParser.readMetadata(formDefStream);
+        forms.add(new XFormItem(formMetadata.getTitle(), formXML, formMetadata.getId(), formMetadata.getVersion(), mediaFiles));
     }
 
     private static class XFormItem {

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/ErrorDialog.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/ErrorDialog.java
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/ErrorDialog.kt b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/ErrorDialog.kt
@@ -0,0 +1,15 @@
+package org.odk.collect.android.support.pages
+
+import org.odk.collect.strings.R
+
+class ErrorDialog : OkDialog() {
+    fun assertOnPage(isFatal: Boolean): ErrorDialog {
+        assertOnPage()
+        if (isFatal) {
+            assertText(R.string.form_cannot_be_used)
+        } else {
+            assertText(R.string.error_occured)
+        }
+        return this
+    }
+}
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormEntryPage.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormEntryPage.java
@@ -147,9 +147,9 @@ public FormEndPage swipeToEndScreen() {
         return WaitFor.waitFor(() -> new FormEndPage(formName).assertOnPage());
     }
 
-    public ErrorDialog swipeToNextQuestionWithError() {
+    public ErrorDialog swipeToNextQuestionWithError(boolean isFatal) {
         flingLeft();
-        return new ErrorDialog().assertOnPage();
+        return new ErrorDialog().assertOnPage(isFatal);
     }
 
     public FormEntryPage swipeToNextQuestionWithConstraintViolation(int constraintText) {

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/MainMenuPage.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/MainMenuPage.java
@@ -54,9 +54,9 @@ public AddNewRepeatDialog startBlankFormWithRepeatGroup(String formName, String
         return new AddNewRepeatDialog(repeatName).assertOnPage();
     }
 
-    public ErrorDialog startBlankFormWithError(String formName) {
+    public ErrorDialog startBlankFormWithError(String formName, boolean isFatal) {
         goToBlankForm(formName);
-        return new ErrorDialog().assertOnPage();
+        return new ErrorDialog().assertOnPage(isFatal);
     }
 
     public OkDialog startBlankFormWithDialog(String formName) {

diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/Page.kt b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/Page.kt
@@ -491,8 +491,8 @@ abstract class Page<T : Page<T>> {
         return this as T
     }
 
-    fun assertTextInDialog(text: Int): T {
-        return assertTextInDialog(getTranslatedString(text))
+    fun assertTextInDialog(text: Int, vararg formatArgs: Any): T {
+        return assertTextInDialog(getTranslatedString(text, *formatArgs))
     }
 
     fun closeSnackbar(): T {

diff --git a/collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java b/collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java
@@ -1512,7 +1512,11 @@ private void createErrorDialog(FormError error) {
         formError = error;
 
         alertDialog = new MaterialAlertDialogBuilder(this).create();
-        alertDialog.setTitle(getString(org.odk.collect.strings.R.string.error_occured));
+        if (formError instanceof FormError.Fatal) {
+            alertDialog.setTitle(getString(org.odk.collect.strings.R.string.form_cannot_be_used));
+        } else {
+            alertDialog.setTitle(getString(org.odk.collect.strings.R.string.error_occured));
+        }
         alertDialog.setMessage(formError.getMessage());
         DialogInterface.OnClickListener errorListener = new DialogInterface.OnClickListener() {
             @Override

diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.java b/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.java
diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormsDataService.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormsDataService.kt
@@ -6,6 +6,7 @@ import kotlinx.coroutines.flow.Flow
 import org.odk.collect.android.formmanagement.download.FormDownloadException
 import org.odk.collect.android.formmanagement.download.ServerFormDownloader
 import org.odk.collect.android.formmanagement.matchexactly.ServerFormsSynchronizer
+import org.odk.collect.android.formmanagement.metadata.FormMetadataParser
 import org.odk.collect.android.notifications.Notifier
 import org.odk.collect.android.projects.ProjectDependencyModule
 import org.odk.collect.android.state.DataKeys
@@ -213,7 +214,7 @@ private fun formDownloader(
         projectDependencyModule.formsRepository,
         File(projectDependencyModule.cacheDir),
         projectDependencyModule.formsDir,
-        FormMetadataParser(),
+        FormMetadataParser,
         clock,
         projectDependencyModule.entitiesRepository
     )

diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/LocalFormUseCases.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/LocalFormUseCases.kt
@@ -3,6 +3,8 @@ package org.odk.collect.android.formmanagement
 import android.database.SQLException
 import org.javarosa.xform.parse.XFormParser
 import org.odk.collect.android.application.Collect
+import org.odk.collect.android.formmanagement.metadata.FormMetadata
+import org.odk.collect.android.formmanagement.metadata.FormMetadataParser
 import org.odk.collect.android.utilities.FileUtils
 import org.odk.collect.androidshared.utils.Validator
 import org.odk.collect.forms.Form
@@ -213,9 +215,8 @@ object LocalFormUseCases {
         // Probably someone overwrite the file on the sdcard
         // So re-parse it and update it's information
         val builder = Form.Builder()
-        val fields: HashMap<String, String>
-        fields = try {
-            FileUtils.getMetadataFromFormDefinition(formDefFile)
+        val formMetadata: FormMetadata = try {
+            FormMetadataParser.readMetadata(formDefFile!!)
         } catch (e: RuntimeException) {
             throw IllegalArgumentException(formDefFile!!.name + " :: " + e.toString())
         } catch (e: XFormParser.ParseException) {
@@ -225,7 +226,7 @@ object LocalFormUseCases {
         // update date
         val now = System.currentTimeMillis()
         builder.date(now)
-        val title = fields[FileUtils.TITLE]
+        val title = formMetadata.title
         if (title != null) {
             builder.displayName(title)
         } else {
@@ -238,7 +239,7 @@ object LocalFormUseCases {
                     )
             )
         }
-        val formid = fields[FileUtils.FORMID]
+        val formid = formMetadata.id
         if (formid != null) {
             builder.formId(formid)
         } else {
@@ -251,11 +252,11 @@ object LocalFormUseCases {
                     )
             )
         }
-        val version = fields[FileUtils.VERSION]
+        val version = formMetadata.version
         if (version != null) {
             builder.version(version)
         }
-        val submission = fields[FileUtils.SUBMISSIONURI]
+        val submission = formMetadata.submissionUri
         if (submission != null) {
             if (Validator.isUrlValid(submission)) {
                 builder.submissionUri(submission)
@@ -269,13 +270,13 @@ object LocalFormUseCases {
                 )
             }
         }
-        val base64RsaPublicKey = fields[FileUtils.BASE64_RSA_PUBLIC_KEY]
+        val base64RsaPublicKey = formMetadata.base64RsaPublicKey
         if (base64RsaPublicKey != null) {
             builder.base64RSAPublicKey(base64RsaPublicKey)
         }
-        builder.autoDelete(fields[FileUtils.AUTO_DELETE])
-        builder.autoSend(fields[FileUtils.AUTO_SEND])
-        builder.geometryXpath(fields[FileUtils.GEOMETRY_XPATH])
+        builder.autoDelete(formMetadata.autoDelete)
+        builder.autoSend(formMetadata.autoSend)
+        builder.geometryXpath(formMetadata.geometryXPath)
 
         // Note, the path doesn't change here, but it needs to be included so the
         // update will automatically update the .md5 and the cache path.