[#12048] Migrate SearchInstructorsAction (#12340)

src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java

@@ -0,0 +1,174 @@
+package teammates.it.storage.sqlsearch;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import teammates.common.datatransfer.SqlDataBundle;
+import teammates.common.exception.SearchServiceException;
+import teammates.common.util.HibernateUtil;
+import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess;
+import teammates.storage.sqlapi.UsersDb;
+import teammates.storage.sqlentity.Instructor;
+import teammates.test.AssertHelper;
+import teammates.test.TestProperties;
+
+/**
+ * SUT: {@link UsersDb},
+ * {@link teammates.storage.sqlsearch.InstructorSearchDocument}.
+ */
+public class InstructorSearchIT extends BaseTestCaseWithSqlDatabaseAccess {
+
+    private final SqlDataBundle typicalBundle = getTypicalSqlDataBundle();
+    private final UsersDb usersDb = UsersDb.inst();
+
+    @Override
+    @BeforeMethod
+    protected void setUp() throws Exception {
+        super.setUp();
+        persistDataBundle(typicalBundle);
+        putDocuments(typicalBundle);
+        HibernateUtil.flushSession();
+    }
+
+    @Test
+    public void allTests() throws Exception {
+        if (!TestProperties.isSearchServiceActive()) {
+            return;
+        }
+
+        Instructor ins1InCourse1 = typicalBundle.instructors.get("instructor1OfCourse1");
+        Instructor ins2InCourse1 = typicalBundle.instructors.get("instructor2OfCourse1");
+        Instructor insInArchivedCourse = typicalBundle.instructors.get("instructorOfArchivedCourse");
+        Instructor insInUnregCourse = typicalBundle.instructors.get("instructorOfUnregisteredCourse");
+        Instructor insUniqueDisplayName = typicalBundle.instructors.get("instructorOfCourse2WithUniqueDisplayName");
+        Instructor ins1InCourse3 = typicalBundle.instructors.get("instructor1OfCourse3");
+
+        ______TS("success: search for instructors in whole system; query string does not match anyone");
+
+        List<Instructor> results = usersDb.searchInstructorsInWholeSystem("non-existent");
+        verifySearchResults(results);
+
+        ______TS("success: search for instructors in whole system; empty query string does not match anyone");
+
+        results = usersDb.searchInstructorsInWholeSystem("");
+        verifySearchResults(results);
+
+        ______TS("success: search for instructors in whole system; query string matches some instructors");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor of\"");
+        verifySearchResults(results, insInArchivedCourse, insInUnregCourse, insUniqueDisplayName);
+
+        ______TS("success: search for instructors in whole system; query string should be case-insensitive");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"InStRuCtOr 2\"");
+        verifySearchResults(results, ins2InCourse1);
+
+        ______TS("success: search for instructors in whole system; instructors in archived courses should be included");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor Of Archived Course\"");
+        verifySearchResults(results, insInArchivedCourse);
+
+        ______TS(
+                "success: search for instructors in whole system; instructors in unregistered course should be included");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor Of Unregistered Course\"");
+        verifySearchResults(results, insInUnregCourse);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by course id");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"course-1\"");
+        verifySearchResults(results, ins1InCourse1, ins2InCourse1);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by course name");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"Typical Course 1\"");
+        verifySearchResults(results, ins1InCourse1, ins2InCourse1);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by their name");
+
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor Of Unregistered Course\"");
+        verifySearchResults(results, insInUnregCourse);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by their email");
+
+        results = usersDb.searchInstructorsInWholeSystem("[email protected]");
+        verifySearchResults(results, ins2InCourse1);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by their role");
+        results = usersDb.searchInstructorsInWholeSystem("\"Co-owner\"");
+        verifySearchResults(results, ins1InCourse1, insInArchivedCourse,
+                insInUnregCourse, insUniqueDisplayName, ins1InCourse3);
+
+        ______TS("success: search for instructors in whole system; instructors should be searchable by displayed name");
+
+        String displayName = insUniqueDisplayName.getDisplayName();
+        results = usersDb.searchInstructorsInWholeSystem(displayName);
+        verifySearchResults(results, insUniqueDisplayName);
+
+        ______TS("success: search for instructors in whole system; deleted instructors no longer searchable");
+
+        usersDb.deleteUser(insUniqueDisplayName);
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor of\"");
+        verifySearchResults(results, insInArchivedCourse, insInUnregCourse);
+
+        // This method used to use usersDb.putEntity, not sure if the .createInstructor method has the same functionality
+        ______TS("success: search for instructors in whole system; instructors created without searchability unsearchable");
+        usersDb.createInstructor(insUniqueDisplayName);
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor of\"");
+        verifySearchResults(results, insInArchivedCourse, insInUnregCourse);
+
+        ______TS("success: search for instructors in whole system; deleting instructor without deleting document:"
+                + "document deleted during search, instructor unsearchable");
+
+        usersDb.deleteUser(ins1InCourse3);
+        results = usersDb.searchInstructorsInWholeSystem("\"Instructor 1\"");
+        verifySearchResults(results, ins1InCourse1);
+    }
+
+    @Test
+    public void testSearchInstructor_deleteAfterSearch_shouldNotBeSearchable() throws Exception {
+        if (!TestProperties.isSearchServiceActive()) {
+            return;
+        }
+
+        Instructor ins1InCourse1 = typicalBundle.instructors.get("instructor1OfCourse1");
+        Instructor ins2InCourse1 = typicalBundle.instructors.get("instructor2OfCourse1");
+
+        List<Instructor> results = usersDb.searchInstructorsInWholeSystem("\"course-1\"");
+        verifySearchResults(results, ins1InCourse1, ins2InCourse1);
+
+        usersDb.deleteUser(ins1InCourse1);
+        results = usersDb.searchInstructorsInWholeSystem("\"course-1\"");
+        verifySearchResults(results, ins2InCourse1);
+
+        // This used to test .deleteInstructors, but we don't seem to have a similar method to delete all users in course
+        usersDb.deleteUser(ins2InCourse1);
+        results = usersDb.searchInstructorsInWholeSystem("\"course-1\"");
+        verifySearchResults(results);
+    }
+
+    @Test
+    public void testSearchInstructor_noSearchService_shouldThrowException() {
+        if (TestProperties.isSearchServiceActive()) {
+            return;
+        }
+
+        assertThrows(SearchServiceException.class,
+                () -> usersDb.searchInstructorsInWholeSystem("anything"));
+    }
+
+    /**
+     * Verifies that search results match with expected output.
+     *
+     * @param actual   the results from the search query.
+     * @param expected the expected results for the search query.
+     */
+    private static void verifySearchResults(List<Instructor> actual,
+            Instructor... expected) {
+        assertEquals(expected.length, actual.size());
+        AssertHelper.assertSameContentIgnoreOrder(Arrays.asList(expected), actual);
+    }
+}

