[TLC-249] Address review feedback

Controller updated to be less DOI-specific in its
registration implementation
item-status.register renamed to registerDOI
display types included in section data, for UI hints
comments / javadoc added
DOI status text sent instead of number in status data

As per tdonohue's revew on 2022-01-18

dspace-api/src/main/java/org/dspace/content/logic/condition/IsArchivedCondition.java

@@ -14,7 +14,7 @@ import org.dspace.content.logic.LogicalStatementException;
 import org.dspace.core.Context;
 
 /**
- * A condition that returns true if the item is withdrawn
+ * A condition that returns true if the item is archived
  *
  * @author Kim Shepherd
  * @version $Revision$
@@ -23,7 +23,7 @@ public class IsArchivedCondition extends AbstractCondition {
     private final static Logger log = LogManager.getLogger();
 
     /**
-     * Return true if item is withdrawn
+     * Return true if item is archived
      * Return false if not
      * @param context   DSpace context
      * @param item      Item to evaluate
@@ -32,7 +32,7 @@ public class IsArchivedCondition extends AbstractCondition {
      */
     @Override
     public boolean getResult(Context context, Item item) throws LogicalStatementException {
-        log.debug("Result of isWithdrawn is " + item.isArchived());
+        log.debug("Result of isArchived is " + item.isArchived());
         return item.isArchived();
     }
 }

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

