Implemented review comments

e2e-test/src/test/java/org/alfresco/test/search/functional/gs/AbstractGSE2ETest.java

@@ -274,8 +274,6 @@ public abstract class AbstractGSE2ETest extends AbstractInsightEngineE2ETest
     /**
      * Helper method to create root category
      *
-     * @param userModel The user under whose privileges this structure is going to be created
-     * @param categoryName The name of the category
      * @return The created category
      */
     public RecordCategoryChild createCategoryFolderInFilePlan()
@@ -289,7 +287,6 @@ public abstract class AbstractGSE2ETest extends AbstractInsightEngineE2ETest
     /**
      * Helper method to create record folder
      *
-     * @param user The user under whose privileges this structure is going to be created
      * @param recordCategoryId The id of the record category
      * @param name The name of the folder
      * @return The created folder
@@ -305,6 +302,7 @@ public abstract class AbstractGSE2ETest extends AbstractInsightEngineE2ETest
     /**
      * Helper method to create a test user with rm role
      *
+     * @param user to which the rm role is to be assigned
      * @param userRole the rm role
      * @return the created user model
      */

e2e-test/src/test/java/org/alfresco/test/search/functional/gs/sql/SearchSqlGSE2ETest.java

@@ -16,7 +16,6 @@ import org.alfresco.rest.rm.community.model.record.Record;
 import org.alfresco.rest.rm.community.model.recordcategory.RecordCategoryChild;
 import org.alfresco.rest.rm.community.model.user.UserPermissions;
 import org.alfresco.rest.rm.enterprise.core.ClassificationData;
-import org.alfresco.rest.search.SearchRequest;
 import org.alfresco.rest.search.SearchResponse;
 import org.alfresco.rest.search.SearchSqlRequest;
 import org.alfresco.search.TestGroup;
@@ -27,7 +26,6 @@ import org.alfresco.utility.model.FileType;
 import org.alfresco.utility.model.FolderModel;
 import org.alfresco.utility.model.UserModel;
 import org.hamcrest.Matchers;
-import org.springframework.http.HttpStatus;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
@@ -42,11 +40,11 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
     private UserModel testUserGSTopSecret, testUserSecret, testUserConfidential, testUserNoAccess;
 
     private FolderModel testFolder;
-    private RecordCategoryChild recordFolder;
 
-    private FileModel fileRecord, fileRecordElectronic, fileUnclassified, fileClassifiedAsTopSecret, fileClassifiedAsSecret, fileClassifiedAsConfidential;
+    private FileModel fileRecord, fileRecordElectronic, fileUnclassified, fileClassifiedAsTopSecret,
+            fileClassifiedAsSecret, fileClassifiedAsConfidential;
     
-    private Record inPlaceRecord, elecRecord;
+    private final String RECORD_IDENTIFIER = "rma:identifier:'*'";
 
     @BeforeClass(alwaysRun = true)
     public void dataPreparation() throws Exception
@@ -93,9 +91,14 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
 
         fileUnclassified = new FileModel(unique_searchString + "Unclassified-1.txt", "Unclassified1", "Unclassified1", FileType.TEXT_PLAIN, "Unclassified1");
         dataContent.usingUser(testUser).usingSite(testSite).createContent(fileUnclassified);
+
+        // Classify files
+        getClassificationService().classifyNode(adminUserModel, fileClassifiedAsTopSecret.getNodeRefWithoutVersion(), ClassificationData.TOP_SECRET_CLASSIFICATION_LEVEL_ID);
+        getClassificationService().classifyNode(testUser, fileClassifiedAsSecret.getNodeRefWithoutVersion(), ClassificationData.SECRET_CLASSIFICATION_LEVEL_ID);
+        getClassificationService().classifyNode(testUser, fileClassifiedAsConfidential.getNodeRefWithoutVersion(), ClassificationData.CONFIDENTIAL_CLASSIFICATION_LEVEL_ID);
     }
 
-    @Test(priority = 1, groups = {TestGroup.AGS_302})
+    @Test(priority = 1, groups = { TestGroup.AGS_302 })
     public void testSQLRespectsSitePermissions()
     {
         // Search for a file name to ensure content is indexed
@@ -113,31 +116,20 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
 
         // Verify that testUserNoAccess can't see content of the Private site that he is not a member of
         RestResponse response = searchSql(sqlRequest, testUserNoAccess);
-
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(0));
 
         // Verify that the site member can see the content: Collaborator
         response = searchSql(sqlRequest, testUserSecret);
-
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
 
         // Verify that the site member can see the content: Consumer
         response = searchSql(sqlRequest, testUserConfidential);
-
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
     }
 
