Skip to content

Commit

Permalink
Merge pull request DSpace#9714 from atmire/w2p-116319_fix-metadata-im…
Browse files Browse the repository at this point in the history 
…port-setting-language-as-any-instead-of-null-upstream-main

Fix issue where CSV Import / Export can clear metadata if there are metadata values with language "*" (Item.ANY)
  • Loading branch information
@tdonohue
tdonohue authored Jul 25, 2024
2 parents 45248a8 + be179ba commit 8bc235b
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 86 deletions.
2 changes: 1 addition & 1 deletion dspace-api/src/main/java/org/dspace/administer/StructBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ private static Element[] handleCollections(Context context,

// default the short description to the empty string
collectionService.setMetadataSingleValue(context, collection,
MD_SHORT_DESCRIPTION, Item.ANY, " ");
MD_SHORT_DESCRIPTION, null, " ");

// import the rest of the metadata
for (Map.Entry<String, MetadataFieldName> entry : collectionMap.entrySet()) {
Expand Down
18 changes: 18 additions & 0 deletions dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,15 @@ public DSpaceCSV(InputStream inputStream, Context c) throws Exception {
// Verify that the heading is valid in the metadata registry
String[] clean = element.split("\\[");
String[] parts = clean[0].split("\\.");
// Check language if present, if it's ANY then throw an exception
if (clean.length > 1 && clean[1].equals(Item.ANY + "]")) {
throw new MetadataImportInvalidHeadingException("Language ANY (*) was found in the heading " +
"of the metadata value to import, " +
"this should never be the case",
MetadataImportInvalidHeadingException.ENTRY,
columnCounter);

}

if (parts.length < 2) {
throw new MetadataImportInvalidHeadingException(element,
Expand Down Expand Up @@ -223,6 +232,15 @@ public DSpaceCSV(InputStream inputStream, Context c) throws Exception {
}
}

// Verify there isn’t already a header that is the same; if it already exists,
// throw MetadataImportInvalidHeadingException
String header = authorityPrefix + element;
if (headings.contains(header)) {
throw new MetadataImportInvalidHeadingException("Duplicate heading found: " + header,
MetadataImportInvalidHeadingException.ENTRY,
columnCounter);
}

// Store the heading
headings.add(authorityPrefix + element);
}
Expand Down
2 changes: 1 addition & 1 deletion dspace-api/src/main/java/org/dspace/content/Collection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public String getLicenseCollection() {
* @throws SQLException if database error
*/
public void setLicense(Context context, String license) throws SQLException {
getCollectionService().setMetadataSingleValue(context, this, MD_LICENSE, Item.ANY, license);
getCollectionService().setMetadataSingleValue(context, this, MD_LICENSE, null, license);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions dspace-api/src/main/java/org/dspace/content/MetadataValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import jakarta.persistence.SequenceGenerator;
import jakarta.persistence.Table;
import jakarta.persistence.Transient;
import org.apache.commons.lang3.StringUtils;
import org.dspace.core.Context;
import org.dspace.core.HibernateProxyHelper;
import org.dspace.core.ReloadableEntity;
Expand Down Expand Up @@ -139,6 +140,9 @@ public String getLanguage() {
* @param language new language
*/
public void setLanguage(String language) {
if (StringUtils.equals(language, Item.ANY)) {
language = null;
}
this.language = language;
}

Expand Down
27 changes: 26 additions & 1 deletion dspace-api/src/test/java/org/dspace/app/bulkedit/MetadataImportIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,31 @@ public void setUp() throws Exception {
context.restoreAuthSystemState();
}

@Test
public void metadataImportTestWithDuplicateHeader() {
String[] csv = {"id,collection,dc.title,dc.title,dc.contributor.author",
"+," + collection.getHandle() + ",\"Test Import 1\",\"Test Import 2\"," + "\"Donald, SmithImported\"," +
"+," + collection.getHandle() + ",\"Test Import 3\",\"Test Import 4\"," + "\"Donald, SmithImported\""};
// Should throw an exception because of duplicate header
try {
performImportScript(csv);
} catch (Exception e) {
assertTrue(e instanceof MetadataImportInvalidHeadingException);
}
}

@Test
public void metadataImportTestWithAnyLanguage() {
String[] csv = {"id,collection,dc.title[*],dc.contributor.author",
"+," + collection.getHandle() + ",\"Test Import 1\"," + "\"Donald, SmithImported\""};
// Should throw an exception because of invalid ANY language (*) in metadata field
try {
performImportScript(csv);
} catch (Exception e) {
assertTrue(e instanceof MetadataImportInvalidHeadingException);
}
}

@Test
public void metadataImportTest() throws Exception {
String[] csv = {"id,collection,dc.title,dc.contributor.author",
Expand Down Expand Up @@ -230,7 +255,7 @@ public void metadataImportRemovingValueTest() throws Exception {
itemService.getMetadata(item, "dc", "contributor", "author", Item.ANY).get(0).getValue(),
"TestAuthorToRemove"));

String[] csv = {"id,collection,dc.title,dc.contributor.author[*]",
String[] csv = {"id,collection,dc.title,dc.contributor.author",
item.getID().toString() + "," + personCollection.getHandle() + "," + item.getName() + ","};
performImportScript(csv);
item = findItemByName(itemTitle);
Expand Down
3 changes: 1 addition & 2 deletions dspace-api/src/test/java/org/dspace/builder/AbstractDSpaceObjectBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.dspace.authorize.AuthorizeException;
import org.dspace.authorize.ResourcePolicy;
import org.dspace.content.DSpaceObject;
import org.dspace.content.Item;
import org.dspace.content.service.DSpaceObjectService;
import org.dspace.core.Constants;
import org.dspace.core.Context;
Expand Down Expand Up @@ -103,7 +102,7 @@ protected <B extends AbstractDSpaceObjectBuilder<T>> B setMetadataSingleValue(fi
final String qualifier,
final String value) {
try {
getService().setMetadataSingleValue(context, dso, schema, element, qualifier, Item.ANY, value);
getService().setMetadataSingleValue(context, dso, schema, element, qualifier, null, value);
} catch (Exception e) {
return handleException(e);
}
Expand Down
64 changes: 32 additions & 32 deletions dspace-api/src/test/java/org/dspace/content/ItemComparatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,98 +141,98 @@ public void testCompare() throws SQLException {
assertTrue("testCompare 0", result == 0);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
result = ic.compare(one, two);
assertTrue("testCompare 1", result >= 1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
result = ic.compare(one, two);
assertTrue("testCompare 2", result <= -1);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

//value in both items
ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "2");
result = ic.compare(one, two);
assertTrue("testCompare 3", result <= -1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
result = ic.compare(one, two);
assertTrue("testCompare 4", result == 0);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
result = ic.compare(one, two);
assertTrue("testCompare 5", result >= 1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

//multiple values (min, max)
ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "0");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "3");
itemService.addMetadata(context, one, "dc", "test", "one", null, "0");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "3");
result = ic.compare(one, two);
assertTrue("testCompare 3", result <= -1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "0");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "-1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "0");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "-1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
result = ic.compare(one, two);
assertTrue("testCompare 4", result == 0);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, true);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "-1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "-1");
result = ic.compare(one, two);
assertTrue("testCompare 5", result >= 1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, false);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "3");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "3");
result = ic.compare(one, two);
assertTrue("testCompare 3", result <= -1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, false);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "5");
itemService.addMetadata(context, one, "dc", "test", "one", null, "1");
itemService.addMetadata(context, one, "dc", "test", "one", null, "2");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "5");
result = ic.compare(one, two);
assertTrue("testCompare 4", result == 0);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
itemService.clearMetadata(context, two, "dc", "test", "one", Item.ANY);

ic = new ItemComparator("test", "one", Item.ANY, false);
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "2");
itemService.addMetadata(context, one, "dc", "test", "one", Item.ANY, "3");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "1");
itemService.addMetadata(context, two, "dc", "test", "one", Item.ANY, "4");
itemService.addMetadata(context, one, "dc", "test", "one", null, "2");
itemService.addMetadata(context, one, "dc", "test", "one", null, "3");
itemService.addMetadata(context, two, "dc", "test", "one", null, "1");
itemService.addMetadata(context, two, "dc", "test", "one", null, "4");
result = ic.compare(one, two);
assertTrue("testCompare 5", result >= 1);
itemService.clearMetadata(context, one, "dc", "test", "one", Item.ANY);
Expand Down
Loading

0 comments on commit 8bc235b

Please sign in to comment.