Automated g4 rollback of changelist 240574585.

*** Reason for rollback ***

The inconsistent class loading is breaking the tests

*** Original change description ***

Validate provided email addresses when creating a Registrar

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=241014945

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']

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

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<Registrar> loadAll() {
     return ImmutableList.copyOf(ofy().load().type(Registrar.class).ancestor(getCrossTldKey()));

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());
     }
 

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 =

javatests/google/registry/tools/CreateRegistrarCommandTest.java

@@ -1772,28 +1772,4 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
                 "--cc US",
                 "clientz"));
   }
-
-  @Test
-  public void testFailure_badEmail() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                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");
-  }
 }

javatests/google/registry/tools/RegistrarContactCommandTest.java

@@ -367,26 +367,4 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--mode=CREATE", "--name=Jim Doe", "--email=jim.doe@example.com", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getContactsRequireSyncing()).isTrue();
   }
-
-  @Test
-  public void testCreate_failure_badEmail() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                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");
-  }
 }

javatests/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -815,17 +815,6 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
     assertThat(loadRegistrar("NewRegistrar").getPoNumber()).isEmpty();
   }
 
-  @Test
-  public void testFailure_badEmail() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () -> runCommand("--email=lolcat", "--force", "NewRegistrar"));
-    assertThat(thrown)
-        .hasMessageThat()
-        .isEqualTo("Provided email lolcat is not a valid email address");
-  }
-
   private void persistWhoisAbuseContact() {
     persistResource(
         AppEngineRule.makeRegistrarContact1()

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 =

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<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
+    args.remove("emailAddress");
+
+    Map<String, Object> 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();