Merge pull request #10241 from atmire/w2p-124362_VersionedHandleIdentifierProviderWithCanonicalHandles-and-com-col-handles

Fix issue with VersionedHandleIdentifierProviderWithCanonicalHandles and com/col handles

dspace-api/src/main/java/org/dspace/identifier/VersionedHandleIdentifierProviderWithCanonicalHandles.java

@@ -15,11 +15,14 @@ import java.util.List;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.Collection;
+import org.dspace.content.Community;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.Item;
 import org.dspace.content.MetadataSchemaEnum;
 import org.dspace.content.MetadataValue;
-import org.dspace.content.service.ItemService;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.DSpaceObjectService;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.core.LogHelper;
@@ -64,9 +67,6 @@ public class VersionedHandleIdentifierProviderWithCanonicalHandles extends Ident
     @Autowired(required = true)
     private HandleService handleService;
 
-    @Autowired(required = true)
-    private ItemService itemService;
-
     /**
      * After all the properties are set check that the versioning is enabled
      *
@@ -173,6 +173,16 @@ public class VersionedHandleIdentifierProviderWithCanonicalHandles extends Ident
                 throw new RuntimeException("The current user is not authorized to change this item.", ex);
             }
         }
+        if (dso instanceof Collection || dso instanceof Community) {
+            try {
+                // Update the metadata with the handle for collections and communities.
+                modifyHandleMetadata(context, dso, getCanonical(id));
+            } catch (SQLException ex) {
+                throw new RuntimeException("A problem with the database connection occured.", ex);
+            } catch (AuthorizeException ex) {
+                throw new RuntimeException("The current user is not authorized to change this item.", ex);
+            }
+        }
 
         return id;
     }
@@ -491,27 +501,29 @@ public class VersionedHandleIdentifierProviderWithCanonicalHandles extends Ident
      * Remove all handles from an item's metadata and add the supplied handle instead.
      *
      * @param context The relevant DSpace Context.
-     * @param item    which item to modify
+     * @param dso    which dso to modify
      * @param handle  which handle to add
      * @throws SQLException       if database error
      * @throws AuthorizeException if authorization error
      */
