Skip to content

Commit

Permalink
Merge branch 'master' into v9-non-course-migration
Browse files Browse the repository at this point in the history
  • Loading branch information
NicolasCwy committed Apr 3, 2024
2 parents 217ebc6 + 38dbf29 commit 57bc285
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 27 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand All @@ -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());
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/teammates/common/util/HibernateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/teammates/logic/api/TaskQueuer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/teammates/logic/api/UserProvision.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/teammates/sqllogic/api/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
17 changes: 16 additions & 1 deletion src/main/java/teammates/ui/servlets/WebApiServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/teammates/ui/webapi/Action.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -13,6 +12,11 @@
*/
public class CreateAccountRequestAction extends AdminOnlyAction {

@Override
public boolean isTransactionNeeded() {
return false;
}

@Override
public JsonResult execute()
throws InvalidHttpRequestBodyException, InvalidOperationException {
Expand All @@ -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.");
Expand Down
47 changes: 33 additions & 14 deletions src/web/data/developers.json
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@
"name": "Andy Daehn",
"username": "andydaehn"
},
{
"name": "Andy",
"username": "Andy-W-Developer"
},
{
"multiple": true,
"name": "Ang Ji Kai",
Expand Down Expand Up @@ -450,6 +454,11 @@
"name": "Chin Yong Wei",
"username": "vertigogarden"
},
{
"multiple": true,
"name": "Ching Ming Yuan",
"username": "mingyuanc"
},
{
"multiple": true,
"name": "Chloe Stapleton",
Expand Down Expand Up @@ -599,11 +608,6 @@
"name": "Dishant Sheth",
"username": "dishant-sheth"
},
{
"multiple": true,
"name": "Yeo Di Sheng",
"username": "dishenggg"
},
{
"multiple": true,
"name": "Divya Pandilla",
Expand Down Expand Up @@ -1219,6 +1223,10 @@
"name": "Leonardo Vitoriano",
"username": "leonardomilv3"
},
{
"name": "Leyan Guan",
"username": "leyguan"
},
{
"multiple": true,
"name": "Lian Wenhui Florine"
Expand Down Expand Up @@ -1337,11 +1345,6 @@
"name": "Marlon Calvo",
"username": "marloncalvo"
},
{
"multiple": true,
"name": "Tye Jia Jun, Marques",
"username": "marquestye"
},
{
"name": "Martin Larsson",
"username": "leddy231"
Expand Down Expand Up @@ -1370,6 +1373,7 @@
"username": "mattlim1207"
},
{
"multiple": true,
"name": "Maureen Chang",
"username": "techMedMau"
},
Expand Down Expand Up @@ -1412,10 +1416,6 @@
"name": "Miguel Araújo",
"username": "miguelarauj1o"
},
{
"name": "Ching Ming Yuan",
"username": "mingyuanc"
},
{
"name": "Minsung Joh",
"username": "jms5049"
Expand Down Expand Up @@ -1448,6 +1448,10 @@
"name": "Mukesh Gupta",
"username": "mukesh14149"
},
{
"name": "Nada Ayesh",
"username": "nadasuhailAyesh12"
},
{
"name": "Naga Rani",
"username": "Nagureddy"
Expand Down Expand Up @@ -2254,6 +2258,11 @@
"name": "Truong Hoang Phuoc",
"username": "hoangphuoc25"
},
{
"multiple": true,
"name": "Tye Jia Jun, Marques",
"username": "marquestye"
},
{
"username": "u6867511"
},
Expand Down Expand Up @@ -2335,6 +2344,10 @@
"multiple": true,
"name": "Wang Chao"
},
{
"name": "Wang JingTing",
"username": "jingting1412"
},
{
"name": "Wang Yuqing",
"username": "yuqingw"
Expand All @@ -2356,6 +2369,7 @@
"username": "a0129998"
},
{
"multiple": true,
"name": "Xenos Fiorenzo Anong",
"username": "xenosf"
},
Expand Down Expand Up @@ -2412,6 +2426,11 @@
"multiple": true,
"name": "Yen Zi Shyun"
},
{
"multiple": true,
"name": "Yeo Di Sheng",
"username": "dishenggg"
},
{
"name": "Yi Chen",
"username": "g3chenyigmailcom"
Expand Down

0 comments on commit 57bc285

Please sign in to comment.