src/it/java/teammates/it/storage/sqlsearch/package-info.java

@@ -0,0 +1,4 @@
+/**
+ * Contains test cases for {@link teammates.storage.search} package.
+ */
+package teammates.it.storage.sqlsearch;

src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java

@@ -20,6 +20,7 @@
 import teammates.common.datatransfer.SqlDataBundle;
 import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.InvalidParametersException;
+import teammates.common.exception.SearchServiceException;
 import teammates.common.util.HibernateUtil;
 import teammates.common.util.JsonUtils;
 import teammates.sqllogic.api.Logic;
@@ -41,6 +42,10 @@
 import teammates.storage.sqlentity.Student;
 import teammates.storage.sqlentity.Team;
 import teammates.storage.sqlentity.UsageStatistics;
+import teammates.storage.sqlsearch.AccountRequestSearchManager;
+import teammates.storage.sqlsearch.InstructorSearchManager;
+import teammates.storage.sqlsearch.SearchManagerFactory;
+import teammates.storage.sqlsearch.StudentSearchManager;
 import teammates.test.BaseTestCase;
 
 /**
@@ -71,6 +76,13 @@ protected static void setUpSuite() throws Exception {
 
         LogicStarter.initializeDependencies();
 
+        SearchManagerFactory.registerAccountRequestSearchManager(
+            new AccountRequestSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+        SearchManagerFactory.registerInstructorSearchManager(
+            new InstructorSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+        SearchManagerFactory.registerStudentSearchManager(
+            new StudentSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+
         // TODO: remove after migration, needed for dual db support
         teammates.logic.core.LogicStarter.initializeDependencies();
         LOCAL_DATASTORE_HELPER.start();
@@ -89,6 +101,10 @@ public void setupClass() {
     @AfterClass
     public void tearDownClass() {
         closeable.close();
+
+        SearchManagerFactory.getAccountRequestSearchManager().resetCollections();
+        SearchManagerFactory.getInstructorSearchManager().resetCollections();
+        SearchManagerFactory.getStudentSearchManager().resetCollections();
     }
 
     @AfterSuite
@@ -120,6 +136,13 @@ protected void persistDataBundle(SqlDataBundle dataBundle)
         logic.persistDataBundle(dataBundle);
     }
 
+    /**
+     * Puts searchable documents from the data bundle to the solr database.
+     */
+    protected void putDocuments(SqlDataBundle dataBundle) throws SearchServiceException {
+        logic.putDocuments(dataBundle);
+    }
+
     /**
      * Verifies that two entities are equal.
      */

