Code review comments (check node path for add to hold and remove from hold audit entries)

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

@@ -101,4 +101,30 @@ public class RMAuditService
                         !auditEntry.getTimestamp().isEmpty() &&
                         Instant.parse(auditEntry.getTimestamp()).compareTo(eventTimestamp) <= 0));
     }
+
+    /**
+     * Checks the rm audit log contains the entry for the given event.
+     *
+     * @param user          the user who checks the audit log
+     * @param auditEvent    the audited event
+     * @param auditUser     the user who did the audited event
+     * @param nodeName      the audited node name if exists or empty string
+     * @param nodePath      the path of the audited node if exists or empty string
+     * @param changedValues the values changed by event if exist or empty list
+     */
+    public void checkAuditLogForEvent(UserModel user, AuditEvents auditEvent, UserModel auditUser,
+                                         String nodeName, String nodePath, List<Object> changedValues)
+    {
+        final Instant eventTimestamp = Instant.now();
+        List<AuditEntry> auditEntries = getAuditEntriesFilteredByEvent(user, auditEvent);
+        assertTrue("The list of events is not filtered by " + auditEvent.event,
+                auditEntries.stream().allMatch(auditEntry -> auditEntry.getEvent().equals(auditEvent.eventDisplayName)));
+        assertTrue("The event details are not audited",
+                auditEntries.stream().anyMatch(auditEntry -> auditEntry.getNodeName().equals(nodeName) &&
+                        auditEntry.getUserName().equals(auditUser.getUsername()) &&
+                        auditEntry.getPath().equals(nodePath) &&
+                        CollectionUtils.isEqualCollection(auditEntry.getChangedValues(), changedValues) &&
+                        !auditEntry.getTimestamp().isEmpty() &&
+                        Instant.parse(auditEntry.getTimestamp()).compareTo(eventTimestamp) <= 0));
+    }
 }

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditAddToHoldTests.java

@@ -32,6 +32,8 @@ import static org.alfresco.rest.rm.community.base.TestData.HOLD_DESCRIPTION;
 import static org.alfresco.rest.rm.community.base.TestData.HOLD_REASON;
 import static org.alfresco.rest.rm.community.model.audit.AuditEvents.ADD_TO_HOLD;
 import static org.alfresco.rest.rm.community.util.CommonTestUtils.generateTestPrefix;
+import static org.alfresco.utility.Utility.buildPath;
+import static org.alfresco.utility.Utility.removeLastSlash;
 import static org.alfresco.utility.data.RandomData.getRandomName;
 import static org.alfresco.utility.report.log.Step.STEP;
 import static org.apache.commons.httpclient.HttpStatus.SC_INTERNAL_SERVER_ERROR;
@@ -121,25 +123,32 @@ public class AuditAddToHoldTests extends BaseRMRestTest
     /**
      * Data provider with valid nodes that can be added to a hold
      *
-     * @return the node id and the node name
+     * @return the node id, the node name and the node path
      * @throws Exception
      */
     @DataProvider (name = "validNodesForAddToHold")
     public Object[][] getValidNodesForAddToHold() throws Exception
     {
+        String documentLibrary = "/documentLibrary";
         FileModel contentToBeAdded = dataContent.usingAdmin().usingSite(privateSite)
                                                 .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
         RecordCategoryChild recordFolderToBeAdded = createRecordFolder(recordCategory.getId(), PREFIX + "recFolderToBeAdded");
         Record recordToBeAdded = createElectronicRecord(recordFolder.getId(), PREFIX + "record");
+        String recordFolderPath = removeLastSlash(buildPath(documentLibrary, recordCategory.getName(),
+                recordFolderToBeAdded.getName()));
+        String recordPath = removeLastSlash(buildPath(documentLibrary, recordCategory.getName(),
+                recordFolder.getName(), recordToBeAdded.getName()));
+        String contentPath = contentToBeAdded.getCmisLocation();
+        contentPath = contentPath.substring(contentPath.indexOf(documentLibrary));
 
         return new String[][]
         {
             // a record folder
-            { recordFolderToBeAdded.getId(), recordFolderToBeAdded.getName() },
+            { recordFolderToBeAdded.getId(), recordFolderToBeAdded.getName(), recordFolderPath },
             // a record
-            { recordToBeAdded.getId(), recordToBeAdded.getName() },
+            { recordToBeAdded.getId(), recordToBeAdded.getName(), recordPath },
             //an active content,
-            { contentToBeAdded.getNodeRefWithoutVersion(), contentToBeAdded.getName() }
+            { contentToBeAdded.getNodeRefWithoutVersion(), contentToBeAdded.getName(), contentPath }
         };
     }
 
