Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#12048] Fix account request indexing #12967

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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("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());
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
Loading