Skip to content

Commit

Permalink
Remove Registrar caching from all console actions
Browse files Browse the repository at this point in the history
Caching turns out to be an anti-pattern for the console.  If we use it, changes from the user just get obliterated by the older, cached version the next time the console refreshes (and it happens to refresh after every update).  Caching is also not very useful here, as the amount of database access driven by the console is very small.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=190650931
  • Loading branch information
mindhog authored and jianglai committed Apr 2, 2018
1 parent 6dec95b commit e1ad4d6
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 18 deletions.
Expand Up @@ -157,7 +157,7 @@ public void run() {

@Override
public Map<String, Object> handleJsonRequest(Map<String, ?> json) {
Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult, false);
Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult);
logger.infofmt("Processing payment: %s", json);
String paymentMethodNonce;
Money amount;
Expand Down
Expand Up @@ -91,7 +91,7 @@ public void run() {

@Override
public Map<String, Object> handleJsonRequest(Map<String, ?> json) {
Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult, false);
Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult);

if (!json.isEmpty()) {
return JsonResponseHelper.create(ERROR, "JSON request object must be empty");
Expand Down
Expand Up @@ -81,7 +81,7 @@ public Map<String, Object> handleJsonRequest(Map<String, ?> input) {
}

// Get the registrar
Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult, true);
Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult);

// Process the operation. Though originally derived from a CRUD handler, registrar-settings
// and registrar-premium-price-action really only support read and update.
Expand Down
Expand Up @@ -93,7 +93,7 @@ public Map<String, Object> handleJsonRequest(Map<String, ?> input) {
throw new BadRequestException("Malformed JSON");
}

Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult, false);
Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult);
// Process the operation. Though originally derived from a CRUD
// handler, registrar-settings really only supports read and update.
String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read");
Expand Down
7 changes: 2 additions & 5 deletions java/google/registry/ui/server/registrar/SessionUtils.java
Expand Up @@ -59,8 +59,7 @@ public SessionUtils() {}
* the registrar console.
*/
@CheckReturnValue
Registrar getRegistrarForAuthResult(
HttpServletRequest request, AuthResult authResult, boolean bypassCache) {
Registrar getRegistrarForAuthResult(HttpServletRequest request, AuthResult authResult) {
if (!authResult.userAuthInfo().isPresent()) {
throw new ForbiddenException("Not logged in");
}
Expand All @@ -69,9 +68,7 @@ Registrar getRegistrarForAuthResult(
}
String clientId = getRegistrarClientId(request);
return checkArgumentPresent(
bypassCache
? Registrar.loadByClientId(clientId)
: Registrar.loadByClientIdCached(clientId),
Registrar.loadByClientId(clientId),
"Registrar %s not found",
clientId);
}
Expand Down
Expand Up @@ -19,7 +19,6 @@
import static google.registry.testing.ReflectiveFieldExtractor.extractField;
import static java.util.Arrays.asList;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -94,7 +93,7 @@ public void before() throws Exception {
when(braintreeGateway.transaction()).thenReturn(transactionGateway);
when(transactionGateway.sale(any(TransactionRequest.class))).thenReturn(result);
when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean()))
any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(loadRegistrar("TheRegistrar"));
}

Expand Down
Expand Up @@ -19,7 +19,6 @@
import static google.registry.testing.DatastoreHelper.persistResource;
import static java.util.Arrays.asList;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -69,7 +68,7 @@ public void before() throws Exception {
.setBillingMethod(Registrar.BillingMethod.BRAINTREE)
.build());
when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean()))
any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(registrar);
when(braintreeGateway.clientToken()).thenReturn(clientTokenGateway);
}
Expand Down Expand Up @@ -112,7 +111,7 @@ public void testNotOnCreditCardBillingTerms_showsErrorPage() throws Exception {
.setBillingMethod(Registrar.BillingMethod.EXTERNAL)
.build());
when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean()))
any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(registrar);
assertThat(action.handleJsonRequest(ImmutableMap.of()))
.containsExactly(
Expand Down
Expand Up @@ -22,7 +22,6 @@
import static google.registry.util.ResourceUtils.readResourceUtf8;
import static java.util.Arrays.asList;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -72,7 +71,7 @@ public void testFailure_updateRegistrarInfo_duplicateContacts() throws Exception
@Test
public void testRead_notAuthorized_failure() throws Exception {
when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean()))
any(HttpServletRequest.class), any(AuthResult.class)))
.thenThrow(new ForbiddenException("Not authorized to access Registrar Console"));
assertThrows(ForbiddenException.class, () -> action.handleJsonRequest(ImmutableMap.of()));
assertNoTasksEnqueued("sheet");
Expand Down
Expand Up @@ -106,7 +106,7 @@ public void setUp() throws Exception {
when(rsp.getWriter()).thenReturn(new PrintWriter(writer));
when(req.getContentType()).thenReturn("application/json");
when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read")));
when(sessionUtils.getRegistrarForAuthResult(req, action.authResult, false))
when(sessionUtils.getRegistrarForAuthResult(req, action.authResult))
.thenReturn(loadRegistrar(CLIENT_ID));
when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname");
}
Expand Down
Expand Up @@ -112,7 +112,7 @@ public void testEmptyOrNullCertificate_doesNotClearOutCurrentOne() throws Except
.setClientCertificate(SAMPLE_CERT, START_OF_TIME)
.setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME)
.build());
when(sessionUtils.getRegistrarForAuthResult(req, action.authResult, false))
when(sessionUtils.getRegistrarForAuthResult(req, action.authResult))
.thenReturn(initialRegistrar);
Map<String, Object> jsonMap = initialRegistrar.toJsonMap();
jsonMap.put("clientCertificate", null);
Expand Down

0 comments on commit e1ad4d6

Please sign in to comment.