-    @Test(priority = 2, groups = {TestGroup.AGS_302})
+    @Test(priority = 2, groups = { TestGroup.AGS_302 })
     public void testSQLFiltersClassifiedFiles()
     {
-        // Classify files as testUserGSTopSecret        
-        getClassificationService().classifyNode(adminUserModel, fileClassifiedAsTopSecret.getNodeRefWithoutVersion(), ClassificationData.TOP_SECRET_CLASSIFICATION_LEVEL_ID);
-        getClassificationService().classifyNode(testUser, fileClassifiedAsSecret.getNodeRefWithoutVersion(), ClassificationData.SECRET_CLASSIFICATION_LEVEL_ID);
-        getClassificationService().classifyNode(testUser, fileClassifiedAsConfidential.getNodeRefWithoutVersion(), ClassificationData.CONFIDENTIAL_CLASSIFICATION_LEVEL_ID);
-
         // Wait for the files to be indexed
         boolean fileFound = isContentInSearchResults("sc_classification:c", fileClassifiedAsConfidential.getName(), true);
         Assert.assertTrue(fileFound, "Expected confidential file, not found");
@@ -147,27 +139,23 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         sqlRequest.setSql("select sc_classification, count(*) from alfresco group by sc_classification");
 
         RestResponse response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
+        response.assertThat().body("list.pagination.count", Matchers.equalTo(4)); // Actual count = 4, TS, S, C, U
-        // TODO: Validate impact of Site not being specified
-        // response.assertThat().body("list.pagination.count", Matchers.equalTo(3)); //Actual count = 4 is Site is not specified
 
         sqlRequest = new SearchSqlRequest();
         sqlRequest.setSql("select sc_classification, count(*) from alfresco where SITE = '" + testSite.getId() + "' group by sc_classification");
+        // TODO: Investigate: Actual count = 3, TS, S, C: Why U is omitted if Site is not specified
+        response.assertThat().body("list.pagination.count", Matchers.equalTo(4));
 
         response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(3));
 
         response = searchSql(sqlRequest, testUserSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(2));
 
         response = searchSql(sqlRequest, testUserConfidential);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
 
         response = searchSql(sqlRequest, testUserNoAccess);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(0));
 
         // Check Query results, in a Solr format
@@ -176,7 +164,6 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         sqlRequest.setFormat("solr");
 
         response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         List<String> results = response.getResponse().getBody().jsonPath().getList("result-set.docs.cm_name");
         Assert.assertTrue(results.contains(fileClassifiedAsTopSecret.getName()), "Top Secret file not included in the results");
         Assert.assertTrue(results.contains(fileClassifiedAsSecret.getName()), "Secret file not included in the results");
@@ -184,7 +171,6 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         Assert.assertFalse(results.contains(fileUnclassified.getName()), "Unclassified file included in the results when not expected");
 
         response = searchSql(sqlRequest, testUserSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         results = response.getResponse().getBody().jsonPath().getList("result-set.docs.cm_name");
         Assert.assertFalse(results.contains(fileClassifiedAsTopSecret.getName()), "Top Secret file is included in the results when not expected");
         Assert.assertTrue(results.contains(fileClassifiedAsSecret.getName()), "Secret file not included in the results");
@@ -192,7 +178,6 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         Assert.assertFalse(results.contains(fileUnclassified.getName()), "Unclassified file included in the results when not expected");
 
         response = searchSql(sqlRequest, testUserConfidential);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         results = response.getResponse().getBody().jsonPath().getList("result-set.docs.cm_name");
         Assert.assertFalse(results.contains(fileClassifiedAsTopSecret.getName()), "Top Secret file is included in the results when not expected");
         Assert.assertFalse(results.contains(fileClassifiedAsSecret.getName()), "Secret file is included in the results when not expected");
@@ -200,7 +185,6 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         Assert.assertFalse(results.contains(fileUnclassified.getName()), "Unclassified file included in the results when not expected");
 
         response = searchSql(sqlRequest, testUserNoAccess);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         results = response.getResponse().getBody().jsonPath().getList("result-set.docs.cm_name");
         Assert.assertFalse(results.contains(fileClassifiedAsTopSecret.getName()), "Top Secret file is included in the results when not expected");
         Assert.assertFalse(results.contains(fileClassifiedAsSecret.getName()), "Secret file is included in the results when not expected");
@@ -208,50 +192,47 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         Assert.assertFalse(results.contains(fileUnclassified.getName()), "Unclassified file included in the results when not expected");
     }
 
