diff --git a/build.gradle b/build.gradle index ab30b57dcf5..90abb909473 100644 --- a/build.gradle +++ b/build.gradle @@ -77,6 +77,7 @@ dependencies { implementation("org.jsoup:jsoup:1.15.2") implementation("org.hibernate.orm:hibernate-core:6.1.6.Final") implementation("org.postgresql:postgresql:42.7.2") + implementation("org.hibernate:hibernate-hikaricp:6.1.6.Final") testAnnotationProcessor(testng) diff --git a/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java b/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java index 70aa4ecc521..dcb1c279ae3 100644 --- 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("ring-bearer@fellowship.net", "The Fellowship of the Ring"); + HibernateUtil.commitTransaction(); assertEquals("ring-bearer@fellowship.net", accountRequest.getEmail()); assertEquals("Frodo Baggins", accountRequest.getName()); assertEquals("The Fellowship of the Ring", accountRequest.getInstitute()); diff --git a/src/main/java/teammates/common/util/HibernateUtil.java b/src/main/java/teammates/common/util/HibernateUtil.java index b23a4dcc95e..3410038727a 100644 --- a/src/main/java/teammates/common/util/HibernateUtil.java +++ b/src/main/java/teammates/common/util/HibernateUtil.java @@ -111,12 +111,18 @@ public static void buildSessionFactory(String dbUrl, String username, String pas Configuration config = new Configuration() .setProperty("hibernate.dialect", "org.hibernate.dialect.PostgreSQLDialect") .setProperty("hibernate.connection.driver_class", "org.postgresql.Driver") + .setProperty("hibernate.connection.provider_class", + "org.hibernate.hikaricp.internal.HikariCPConnectionProvider") .setProperty("hibernate.connection.username", username) .setProperty("hibernate.connection.password", password) .setProperty("hibernate.connection.url", dbUrl) .setProperty("hibernate.hbm2ddl.auto", "update") .setProperty("show_sql", "true") .setProperty("hibernate.current_session_context_class", "thread") + .setProperty("hibernate.hikari.minimumIdle", "10") + .setProperty("hibernate.hikari.maximumPoolSize", "30") + .setProperty("hibernate.hikari.idleTimeout", "300000") + .setProperty("hibernate.hikari.connectionTimeout", "30000") // Uncomment only during migration for optimized batch-insertion, batch-update, and batch-fetch. .setProperty("hibernate.jdbc.batch_size", "50") .setProperty("hibernate.order_updates", "true") diff --git a/src/main/java/teammates/logic/api/TaskQueuer.java b/src/main/java/teammates/logic/api/TaskQueuer.java index 32a5ba9e13f..a3db7d7d359 100644 --- 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 index 2401b871e68..140cfe0c34d 100644 --- 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 index a10fe1bae21..f83ff914ec7 100644 --- 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 index fd7742b3a73..f0797adf034 100644 --- 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 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 index 60b4f5fa3dc..f4ee00059d8 100644 --- 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 index 87e51ed74dc..ab32d4ed21d 100644 --- 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 index 27de0e8437b..f0f214a04f9 100644 --- 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."); diff --git a/src/web/data/developers.json b/src/web/data/developers.json index 0482ddedcdb..842e83e1356 100644 --- a/src/web/data/developers.json +++ b/src/web/data/developers.json @@ -215,6 +215,10 @@ "name": "Andy Daehn", "username": "andydaehn" }, + { + "name": "Andy", + "username": "Andy-W-Developer" + }, { "multiple": true, "name": "Ang Ji Kai", @@ -450,6 +454,11 @@ "name": "Chin Yong Wei", "username": "vertigogarden" }, + { + "multiple": true, + "name": "Ching Ming Yuan", + "username": "mingyuanc" + }, { "multiple": true, "name": "Chloe Stapleton", @@ -599,11 +608,6 @@ "name": "Dishant Sheth", "username": "dishant-sheth" }, - { - "multiple": true, - "name": "Yeo Di Sheng", - "username": "dishenggg" - }, { "multiple": true, "name": "Divya Pandilla", @@ -1219,6 +1223,10 @@ "name": "Leonardo Vitoriano", "username": "leonardomilv3" }, + { + "name": "Leyan Guan", + "username": "leyguan" + }, { "multiple": true, "name": "Lian Wenhui Florine" @@ -1337,11 +1345,6 @@ "name": "Marlon Calvo", "username": "marloncalvo" }, - { - "multiple": true, - "name": "Tye Jia Jun, Marques", - "username": "marquestye" - }, { "name": "Martin Larsson", "username": "leddy231" @@ -1370,6 +1373,7 @@ "username": "mattlim1207" }, { + "multiple": true, "name": "Maureen Chang", "username": "techMedMau" }, @@ -1412,10 +1416,6 @@ "name": "Miguel Araújo", "username": "miguelarauj1o" }, - { - "name": "Ching Ming Yuan", - "username": "mingyuanc" - }, { "name": "Minsung Joh", "username": "jms5049" @@ -1448,6 +1448,10 @@ "name": "Mukesh Gupta", "username": "mukesh14149" }, + { + "name": "Nada Ayesh", + "username": "nadasuhailAyesh12" + }, { "name": "Naga Rani", "username": "Nagureddy" @@ -2254,6 +2258,11 @@ "name": "Truong Hoang Phuoc", "username": "hoangphuoc25" }, + { + "multiple": true, + "name": "Tye Jia Jun, Marques", + "username": "marquestye" + }, { "username": "u6867511" }, @@ -2335,6 +2344,10 @@ "multiple": true, "name": "Wang Chao" }, + { + "name": "Wang JingTing", + "username": "jingting1412" + }, { "name": "Wang Yuqing", "username": "yuqingw" @@ -2356,6 +2369,7 @@ "username": "a0129998" }, { + "multiple": true, "name": "Xenos Fiorenzo Anong", "username": "xenosf" }, @@ -2412,6 +2426,11 @@ "multiple": true, "name": "Yen Zi Shyun" }, + { + "multiple": true, + "name": "Yeo Di Sheng", + "username": "dishenggg" + }, { "name": "Yi Chen", "username": "g3chenyigmailcom"