diff --git a/gradle/core/build.gradle b/gradle/core/build.gradle index 9b36cf148..bdf2e17cd 100644 --- a/gradle/core/build.gradle +++ b/gradle/core/build.gradle @@ -125,7 +125,9 @@ dependencies { compile deps['com.google.apis:google-api-services-monitoring'] compile deps['com.google.apis:google-api-services-sheets'] maybeRuntime deps['com.google.apis:google-api-services-storage'] - testCompileOnly deps['com.google.appengine:appengine-api-1.0-sdk'] + // TODO(b/71631624): change appengine:appengine-api-1.0-sdk to + // testCompileOnly after BillingEmailUtilsTest.java is fixed. + compile deps['com.google.appengine:appengine-api-1.0-sdk'] maybeRuntime deps['com.google.appengine:appengine-api-labs'] maybeRuntime deps['com.google.appengine:appengine-api-stubs'] testCompile deps['com.google.appengine:appengine-api-stubs'] diff --git a/gradle/core/gradle/dependency-locks/testCompileOnly.lockfile b/gradle/core/gradle/dependency-locks/testCompileOnly.lockfile index b308d7773..656c5dbcc 100644 --- a/gradle/core/gradle/dependency-locks/testCompileOnly.lockfile +++ b/gradle/core/gradle/dependency-locks/testCompileOnly.lockfile @@ -1,4 +1,3 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.appengine:appengine-api-1.0-sdk:1.9.48 diff --git a/java/google/registry/model/registrar/Registrar.java b/java/google/registry/model/registrar/Registrar.java index cfa24ac60..019ccd549 100644 --- a/java/google/registry/model/registrar/Registrar.java +++ b/java/google/registry/model/registrar/Registrar.java @@ -16,7 +16,6 @@ package google.registry.model.registrar; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.ImmutableMap.toImmutableMap; @@ -85,8 +84,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import javax.annotation.Nullable; -import javax.mail.internet.AddressException; -import javax.mail.internet.InternetAddress; import org.joda.money.CurrencyUnit; import org.joda.time.DateTime; @@ -831,7 +828,7 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable } public Builder setEmailAddress(String emailAddress) { - getInstance().emailAddress = checkValidEmail(emailAddress); + getInstance().emailAddress = emailAddress; return this; } @@ -851,7 +848,7 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable } public Builder setIcannReferralEmail(String icannReferralEmail) { - getInstance().icannReferralEmail = checkValidEmail(icannReferralEmail); + getInstance().icannReferralEmail = icannReferralEmail; return this; } @@ -894,20 +891,6 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable } } - /** Verifies that the email address in question is not null and has a valid format. */ - static String checkValidEmail(String email) { - checkNotNull(email, "Provided email was null"); - try { - InternetAddress internetAddress = new InternetAddress(email, true); - internetAddress.validate(); - } catch (AddressException e) { - throw new IllegalArgumentException( - String.format("Provided email %s is not a valid email address", email)); - } - return email; - } - - /** Loads all registrar entities directly from Datastore. */ public static Iterable loadAll() { return ImmutableList.copyOf(ofy().load().type(Registrar.class).ancestor(getCrossTldKey())); diff --git a/java/google/registry/model/registrar/RegistrarContact.java b/java/google/registry/model/registrar/RegistrarContact.java index b7ff0d85f..3faef7fbf 100644 --- a/java/google/registry/model/registrar/RegistrarContact.java +++ b/java/google/registry/model/registrar/RegistrarContact.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Sets.difference; import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.model.registrar.Registrar.checkValidEmail; import static google.registry.util.CollectionUtils.nullToEmptyImmutableSortedCopy; import static java.util.stream.Collectors.joining; @@ -297,7 +296,7 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable { @Override public RegistrarContact build() { checkNotNull(getInstance().parent, "Registrar parent cannot be null"); - checkValidEmail(getInstance().emailAddress); + checkNotNull(getInstance().emailAddress, "Email address cannot be null"); return cloneEmptyToNull(super.build()); } diff --git a/javatests/google/registry/model/registrar/RegistrarTest.java b/javatests/google/registry/model/registrar/RegistrarTest.java index 9fa471426..21d4041f3 100644 --- a/javatests/google/registry/model/registrar/RegistrarTest.java +++ b/javatests/google/registry/model/registrar/RegistrarTest.java @@ -452,42 +452,6 @@ public class RegistrarTest extends EntityTestCase { () -> registrar.asBuilder().setAllowedTldsUncached(ImmutableSet.of("bad"))); } - @Test - public void testFailure_nullEmail() { - NullPointerException thrown = - assertThrows(NullPointerException.class, () -> registrar.asBuilder().setEmailAddress(null)); - assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null"); - } - - @Test - public void testFailure_invalidEmail() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, () -> registrar.asBuilder().setEmailAddress("lolcat")); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Provided email lolcat is not a valid email address"); - } - - @Test - public void testFailure_nullIcannReferralEmail() { - NullPointerException thrown = - assertThrows( - NullPointerException.class, () -> registrar.asBuilder().setIcannReferralEmail(null)); - assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null"); - } - - @Test - public void testFailure_invalidIcannEmail() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> registrar.asBuilder().setIcannReferralEmail("lolcat")); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Provided email lolcat is not a valid email address"); - } - @Test public void testSuccess_setAllowedTldsUncached_newTldNotInCache() { int origSingletonCacheRefreshSeconds = diff --git a/javatests/google/registry/tools/CreateRegistrarCommandTest.java b/javatests/google/registry/tools/CreateRegistrarCommandTest.java index e978bdfa1..1f5b3f9ba 100644 --- a/javatests/google/registry/tools/CreateRegistrarCommandTest.java +++ b/javatests/google/registry/tools/CreateRegistrarCommandTest.java @@ -1772,28 +1772,4 @@ public class CreateRegistrarCommandTest extends CommandTestCase - runCommandForced( - "--name=blobio", - "--password=some_password", - "--registrar_type=REAL", - "--iana_id=8", - "--passcode=01234", - "--icann_referral_email=lolcat", - "--street=\"123 Fake St\"", - "--city Fakington", - "--state MA", - "--zip 00351", - "--cc US", - "clientz")); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Provided email lolcat is not a valid email address"); - } } diff --git a/javatests/google/registry/tools/RegistrarContactCommandTest.java b/javatests/google/registry/tools/RegistrarContactCommandTest.java index 12b04b8ea..9b007ef07 100644 --- a/javatests/google/registry/tools/RegistrarContactCommandTest.java +++ b/javatests/google/registry/tools/RegistrarContactCommandTest.java @@ -367,26 +367,4 @@ public class RegistrarContactCommandTest extends CommandTestCase - runCommandForced( - "--mode=CREATE", "--name=Jim Doe", "--email=lolcat", "NewRegistrar")); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Provided email lolcat is not a valid email address"); - } - - @Test - public void testCreate_failure_nullEmail() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> runCommandForced("--mode=CREATE", "--name=Jim Doe", "NewRegistrar")); - assertThat(thrown).hasMessageThat().isEqualTo("--email is required when --mode=CREATE"); - } } diff --git a/javatests/google/registry/tools/UpdateRegistrarCommandTest.java b/javatests/google/registry/tools/UpdateRegistrarCommandTest.java index aab7aa5c0..6f458926a 100644 --- a/javatests/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/javatests/google/registry/tools/UpdateRegistrarCommandTest.java @@ -815,17 +815,6 @@ public class UpdateRegistrarCommandTest extends CommandTestCase runCommand("--email=lolcat", "--force", "NewRegistrar")); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Provided email lolcat is not a valid email address"); - } - private void persistWhoisAbuseContact() { persistResource( AppEngineRule.makeRegistrarContact1() diff --git a/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java index 1afd70a72..a08278d23 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java @@ -405,27 +405,6 @@ public final class ConsoleRegistrarCreatorActionTest { assertThat(registrar.getPhonePasscode()).isEqualTo("10203"); } - @Test - public void testPost_badEmailFails() { - action.clientId = Optional.of("myclientid"); - action.name = Optional.of("registrar name"); - action.billingAccount = Optional.of("USD=billing-account"); - action.ianaId = Optional.of(12321); - action.referralEmail = Optional.of("lolcat"); - action.driveId = Optional.of("drive-id"); - action.consoleUserEmail = Optional.of("myclientid@registry.example"); - - action.street1 = Optional.of("my street"); - action.city = Optional.of("my city"); - action.countryCode = Optional.of("CC"); - - action.method = Method.POST; - action.run(); - - assertThat(response.getPayload()) - .contains("Failed: Provided email lolcat is not a valid email address"); - } - @Test public void testPost_unauthorized() { action.registrarAccessor = diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 97b9c0379..cfbe6c122 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -149,6 +149,24 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException"); } + @Test + public void testUpdate_emptyJsonObject_emailFieldNotRequiredWhenEmpty() { + persistResource(loadRegistrar(CLIENT_ID).asBuilder().setEmailAddress(null).build()); + Map args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap()); + args.remove("emailAddress"); + + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "id", CLIENT_ID, + "args", args)); + + assertThat(response).containsExactly( + "status", "SUCCESS", + "message", "Saved TheRegistrar", + "results", ImmutableList.of(loadRegistrar(CLIENT_ID).toJsonMap())); + assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS"); + } + @Test public void testFailure_updateRegistrarInfo_notAuthorized() { setUserWithoutAccess();