-    @Test(priority = 3, groups = {TestGroup.AGS_302})
+    @Test(priority = 3, groups = { TestGroup.AGS_302 })
     public void testSQLFiltersElectronicRecords()
     {
         // File a Electronic Record
-        recordFolder = createCategoryFolderInFilePlan();
+        RecordCategoryChild recordFolder = createCategoryFolderInFilePlan();
-        elecRecord = createElectronicRecord(recordFolder.getId(), fileRecord.getName());        
+        Record elecRecord = createElectronicRecord(recordFolder.getId(), fileRecord.getName());
 
         // Add permission for the GS user to Read Records
         getRestAPIFactory().getRMUserAPI().addUserPermission(recordFolder.getParentId(), testUser, UserPermissions.PERMISSION_READ_RECORDS);
 
         // Wait for the record to be indexed and check that it can be found
-        boolean fileFound = isContentInSearchResults("rma:identifier:'*'", elecRecord.getName(), true);
+        boolean fileFound = isContentInSearchResults(RECORD_IDENTIFIER, elecRecord.getName(), true);
         Assert.assertTrue(fileFound, "Expected record file, not found");
 
-        SearchResponse result = queryAsUser(testUserGSTopSecret, "rma:identifier:'*'");
+        SearchResponse result = queryAsUser(testUserGSTopSecret, RECORD_IDENTIFIER);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
+        Assert.assertEquals(result.getPagination().getCount(), 0, "Expected count of entries 0, found more");
-        Assert.assertTrue(result.getPagination().getCount() == 0, "Expected count of entries 0, found more");
 
         // Verify that user can see the electronic record with minimum read records permissions
         SearchSqlRequest sqlRequest = new SearchSqlRequest();
         sqlRequest.setSql("select cm_name, PATH from alfresco where rma_identifier = '*' and type = 'cm:content'");
 
         RestResponse response = searchSql(sqlRequest, testUser);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
+
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
 
         response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(0));
 
         // Verify that user can not see the electronic record with no read permissions
         response = searchSql(sqlRequest, testUserConfidential);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(0));
     }
 
-    @Test(priority = 4, groups = {TestGroup.AGS_302})
+    @Test(priority = 4, groups = { TestGroup.AGS_302 })
     public void testSQLFiltersInPlaceRecords()
     {
         // Declare a file as Record
-        inPlaceRecord = getRestAPIFactory().getFilesAPI().declareAsRecord(fileRecord.getNodeRefWithoutVersion());
+        Record inPlaceRecord = getRestAPIFactory().getFilesAPI().declareAsRecord(fileRecord.getNodeRefWithoutVersion());
 
         // Wait for the record to be indexed
-        boolean fileFound = isContentInSearchResults("rma:identifier:'*'", inPlaceRecord.getName(), true);
+        boolean fileFound = isContentInSearchResults(RECORD_IDENTIFIER, inPlaceRecord.getName(), true);
         Assert.assertTrue(fileFound, "Expected in place record file, not found");
 
         // Verify that user can see the in place record without any additional permissions
@@ -259,37 +240,35 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         sqlRequest.setSql("select cm_name from alfresco where rma_identifier = '*' and Site = '" + testSite.getId() + "'");
 
         RestResponse response = searchSql(sqlRequest, testUser);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
         response.assertThat().body("list.entries.entry[0].value", Matchers.contains(inPlaceRecord.getName()));
 
         // Verify that other site members can see the in place record with no additional permissions
         response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
         response.assertThat().body("list.entries.entry[0].value", Matchers.contains(inPlaceRecord.getName()));
 
         response = searchSql(sqlRequest, testUserConfidential);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(1));
         response.assertThat().body("list.entries.entry[0].value", Matchers.contains(inPlaceRecord.getName()));
 
         response = searchSql(sqlRequest, testUserNoAccess);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(0));
     }
 
-    @Test(priority = 5, groups = {TestGroup.AGS_302})
+    @Test(priority = 5, groups = { TestGroup.AGS_302 })
     public void testSQLFiltersRecordsCascadedPermissions()
     {
         // Search for records when user does not have read permission
         boolean fileFound = isContentInSearchResults(NON_ELECTRONIC_FILE, nonElectronicRecord.getName(), false);
         Assert.assertTrue(fileFound, "Non electronic record file found in the results, when user doesn't have read permissions");
         
+
         // Search for records when user does not have read permission
         fileFound = isContentInSearchResults(ELECTRONIC_FILE, electronicRecord.getName(), false);
         Assert.assertTrue(fileFound, "Electronic record file found in the results, when user doesn't have read permissions");
         
+
         // Assign Read permissions to the user at rootCategory level
         getRestAPIFactory().getRMUserAPI().addUserPermission(rootCategory.getId(), testUser, UserPermissions.PERMISSION_READ_RECORDS);
         getRestAPIFactory().getRMUserAPI().addUserPermission(rootCategory.getId(), testUserGSTopSecret, UserPermissions.PERMISSION_READ_RECORDS);
@@ -308,13 +287,11 @@ public class SearchSqlGSE2ETest extends AbstractGSE2ETest
         sqlRequest.setSql("select cm_name from alfresco where rma_identifier = '*' and cm_name in " + recordFiles + " order by cm_name");
 
         RestResponse response = searchSql(sqlRequest, testUser);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(2));
         response.assertThat().body("list.entries.entry[0].value", Matchers.contains(electronicRecord.getName()));
         response.assertThat().body("list.entries.entry[1].value", Matchers.contains(nonElectronicRecord.getName()));
 
         response = searchSql(sqlRequest, testUserGSTopSecret);
-        restClient.assertStatusCodeIs(HttpStatus.OK);
         response.assertThat().body("list.pagination.count", Matchers.equalTo(2));
         response.assertThat().body("list.entries.entry[0].value", Matchers.contains(electronicRecord.getName()));
         response.assertThat().body("list.entries.entry[1].value", Matchers.contains(nonElectronicRecord.getName()));