From 95e60b385f7e940fac46dc59683e4485ca52e7c7 Mon Sep 17 00:00:00 2001 From: bencomp Date: Mon, 3 Oct 2022 01:58:47 +0200 Subject: [PATCH 1/5] Overload DatasetField method to remove type check I spotted a TODO near a type check based on a string comparison of the class name. --- .../edu/harvard/iq/dataverse/DatasetField.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java index 31d08f84c02..772538aa5cb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java @@ -54,15 +54,18 @@ public int compare(DatasetField o1, DatasetField o2) { o2.getDatasetFieldType().getDisplayOrder() ); }}; - public static DatasetField createNewEmptyDatasetField(DatasetFieldType dsfType, Object dsv) { + public static DatasetField createNewEmptyDatasetField(DatasetFieldType dsfType, DatasetVersion dsv) { DatasetField dsfv = createNewEmptyDatasetField(dsfType); - //TODO - a better way to handle this? - if (dsv.getClass().getName().equals("edu.harvard.iq.dataverse.DatasetVersion")){ - dsfv.setDatasetVersion((DatasetVersion)dsv); - } else { - dsfv.setTemplate((Template)dsv); - } + dsfv.setDatasetVersion(dsv); + + return dsfv; + } + + public static DatasetField createNewEmptyDatasetField(DatasetFieldType dsfType, Template dsv) { + + DatasetField dsfv = createNewEmptyDatasetField(dsfType); + dsfv.setTemplate(dsv); return dsfv; } From daccc0e3f4d4efee653489f249f8a1db9e3828bd Mon Sep 17 00:00:00 2001 From: bencomp Date: Mon, 3 Oct 2022 12:32:13 +0200 Subject: [PATCH 2/5] Compare types using instanceof --- src/main/java/edu/harvard/iq/dataverse/DatasetField.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java index 772538aa5cb..e6a4ca21fea 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java @@ -563,7 +563,7 @@ private DatasetField copy(Object version, DatasetFieldCompoundValue parent) { dsf.setDatasetFieldType(datasetFieldType); if (version != null) { - if (version.getClass().getName().equals("edu.harvard.iq.dataverse.DatasetVersion")) { + if (version instanceof DatasetVersion) { dsf.setDatasetVersion((DatasetVersion) version); } else { dsf.setTemplate((Template) version); From bdb62163d8ad4ed01b94943ad3c45f13406a6dae Mon Sep 17 00:00:00 2001 From: bencomp Date: Mon, 3 Oct 2022 13:07:46 +0200 Subject: [PATCH 3/5] Create type-specific copy methods in DatasetField Both `copy(DatasetVersion)` and `copy(Template)` still refer to `copy(Object, DatasetFieldCompoundValue)`, because copying that method would make the code less DRY. --- .../edu/harvard/iq/dataverse/DatasetField.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java index e6a4ca21fea..35a8184e45b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetField.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetField.java @@ -548,9 +548,12 @@ public String toString() { return "edu.harvard.iq.dataverse.DatasetField[ id=" + id + " ]"; } - public DatasetField copy(Object version) { + public DatasetField copy(DatasetVersion version) { return copy(version, null); } + public DatasetField copy(Template template) { + return copy(template, null); + } // originally this was an overloaded method, but we renamed it to get around an issue with Bean Validation // (that looked t overloaded methods, when it meant to look at overriden methods @@ -558,15 +561,15 @@ public DatasetField copyChild(DatasetFieldCompoundValue parent) { return copy(null, parent); } - private DatasetField copy(Object version, DatasetFieldCompoundValue parent) { + private DatasetField copy(Object versionOrTemplate, DatasetFieldCompoundValue parent) { DatasetField dsf = new DatasetField(); dsf.setDatasetFieldType(datasetFieldType); - if (version != null) { - if (version instanceof DatasetVersion) { - dsf.setDatasetVersion((DatasetVersion) version); + if (versionOrTemplate != null) { + if (versionOrTemplate instanceof DatasetVersion) { + dsf.setDatasetVersion((DatasetVersion) versionOrTemplate); } else { - dsf.setTemplate((Template) version); + dsf.setTemplate((Template) versionOrTemplate); } } From 4579283f8e5039cbbc6c8df43ac73ee5fd63b967 Mon Sep 17 00:00:00 2001 From: bencomp Date: Mon, 18 Sep 2023 00:07:16 +0200 Subject: [PATCH 4/5] Add basic DatasetFieldTest --- .../iq/dataverse/DatasetFieldTest.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java diff --git a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java new file mode 100644 index 00000000000..23ba2d69fff --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java @@ -0,0 +1,41 @@ +package edu.harvard.iq.dataverse; + +import edu.harvard.iq.dataverse.DatasetFieldType.FieldType; +import edu.harvard.iq.dataverse.mocks.MocksFactory; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + + +class DatasetFieldTest { + @Test + void testCreateNewEmptyDatasetField_withEmptyTemplate() { + Template template = new Template(); + + DatasetField field = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), template); + assertTrue(field.getTemplate() == template); + assertTrue(template.getDatasetFields().isEmpty()); + } + + @Test + void testNotEqualDatasetFields() { + DatasetField field1 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + field1.setId(MocksFactory.nextId()); + DatasetField field2 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + field2.setId(MocksFactory.nextId()); + + assertNotEquals(field1, field2); + } + + @Test + void testEqualDatasetFields() { + DatasetField field1 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + field1.setId(100L); + DatasetField field2 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + field2.setId(100L); + + assertEquals(field1, field2); + } +} \ No newline at end of file From 87bfe16c6a4390075bd0b481071ebddf2f1c1e5d Mon Sep 17 00:00:00 2001 From: bencomp Date: Mon, 18 Sep 2023 00:25:52 +0200 Subject: [PATCH 5/5] Test DatasetField identities --- .../iq/dataverse/DatasetFieldTest.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java index 23ba2d69fff..97999af3244 100644 --- a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNull; class DatasetFieldTest { @@ -21,12 +22,17 @@ void testCreateNewEmptyDatasetField_withEmptyTemplate() { @Test void testNotEqualDatasetFields() { - DatasetField field1 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + DatasetFieldType type1 = new DatasetFieldType("subject", FieldType.TEXT, false); + Template template1 = new Template(); + DatasetField field1 = DatasetField.createNewEmptyDatasetField(type1, template1); field1.setId(MocksFactory.nextId()); - DatasetField field2 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + DatasetFieldType type2 = new DatasetFieldType("subject", FieldType.TEXT, false); + Template template2 = new Template(); + DatasetField field2 = DatasetField.createNewEmptyDatasetField(type2, template2); field2.setId(MocksFactory.nextId()); assertNotEquals(field1, field2); + assertNotEquals(field1, template2); } @Test @@ -34,8 +40,25 @@ void testEqualDatasetFields() { DatasetField field1 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); field1.setId(100L); DatasetField field2 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + + // Fields are not equal before both have IDs set + assertNotEquals(field1, field2); + field2.setId(100L); assertEquals(field1, field2); } + + @Test + void testCopyDatasetFields() { + DatasetField field1 = DatasetField.createNewEmptyDatasetField(new DatasetFieldType("subject", FieldType.TEXT, false), new Template()); + field1.setId(100L); + DatasetField field2 = field1.copy(field1.getTemplate()); + + assertNull(field2.getId()); + // A copy of a field should not be equal + assertNotEquals(field1, field2); + + assertEquals(field2.getDatasetFieldType(), field1.getDatasetFieldType()); + } } \ No newline at end of file