code review changes

rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/core/v0/APIUtils.java

@@ -26,12 +26,18 @@
  */
 package org.alfresco.rest.core.v0;
 
+import javax.json.Json;
+import javax.json.JsonReader;
 import java.io.IOException;
+import java.io.InputStream;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
+import org.apache.http.ParseException;
+import org.json.JSONException;
 import org.json.JSONObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -76,4 +82,40 @@ public class APIUtils
         LOGGER.info("Response body:\n{}", source);
         return new JSONObject(source);
     }
+
+    /**
+     * Util method to extract the message string from the HTTP response
+     *
+     * @param httpResponse http response
+     * @return error message from the http response
+     */
+    public static String extractErrorMessageFromHttpResponse(HttpResponse httpResponse)
+    {
+        final HttpEntity entity = httpResponse.getEntity();
+        JsonReader reader = null;
+        try
+        {
+            final InputStream responseStream = entity.getContent();
+            reader = Json.createReader(responseStream);
+            return reader.readObject().getString("message");
+        }
+        catch (JSONException error)
+        {
+
+            LOGGER.error("Converting message body to JSON failed. Body: {}", httpResponse, error);
+        }
+        catch (ParseException | IOException error)
+        {
+
+            LOGGER.error("Parsing message body failed.", error);
+        }
+        finally
+        {
+            if (reader != null)
+            {
+                reader.close();
+            }
+        }
+        return null;
+    }
 }

rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/HoldsAPI.java

@@ -30,21 +30,17 @@ import static org.alfresco.rest.core.v0.APIUtils.convertHTTPResponseToJSON;
 import static org.apache.http.HttpStatus.SC_OK;
 import static org.testng.AssertJUnit.assertNotNull;
 
-import javax.json.Json;
-import javax.json.JsonReader;
-import java.io.IOException;
-import java.io.InputStream;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import org.alfresco.rest.core.v0.APIUtils;
 import org.alfresco.rest.core.v0.BaseAPI;
 import org.alfresco.rest.rm.community.model.hold.HoldEntry;
 import org.alfresco.rest.rm.community.util.PojoUtility;
 import org.alfresco.utility.Utility;
 import org.apache.chemistry.opencmis.client.api.CmisObject;
-import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.ParseException;
 import org.json.JSONArray;
@@ -67,7 +63,9 @@ public class HoldsAPI extends BaseAPI
      */
     private static final String CREATE_HOLDS_API = "{0}type/rma:hold/formprocessor";
 
-    /** The URI to add items to hold.*/
+    /**
+     * The URI to add items to hold or to remove items from hold
+     */
     private static final String RM_HOLDS_API = "{0}rma/holds";
     /**
      * The URI to  get holds.
@@ -77,7 +75,7 @@ public class HoldsAPI extends BaseAPI
     /**
      * Util method to create a hold
      *
-     * @param user        the user creating the category
+     * @param user        the user creating the hold
      * @param password    the user's password
      * @param holdName    the hold name
      * @param reason      hold reason
@@ -111,13 +109,13 @@ public class HoldsAPI extends BaseAPI
     }
 
     /**
-     *  Create a hold and get the node ref of the hold from the response body
+     * Create a hold and get the node ref of the hold from the response body
      *
-     * @param user
+     * @param user        the user creating the hold
-     * @param password
+     * @param password    the user's password
-     * @param holdName
+     * @param holdName    the hold name to be created
-     * @param reason
+     * @param reason      reason of the hold to be created
-     * @param description
+     * @param description hold description
      * @return node ref of the hold created
      */
 
@@ -159,7 +157,7 @@ public class HoldsAPI extends BaseAPI
     }
 
     /**
-     * Adds item (active content /record/ record folder) to the hold
+     * Adds item (content /record/ record folder) to the hold
      *
      * @param user        the user who adds the item to the hold
      * @param password    the user's password
@@ -173,7 +171,7 @@ public class HoldsAPI extends BaseAPI
     }
 
     /**
-     * Adds item (record/ record folder) to the hold
+     * Adds item (content /record/ record folder) to the hold
      *
      * @param user        the user who adds the item to the hold
      * @param password    the user's password
@@ -184,12 +182,12 @@ public class HoldsAPI extends BaseAPI
     public HttpResponse addItemToHold(String user, String password, int expectedStatus, String itemNodeRef,
                                       String holdName)
     {
-        final JSONObject requestParams = addToHoldJsonObject(user, password, itemNodeRef, holdName);
+        final JSONObject requestParams = addOrRemoveToFromHoldJsonObject(user, password, itemNodeRef, holdName);
         return doPostJsonRequest(user, password, expectedStatus, requestParams, RM_HOLDS_API);
     }
 
     /**
-     * Util method to add item (active content /record/ record folder) to the hold and get the error message
+     * Util method to add item ( content /record/ record folder) to the hold and get the error message
      *
      * @param user        the user who adds the item to the hold
      * @param password    the user's password
@@ -201,18 +199,18 @@ public class HoldsAPI extends BaseAPI
             holdName)
     {
         final HttpResponse httpResponse = addItemToHold(user, password, expectedStatus, itemNodeRef, holdName);
-        return extractErrorMessageFromHttpResponse(httpResponse);
+        return APIUtils.extractErrorMessageFromHttpResponse(httpResponse);
     }
 
     /**
-     * Util method to create the request body for adding an item to hold
+     * Util method to create the request body used adding an item to hold or to remove items from hold
-     * @param user
+     * @param user user to create the request body for add/remove an item to/from hold
-     * @param password
+     * @param password  the user's password
-     * @param itemNodeRef
+     * @param itemNodeRef node ref to be added to hold
-     * @param holdName
+     * @param holdName hold names for add/remove item
      * @return JSONObject fo
      */
