Applied feedback

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

@@ -246,7 +246,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
         for (int i = 0; i < values.size(); i++) {
 
             if (authorities != null && authorities.size() >= i) {
-                if (StringUtils.startsWith(authorities.get(i), "virtual::")) {
+                if (StringUtils.startsWith(authorities.get(i), Constants.VIRTUAL_AUTHORITY_PREFIX)) {
                     continue;
                 }
             }
@@ -561,7 +561,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
             }
             for (MetadataValue metadataValue : metadataValues) {
                 //Retrieve & store the place for each metadata value
-                if (StringUtils.startsWith(metadataValue.getAuthority(), "virtual::") &&
+                if (StringUtils.startsWith(metadataValue.getAuthority(), Constants.VIRTUAL_AUTHORITY_PREFIX) &&
                     ((RelationshipMetadataValue) metadataValue).isUseForPlace()) {
                     int mvPlace = getMetadataValuePlace(fieldToLastPlace, metadataValue);
                     metadataValue.setPlace(mvPlace);
@@ -575,7 +575,8 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
                     }
                     relationshipService.update(context, relationship);
 
-                } else if (!StringUtils.startsWith(metadataValue.getAuthority(), "virtual::")) {
+                } else if (!StringUtils.startsWith(metadataValue.getAuthority(),
+                                                   Constants.VIRTUAL_AUTHORITY_PREFIX)) {
                     int mvPlace = getMetadataValuePlace(fieldToLastPlace, metadataValue);
                     metadataValue.setPlace(mvPlace);
                 }
@@ -588,14 +589,14 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
      *
      * @param fieldToLastPlace the map containing the latest place of each metadata field
      * @param metadataValue    the metadata value that needs to get a place
-     * @return The new place for the metadata valu
+     * @return The new place for the metadata value
      */
     protected int getMetadataValuePlace(Map<MetadataField, Integer> fieldToLastPlace, MetadataValue metadataValue) {
         MetadataField metadataField = metadataValue.getMetadataField();
         if (fieldToLastPlace.containsKey(metadataField)) {
             fieldToLastPlace.put(metadataField, fieldToLastPlace.get(metadataField) + 1);
         } else {
-            // The metadata value place starts at 0q
+            // The metadata value place starts at 0
             fieldToLastPlace.put(metadataField, 0);
         }
         return fieldToLastPlace.get(metadataField);

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

@@ -1398,7 +1398,7 @@ prevent the generation of resource policy entry values with null dspace_object a
         RelationshipMetadataValue metadataValue = constructMetadataValue(context,
                                                                          MetadataSchemaEnum.RELATION
                                                                              .getName() + "." + relationName);
-        metadataValue.setAuthority("virtual::" + relationshipId);
+        metadataValue.setAuthority(Constants.VIRTUAL_AUTHORITY_PREFIX + relationshipId);
         metadataValue.setValue(otherItem.getID().toString());
         return metadataValue;
     }
@@ -1421,7 +1421,7 @@ prevent the generation of resource policy entry values with null dspace_object a
                                                                       RelationshipMetadataValue metadataValue,
                                                                       int relationshipId) {
         metadataValue.setValue(value);
-        metadataValue.setAuthority("virtual::" + relationshipId);
+        metadataValue.setAuthority(Constants.VIRTUAL_AUTHORITY_PREFIX + relationshipId);
         metadataValue.setConfidence(-1);
         metadataValue.setDSpaceObject(item);
         return metadataValue;
@@ -1437,10 +1437,9 @@ prevent the generation of resource policy entry values with null dspace_object a
             MetadataField metadataField = metadataFieldService
                 .findByElement(context, metadataSchema, metadataElement, metadataQualifier);
             if (metadataField == null) {
-                log.error(
+                log.error("A MetadataValue was attempted to construct with MetadataField for parameters: " +
-                    "A MetadataValue was attempted to construct with MetadataField for paremeters: metadataschema: "
+                              "metadataschema: {}, metadataelement: {}, metadataqualifier: {}",
-                        + metadataSchema + ", metadataelement:" + metadataElement +
+                          metadataSchema, metadataElement, metadataQualifier);
-                        ", metadataqualifier: " + metadataQualifier);
                 return null;
             }
             metadataValue.setMetadataField(metadataField);

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

@@ -9,8 +9,17 @@ package org.dspace.content;
 
 import org.hibernate.proxy.HibernateProxyHelper;
 
+/**
+ * This class is used as a representation of MetadataValues for the MetadataValues that are derived from the
+ * Relationships that the item has. This includes the useForPlace property which we'll have to use to determine
+ * whether these Values should be counted for place calculation on both the native MetadataValues and the
+ * Relationship's place attributes.
+ */
 public class RelationshipMetadataValue extends MetadataValue {
 
+    /**
+     * This property determines whether this RelationshipMetadataValue should be used in place calculation or not
+     */
     private boolean useForPlace;
 
     public boolean isUseForPlace() {

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

@@ -66,6 +66,7 @@ public class RelationshipServiceImpl implements RelationshipService {
         }
     }
 
+    @Override
     public void updatePlaceInRelationship(Context context, Relationship relationship) throws SQLException {
         List<Relationship> leftRelationships = findByItemAndRelationshipType(context,
                                                                              relationship.getLeftItem(),

dspace-api/src/main/java/org/dspace/content/service/RelationshipService.java

@@ -88,6 +88,16 @@ public interface RelationshipService extends DSpaceCRUDService<Relationship> {
                                                             RelationshipType relationshipType)
         throws SQLException;
 
+    /**
+     * This method will update the place for the Relationship and all other relationships found by the items and
+     * relationship type of the given Relatonship. It will give this Relationship the last place in both the
+     * left and right place determined by querying for the list of leftRelationships and rightRelationships
+     * by the leftItem, rightItem and relationshipType of the given Relationship.
+     * @param context           The relevant DSpace context
+     * @param relationship      The Relationship object that will have it's place updated and that will be used
+     *                          to retrieve the other relationships whose place might need to be updated
+     * @throws SQLException     If something goes wrong
+     */
     public void updatePlaceInRelationship(Context context, Relationship relationship) throws SQLException;
 
 }

dspace-api/src/main/java/org/dspace/content/virtual/UUIDValue.java

@@ -14,6 +14,10 @@ import java.util.List;
 import org.dspace.content.Item;
 import org.dspace.core.Context;
 
+/**
+ * This class is used by the VirtualMetadataPopulator. It will simply take the ID of the item that's passed along
+ * to this and return that as it's value
+ */
 public class UUIDValue implements VirtualBean {
 
     private boolean useForPlace;

dspace-api/src/main/java/org/dspace/core/Constants.java

@@ -224,6 +224,7 @@ public class Constants {
 
     public static final String DEFAULT_ENCODING = "UTF-8";
 
+    public static final String VIRTUAL_AUTHORITY_PREFIX = "virtual::";
     /**
      * Default constructor
      */

dspace-spring-rest/src/main/java/org/dspace/app/rest/RestResourceController.java

@@ -986,6 +986,25 @@ public class RestResourceController implements InitializingBean {
         return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT);
     }
 
+    /**
+     * Execute a PUT request for an entity with id of type Integer;
+     *
+     * curl -X PUT http://<dspace.url>/dspace-spring-rest/api/{apiCategory}/{model}/{id}
+     *
+     * Example:
+     * <pre>
+     * {@code
+     *      curl -X PUT http://<dspace.url>/dspace-spring-rest/api/core/metadatafield/1
+     * }
+     * </pre>
+     *
+     * @param request     the http request
+     * @param apiCategory the API category e.g. "api"
+     * @param model       the DSpace model e.g. "collection"
+     * @param id        the ID of the target REST object
+     * @param jsonNode    the part of the request body representing the updated rest object
+     * @return the relevant REST resource
+     */
     @RequestMapping(method = RequestMethod.PUT, value = REGEX_REQUESTMAPPING_IDENTIFIER_AS_DIGIT)
     public DSpaceResource<RestAddressableModel> put(HttpServletRequest request,
                                                     @PathVariable String apiCategory, @PathVariable String model,

dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java

@@ -408,6 +408,16 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
         throw new RepositoryMethodNotImplementedException("No implementation found; Method not allowed!", "");
     }
 
+    /**
+     * Method to support updating a DSpace instance.
+     *
+     * @param request     the http request
+     * @param apiCategory the API category e.g. "api"
+     * @param model       the DSpace model e.g. "metadatafield"
+     * @param id        the ID of the target REST object
+     * @param jsonNode    the part of the request body representing the updated rest object
+     * @return the updated REST object
+     */
     public T put(HttpServletRequest request, String apiCategory, String model, ID id, JsonNode jsonNode) {
         Context context = obtainContext();
         try {
@@ -419,6 +429,20 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
         return findOne(id);
     }
 
+    /**
+     * Implement this method in the subclass to support updating a DSpace instance.
+     *
+     * @param context     the dspace context
+     * @param apiCategory the API category e.g. "api"
+     * @param model       the DSpace model e.g. "metadatafield"
+     * @param id          the ID of the target REST object
+     * @param jsonNode    the part of the request body representing the updated rest object
+     * @return the updated REST object
+     * @throws AuthorizeException if the context user is not authorized to perform this operation
+     * @throws SQLException when the database returns an error
+     * @throws RepositoryMethodNotImplementedException
+     *             returned by the default implementation when the operation is not supported for the entity
+     */
     protected T put(Context context, HttpServletRequest request, String apiCategory, String model, ID id,
                     JsonNode jsonNode)
         throws RepositoryMethodNotImplementedException, SQLException, AuthorizeException {

dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/RelationshipRestRepository.java

@@ -187,7 +187,7 @@ public class RelationshipRestRepository extends DSpaceRestRepository<Relationshi
                 }
             }
         } catch (SQLException e) {
-            log.error(e.getMessage(), e);
+            log.error("Error deleting Relationship specified by ID:" + id, e);
         }
     }
 }