Merge pull request #1459 from tdonohue/DS-3262

Fixes for DS-3262 and DS-3263 (deleting or renaming/missing metadata fields)

dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java

@@ -123,14 +123,23 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
 
     @Override
     public List<Collection> findAll(Context context) throws SQLException {
-        MetadataField nameField = metadataFieldService.findByElement(context, "dc", "title", null);
+        MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
-        return collectionDAO.findAll(context, nameField);
+        if(nameField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".title' doesn't exist!");
+        }
 
+        return collectionDAO.findAll(context, nameField);
     }
 
     @Override
     public List<Collection> findAll(Context context, Integer limit, Integer offset) throws SQLException {
-        MetadataField nameField = metadataFieldService.findByElement(context, "dc", "title", null);
+        MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
+        if(nameField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".title' doesn't exist!");
+        }
+
         return collectionDAO.findAll(context, nameField, limit, offset);
     }
 

dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java

@@ -137,12 +137,22 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
     @Override
     public List<Community> findAll(Context context) throws SQLException {
         MetadataField sortField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
+        if(sortField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".title' doesn't exist!");
+        }
+
         return communityDAO.findAll(context, sortField);
     }
 
     @Override
     public List<Community> findAll(Context context, Integer limit, Integer offset) throws SQLException {
         MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
+        if(nameField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".title' doesn't exist!");
+        }
+
         return communityDAO.findAll(context, nameField, limit, offset);
     }
 
@@ -151,6 +161,11 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
     {
         // get all communities that are not children
         MetadataField sortField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
+        if(sortField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".title' doesn't exist!");
+        }
+
         return communityDAO.findAllNoParent(context, sortField);
     }
 

dspace-api/src/main/java/org/dspace/content/DSpaceObjectServiceImpl.java

@@ -207,7 +207,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
     public void addMetadata(Context context, T dso, String schema, String element, String qualifier, String lang, List<String> values) throws SQLException {
         MetadataField metadataField = metadataFieldService.findByElement(context, schema, element, qualifier);
         if (metadataField == null) {
-            throw new SQLException("bad_dublin_core schema=" + schema + "." + element + "." + qualifier);
+            throw new SQLException("bad_dublin_core schema=" + schema + "." + element + "." + qualifier + ". Metadata field does not exist!");
         }
 
         addMetadata(context, dso, metadataField, lang, values);
@@ -219,7 +219,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
         // until update() is called.
         MetadataField metadataField = metadataFieldService.findByElement(context, schema, element, qualifier);
         if (metadataField == null) {
-            throw new SQLException("bad_dublin_core schema=" + schema + "." + element + "." + qualifier);
+            throw new SQLException("bad_dublin_core schema=" + schema + "." + element + "." + qualifier + ". Metadata field does not exist!");
         }
         addMetadata(context, dso, metadataField, lang, values, authorities, confidences);
     }
@@ -332,16 +332,15 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
 
     @Override
     public void clearMetadata(Context context, T dso, String schema, String element, String qualifier, String lang) throws SQLException {
-        // We will build a list of values NOT matching the values to clear
         Iterator<MetadataValue> metadata = dso.getMetadata().iterator();
         while (metadata.hasNext())
         {
             MetadataValue metadataValue = metadata.next();
+            // If this value matches, delete it
             if (match(schema, element, qualifier, lang, metadataValue))
             {
-                metadataValue.setDSpaceObject(null);
                 metadata.remove();
-//                metadataValueService.delete(context, metadataValue);
+                metadataValueService.delete(context, metadataValue);
             }
         }
         dso.setMetadataModified();
@@ -355,7 +354,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
             if(values.contains(metadataValue))
             {
                 metadata.remove();
-//                metadataValueService.delete(context, metadataValue);
+                metadataValueService.delete(context, metadataValue);
             }
         }
         dso.setMetadataModified();

dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java

@@ -192,6 +192,11 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
     public Iterator<Item> findBySubmitterDateSorted(Context context, EPerson eperson, Integer limit) throws SQLException {
 
         MetadataField metadataField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "date", "accessioned");
+        if(metadataField==null)
+        {
+            throw new IllegalArgumentException("Required metadata field '" + MetadataSchema.DC_SCHEMA + ".date.accessioned' doesn't exist!");
+        }
+
         return itemDAO.findBySubmitter(context, eperson, metadataField, limit);
     }
 

dspace-api/src/main/java/org/dspace/content/MetadataFieldServiceImpl.java

@@ -20,6 +20,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
+import org.apache.commons.collections.CollectionUtils;
 
 /**
  * Service implementation for the MetadataField object.
@@ -140,11 +141,29 @@ public class MetadataFieldServiceImpl implements MetadataFieldService {
                     "Only administrators may modify the metadata registry");
         }
 
+        // Check for existing usages of this field
+        List<MetadataValue> values = null;
+        try
+        {
+           values = metadataValueService.findByField(context, metadataField);
+        }
+        catch(IOException io)
+        {
+            // ignore
+        }
+
+        // Only remove this field if it is NOT in use (as we don't want to bulk delete metadata values)
+        if(CollectionUtils.isEmpty(values))
+        {
+            metadataFieldDAO.delete(context, metadataField);
+        }
+        else
+        {
+            throw new IllegalStateException("Metadata field " + metadataField.toString() + " cannot be deleted as it is currently used by one or more objects.");
+        }
+
         log.info(LogManager.getHeader(context, "delete_metadata_field",
                 "metadata_field_id=" + metadataField.getID()));
-
-        metadataValueService.deleteByMetadataField(context, metadataField);
-        metadataFieldDAO.delete(context, metadataField);
     }
 
     /**

dspace-api/src/test/java/org/dspace/content/ItemComparatorTest.java

@@ -34,6 +34,7 @@ public class ItemComparatorTest extends AbstractUnitTest
     protected InstallItemService installItemService = ContentServiceFactory.getInstance().getInstallItemService();
     protected MetadataSchemaService metadataSchemaService = ContentServiceFactory.getInstance().getMetadataSchemaService();
     protected MetadataFieldService metadataFieldService = ContentServiceFactory.getInstance().getMetadataFieldService();
+    private MetadataValueService metadataValueService = ContentServiceFactory.getInstance().getMetadataValueService();
 
     /**
      * Item instance for the tests
@@ -104,6 +105,9 @@ public class ItemComparatorTest extends AbstractUnitTest
     {
         context.turnOffAuthorisationSystem();
           try{
+              // Remove all values added to the test MetadataField (MetadataField cannot be deleted if it is still used)
+              metadataValueService.deleteByMetadataField(context, metadataField);
+              // Delete the (unused) metadataField
               metadataFieldService.delete(context, metadataField);
               communityService.delete(context, owningCommunity);
               context.restoreAuthSystemState();