-    protected void modifyHandleMetadata(Context context, Item item, String handle)
+    protected void modifyHandleMetadata(Context context, DSpaceObject dso, String handle)
         throws SQLException, AuthorizeException {
         // we want to exchange the old handle against the new one. To do so, we
         // load all identifiers, clear the metadata field, re add all
         // identifiers which are not from type handle and add the new handle.
         String handleref = handleService.getCanonicalForm(handle);
-        List<MetadataValue> identifiers = itemService
+        DSpaceObjectService<DSpaceObject> dSpaceObjectService =
-            .getMetadata(item, MetadataSchemaEnum.DC.getName(), "identifier", "uri", Item.ANY);
+            ContentServiceFactory.getInstance().getDSpaceObjectService(dso);
-        itemService.clearMetadata(context, item, MetadataSchemaEnum.DC.getName(), "identifier", "uri", Item.ANY);
+        List<MetadataValue> identifiers = dSpaceObjectService
+            .getMetadata(dso, MetadataSchemaEnum.DC.getName(), "identifier", "uri", Item.ANY);
+        dSpaceObjectService.clearMetadata(context, dso, MetadataSchemaEnum.DC.getName(), "identifier", "uri", Item.ANY);
         for (MetadataValue identifier : identifiers) {
             if (this.supports(identifier.getValue())) {
                 // ignore handles
                 continue;
             }
-            itemService.addMetadata(context,
+            dSpaceObjectService.addMetadata(context,
-                                    item,
+                                    dso,
                                     identifier.getMetadataField(),
                                     identifier.getLanguage(),
                                     identifier.getValue(),
@@ -519,9 +531,9 @@ public class VersionedHandleIdentifierProviderWithCanonicalHandles extends Ident
                                     identifier.getConfidence());
         }
         if (!StringUtils.isEmpty(handleref)) {
-            itemService.addMetadata(context, item, MetadataSchemaEnum.DC.getName(),
+            dSpaceObjectService.addMetadata(context, dso, MetadataSchemaEnum.DC.getName(),
                                     "identifier", "uri", null, handleref);
         }
-        itemService.update(context, item);
+        dSpaceObjectService.update(context, dso);
     }
 }

dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java

@@ -8,9 +8,9 @@
 package org.dspace.identifier;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 import java.sql.SQLException;
+import java.util.List;
 
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.builder.CollectionBuilder;
@@ -18,13 +18,18 @@ import org.dspace.builder.CommunityBuilder;
 import org.dspace.builder.ItemBuilder;
 import org.dspace.builder.VersionBuilder;
 import org.dspace.content.Collection;
+import org.dspace.content.Community;
 import org.dspace.content.Item;
+import org.dspace.content.MetadataValue;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.services.factory.DSpaceServicesFactory;
 import org.junit.Before;
 import org.junit.Test;
 
 public class VersionedHandleIdentifierProviderIT extends AbstractIdentifierProviderIT  {
 
     private String firstHandle;
+    private String dspaceUrl;
 
     private Collection collection;
     private Item itemV1;
@@ -36,12 +41,13 @@ public class VersionedHandleIdentifierProviderIT extends AbstractIdentifierProvi
     public void setUp() throws Exception {
         super.setUp();
         context.turnOffAuthorisationSystem();
+        dspaceUrl = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.ui.url");
         parentCommunity = CommunityBuilder.createCommunity(context)
-                .withName("Parent Community")
+                                          .withName("Parent Community")
-                .build();
+                                          .build();
         collection = CollectionBuilder.createCollection(context, parentCommunity)
-                .withName("Collection")
+                                      .withName("Collection")
-                .build();
+                                      .build();
     }
 
     private void createVersions() throws SQLException, AuthorizeException {
@@ -69,28 +75,38 @@ public class VersionedHandleIdentifierProviderIT extends AbstractIdentifierProvi
     }
 
     @Test
-    public void testCanonicalVersionedHandleProvider() throws Exception {
+    public void testCollectionHandleMetadata() {
-        registerProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class);
-        createVersions();
-
-        // Confirm the original item only has a version handle
-        assertEquals(firstHandle + ".1", itemV1.getHandle());
-        assertEquals(1, itemV1.getHandles().size());
-        // Confirm the second item has the correct version handle
-        assertEquals(firstHandle + ".2", itemV2.getHandle());
-        assertEquals(1, itemV2.getHandles().size());
-        // Confirm the last item has both the correct version handle and the original handle
-        assertEquals(firstHandle, itemV3.getHandle());
-        assertEquals(2, itemV3.getHandles().size());
-        containsHandle(itemV3, firstHandle + ".3");
-
-        // Unregister this non-default provider
-        unregisterProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class);
-        // Re-register the default provider (for later tests)
         registerProvider(VersionedHandleIdentifierProvider.class);
+
+        Community testCommunity = CommunityBuilder.createCommunity(context)
+                                                  .withName("Test community")
+                                                  .build();
+
+        Collection testCollection = CollectionBuilder.createCollection(context, testCommunity)
+                                                     .withName("Test Collection")
+                                                     .build();
+
+        List<MetadataValue> metadata = ContentServiceFactory.getInstance().getDSpaceObjectService(testCollection)
+                                                            .getMetadata(testCollection, "dc", "identifier", "uri",
+                                                                         Item.ANY);
+
+        assertEquals(1, metadata.size());
+        assertEquals(dspaceUrl + "/handle/" + testCollection.getHandle(), metadata.get(0).getValue());
     }
 
-    private void containsHandle(Item item, String handle) {
+    @Test
-        assertTrue(item.getHandles().stream().anyMatch(h -> handle.equals(h.getHandle())));
+    public void testCommunityHandleMetadata() {
+        registerProvider(VersionedHandleIdentifierProvider.class);
+
+        Community testCommunity = CommunityBuilder.createCommunity(context)
+                                                  .withName("Test community")
+                                                  .build();
+
+        List<MetadataValue> metadata = ContentServiceFactory.getInstance().getDSpaceObjectService(testCommunity)
+                                                            .getMetadata(testCommunity, "dc", "identifier", "uri",
+                                                                         Item.ANY);
+
+        assertEquals(1, metadata.size());
+        assertEquals(dspaceUrl + "/handle/" + testCommunity.getHandle(), metadata.get(0).getValue());
     }
 }

dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderWithCanonicalHandlesIT.java

@@ -0,0 +1,146 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.identifier;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.dspace.AbstractIntegrationTestWithDatabase;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.builder.CollectionBuilder;
+import org.dspace.builder.CommunityBuilder;
+import org.dspace.builder.ItemBuilder;
+import org.dspace.builder.VersionBuilder;
+import org.dspace.content.Collection;
+import org.dspace.content.Community;
+import org.dspace.content.Item;
+import org.dspace.content.MetadataValue;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.kernel.ServiceManager;
+import org.dspace.services.factory.DSpaceServicesFactory;
+import org.junit.Before;
+import org.junit.Test;
+
+public class VersionedHandleIdentifierProviderWithCanonicalHandlesIT extends AbstractIntegrationTestWithDatabase {
+    private ServiceManager serviceManager;
+    private IdentifierServiceImpl identifierService;
+
+    private String firstHandle;
+    private String dspaceUrl;
+
+
+    private Collection collection;
+    private Item itemV1;
+    private Item itemV2;
+    private Item itemV3;
+
+    @Before
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        context.turnOffAuthorisationSystem();
+
+        serviceManager = DSpaceServicesFactory.getInstance().getServiceManager();
+        dspaceUrl = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.ui.url");
+        identifierService = serviceManager.getServicesByType(IdentifierServiceImpl.class).get(0);
+        // Clean out providers to avoid any being used for creation of community and collection
+        identifierService.setProviders(new ArrayList<>());
+
+        parentCommunity = CommunityBuilder.createCommunity(context)
+                                          .withName("Parent Community")
+                                          .build();
+        collection = CollectionBuilder.createCollection(context, parentCommunity)
+                                      .withName("Collection")
+                                      .build();
+
+
+    }
+
+    private void registerProvider(Class type) {
+        // Register our new provider
+        List servicesByType = serviceManager.getServicesByType(type);
+        if (servicesByType.isEmpty()) {
+            serviceManager.registerServiceClass(type.getName(), type);
+        }
+        IdentifierProvider identifierProvider =
+            (IdentifierProvider) serviceManager.getServiceByName(type.getName(), type);
+
+        // Overwrite the identifier-service's providers with the new one to ensure only this provider is used
+        identifierService.setProviders(List.of(identifierProvider));
+    }
+
+    private void createVersions() throws SQLException, AuthorizeException {
+        itemV1 = ItemBuilder.createItem(context, collection)
+                            .withTitle("First version")
+                            .build();
+        firstHandle = itemV1.getHandle();
+        itemV2 = VersionBuilder.createVersion(context, itemV1, "Second version").build().getItem();
+        itemV3 = VersionBuilder.createVersion(context, itemV1, "Third version").build().getItem();
+    }
+
+    @Test
+    public void testCanonicalVersionedHandleProvider() throws Exception {
+        registerProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class);
+        createVersions();
+
+        // Confirm the original item only has a version handle
+        assertEquals(firstHandle + ".1", itemV1.getHandle());
+        assertEquals(1, itemV1.getHandles().size());
+        // Confirm the second item has the correct version handle
+        assertEquals(firstHandle + ".2", itemV2.getHandle());
+        assertEquals(1, itemV2.getHandles().size());
+        // Confirm the last item has both the correct version handle and the original handle
+        assertEquals(firstHandle, itemV3.getHandle());
+        assertEquals(2, itemV3.getHandles().size());
+        containsHandle(itemV3, firstHandle + ".3");
+    }
+
+    @Test
+    public void testCollectionHandleMetadata() {
+        registerProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class);
+
+        Community testCommunity = CommunityBuilder.createCommunity(context)
+                                                  .withName("Test community")
+                                                  .build();
+
+        Collection testCollection = CollectionBuilder.createCollection(context, testCommunity)
+                                                     .withName("Test Collection")
+                                                     .build();
+
+        List<MetadataValue> metadata = ContentServiceFactory.getInstance().getDSpaceObjectService(testCollection)
+                                                            .getMetadata(testCollection, "dc", "identifier", "uri",
+                                                                         Item.ANY);
+
+        assertEquals(1, metadata.size());
+        assertEquals(dspaceUrl + "/handle/" + testCollection.getHandle(), metadata.get(0).getValue());
+    }
+
+    @Test
+    public void testCommunityHandleMetadata() {
+        registerProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class);
+
+        Community testCommunity = CommunityBuilder.createCommunity(context)
+                                                  .withName("Test community")
+                                                  .build();
+
+        List<MetadataValue> metadata = ContentServiceFactory.getInstance().getDSpaceObjectService(testCommunity)
+                                                            .getMetadata(testCommunity, "dc", "identifier", "uri",
+                                                                         Item.ANY);
+
+        assertEquals(1, metadata.size());
+        assertEquals(dspaceUrl + "/handle/" + testCommunity.getHandle(), metadata.get(0).getValue());
+    }
+
+    private void containsHandle(Item item, String handle) {
+        assertTrue(item.getHandles().stream().anyMatch(h -> handle.equals(h.getHandle())));
+    }
+}

dspace/config/spring/api/identifier-service.xml

@@ -19,11 +19,11 @@
     </bean>
     -->
 
-      <!-- If you enabled versioning, you should use one of the versioned
+    <!-- If you enabled versioning, you should use one of the versioned
-           handle identifier provider instead of the default one.
+         handle identifier provider instead of the default one.
-           The VersionedHandleIdentifierProvider creates a new versioned
+         The VersionedHandleIdentifierProvider creates a new versioned
-           handle for every new version.
+         handle for every new version.
-           -->
+         -->
     <bean id="org.dspace.identifier.HandleIdentifierProvider" class="org.dspace.identifier.VersionedHandleIdentifierProvider" scope="singleton">
         <property name="configurationService" ref="org.dspace.services.ConfigurationService"/>
     </bean>