@@ -97,6 +97,21 @@ public class DOIIdentifierProvider extends FilteredIdentifierProvider {
     // The DOI is created in the database, but no more context is known
     public static final Integer MINTED = 11;
 
+    public static final String[] statusText = {
+        "UNKNOWN",                      // 0
+        "TO_BE_REGISTERED",             // 1
+        "TO_BE_RESERVED",               // 2
+        "IS_REGISTERED",                // 3
+        "IS_RESERVED",                  // 4
+        "UPDATE_RESERVED",              // 5
+        "UPDATE_REGISTERED",            // 6
+        "UPDATE_BEFORE_REGISTRATION",   // 7
+        "TO_BE_DELETED",                // 8
+        "DELETED",                      // 9
+        "PENDING",                      // 10
+        "MINTED",                       // 11
+    };
+
     @Autowired(required = true)
     protected DOIService doiService;
     @Autowired(required = true)

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

@@ -117,9 +117,8 @@ public class IdentifierServiceImpl implements IdentifierService {
     @Override
     public void register(Context context, DSpaceObject dso, Class<? extends Identifier> type, Filter filter)
             throws AuthorizeException, SQLException, IdentifierException {
-        //We need to commit our context because one of the providers might require the handle created above
-        // Next resolve all other services
         boolean registered = false;
+        // Iterate all services and register identifiers as appropriate
         for (IdentifierProvider service : providers) {
             if (service.supports(type)) {
                 try {
@@ -139,16 +138,15 @@ public class IdentifierServiceImpl implements IdentifierService {
             throw new IdentifierException("Cannot register identifier: Didn't "
                     + "find a provider that supports this identifier.");
         }
-        //Update our item / collection / community
+        // Update our item / collection / community
         contentServiceFactory.getDSpaceObjectService(dso).update(context, dso);
     }
 
     @Override
     public void register(Context context, DSpaceObject dso, Class<? extends Identifier> type)
             throws AuthorizeException, SQLException, IdentifierException {
-        //We need to commit our context because one of the providers might require the handle created above
-        // Next resolve all other services
         boolean registered = false;
+        // Iterate all services and register identifiers as appropriate
         for (IdentifierProvider service : providers) {
             if (service.supports(type)) {
                 try {
@@ -163,15 +161,14 @@ public class IdentifierServiceImpl implements IdentifierService {
             throw new IdentifierException("Cannot register identifier: Didn't "
                     + "find a provider that supports this identifier.");
         }
-        //Update our item / collection / community
+        // Update our item / collection / community
         contentServiceFactory.getDSpaceObjectService(dso).update(context, dso);
     }
 
     @Override
     public void register(Context context, DSpaceObject dso, Map<Class<? extends Identifier>, Filter> typeFilters)
             throws AuthorizeException, SQLException, IdentifierException {
-        //We need to commit our context because one of the providers might require the handle created above
-        // Next resolve all other services
+        // Iterate all services and register identifiers as appropriate
         for (IdentifierProvider service : providers) {
             try {
                 // If the service supports filtering, look through the map and the first supported class
@@ -200,7 +197,7 @@ public class IdentifierServiceImpl implements IdentifierService {
                 log.warn("Identifier not registered (inapplicable): " + e.getMessage());
             }
         }
-        //Update our item / collection / community
+        // Update our item / collection / community
         contentServiceFactory.getDSpaceObjectService(dso).update(context, dso);
     }
 
@@ -209,8 +206,7 @@ public class IdentifierServiceImpl implements IdentifierService {
     @Override
     public void register(Context context, DSpaceObject object, String identifier)
         throws AuthorizeException, SQLException, IdentifierException {
-        //We need to commit our context because one of the providers might require the handle created above
-        // Next resolve all other services
+        // Iterate all services and register identifiers as appropriate
         boolean registered = false;
         for (IdentifierProvider service : providers) {
             if (service.supports(identifier)) {
@@ -226,7 +222,7 @@ public class IdentifierServiceImpl implements IdentifierService {
             throw new IdentifierException("Cannot register identifier: Didn't "
                                               + "find a provider that supports this identifier.");
         }
-        //Update our item / collection / community
+        // pdate our item / collection / community
         contentServiceFactory.getDSpaceObjectService(object).update(context, object);
     }
 

dspace-server-webapp/src/main/java/org/dspace/app/rest/ItemIdentifierController.java

@@ -10,15 +10,12 @@ package org.dspace.app.rest;
 import static org.dspace.app.rest.utils.RegexUtils.REGEX_REQUESTMAPPING_IDENTIFIER_AS_UUID;
 
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.UUID;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.dspace.app.rest.converter.ConverterService;
 import org.dspace.app.rest.converter.MetadataConverter;
-import org.dspace.app.rest.model.IdentifierRest;
 import org.dspace.app.rest.model.IdentifiersRest;
 import org.dspace.app.rest.model.ItemRest;
 import org.dspace.app.rest.model.hateoas.ItemResource;
@@ -49,6 +46,7 @@ import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 
 /**
@@ -84,28 +82,46 @@ public class ItemIdentifierController {
     Utils utils;
 
     /**
-     * Queue a new, pending or minted DOI for registration for a given item. Requires administrative privilege.
-     * This request is sent from the Register DOI button (configurable) on the item status page.
+     * Request that an identifier of a given type is 'created' for an item. Depending on the identifier type
+     * this could mean minting, registration, reservation, queuing for registration later, etc.
      *
-     * @return 302 FOUND if the DOI is already registered or reserved, 201 CREATED if queued for registration
+     * @return 201 CREATED on success, or 302 FOUND if already created, or an error
      */
     @RequestMapping(method = RequestMethod.POST)
     @PreAuthorize("hasPermission(#uuid, 'ITEM', 'ADMIN')")
     public ResponseEntity<RepresentationModel<?>> registerIdentifierForItem(@PathVariable UUID uuid,
                                                                             HttpServletRequest request,
-                                                                            HttpServletResponse response)
+                                                                            HttpServletResponse response,
+                                                                            @RequestParam(name = "type") String type)
             throws SQLException, AuthorizeException {
         Context context = ContextUtil.obtainContext(request);
 
         Item item = itemService.find(context, uuid);
-        ItemRest itemRest;
+
 
         if (item == null) {
             throw new ResourceNotFoundException("Could not find item with id " + uuid);
         }
 
+        // Check for a valid identifier type and register the appropriate type of identifier
+        if ("doi".equals(type)) {
+            return registerDOI(context, item);
+        } else {
+            throw new IllegalArgumentException("Valid identifier type (eg. 'doi') is required, parameter name 'type'");
+        }
+    }
+
+    /**
+     * Queue a new, pending or minted DOI for registration for a given item. Requires administrative privilege.
+     * This request is sent from the Register DOI button (configurable) on the item status page.
+     *
+     * @return 302 FOUND if the DOI is already registered or reserved, 201 CREATED if queued for registration
+     */
+    private ResponseEntity<RepresentationModel<?>> registerDOI(Context context, Item item)
+            throws SQLException, AuthorizeException {
         String identifier = null;
         HttpStatus httpStatus = HttpStatus.BAD_REQUEST;
+        ItemRest itemRest;
         try {
             DOIIdentifierProvider doiIdentifierProvider = DSpaceServicesFactory.getInstance().getServiceManager()
                     .getServiceByName("org.dspace.identifier.DOIIdentifierProvider", DOIIdentifierProvider.class);
@@ -148,5 +164,4 @@ public class ItemIdentifierController {
         // Return the status and item resource
         return ControllerUtils.toResponseEntity(httpStatus, new HttpHeaders(), itemResource);
     }
-
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/model/step/DataIdentifiers.java

@@ -19,6 +19,8 @@ public class DataIdentifiers implements SectionData {
     String handle;
     String doi;
     List<String> otherIdentifiers;
+    // Types to display, a hint for te UI
+    List<String> displayTypes;
 
     public DataIdentifiers() {
 
@@ -48,6 +50,14 @@ public class DataIdentifiers implements SectionData {
         this.otherIdentifiers = otherIdentifiers;
     }
 
+    public List<String> getDisplayTypes() {
+        return displayTypes;
+    }
+
+    public void setDisplayTypes(List<String> displayTypes) {
+        this.displayTypes = displayTypes;
+    }
+
     public String toString() {
         StringBuilder sb = new StringBuilder();
         sb.append("Handle: ").append(handle);

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemIdentifierLinkRepository.java

@@ -25,6 +25,7 @@ import org.dspace.content.service.ItemService;
 import org.dspace.core.Context;
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.dspace.identifier.DOI;
+import org.dspace.identifier.DOIIdentifierProvider;
 import org.dspace.identifier.IdentifierException;
 import org.dspace.identifier.service.DOIService;
 import org.dspace.identifier.service.IdentifierService;
@@ -65,7 +66,8 @@ public class ItemIdentifierLinkRepository extends AbstractDSpaceRestRepository i
         try {
             if (doi != null) {
                 String doiUrl = doiService.DOIToExternalForm(doi.getDoi());
-                IdentifierRest identifierRest = new IdentifierRest(doiUrl, "doi", String.valueOf(doi.getStatus()));
+                IdentifierRest identifierRest = new IdentifierRest(
+                        doiUrl, "doi", DOIIdentifierProvider.statusText[doi.getStatus()]);
                 identifierRestList.add(identifierRest);
             }
             if (handle != null) {

dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/step/ShowIdentifiersStep.java

@@ -9,6 +9,7 @@ package org.dspace.app.rest.submit.step;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 
@@ -91,6 +92,11 @@ public class ShowIdentifiersStep extends AbstractProcessingStep {
         IdentifierService identifierService =
                 IdentifierServiceFactory.getInstance().getIdentifierService();
         // Attempt to look up handle and DOI identifiers for this item
+        String[] defaultTypes = {"handle", "doi"};
+        List<String> displayTypes = Arrays.asList(configurationService.getArrayProperty(
+                "identifiers.submission.display",
+                defaultTypes));
+        result.setDisplayTypes(displayTypes);
         String handle = identifierService.lookup(context, obj.getItem(), Handle.class);
         String simpleDoi = identifierService.lookup(context, obj.getItem(), DOI.class);
         DOI doi = null;
@@ -126,7 +132,7 @@ public class ShowIdentifiersStep extends AbstractProcessingStep {
             handle = HandleServiceFactory.getInstance().getHandleService().getCanonicalForm(handle);
         }
 
-        // Populate bean with data and return
+        // Populate bean with data and return, if the identifier type is configured for exposure
         result.setDoi(doiString);
         result.setHandle(handle);
         result.setOtherIdentifiers(otherIdentifiers);

dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemIdentifierControllerIT.java

@@ -71,7 +71,7 @@ public class ItemIdentifierControllerIT extends AbstractControllerIntegrationTes
         // Expect first forbidden
         String token = getAuthToken(eperson.getEmail(), password);
         getClient(token).perform(post("/api/core/items/" +
-                        publicItem1.getID().toString() + "/identifiers"))
+                        publicItem1.getID().toString() + "/identifiers?type=doi"))
                 .andExpect(status().isForbidden());
 
         // Set token to admin credentials
@@ -79,12 +79,12 @@ public class ItemIdentifierControllerIT extends AbstractControllerIntegrationTes
 
         // Expect a successful 201 CREATED for this item with no DOI
         getClient(token).perform(post("/api/core/items/" +
-                        publicItem1.getID().toString() + "/identifiers"))
+                        publicItem1.getID().toString() + "/identifiers?type=doi"))
                 .andExpect(status().isCreated());
 
         // Expected 302 FOUND status code for a DOI already in REGISTERED / TO_BE_REGISTERED state
         getClient(token).perform(post("/api/core/items/" +
-                        publicItem1.getID().toString() + "/identifiers"))
+                        publicItem1.getID().toString() + "/identifiers?type=doi"))
                 .andExpect(status().isFound());
 
         // Get the doi we minted and queued for registration

dspace/config/item-submission.xml

@@ -240,7 +240,9 @@
             <!--<step id="sample"/> -->
 
             <step id="collection"/>
-            <step id="identifiers"/>
+            <!-- To include the 'Show Identifiers' step, uncomment the line below. Make sure
+                 that identifiers.cfg and identifier-service.xml are properly configured. -->
+            <!--<step id="identifiers"/>-->
             <!--Step will be to Describe the item. -->
             <step id="traditionalpageone"/>
             <step id="traditionalpagetwo"/>

dspace/config/modules/identifiers.cfg

@@ -5,7 +5,8 @@
 # as the Show Identifiers step which can "pre-mint" DOIs and Handles   #
 #----------------------------------------------------------------------#
 
-# Should DOIs be minted for (future) registration at workspace item creation?
+# Should configured  identifiers (eg handle and DOI) be minted for (future) registration at workspace item creation?
+# A handle created at this stage will act just like a regular handle created at archive time.
 # A DOI created at this stage will be in a 'PENDING' status while in workspace and workflow.
 # At the time of item install, the DOI filter (if any) will be applied and if the item matches the filter, the DOI
 # status will be updated to TO_BE_REGISTERED. An administrator can also manually progress the DOI status, overriding
@@ -40,4 +41,10 @@
 
 # Show Register DOI button in item status page?
 # Default: false
-#identifiers.item-status.register = true
+#identifiers.item-status.registerDOI = true
+
+# Which identifier types to show in submission step?
+# Default: handle, doi, other (this is all the current supported identifier 'types')
+#identifiers.submission.display = handle
+#identifiers.submission.display = doi
+#identifiers.submission.display = other

dspace/config/modules/rest.cfg

@@ -46,7 +46,7 @@ rest.properties.exposed = google.recaptcha.key.site
 rest.properties.exposed = google.recaptcha.version
 rest.properties.exposed = google.recaptcha.mode
 rest.properties.exposed = cc.license.jurisdiction
-rest.properties.exposed = identifiers.item-status.register
+rest.properties.exposed = identifiers.item-status.register-doi
 rest.properties.exposed = authentication-password.domain.valid
 
 #---------------------------------------------------------------#