-    private JSONObject addToHoldJsonObject(String user, String password, String itemNodeRef, String holdName)
+    private JSONObject addOrRemoveToFromHoldJsonObject(String user, String password, String itemNodeRef, String holdName)
     {
 
         final JSONArray nodeRefs = new JSONArray().put(getNodeRefSpacesStore() + itemNodeRef);
@@ -230,9 +228,9 @@ public class HoldsAPI extends BaseAPI
     }
 
     /**
-     * Remove item (active content /record/ record folder) from the hold
+     * Remove item ( content /record/ record folder) from the hold
      *
-     * @param user        the user who adds the item to the hold
+     * @param user        the user who removes the item from the hold
      * @param password    the user's password
      * @param itemNodeRef the nodeRef of the item to be added to hold
      * @param holdName    the hold name
@@ -244,7 +242,7 @@ public class HoldsAPI extends BaseAPI
     }
 
     /**
-     * Remove item (record/ record folder) to the hold
+     * Remove item (content/ record/ record folder) to the hold
      *
      * @param user        the user who adds the item to the hold
      * @param password    the user's password
@@ -256,14 +254,14 @@ public class HoldsAPI extends BaseAPI
     public HttpResponse removeItemFromHold(String user, String password, int expectedStatus, String itemNodeRef, String
             holdName)
     {
-        final JSONObject requestParams = addToHoldJsonObject(user, password, itemNodeRef, holdName);
+        final JSONObject requestParams = addOrRemoveToFromHoldJsonObject(user, password, itemNodeRef, holdName);
         return doPutJsonRequest(user, password, expectedStatus, requestParams, RM_HOLDS_API);
     }
 
     /**
-     * Util method to remove item (active content /record/ record folder) from hold and get the error message
+     * Util method to remove item (content /record/ record folder) from hold and get the error message
      *
-     * @param user        the user who adds the item to the hold
+     * @param user        the user who removes the item from hold
      * @param password    the user's password
      * @param itemNodeRef the nodeRef of the item to be added to hold
      * @param holdName    the hold name
@@ -273,40 +271,7 @@ public class HoldsAPI extends BaseAPI
             holdName)
     {
         final HttpResponse httpResponse = removeItemFromHold(user, password, expectedStatus, itemNodeRef, holdName);
-        return extractErrorMessageFromHttpResponse(httpResponse);
+        return APIUtils.extractErrorMessageFromHttpResponse(httpResponse);
-    }
-
-    /**
-     * Util method to extract the message string from the HTTP response
-     * @param httpResponse
-     * @return
-     */
-    private String extractErrorMessageFromHttpResponse(HttpResponse httpResponse)
-    {
-        final HttpEntity entity = httpResponse.getEntity();
-        JsonReader reader = null;
-        try
-        {
-            final InputStream responseStream = entity.getContent();
-            reader = Json.createReader(responseStream);
-            return reader.readObject().getString("message");
-        }
-        catch (JSONException error)
-        {
-            LOGGER.error("Converting message body to JSON failed. Body: {}", httpResponse, error);
-        }
-        catch (ParseException | IOException error)
-        {
-            LOGGER.error("Parsing message body failed.", error);
-        }
-        finally
-        {
-            if (reader != null)
-            {
-                reader.close();
-            }
-        }
-        return null;
     }
 
     /**

rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/RoleService.java

@@ -155,8 +155,8 @@ public class RoleService
      * Helper method to create a user with rm role and permissions on the node ref
      *
      * @param userRole       the rm role
-     * @param userPermission the permissions over the record category
+     * @param userPermission the permissions over the rm node
-     * @param componentId the component id to grant rm permission
+     * @param componentId the node id to grant rm permission
      * @return the created user model
      */
     public UserModel createUserWithRMRoleAndRMNodePermission(String userRole, String componentId,
@@ -167,6 +167,7 @@ public class RoleService
         getRestAPIFactory().getRmRestWrapper().assertStatusCodeIs(OK);
         return rmUser;
     }
