Issue #799 Fix getting form name for form keys

src/org/opendatakit/aggregate/parser/BaseFormParserForJavaRosa.java

@@ -230,7 +230,7 @@ private static synchronized final XFormParserWithBindEnhancements parseFormDefin
   /**
    * The XForm definition in XML
    */
-  protected final String xml;
+  public final String xml;
 
   // original bindings from parse-time for later comparison
   private transient final List<Element> bindElements = new ArrayList<>();

src/org/opendatakit/briefcase/model/form/FormKey.java

@@ -7,8 +7,11 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import java.util.Objects;
 import java.util.Optional;
+import org.opendatakit.briefcase.export.XmlElement;
+import org.opendatakit.briefcase.model.BriefcaseFormDefinition;
 import org.opendatakit.briefcase.model.FormStatus;
 import org.opendatakit.briefcase.reused.BriefcaseException;
+import org.opendatakit.briefcase.reused.Pair;
 
 public class FormKey implements AsJson {
   private final String name;
@@ -22,9 +25,10 @@ private FormKey(String name, String id, Optional<String> version) {
   }
 
   public static FormKey from(FormStatus formStatus) {
+    Pair<String, String> formNameAndId = getFormNameAndId(formStatus);
     return new FormKey(
-        formStatus.getFormName(),
-        formStatus.getFormId(),
+        formNameAndId.getLeft(),
+        formNameAndId.getRight(),
         formStatus.getVersion()
     );
   }
@@ -49,6 +53,33 @@ public static FormKey of(String name, String id, Optional<String> version) {
     return new FormKey(name, id, version);
   }
 
+  private static Pair<String, String> getFormNameAndId(FormStatus formStatus) {
+    if (!(formStatus.getFormDefinition() instanceof BriefcaseFormDefinition))
+      return Pair.of(
+          formStatus.getFormName(),
+          formStatus.getFormId()
+      );
+
+    // We can't trust the form's title because JavaRosaParserWrapper strips illegal chars
+    // from it for some reason and then we can't use it to match any stored form metadata
+    BriefcaseFormDefinition formDef = (BriefcaseFormDefinition) formStatus.getFormDefinition();
+    XmlElement root = XmlElement.from(formDef.formDefn.xml);
+    String formName = root.findElement("head")
+        .flatMap(e -> e.findElement("title"))
+        .flatMap(XmlElement::maybeValue)
+        // Revert to the name in the formdef if we get to this point
+        .orElseGet(formStatus::getFormName);
+
+    XmlElement instance = root.findElements("head", "model", "instance")
+        .stream()
+        .filter(e -> !e.hasAttribute("id"))
+        .findFirst()
+        .orElseThrow(BriefcaseException::new);
+
+    String formId = instance.childrenOf().get(0).getAttributeValue("id").orElseThrow(BriefcaseException::new);
+    return Pair.of(formName, formId);
+  }
+
   public String getId() {
     return id;
   }

test/java/org/opendatakit/briefcase/model/form/FormKeyTest.java

@@ -0,0 +1,30 @@
+package org.opendatakit.briefcase.model.form;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.opendatakit.briefcase.ui.pull.FormInstallerTest.getPath;
+import static org.opendatakit.briefcase.util.StringUtils.stripIllegalChars;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.Test;
+import org.opendatakit.briefcase.model.BriefcaseFormDefinition;
+import org.opendatakit.briefcase.model.FormStatus;
+import org.opendatakit.briefcase.util.BadFormDefinition;
+
+public class FormKeyTest {
+  @Test
+  public void regression_wrong_form_name_when_creating_keys_from_briefcase_form_defs() throws BadFormDefinition, URISyntaxException, IOException {
+    String sanitizedName = stripIllegalChars("this-title-has-dashes/and/slashes");
+    Path briefcaseFolder = Files.createTempDirectory("briefcase");
+    Path formFile = briefcaseFolder.resolve("forms").resolve(sanitizedName).resolve(sanitizedName + ".xml");
+    Path sourceFormFile = getPath("form-with-special-chars.xml");
+    Files.createDirectories(formFile.getParent());
+    Files.copy(sourceFormFile, formFile);
+    FormKey key1 = FormKey.of("this-title-has-dashes/and/slashes", "this-id-has-dashes/and/slashes");
+    FormKey key2 = FormKey.from(new FormStatus(BriefcaseFormDefinition.resolveAgainstBriefcaseDefn(formFile.toFile(), false, briefcaseFolder.toFile())));
+    assertThat(key1, is(key2));
+  }
+}

test/java/org/opendatakit/briefcase/ui/pull/FormInstallerTest.java

@@ -209,9 +209,9 @@ private static FormStatus buildForm(Path formPath) throws BadFormDefinition {
     return new FormStatus(new OdkCollectFormDefinition(formPath.toFile()));
   }
 
-  private static Path getPath(String fileName) throws URISyntaxException {
+  public static Path getPath(String fileName) throws URISyntaxException {
     return Paths.get(FormInstallerTest.class.getClassLoader().getResource(fileName).toURI());
   }
 
 
-}
+}

test/resources/form-with-special-chars.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0"?>
+<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
+  <h:head>
+    <h:title>this-title-has-dashes/and/slashes</h:title>
+    <model>
+      <instance>
+        <data id="this-id-has-dashes/and/slashes">
+          <some-field/>
+          <meta>
+            <instanceID/>
+          </meta>
+        </data>
+      </instance>
+      <bind nodeset="/data/some-field" type="string"/>
+      <bind calculate="concat('uuid:', uuid())" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
+    </model>
+  </h:head>
+  <h:body>
+    <input ref="/data/some-field">
+      <label>Some field</label>
+    </input>
+  </h:body>
+</h:html>