Remove Registrar caching from all console actions

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
This commit is contained in:
mmuller 2018-03-27 12:02:50 -07:00 committed by jianglai
parent 6dec95b980
commit e1ad4d663c
10 changed files with 12 additions and 18 deletions

View file

@ -157,7 +157,7 @@ public final class RegistrarPaymentAction implements Runnable, JsonAction {
@Override @Override
public Map<String, Object> handleJsonRequest(Map<String, ?> json) { 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); logger.infofmt("Processing payment: %s", json);
String paymentMethodNonce; String paymentMethodNonce;
Money amount; Money amount;

View file

@ -91,7 +91,7 @@ public final class RegistrarPaymentSetupAction implements Runnable, JsonAction {
@Override @Override
public Map<String, Object> handleJsonRequest(Map<String, ?> json) { public Map<String, Object> handleJsonRequest(Map<String, ?> json) {
Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult, false); Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult);
if (!json.isEmpty()) { if (!json.isEmpty()) {
return JsonResponseHelper.create(ERROR, "JSON request object must be empty"); return JsonResponseHelper.create(ERROR, "JSON request object must be empty");

View file

@ -81,7 +81,7 @@ public class RegistrarPremiumPriceAckAction implements Runnable, JsonActionRunne
} }
// Get the registrar // 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 // Process the operation. Though originally derived from a CRUD handler, registrar-settings
// and registrar-premium-price-action really only support read and update. // and registrar-premium-price-action really only support read and update.

View file

@ -93,7 +93,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
throw new BadRequestException("Malformed JSON"); 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 // Process the operation. Though originally derived from a CRUD
// handler, registrar-settings really only supports read and update. // handler, registrar-settings really only supports read and update.
String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read"); String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read");

View file

@ -59,8 +59,7 @@ public class SessionUtils {
* the registrar console. * the registrar console.
*/ */
@CheckReturnValue @CheckReturnValue
Registrar getRegistrarForAuthResult( Registrar getRegistrarForAuthResult(HttpServletRequest request, AuthResult authResult) {
HttpServletRequest request, AuthResult authResult, boolean bypassCache) {
if (!authResult.userAuthInfo().isPresent()) { if (!authResult.userAuthInfo().isPresent()) {
throw new ForbiddenException("Not logged in"); throw new ForbiddenException("Not logged in");
} }
@ -69,9 +68,7 @@ public class SessionUtils {
} }
String clientId = getRegistrarClientId(request); String clientId = getRegistrarClientId(request);
return checkArgumentPresent( return checkArgumentPresent(
bypassCache Registrar.loadByClientId(clientId),
? Registrar.loadByClientId(clientId)
: Registrar.loadByClientIdCached(clientId),
"Registrar %s not found", "Registrar %s not found",
clientId); clientId);
} }

View file

@ -19,7 +19,6 @@ import static google.registry.testing.DatastoreHelper.loadRegistrar;
import static google.registry.testing.ReflectiveFieldExtractor.extractField; import static google.registry.testing.ReflectiveFieldExtractor.extractField;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@ -94,7 +93,7 @@ public class RegistrarPaymentActionTest {
when(braintreeGateway.transaction()).thenReturn(transactionGateway); when(braintreeGateway.transaction()).thenReturn(transactionGateway);
when(transactionGateway.sale(any(TransactionRequest.class))).thenReturn(result); when(transactionGateway.sale(any(TransactionRequest.class))).thenReturn(result);
when(sessionUtils.getRegistrarForAuthResult( when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean())) any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(loadRegistrar("TheRegistrar")); .thenReturn(loadRegistrar("TheRegistrar"));
} }

View file

@ -19,7 +19,6 @@ import static google.registry.testing.DatastoreHelper.loadRegistrar;
import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DatastoreHelper.persistResource;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.eq; import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@ -69,7 +68,7 @@ public class RegistrarPaymentSetupActionTest {
.setBillingMethod(Registrar.BillingMethod.BRAINTREE) .setBillingMethod(Registrar.BillingMethod.BRAINTREE)
.build()); .build());
when(sessionUtils.getRegistrarForAuthResult( when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean())) any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(registrar); .thenReturn(registrar);
when(braintreeGateway.clientToken()).thenReturn(clientTokenGateway); when(braintreeGateway.clientToken()).thenReturn(clientTokenGateway);
} }
@ -112,7 +111,7 @@ public class RegistrarPaymentSetupActionTest {
.setBillingMethod(Registrar.BillingMethod.EXTERNAL) .setBillingMethod(Registrar.BillingMethod.EXTERNAL)
.build()); .build());
when(sessionUtils.getRegistrarForAuthResult( when(sessionUtils.getRegistrarForAuthResult(
any(HttpServletRequest.class), any(AuthResult.class), anyBoolean())) any(HttpServletRequest.class), any(AuthResult.class)))
.thenReturn(registrar); .thenReturn(registrar);
assertThat(action.handleJsonRequest(ImmutableMap.of())) assertThat(action.handleJsonRequest(ImmutableMap.of()))
.containsExactly( .containsExactly(

View file

@ -22,7 +22,6 @@ import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
import static google.registry.util.ResourceUtils.readResourceUtf8; import static google.registry.util.ResourceUtils.readResourceUtf8;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyInt;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@ -72,7 +71,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
@Test @Test
public void testRead_notAuthorized_failure() throws Exception { public void testRead_notAuthorized_failure() throws Exception {
when(sessionUtils.getRegistrarForAuthResult( 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")); .thenThrow(new ForbiddenException("Not authorized to access Registrar Console"));
assertThrows(ForbiddenException.class, () -> action.handleJsonRequest(ImmutableMap.of())); assertThrows(ForbiddenException.class, () -> action.handleJsonRequest(ImmutableMap.of()));
assertNoTasksEnqueued("sheet"); assertNoTasksEnqueued("sheet");

View file

@ -106,7 +106,7 @@ public class RegistrarSettingsActionTestCase {
when(rsp.getWriter()).thenReturn(new PrintWriter(writer)); when(rsp.getWriter()).thenReturn(new PrintWriter(writer));
when(req.getContentType()).thenReturn("application/json"); when(req.getContentType()).thenReturn("application/json");
when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); 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)); .thenReturn(loadRegistrar(CLIENT_ID));
when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname"); when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname");
} }

View file

@ -112,7 +112,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
.setClientCertificate(SAMPLE_CERT, START_OF_TIME) .setClientCertificate(SAMPLE_CERT, START_OF_TIME)
.setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME) .setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME)
.build()); .build());
when(sessionUtils.getRegistrarForAuthResult(req, action.authResult, false)) when(sessionUtils.getRegistrarForAuthResult(req, action.authResult))
.thenReturn(initialRegistrar); .thenReturn(initialRegistrar);
Map<String, Object> jsonMap = initialRegistrar.toJsonMap(); Map<String, Object> jsonMap = initialRegistrar.toJsonMap();
jsonMap.put("clientCertificate", null); jsonMap.put("clientCertificate", null);