@@ -166,9 +175,10 @@ public class AuditAddToHoldTests extends BaseRMRestTest
      *      name of the document/record/record folder added
      *      user who added the content
      *      date the content was added
+     *      path of the node
      */
     @Test (dataProvider = "validNodesForAddToHold")
-    public void addToHoldEventIsAudited(String nodeId, String nodeName)
+    public void addToHoldEventIsAudited(String nodeId, String nodeName, String nodePath)
     {
         rmAuditService.clearAuditLog();
 
@@ -176,7 +186,7 @@ public class AuditAddToHoldTests extends BaseRMRestTest
         holdsAPI.addItemToHold(rmAdmin.getUsername(), rmAdmin.getPassword(), nodeId, HOLD1);
 
         STEP("Check the audit log contains the entry for the add to hold event.");
-        rmAuditService.checkAuditLogForEvent(getAdminUser(), ADD_TO_HOLD, rmAdmin, nodeName,
+        rmAuditService.checkAuditLogForEvent(getAdminUser(), ADD_TO_HOLD, rmAdmin, nodeName, nodePath,
                 asList(ImmutableMap.of("new", nodeName, "previous", "", "name", "Name"),
                         ImmutableMap.of("new", HOLD1, "previous", "", "name", "Hold Name")));
     }

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditRemoveFromHoldTests.java

@@ -32,6 +32,8 @@ import static org.alfresco.rest.rm.community.base.TestData.HOLD_DESCRIPTION;
 import static org.alfresco.rest.rm.community.base.TestData.HOLD_REASON;
 import static org.alfresco.rest.rm.community.model.audit.AuditEvents.REMOVE_FROM_HOLD;
 import static org.alfresco.rest.rm.community.util.CommonTestUtils.generateTestPrefix;
+import static org.alfresco.utility.Utility.buildPath;
+import static org.alfresco.utility.Utility.removeLastSlash;
 import static org.alfresco.utility.data.RandomData.getRandomName;
 import static org.alfresco.utility.report.log.Step.STEP;
 import static org.apache.commons.httpclient.HttpStatus.SC_INTERNAL_SERVER_ERROR;
@@ -137,19 +139,26 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
     /**
      * Data provider with valid nodes that can be removed from a hold
      *
-     * @return the node id and the node name
+     * @return the node id, the node name and the node path
      */
     @DataProvider (name = "validNodesForRemoveFromHold")
     public Object[][] getValidNodesForRemoveFromHold()
     {
+        String documentLibrary = "/documentLibrary";
+        String recordFolderPath = removeLastSlash(buildPath(documentLibrary, recordCategory.getName(),
+                heldRecordFolder.getName()));
+        String recordPath = removeLastSlash(buildPath(documentLibrary, recordCategory.getName(),
+                recordFolder.getName(), heldRecord.getName()));
+        String contentPath = heldContent.getCmisLocation();
+        contentPath = contentPath.substring(contentPath.indexOf(documentLibrary));
         return new String[][]
         {
             // a record folder
-            { heldRecordFolder.getId(), heldRecordFolder.getName() },
+            { heldRecordFolder.getId(), heldRecordFolder.getName(), recordFolderPath },
             // a record
-            { heldRecord.getId(), heldRecord.getName() },
+            { heldRecord.getId(), heldRecord.getName(), recordPath },
             //an active content,
-            { heldContent.getNodeRefWithoutVersion(), heldContent.getName() }
+            { heldContent.getNodeRefWithoutVersion(), heldContent.getName(), contentPath }
         };
     }
 
@@ -176,9 +185,10 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
      *      name of the document/record/record folder removed
      *      user who removed the content
      *      date the content was removed
+     *      path of the node
      */
     @Test (dataProvider = "validNodesForRemoveFromHold")
-    public void removeFromHoldEventIsAudited(String nodeId, String nodeName)
+    public void removeFromHoldEventIsAudited(String nodeId, String nodeName, String nodePath)
     {
         rmAuditService.clearAuditLog();
 
@@ -186,7 +196,7 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
         holdsAPI.removeItemFromHold(rmAdmin.getUsername(), rmAdmin.getPassword(), nodeId, HOLD3);
 
         STEP("Check the audit log contains the entry for the remove from hold event.");
-        rmAuditService.checkAuditLogForEvent(getAdminUser(), REMOVE_FROM_HOLD, rmAdmin, nodeName,
+        rmAuditService.checkAuditLogForEvent(getAdminUser(), REMOVE_FROM_HOLD, rmAdmin, nodeName, nodePath,
                 asList(ImmutableMap.of("new", "", "previous", nodeName, "name", "Name"),
                         ImmutableMap.of("new", "", "previous", HOLD3, "name", "Hold Name")));
     }