+
     /**
      * Helper method to create a  user with rm role and permissions over the recordCategory and collaborator role
      * in collaboration site
@@ -184,25 +185,24 @@ public class RoleService
                                                         userRole, userPermission);
     }
 
-
     /**
      * Helper method to create a test user with a rm role and permissions over a rm component and a role
      * in collaboration site
      *
      * @param siteModel      collaboration site
-     * @param userSiteRoles  user role in the collaboration site
+     * @param userSiteRole   user role in the collaboration site
      * @param rmNodeId       rm node id to grant rm permission
      * @param userRole       the rm role
      * @param userPermission the permissions over the rmNodeId
      * @return the created user model
      */
-    public UserModel createUserWithSiteRoleRMRoleAndPermission(SiteModel siteModel, UserRole userSiteRoles,
+    public UserModel createUserWithSiteRoleRMRoleAndPermission(SiteModel siteModel, UserRole userSiteRole,
                                                                String rmNodeId, UserRoles userRole,
                                                                UserPermissions userPermission)
     {
         final UserModel rmUser = createUserWithRMRoleAndRMNodePermission(userRole.roleId, rmNodeId,
                 userPermission);
-        getDataUser().addUserToSite(rmUser, siteModel, userSiteRoles);
+        getDataUser().addUserToSite(rmUser, siteModel, userSiteRole);
         return rmUser;
     }
 }

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/base/BaseRMRestTest.java

@@ -39,6 +39,7 @@ import static org.alfresco.rest.rm.community.model.fileplancomponents.FilePlanCo
 import static org.alfresco.rest.rm.community.model.fileplancomponents.FilePlanComponentType.RECORD_TYPE;
 import static org.alfresco.rest.rm.community.model.fileplancomponents.FilePlanComponentType.UNFILED_CONTAINER_TYPE;
 import static org.alfresco.rest.rm.community.model.fileplancomponents.FilePlanComponentType.UNFILED_RECORD_FOLDER_TYPE;
+import static org.alfresco.rest.rm.community.utils.CoreUtil.toFileModel;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createRecordCategoryChildModel;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createRecordCategoryModel;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createTempFile;
@@ -823,7 +824,7 @@ public class BaseRMRestTest extends RestTest
      */
     protected boolean hasRecordAspect(FileModel testFile) throws Exception
     {
-        return hasAspect(testFile,RECORD_TYPE);
+        return hasAspect(testFile, RECORD_TYPE);
     }
 
     /**
@@ -839,6 +840,18 @@ public class BaseRMRestTest extends RestTest
                                   .getAspectNames().contains(aspectName);
     }
 
+    /**
+     * Checks if the given node has the given aspect
+     *
+     * @param nodeId   the node to be checked
+     * @param aspectName the matching aspect
+     * @return true if the file has the aspect, false otherwise
+     */
+    protected boolean hasAspect(String nodeId, String aspectName) throws Exception
+    {
+        return hasAspect(toFileModel(nodeId),aspectName);
+    }
+
     /**
      * Helper method to verify if the declared record is in Unfiled Records location
      *

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/hold/AddToHoldsTests.java

@@ -56,6 +56,7 @@ import java.util.List;
 import java.util.stream.Collectors;
 
 import org.alfresco.dataprep.CMISUtil;
+import org.alfresco.dataprep.ContentActions;
 import org.alfresco.rest.model.RestNodeModel;
 import org.alfresco.rest.rm.community.base.BaseRMRestTest;
 import org.alfresco.rest.rm.community.model.hold.HoldEntry;
@@ -78,20 +79,20 @@ import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 /**
- * API tests for adding content to holds
+ * API tests for adding content/record folder/records to holds
  *
  * @author Rodica Sutu
  * @since 3.2
  */
 @AlfrescoTest (jira = "RM-6874")
-public class AddContentToHoldsTests extends BaseRMRestTest
+public class AddToHoldsTests extends BaseRMRestTest
 {
-    private static final String HOLD = "HOLD" + generateTestPrefix(AddContentToHoldsTests.class);
+    private static final String HOLD = "HOLD" + generateTestPrefix(AddToHoldsTests.class);
     private static final String ACCESS_DENIED_ERROR_MESSAGE = "Access Denied.  You do not have the appropriate " +
             "permissions to perform this operation.";
     private static final String INVALID_TYPE_ERROR_MESSAGE = "Items added to a hold must be either a record, a " +
             "record folder or active content.";
-
+    private static final String LOCKED_FILE_ERROR_MESSAGE = "Locked nodes can't be added to hold";
     private SiteModel testSite;
     private String holdNodeRef;
     private FileModel documentHeld, contentToAddToHold, contentAddToHoldNoPermission;
@@ -103,6 +104,8 @@ public class AddContentToHoldsTests extends BaseRMRestTest
     private HoldsAPI holdsAPI;
     @Autowired
     private RoleService roleService;
+    @Autowired
+    private ContentActions contentActions;
 
     @BeforeClass (alwaysRun = true)
     public void preconditionForAddContentToHold() throws Exception
@@ -148,7 +151,7 @@ public class AddContentToHoldsTests extends BaseRMRestTest
                                                               .getName()))
                                                       .collect(Collectors.toList());
         STEP("Check the list of active content");
