diff --git a/java/google/registry/ui/externs/json.js b/java/google/registry/ui/externs/json.js index 13d72b53f..b5624949a 100644 --- a/java/google/registry/ui/externs/json.js +++ b/java/google/registry/ui/externs/json.js @@ -78,6 +78,8 @@ registry.json.Response.prototype.results; * icannReferralEmail: string, * ipAddressWhitelist: !Array, * emailAddress: (string?|undefined), + * lastUpdateTime: string, + * url: (string?|undefined), * phonePasscode: (string?|undefined), * phoneNumber: (string?|undefined), * faxNumber: (string?|undefined), diff --git a/java/google/registry/ui/server/RegistrarFormFields.java b/java/google/registry/ui/server/RegistrarFormFields.java index 3d288c426..61fc412ad 100644 --- a/java/google/registry/ui/server/RegistrarFormFields.java +++ b/java/google/registry/ui/server/RegistrarFormFields.java @@ -37,7 +37,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Function; +import javax.annotation.Nullable; +import org.joda.time.DateTime; /** Form fields for validating input for the {@code Registrar} class. */ public final class RegistrarFormFields { @@ -46,31 +47,18 @@ public final class RegistrarFormFields { public static final Pattern ASCII_PATTERN = Pattern.compile("[[:ascii:]]*"); public static final String ASCII_ERROR = "Please only use ASCII-US characters."; - private static final Function CIDR_TRANSFORM = - input -> { - try { - return input != null ? CidrAddressBlock.create(input) : null; - } catch (IllegalArgumentException e) { - throw new FormFieldException("Not a valid CIDR notation IP-address block.", e); - } - }; - - private static final Function HOSTNAME_TRANSFORM = - input -> { - if (input == null) { - return null; - } - if (!InternetDomainName.isValid(input)) { - throw new FormFieldException("Not a valid hostname."); - } - return canonicalizeDomainName(input); - }; - public static final FormField NAME_FIELD = FormFields.NAME.asBuilderNamed("registrarName") .required() .build(); + public static final FormField LAST_UPDATE_TIME = + FormFields.LABEL + .asBuilderNamed("lastUpdateTime") + .transform(DateTime.class, RegistrarFormFields::parseDateTime) + .required() + .build(); + public static final FormField EMAIL_ADDRESS_FIELD_REQUIRED = FormFields.EMAIL.asBuilderNamed("emailAddress") .matches(ASCII_PATTERN, ASCII_ERROR) @@ -111,7 +99,7 @@ public final class RegistrarFormFields { public static final FormField WHOIS_SERVER_FIELD = FormFields.LABEL.asBuilderNamed("whoisServer") - .transform(HOSTNAME_TRANSFORM) + .transform(RegistrarFormFields::parseHostname) .build(); public static final FormField BLOCK_PREMIUM_NAMES_FIELD = @@ -173,7 +161,7 @@ public final class RegistrarFormFields { public static final FormField, List> IP_ADDRESS_WHITELIST_FIELD = FormField.named("ipAddressWhitelist") .emptyToNull() - .transform(CidrAddressBlock.class, CIDR_TRANSFORM) + .transform(CidrAddressBlock.class, RegistrarFormFields::parseCidr) .asList() .build(); @@ -228,34 +216,9 @@ public final class RegistrarFormFields { .asSet(Splitter.on(',').omitEmptyStrings().trimResults()) .build(); - public static final Function, RegistrarContact.Builder> - REGISTRAR_CONTACT_TRANSFORM = - args -> { - if (args == null) { - return null; - } - RegistrarContact.Builder builder = new RegistrarContact.Builder(); - CONTACT_NAME_FIELD.extractUntyped(args).ifPresent(builder::setName); - CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).ifPresent(builder::setEmailAddress); - CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInWhoisAsAdmin); - CONTACT_VISIBLE_IN_WHOIS_AS_TECH_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInWhoisAsTech); - PHONE_AND_EMAIL_VISIBLE_IN_DOMAIN_WHOIS_AS_ABUSE_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInDomainWhoisAsAbuse); - CONTACT_PHONE_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setPhoneNumber); - CONTACT_FAX_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setFaxNumber); - CONTACT_TYPES.extractUntyped(args).ifPresent(builder::setTypes); - CONTACT_GAE_USER_ID_FIELD.extractUntyped(args).ifPresent(builder::setGaeUserId); - return builder; - }; - public static final FormField>, List> CONTACTS_FIELD = FormField.mapNamed("contacts") - .transform(RegistrarContact.Builder.class, REGISTRAR_CONTACT_TRANSFORM) + .transform(RegistrarContact.Builder.class, RegistrarFormFields::toRegistrarContactBuilder) .asList() .build(); @@ -316,42 +279,94 @@ public final class RegistrarFormFields { public static final FormField, RegistrarAddress> L10N_ADDRESS_FIELD = FormField.mapNamed("localizedAddress") - .transform(RegistrarAddress.class, newAddressTransform( - L10N_STREET_FIELD, L10N_CITY_FIELD, L10N_STATE_FIELD, L10N_ZIP_FIELD)) + .transform(RegistrarAddress.class, (args) -> toNewAddress( + args, L10N_STREET_FIELD, L10N_CITY_FIELD, L10N_STATE_FIELD, L10N_ZIP_FIELD)) .build(); public static final FormField PREMIUM_PRICE_ACK_REQUIRED = FormField.named("premiumPriceAckRequired", Boolean.class).build(); - private static Function, RegistrarAddress> newAddressTransform( + private static @Nullable RegistrarAddress toNewAddress( + @Nullable Map args, final FormField, List> streetField, final FormField cityField, final FormField stateField, final FormField zipField) { - return args -> { - if (args == null) { - return null; - } - RegistrarAddress.Builder builder = new RegistrarAddress.Builder(); - String countryCode = COUNTRY_CODE_FIELD.extractUntyped(args).get(); - builder.setCountryCode(countryCode); - streetField - .extractUntyped(args) - .ifPresent(streets -> builder.setStreet(ImmutableList.copyOf(streets))); - cityField.extractUntyped(args).ifPresent(builder::setCity); - Optional stateFieldValue = stateField.extractUntyped(args); - if (stateFieldValue.isPresent()) { - String state = stateFieldValue.get(); - if ("US".equals(countryCode)) { - state = Ascii.toUpperCase(state); - if (!StateCode.US_MAP.containsKey(state)) { - throw new FormFieldException(stateField, "Unknown US state code."); - } + if (args == null) { + return null; + } + RegistrarAddress.Builder builder = new RegistrarAddress.Builder(); + String countryCode = COUNTRY_CODE_FIELD.extractUntyped(args).get(); + builder.setCountryCode(countryCode); + streetField + .extractUntyped(args) + .ifPresent(streets -> builder.setStreet(ImmutableList.copyOf(streets))); + cityField.extractUntyped(args).ifPresent(builder::setCity); + Optional stateFieldValue = stateField.extractUntyped(args); + if (stateFieldValue.isPresent()) { + String state = stateFieldValue.get(); + if ("US".equals(countryCode)) { + state = Ascii.toUpperCase(state); + if (!StateCode.US_MAP.containsKey(state)) { + throw new FormFieldException(stateField, "Unknown US state code."); } - builder.setState(state); } - zipField.extractUntyped(args).ifPresent(builder::setZip); - return builder.build(); - }; + builder.setState(state); + } + zipField.extractUntyped(args).ifPresent(builder::setZip); + return builder.build(); + } + + private static CidrAddressBlock parseCidr(String input) { + try { + return input != null ? CidrAddressBlock.create(input) : null; + } catch (IllegalArgumentException e) { + throw new FormFieldException("Not a valid CIDR notation IP-address block.", e); + } + } + + private static @Nullable String parseHostname(@Nullable String input) { + if (input == null) { + return null; + } + if (!InternetDomainName.isValid(input)) { + throw new FormFieldException("Not a valid hostname."); + } + return canonicalizeDomainName(input); + } + + public static @Nullable DateTime parseDateTime(@Nullable String input) { + if (input == null) { + return null; + } + try { + return DateTime.parse(input); + } catch (IllegalArgumentException e) { + throw new FormFieldException("Not a valid ISO date-time string.", e); + } + } + + private static @Nullable RegistrarContact.Builder toRegistrarContactBuilder( + @Nullable Map args) { + if (args == null) { + return null; + } + RegistrarContact.Builder builder = new RegistrarContact.Builder(); + CONTACT_NAME_FIELD.extractUntyped(args).ifPresent(builder::setName); + CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).ifPresent(builder::setEmailAddress); + CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD + .extractUntyped(args) + .ifPresent(builder::setVisibleInWhoisAsAdmin); + CONTACT_VISIBLE_IN_WHOIS_AS_TECH_FIELD + .extractUntyped(args) + .ifPresent(builder::setVisibleInWhoisAsTech); + PHONE_AND_EMAIL_VISIBLE_IN_DOMAIN_WHOIS_AS_ABUSE_FIELD + .extractUntyped(args) + .ifPresent(builder::setVisibleInDomainWhoisAsAbuse); + CONTACT_PHONE_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setPhoneNumber); + CONTACT_FAX_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setFaxNumber); + CONTACT_TYPES.extractUntyped(args).ifPresent(builder::setTypes); + CONTACT_GAE_USER_ID_FIELD.extractUntyped(args).ifPresent(builder::setGaeUserId); + return builder; } } diff --git a/java/google/registry/ui/server/registrar/BUILD b/java/google/registry/ui/server/registrar/BUILD index 897085ea8..3384d5598 100644 --- a/java/google/registry/ui/server/registrar/BUILD +++ b/java/google/registry/ui/server/registrar/BUILD @@ -34,6 +34,7 @@ java_library( "@com_google_re2j", "@io_bazel_rules_closure//closure/templates", "@javax_servlet_api", + "@joda_time", "@org_joda_money", ], ) diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index f85c94853..89687d337 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -27,7 +27,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.collect.Streams; -import com.googlecode.objectify.Work; import google.registry.config.RegistryConfig.Config; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.model.registrar.Registrar; @@ -55,6 +54,7 @@ import java.util.Set; import java.util.function.Predicate; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; +import org.joda.time.DateTime; /** * Admin servlet that allows creating or updating a registrar. Deletes are not allowed so as to @@ -112,7 +112,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA try { switch (op) { case "update": - return update(args, initialRegistrar); + return update(args, initialRegistrar.getClientId()); case "read": return JsonResponseHelper.create(SUCCESS, "Success", initialRegistrar.toJsonMap()); default: @@ -135,41 +135,66 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA args); return JsonResponseHelper.create(ERROR, e.getMessage()); } - } - Map update(final Map args, final Registrar registrar) { - final String clientId = sessionUtils.getRegistrarClientId(request); + Map update(final Map args, String clientId) { return ofy() .transact( - (Work>) - () -> { - ImmutableSet oldContacts = registrar.getContacts(); - Map existingRegistrarMap = - expandRegistrarWithContacts(oldContacts, registrar); - Registrar.Builder builder = registrar.asBuilder(); - ImmutableSet updatedContacts = - changeRegistrarFields(registrar, builder, args); - if (!updatedContacts.isEmpty()) { - builder.setContactsRequireSyncing(true); - } - Registrar updatedRegistrar = builder.build(); - ofy().save().entity(updatedRegistrar); - if (!updatedContacts.isEmpty()) { - checkContactRequirements(oldContacts, updatedContacts); - RegistrarContact.updateContacts(updatedRegistrar, updatedContacts); - } - // Update the registrar map with updated contacts to bypass Objectify caching - // issues that come into play with calling getContacts(). - Map updatedRegistrarMap = - expandRegistrarWithContacts(updatedContacts, updatedRegistrar); - sendExternalUpdatesIfNecessary( - updatedRegistrar.getRegistrarName(), - existingRegistrarMap, - updatedRegistrarMap); - return JsonResponseHelper.create( - SUCCESS, "Saved " + clientId, updatedRegistrar.toJsonMap()); - }); + () -> { + // We load the registrar here rather than use the initialRegistrar above - to make + // sure we have the latest version. This one is loaded inside the transaction, so it's + // guaranteed to not change before we update it. + Registrar registrar = Registrar.loadByClientId(clientId).get(); + // Verify that the registrar hasn't been changed. + // To do that - we find the latest update time (or null if the registrar has been + // deleted) and compare to the update time from the args. The update time in the args + // comes from the read that gave the UI the data - if it's out of date, then the UI + // had out of date data. + DateTime latest = registrar.getLastUpdateTime(); + DateTime latestFromArgs = + RegistrarFormFields.LAST_UPDATE_TIME.extractUntyped(args).get(); + if (!latestFromArgs.equals(latest)) { + logger.warningfmt( + "registrar changed since reading the data! " + + " Last updated at %s, but args data last updated at %s", + latest, latestFromArgs); + return JsonResponseHelper.create( + ERROR, "registrar has been changed by someone else. Please reload and retry."); + } + + // Keep the current contacts so we can later check that no required contact was + // removed, email the changes to the contacts + ImmutableSet contacts = registrar.getContacts(); + + // Update the registrar from the request. + Registrar.Builder builder = registrar.asBuilder(); + changeRegistrarFields(registrar, builder, args); + + // read the contacts from the request. + ImmutableSet updatedContacts = + readContacts(registrar, args); + if (!updatedContacts.isEmpty()) { + builder.setContactsRequireSyncing(true); + } + + // Save the updated registrar + Registrar updatedRegistrar = builder.build(); + if (!updatedRegistrar.equals(registrar)) { + ofy().save().entity(updatedRegistrar); + } + + // Save the updated contacts + if (!updatedContacts.isEmpty()) { + checkContactRequirements(contacts, updatedContacts); + RegistrarContact.updateContacts(updatedRegistrar, updatedContacts); + } + + // Email and return update. + sendExternalUpdatesIfNecessary( + registrar, contacts, updatedRegistrar, updatedContacts); + return JsonResponseHelper.create( + SUCCESS, "Saved " + clientId, updatedRegistrar.toJsonMap()); + }); } private Map expandRegistrarWithContacts(Iterable contacts, @@ -186,12 +211,16 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } /** - * Updates a registrar builder with the supplied args from the http request, and returns a list of - * the new registrar contacts. + * Updates a registrar builder with the supplied args from the http request; */ - public static ImmutableSet changeRegistrarFields( + public static void changeRegistrarFields( Registrar existingRegistrarObj, Registrar.Builder builder, Map args) { + // BILLING + RegistrarFormFields.PREMIUM_PRICE_ACK_REQUIRED + .extractUntyped(args) + .ifPresent(builder::setPremiumPriceAckRequired); + // WHOIS builder.setWhoisServer( RegistrarFormFields.WHOIS_SERVER_FIELD.extractUntyped(args).orElse(null)); @@ -212,6 +241,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA builder.setLocalizedAddress( RegistrarFormFields.L10N_ADDRESS_FIELD.extractUntyped(args).orElse(null)); + builder.setUrl(RegistrarFormFields.URL_FIELD.extractUntyped(args).orElse(null)); + // Security builder.setIpAddressWhitelist( RegistrarFormFields.IP_ADDRESS_WHITELIST_FIELD @@ -226,17 +257,16 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA .ifPresent( certificate -> builder.setFailoverClientCertificate(certificate, ofy().getTransactionTime())); + } - builder.setUrl( - RegistrarFormFields.URL_FIELD.extractUntyped(args).orElse(null)); - builder.setReferralUrl( - RegistrarFormFields.REFERRAL_URL_FIELD.extractUntyped(args).orElse(null)); + /** Reads the contacts from the supplied args. */ + public static ImmutableSet readContacts( + Registrar registrar, Map args) { - // Contact ImmutableSet.Builder contacts = new ImmutableSet.Builder<>(); Optional> builders = RegistrarFormFields.CONTACTS_FIELD.extractUntyped(args); if (builders.isPresent()) { - builders.get().forEach(c -> contacts.add(c.setParent(existingRegistrarObj).build())); + builders.get().forEach(c -> contacts.add(c.setParent(registrar).build())); } return contacts.build(); @@ -332,10 +362,19 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA * enqueues a task to re-sync the registrar sheet. */ private void sendExternalUpdatesIfNecessary( - String registrarName, - Map existingRegistrar, - Map updatedRegistrar) { - Map diffs = DiffUtils.deepDiff(existingRegistrar, updatedRegistrar, true); + Registrar existingRegistrar, + ImmutableSet existingContacts, + Registrar updatedRegistrar, + ImmutableSet updatedContacts) { + if (registrarChangesNotificationEmailAddresses.isEmpty()) { + return; + } + + Map diffs = + DiffUtils.deepDiff( + expandRegistrarWithContacts(existingContacts, existingRegistrar), + expandRegistrarWithContacts(updatedContacts, updatedRegistrar), + true); @SuppressWarnings("unchecked") Set changedKeys = (Set) diffs.keySet(); if (CollectionUtils.difference(changedKeys, "lastUpdateTime").isEmpty()) { @@ -345,7 +384,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA if (!registrarChangesNotificationEmailAddresses.isEmpty()) { sendEmailUtils.sendEmail( registrarChangesNotificationEmailAddresses, - String.format("Registrar %s updated", registrarName), + String.format("Registrar %s updated", existingRegistrar.getRegistrarName()), "The following changes were made to the registrar:\n" + DiffUtils.prettyPrintDiffedMap(diffs, null)); } diff --git a/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java b/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java index 0eaf8c40a..12c965279 100644 --- a/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java +++ b/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java @@ -113,7 +113,8 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { .setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN, RegistrarContact.Type.TECH)) .build(); // Lest we anger the timestamp inversion bug. - persistResource(registrar); + // (we also update the registrar so we get the timestamp right) + registrar = persistResource(registrar); persistSimpleResource(rc); // Now try to remove the phone number. @@ -138,7 +139,8 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { .setVisibleInDomainWhoisAsAbuse(true) .build(); // Lest we anger the timestamp inversion bug. - persistResource(registrar); + // (we also update the registrar so we get the timestamp right) + registrar = persistResource(registrar); persistSimpleResource(rc); // Now try to remove the contact. @@ -164,7 +166,8 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { .setVisibleInDomainWhoisAsAbuse(true) .build(); // Lest we anger the timestamp inversion bug. - persistResource(registrar); + // (we also update the registrar so we get the timestamp right) + registrar = persistResource(registrar); persistSimpleResource(rc); // Now try to set the phone number to null. diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 48f453804..4fce2b8b6 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -36,6 +36,8 @@ import google.registry.testing.TaskQueueHelper.TaskMatcher; import java.util.Map; import javax.mail.internet.InternetAddress; import javax.servlet.http.HttpServletRequest; +import org.json.simple.JSONValue; +import org.json.simple.parser.ParseException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -47,7 +49,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase @Test public void testSuccess_updateRegistrarInfo_andSendsNotificationEmail() throws Exception { String expectedEmailBody = loadFile(getClass(), "update_registrar_email.txt"); - action.handleJsonRequest(readJsonFromFile("update_registrar.json")); + action.handleJsonRequest(readJsonFromFile("update_registrar.json", getLastUpdateTime())); verify(rsp, never()).setStatus(anyInt()); verify(emailService).createMessage(); verify(emailService).sendMessage(message); @@ -64,7 +66,8 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase @Test public void testFailure_updateRegistrarInfo_duplicateContacts() throws Exception { Map response = - action.handleJsonRequest(readJsonFromFile("update_registrar_duplicate_contacts.json")); + action.handleJsonRequest( + readJsonFromFile("update_registrar_duplicate_contacts.json", getLastUpdateTime())); assertThat(response).containsEntry("status", "ERROR"); assertThat((String) response.get("message")).startsWith("One email address"); } @@ -89,11 +92,22 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase } @Test - public void testUpdate_emptyJsonObject_errorEmailFieldRequired() throws Exception { + public void testUpdate_emptyJsonObject_errorLastUpdateTimeFieldRequired() throws Exception { Map response = action.handleJsonRequest(ImmutableMap.of( "op", "update", "args", ImmutableMap.of())); assertThat(response).containsEntry("status", "ERROR"); + assertThat(response).containsEntry("field", "lastUpdateTime"); + assertThat(response).containsEntry("message", "This field is required."); + assertNoTasksEnqueued("sheet"); + } + + @Test + public void testUpdate_noEmail_errorEmailFieldRequired() throws Exception { + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime()))); + assertThat(response).containsEntry("status", "ERROR"); assertThat(response).containsEntry("field", "emailAddress"); assertThat(response).containsEntry("message", "This field is required."); assertNoTasksEnqueued("sheet"); @@ -105,7 +119,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase Map response = action.handleJsonRequest(ImmutableMap.of( "op", "update", - "args", ImmutableMap.of())); + "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime()))); assertThat(response).containsEntry("status", "SUCCESS"); } @@ -113,23 +127,53 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase public void testUpdate_badEmail_errorEmailField() throws Exception { Map response = action.handleJsonRequest(ImmutableMap.of( "op", "update", - "args", ImmutableMap.of( - "emailAddress", "lolcat"))); + "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime(), "emailAddress", "lolcat"))); assertThat(response).containsEntry("status", "ERROR"); assertThat(response).containsEntry("field", "emailAddress"); assertThat(response).containsEntry("message", "Please enter a valid email address."); assertNoTasksEnqueued("sheet"); } + @Test + public void testPost_nonParsableTime_getsAngry() throws Exception { + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "args", ImmutableMap.of("lastUpdateTime", "cookies"))); + assertThat(response).containsEntry("status", "ERROR"); + assertThat(response).containsEntry("field", "lastUpdateTime"); + assertThat(response).containsEntry("message", "Not a valid ISO date-time string."); + assertNoTasksEnqueued("sheet"); + } + @Test public void testPost_nonAsciiCharacters_getsAngry() throws Exception { Map response = action.handleJsonRequest(ImmutableMap.of( "op", "update", "args", ImmutableMap.of( + "lastUpdateTime", getLastUpdateTime(), "emailAddress", "ヘ(◕。◕ヘ)@example.com"))); assertThat(response).containsEntry("status", "ERROR"); assertThat(response).containsEntry("field", "emailAddress"); assertThat(response).containsEntry("message", "Please only use ASCII-US characters."); assertNoTasksEnqueued("sheet"); } + + private static String getLastUpdateTime() { + return loadRegistrar(CLIENT_ID).getLastUpdateTime().toString(); + } + + static Map readJsonFromFile(String filename, String lastUpdateTime) { + String contents = + loadFile( + RegistrarSettingsActionTestCase.class, + filename, + ImmutableMap.of("LAST_UPDATE_TIME", lastUpdateTime)); + try { + @SuppressWarnings("unchecked") + Map json = (Map) JSONValue.parseWithException(contents); + return json; + } catch (ParseException ex) { + throw new RuntimeException(ex); + } + } } diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index 9fb60d424..f0e2ed244 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -19,7 +19,6 @@ import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDispla import static google.registry.security.JsonHttpTestUtils.createJsonPayload; import static google.registry.security.JsonHttpTestUtils.createJsonResponseSupplier; import static google.registry.testing.DatastoreHelper.loadRegistrar; -import static google.registry.testing.TestDataHelper.loadFile; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,8 +49,6 @@ import javax.mail.internet.MimeMessage; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.joda.time.DateTime; -import org.json.simple.JSONValue; -import org.json.simple.parser.ParseException; import org.junit.Before; import org.junit.Rule; import org.junit.runner.RunWith; @@ -114,15 +111,4 @@ public class RegistrarSettingsActionTestCase { .thenAnswer(x -> loadRegistrar(CLIENT_ID)); when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname"); } - - static Map readJsonFromFile(String filename) { - String contents = loadFile(RegistrarSettingsActionTestCase.class, filename); - try { - @SuppressWarnings("unchecked") - Map json = (Map) JSONValue.parseWithException(contents); - return json; - } catch (ParseException ex) { - throw new RuntimeException(ex); - } - } } diff --git a/javatests/google/registry/ui/server/registrar/testdata/update_registrar.json b/javatests/google/registry/ui/server/registrar/testdata/update_registrar.json index 9dc90420a..b15ad1d2a 100644 --- a/javatests/google/registry/ui/server/registrar/testdata/update_registrar.json +++ b/javatests/google/registry/ui/server/registrar/testdata/update_registrar.json @@ -4,7 +4,7 @@ "clientIdentifier": "theregistrar", "driveFolderId": null, "registrarName": "The Registrar", - "lastUpdateTime": "2015-01-22T17:27:36.999Z", + "lastUpdateTime": "%LAST_UPDATE_TIME%", "state": "ACTIVE", "type": "REAL", "contacts": [ diff --git a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_duplicate_contacts.json b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_duplicate_contacts.json index 12e13f466..228eaf39a 100644 --- a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_duplicate_contacts.json +++ b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_duplicate_contacts.json @@ -4,7 +4,7 @@ "clientIdentifier": "theregistrar", "driveFolderId": null, "registrarName": "The Registrar", - "lastUpdateTime": "2015-01-22T17:27:36.999Z", + "lastUpdateTime": "%LAST_UPDATE_TIME%", "state": "ACTIVE", "type": "REAL", "contacts": [