src/it/java/teammates/it/test/TestProperties.java

@@ -17,6 +17,9 @@ public final class TestProperties {
     /** The value of "test.localdatastore.port" in test.properties file. */
     public static final int TEST_LOCALDATASTORE_PORT;
 
+    /** The value of "test.search.service.host" in test.search.service.host file. */
+    public static final String SEARCH_SERVICE_HOST;
+
     private TestProperties() {
         // prevent instantiation
     }
@@ -31,6 +34,7 @@ private TestProperties() {
 
             TEST_LOCALDATASTORE_PORT = Integer.parseInt(prop.getProperty("test.localdatastore.port"));
 
+            SEARCH_SERVICE_HOST = prop.getProperty("test.search.service.host");
         } catch (IOException | NumberFormatException e) {
             throw new RuntimeException(e);
         }

src/test/java/teammates/ui/webapi/SearchInstructorsActionTest.java → src/it/java/teammates/it/ui/webapi/SearchInstructorsActionIT.java

@@ -1,27 +1,35 @@
-package teammates.ui.webapi;
+package teammates.it.ui.webapi;
 
 import org.apache.http.HttpStatus;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-import teammates.common.datatransfer.DataBundle;
-import teammates.common.datatransfer.attributes.InstructorAttributes;
+import teammates.common.exception.EntityAlreadyExistsException;
+import teammates.common.exception.InvalidParametersException;
 import teammates.common.util.Const;
+import teammates.common.util.HibernateUtil;
+import teammates.storage.sqlentity.Course;
+import teammates.storage.sqlentity.Instructor;
 import teammates.test.TestProperties;
 import teammates.ui.output.InstructorsData;
 import teammates.ui.output.MessageOutput;
+import teammates.ui.webapi.JsonResult;
+import teammates.ui.webapi.SearchInstructorsAction;
 
 /**
  * SUT: {@link SearchInstructorsAction}.
  */
-public class SearchInstructorsActionTest extends BaseActionTest<SearchInstructorsAction> {
+public class SearchInstructorsActionIT extends BaseActionIT<SearchInstructorsAction> {
 
-    private final InstructorAttributes acc = typicalBundle.instructors.get("instructor1OfCourse1");
+    private final Instructor instructor = typicalBundle.instructors.get("instructor1OfCourse1");
 
     @Override
-    protected void prepareTestData() {
-        DataBundle dataBundle = getTypicalDataBundle();
-        removeAndRestoreDataBundle(dataBundle);
-        putDocuments(dataBundle);
+    @BeforeMethod
+    protected void setUp() throws Exception {
+        super.setUp();
+        persistDataBundle(typicalBundle);
+        putDocuments(typicalBundle);
+        HibernateUtil.flushSession();
     }
 
     @Override
@@ -52,12 +60,12 @@ protected void testExecute_searchCourseId_shouldSucceed() {
         }
 
         loginAsAdmin();
-        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, acc.getCourseId() };
+        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, instructor.getCourseId() };
         SearchInstructorsAction action = getAction(submissionParams);
         JsonResult result = getJsonResult(action);
         InstructorsData response = (InstructorsData) result.getOutput();
         assertTrue(response.getInstructors().stream()
-                .filter(i -> i.getName().equals(acc.getName()))
+                .filter(i -> i.getName().equals(instructor.getName()))
                 .findAny()
                 .isPresent());
     }
@@ -69,12 +77,12 @@ protected void testExecute_searchDisplayedName_shouldSucceed() {
         }
 
         loginAsAdmin();
-        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, acc.getDisplayedName() };
+        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, instructor.getDisplayName() };
         SearchInstructorsAction action = getAction(submissionParams);
         JsonResult result = getJsonResult(action);
         InstructorsData response = (InstructorsData) result.getOutput();
         assertTrue(response.getInstructors().stream()
-                .filter(i -> i.getName().equals(acc.getName()))
+                .filter(i -> i.getName().equals(instructor.getName()))
                 .findAny()
                 .isPresent());
     }