-        assertEquals(documentsHeld.size(), 1, "The active content is not retrive when getting the children from the " +
+        assertEquals(documentsHeld.size(), 1, "The active content is not retrieve when getting the children from the " +
                 "hold folder");
         assertEquals(documentsHeld.get(0).onModel().getName(), documentHeld.getName());
     }
@@ -204,27 +207,25 @@ public class AddContentToHoldsTests extends BaseRMRestTest
     }
 
     /**
-     * Given active content not on hold
+     * Given record folder/record/document not on hold
      * And a hold
      * And file permission on the hold
      * And the appropriate capability to add to hold
-     * When I use the existing REST API to add the active content to the hold
+     * When I use the existing REST API to add the node to the hold
-     * Then the active content is added to the hold
+     * Then the record folder/record/document is added to the hold
-     * And the active content is frozen
+     * And the item is frozen
      *
      * @throws Exception
      */
     @Test (dataProvider = "validNodesForAddToHold")
     public void addValidNodesToHoldWithAllowedUser(String nodeId) throws Exception
     {
-        STEP("Add the content to the hold with user with permission.");
+        STEP("Add node to hold with user with permission.");
         holdsAPI.addItemToHold(userAddHoldPermission.getUsername(), userAddHoldPermission.getPassword(),
                 nodeId, HOLD);
 
-        STEP("Check active content is frozen.");
+        STEP("Check the node is frozen.");
-        RestNodeModel heldActiveContent = restClient.authenticateUser(userAddHoldPermission)
+        assertTrue(hasAspect(nodeId, FROZEN_ASPECT));
-                                                    .withCoreAPI().usingNode(toContentModel(nodeId)).getNode();
-        assertTrue(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
 
     }
 
@@ -273,12 +274,12 @@ public class AddContentToHoldsTests extends BaseRMRestTest
     }
 
     /**
-     * Given active content not on hold
+     * Given a node not on hold
      * And a hold
      * And user without right permission to add to hold
-     * When I use the existing REST API to add the active content to the hold
+     * When I use the existing REST API to add the node to the hold
-     * Then the active content is not added to the hold
+     * Then the node is not added to the hold
-     * And the active content is frozen
+     * And the node is not frozen
      *
      * @throws Exception
      */
