From 7695411a79b47b0056604354b7b66418bdc91cd0 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 23:02:36 +0200 Subject: [PATCH 01/19] Added failing tests --- .../feature/formentry/EntityFormTest.kt | 32 +++++++++++++++++++ ...e-question-entity-registration-v2020.1.xml | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 test-forms/src/main/resources/forms/one-question-entity-registration-v2020.1.xml 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 index ec103112579..eeaa1d4ac3c 100644 --- 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 @@ -190,4 +190,36 @@ class EntityFormTest { .assertText("Roman Roy") .assertTextDoesNotExist("Logan Roy") } + + @Test + fun manualEntityFormDownload_withUnsupportedSpecVersion_completesSuccessfully() { + testDependencies.server.addForm("one-question-entity-registration-v2020.1.xml") + + rule.withProject(testDependencies.server) + .clickGetBlankForm() + .clickClearAll() + .clickForm("One Question Entity Registration") + .clickGetSelected() + .clickOK(MainMenuPage()) + .clickFillBlankForm() + .clickOnForm("One Question Entity Registration") + } + + @Test + fun automaticEntityFormDownload_withUnsupportedSpecVersion_completesSuccessfully() { + testDependencies.server.addForm("one-question-entity-registration-v2020.1.xml") + + rule.withMatchExactlyProject(testDependencies.server.url) + .clickFillBlankForm() + .clickOnForm("One Question Entity Registration") + } + + @Test + fun syncEntityFormFromDisc_withUnsupportedSpecVersion_completesSuccessfully() { + rule.startAtFirstLaunch() + .clickTryCollect() + .copyForm("one-question-entity-registration-v2020.1.xml") + .clickFillBlankForm() + .clickOnForm("One Question Entity Registration") + } } diff --git a/test-forms/src/main/resources/forms/one-question-entity-registration-v2020.1.xml b/test-forms/src/main/resources/forms/one-question-entity-registration-v2020.1.xml new file mode 100644 index 00000000000..7f8d50f383c --- /dev/null +++ b/test-forms/src/main/resources/forms/one-question-entity-registration-v2020.1.xml @@ -0,0 +1,32 @@ + + + + One Question Entity Registration + + + + + + + + + + + + + + + + + + + + + + + + + + + From 29e6b846c3ee59db6a641786598db54b34ef2cfa Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 21:21:48 +0200 Subject: [PATCH 02/19] Reoworked the FormMetadataParser to use simple form parsing --- .../formmanagement/FormMetadataParser.java | 24 --- .../formmanagement/FormMetadataParser.kt | 143 ++++++++++++++++++ 2 files changed, 143 insertions(+), 24 deletions(-) delete mode 100644 collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.java create mode 100644 collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt 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 deleted file mode 100644 index 227b0b759c6..00000000000 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.odk.collect.android.formmanagement; - -import org.javarosa.xform.parse.XFormParser; -import org.odk.collect.android.utilities.FileUtils; - -import java.io.File; -import java.util.HashMap; -import java.util.Map; - -import timber.log.Timber; - -public class FormMetadataParser { - public Map parse(File file, File mediaDir) throws XFormParser.ParseException { - HashMap metadata; - try { - metadata = FileUtils.getMetadataFromFormDefinition(file); - } catch (Exception e) { - Timber.e(e); - throw e; - } - - return metadata; - } -} diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt new file mode 100644 index 00000000000..39a93a6a952 --- /dev/null +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt @@ -0,0 +1,143 @@ +package org.odk.collect.android.formmanagement + +import org.javarosa.core.model.actions.setgeopoint.SetGeopointActionHandler +import org.javarosa.xform.parse.XFormParser +import org.kxml2.kdom.Element +import java.io.File +import java.io.InputStream + +object FormMetadataParser { + @JvmStatic + fun readMetadata(formFile: File): FormMetadata { + return readMetadata(formFile.inputStream()) + } + + @JvmStatic + fun readMetadata(formFile: InputStream): FormMetadata { + val doc = XFormParser.getXMLDocument(formFile.reader()) + val head = doc.getRootElement().getElement(null, "head") + val model = head.getElement(null, "model") + val body = doc.getRootElement().getElement(null, "body") + val title = head.getElement(null, "title").getChild(0).toString() + + lateinit var mainInstance: Element + var submission: Element? = null + for (i in 0 until model.childCount) { + val child = model.getElement(i) ?: continue + + if (child.name == "instance" && child.attributeCount == 0) { + mainInstance = child + } else if (child.name == "submission") { + submission = child + } + } + + val id = mainInstance.getElement(0).getAttributeValue(null, "id") + val version = mainInstance.getElement(0).getAttributeValue(null, "version") + val submissionUri = submission?.getAttributeValue(null, "action") + val base64RsaPublicKey = submission?.getAttributeValue(null, "base64RsaPublicKey") + val autoDelete = submission?.getAttributeValue(null, "auto-delete") + val autoSend = submission?.getAttributeValue(null, "auto-send") + val geometryXPath = getOverallFirstGeoPointXPath(model, mainInstance, body) + + return FormMetadata( + title, + id, + if (version.isNullOrBlank()) null else version, + submissionUri, + base64RsaPublicKey, + autoDelete, + autoSend, + geometryXPath + ) + } + + /** + * Returns an XPath path representing the first geopoint of this form definition or null if the + * definition does not contain any field of type geopoint. + * + * The first geopoint is either of: + * (1) the first geopoint in the body that is not in a repeat + * (2) if the form has a setgeopoint action, the first geopoint in the instance that occurs + * before (1) or (1) if there is no geopoint defined before it in the instance. + */ + private fun getOverallFirstGeoPointXPath(model: Element, mainInstance: Element, element: Element): String? { + val firstTopLevelBodyGeoPoint = getFirstToplevelBodyGeoPointXPath(model, mainInstance, element) + + return if (containsSetgeopointAction(model)) { + getInstanceGeoPointBeforeXPath(firstTopLevelBodyGeoPoint, model, mainInstance) + } else { + firstTopLevelBodyGeoPoint + } + } + + /** + * Returns the reference of the first geopoint in the body that is not in a repeat. + */ + private fun getFirstToplevelBodyGeoPointXPath(model: Element, mainInstance: Element, element: Element): String? { + for (elementId in 0 until element.childCount) { + val child = element.getElement(elementId) + val ref = child.getAttributeValue(null, "ref") + if (child.name == "input" && isGeopoint(model, ref)) { + return ref + } else if (child.name == "group") { + return getFirstToplevelBodyGeoPointXPath(model, mainInstance, child) + } + } + return null + } + + /** + * Returns the XPath path for the first geopoint in the primary instance that is before the given + * reference and not in a repeat. + */ + private fun getInstanceGeoPointBeforeXPath(firstTopLevelBodyGeoPoint: String?, model: Element, mainInstance: Element): String? { + val mainInstanceRoot = mainInstance.getElement(0) + for (elementId in 0 until mainInstanceRoot.childCount) { + val child = mainInstanceRoot.getElement(elementId) + val ref = "/${mainInstanceRoot.name}/${child.name}" + if (ref == firstTopLevelBodyGeoPoint) { + return null + } else if (isGeopoint(model, ref)) { + return ref + } else if (child.childCount > 0) { + return getInstanceGeoPointBeforeXPath(firstTopLevelBodyGeoPoint, model, child) + } + } + return null + } + + private fun containsSetgeopointAction(model: Element): Boolean { + for (elementId in 0 until model.childCount) { + val child = model.getElement(elementId) + if (child.name == SetGeopointActionHandler.ELEMENT_NAME) { + return true + } + } + return false + } + + private fun isGeopoint(model: Element, ref: String): Boolean { + for (elementId in 0 until model.childCount) { + val child = model.getElement(elementId) + if (child.name == "bind" && + child.getAttributeValue(null, "nodeset") == ref && + child.getAttributeValue(null, "type") == "geopoint" + ) { + return true + } + } + return false + } +} + +data class FormMetadata( + val title: String?, + val id: String?, + val version: String?, + val submissionUri: String?, + val base64RsaPublicKey: String?, + val autoDelete: String?, + val autoSend: String?, + val geometryXPath: String? +) From c2f9a0f4f44cc2d47f50a17abf25be691694bb41 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 21:23:35 +0200 Subject: [PATCH 03/19] Use the new form parser --- .../android/support/StubOpenRosaServer.java | 35 +--- .../formmanagement/FormsDataService.kt | 2 +- .../formmanagement/LocalFormUseCases.kt | 21 ++- .../download/ServerFormDownloader.java | 44 +++-- .../collect/android/utilities/FileUtils.java | 159 ------------------ .../FormMetadataParserTest.java | 22 +-- .../download/ServerFormDownloaderTest.java | 52 ++---- .../android/utilities/FileUtilsTest.java | 82 ++++----- 8 files changed, 103 insertions(+), 314 deletions(-) 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 index 44ee96c92f8..5c627f82a68 100644 --- 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.FormMetadata; +import org.odk.collect.android.formmanagement.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 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/main/java/org/odk/collect/android/formmanagement/FormsDataService.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormsDataService.kt index be41d7cab5a..8e623dcf725 100644 --- 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 @@ -213,7 +213,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 index 6b014b9dde7..6b25728be81 100644 --- 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 @@ -213,9 +213,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 - 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 +224,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 +237,7 @@ object LocalFormUseCases { ) ) } - val formid = fields[FileUtils.FORMID] + val formid = formMetadata.id if (formid != null) { builder.formId(formid) } else { @@ -251,11 +250,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 +268,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. diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java b/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java index 1e9e318d933..89556eb66ae 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java @@ -2,8 +2,8 @@ import static org.odk.collect.android.utilities.FileUtils.interuptablyWriteFile; -import org.javarosa.xform.parse.XFormParser; import org.jetbrains.annotations.NotNull; +import org.odk.collect.android.formmanagement.FormMetadata; import org.odk.collect.android.formmanagement.FormMetadataParser; import org.odk.collect.android.formmanagement.ServerFormDetails; import org.odk.collect.android.formmanagement.ServerFormUseCases; @@ -24,7 +24,6 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.UUID; import java.util.function.Supplier; @@ -116,17 +115,16 @@ private void processOneForm(ServerFormDetails fd, OngoingWorkListener stateListe throw new FormDownloadException.DownloadingInterrupted(); } - Map parsedFields = null; + FormMetadata formMetadata = null; if (fileResult.isNew) { try { final long start = System.currentTimeMillis(); Timber.i("Parsing document %s", fileResult.file.getAbsolutePath()); - parsedFields = formMetadataParser - .parse(fileResult.file, new File(tempMediaPath)); + formMetadata = formMetadataParser.readMetadata(fileResult.file); Timber.i("Parse finished in %.3f seconds.", (System.currentTimeMillis() - start) / 1000F); - } catch (RuntimeException | XFormParser.ParseException e) { + } catch (RuntimeException e) { throw new FormDownloadException.FormParsingError(); } } @@ -135,24 +133,24 @@ private void processOneForm(ServerFormDetails fd, OngoingWorkListener stateListe throw new FormDownloadException.DownloadingInterrupted(); } - if (fileResult.isNew && !isSubmissionOk(parsedFields)) { + if (fileResult.isNew && !isSubmissionOk(formMetadata)) { throw new FormDownloadException.InvalidSubmission(); } try { - installEverything(tempMediaPath, fileResult, parsedFields, formsDirPath, newAttachmentsDetected); + installEverything(tempMediaPath, fileResult, formMetadata, formsDirPath, newAttachmentsDetected); } catch (FormDownloadException.DiskError e) { cleanUp(fileResult, tempMediaPath); throw e; } } - private boolean isSubmissionOk(Map parsedFields) { - String submission = parsedFields.get(FileUtils.SUBMISSIONURI); + private boolean isSubmissionOk(FormMetadata formMetadata) { + String submission = formMetadata.getSubmissionUri(); return submission == null || Validator.isUrlValid(submission); } - private void installEverything(String tempMediaPath, FileResult fileResult, Map parsedFields, String formsDirPath, boolean newAttachmentsDetected) throws FormDownloadException.DiskError { + private void installEverything(String tempMediaPath, FileResult fileResult, FormMetadata formMetadata, String formsDirPath, boolean newAttachmentsDetected) throws FormDownloadException.DiskError { FormResult formResult; File formFile; @@ -173,7 +171,7 @@ private void installEverything(String tempMediaPath, FileResult fileResult, Map< } // Save form in database - formResult = findOrCreateForm(formFile, parsedFields); + formResult = findOrCreateForm(formFile, formMetadata); // move the media files in the media folder if (tempMediaPath != null) { @@ -210,32 +208,32 @@ private void cleanUp(FileResult fileResult, String tempMediaPath) { } } - private FormResult findOrCreateForm(File formFile, Map formInfo) { + private FormResult findOrCreateForm(File formFile, FormMetadata formMetadata) { final String formFilePath = formFile.getAbsolutePath(); String mediaPath = FileUtils.constructMediaPath(formFilePath); Form existingForm = formsRepository.getOneByPath(formFile.getAbsolutePath()); if (existingForm == null) { - Form newForm = saveNewForm(formInfo, formFile, mediaPath); + Form newForm = saveNewForm(formMetadata, formFile, mediaPath); return new FormResult(newForm, true); } else { return new FormResult(existingForm, false); } } - private Form saveNewForm(Map formInfo, File formFile, String mediaPath) { + private Form saveNewForm(FormMetadata formMetadata, File formFile, String mediaPath) { Form form = new Form.Builder() .formFilePath(formFile.getAbsolutePath()) .formMediaPath(mediaPath) - .displayName(formInfo.get(FileUtils.TITLE)) - .version(formInfo.get(FileUtils.VERSION)) - .formId(formInfo.get(FileUtils.FORMID)) - .submissionUri(formInfo.get(FileUtils.SUBMISSIONURI)) - .base64RSAPublicKey(formInfo.get(FileUtils.BASE64_RSA_PUBLIC_KEY)) - .autoDelete(formInfo.get(FileUtils.AUTO_DELETE)) - .autoSend(formInfo.get(FileUtils.AUTO_SEND)) - .geometryXpath(formInfo.get(FileUtils.GEOMETRY_XPATH)) + .displayName(formMetadata.getTitle()) + .version(formMetadata.getVersion()) + .formId(formMetadata.getId()) + .submissionUri(formMetadata.getSubmissionUri()) + .base64RSAPublicKey(formMetadata.getBase64RsaPublicKey()) + .autoDelete(formMetadata.getAutoDelete()) + .autoSend(formMetadata.getAutoSend()) + .geometryXpath(formMetadata.getGeometryXPath()) .build(); return formsRepository.save(form); diff --git a/collect_app/src/main/java/org/odk/collect/android/utilities/FileUtils.java b/collect_app/src/main/java/org/odk/collect/android/utilities/FileUtils.java index f49491573a7..832baa70ac5 100644 --- a/collect_app/src/main/java/org/odk/collect/android/utilities/FileUtils.java +++ b/collect_app/src/main/java/org/odk/collect/android/utilities/FileUtils.java @@ -28,20 +28,8 @@ import com.google.common.base.CharMatcher; import org.apache.commons.io.IOUtils; -import org.javarosa.core.model.Constants; -import org.javarosa.core.model.FormDef; -import org.javarosa.core.model.GroupDef; -import org.javarosa.core.model.IFormElement; -import org.javarosa.core.model.QuestionDef; -import org.javarosa.core.model.actions.setgeopoint.SetGeopointActionHandler; -import org.javarosa.core.model.instance.FormInstance; -import org.javarosa.core.model.instance.TreeElement; -import org.javarosa.core.model.instance.TreeReference; -import org.javarosa.xform.parse.XFormParser; -import org.javarosa.xform.util.XFormUtils; import org.odk.collect.android.application.Collect; import org.odk.collect.async.OngoingWorkListener; -import org.odk.collect.shared.strings.StringUtils; import java.io.File; import java.io.FileInputStream; @@ -55,11 +43,8 @@ import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; -import java.util.Set; import timber.log.Timber; @@ -69,16 +54,6 @@ * @author Carl Hartung (carlhartung@gmail.com) */ public final class FileUtils { - - public static final String FORMID = "formid"; - public static final String VERSION = "version"; // arbitrary string in OpenRosa 1.0 - public static final String TITLE = "title"; - public static final String SUBMISSIONURI = "submission"; - public static final String BASE64_RSA_PUBLIC_KEY = "base64RsaPublicKey"; - public static final String AUTO_DELETE = "autoDelete"; - public static final String AUTO_SEND = "autoSend"; - public static final String GEOMETRY_XPATH = "geometryXpath"; - /** Suffix for the form media directory. */ public static final String MEDIA_SUFFIX = "-media"; @@ -163,127 +138,6 @@ private static String actualCopy(File sourceFile, File destFile) { } } - /** - * Given a form definition file, return a map containing form metadata. The form ID is required - * by the specification and will always be included. Title and version are optionally included. - * If the form definition contains a submission block, any or all of submission URI, base 64 RSA - * public key, auto-delete and auto-send may be included. - */ - public static HashMap getMetadataFromFormDefinition(File formDefinitionXml) throws XFormParser.ParseException { - FormDef formDef = XFormUtils.getFormFromFormXml(formDefinitionXml.getAbsolutePath(), "jr://file/" + LAST_SAVED_FILENAME); - - final HashMap fields = new HashMap<>(); - - fields.put(TITLE, formDef.getTitle()); - fields.put(FORMID, formDef.getMainInstance().getRoot().getAttributeValue(null, "id")); - String version = formDef.getMainInstance().getRoot().getAttributeValue(null, "version"); - if (version != null && StringUtils.isBlank(version)) { - version = null; - } - fields.put(VERSION, version); - - if (formDef.getSubmissionProfile() != null) { - fields.put(SUBMISSIONURI, formDef.getSubmissionProfile().getAction()); - - final String key = formDef.getSubmissionProfile().getAttribute("base64RsaPublicKey"); - if (key != null && key.trim().length() > 0) { - fields.put(BASE64_RSA_PUBLIC_KEY, key.trim()); - } - - fields.put(AUTO_DELETE, formDef.getSubmissionProfile().getAttribute("auto-delete")); - fields.put(AUTO_SEND, formDef.getSubmissionProfile().getAttribute("auto-send")); - } - - fields.put(GEOMETRY_XPATH, getOverallFirstGeoPoint(formDef)); - return fields; - } - - /** - * Returns an XPath path representing the first geopoint of this form definition or null if the - * definition does not contain any field of type geopoint. - * - * The first geopoint is either of: - * (1) the first geopoint in the body that is not in a repeat - * (2) if the form has a setgeopoint action, the first geopoint in the instance that occurs - * before (1) or (1) if there is no geopoint defined before it in the instance. - */ - private static String getOverallFirstGeoPoint(FormDef formDef) { - TreeReference firstTopLevelBodyGeoPoint = getFirstToplevelBodyGeoPoint(formDef); - - if (!formDef.hasAction(SetGeopointActionHandler.ELEMENT_NAME)) { - return firstTopLevelBodyGeoPoint == null ? null : firstTopLevelBodyGeoPoint.toString(false); - } else { - return getInstanceGeoPointBefore(firstTopLevelBodyGeoPoint, formDef.getMainInstance().getRoot()); - } - } - - /** - * Returns the reference of the first geopoint in the body that is not in a repeat. - */ - private static TreeReference getFirstToplevelBodyGeoPoint(FormDef formDef) { - if (formDef.getChildren().size() == 0) { - return null; - } else { - return getFirstTopLevelBodyGeoPoint(formDef, formDef.getMainInstance()); - } - } - - /** - * Returns the reference of the first child of the given element that is of type geopoint and - * is not contained in a repeat. - */ - private static TreeReference getFirstTopLevelBodyGeoPoint(IFormElement element, FormInstance primaryInstance) { - if (element instanceof QuestionDef) { - QuestionDef question = (QuestionDef) element; - int dataType = primaryInstance.resolveReference((TreeReference) element.getBind().getReference()).getDataType(); - - if (dataType == Constants.DATATYPE_GEOPOINT) { - return (TreeReference) question.getBind().getReference(); - } - } else if (element instanceof FormDef || element instanceof GroupDef) { - if (element instanceof GroupDef && ((GroupDef) element).getRepeat()) { - return null; - } else { - for (IFormElement child : element.getChildren()) { - // perform recursive depth-first search - TreeReference geoRef = getFirstTopLevelBodyGeoPoint(child, primaryInstance); - if (geoRef != null) { - return geoRef; - } - } - } - } - - return null; - } - - /** - * Returns the XPath path for the first geopoint in the primary instance that is before the given - * reference and not in a repeat. - */ - private static String getInstanceGeoPointBefore(TreeReference firstBodyGeoPoint, TreeElement element) { - if (element.getRef().equals(firstBodyGeoPoint)) { - return null; - } else if (element.getDataType() == Constants.DATATYPE_GEOPOINT) { - return element.getRef().toString(false); - } else if (element.hasChildren()) { - Set childrenToAvoid = new HashSet<>(); - - for (int i = 0; i < element.getNumChildren(); i++) { - if (element.getChildAt(i).getMultiplicity() == TreeReference.INDEX_TEMPLATE) { - childrenToAvoid.addAll(element.getChildrenWithName(element.getChildAt(i).getName())); - } else if (!childrenToAvoid.contains(element.getChildAt(i))) { - String geoPath = getInstanceGeoPointBefore(firstBodyGeoPoint, element.getChildAt(i)); - if (geoPath != null) { - return geoPath; - } - } - } - } - - return null; - } - public static void deleteAndReport(File file) { if (file != null && file.exists()) { // remove garbage @@ -403,19 +257,6 @@ public static void write(File file, byte[] data) { } } - /** Sorts file paths as if sorting the path components and extensions lexicographically. */ - public static int comparePaths(String a, String b) { - // Regular string compareTo() is incorrect, because it will sort "/" and "." - // after other punctuation (e.g. "foo/bar" will sort AFTER "foo-2/bar" and - // "pic.jpg" will sort AFTER "pic-2.jpg"). Replacing these delimiters with - // '\u0000' and '\u0001' causes paths to sort correctly (assuming the paths - // don't already contain '\u0000' or '\u0001'). This is a bit of a hack, - // but it's a lot simpler and faster than comparing components one by one. - String sortKeyA = a.replace('/', '\u0000').replace('.', '\u0001'); - String sortKeyB = b.replace('/', '\u0000').replace('.', '\u0001'); - return sortKeyA.compareTo(sortKeyB); - } - public static String getFileExtension(String fileName) { int dotIndex = fileName.lastIndexOf('.'); if (dotIndex == -1) { diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java index f117412e759..cfa8a01cbae 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java @@ -7,7 +7,6 @@ import org.odk.collect.android.utilities.FileUtils; import java.io.File; -import java.util.Map; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -29,9 +28,8 @@ public void canParseFormWithExternalSecondaryInstance() throws Exception { File externalInstance = new File(mediaDir, "external-data.xml"); FileUtils.write(externalInstance, EXTERNAL_INSTANCE.getBytes()); - FormMetadataParser formMetadataParser = new FormMetadataParser(); - Map metaData = formMetadataParser.parse(formXml, mediaDir); - assertThat(metaData.get(FileUtils.FORMID), is("basic-external-xml-instance")); + FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); + assertThat(formMetadata.getId(), is("basic-external-xml-instance")); } @Test @@ -42,9 +40,8 @@ public void canParseFormWithCSVExternalSecondaryInstance() throws Exception { File externalInstance = new File(mediaDir, "external-data.csv"); FileUtils.write(externalInstance, CSV_EXTERNAL_INSTANCE.getBytes()); - FormMetadataParser formMetadataParser = new FormMetadataParser(); - Map metaData = formMetadataParser.parse(formXml, mediaDir); - assertThat(metaData.get(FileUtils.FORMID), is("basic-external-csv-instance")); + FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); + assertThat(formMetadata.getId(), is("basic-external-csv-instance")); } @Test @@ -52,9 +49,8 @@ public void canParseFormWithLastSaved() throws Exception { File formXml = File.createTempFile("form", ".xml"); FileUtils.write(formXml, LAST_SAVED.getBytes()); - FormMetadataParser formMetadataParser = new FormMetadataParser(); - Map metaData = formMetadataParser.parse(formXml, mediaDir); - assertThat(metaData.get(FileUtils.FORMID), is("basic-last-saved")); + FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); + assertThat(formMetadata.getId(), is("basic-last-saved")); } @Test @@ -62,9 +58,7 @@ public void doesNotLeaveFilesInMediaDir() throws Exception { File formXml = File.createTempFile("form", ".xml"); FileUtils.write(formXml, LAST_SAVED.getBytes()); - FormMetadataParser formMetadataParser = new FormMetadataParser(); - formMetadataParser.parse(formXml, mediaDir); - + FormMetadataParser.readMetadata(formXml); assertThat(mediaDir.listFiles().length, is(0)); } @@ -156,4 +150,4 @@ public void doesNotLeaveFilesInMediaDir() throws Exception { " \n" + " \n" + ""; -} \ No newline at end of file +} diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java index ad68e6fe30f..94874dd4bb6 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java @@ -2,7 +2,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; @@ -16,12 +15,9 @@ import static org.odk.collect.formstest.FormUtils.buildForm; import static org.odk.collect.formstest.FormUtils.createXFormBody; import static java.util.Arrays.asList; -import static java.util.Arrays.stream; -import static java.util.stream.Collectors.toList; import com.google.common.io.Files; -import org.javarosa.xform.parse.XFormParser; import org.junit.Test; import org.odk.collect.android.formmanagement.FormMetadataParser; import org.odk.collect.android.formmanagement.ServerFormDetails; @@ -44,7 +40,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.function.Supplier; @SuppressWarnings("PMD.DoubleBraceInitialization") @@ -73,7 +68,7 @@ public void downloadsAndSavesForm() throws Exception { FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); List
allForms = formsRepository.getAll(); @@ -102,7 +97,7 @@ public void whenFormToDownloadIsUpdate_savesNewVersionAlongsideOldVersion() thro FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); String xformUpdate = createXFormBody("id", "updated"); @@ -146,7 +141,7 @@ public void whenFormToDownloadIsUpdate_withSameFormIdAndVersion_replacePreExisti when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); List formsBeforeUpdate = formsRepository.getAllByFormIdAndVersion("id", "version"); @@ -199,7 +194,7 @@ public void whenFormListMissingHash_throwsError() throws Exception { FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); try { downloader.downloadForm(serverFormDetails, null, null); fail("Expected exception because of missing form hash"); @@ -229,7 +224,7 @@ public void whenFormHasMediaFiles_downloadsAndSavesFormAndMediaFiles() throws Ex when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents2".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); List allForms = formsRepository.getAll(); @@ -271,7 +266,7 @@ public void whenFormHasMediaFiles_andIsFormToDownloadIsUpdate_doesNotRedownloadM when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents2".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); String xformUpdate = createXFormBody("id", "updated"); @@ -316,7 +311,7 @@ public void whenFormHasMediaFiles_andIsFormToDownloadIsUpdate_downloadsFilesWith when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents2".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); String xformUpdate = createXFormBody("id", "updated"); @@ -368,18 +363,7 @@ public void whenFormHasMediaFiles_downloadsAndSavesFormAndMediaFiles_beforeParsi when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents2".getBytes())); - FormMetadataParser formMetadataParser = new FormMetadataParser() { - @Override - public Map parse(File file, File mediaDir) throws XFormParser.ParseException { - File[] mediaFiles = mediaDir.listFiles(); - assertThat(mediaFiles.length, is(2)); - assertThat(stream(mediaFiles).map(File::getName).collect(toList()), containsInAnyOrder("file1", "file2")); - - return super.parse(file, mediaDir); - } - }; - - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), formMetadataParser, clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); } @@ -402,7 +386,7 @@ public void whenFormHasMediaFiles_andFetchingMediaFileFails_throwsFetchErrorAndD when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); when(formSource.fetchMediaFile("http://file1")).thenThrow(new FormSourceException.FetchError()); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); try { downloader.downloadForm(serverFormDetails, null, null); @@ -436,7 +420,7 @@ public void whenFormHasMediaFiles_andFileExistsInMediaDirPath_throwsDiskExceptio // Create file where media dir would go assertThat(new File(formsDir, "Form-media").createNewFile(), is(true)); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); try { downloader.downloadForm(serverFormDetails, null, null); @@ -469,7 +453,7 @@ public void beforeDownloadingEachMediaFile_reportsProgress() throws Exception { when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents".getBytes())); when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); RecordingProgressReporter progressReporter = new RecordingProgressReporter(); downloader.downloadForm(serverFormDetails, progressReporter, null); @@ -498,7 +482,7 @@ public void whenFormIsSoftDeleted_unDeletesForm() throws Exception { FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); assertThat(formsRepository.get(1L).isDeleted(), is(false)); } @@ -530,7 +514,7 @@ public void whenMultipleFormsWithSameFormIdVersionDeleted_reDownloadUnDeletesFor FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform2.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); downloader.downloadForm(serverFormDetails, null, null); assertThat(formsRepository.get(1L).isDeleted(), is(true)); assertThat(formsRepository.get(2L).isDeleted(), is(false)); @@ -553,7 +537,7 @@ public void whenFormAlreadyDownloaded_formRemainsOnDevice() throws Exception { FormSource formSource = mock(FormSource.class); when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); // Initial download downloader.downloadForm(serverFormDetails, null, null); @@ -598,7 +582,7 @@ public void whenFormAlreadyDownloaded_andFormHasNewMediaFiles_updatesMediaFilesA when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); // Initial download downloader.downloadForm(serverFormDetails, null, null); @@ -661,7 +645,7 @@ public void whenFormAlreadyDownloaded_andFormHasNewMediaFiles_andMediaFetchFails when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents".getBytes())); - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); // Initial download downloader.downloadForm(serverFormDetails, null, null); @@ -710,7 +694,7 @@ public void afterDownloadingXForm_cancelling_throwsDownloadingInterruptedExcepti null); CancelAfterFormDownloadFormSource formListApi = new CancelAfterFormDownloadFormSource(xform); - ServerFormDownloader downloader = new ServerFormDownloader(formListApi, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formListApi, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); try { downloader.downloadForm(serverFormDetails, null, formListApi); @@ -739,7 +723,7 @@ public void afterDownloadingMediaFile_cancelling_throwsDownloadingInterruptedExc ))); CancelAfterMediaFileDownloadFormSource formListApi = new CancelAfterMediaFileDownloadFormSource(xform); - ServerFormDownloader downloader = new ServerFormDownloader(formListApi, formsRepository, cacheDir, formsDir.getAbsolutePath(), new FormMetadataParser(), clock, entitiesRepository); + ServerFormDownloader downloader = new ServerFormDownloader(formListApi, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); try { downloader.downloadForm(serverFormDetails, null, formListApi); diff --git a/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java b/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java index a72be3830e2..20c4db49118 100644 --- a/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java @@ -1,14 +1,14 @@ package org.odk.collect.android.utilities; import org.hamcrest.Matchers; -import org.javarosa.xform.parse.XFormParser; import org.junit.Test; +import org.odk.collect.android.formmanagement.FormMetadata; +import org.odk.collect.android.formmanagement.FormMetadataParser; import java.io.BufferedWriter; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.util.HashMap; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; @@ -30,7 +30,7 @@ public void mediaDirNameIsCorrect() { assertEquals(expected, FileUtils.constructMediaPath("sample-file.docx")); } - @Test public void getMetadataFromFormDefinition_withoutSubmission_returnsMetaDataFields() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withoutSubmission_returnsMetaDataFields() throws IOException { String simpleForm = "\n" + " metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("My Survey")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("mysurvey")); - assertThat(metadataFromFormDefinition.get(FileUtils.VERSION), is(nullValue())); - assertThat(metadataFromFormDefinition.get(FileUtils.BASE64_RSA_PUBLIC_KEY), is(nullValue())); + assertThat(formMetadata.getTitle(), is("My Survey")); + assertThat(formMetadata.getId(), is("mysurvey")); + assertThat(formMetadata.getVersion(), is(nullValue())); + assertThat(formMetadata.getBase64RsaPublicKey(), is(nullValue())); } - @Test public void getMetadataFromFormDefinition_withSubmission_returnsMetaDataFields() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withSubmission_returnsMetaDataFields() throws IOException { String submissionForm = "\n" + " metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("My Survey")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("mysurvey")); - assertThat(metadataFromFormDefinition.get(FileUtils.VERSION), is("2014083101")); - assertThat(metadataFromFormDefinition.get(FileUtils.SUBMISSIONURI), is("foo")); - assertThat(metadataFromFormDefinition.get(FileUtils.AUTO_SEND), is("bar")); - assertThat(metadataFromFormDefinition.get(FileUtils.AUTO_DELETE), is("baz")); - assertThat(metadataFromFormDefinition.get(FileUtils.BASE64_RSA_PUBLIC_KEY), is("quux")); - assertThat(metadataFromFormDefinition.get(FileUtils.GEOMETRY_XPATH), is(nullValue())); + assertThat(formMetadata.getTitle(), is("My Survey")); + assertThat(formMetadata.getId(), is("mysurvey")); + assertThat(formMetadata.getVersion(), is("2014083101")); + assertThat(formMetadata.getSubmissionUri(), is("foo")); + assertThat(formMetadata.getAutoSend(), is("bar")); + assertThat(formMetadata.getAutoDelete(), is("baz")); + assertThat(formMetadata.getBase64RsaPublicKey(), is("quux")); + assertThat(formMetadata.getGeometryXPath(), is(nullValue())); } - @Test public void getMetadataFromFormDefinition_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() throws IOException { String submissionForm = "\n" + "\n" + @@ -139,14 +139,14 @@ public void mediaDirNameIsCorrect() { out.write(submissionForm); out.close(); - HashMap metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("Two geopoints")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("two-geopoints")); - assertThat(metadataFromFormDefinition.get(FileUtils.GEOMETRY_XPATH), is("/data/location1")); + assertThat(formMetadata.getTitle(), is("Two geopoints")); + assertThat(formMetadata.getId(), is("two-geopoints")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); } - @Test public void getMetadataFromFormDefinition_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() throws IOException { String submissionForm = "\n" + "\n" + @@ -181,14 +181,14 @@ public void mediaDirNameIsCorrect() { out.write(submissionForm); out.close(); - HashMap metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("Two geopoints in group")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("two-geopoints-group")); - assertThat(metadataFromFormDefinition.get(FileUtils.GEOMETRY_XPATH), is("/data/my-group/location1")); + assertThat(formMetadata.getTitle(), is("Two geopoints in group")); + assertThat(formMetadata.getId(), is("two-geopoints-group")); + assertThat(formMetadata.getGeometryXPath(), is("/data/my-group/location1")); } - @Test public void getMetadataFromFormDefinition_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() throws IOException { String submissionForm = "\n" + "\n" + @@ -222,14 +222,14 @@ public void mediaDirNameIsCorrect() { out.write(submissionForm); out.close(); - HashMap metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("Two geopoints repeat")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("two-geopoints-repeat")); - assertThat(metadataFromFormDefinition.get(FileUtils.GEOMETRY_XPATH), is("/data/location2")); + assertThat(formMetadata.getTitle(), is("Two geopoints repeat")); + assertThat(formMetadata.getId(), is("two-geopoints-repeat")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location2")); } - @Test public void getMetadataFromFormDefinition_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() throws IOException, XFormParser.ParseException { + @Test public void readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() throws IOException { String submissionForm = "\n" + " metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.TITLE), is("Setgeopoint before")); - assertThat(metadataFromFormDefinition.get(FileUtils.FORMID), is("set-geopoint-before")); - assertThat(metadataFromFormDefinition.get(FileUtils.GEOMETRY_XPATH), is("/data/location1")); + assertThat(formMetadata.getTitle(), is("Setgeopoint before")); + assertThat(formMetadata.getId(), is("set-geopoint-before")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); } - @Test public void whenFormVersionIsEmpty_shouldBeTreatedAsNull() throws IOException, XFormParser.ParseException { + @Test public void whenFormVersionIsEmpty_shouldBeTreatedAsNull() throws IOException { String simpleForm = "\n" + " metadataFromFormDefinition = FileUtils.getMetadataFromFormDefinition(temp); - assertThat(metadataFromFormDefinition.get(FileUtils.VERSION), is(nullValue())); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + assertThat(formMetadata.getVersion(), is(nullValue())); } @Test From 6afec6e125a82f6ec26d5012cfa603030de0fd02 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 23:14:13 +0200 Subject: [PATCH 04/19] Removed redundant tests --- .../FormMetadataParserTest.java | 148 ------------------ 1 file changed, 148 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java index cfa8a01cbae..caab08873e1 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java @@ -1,153 +1,5 @@ package org.odk.collect.android.formmanagement; -import com.google.common.io.Files; - -import org.junit.Before; -import org.junit.Test; -import org.odk.collect.android.utilities.FileUtils; - -import java.io.File; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; - public class FormMetadataParserTest { - private File mediaDir; - - @Before - public void setup() { - mediaDir = Files.createTempDir(); - } - - @Test - public void canParseFormWithExternalSecondaryInstance() throws Exception { - File formXml = File.createTempFile("form", ".xml"); - FileUtils.write(formXml, EXTERNAL_SECONDARY_INSTANCE.getBytes()); - - File externalInstance = new File(mediaDir, "external-data.xml"); - FileUtils.write(externalInstance, EXTERNAL_INSTANCE.getBytes()); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); - assertThat(formMetadata.getId(), is("basic-external-xml-instance")); - } - - @Test - public void canParseFormWithCSVExternalSecondaryInstance() throws Exception { - File formXml = File.createTempFile("form", ".xml"); - FileUtils.write(formXml, CSV_EXTERNAL_SECONDARY_INSTANCE.getBytes()); - - File externalInstance = new File(mediaDir, "external-data.csv"); - FileUtils.write(externalInstance, CSV_EXTERNAL_INSTANCE.getBytes()); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); - assertThat(formMetadata.getId(), is("basic-external-csv-instance")); - } - - @Test - public void canParseFormWithLastSaved() throws Exception { - File formXml = File.createTempFile("form", ".xml"); - FileUtils.write(formXml, LAST_SAVED.getBytes()); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(formXml); - assertThat(formMetadata.getId(), is("basic-last-saved")); - } - - @Test - public void doesNotLeaveFilesInMediaDir() throws Exception { - File formXml = File.createTempFile("form", ".xml"); - FileUtils.write(formXml, LAST_SAVED.getBytes()); - - FormMetadataParser.readMetadata(formXml); - assertThat(mediaDir.listFiles().length, is(0)); - } - - private static final String EXTERNAL_SECONDARY_INSTANCE = "\n" + - " \n" + - " External Secondary Instance\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - private static final String EXTERNAL_INSTANCE = "\n" + - " \n" + - " \n" + - " a\n" + - " \n" + - " \n" + - " \n" + - " b\n" + - " \n" + - " \n" + - " \n" + - " c\n" + - " \n" + - " "; - - private static final String CSV_EXTERNAL_SECONDARY_INSTANCE = "\n" + - " \n" + - " basic-external-csv-instance\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - private static final String CSV_EXTERNAL_INSTANCE = "label,name\n" + - "A, a\n" + - "B, b\n" + - "C, c\n"; - - private static final String LAST_SAVED = "\n" + - " \n" + - " basic-last-saved\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; } From 5ac242d95b7fba469bf2f6c9bca7f7f10a552802 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 23:17:00 +0200 Subject: [PATCH 05/19] Moved testing FormMetadataParser to FormMetadataParserTest --- .../FormMetadataParserTest.java | 276 ++++++++++++++++++ .../android/utilities/FileUtilsTest.java | 271 ----------------- 2 files changed, 276 insertions(+), 271 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java index caab08873e1..2e2b35db9bc 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java @@ -1,5 +1,281 @@ package org.odk.collect.android.formmanagement; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; + +import org.junit.Test; + +import java.io.BufferedWriter; +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; + public class FormMetadataParserTest { + @Test + public void readMetadata_withoutSubmission_returnsMetaDataFields() throws IOException { + String simpleForm = "\n" + + "\n" + + " \n" + + " My Survey\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n" + + " \n" + + ""; + File temp = File.createTempFile("simple_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(simpleForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("My Survey")); + assertThat(formMetadata.getId(), is("mysurvey")); + assertThat(formMetadata.getVersion(), is(nullValue())); + assertThat(formMetadata.getBase64RsaPublicKey(), is(nullValue())); + } + + @Test public void readMetadata_withSubmission_returnsMetaDataFields() throws IOException { + String submissionForm = "\n" + + "\n" + + " \n" + + " My Survey\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n" + + " \n" + + ""; + + File temp = File.createTempFile("submission_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(submissionForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("My Survey")); + assertThat(formMetadata.getId(), is("mysurvey")); + assertThat(formMetadata.getVersion(), is("2014083101")); + assertThat(formMetadata.getSubmissionUri(), is("foo")); + assertThat(formMetadata.getAutoSend(), is("bar")); + assertThat(formMetadata.getAutoDelete(), is("baz")); + assertThat(formMetadata.getBase64RsaPublicKey(), is("quux")); + assertThat(formMetadata.getGeometryXPath(), is(nullValue())); + } + + @Test public void readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() throws IOException { + String submissionForm = "\n" + + "\n" + + " \n" + + " Two geopoints\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + File temp = File.createTempFile("geopoints_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(submissionForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("Two geopoints")); + assertThat(formMetadata.getId(), is("two-geopoints")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); + } + + @Test public void readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() throws IOException { + String submissionForm = "\n" + + "\n" + + " \n" + + " Two geopoints in group\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n" + + " \n" + + " \n" + + ""; + + File temp = File.createTempFile("geopoints_group_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(submissionForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("Two geopoints in group")); + assertThat(formMetadata.getId(), is("two-geopoints-group")); + assertThat(formMetadata.getGeometryXPath(), is("/data/my-group/location1")); + } + + @Test public void readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() throws IOException { + String submissionForm = "\n" + + "\n" + + " \n" + + " Two geopoints repeat\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + File temp = File.createTempFile("geopoints_repeat_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(submissionForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("Two geopoints repeat")); + assertThat(formMetadata.getId(), is("two-geopoints-repeat")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location2")); + } + + @Test public void readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() throws IOException { + String submissionForm = "\n" + + "\n" + + " \n" + + " Setgeopoint before\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + File temp = File.createTempFile("geopoints_repeat_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(submissionForm); + out.close(); + + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + + assertThat(formMetadata.getTitle(), is("Setgeopoint before")); + assertThat(formMetadata.getId(), is("set-geopoint-before")); + assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); + } + + @Test public void whenFormVersionIsEmpty_shouldBeTreatedAsNull() throws IOException { + String simpleForm = "\n" + + "\n" + + " \n" + + " My Survey\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n" + + " \n" + + ""; + File temp = File.createTempFile("simple_form", ".xml"); + temp.deleteOnExit(); + + BufferedWriter out = new BufferedWriter(new FileWriter(temp)); + out.write(simpleForm); + out.close(); + FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); + assertThat(formMetadata.getVersion(), is(nullValue())); + } } diff --git a/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java b/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java index 20c4db49118..7a84f862178 100644 --- a/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/utilities/FileUtilsTest.java @@ -2,13 +2,8 @@ import org.hamcrest.Matchers; import org.junit.Test; -import org.odk.collect.android.formmanagement.FormMetadata; -import org.odk.collect.android.formmanagement.FormMetadataParser; -import java.io.BufferedWriter; import java.io.File; -import java.io.FileWriter; -import java.io.IOException; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; @@ -30,272 +25,6 @@ public void mediaDirNameIsCorrect() { assertEquals(expected, FileUtils.constructMediaPath("sample-file.docx")); } - @Test public void readMetadata_withoutSubmission_returnsMetaDataFields() throws IOException { - String simpleForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - File temp = File.createTempFile("simple_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(simpleForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("My Survey")); - assertThat(formMetadata.getId(), is("mysurvey")); - assertThat(formMetadata.getVersion(), is(nullValue())); - assertThat(formMetadata.getBase64RsaPublicKey(), is(nullValue())); - } - - @Test public void readMetadata_withSubmission_returnsMetaDataFields() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - - File temp = File.createTempFile("submission_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("My Survey")); - assertThat(formMetadata.getId(), is("mysurvey")); - assertThat(formMetadata.getVersion(), is("2014083101")); - assertThat(formMetadata.getSubmissionUri(), is("foo")); - assertThat(formMetadata.getAutoSend(), is("bar")); - assertThat(formMetadata.getAutoDelete(), is("baz")); - assertThat(formMetadata.getBase64RsaPublicKey(), is("quux")); - assertThat(formMetadata.getGeometryXPath(), is(nullValue())); - } - - @Test public void readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints")); - assertThat(formMetadata.getId(), is("two-geopoints")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); - } - - @Test public void readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints in group\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_group_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints in group")); - assertThat(formMetadata.getId(), is("two-geopoints-group")); - assertThat(formMetadata.getGeometryXPath(), is("/data/my-group/location1")); - } - - @Test public void readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints repeat\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_repeat_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints repeat")); - assertThat(formMetadata.getId(), is("two-geopoints-repeat")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location2")); - } - - @Test public void readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Setgeopoint before\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_repeat_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Setgeopoint before")); - assertThat(formMetadata.getId(), is("set-geopoint-before")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); - } - - @Test public void whenFormVersionIsEmpty_shouldBeTreatedAsNull() throws IOException { - String simpleForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - File temp = File.createTempFile("simple_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(simpleForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(formMetadata.getVersion(), is(nullValue())); - } - @Test @SuppressWarnings("PMD.DoNotHardCodeSDCard") public void simplifyScopedStoragePathTest() { From b1b4d34ba757eaff4d1a5f6d28f3cf457169bc21 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 24 Sep 2024 23:23:11 +0200 Subject: [PATCH 06/19] Converted FormMetadataParserTest to kotlin and simplified it --- .../FormMetadataParserTest.java | 281 ------------------ .../formmanagement/FormMetadataParserTest.kt | 270 +++++++++++++++++ 2 files changed, 270 insertions(+), 281 deletions(-) delete mode 100644 collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java create mode 100644 collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java deleted file mode 100644 index 2e2b35db9bc..00000000000 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.java +++ /dev/null @@ -1,281 +0,0 @@ -package org.odk.collect.android.formmanagement; - -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.nullValue; -import static org.hamcrest.MatcherAssert.assertThat; - -import org.junit.Test; - -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; - -public class FormMetadataParserTest { - @Test - public void readMetadata_withoutSubmission_returnsMetaDataFields() throws IOException { - String simpleForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - File temp = File.createTempFile("simple_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(simpleForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("My Survey")); - assertThat(formMetadata.getId(), is("mysurvey")); - assertThat(formMetadata.getVersion(), is(nullValue())); - assertThat(formMetadata.getBase64RsaPublicKey(), is(nullValue())); - } - - @Test public void readMetadata_withSubmission_returnsMetaDataFields() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - - File temp = File.createTempFile("submission_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("My Survey")); - assertThat(formMetadata.getId(), is("mysurvey")); - assertThat(formMetadata.getVersion(), is("2014083101")); - assertThat(formMetadata.getSubmissionUri(), is("foo")); - assertThat(formMetadata.getAutoSend(), is("bar")); - assertThat(formMetadata.getAutoDelete(), is("baz")); - assertThat(formMetadata.getBase64RsaPublicKey(), is("quux")); - assertThat(formMetadata.getGeometryXPath(), is(nullValue())); - } - - @Test public void readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints")); - assertThat(formMetadata.getId(), is("two-geopoints")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); - } - - @Test public void readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints in group\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_group_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints in group")); - assertThat(formMetadata.getId(), is("two-geopoints-group")); - assertThat(formMetadata.getGeometryXPath(), is("/data/my-group/location1")); - } - - @Test public void readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Two geopoints repeat\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_repeat_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Two geopoints repeat")); - assertThat(formMetadata.getId(), is("two-geopoints-repeat")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location2")); - } - - @Test public void readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() throws IOException { - String submissionForm = "\n" + - "\n" + - " \n" + - " Setgeopoint before\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - ""; - - File temp = File.createTempFile("geopoints_repeat_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(submissionForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - - assertThat(formMetadata.getTitle(), is("Setgeopoint before")); - assertThat(formMetadata.getId(), is("set-geopoint-before")); - assertThat(formMetadata.getGeometryXPath(), is("/data/location1")); - } - - @Test public void whenFormVersionIsEmpty_shouldBeTreatedAsNull() throws IOException { - String simpleForm = "\n" + - "\n" + - " \n" + - " My Survey\n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - " \n" + - "\n" + - " \n" + - ""; - File temp = File.createTempFile("simple_form", ".xml"); - temp.deleteOnExit(); - - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(simpleForm); - out.close(); - - FormMetadata formMetadata = FormMetadataParser.readMetadata(temp); - assertThat(formMetadata.getVersion(), is(nullValue())); - } -} diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt new file mode 100644 index 00000000000..b1c56d32802 --- /dev/null +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt @@ -0,0 +1,270 @@ +package org.odk.collect.android.formmanagement + +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo +import org.junit.Test +import org.odk.collect.android.formmanagement.FormMetadataParser.readMetadata + +class FormMetadataParserTest { + @Test + fun readMetadata_withoutSubmission_returnsFormMetadata() { + val formMetadata = readMetadata( + """ + + + + My Survey + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("My Survey")) + assertThat(formMetadata.id, equalTo("mysurvey")) + assertThat(formMetadata.version, equalTo(null)) + assertThat(formMetadata.base64RsaPublicKey, equalTo(null)) + } + + @Test + fun readMetadata_withSubmission_returnsFormMetadata() { + val formMetadata = readMetadata( + """ + + + + My Survey + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("My Survey")) + assertThat(formMetadata.id, equalTo("mysurvey")) + assertThat(formMetadata.version, equalTo("2014083101")) + assertThat(formMetadata.submissionUri, equalTo("foo")) + assertThat(formMetadata.autoSend, equalTo("bar")) + assertThat(formMetadata.autoDelete, equalTo("baz")) + assertThat(formMetadata.base64RsaPublicKey, equalTo("quux")) + assertThat(formMetadata.geometryXPath, equalTo(null)) + } + + @Test + fun readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() { + val formMetadata = readMetadata( + """ + + + + Two geopoints + + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("Two geopoints")) + assertThat(formMetadata.id, equalTo("two-geopoints")) + assertThat(formMetadata.geometryXPath, equalTo("/data/location1")) + } + + @Test + fun readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() { + val formMetadata = readMetadata( + """ + + + + Two geopoints in group + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("Two geopoints in group")) + assertThat(formMetadata.id, equalTo("two-geopoints-group")) + assertThat(formMetadata.geometryXPath, equalTo("/data/my-group/location1")) + } + + @Test + fun readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() { + val formMetadata = readMetadata( + """ + + + + Two geopoints repeat + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("Two geopoints repeat")) + assertThat(formMetadata.id, equalTo("two-geopoints-repeat")) + assertThat(formMetadata.geometryXPath, equalTo("/data/location2")) + } + + @Test + fun readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() { + val formMetadata = readMetadata( + """ + + + + Setgeopoint before + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.title, equalTo("Setgeopoint before")) + assertThat(formMetadata.id, equalTo("set-geopoint-before")) + assertThat(formMetadata.geometryXPath, equalTo("/data/location1")) + } + + @Test + fun whenFormVersionIsEmpty_shouldBeTreatedAsNull() { + val formMetadata = readMetadata( + """ + + + + My Survey + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + + assertThat(formMetadata.version, equalTo(null)) + } +} From 7aa24271865c9cde4e8c9850ff2ecb661e80693b Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 25 Sep 2024 00:39:05 +0200 Subject: [PATCH 07/19] Removed redundant test --- .../download/ServerFormDownloaderTest.java | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java index 94874dd4bb6..91526d07135 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java @@ -338,35 +338,6 @@ public void whenFormHasMediaFiles_andIsFormToDownloadIsUpdate_downloadsFilesWith assertThat(new String(read(mediaFile2)), is("contents3")); } - /** - * Form parsing might need access to media files (external secondary instances) for example - * so we need to make sure we've got those files in the right place before we parse. - */ - @Test - public void whenFormHasMediaFiles_downloadsAndSavesFormAndMediaFiles_beforeParsingForm() throws Exception { - String xform = createXFormBody("id", "version"); - ServerFormDetails serverFormDetails = new ServerFormDetails( - "Form", - "http://downloadUrl", - "id", - "version", - Md5.getMd5Hash(new ByteArrayInputStream(xform.getBytes())), - true, - false, - new ManifestFile("", asList( - new MediaFile("file1", "hash-1", "http://file1"), - new MediaFile("file2", "hash-2", "http://file2") - ))); - - FormSource formSource = mock(FormSource.class); - when(formSource.fetchForm("http://downloadUrl")).thenReturn(new ByteArrayInputStream(xform.getBytes())); - when(formSource.fetchMediaFile("http://file1")).thenReturn(new ByteArrayInputStream("contents1".getBytes())); - when(formSource.fetchMediaFile("http://file2")).thenReturn(new ByteArrayInputStream("contents2".getBytes())); - - ServerFormDownloader downloader = new ServerFormDownloader(formSource, formsRepository, cacheDir, formsDir.getAbsolutePath(), FormMetadataParser.INSTANCE, clock, entitiesRepository); - downloader.downloadForm(serverFormDetails, null, null); - } - @Test public void whenFormHasMediaFiles_andFetchingMediaFileFails_throwsFetchErrorAndDoesNotSaveAnything() throws Exception { String xform = createXFormBody("id", "version"); From 7aa4a86498909d9189f74b5758877c8714de6f48 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 25 Sep 2024 00:51:18 +0200 Subject: [PATCH 08/19] Added clear error message for unrecognized entity version --- .../java/org/odk/collect/android/tasks/FormLoaderTask.java | 3 +++ strings/src/main/res/values/strings.xml | 3 +++ 2 files changed, 6 insertions(+) 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 index 7d43e304e74..0dd6cd9ab9e 100644 --- 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 @@ -56,6 +56,7 @@ import org.odk.collect.android.utilities.ZipUtils; import org.odk.collect.async.Scheduler; import org.odk.collect.async.SchedulerAsyncTaskMimic; +import org.odk.collect.entities.javarosa.spec.UnrecognizedEntityVersionException; import org.odk.collect.forms.Form; import org.odk.collect.forms.instances.Instance; import org.odk.collect.forms.savepoints.Savepoint; @@ -197,6 +198,8 @@ protected FECWrapper doInBackground(Void... ignored) { } catch (StackOverflowError e) { Timber.e(e); errorMsg = getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.too_complex_form); + } catch (UnrecognizedEntityVersionException e) { + errorMsg = getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.unrecognized_entity_version); } catch (Exception e) { Timber.w(e); errorMsg = "An unknown error has occurred. Please ask your project leadership to email support@getodk.org with information about this form."; diff --git a/strings/src/main/res/values/strings.xml b/strings/src/main/res/values/strings.xml index 02c7c698cb9..b396a1f8ff9 100644 --- a/strings/src/main/res/values/strings.xml +++ b/strings/src/main/res/values/strings.xml @@ -1185,6 +1185,9 @@ Offline + + The form contains an unrecognized entity version. Please update the form and download it again. + + + + + + Two geopoints + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + } } From 2cbdd45093cd34042f0048b4e71f1d37a2a6157f Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 25 Sep 2024 15:38:11 +0200 Subject: [PATCH 13/19] Moved form parsing to its own package --- .../collect/android/support/StubOpenRosaServer.java | 4 ++-- .../android/formmanagement/FormsDataService.kt | 1 + .../android/formmanagement/LocalFormUseCases.kt | 2 ++ .../download/ServerFormDownloader.java | 4 ++-- .../android/formmanagement/metadata/FormMetadata.kt | 12 ++++++++++++ .../{ => metadata}/FormMetadataParser.kt | 13 +------------ .../download/ServerFormDownloaderTest.java | 2 +- .../{ => metadata}/FormMetadataParserTest.kt | 4 ++-- 8 files changed, 23 insertions(+), 19 deletions(-) create mode 100644 collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadata.kt rename collect_app/src/main/java/org/odk/collect/android/formmanagement/{ => metadata}/FormMetadataParser.kt (94%) rename collect_app/src/test/java/org/odk/collect/android/formmanagement/{ => metadata}/FormMetadataParserTest.kt (98%) 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 index 5c627f82a68..dec375fb582 100644 --- 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 @@ -8,8 +8,8 @@ import androidx.annotation.Nullable; import org.jetbrains.annotations.NotNull; -import org.odk.collect.android.formmanagement.FormMetadata; -import org.odk.collect.android.formmanagement.FormMetadataParser; +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; 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 index 8e623dcf725..42decb629c7 100644 --- 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 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 index 6b25728be81..dcf2b55e98d 100644 --- 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 diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java b/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java index 89556eb66ae..330dbd8ce7c 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java @@ -3,8 +3,8 @@ import static org.odk.collect.android.utilities.FileUtils.interuptablyWriteFile; import org.jetbrains.annotations.NotNull; -import org.odk.collect.android.formmanagement.FormMetadata; -import org.odk.collect.android.formmanagement.FormMetadataParser; +import org.odk.collect.android.formmanagement.metadata.FormMetadata; +import org.odk.collect.android.formmanagement.metadata.FormMetadataParser; import org.odk.collect.android.formmanagement.ServerFormDetails; import org.odk.collect.android.formmanagement.ServerFormUseCases; import org.odk.collect.android.utilities.FileUtils; diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadata.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadata.kt new file mode 100644 index 00000000000..310707fbfba --- /dev/null +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadata.kt @@ -0,0 +1,12 @@ +package org.odk.collect.android.formmanagement.metadata + +data class FormMetadata( + val title: String?, + val id: String?, + val version: String?, + val submissionUri: String?, + val base64RsaPublicKey: String?, + val autoDelete: String?, + val autoSend: String?, + val geometryXPath: String? +) diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt similarity index 94% rename from collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt rename to collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt index 6d937bb0c96..5d43e936bf2 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/FormMetadataParser.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt @@ -1,4 +1,4 @@ -package org.odk.collect.android.formmanagement +package org.odk.collect.android.formmanagement.metadata import org.javarosa.core.model.actions.setgeopoint.SetGeopointActionHandler import org.javarosa.xform.parse.XFormParser @@ -129,14 +129,3 @@ object FormMetadataParser { return false } } - -data class FormMetadata( - val title: String?, - val id: String?, - val version: String?, - val submissionUri: String?, - val base64RsaPublicKey: String?, - val autoDelete: String?, - val autoSend: String?, - val geometryXPath: String? -) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java index 91526d07135..1c078e80181 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java @@ -19,7 +19,7 @@ import com.google.common.io.Files; import org.junit.Test; -import org.odk.collect.android.formmanagement.FormMetadataParser; +import org.odk.collect.android.formmanagement.metadata.FormMetadataParser; import org.odk.collect.android.formmanagement.ServerFormDetails; import org.odk.collect.entities.storage.EntitiesRepository; import org.odk.collect.entities.storage.InMemEntitiesRepository; diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt similarity index 98% rename from collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt rename to collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt index 084309b0a29..995c942a3b6 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/FormMetadataParserTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt @@ -1,9 +1,9 @@ -package org.odk.collect.android.formmanagement +package org.odk.collect.android.formmanagement.metadata import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test -import org.odk.collect.android.formmanagement.FormMetadataParser.readMetadata +import org.odk.collect.android.formmanagement.metadata.FormMetadataParser.readMetadata class FormMetadataParserTest { @Test From 454d0d515254ac6c0a35c2b5601846f20c7d82bc Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 26 Sep 2024 12:15:28 +0200 Subject: [PATCH 14/19] Improved the error message --- .../odk/collect/android/feature/formentry/EntityFormTest.kt | 6 +++--- .../java/org/odk/collect/android/support/pages/Page.kt | 4 ++-- .../java/org/odk/collect/android/tasks/FormLoaderTask.java | 2 +- .../entities/javarosa/parse/EntityFormParseProcessor.java | 2 +- .../javarosa/spec/UnrecognizedEntityVersionException.kt | 2 +- strings/src/main/res/values/strings.xml | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) 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 index b7396ec696d..d255366ea1f 100644 --- 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 @@ -203,7 +203,7 @@ class EntityFormTest { .clickOK(MainMenuPage()) .clickFillBlankForm() .clickOnForm("One Question Entity Registration") - .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version) + .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version, "2020.1.0") .clickOKOnDialog(MainMenuPage()) } @@ -214,7 +214,7 @@ class EntityFormTest { rule.withMatchExactlyProject(testDependencies.server.url) .clickFillBlankForm() .clickOnForm("One Question Entity Registration") - .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version) + .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version, "2020.1.0") .clickOKOnDialog(MainMenuPage()) } @@ -225,7 +225,7 @@ class EntityFormTest { .copyForm("one-question-entity-registration-v2020.1.xml") .clickFillBlankForm() .clickOnForm("One Question Entity Registration") - .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version) + .assertTextInDialog(org.odk.collect.strings.R.string.unrecognized_entity_version, "2020.1.0") .clickOKOnDialog(MainMenuPage()) } } 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 index 0f976ee7e7a..3ca7476d672 100644 --- 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> { 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/tasks/FormLoaderTask.java b/collect_app/src/main/java/org/odk/collect/android/tasks/FormLoaderTask.java index 0dd6cd9ab9e..6372c841feb 100644 --- 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 @@ -199,7 +199,7 @@ protected FECWrapper doInBackground(Void... ignored) { Timber.e(e); errorMsg = getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.too_complex_form); } catch (UnrecognizedEntityVersionException e) { - errorMsg = getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.unrecognized_entity_version); + errorMsg = getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.unrecognized_entity_version, e.getEntityVersion()); } catch (Exception e) { Timber.w(e); errorMsg = "An unknown error has occurred. Please ask your project leadership to email support@getodk.org with information about this form."; diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/parse/EntityFormParseProcessor.java b/entities/src/main/java/org/odk/collect/entities/javarosa/parse/EntityFormParseProcessor.java index 930e1a6af3a..f3b6e3e95bc 100644 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/parse/EntityFormParseProcessor.java +++ b/entities/src/main/java/org/odk/collect/entities/javarosa/parse/EntityFormParseProcessor.java @@ -37,7 +37,7 @@ public void processModelAttribute(String name, String value) throws XFormParser. version = value; if (Stream.of(SUPPORTED_VERSIONS).noneMatch(value::startsWith)) { - throw new UnrecognizedEntityVersionException(); + throw new UnrecognizedEntityVersionException(version); } } diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/spec/UnrecognizedEntityVersionException.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/spec/UnrecognizedEntityVersionException.kt index e064ad9328a..2343a897a6c 100644 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/spec/UnrecognizedEntityVersionException.kt +++ b/entities/src/main/java/org/odk/collect/entities/javarosa/spec/UnrecognizedEntityVersionException.kt @@ -2,4 +2,4 @@ package org.odk.collect.entities.javarosa.spec import org.javarosa.xform.parse.XFormParser -class UnrecognizedEntityVersionException : XFormParser.ParseException() +class UnrecognizedEntityVersionException(val entityVersion: String) : XFormParser.ParseException() diff --git a/strings/src/main/res/values/strings.xml b/strings/src/main/res/values/strings.xml index b396a1f8ff9..f46212764ec 100644 --- a/strings/src/main/res/values/strings.xml +++ b/strings/src/main/res/values/strings.xml @@ -1186,7 +1186,7 @@ Offline - The form contains an unrecognized entity version. Please update the form and download it again. + This form is not supported by this version of ODK Collect. Please upgrade Collect. If you keep having this problem, report it to the person who asked you to collect data.\n\nForm Entities spec version: %s Unrecognized URI: %s + + Form can\'t be used Could not start recording. From 3d65c6f26868ebe3e527e8e76b8d67a25c107c08 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Mon, 30 Sep 2024 15:58:36 +0200 Subject: [PATCH 16/19] Improved reading geometry xpaths --- .../metadata/FormMetadataParser.kt | 102 +++++++++--------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt index 5d43e936bf2..9f3bf568e65 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt @@ -1,6 +1,5 @@ package org.odk.collect.android.formmanagement.metadata -import org.javarosa.core.model.actions.setgeopoint.SetGeopointActionHandler import org.javarosa.xform.parse.XFormParser import org.kxml2.kdom.Element import java.io.File @@ -42,7 +41,7 @@ object FormMetadataParser { val base64RsaPublicKey = submission?.getAttributeValue(null, "base64RsaPublicKey") val autoDelete = submission?.getAttributeValue(null, "auto-delete") val autoSend = submission?.getAttributeValue(null, "auto-send") - val geometryXPath = getOverallFirstGeoPointXPath(model, mainInstanceRoot, body) + val geometryXPath = getFirstGeopointXPath(model, mainInstanceRoot, body) return FormMetadata( title, @@ -57,75 +56,70 @@ object FormMetadataParser { } /** - * Returns an XPath path representing the first geopoint of this form definition or null if the - * definition does not contain any field of type geopoint. + * Finds the first geopoint reference in the primary instance by: + * 1. Retrieving all geopoint binds from the model. + * 2. Iterating through the elements of the primary instance root. + * 3. Returning the first reference found in the primary instance that matches one of + * the geopoint binds and is not inside a repeat. + * + * This solution is not perfect because it assumes that the references in the model + * appear in the same order as in the body, which is not guaranteed by XForms. + * However, in practice, this is typically the case. * - * The first geopoint is either of: - * (1) the first geopoint in the body that is not in a repeat - * (2) if the form has a setgeopoint action, the first geopoint in the instance that occurs - * before (1) or (1) if there is no geopoint defined before it in the instance. */ - private fun getOverallFirstGeoPointXPath(model: Element, mainInstanceRoot: Element, body: Element): String? { - return if (containsSetgeopointAction(model)) { - getInstanceGeoPointBeforeXPath(model, mainInstanceRoot) + private fun getFirstGeopointXPath(model: Element, mainInstanceRoot: Element, body: Element): String? { + val geopointXPaths = getGeopointXPaths(model) + return if (geopointXPaths.isEmpty()) { + null } else { - getFirstToplevelBodyGeoPointXPath(model, body) + val repeatXPaths = getRepeatXPaths(body) + getFirstPrimaryInstanceGeopointXPath(geopointXPaths, repeatXPaths, mainInstanceRoot, null) } } - /** - * Returns the reference of the first geopoint in the body that is not in a repeat. - */ - private fun getFirstToplevelBodyGeoPointXPath(model: Element, body: Element): String? { - for (elementId in 0 until body.childCount) { - val child = body.getElement(elementId) ?: continue - val ref = child.getAttributeValue(null, "ref") - if (child.name == "input" && isGeopoint(model, ref)) { - return ref - } else if (child.name == "group") { - return getFirstToplevelBodyGeoPointXPath(model, child) + private fun getGeopointXPaths(model: Element): List { + val geopointXPaths = mutableListOf() + for (elementId in 0 until model.childCount) { + val child = model.getElement(elementId) ?: continue + if (child.name == "bind" && child.getAttributeValue(null, "type") == "geopoint") { + geopointXPaths.add(child.getAttributeValue(null, "nodeset")) } } - return null + return geopointXPaths } - /** - * Returns the XPath path for the first geopoint in the primary instance that is before the given - * reference and not in a repeat. - */ - private fun getInstanceGeoPointBeforeXPath(model: Element, mainInstanceRoot: Element): String? { - for (elementId in 0 until mainInstanceRoot.childCount) { - val child = mainInstanceRoot.getElement(elementId) ?: continue - val ref = "/${mainInstanceRoot.name}/${child.name}" - if (isGeopoint(model, ref)) { - return ref + private fun getRepeatXPaths(body: Element): List { + val repeatXPaths = mutableListOf() + for (elementId in 0 until body.childCount) { + val child = body.getElement(elementId) ?: continue + if (child.name == "repeat") { + repeatXPaths.add(child.getAttributeValue(null, "nodeset")) } else if (child.childCount > 0) { - return getInstanceGeoPointBeforeXPath(model, child) + repeatXPaths.addAll(getRepeatXPaths(child)) } } - return null + return repeatXPaths } - private fun containsSetgeopointAction(model: Element): Boolean { - for (elementId in 0 until model.childCount) { - val child = model.getElement(elementId) ?: continue - if (child.name == SetGeopointActionHandler.ELEMENT_NAME) { - return true + private fun getFirstPrimaryInstanceGeopointXPath( + geopointXPaths: List, + repeatXPaths: List, + mainInstanceRoot: Element, + parentXPath: String? + ): String? { + for (elementId in 0 until mainInstanceRoot.childCount) { + val child = mainInstanceRoot.getElement(elementId) ?: continue + val xpath = if (parentXPath == null) { + "/${mainInstanceRoot.name}/${child.name}" + } else { + "$parentXPath/${child.name}" } - } - return false - } - - private fun isGeopoint(model: Element, ref: String): Boolean { - for (elementId in 0 until model.childCount) { - val child = model.getElement(elementId) ?: continue - if (child.name == "bind" && - child.getAttributeValue(null, "nodeset") == ref && - child.getAttributeValue(null, "type") == "geopoint" - ) { - return true + if (geopointXPaths.contains(xpath)) { + return xpath + } else if (child.childCount > 0 && !repeatXPaths.contains(xpath)) { + return getFirstPrimaryInstanceGeopointXPath(geopointXPaths, repeatXPaths, child, xpath) } } - return false + return null } } From 126896e47f652baef637c68467a99d4ebd5bf373 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Mon, 30 Sep 2024 15:58:46 +0200 Subject: [PATCH 17/19] Reworked tests --- .../metadata/FormMetadataParserTest.kt | 263 ++++++++++-------- 1 file changed, 143 insertions(+), 120 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt index 995c942a3b6..a70373ccd88 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt @@ -7,7 +7,56 @@ import org.odk.collect.android.formmanagement.metadata.FormMetadataParser.readMe class FormMetadataParserTest { @Test - fun readMetadata_withoutSubmission_returnsFormMetadata() { + fun readMetadata_canParseFormsWithComments() { + readMetadata( + """ + + + + + + + Two geopoints + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() + ) + } + + @Test + fun readMetadata_returnsCorrectValuesForMandatoryElements() { val formMetadata = readMetadata( """ @@ -31,12 +80,10 @@ class FormMetadataParserTest { assertThat(formMetadata.title, equalTo("My Survey")) assertThat(formMetadata.id, equalTo("mysurvey")) - assertThat(formMetadata.version, equalTo(null)) - assertThat(formMetadata.base64RsaPublicKey, equalTo(null)) } @Test - fun readMetadata_withSubmission_returnsFormMetadata() { + fun readMetadata_withoutOptionalMetadata_returnsNullValuesForThoseElements() { val formMetadata = readMetadata( """ @@ -47,23 +94,9 @@ class FormMetadataParserTest { My Survey - - - - + - - @@ -72,35 +105,36 @@ class FormMetadataParserTest { """.trimIndent().byteInputStream() ) - assertThat(formMetadata.title, equalTo("My Survey")) - assertThat(formMetadata.id, equalTo("mysurvey")) - assertThat(formMetadata.version, equalTo("2014083101")) - assertThat(formMetadata.submissionUri, equalTo("foo")) - assertThat(formMetadata.autoSend, equalTo("bar")) - assertThat(formMetadata.autoDelete, equalTo("baz")) - assertThat(formMetadata.base64RsaPublicKey, equalTo("quux")) + assertThat(formMetadata.version, equalTo(null)) + assertThat(formMetadata.submissionUri, equalTo(null)) + assertThat(formMetadata.autoSend, equalTo(null)) + assertThat(formMetadata.autoDelete, equalTo(null)) + assertThat(formMetadata.base64RsaPublicKey, equalTo(null)) assertThat(formMetadata.geometryXPath, equalTo(null)) } @Test - fun readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointBasedOnBodyOrder() { + fun readMetadata_witOptionalMetadata_returnsCorrectValuesForThoseElements() { val formMetadata = readMetadata( """ - + - Two geopoints + My Survey - - - + - - + @@ -108,91 +142,74 @@ class FormMetadataParserTest { - - - - - - """.trimIndent().byteInputStream() ) - assertThat(formMetadata.title, equalTo("Two geopoints")) - assertThat(formMetadata.id, equalTo("two-geopoints")) + assertThat(formMetadata.version, equalTo("2014083101")) + assertThat(formMetadata.submissionUri, equalTo("foo")) + assertThat(formMetadata.autoSend, equalTo("bar")) + assertThat(formMetadata.autoDelete, equalTo("baz")) + assertThat(formMetadata.base64RsaPublicKey, equalTo("quux")) assertThat(formMetadata.geometryXPath, equalTo("/data/location1")) } @Test - fun readMetadata_withGeopointInGroup_returnsFirstGeopointBasedOnBodyOrder() { + fun readMetadata_withEmptyFormVersion_returnsNullFormVersion() { val formMetadata = readMetadata( """ - + - Two geopoints in group + My Survey - - - - - + - - - - - - - - - - """.trimIndent().byteInputStream() ) - assertThat(formMetadata.title, equalTo("Two geopoints in group")) - assertThat(formMetadata.id, equalTo("two-geopoints-group")) - assertThat(formMetadata.geometryXPath, equalTo("/data/my-group/location1")) + assertThat(formMetadata.version, equalTo(null)) } @Test - fun readMetadata_withGeopointInRepeat_returnsFirstGeopointBasedOnBodyOrder() { + fun readMetadata_withGeopointsAtTopLevel_returnsFirstGeopointXPath() { val formMetadata = readMetadata( """ - Two geopoints repeat + Two geopoints - - - - + + + + + - - - - - - + + + + + + @@ -201,34 +218,37 @@ class FormMetadataParserTest { """.trimIndent().byteInputStream() ) - assertThat(formMetadata.title, equalTo("Two geopoints repeat")) - assertThat(formMetadata.id, equalTo("two-geopoints-repeat")) - assertThat(formMetadata.geometryXPath, equalTo("/data/location2")) + assertThat(formMetadata.geometryXPath, equalTo("/data/location1")) } @Test - fun readMetadata_withSetGeopointBeforeBodyGeopoint_returnsFirstGeopointInInstance() { + fun readMetadata_withGeopointsAtTopLevel_returnsGeopointXPathThatBelongsToSetgeopointActionIfItIsTheFirstOne() { val formMetadata = readMetadata( """ + xmlns="http://www.w3.org/2002/xforms" + xmlns:odk="http://www.opendatakit.org/xforms"> - Setgeopoint before + Two geopoints - + + - + + + + + @@ -237,77 +257,78 @@ class FormMetadataParserTest { """.trimIndent().byteInputStream() ) - assertThat(formMetadata.title, equalTo("Setgeopoint before")) - assertThat(formMetadata.id, equalTo("set-geopoint-before")) assertThat(formMetadata.geometryXPath, equalTo("/data/location1")) } @Test - fun whenFormVersionIsEmpty_shouldBeTreatedAsNull() { + fun readMetadata_withGeopointInGroup_returnsFirstGeopointXPath() { val formMetadata = readMetadata( """ - + - My Survey + Two geopoints in group - + + + + + + + + + + + + + + + + + """.trimIndent().byteInputStream() ) - assertThat(formMetadata.version, equalTo(null)) + assertThat(formMetadata.geometryXPath, equalTo("/data/my-group1/my-group2/location1")) } @Test - fun formWithComments_isParsedSuccessfully() { - readMetadata( + fun readMetadata_withGeopointInRepeat_returnsFirstGeopointXPathThatIsNotInsideRepeat() { + val formMetadata = readMetadata( """ - - - - Two geopoints - + Two geopoints repeat - - - - + + + + - - - - - - + - - - - - - - - + + + + + @@ -315,5 +336,7 @@ class FormMetadataParserTest { """.trimIndent().byteInputStream() ) + + assertThat(formMetadata.geometryXPath, equalTo("/data/location2")) } } From f49862eba6cf2f975126a61b8aea2314e7d9e3ba Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Mon, 30 Sep 2024 20:03:15 +0200 Subject: [PATCH 18/19] Naming improvements --- .../metadata/FormMetadataParser.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt index 9f3bf568e65..758bd123a99 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParser.kt @@ -79,8 +79,8 @@ object FormMetadataParser { private fun getGeopointXPaths(model: Element): List { val geopointXPaths = mutableListOf() - for (elementId in 0 until model.childCount) { - val child = model.getElement(elementId) ?: continue + for (position in 0 until model.childCount) { + val child = model.getElement(position) ?: continue if (child.name == "bind" && child.getAttributeValue(null, "type") == "geopoint") { geopointXPaths.add(child.getAttributeValue(null, "nodeset")) } @@ -90,8 +90,8 @@ object FormMetadataParser { private fun getRepeatXPaths(body: Element): List { val repeatXPaths = mutableListOf() - for (elementId in 0 until body.childCount) { - val child = body.getElement(elementId) ?: continue + for (position in 0 until body.childCount) { + val child = body.getElement(position) ?: continue if (child.name == "repeat") { repeatXPaths.add(child.getAttributeValue(null, "nodeset")) } else if (child.childCount > 0) { @@ -104,13 +104,13 @@ object FormMetadataParser { private fun getFirstPrimaryInstanceGeopointXPath( geopointXPaths: List, repeatXPaths: List, - mainInstanceRoot: Element, + parentRoot: Element, parentXPath: String? ): String? { - for (elementId in 0 until mainInstanceRoot.childCount) { - val child = mainInstanceRoot.getElement(elementId) ?: continue + for (position in 0 until parentRoot.childCount) { + val child = parentRoot.getElement(position) ?: continue val xpath = if (parentXPath == null) { - "/${mainInstanceRoot.name}/${child.name}" + "/${parentRoot.name}/${child.name}" } else { "$parentXPath/${child.name}" } From df9d70e6897d627a9e85d94afc8f42b09d60daae Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Mon, 30 Sep 2024 20:13:09 +0200 Subject: [PATCH 19/19] Simplified readMetadata_canParseFormsWithComments test --- .../metadata/FormMetadataParserTest.kt | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt index a70373ccd88..4ad0b233746 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/metadata/FormMetadataParserTest.kt @@ -17,38 +17,22 @@ class FormMetadataParserTest { - Two geopoints + Form with comments - + - - - - - - - - - - - - - - - - """.trimIndent().byteInputStream()