[#12048] Fix account request indexing (#12967)

* Add isTransactionNeeded method to Action * Remove delay from taskqueuer * Change CreateAccountRequest to handle own transactions
TEAMMATES · Mar 29, 2024 · 356c318 · 356c318
1 parent c4fe140
commit 356c318
Show file tree

Hide file tree

Showing 8 changed files with 97 additions and 13 deletions.
diff --git a/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java b/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java
@@ -1,10 +1,12 @@
 package teammates.it.ui.webapi;
 
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import teammates.common.util.Const;
 import teammates.common.util.EmailType;
 import teammates.common.util.EmailWrapper;
+import teammates.common.util.HibernateUtil;
 import teammates.storage.sqlentity.AccountRequest;
 import teammates.ui.output.JoinLinkData;
 import teammates.ui.request.AccountCreateRequest;
@@ -26,6 +28,13 @@ String getRequestMethod() {
         return POST;
     }
 
+    @BeforeMethod
+    @Override
+    protected void setUp() {
+        // CreateAccountRequestAction handles its own transactions;
+        // There is thus no need to setup a transaction.
+    }
+
     @Override
     @Test
     protected void testExecute() throws Exception {
@@ -37,7 +46,9 @@ protected void testExecute() throws Exception {
         CreateAccountRequestAction action = getAction(request);
         JsonResult result = getJsonResult(action);
         JoinLinkData output = (JoinLinkData) result.getOutput();
+        HibernateUtil.beginTransaction();
         AccountRequest accountRequest = logic.getAccountRequest("[email protected]", "The Fellowship of the Ring");
+        HibernateUtil.commitTransaction();
         assertEquals("[email protected]", accountRequest.getEmail());
         assertEquals("Frodo Baggins", accountRequest.getName());
         assertEquals("The Fellowship of the Ring", accountRequest.getInstitute());

diff --git a/src/main/java/teammates/logic/api/TaskQueuer.java b/src/main/java/teammates/logic/api/TaskQueuer.java
@@ -228,10 +228,8 @@ public void scheduleAccountRequestForSearchIndexing(String email, String institu
         paramMap.put(ParamsNames.INSTRUCTOR_EMAIL, email);
         paramMap.put(ParamsNames.INSTRUCTOR_INSTITUTION, institute);
 
-        // TODO: change the action CreateAccountRequestAction to call scheduleAccountRequestForSearchIndexing
-        // after AccountRequest is inserted in the DB
-        addDeferredTask(TaskQueue.SEARCH_INDEXING_QUEUE_NAME, TaskQueue.ACCOUNT_REQUEST_SEARCH_INDEXING_WORKER_URL,
-                paramMap, null, 60);
+        addTask(TaskQueue.SEARCH_INDEXING_QUEUE_NAME, TaskQueue.ACCOUNT_REQUEST_SEARCH_INDEXING_WORKER_URL,
+                paramMap, null);
     }
 
     /**

diff --git a/src/main/java/teammates/logic/api/UserProvision.java b/src/main/java/teammates/logic/api/UserProvision.java
@@ -3,6 +3,7 @@
 import teammates.common.datatransfer.UserInfo;
 import teammates.common.datatransfer.UserInfoCookie;
 import teammates.common.util.Config;
+import teammates.common.util.HibernateUtil;
 import teammates.logic.core.InstructorsLogic;
 import teammates.logic.core.StudentsLogic;
 import teammates.sqllogic.core.UsersLogic;
@@ -48,6 +49,16 @@ public UserInfo getCurrentUser(UserInfoCookie uic) {
         return user;
     }
 
+    /**
+     * Gets the information of the current logged in user, with an SQL transaction.
+     */
+    public UserInfo getCurrentUserWithTransaction(UserInfoCookie uic) {
+        HibernateUtil.beginTransaction();
+        UserInfo userInfo = getCurrentUser(uic);
+        HibernateUtil.commitTransaction();
+        return userInfo;
+    }
+
     // TODO: method visibility to package-private after migration
     /**
      * Gets the current logged in user.

diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java
@@ -94,6 +94,19 @@ public AccountRequest createAccountRequest(String name, String email, String ins
         return accountRequestLogic.createAccountRequest(name, email, institute);
     }
 
+    /**
+     * Creates a or gets an account request.
+     *
+     * @return newly created account request.
+     * @throws InvalidParametersException if the account request details are invalid.
+     * @throws EntityAlreadyExistsException if the account request already exists.
+     */
+    public AccountRequest createAccountRequestWithTransaction(String name, String email, String institute)
+            throws InvalidParametersException {
+
+        return accountRequestLogic.createOrGetAccountRequestWithTransaction(name, email, institute);
+    }
+
     /**
      * Gets the account request with the given email and institute.
      *

diff --git a/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java b/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java
@@ -6,6 +6,7 @@
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.exception.InvalidParametersException;
 import teammates.common.exception.SearchServiceException;
+import teammates.common.util.HibernateUtil;
 import teammates.storage.sqlapi.AccountRequestsDb;
 import teammates.storage.sqlentity.AccountRequest;
 import teammates.storage.sqlsearch.AccountRequestSearchManager;
@@ -126,4 +127,26 @@ public List<AccountRequest> searchAccountRequestsInWholeSystem(String queryStrin
             throws SearchServiceException {
         return accountRequestDb.searchAccountRequestsInWholeSystem(queryString);
     }
+
+    /**
+     * Creates an or gets an account request.
+     */
+    public AccountRequest createOrGetAccountRequestWithTransaction(String name, String email, String institute)
+            throws InvalidParametersException {
+        AccountRequest toCreate = new AccountRequest(email, name, institute);
+        HibernateUtil.beginTransaction();
+        AccountRequest accountRequest;
+        try {
+            accountRequest = accountRequestDb.createAccountRequest(toCreate);
+            HibernateUtil.commitTransaction();
+        } catch (InvalidParametersException ipe) {
+            HibernateUtil.rollbackTransaction();
+            throw new InvalidParametersException(ipe);
+        } catch (EntityAlreadyExistsException eaee) {
+            // Use existing account request
+            accountRequest = getAccountRequest(email, institute);
+            HibernateUtil.commitTransaction();
+        }
+        return accountRequest;
+    }
 }
diff --git a/src/main/java/teammates/ui/servlets/WebApiServlet.java b/src/main/java/teammates/ui/servlets/WebApiServlet.java
@@ -60,7 +60,14 @@ private void invokeServlet(HttpServletRequest req, HttpServletResponse resp) thr
 
         try {
             action = ActionFactory.getAction(req, req.getMethod());
-            ActionResult result = executeWithTransaction(action, req);
+            ActionResult result;
+
+            if (action.isTransactionNeeded()) {
+                result = executeWithTransaction(action, req);
+            } else {
+                result = executeWithoutTransaction(action, req);
+            }
+
             statusCode = result.getStatusCode();
             result.send(resp);
         } catch (ActionMappingException e) {
@@ -127,6 +134,14 @@ private ActionResult executeWithTransaction(Action action, HttpServletRequest re
         }
     }
 
+    private ActionResult executeWithoutTransaction(Action action, HttpServletRequest req)
+            throws InvalidOperationException, InvalidHttpRequestBodyException, UnauthorizedAccessException {
+        action.init(req);
+        action.checkAccessControl();
+
+        return action.execute();
+    }
+
     private void throwErrorBasedOnRequester(HttpServletRequest req, HttpServletResponse resp, Exception e, int statusCode)
             throws IOException {
         // The header X-AppEngine-QueueName cannot be spoofed as GAE will strip any user-sent X-AppEngine-QueueName headers.

diff --git a/src/main/java/teammates/ui/webapi/Action.java b/src/main/java/teammates/ui/webapi/Action.java
@@ -216,7 +216,11 @@ private void initAuthInfo() {
         } else {
             String cookie = HttpRequestHelper.getCookieValueFromRequest(req, Const.SecurityConfig.AUTH_COOKIE_NAME);
             UserInfoCookie uic = UserInfoCookie.fromCookie(cookie);
-            userInfo = userProvision.getCurrentUser(uic);
+            if (isTransactionNeeded()) {
+                userInfo = userProvision.getCurrentUser(uic);
+            } else {
+                userInfo = userProvision.getCurrentUserWithTransaction(uic);
+            }
         }
 
         authType = userInfo == null ? AuthType.PUBLIC : AuthType.LOGGED_IN;
@@ -480,6 +484,14 @@ InstructorPermissionSet constructInstructorPrivileges(Instructor instructor, Str
         return privilege;
     }
 
+    /**
+     * Checks if the action requires a SQL transaction when executed.
+     * If false, the action will have to handle its own SQL transactions.
+     */
+    public boolean isTransactionNeeded() {
+        return true;
+    }
+
     /**
      * Gets the minimum access control level required to access the resource.
      */

diff --git a/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java b/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java
@@ -1,6 +1,5 @@
 package teammates.ui.webapi;
 
-import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.InvalidParametersException;
 import teammates.common.util.EmailWrapper;
 import teammates.storage.sqlentity.AccountRequest;
@@ -13,6 +12,11 @@
  */
 public class CreateAccountRequestAction extends AdminOnlyAction {
 
+    @Override
+    public boolean isTransactionNeeded() {
+        return false;
+    }
+
     @Override
     public JsonResult execute()
             throws InvalidHttpRequestBodyException, InvalidOperationException {
@@ -25,16 +29,13 @@ public JsonResult execute()
         AccountRequest accountRequest;
 
         try {
-            accountRequest = sqlLogic.createAccountRequest(instructorName, instructorEmail, instructorInstitution);
-            taskQueuer.scheduleAccountRequestForSearchIndexing(instructorEmail, instructorInstitution);
+            accountRequest =
+                sqlLogic.createAccountRequestWithTransaction(instructorName, instructorEmail, instructorInstitution);
         } catch (InvalidParametersException ipe) {
             throw new InvalidHttpRequestBodyException(ipe);
-        } catch (EntityAlreadyExistsException eaee) {
-            // Use existing account request
-            accountRequest = sqlLogic.getAccountRequest(instructorEmail, instructorInstitution);
         }
 
-        assert accountRequest != null;
+        taskQueuer.scheduleAccountRequestForSearchIndexing(instructorEmail, instructorInstitution);
 
         if (accountRequest.getRegisteredAt() != null) {
             throw new InvalidOperationException("Cannot create account request as instructor has already registered.");