From 5e9411d8ffcb5a383c4041e51391ad5251ce76d6 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 13 May 2024 14:15:24 +0100 Subject: [PATCH 1/5] Add failing test for null ID entities --- .../core/model/instance/TreeElement.java | 9 ++++- .../org/javarosa/entities/EntitiesTest.java | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/javarosa/core/model/instance/TreeElement.java b/src/main/java/org/javarosa/core/model/instance/TreeElement.java index 48bdad219..43cfa6e5a 100644 --- a/src/main/java/org/javarosa/core/model/instance/TreeElement.java +++ b/src/main/java/org/javarosa/core/model/instance/TreeElement.java @@ -580,11 +580,18 @@ private String getAttributeValue(TreeElement attribute) { } } + @Nullable public String getAttributeValue() { if ( !isAttribute() ) { throw new IllegalStateException("this is not an attribute"); } - return getValue().uncast().getString(); + + IAnswerData value = getValue(); + if (value != null) { + return value.uncast().getString(); + } else { + return null; + } } @Override diff --git a/src/test/java/org/javarosa/entities/EntitiesTest.java b/src/test/java/org/javarosa/entities/EntitiesTest.java index 193a4cddc..41537a0bc 100644 --- a/src/test/java/org/javarosa/entities/EntitiesTest.java +++ b/src/test/java/org/javarosa/entities/EntitiesTest.java @@ -213,6 +213,39 @@ public void fillingFormWithUpdate_andNoLabel_makesEntityAvailableWithNullLabel() assertThat(entities.get(0).properties, equalTo(asList(new Pair<>("name", "Tom Wambsgans")))); } + @Test + public void fillingFormWithUpdate_withNullId_doesNotMakeEntityAvailable() throws IOException, XFormParser.ParseException { + Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( + asList( + new Pair<>("entities", "http://www.opendatakit.org/xforms/entities") + ), + head( + title("Update entity form"), + model(asList(new Pair<>("entities:entities-version", "2023.1.0")), + mainInstance( + t("data id=\"update-entity-form\"", + t("id"), + t("meta", + t("entity dataset=\"people\" update=\"1\" id=\"\" baseVersion=\"\"") + ) + ) + ), + bind("/data/id").type("string"), + bind("/data/meta/entity/@id").type("string").calculate("/data/id").readonly() + ) + ), + body( + input("/data/id") + ) + )); + + scenario.getFormEntryController().addPostProcessor(new EntityFormFinalizationProcessor()); + scenario.finalizeInstance(); + + List entities = scenario.getFormEntryController().getModel().getExtras().get(Entities.class).getEntities(); + assertThat(entities.size(), equalTo(0)); + } + @Test public void fillingFormWithCreateAndUpdate_makesEntityAvailableAsSecondVersion() throws IOException, XFormParser.ParseException { Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( From 29082cdc9b4ab23425a91a3f4e9481edfad1227b Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 13 May 2024 14:22:00 +0100 Subject: [PATCH 2/5] Don't return entities without an ID when updating --- .../javarosa/core/model/instance/TreeElement.java | 15 ++++++++------- .../entities/EntityFormFinalizationProcessor.java | 14 +++++++++++--- .../entities/internal/EntityFormParser.java | 1 + .../java/org/javarosa/entities/EntitiesTest.java | 8 ++++---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/javarosa/core/model/instance/TreeElement.java b/src/main/java/org/javarosa/core/model/instance/TreeElement.java index 43cfa6e5a..f73226662 100644 --- a/src/main/java/org/javarosa/core/model/instance/TreeElement.java +++ b/src/main/java/org/javarosa/core/model/instance/TreeElement.java @@ -15,13 +15,6 @@ */ package org.javarosa.core.model.instance; -import java.io.DataInputStream; -import java.io.DataOutputStream; -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; - import org.javarosa.core.model.Constants; import org.javarosa.core.model.FormDef; import org.javarosa.core.model.FormElementStateListener; @@ -52,6 +45,13 @@ import org.javarosa.xpath.expr.XPathStringLiteral; import org.jetbrains.annotations.Nullable; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + /** *

An element of a FormInstance.

* @@ -600,6 +600,7 @@ public TreeElement getAttribute(String namespace, String name) { } @Override + @Nullable public String getAttributeValue(String namespace, String name) { TreeElement element = getAttribute(namespace,name); return element == null ? null: getAttributeValue(element); diff --git a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java index 823bffe48..d1eaf11ef 100644 --- a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java +++ b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java @@ -41,7 +41,11 @@ public void processForm(FormEntryModel formEntryModel) { int baseVersion = EntityFormParser.parseBaseVersion(entityElement); int newVersion = baseVersion + 1; Entity entity = createEntity(entityElement, newVersion, dataset, saveTos, mainInstance, action); - formEntryModel.getExtras().put(new Entities(asList(entity))); + if (entity != null) { + formEntryModel.getExtras().put(new Entities(asList(entity))); + } else { + formEntryModel.getExtras().put(new Entities(emptyList())); + } } else { formEntryModel.getExtras().put(new Entities(emptyList())); } @@ -61,7 +65,11 @@ private Entity createEntity(TreeElement entityElement, int version, String datas }).collect(Collectors.toList()); String id = EntityFormParser.parseId(entityElement); - String label = EntityFormParser.parseLabel(entityElement); - return new Entity(action, dataset, id, label, version, fields); + if (id == null) { + return null; + } else { + String label = EntityFormParser.parseLabel(entityElement); + return new Entity(action, dataset, id, label, version, fields); + } } } diff --git a/src/main/java/org/javarosa/entities/internal/EntityFormParser.java b/src/main/java/org/javarosa/entities/internal/EntityFormParser.java index ed6395249..cb7fe29b5 100644 --- a/src/main/java/org/javarosa/entities/internal/EntityFormParser.java +++ b/src/main/java/org/javarosa/entities/internal/EntityFormParser.java @@ -28,6 +28,7 @@ public static String parseLabel(TreeElement entity) { } } + @Nullable public static String parseId(TreeElement entity) { return entity.getAttributeValue("", "id"); } diff --git a/src/test/java/org/javarosa/entities/EntitiesTest.java b/src/test/java/org/javarosa/entities/EntitiesTest.java index 41537a0bc..c73255d07 100644 --- a/src/test/java/org/javarosa/entities/EntitiesTest.java +++ b/src/test/java/org/javarosa/entities/EntitiesTest.java @@ -398,7 +398,7 @@ public void entityFormCanBeSerialized() throws IOException, DeserializationExcep t("data id=\"create-entity-form\"", t("name"), t("meta", - t("entities:entity dataset=\"people\" create=\"1\"") + t("entities:entity dataset=\"people\" create=\"1\" id=\"1\"") ) ) ), @@ -438,7 +438,7 @@ public void entitiesNamespaceWorksRegardlessOfName() throws IOException, Deseria t("data id=\"create-entity-form\"", t("name"), t("meta", - t("entity dataset=\"people\" create=\"1\"") + t("entity dataset=\"people\" create=\"1\" id=\"1\"") ) ) ), @@ -474,7 +474,7 @@ public void fillingFormWithSelectSaveTo_andWithCreate_savesValuesCorrectlyToEnti t("data id=\"create-entity-form\"", t("team"), t("meta", - t("entity dataset=\"people\" create=\"1\"") + t("entity dataset=\"people\" create=\"1\" id=\"1\"") ) ) ), @@ -510,7 +510,7 @@ public void whenSaveToQuestionIsNotAnswered_entityPropertyIsEmptyString() throws t("data id=\"create-entity-form\"", t("name"), t("meta", - t("entity dataset=\"people\" create=\"1\"") + t("entity dataset=\"people\" create=\"1\" id=\"1\"") ) ) ), From 1772325ce084a0a50cc56a7a2017473c019107a3 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 13 May 2024 14:27:11 +0100 Subject: [PATCH 3/5] Fix null ID on create case --- .../EntityFormFinalizationProcessor.java | 8 +++- .../org/javarosa/entities/EntitiesTest.java | 38 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java index d1eaf11ef..b49fd389a 100644 --- a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java +++ b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java @@ -12,6 +12,7 @@ import org.javarosa.form.api.FormEntryFinalizationProcessor; import org.javarosa.form.api.FormEntryModel; import org.javarosa.model.xform.XPathReference; +import org.jetbrains.annotations.Nullable; import java.util.List; import java.util.stream.Collectors; @@ -36,7 +37,11 @@ public void processForm(FormEntryModel formEntryModel) { if (action == EntityAction.CREATE) { Entity entity = createEntity(entityElement, 1, dataset, saveTos, mainInstance, action); - formEntryModel.getExtras().put(new Entities(asList(entity))); + if (entity != null) { + formEntryModel.getExtras().put(new Entities(asList(entity))); + } else { + formEntryModel.getExtras().put(new Entities(emptyList())); + } } else if (action == EntityAction.UPDATE){ int baseVersion = EntityFormParser.parseBaseVersion(entityElement); int newVersion = baseVersion + 1; @@ -52,6 +57,7 @@ public void processForm(FormEntryModel formEntryModel) { } } + @Nullable private Entity createEntity(TreeElement entityElement, int version, String dataset, List> saveTos, FormInstance mainInstance, EntityAction action) { List> fields = saveTos.stream().map(saveTo -> { IDataReference reference = saveTo.getFirst(); diff --git a/src/test/java/org/javarosa/entities/EntitiesTest.java b/src/test/java/org/javarosa/entities/EntitiesTest.java index c73255d07..7d9d00841 100644 --- a/src/test/java/org/javarosa/entities/EntitiesTest.java +++ b/src/test/java/org/javarosa/entities/EntitiesTest.java @@ -127,6 +127,42 @@ public void fillingFormWithCreate_makesEntityAvailable() throws IOException, XFo assertThat(entities.get(0).action, equalTo(EntityAction.CREATE)); } + @Test + public void fillingFormWithCreate_withoutAnId_doesNotMakeEntityAvailable() throws IOException, XFormParser.ParseException { + Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( + asList( + new Pair<>("entities", "http://www.opendatakit.org/xforms/entities") + ), + head( + title("Create entity form"), + model(asList(new Pair<>("entities:entities-version", "2022.1.1")), + mainInstance( + t("data id=\"create-entity-form\"", + t("id"), + t("meta", + t("entity dataset=\"people\" create=\"1\" id=\"\"", + t("label") + ) + ) + ) + ), + bind("/data/id").type("string"), + bind("/data/meta/entity/@id").type("string").calculate("/data/id"), + bind("/data/meta/entity/label").type("string").calculate("/data/name") + ) + ), + body( + input("/data/id") + ) + )); + + scenario.getFormEntryController().addPostProcessor(new EntityFormFinalizationProcessor()); + scenario.finalizeInstance(); + + List entities = scenario.getFormEntryController().getModel().getExtras().get(Entities.class).getEntities(); + assertThat(entities.size(), equalTo(0)); + } + @Test public void fillingFormWithUpdate_makesEntityAvailable() throws IOException, XFormParser.ParseException { Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( @@ -348,7 +384,7 @@ public void fillingFormWithDynamicCreateExpression_conditionallyCreatesEntities( t("name"), t("join"), t("meta", - t("entity dataset=\"members\" create=\"\"") + t("entity dataset=\"members\" create=\"\" id=\"1\"") ) ) ), From 71655486ccb59ca37c755d53fc4751901405d200 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 13 May 2024 14:52:22 +0100 Subject: [PATCH 4/5] Allow clients to handle null entity IDs --- .../java/org/javarosa/entities/Entity.java | 5 ++++- .../EntityFormFinalizationProcessor.java | 22 ++++--------------- .../entities/internal/EntityFormParser.java | 9 +++++++- .../org/javarosa/entities/EntitiesTest.java | 18 ++++++++++----- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/javarosa/entities/Entity.java b/src/main/java/org/javarosa/entities/Entity.java index 9e67067e1..38dcea514 100644 --- a/src/main/java/org/javarosa/entities/Entity.java +++ b/src/main/java/org/javarosa/entities/Entity.java @@ -9,14 +9,17 @@ public class Entity { public final String dataset; public final List> properties; + + @Nullable public final String id; @Nullable public final String label; + public final Integer version; public final EntityAction action; - public Entity(EntityAction action, String dataset, String id, @Nullable String label, Integer version, List> properties) { + public Entity(EntityAction action, String dataset, @Nullable String id, @Nullable String label, Integer version, List> properties) { this.dataset = dataset; this.id = id; this.label = label; diff --git a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java index b49fd389a..823bffe48 100644 --- a/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java +++ b/src/main/java/org/javarosa/entities/EntityFormFinalizationProcessor.java @@ -12,7 +12,6 @@ import org.javarosa.form.api.FormEntryFinalizationProcessor; import org.javarosa.form.api.FormEntryModel; import org.javarosa.model.xform.XPathReference; -import org.jetbrains.annotations.Nullable; import java.util.List; import java.util.stream.Collectors; @@ -37,27 +36,18 @@ public void processForm(FormEntryModel formEntryModel) { if (action == EntityAction.CREATE) { Entity entity = createEntity(entityElement, 1, dataset, saveTos, mainInstance, action); - if (entity != null) { - formEntryModel.getExtras().put(new Entities(asList(entity))); - } else { - formEntryModel.getExtras().put(new Entities(emptyList())); - } + formEntryModel.getExtras().put(new Entities(asList(entity))); } else if (action == EntityAction.UPDATE){ int baseVersion = EntityFormParser.parseBaseVersion(entityElement); int newVersion = baseVersion + 1; Entity entity = createEntity(entityElement, newVersion, dataset, saveTos, mainInstance, action); - if (entity != null) { - formEntryModel.getExtras().put(new Entities(asList(entity))); - } else { - formEntryModel.getExtras().put(new Entities(emptyList())); - } + formEntryModel.getExtras().put(new Entities(asList(entity))); } else { formEntryModel.getExtras().put(new Entities(emptyList())); } } } - @Nullable private Entity createEntity(TreeElement entityElement, int version, String dataset, List> saveTos, FormInstance mainInstance, EntityAction action) { List> fields = saveTos.stream().map(saveTo -> { IDataReference reference = saveTo.getFirst(); @@ -71,11 +61,7 @@ private Entity createEntity(TreeElement entityElement, int version, String datas }).collect(Collectors.toList()); String id = EntityFormParser.parseId(entityElement); - if (id == null) { - return null; - } else { - String label = EntityFormParser.parseLabel(entityElement); - return new Entity(action, dataset, id, label, version, fields); - } + String label = EntityFormParser.parseLabel(entityElement); + return new Entity(action, dataset, id, label, version, fields); } } diff --git a/src/main/java/org/javarosa/entities/internal/EntityFormParser.java b/src/main/java/org/javarosa/entities/internal/EntityFormParser.java index cb7fe29b5..add81232e 100644 --- a/src/main/java/org/javarosa/entities/internal/EntityFormParser.java +++ b/src/main/java/org/javarosa/entities/internal/EntityFormParser.java @@ -1,5 +1,6 @@ package org.javarosa.entities.internal; +import org.javarosa.core.model.data.IAnswerData; import org.javarosa.core.model.instance.FormInstance; import org.javarosa.core.model.instance.TreeElement; import org.javarosa.entities.EntityAction; @@ -22,7 +23,13 @@ public static String parseLabel(TreeElement entity) { TreeElement labelElement = entity.getFirstChild("label"); if (labelElement != null) { - return (String) labelElement.getValue().getValue(); + IAnswerData labelValue = labelElement.getValue(); + + if (labelValue != null) { + return (String) labelValue.getValue(); + } else { + return null; + } } else { return null; } diff --git a/src/test/java/org/javarosa/entities/EntitiesTest.java b/src/test/java/org/javarosa/entities/EntitiesTest.java index 7d9d00841..47a56971c 100644 --- a/src/test/java/org/javarosa/entities/EntitiesTest.java +++ b/src/test/java/org/javarosa/entities/EntitiesTest.java @@ -128,7 +128,7 @@ public void fillingFormWithCreate_makesEntityAvailable() throws IOException, XFo } @Test - public void fillingFormWithCreate_withoutAnId_doesNotMakeEntityAvailable() throws IOException, XFormParser.ParseException { + public void fillingFormWithCreate_withoutAnId_makesEntityAvailable() throws IOException, XFormParser.ParseException { Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( asList( new Pair<>("entities", "http://www.opendatakit.org/xforms/entities") @@ -139,6 +139,7 @@ public void fillingFormWithCreate_withoutAnId_doesNotMakeEntityAvailable() throw mainInstance( t("data id=\"create-entity-form\"", t("id"), + t("name"), t("meta", t("entity dataset=\"people\" create=\"1\" id=\"\"", t("label") @@ -152,7 +153,8 @@ public void fillingFormWithCreate_withoutAnId_doesNotMakeEntityAvailable() throw ) ), body( - input("/data/id") + input("/data/id"), + input("/data/name") ) )); @@ -160,7 +162,10 @@ public void fillingFormWithCreate_withoutAnId_doesNotMakeEntityAvailable() throw scenario.finalizeInstance(); List entities = scenario.getFormEntryController().getModel().getExtras().get(Entities.class).getEntities(); - assertThat(entities.size(), equalTo(0)); + assertThat(entities.size(), equalTo(1)); + assertThat(entities.get(0).dataset, equalTo("people")); + assertThat(entities.get(0).id, equalTo(null)); + assertThat(entities.get(0).action, equalTo(EntityAction.CREATE)); } @Test @@ -250,7 +255,7 @@ public void fillingFormWithUpdate_andNoLabel_makesEntityAvailableWithNullLabel() } @Test - public void fillingFormWithUpdate_withNullId_doesNotMakeEntityAvailable() throws IOException, XFormParser.ParseException { + public void fillingFormWithUpdate_withNullId_makesEntityAvailable() throws IOException, XFormParser.ParseException { Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( asList( new Pair<>("entities", "http://www.opendatakit.org/xforms/entities") @@ -279,7 +284,10 @@ public void fillingFormWithUpdate_withNullId_doesNotMakeEntityAvailable() throws scenario.finalizeInstance(); List entities = scenario.getFormEntryController().getModel().getExtras().get(Entities.class).getEntities(); - assertThat(entities.size(), equalTo(0)); + assertThat(entities.size(), equalTo(1)); + assertThat(entities.get(0).dataset, equalTo("people")); + assertThat(entities.get(0).id, equalTo(null)); + assertThat(entities.get(0).action, equalTo(EntityAction.UPDATE)); } @Test From a1dc506e837adb733236fbdcb2119e3973842ead Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 27 May 2024 09:53:29 +0100 Subject: [PATCH 5/5] Correct form name --- src/test/java/org/javarosa/entities/EntitiesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/javarosa/entities/EntitiesTest.java b/src/test/java/org/javarosa/entities/EntitiesTest.java index 47a56971c..7675b0844 100644 --- a/src/test/java/org/javarosa/entities/EntitiesTest.java +++ b/src/test/java/org/javarosa/entities/EntitiesTest.java @@ -215,7 +215,7 @@ public void fillingFormWithUpdate_makesEntityAvailable() throws IOException, XFo @Test public void fillingFormWithUpdate_andNoLabel_makesEntityAvailableWithNullLabel() throws IOException, XFormParser.ParseException { - Scenario scenario = Scenario.init("Create entity form", XFormsElement.html( + Scenario scenario = Scenario.init("Update entity form", XFormsElement.html( asList( new Pair<>("entities", "http://www.opendatakit.org/xforms/entities") ),