@@ -287,68 +288,74 @@ public class AddContentToHoldsTests extends BaseRMRestTest
                                                                                                                Exception
     {
         users.add(userModel);
-        STEP("Add the content to the hold with user with permission.");
+        STEP("Add the node to the hold with user without permission.");
-        String response = holdsAPI.addToHoldAndGetMessage(userModel.getUsername(),
+        String response = holdsAPI.addToHoldAndGetMessage(userModel.getUsername(), userModel.getPassword(),
-                userModel.getPassword(), SC_INTERNAL_SERVER_ERROR, nodeToBeAddedToHold, HOLD);
+                SC_INTERNAL_SERVER_ERROR, nodeToBeAddedToHold, HOLD);
         assertTrue(response.contains(ACCESS_DENIED_ERROR_MESSAGE));
 
-        STEP("Check active content is not frozen.");
+        STEP("Check the node is not frozen.");
-        RestNodeModel heldActiveContent = restClient.authenticateUser(getAdminUser())
+        assertFalse(hasAspect(nodeToBeAddedToHold,FROZEN_ASPECT));
-                                                    .withCoreAPI().usingNode(toContentModel(nodeToBeAddedToHold))
-                                                    .getNode();
-        assertFalse(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
 
     }
 
 
-
     /**
-     * Data provider with invalid item types that can be added to a hold
+     * Data provider with invalid node types that can be added to a hold
      */
     @DataProvider (name = "invalidNodesForAddToHold")
     public Object[][] getInvalidNodesForAddToHold() throws Exception
     {
+        //create locked file
+        FileModel contentLocked = dataContent.usingAdmin().usingSite(testSite)
+                                            .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
+
+        contentActions.checkOut(getAdminUser().getUsername(), getAdminUser().getPassword(),
+                testSite.getId(), contentLocked.getName());
         RecordCategory category = createRootCategory(getRandomAlphanumeric());
         nodesToBeClean.add(category.getId());
         return new Object[][]
                 {       // file plan node id
-                        { getFilePlan(FILE_PLAN_ALIAS).getId(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE  },
+                        { getFilePlan(FILE_PLAN_ALIAS).getId(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE },
                         //transfer container
                         { getTransferContainer(TRANSFERS_ALIAS).getId(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE },
                         // a record category
                         { category.getId(), SC_INTERNAL_SERVER_ERROR, ACCESS_DENIED_ERROR_MESSAGE },
                         // unfiled records root
-                        { getUnfiledContainer(UNFILED_RECORDS_CONTAINER_ALIAS).getId(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE },
+                        { getUnfiledContainer(UNFILED_RECORDS_CONTAINER_ALIAS).getId(), SC_BAD_REQUEST,
+                                INVALID_TYPE_ERROR_MESSAGE },
                         // an arbitrary unfiled records folder
                         { createUnfiledContainerChild(UNFILED_RECORDS_CONTAINER_ALIAS, "Unfiled Folder " +
-                                getRandomAlphanumeric(), UNFILED_RECORD_FOLDER_TYPE).getId(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE },
+                                getRandomAlphanumeric(), UNFILED_RECORD_FOLDER_TYPE).getId(), SC_BAD_REQUEST,
+                                INVALID_TYPE_ERROR_MESSAGE },
                         //folder,
-                        { dataContent.usingAdmin().usingSite(testSite).createFolder().getNodeRef(), SC_BAD_REQUEST, INVALID_TYPE_ERROR_MESSAGE }
+                        { dataContent.usingAdmin().usingSite(testSite).createFolder().getNodeRef(), SC_BAD_REQUEST,
+                                INVALID_TYPE_ERROR_MESSAGE },
+                        //document locked
+                        { contentLocked.getNodeRefWithoutVersion(), SC_INTERNAL_SERVER_ERROR, LOCKED_FILE_ERROR_MESSAGE }
                 };
     }
 
     /**
-     * unfiled container
+     * Given a node that is not a document/record/ record folder ( a valid node type to be added to hold)
-     * unfiled folder
+     * And a hold
-     * folder
+     * And user without right permission to add to hold
-     * category
+     * When I use the existing REST API to add the node to the hold
-     * fileplan
+     * Then the node is not added to the hold
+     * And the node is not frozen
      *
      * @throws Exception
      */
     @Test (dataProvider = "invalidNodesForAddToHold")
     public void addInvalidNodesToHold(String itemNodeRef, int responseCode, String errorMessage) throws Exception
     {
-        STEP("Add the item to the hold ");
+        STEP("Add the node to the hold ");
-        String responseErrorMessage = holdsAPI.addToHoldAndGetMessage(userAddHoldPermission.getUsername(),
+        String responseErrorMessage = holdsAPI.addToHoldAndGetMessage(getAdminUser().getUsername(),
-                userAddHoldPermission.getPassword(), responseCode, itemNodeRef, HOLD);
+                getAdminUser().getPassword(), responseCode, itemNodeRef, HOLD);
         assertTrue(responseErrorMessage.contains(errorMessage),
-                "Actual error message " + responseErrorMessage + " expected " + responseErrorMessage);
+                "Actual error message " + responseErrorMessage + " expected " + errorMessage);
 
-        STEP("Check active content is not frozen.");
+        STEP("Check node is not frozen.");
-        RestNodeModel heldActiveContent = restClient.authenticateUser(getAdminUser())
+        assertFalse(hasAspect(itemNodeRef, FROZEN_ASPECT));
-                                                    .withCoreAPI().usingNode(toContentModel(itemNodeRef)).getNode();
-        assertFalse(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
     }
 
     @AfterClass (alwaysRun = true)

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/hold/PreventActionsOnFrozenContentTests.java

@@ -31,6 +31,7 @@ import static org.alfresco.rest.rm.community.base.TestData.HOLD_REASON;
 import static org.alfresco.rest.rm.community.util.CommonTestUtils.generateTestPrefix;
 import static org.alfresco.rest.rm.community.utils.CoreUtil.createBodyForMoveCopy;
 import static org.alfresco.utility.report.log.Step.STEP;
+import static org.springframework.http.HttpStatus.FORBIDDEN;
 
 import javax.json.Json;
 import javax.json.JsonObject;
@@ -40,11 +41,11 @@ import org.alfresco.dataprep.CMISUtil;
 import org.alfresco.rest.core.JsonBodyGenerator;
 import org.alfresco.rest.rm.community.base.BaseRMRestTest;
 import org.alfresco.rest.v0.HoldsAPI;
+import org.alfresco.test.AlfrescoTest;
 import org.alfresco.utility.Utility;
 import org.alfresco.utility.model.FileModel;
 import org.alfresco.utility.model.FolderModel;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.HttpStatus;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
@@ -55,6 +56,7 @@ import org.testng.annotations.Test;
  * @author Rodica Sutu
  * @since 3.2
  */
+@AlfrescoTest (jira = "RM-6903")
 public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
 {
     private static final String HOLD_ONE = "HOLD" + generateTestPrefix(PreventActionsOnFrozenContentTests.class);
@@ -73,7 +75,7 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
         holdNodeRef = holdsAPI.createHoldAndGetNodeRef(getAdminUser().getUsername(), getAdminUser().getUsername(),
                 HOLD_ONE, HOLD_REASON, HOLD_DESCRIPTION);
 
-        STEP("Create a test file and add to hold");
+        STEP("Create a test file.");
         testSite = dataSite.usingAdmin().createPublicRandomSite();
         contentHeld = dataContent.usingAdmin().usingSite(testSite)
                                  .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
@@ -100,11 +102,11 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
     public void editPropertiesForContentHeld() throws Exception
     {
         STEP("Update name property of the held content");
-        JsonObject nameUpdated = Json.createObjectBuilder().add("name","HeldNameUpdated").build();
+        JsonObject nameUpdated = Json.createObjectBuilder().add("name", "HeldNameUpdated").build();
         restClient.authenticateUser(getAdminUser()).withCoreAPI().usingNode(contentHeld).updateNode(nameUpdated.toString());
 
         STEP("Check the request failed.");
-        restClient.assertStatusCodeIs(HttpStatus.FORBIDDEN);
+        restClient.assertStatusCodeIs(FORBIDDEN);
         restClient.assertLastError().containsSummary("Frozen nodes can not be updated.");
 
     }
@@ -115,13 +117,14 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
      * Then I am not successful
      */
     @Test
+    @AlfrescoTest (jira = "RM-6925")
     public void updateContentForFrozenFile() throws Exception
     {
         STEP("Update content of the held file");
         restClient.authenticateUser(getAdminUser()).withCoreAPI().usingNode(contentHeld).updateNodeContent(updatedFile);
 
         STEP("Check the request failed.");
-        restClient.assertStatusCodeIs(HttpStatus.INTERNAL_SERVER_ERROR);
+        restClient.assertStatusCodeIs(FORBIDDEN);
         restClient.assertLastError().containsSummary("Frozen nodes can not be updated.");
 
     }
@@ -137,7 +140,7 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
         restClient.authenticateUser(getAdminUser()).withCoreAPI().usingNode(contentHeld).deleteNode(contentHeld.getNodeRefWithoutVersion());
 
         STEP("Check the request failed.");
-        restClient.assertStatusCodeIs(HttpStatus.FORBIDDEN);
+        restClient.assertStatusCodeIs(FORBIDDEN);
         restClient.assertLastError().containsSummary("Frozen nodes can not be deleted.");
 
     }
@@ -148,6 +151,7 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
      * Then I am not successful
      */
     @Test
+    @AlfrescoTest(jira = "RM-6924")
     public void copyFrozenFile() throws Exception
     {
         STEP("Copy frozen file");
@@ -155,8 +159,8 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
         getRestAPIFactory().getNodeAPI(contentHeld).copyNode(postBody);
 
         STEP("Check the request failed.");
-        assertStatusCode(HttpStatus.FORBIDDEN);
+        assertStatusCode(FORBIDDEN);
-        restClient.assertLastError().containsSummary("Frozen nodes can not be copied.");
+        getRestAPIFactory().getRmRestWrapper().assertLastError().containsSummary("Frozen nodes can not be copied.");
     }
 
     /**
@@ -172,8 +176,8 @@ public class PreventActionsOnFrozenContentTests extends BaseRMRestTest
         getRestAPIFactory().getNodeAPI(contentHeld).move(createBodyForMoveCopy(folderModel.getNodeRef()));
 
         STEP("Check the request failed.");
-        restClient.assertStatusCodeIs(HttpStatus.FORBIDDEN);
+        assertStatusCode(FORBIDDEN);
-        restClient.assertLastError().containsSummary("Frozen nodes can not be updated.");
+        getRestAPIFactory().getRmRestWrapper().assertLastError().containsSummary("Frozen nodes can not be moved.");
     }
 
 

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/hold/RemoveFromHoldsTests.java

@@ -33,7 +33,6 @@ import static org.alfresco.rest.rm.community.model.user.UserPermissions.PERMISSI
 import static org.alfresco.rest.rm.community.model.user.UserPermissions.PERMISSION_READ_RECORDS;
 import static org.alfresco.rest.rm.community.model.user.UserRoles.ROLE_RM_MANAGER;
 import static org.alfresco.rest.rm.community.util.CommonTestUtils.generateTestPrefix;
-import static org.alfresco.rest.rm.community.utils.CoreUtil.toContentModel;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.IMAGE_FILE;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createElectronicRecordModel;
 import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicRecordModel;
@@ -48,7 +47,6 @@ import java.util.Arrays;
 import java.util.List;
 
 import org.alfresco.dataprep.CMISUtil;
-import org.alfresco.rest.model.RestNodeModel;
 import org.alfresco.rest.rm.community.base.BaseRMRestTest;
 import org.alfresco.rest.rm.community.model.hold.HoldEntry;
 import org.alfresco.rest.rm.community.model.record.Record;
@@ -68,16 +66,16 @@ import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 /**
- * API tests for removing content from holds
+ * API tests for removing content/record folder/record from holds
  *
  * @author Rodica Sutu
  * @since 3.2
  */
 @AlfrescoTest (jira = "RM-6874, RM-6873")
-public class RemoveContentFromHoldsTests extends BaseRMRestTest
+public class RemoveFromHoldsTests extends BaseRMRestTest
 {
-    private static final String HOLD_ONE = "HOLD_ONE" + generateTestPrefix(RemoveContentFromHoldsTests.class);
+    private static final String HOLD_ONE = "HOLD_ONE" + generateTestPrefix(RemoveFromHoldsTests.class);
-    private static final String HOLD_TWO = "HOLD_TWO" + generateTestPrefix(RemoveContentFromHoldsTests.class);
+    private static final String HOLD_TWO = "HOLD_TWO" + generateTestPrefix(RemoveFromHoldsTests.class);
     private static final String ACCESS_DENIED_ERROR_MESSAGE = "Access Denied.  You do not have the appropriate " +
             "permissions to perform this operation.";
 
@@ -99,14 +97,14 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
         String holdNodeRefTwo = holdsAPI.createHoldAndGetNodeRef(getAdminUser().getUsername(), getAdminUser()
                         .getUsername(), HOLD_TWO, HOLD_REASON, HOLD_DESCRIPTION);
 
-        STEP("Create test files and add them to hold");
+        STEP("Create test files.");
         testSite = dataSite.usingAdmin().createPublicRandomSite();
         contentHeld = dataContent.usingSite(testSite)
                               .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
         contentAddToManyHolds = dataContent.usingSite(testSite)
                                                   .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
 
-        STEP("Add the content to the hold.");
+        STEP("Add the content to the holds.");
         holdsAPI.addItemToHold(getAdminUser().getUsername(), getAdminUser().getPassword(), contentHeld
                 .getNodeRefWithoutVersion(), HOLD_ONE);
         holdsAPI.addItemToHold(getAdminUser().getUsername(), getAdminUser().getPassword(), contentAddToManyHolds
@@ -148,21 +146,20 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
     }
 
     /**
-     * Given active content that is held
+     * Given content/record folder/record that is held
      * And the corresponding hold
-     * When I use the existing REST API to remove the active content from the hold
+     * When I use the existing REST API to remove the node from the hold
-     * Then the active content is removed from the hold
+     * Then the node is removed from the hold
      * And is no longer frozen
      */
     @Test(dataProvider = "validNodesToRemoveFromHold")
     public void removeContentFromHold(String nodeId) throws Exception
     {
-        STEP("Remove content from hold");
+        STEP("Remove node from hold");
         holdsAPI.removeItemFromHold(getAdminUser().getUsername(), getAdminUser().getPassword(), nodeId, HOLD_ONE);
-        STEP("Check the content is not held");
+
-        RestNodeModel heldActiveContent = restClient.authenticateUser(getAdminUser())
+        STEP("Check the node is not held");
-                                                    .withCoreAPI().usingNode(toContentModel(nodeId)).getNode();
+        assertFalse(hasAspect(nodeId, FROZEN_ASPECT));
-        assertFalse(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
 
         STEP("Check node is not in any hold");
         List<HoldEntry> holdEntries = holdsAPI.getHolds(getAdminUser().getUsername(), getAdminUser().getPassword(),
@@ -180,16 +177,14 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
     @Test
     public void removeContentAddedToManyHolds() throws Exception
     {
-        STEP("Remove content from hold");
+        STEP("Remove content from hold. ");
         holdsAPI.removeItemFromHold(getAdminUser().getUsername(), getAdminUser().getPassword(), contentAddToManyHolds
                 .getNodeRefWithoutVersion(), HOLD_ONE);
 
-        STEP("Check the content is held");
+        STEP("Check the content is held. ");
-        RestNodeModel heldActiveContent = restClient.authenticateUser(getAdminUser())
+        assertTrue(hasAspect(contentAddToManyHolds.getNodeRefWithoutVersion(), FROZEN_ASPECT));
-                                                    .withCoreAPI().usingNode(contentAddToManyHolds).getNode();
-        assertTrue(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
 
-        STEP("Check node is not in any hold");
+        STEP("Check node is in hold HOLD_TWO. ");
         List<HoldEntry> holdEntries = holdsAPI.getHolds(getAdminUser().getUsername(), getAdminUser().getPassword(),
                 contentAddToManyHolds.getNodeRefWithoutVersion(), true, null);
         assertFalse(holdEntries.isEmpty(), "Content held is not held after removing from one hold.");
@@ -198,7 +193,7 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
     }
 
     /**
-     *
+     * Data provider with user without right permission or capability to remove from hold a specific node
      * @return
      * @throws Exception
      */
@@ -241,7 +236,7 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
                         {
                                 roleService.createUserWithSiteRoleRMRoleAndPermission(testSite, UserRole
                                                 .SiteCollaborator,
-                                        holdNodeRefOne, UserRoles.ROLE_RM_POWER_USER, PERMISSION_READ_RECORDS),
+                                        holdNodeRefOne, UserRoles.ROLE_RM_POWER_USER, PERMISSION_FILING),
                                 contentNoHoldCap.getNodeRefWithoutVersion()
                         },
                         //user without write permission on RM  record folder
@@ -252,25 +247,22 @@ public class RemoveContentFromHoldsTests extends BaseRMRestTest
                 };
     }
     /**
-     * Given active content on hold in a single hold location
+     * Given node on hold in a single hold location
-     * And the user does not have sufficient permissions or capabilities to remove the active content from the hold
+     * And the user does not have sufficient permissions or capabilities to remove the node from the hold
-     * When the user tries to remove the active content from the hold
+     * When the user tries to remove the node from the hold
-     * Then they are unsuccessful
+     * Then it's unsuccessful
      * @throws Exception
      */
     @Test (dataProvider = "userWithoutPermissionForRemoveFromHold")
     public void removeFromHoldWithUserWithoutPermission(UserModel userModel, String nodeIdToBeRemoved) throws Exception
     {
-        STEP("Remove content from hold with user without right permission or capability");
+        STEP("Remove node from hold with user without right permission or capability");
         String responseNoHoldPermission = holdsAPI.removeFromHoldAndGetMessage(userModel.getUsername(),
                 userModel.getPassword(), SC_INTERNAL_SERVER_ERROR, nodeIdToBeRemoved, HOLD_ONE);
         assertTrue(responseNoHoldPermission.contains(ACCESS_DENIED_ERROR_MESSAGE));
 
-        STEP("Check active content is frozen.");
+        STEP("Check node is frozen.");
-        RestNodeModel heldActiveContent = restClient.authenticateUser(getAdminUser())
+        assertTrue(hasAspect(nodeIdToBeRemoved, FROZEN_ASPECT));
-                                                    .withCoreAPI().usingNode(toContentModel(nodeIdToBeRemoved))
-                                                    .getNode();
-        assertTrue(heldActiveContent.getAspectNames().contains(FROZEN_ASPECT));
 
     }
 

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/utils/CoreUtil.java

@@ -26,8 +26,12 @@
  */
 package org.alfresco.rest.rm.community.utils;
 
+import java.lang.reflect.InvocationTargetException;
+
 import org.alfresco.rest.model.RestNodeBodyMoveCopyModel;
 import org.alfresco.utility.model.ContentModel;
+import org.alfresco.utility.model.FileModel;
+import org.alfresco.utility.model.RepoTestModel;
 
 /**
  * Utility class for core components models
@@ -63,8 +67,43 @@ public class CoreUtil
      */
     public static ContentModel toContentModel(String nodeId)
     {
-        ContentModel node = new ContentModel();
+        return toModel(nodeId, ContentModel.class);
-        node.setNodeRef(nodeId);
-        return node;
     }
+
+    /**
+     * Helper method to create a File Model
+     *
+     * @return ContentModel
+     * @throws Exception
+     */
+    public  static FileModel toFileModel(String nodeId)
+    {
+        return toModel(nodeId,FileModel.class);
+    }
+
+    /**
+     * Helper method to create a RepoTestModel using the node id
+     *
+     * @param nodeId  node ref of the test model
+     * @param classOf repo test model class
+     * @return
+     */
+    private static <T extends RepoTestModel> T toModel(String nodeId, Class classOf)
+    {
+        T target = null;
+        try
+        {
+            target = (T) classOf.getDeclaredConstructor().newInstance();
+        }
+        catch (InvocationTargetException| NoSuchMethodException| IllegalAccessException | InstantiationException e)
+        {
+            e.printStackTrace();
+        }
+
+        target.setNodeRef(nodeId);
+        return target;
+
+    }
+
+
 }