@@ -86,12 +94,12 @@ protected void testExecute_searchEmail_shouldSucceed() {
         }
 
         loginAsAdmin();
-        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, acc.getEmail() };
+        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, instructor.getEmail() };
         SearchInstructorsAction action = getAction(submissionParams);
         JsonResult result = getJsonResult(action);
         InstructorsData response = (InstructorsData) result.getOutput();
         assertTrue(response.getInstructors().stream()
-                .filter(i -> i.getName().equals(acc.getName()))
+                .filter(i -> i.getName().equals(instructor.getName()))
                 .findAny()
                 .isPresent());
         assertTrue(response.getInstructors().get(0).getKey() != null);
@@ -105,12 +113,12 @@ protected void testExecute_searchGoogleId_shouldSucceed() {
         }
 
         loginAsAdmin();
-        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, acc.getGoogleId() };
+        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, instructor.getGoogleId() };
         SearchInstructorsAction action = getAction(submissionParams);
         JsonResult result = getJsonResult(action);
         InstructorsData response = (InstructorsData) result.getOutput();
         assertTrue(response.getInstructors().stream()
-                .filter(i -> i.getName().equals(acc.getName()))
+                .filter(i -> i.getName().equals(instructor.getName()))
                 .findAny()
                 .isPresent());
         assertTrue(response.getInstructors().get(0).getKey() != null);
@@ -124,12 +132,12 @@ protected void testExecute_searchName_shouldSucceed() {
         }
 
         loginAsAdmin();
-        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, acc.getName() };
+        String[] submissionParams = new String[] { Const.ParamsNames.SEARCH_KEY, instructor.getName() };
         SearchInstructorsAction action = getAction(submissionParams);
         JsonResult result = getJsonResult(action);
         InstructorsData response = (InstructorsData) result.getOutput();
         assertTrue(response.getInstructors().stream()
-                .filter(i -> i.getName().equals(acc.getName()))
+                .filter(i -> i.getName().equals(instructor.getName()))
                 .findAny()
                 .isPresent());
         assertTrue(response.getInstructors().get(0).getKey() != null);
@@ -169,8 +177,9 @@ public void testExecute_noSearchService_shouldReturn501() {
 
     @Override
     @Test
-    protected void testAccessControl() {
-        verifyOnlyAdminCanAccess();
+    protected void testAccessControl() throws InvalidParametersException, EntityAlreadyExistsException {
+        Course course = typicalBundle.courses.get("course1");
+        verifyOnlyAdminCanAccess(course);
     }
 
 }