Merged w2p-57159_permission-to-create-relations into w2p-58898_place-column-calculation-error

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

@@ -1476,26 +1476,28 @@ prevent the generation of resource policy entry values with null dspace_object a
     }
 
     private RelationshipMetadataValue constructMetadataValue(Context context, String key) {
+        String[] splittedKey = key.split("\\.");
+        RelationshipMetadataValue metadataValue = new RelationshipMetadataValue();
+        String metadataSchema = splittedKey.length > 0 ? splittedKey[0] : null;
+        String metadataElement = splittedKey.length > 1 ? splittedKey[1] : null;
+        String metadataQualifier = splittedKey.length > 2 ? splittedKey[2] : null;
+        MetadataField metadataField = null;
         try {
-            String[] splittedKey = key.split("\\.");
+            metadataField = metadataFieldService
-            RelationshipMetadataValue metadataValue = new RelationshipMetadataValue();
-            String metadataSchema = splittedKey.length > 0 ? splittedKey[0] : null;
-            String metadataElement = splittedKey.length > 1 ? splittedKey[1] : null;
-            String metadataQualifier = splittedKey.length > 2 ? splittedKey[2] : null;
-            MetadataField metadataField = metadataFieldService
                 .findByElement(context, metadataSchema, metadataElement, metadataQualifier);
-            if (metadataField == null) {
-                log.error("A MetadataValue was attempted to construct with MetadataField for parameters: " +
-                              "metadataschema: {}, metadataelement: {}, metadataqualifier: {}",
-                          metadataSchema, metadataElement, metadataQualifier);
-                return null;
-            }
-            metadataValue.setMetadataField(metadataField);
-            metadataValue.setLanguage(Item.ANY);
-            return metadataValue;
         } catch (SQLException e) {
-            log.error(e.getMessage(), e);
+            log.error("Could not find element with MetadataSchema: " + metadataSchema +
+                          ", MetadataElement: " + metadataElement + " and MetadataQualifier: " + metadataQualifier, e);
+            return null;
         }
-        return null;
+        if (metadataField == null) {
+            log.error("A MetadataValue was attempted to construct with MetadataField for parameters: " +
+                          "metadataschema: {}, metadataelement: {}, metadataqualifier: {}",
+                      metadataSchema, metadataElement, metadataQualifier);
+            return null;
+        }
+        metadataValue.setMetadataField(metadataField);
+        metadataValue.setLanguage(Item.ANY);
+        return metadataValue;
     }
 }

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

@@ -22,6 +22,17 @@ public class RelationshipMetadataValue extends MetadataValue {
      */
     private boolean useForPlace;
 
+    /**
+     * This property determines whether this RelationshipMetadataValue should be used in place calculation or not.
+     * This is retrieved from Spring configuration when constructing RelationshipMetadataValues. This Spring
+     * configuration is located in the core-services.xml configuration file.
+     * Putting this property on true will imply that we're now mixing plain-text metadatavalues with the
+     * metadatavalues that are constructed through Relationships with regards to the place attribute.
+     * For example, currently the RelationshipMetadataValue dc.contributor.author that is constructed through a
+     * Relationship for a Publication will have its useForPlace set to true. This means that the place
+     * calculation will take both these RelationshipMetadataValues into account together with the normal
+     * plain text metadatavalues.
+     */
     public boolean isUseForPlace() {
         return useForPlace;
     }

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

@@ -272,15 +272,14 @@ public class RelationshipServiceImpl implements RelationshipService {
 
     public void update(Context context, List<Relationship> relationships) throws SQLException, AuthorizeException {
         if (CollectionUtils.isNotEmpty(relationships)) {
-            // Check authorisation - only administrators can change formats
-            if (!authorizeService.isAdmin(context)) {
-                throw new AuthorizeException(
-                    "Only administrators can modify relationship");
-            }
-
             for (Relationship relationship : relationships) {
-                if (isRelationshipValidToCreate(context, relationship)) {
+                if (authorizeService.authorizeActionBoolean(context, relationship.getLeftItem(), Constants.WRITE) ||
-                    relationshipDAO.save(context, relationship);
+                    authorizeService.authorizeActionBoolean(context, relationship.getRightItem(), Constants.WRITE)) {
+                    if (isRelationshipValidToCreate(context, relationship)) {
+                        relationshipDAO.save(context, relationship);
+                    }
+                } else {
+                    throw new AuthorizeException("You do not have write rights on this relationship's items");
                 }
             }
         }
@@ -288,6 +287,7 @@ public class RelationshipServiceImpl implements RelationshipService {
 
     public void delete(Context context, Relationship relationship) throws SQLException, AuthorizeException {
         if (isRelationshipValidToDelete(context, relationship)) {
+            // To delete a relationship, a user must have WRITE permissions on one of the related Items
             if (authorizeService.authorizeActionBoolean(context, relationship.getLeftItem(), Constants.WRITE) ||
                 authorizeService.authorizeActionBoolean(context, relationship.getRightItem(), Constants.WRITE)) {
                 relationshipDAO.delete(context, relationship);

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

@@ -29,7 +29,15 @@ public class Collected implements VirtualBean {
     private ItemService itemService;
 
     /**
-     * The boolean value indicating whether this field should be used for place or not
+     * This property determines whether this RelationshipMetadataValue should be used in place calculation or not.
+     * This is retrieved from Spring configuration when constructing RelationshipMetadataValues. This Spring
+     * configuration is located in the core-services.xml configuration file.
+     * Putting this property on true will imply that we're now mixing plain-text metadatavalues with the
+     * metadatavalues that are constructed through Relationships with regards to the place attribute.
+     * For example, currently the RelationshipMetadataValue dc.contributor.author that is constructed through a
+     * Relationship for a Publication will have its useForPlace set to true. This means that the place
+     * calculation will take both these RelationshipMetadataValues into account together with the normal
+     * plain text metadatavalues.
      */
     private boolean useForPlace;
     /**

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

@@ -474,11 +474,12 @@ public class RestResourceController implements InitializingBean {
         checkModelPluralForm(apiCategory, model);
         DSpaceRestRepository<RestAddressableModel, ID> repository = utils.getResourceRepository(apiCategory, model);
         RestAddressableModel modelObject = null;
+        List<DSpaceObject> dSpaceObjectList = utils.getdSpaceObjectsFromRequest(request);
         try {
-            List<DSpaceObject> dSpaceObjectList = utils.getdSpaceObjectsFromRequest(request);
             modelObject = repository.createAndReturn(dSpaceObjectList);
-        } catch (ClassCastException | IOException e) {
+        } catch (ClassCastException e) {
-            log.error(e.getMessage(), e);
+            log.error("Something went wrong whilst creating the object for apiCategory: " + apiCategory +
+                          " and model: " + model, e);
             return ControllerUtils.toEmptyResponse(HttpStatus.INTERNAL_SERVER_ERROR);
         }
         if (modelObject == null) {

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

@@ -30,7 +30,6 @@ import org.dspace.content.service.RelationshipService;
 import org.dspace.content.service.RelationshipTypeService;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
-import org.dspace.eperson.EPerson;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
@@ -92,6 +91,7 @@ public class RelationshipRestRepository extends DSpaceRestRepository<Relationshi
         return new RelationshipResource(model, utils, rels);
     }
 
+    @Override
     protected RelationshipRest createAndReturn(Context context, List<DSpaceObject> list)
         throws AuthorizeException, SQLException, RepositoryMethodNotImplementedException {
 
@@ -104,21 +104,15 @@ public class RelationshipRestRepository extends DSpaceRestRepository<Relationshi
             RelationshipType relationshipType = relationshipTypeService
                 .find(context, Integer.parseInt(req.getParameter("relationshipType")));
 
-            EPerson ePerson = context.getCurrentUser();
+            relationship.setLeftItem(leftItem);
-            if (authorizeService.authorizeActionBoolean(context, leftItem, Constants.WRITE) ||
+            relationship.setRightItem(rightItem);
-                authorizeService.authorizeActionBoolean(context, rightItem, Constants.WRITE)) {
+            relationship.setRelationshipType(relationshipType);
-                relationship.setLeftItem(leftItem);
+            try {
-                relationship.setRightItem(rightItem);
-                relationship.setRelationshipType(relationshipType);
                 relationship = relationshipService.create(context, relationship);
-                context.turnOffAuthorisationSystem();
+            } catch (AuthorizeException e) {
-                relationshipService.updateItem(context, relationship.getLeftItem());
-                relationshipService.updateItem(context, relationship.getRightItem());
-                context.restoreAuthSystemState();
-                return relationshipConverter.fromModel(relationship);
-            } else {
                 throw new AccessDeniedException("You do not have write rights on this relationship's items");
             }
+            return relationshipConverter.fromModel(relationship);
         } else {
             throw new UnprocessableEntityException("The given items in the request were not valid items");
         }
@@ -165,10 +159,9 @@ public class RelationshipRestRepository extends DSpaceRestRepository<Relationshi
         try {
             relationship = relationshipService.find(context, id);
             if (relationship != null) {
-                if (authorizeService.authorizeActionBoolean(context, relationship.getLeftItem(), Constants.WRITE) ||
+                try {
-                    authorizeService.authorizeActionBoolean(context, relationship.getRightItem(), Constants.WRITE)) {
                     relationshipService.delete(context, relationship);
-                } else {
+                } catch (AuthorizeException e) {
                     throw new AccessDeniedException("You do not have write rights on this relationship's items");
                 }
             }

dspace-spring-rest/src/main/java/org/dspace/app/rest/utils/Utils.java

@@ -237,12 +237,29 @@ public class Utils {
         }
     }
 
-    private List<DSpaceObject> constructDSpaceObjectList(Context context,
+    /**
-                                                         HttpServletRequest request) throws IOException {
+     * This method will construct a List of DSpaceObjects by executing the method
-        List<String> list = readFromRequest(request);
+     * {@link Utils#readFromRequest(HttpServletRequest)} and fetching the List of Strings from the request.
+     * The method will iterate over this list of Strings and parse the String to retrieve the UUID from it.
+     * It will then look through all the DSpaceObjectServices to try and match this UUID to a DSpaceObject.
+     * If one is found, this DSpaceObject is added to the List of DSpaceObjects that we will return.
+     * @param context   The relevant DSpace context
+     * @param request   The request out of which we'll create the List of DSpaceObjects
+     * @return          The resulting list of DSpaceObjects that we parsed out of the request
+     */
+    private List<DSpaceObject> constructDSpaceObjectList(Context context, HttpServletRequest request) {
+        List<String> list = null;
+        try {
+            list = readFromRequest(request);
+        } catch (IOException e) {
+            log.error("Something went wrong with reading in the inputstream from the request", e);
+        }
 
         List<DSpaceObject> dSpaceObjects = new LinkedList<>();
         for (String string : list) {
+            if (string.endsWith("/")) {
+                string = string.substring(0, string.length() - 1);
+            }
             String uuid = string.substring(string.lastIndexOf('/') + 1);
             try {
                 for (DSpaceObjectService dSpaceObjectService : dSpaceObjectServices) {
@@ -253,13 +270,19 @@ public class Utils {
                     }
                 }
             } catch (SQLException e) {
-                log.error(e.getMessage(), e);
+                log.error("Could not find DSpaceObject for UUID: " + uuid, e);
             }
 
         }
         return dSpaceObjects;
     }
 
+    /**
+     * This method reads lines from the request's InputStream and will add this to a list of Strings.
+     * @param request       The request from which the InputStream will be fetched
+     * @return              A list of String constructed from the request's InputStream
+     * @throws IOException  If something goes wrong
+     */
     private List<String> readFromRequest(HttpServletRequest request) throws IOException {
         List<String> list = new LinkedList<>();
         Scanner scanner = new Scanner(request.getInputStream());
@@ -289,7 +312,7 @@ public class Utils {
      * @return              The list of DSpaceObjects that we could find in the InputStream
      * @throws IOException  If something goes wrong
      */
-    public List<DSpaceObject> getdSpaceObjectsFromRequest(HttpServletRequest request) throws IOException {
+    public List<DSpaceObject> getdSpaceObjectsFromRequest(HttpServletRequest request) {
         return constructDSpaceObjectList(ContextUtil.obtainContext(request), request);
     }
 }