Don't retry permanent failures when uploading ICANN monthly reports (#328)

* Don't retry permanent failures when uploading ICANN monthly reports

There are two kinds of permanent failures that this checks for that we know will
never succeed, so it makes no sense to continue retrying 11 more times before
moving onto the next file to upload. These errors are:

1.
com.google.api.client.http.HttpResponseException: 403
Your IP address xx.xx.xx.xx is not allowed to connect

2.
com.google.api.client.http.HttpResponseException: 400
<?xml version="1.0" encoding="UTF-8" standalone="yes"?><response xmlns="urn:ietf:params:xml:ns:iirdea-1.0"><result code="2002"><msg>A report for that month already exists, the cut-off date already passed.</msg><description>Date: 2019-09</description></result></response>

In order to implement this new functionality, this commit also adds a new way to
call Retriable that allows specifying the isRetryable Predicate (which is quite
useful).

buildSrc/gradle/dependency-locks/buildscript-classpath.lockfile

@@ -16,7 +16,7 @@ com.jcraft:jsch:0.1.54
 com.jcraft:jzlib:1.1.1
 com.netflix.nebula:gradle-lint-plugin:10.4.2
 com.netflix.nebula:nebula-gradle-interop:1.0.11
-com.netflix.nebula:nebula-test:7.3.0
+com.netflix.nebula:nebula-test:7.4.0
 commons-codec:commons-codec:1.9
 commons-io:commons-io:2.5
 commons-lang:commons-lang:2.6
@@ -41,7 +41,7 @@ org.apache.maven:maven-builder-support:3.6.2
 org.apache.maven:maven-model-builder:3.6.2
 org.apache.maven:maven-model:3.6.2
 org.codehaus.gpars:gpars:1.2.1
-org.codehaus.groovy:groovy-all:2.4.9
+org.codehaus.groovy:groovy-all:2.4.15
 org.codehaus.groovy:groovy-ant:2.1.8
 org.codehaus.groovy:groovy-groovydoc:2.1.8
 org.codehaus.groovy:groovy-templates:2.1.8
@@ -65,4 +65,4 @@ org.multiverse:multiverse-core:0.7.0
 org.objenesis:objenesis:2.4
 org.ow2.asm:asm:7.0
 org.slf4j:slf4j-api:1.7.2
-org.spockframework:spock-core:1.1-groovy-2.4-rc-4
+org.spockframework:spock-core:1.3-groovy-2.4

config/dependency-license/allowed_licenses.json

@@ -90,6 +90,9 @@
     {
       "moduleLicense": "The BSD License"
     },
+    {
+      "moduleLicense": "The New BSD License"
+    },
     {
       "moduleLicense": "The PostgreSQL License"
     },
@@ -198,6 +201,9 @@
     {
       "moduleLicense": "The MIT license"
     },
+    {
+      "moduleLicense": "The MIT License (MIT)"
+    },
     {
       "moduleLicense": "The PostgreSQL License"
     },

core/build.gradle

@@ -192,7 +192,7 @@ dependencies {
   compile deps['com.jcraft:jsch']
   testCompile deps['com.thoughtworks.qdox:qdox']
   compile deps['dnsjava:dnsjava']
-  runtime deps['org.glassfish.jaxb:jaxb-runtime']
+  testCompile deps['io.github.classgraph:classgraph']
   testCompile deps['javax.annotation:jsr250-api']
   compile deps['javax.inject:javax.inject']
   compile deps['javax.mail:mail']
@@ -218,6 +218,7 @@ dependencies {
   compile deps['org.bouncycastle:bcpg-jdk15on']
   testCompile deps['org.bouncycastle:bcpkix-jdk15on']
   compile deps['org.bouncycastle:bcprov-jdk15on']
+  runtime deps['org.glassfish.jaxb:jaxb-runtime']
   compile deps['org.joda:joda-money']
   compile deps['org.json:json']
   testCompile deps['org.mortbay.jetty:jetty']
@@ -606,6 +607,15 @@ task outcastTest(type: FilteringTest) {
   maxParallelForks 5
 }
 
+// Dedicated test suite for schema-dependent tests.
+task sqlIntegrationTest(type: Test) {
+  include 'google/registry/schema/integration/SqlIntegrationTestSuite.*'
+  // Copied from FilteringTest. Not inheriting b/c it excludes TestSuites.
+  if (project.testFilter) {
+    testNameIncludePatterns = project.testFilter.split(',')
+  }
+}
+
 task findGoldenImages(type: JavaExec) {
   classpath = sourceSets.test.runtimeClasspath
   main = 'google.registry.webdriver.GoldenImageFinder'

core/gradle/dependency-locks/testCompile.lockfile

@@ -130,6 +130,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

core/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -129,6 +129,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

core/gradle/dependency-locks/testRuntime.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

core/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

core/src/main/java/google/registry/model/registry/RegistryLockDao.java

@@ -17,6 +17,7 @@ package google.registry.model.registry;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.model.transaction.TransactionManagerFactory.jpaTm;
 
+import com.google.common.collect.ImmutableList;
 import google.registry.schema.domain.RegistryLock;
 import javax.persistence.EntityManager;
 
@@ -45,8 +46,23 @@ public final class RegistryLockDao {
             });
   }
 
-  public static void save(RegistryLock registryLock) {
+  public static ImmutableList<RegistryLock> getByRegistrarId(String registrarId) {
+    return jpaTm()
+        .transact(
+            () ->
+                ImmutableList.copyOf(
+                    jpaTm()
+                        .getEntityManager()
+                        .createQuery(
+                            "SELECT lock FROM RegistryLock lock WHERE"
+                                + " lock.registrarId = :registrarId",
+                            RegistryLock.class)
+                        .setParameter("registrarId", registrarId)
+                        .getResultList()));
+  }
+
+  public static RegistryLock save(RegistryLock registryLock) {
     checkNotNull(registryLock, "Null registry lock cannot be saved");
-    jpaTm().transact(() -> jpaTm().getEntityManager().persist(registryLock));
+    return jpaTm().transact(() -> jpaTm().getEntityManager().merge(registryLock));
   }
 }

core/src/main/java/google/registry/model/transaction/TransactionManagerFactory.java

@@ -14,8 +14,15 @@
 
 package google.registry.model.transaction;
 
+import static google.registry.config.RegistryEnvironment.ALPHA;
+import static google.registry.config.RegistryEnvironment.CRASH;
+
+import com.google.appengine.api.utils.SystemProperty;
+import com.google.appengine.api.utils.SystemProperty.Environment.Value;
 import com.google.common.annotations.VisibleForTesting;
+import google.registry.config.RegistryEnvironment;
 import google.registry.model.ofy.DatastoreTransactionManager;
+import google.registry.persistence.DaggerPersistenceComponent;
 
 /** Factory class to create {@link TransactionManager} instance. */
 // TODO: Rename this to PersistenceFactory and move to persistence package.
@@ -27,19 +34,11 @@ public class TransactionManagerFactory {
   private TransactionManagerFactory() {}
 
   private static JpaTransactionManager createJpaTransactionManager() {
-    // TODO(shicong): There is currently no environment where we want to create a real JPA
-    // transaction manager here.  The unit tests that require one are all set up using
-    // JpaTransactionManagerRule which launches its own PostgreSQL instance.  When we actually have
-    // PostgreSQL tables in production, ensure that all of the test environments are set up
-    // correctly and restore the code that creates a JpaTransactionManager when
-    // RegistryEnvironment.get() != UNITTEST.
-    //
-    // We removed the original code because it didn't work in sandbox (due to the absence of the
-    // persistence.xml file, which has since been added), and then (after adding this) didn't work
-    // in crash because the postgresql password hadn't been set up.  Prior to restoring, we'll need
-    // to do setup in all environments, and we probably only want to do this once we're actually
-    // using Cloud SQL for one of the new tables.
-    return DummyJpaTransactionManager.create();
+    if (shouldEnableJpaTm() && isInAppEngine()) {
+      return DaggerPersistenceComponent.create().appEngineJpaTransactionManager();
+    } else {
+      return DummyJpaTransactionManager.create();
+    }
   }
 
   private static TransactionManager createTransactionManager() {
@@ -54,8 +53,27 @@ public class TransactionManagerFactory {
    * {@link google.registry.tools.RegistryCli} to initialize jpaTm.
    */
   public static void initForTool() {
-    // TODO(shicong): Uncomment the line below when we set up Cloud SQL instance in all environments
-    // jpaTm = DaggerPersistenceComponent.create().nomulusToolJpaTransactionManager();
+    if (shouldEnableJpaTm()) {
+      jpaTm = DaggerPersistenceComponent.create().nomulusToolJpaTransactionManager();
+    }
+  }
+
+  // TODO(shicong): Enable JpaTm for all environments and remove this function
+  private static boolean shouldEnableJpaTm() {
+    return RegistryEnvironment.get() == ALPHA || RegistryEnvironment.get() == CRASH;
+  }
+
+  /**
+   * This function uses App Engine API to determine if the current runtime environment is App
+   * Engine.
+   *
+   * @see <a
+   *     href="https://cloud.google.com/appengine/docs/standard/java/javadoc/com/google/appengine/api/utils/SystemProperty">App
+   *     Engine API public doc</a>
+   */
+  private static boolean isInAppEngine() {
+    // SystemProperty.environment.value() returns null if the current runtime is local JVM
+    return SystemProperty.environment.value() == Value.Production;
   }
 
   /** Returns {@link TransactionManager} instance. */

core/src/main/java/google/registry/persistence/PersistenceModule.java

@@ -51,6 +51,10 @@ public class PersistenceModule {
   public static final String HIKARI_MAXIMUM_POOL_SIZE = "hibernate.hikari.maximumPoolSize";
   public static final String HIKARI_IDLE_TIMEOUT = "hibernate.hikari.idleTimeout";
 
+  public static final String HIKARI_DS_SOCKET_FACTORY = "hibernate.hikari.dataSource.socketFactory";
+  public static final String HIKARI_DS_CLOUD_SQL_INSTANCE =
+      "hibernate.hikari.dataSource.cloudSqlInstance";
+
   @Provides
   @DefaultHibernateConfigs
   public static ImmutableMap<String, String> providesDefaultDatabaseConfigs() {
@@ -79,27 +83,31 @@ public class PersistenceModule {
     return properties.build();
   }
 
+  @Provides
+  @Singleton
+  @PartialCloudSqlConfigs
+  public static ImmutableMap<String, String> providesPartialCloudSqlConfigs(
+      @Config("cloudSqlJdbcUrl") String jdbcUrl,
+      @Config("cloudSqlInstanceConnectionName") String instanceConnectionName,
+      @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs) {
+    HashMap<String, String> overrides = Maps.newHashMap(defaultConfigs);
+    overrides.put(Environment.URL, jdbcUrl);
+    overrides.put(HIKARI_DS_SOCKET_FACTORY, "com.google.cloud.sql.postgres.SocketFactory");
+    overrides.put(HIKARI_DS_CLOUD_SQL_INSTANCE, instanceConnectionName);
+    return ImmutableMap.copyOf(overrides);
+  }
+
   @Provides
   @Singleton
   @AppEngineJpaTm
   public static JpaTransactionManager providesAppEngineJpaTm(
-      @Config("cloudSqlJdbcUrl") String jdbcUrl,
       @Config("cloudSqlUsername") String username,
-      @Config("cloudSqlInstanceConnectionName") String instanceConnectionName,
       KmsKeyring kmsKeyring,
-      @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs,
+      @PartialCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
       Clock clock) {
-    HashMap<String, String> overrides = Maps.newHashMap(defaultConfigs);
-
-    // For Java users, the Cloud SQL JDBC Socket Factory can provide authenticated connections.
-    // See https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory for details.
-    overrides.put("socketFactory", "com.google.cloud.sql.postgres.SocketFactory");
-    overrides.put("cloudSqlInstance", instanceConnectionName);
-
-    overrides.put(Environment.URL, jdbcUrl);
+    HashMap<String, String> overrides = Maps.newHashMap(cloudSqlConfigs);
     overrides.put(Environment.USER, username);
     overrides.put(Environment.PASS, kmsKeyring.getCloudSqlPassword());
-
     return new JpaTransactionManagerImpl(create(overrides), clock);
   }
 
@@ -107,27 +115,13 @@ public class PersistenceModule {
   @Singleton
   @NomulusToolJpaTm
   public static JpaTransactionManager providesNomulusToolJpaTm(
-      @Config("toolsCloudSqlJdbcUrl") String jdbcUrl,
       @Config("toolsCloudSqlUsername") String username,
-      @Config("cloudSqlInstanceConnectionName") String instanceConnectionName,
       KmsKeyring kmsKeyring,
-      @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs,
+      @PartialCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
       Clock clock) {
-
-    // Cloud SQL JDBC Socket Factory library requires the jdbc url to include all connection
-    // information, otherwise the connection initialization will fail. See here for more details:
-    // https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory
-    String fullJdbcUrl =
-        new StringBuilder(jdbcUrl)
-            .append("?cloudSqlInstance=" + instanceConnectionName)
-            .append("&socketFactory=com.google.cloud.sql.postgres.SocketFactory")
-            .append("&user=" + username)
-            .append("&password=" + kmsKeyring.getCloudSqlPassword())
-            .toString();
-
-    HashMap<String, String> overrides = Maps.newHashMap(defaultConfigs);
-    overrides.put(Environment.URL, fullJdbcUrl);
-
+    HashMap<String, String> overrides = Maps.newHashMap(cloudSqlConfigs);
+    overrides.put(Environment.USER, username);
+    overrides.put(Environment.PASS, kmsKeyring.getToolsCloudSqlPassword());
     return new JpaTransactionManagerImpl(create(overrides), clock);
   }
 
@@ -166,6 +160,11 @@ public class PersistenceModule {
   @Documented
   @interface NomulusToolJpaTm {}
 
+  /** Dagger qualifier for the partial Cloud SQL configs. */
+  @Qualifier
+  @Documented
+  @interface PartialCloudSqlConfigs {}
+
   /** Dagger qualifier for the default Hibernate configurations. */
   // TODO(shicong): Change annotations in this class to none public or put them in a top level
   //  package

core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java

@@ -39,6 +39,7 @@ import google.registry.util.Retrier;
 import google.registry.util.SendEmailService;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.mail.internet.InternetAddress;
@@ -102,7 +103,7 @@ public final class IcannReportingUploadAction implements Runnable {
                   final byte[] payload = readBytesFromGcs(gcsFilename);
                   return icannReporter.send(payload, reportFilename);
                 },
-                IOException.class);
+                IcannReportingUploadAction::isUploadFailureRetryable);
       } catch (RuntimeException e) {
         logger.atWarning().withCause(e).log("Upload to %s failed.", gcsFilename);
       }
@@ -115,6 +116,21 @@ public final class IcannReportingUploadAction implements Runnable {
         String.format("OK, attempted uploading %d reports", manifestedFiles.size()));
   }
 
+  /** Don't retry when reports are already uploaded or can't be uploaded. */
+  private static final String ICANN_UPLOAD_PERMANENT_ERROR_MESSAGE =
+      "A report for that month already exists, the cut-off date already passed.";
+
+  /** Don't retry when the IP address isn't whitelisted, as retries go through the same IP. */
+  private static final Pattern ICANN_UPLOAD_WHITELIST_ERROR =
+      Pattern.compile("Your IP address .+ is not allowed to connect");
+
+  /** Predicate to retry uploads on IOException, so long as they aren't non-retryable errors. */
+  private static boolean isUploadFailureRetryable(Throwable e) {
+    return (e instanceof IOException)
+        && !e.getMessage().contains(ICANN_UPLOAD_PERMANENT_ERROR_MESSAGE)
+        && !ICANN_UPLOAD_WHITELIST_ERROR.matcher(e.getMessage()).matches();
+  }
+
   private void emailUploadResults(ImmutableMap<String, Boolean> reportSummary) {
     String subject = String.format(
         "ICANN Monthly report upload summary: %d/%d succeeded",

core/src/main/java/google/registry/schema/domain/RegistryLock.java

@@ -65,7 +65,8 @@ import org.joda.time.DateTime;
           name = "idx_registry_lock_repo_id_revision_id",
           columnList = "repoId, revisionId",
           unique = true),
-      @Index(name = "idx_registry_lock_verification_code", columnList = "verificationCode")
+      @Index(name = "idx_registry_lock_verification_code", columnList = "verificationCode"),
+      @Index(name = "idx_registry_lock_registrar_id", columnList = "registrarId")
     })
 public final class RegistryLock extends ImmutableObject implements Buildable {
 
@@ -177,10 +178,7 @@ public final class RegistryLock extends ImmutableObject implements Buildable {
 
   @Override
   public Builder asBuilder() {
-    RegistryLock clone = clone(this);
-    // Revision ID should be different for every object
-    clone.revisionId = null;
-    return new Builder(clone);
+    return new Builder(clone(this));
   }
 
   /** Builder for {@link google.registry.schema.domain.RegistryLock}. */

core/src/main/java/google/registry/tools/UploadClaimsListCommand.java

@@ -32,7 +32,7 @@ import java.util.List;
 
 /** A command to upload a {@link ClaimsListShard}. */
 @Parameters(separators = " =", commandDescription = "Manually upload a new claims list file")
-final class UploadClaimsListCommand extends ConfirmingCommand implements CommandWithRemoteApi {
+final class UploadClaimsListCommand extends ConfirmingCommand implements CommandWithCloudSql {
 
   @Parameter(description = "Claims list filename")
   private List<String> mainParameters = new ArrayList<>();

core/src/main/java/google/registry/ui/server/RegistrarFormFields.java

@@ -21,7 +21,9 @@ import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
 
 import com.google.common.base.Ascii;
 import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.net.InternetDomainName;
 import com.google.re2j.Pattern;
 import google.registry.model.registrar.Registrar;
@@ -194,12 +196,10 @@ public final class RegistrarFormFields {
           FormField.named("visibleInDomainWhoisAsAbuse", Boolean.class).build();
 
   public static final FormField<String, String> CONTACT_PHONE_NUMBER_FIELD =
-      FormFields.PHONE_NUMBER.asBuilder()
-          .build();
+      FormFields.PHONE_NUMBER.asBuilder().build();
 
   public static final FormField<String, String> CONTACT_FAX_NUMBER_FIELD =
-      FormFields.PHONE_NUMBER.asBuilderNamed("faxNumber")
-          .build();
+      FormFields.PHONE_NUMBER.asBuilderNamed("faxNumber").build();
 
   public static final FormField<String, String> CONTACT_GAE_USER_ID_FIELD =
       FormFields.NAME.asBuilderNamed("gaeUserId").build();
@@ -217,11 +217,8 @@ public final class RegistrarFormFields {
           .asSet(Splitter.on(',').omitEmptyStrings().trimResults())
           .build();
 
-  public static final FormField<List<Map<String, ?>>, List<RegistrarContact.Builder>>
-      CONTACTS_FIELD = FormField.mapNamed("contacts")
-          .transform(RegistrarContact.Builder.class, RegistrarFormFields::toRegistrarContactBuilder)
-          .asList()
-          .build();
+  public static final FormField<List<Map<String, ?>>, List<Map<String, ?>>> CONTACTS_AS_MAPS =
+      FormField.mapNamed("contacts").asList().build();
 
   public static final FormField<List<String>, List<String>> I18N_STREET_FIELD =
       FormFields.XS_NORMALIZED_STRING.asBuilderNamed("street")
@@ -344,33 +341,60 @@ public final class RegistrarFormFields {
     }
   }
 
-  private static @Nullable RegistrarContact.Builder toRegistrarContactBuilder(
-      @Nullable Map<String, ?> args) {
+  public static ImmutableList<RegistrarContact.Builder> getRegistrarContactBuilders(
+      ImmutableSet<RegistrarContact> existingContacts, @Nullable Map<String, ?> args) {
     if (args == null) {
-      return null;
+      return ImmutableList.of();
     }
-    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);
-    CONTACT_ALLOWED_TO_SET_REGISTRY_LOCK_PASSWORD
-        .extractUntyped(args)
-        .ifPresent(builder::setAllowedToSetRegistryLockPassword);
+    Optional<List<Map<String, ?>>> contactsAsMaps = CONTACTS_AS_MAPS.extractUntyped(args);
+    if (!contactsAsMaps.isPresent()) {
+      return ImmutableList.of();
+    }
+    ImmutableList.Builder<RegistrarContact.Builder> result = new ImmutableList.Builder<>();
+    for (Map<String, ?> contactAsMap : contactsAsMaps.get()) {
+      String emailAddress =
+          CONTACT_EMAIL_ADDRESS_FIELD
+              .extractUntyped(contactAsMap)
+              .orElseThrow(
+                  () -> new IllegalArgumentException("Contacts from UI must have email addresses"));
+      // Start with a new builder if the contact didn't previously exist
+      RegistrarContact.Builder contactBuilder =
+          existingContacts.stream()
+              .filter(rc -> rc.getEmailAddress().equals(emailAddress))
+              .findFirst()
+              .map(RegistrarContact::asBuilder)
+              .orElse(new RegistrarContact.Builder());
+      applyRegistrarContactArgs(contactBuilder, contactAsMap);
+      result.add(contactBuilder);
+    }
+    return result.build();
+  }
+
+  private static void applyRegistrarContactArgs(
+      RegistrarContact.Builder builder, Map<String, ?> args) {
+    builder.setName(CONTACT_NAME_FIELD.extractUntyped(args).orElse(null));
+    builder.setEmailAddress(CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).orElse(null));
+    builder.setVisibleInWhoisAsAdmin(
+        CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD.extractUntyped(args).orElse(false));
+    builder.setVisibleInWhoisAsTech(
+        CONTACT_VISIBLE_IN_WHOIS_AS_TECH_FIELD.extractUntyped(args).orElse(false));
+    builder.setVisibleInDomainWhoisAsAbuse(
+        PHONE_AND_EMAIL_VISIBLE_IN_DOMAIN_WHOIS_AS_ABUSE_FIELD.extractUntyped(args).orElse(false));
+    builder.setPhoneNumber(CONTACT_PHONE_NUMBER_FIELD.extractUntyped(args).orElse(null));
+    builder.setFaxNumber(CONTACT_FAX_NUMBER_FIELD.extractUntyped(args).orElse(null));
+    builder.setTypes(CONTACT_TYPES.extractUntyped(args).orElse(ImmutableSet.of()));
+    builder.setGaeUserId(CONTACT_GAE_USER_ID_FIELD.extractUntyped(args).orElse(null));
+    builder.setAllowedToSetRegistryLockPassword(
+        CONTACT_ALLOWED_TO_SET_REGISTRY_LOCK_PASSWORD.extractUntyped(args).orElse(false));
+
+    // Registry lock password does not need to be set every time
     CONTACT_REGISTRY_LOCK_PASSWORD_FIELD
         .extractUntyped(args)
-        .ifPresent(builder::setRegistryLockPassword);
-    return builder;
+        .ifPresent(
+            password -> {
+              if (!Strings.isNullOrEmpty(password)) {
+                builder.setRegistryLockPassword(password);
+              }
+            });
   }
 }

core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java

@@ -60,7 +60,6 @@ import google.registry.util.CollectionUtils;
 import google.registry.util.DiffUtils;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -175,8 +174,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
   }
 
   private RegistrarResult update(final Map<String, ?> args, String clientId) {
-    return tm()
-        .transact(
+    return tm().transact(
             () -> {
               // We load the registrar here rather than outside of the transaction - to make
               // sure we have the latest version. This one is loaded inside the transaction, so it's
@@ -215,7 +213,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
               updatedRegistrar = checkAndUpdateAdminControlledFields(updatedRegistrar, args);
 
               // read the contacts from the request.
-              ImmutableSet<RegistrarContact> updatedContacts = readContacts(registrar, args);
+              ImmutableSet<RegistrarContact> updatedContacts =
+                  readContacts(registrar, contacts, args);
 
               // Save the updated contacts
               if (!updatedContacts.equals(contacts)) {
@@ -240,11 +239,18 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
             });
   }
 
-  private Map<String, Object> expandRegistrarWithContacts(Iterable<RegistrarContact> contacts,
-                                                          Registrar registrar) {
+  private Map<String, Object> expandRegistrarWithContacts(
+      Iterable<RegistrarContact> contacts, Registrar registrar) {
     ImmutableSet<Map<String, Object>> expandedContacts =
         Streams.stream(contacts)
             .map(RegistrarContact::toDiffableFieldMap)
+            // Note: per the javadoc, toDiffableFieldMap includes sensitive data but we don't want
+            // to display it here
+            .peek(
+                map -> {
+                  map.remove("registryLockPasswordHash");
+                  map.remove("registryLockPasswordSalt");
+                })
             .collect(toImmutableSet());
     // Use LinkedHashMap here to preserve ordering; null values mean we can't use ImmutableMap.
     LinkedHashMap<String, Object> result = new LinkedHashMap<>(registrar.toDiffableFieldMap());
@@ -377,16 +383,10 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
 
   /** Reads the contacts from the supplied args. */
   public static ImmutableSet<RegistrarContact> readContacts(
-      Registrar registrar, Map<String, ?> args) {
-
-    ImmutableSet.Builder<RegistrarContact> contacts = new ImmutableSet.Builder<>();
-    Optional<List<RegistrarContact.Builder>> builders =
-        RegistrarFormFields.CONTACTS_FIELD.extractUntyped(args);
-    if (builders.isPresent()) {
-      builders.get().forEach(c -> contacts.add(c.setParent(registrar).build()));
-    }
-
-    return contacts.build();
+      Registrar registrar, ImmutableSet<RegistrarContact> existingContacts, Map<String, ?> args) {
+    return RegistrarFormFields.getRegistrarContactBuilders(existingContacts, args).stream()
+        .map(builder -> builder.setParent(registrar).build())
+        .collect(toImmutableSet());
   }
 
   /**
@@ -452,8 +452,17 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
                     () ->
                         new FormException(
                             "Not allowed to set registry lock password directly on new contact"));
-        if (!existingContact.isAllowedToSetRegistryLockPassword()) {
-          throw new FormException("Registrar contact not allowed to set registry lock password");
+        if (updatedContact.isRegistryLockAllowed()) {
+          // the password must have been set before or the user was allowed to set it now
+          if (!existingContact.isAllowedToSetRegistryLockPassword()
+              && !existingContact.isRegistryLockAllowed()) {
+            throw new FormException("Registrar contact not allowed to set registry lock password");
+          }
+        }
+        if (updatedContact.isAllowedToSetRegistryLockPassword()) {
+          if (!existingContact.isAllowedToSetRegistryLockPassword()) {
+            throw new FormException("Cannot set isAllowedToSetRegistryLockPassword through UI");
+          }
         }
       }
     }

core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java

@@ -14,6 +14,7 @@
 
 package google.registry.model.registry;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.JUnitBackports.assertThrows;
@@ -22,6 +23,7 @@ import google.registry.model.transaction.JpaTransactionManagerRule;
 import google.registry.schema.domain.RegistryLock;
 import google.registry.schema.domain.RegistryLock.Action;
 import google.registry.testing.AppEngineRule;
+import java.util.Optional;
 import java.util.UUID;
 import javax.persistence.PersistenceException;
 import org.junit.Rule;
@@ -71,10 +73,10 @@ public final class RegistryLockDaoTest {
     jpaTm()
         .transact(
             () -> {
-              RegistryLock secondLock =
+              RegistryLock updatedLock =
                   RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
-              secondLock.setCompletionTimestamp(jpaTmRule.getTxnClock().nowUtc());
-              RegistryLockDao.save(secondLock);
+              updatedLock.setCompletionTimestamp(jpaTmRule.getTxnClock().nowUtc());
+              RegistryLockDao.save(updatedLock);
             });
     jpaTm()
         .transact(
@@ -86,11 +88,43 @@ public final class RegistryLockDaoTest {
             });
   }
 
+  @Test
+  public void testUpdateLock_usingSamePrimaryKey() {
+    RegistryLock lock = RegistryLockDao.save(createLock());
+    jpaTmRule.getTxnClock().advanceOneMilli();
+    RegistryLock updatedLock =
+        lock.asBuilder().setCompletionTimestamp(jpaTmRule.getTxnClock().nowUtc()).build();
+    jpaTm().transact(() -> RegistryLockDao.save(updatedLock));
+    jpaTm()
+        .transact(
+            () -> {
+              RegistryLock fromDatabase =
+                  RegistryLockDao.getByVerificationCode(lock.getVerificationCode());
+              assertThat(fromDatabase.getCompletionTimestamp())
+                  .isEqualTo(Optional.of(jpaTmRule.getTxnClock().nowUtc()));
+            });
+  }
+
   @Test
   public void testFailure_saveNull() {
     assertThrows(NullPointerException.class, () -> RegistryLockDao.save(null));
   }
 
+  @Test
+  public void testLoad_byRegistrarId() {
+    RegistryLock lock = createLock();
+    RegistryLock secondLock = createLock().asBuilder().setDomainName("otherexample.test").build();
+    RegistryLockDao.save(lock);
+    RegistryLockDao.save(secondLock);
+
+    assertThat(
+            RegistryLockDao.getByRegistrarId("TheRegistrar").stream()
+                .map(RegistryLock::getDomainName)
+                .collect(toImmutableSet()))
+        .containsExactly("example.test", "otherexample.test");
+    assertThat(RegistryLockDao.getByRegistrarId("nonexistent")).isEmpty();
+  }
+
   private RegistryLock createLock() {
     return new RegistryLock.Builder()
         .setRepoId("repoId")

core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRule.java

@@ -15,17 +15,24 @@
 package google.registry.model.transaction;
 
 import static org.joda.time.DateTimeZone.UTC;
+import static org.testcontainers.containers.PostgreSQLContainer.POSTGRESQL_PORT;
 
+import com.google.common.base.Charsets;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
+import com.google.common.io.Resources;
 import google.registry.persistence.PersistenceModule;
 import google.registry.testing.FakeClock;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import javax.persistence.EntityManagerFactory;
 import org.hibernate.cfg.Environment;
 import org.hibernate.jpa.boot.internal.ParsedPersistenceXmlDescriptor;
@@ -33,9 +40,6 @@ import org.hibernate.jpa.boot.internal.PersistenceXmlParser;
 import org.hibernate.jpa.boot.spi.Bootstrap;
 import org.joda.time.DateTime;
 import org.junit.rules.ExternalResource;
-import org.junit.rules.RuleChain;
-import org.junit.runner.Description;
-import org.junit.runners.model.Statement;
 import org.testcontainers.containers.JdbcDatabaseContainer;
 import org.testcontainers.containers.PostgreSQLContainer;
 
@@ -49,37 +53,42 @@ import org.testcontainers.containers.PostgreSQLContainer;
  */
 public class JpaTransactionManagerRule extends ExternalResource {
   private static final String SCHEMA_GOLDEN_SQL = "sql/schema/nomulus.golden.sql";
+  private static final String DB_CLEANUP_SQL =
+      "google/registry/model/transaction/cleanup_database.sql";
+  private static final String MANAGEMENT_DB_NAME = "management";
+  private static final String POSTGRES_DB_NAME = "postgres";
 
   private final DateTime now = DateTime.now(UTC);
   private final FakeClock clock = new FakeClock(now);
-  private final String initScript;
+  private final String initScriptPath;
   private final ImmutableList<Class> extraEntityClasses;
   private final ImmutableMap userProperties;
 
-  private JdbcDatabaseContainer database;
+  private static final JdbcDatabaseContainer database = create();
   private EntityManagerFactory emf;
   private JpaTransactionManager cachedTm;
 
   private JpaTransactionManagerRule(
-      String initScript,
+      String initScriptPath,
       ImmutableList<Class> extraEntityClasses,
       ImmutableMap<String, String> userProperties) {
-    this.initScript = initScript;
+    this.initScriptPath = initScriptPath;
     this.extraEntityClasses = extraEntityClasses;
     this.userProperties = userProperties;
   }
 
-  /** Wraps {@link JpaTransactionManagerRule} in a {@link PostgreSQLContainer}. */
-  @Override
-  public Statement apply(Statement base, Description description) {
-    database = new PostgreSQLContainer().withInitScript(initScript);
-    return RuleChain.outerRule(database)
-        .around(JpaTransactionManagerRule.super::apply)
-        .apply(base, description);
+  private static JdbcDatabaseContainer create() {
+    PostgreSQLContainer container = new PostgreSQLContainer().withDatabaseName(MANAGEMENT_DB_NAME);
+    container.start();
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> container.close()));
+    return container;
   }
 
   @Override
-  public void before() {
+  public void before() throws Exception {
+    executeSql(MANAGEMENT_DB_NAME, DB_CLEANUP_SQL);
+    executeSql(POSTGRES_DB_NAME, initScriptPath);
+
     ImmutableMap properties = PersistenceModule.providesDefaultDatabaseConfigs();
     if (!userProperties.isEmpty()) {
       // If there are user properties, create a new properties object with these added.
@@ -90,7 +99,7 @@ public class JpaTransactionManagerRule extends ExternalResource {
 
     emf =
         createEntityManagerFactory(
-            database.getJdbcUrl(),
+            getJdbcUrlFor(POSTGRES_DB_NAME),
             database.getUsername(),
             database.getPassword(),
             properties,
@@ -109,6 +118,38 @@ public class JpaTransactionManagerRule extends ExternalResource {
     cachedTm = null;
   }
 
+  private void executeSql(String dbName, String sqlScriptPath) {
+    try (Connection conn = createConnection(dbName)) {
+      String sqlScript = Resources.toString(Resources.getResource(sqlScriptPath), Charsets.UTF_8);
+      conn.createStatement().execute(sqlScript);
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  private String getJdbcUrlFor(String dbName) {
+    // Disable Postgres driver use of java.util.logging to reduce noise at startup time
+    return "jdbc:postgresql://"
+        + database.getContainerIpAddress()
+        + ":"
+        + database.getMappedPort(POSTGRESQL_PORT)
+        + "/"
+        + dbName
+        + "?loggerLevel=OFF";
+  }
+
+  private Connection createConnection(String dbName) {
+    final Properties info = new Properties();
+    info.put("user", database.getUsername());
+    info.put("password", database.getPassword());
+    final Driver jdbcDriverInstance = database.getJdbcDriverInstance();
+    try {
+      return jdbcDriverInstance.connect(getJdbcUrlFor(dbName), info);
+    } catch (SQLException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   /** Constructs the {@link EntityManagerFactory} instance. */
   private static EntityManagerFactory createEntityManagerFactory(
       String jdbcUrl,

core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java

@@ -154,6 +154,45 @@ public class IcannReportingUploadActionTest {
                 new InternetAddress("recipient@example.com"),
                 new InternetAddress("sender@example.com")));
   }
+
+  @Test
+  public void testFailure_quicklySkipsOverNonRetryableUploadException() throws Exception {
+    runTest_nonRetryableException(
+        new IOException(
+            "<msg>A report for that month already exists, the cut-off date already"
+                + " passed.</msg>"));
+  }
+
+  @Test
+  public void testFailure_quicklySkipsOverIpWhitelistException() throws Exception {
+    runTest_nonRetryableException(
+        new IOException("Your IP address 25.147.130.158 is not allowed to connect"));
+  }
+
+  private void runTest_nonRetryableException(Exception nonRetryableException) throws Exception {
+    IcannReportingUploadAction action = createAction();
+    when(mockReporter.send(PAYLOAD_FAIL, "a-activity-201706.csv"))
+        .thenThrow(nonRetryableException)
+        .thenThrow(
+            new AssertionError(
+                "This should never be thrown because the previous exception isn't retryable"));
+    action.run();
+    verify(mockReporter, times(1)).send(PAYLOAD_FAIL, "a-activity-201706.csv");
+    verify(mockReporter).send(PAYLOAD_SUCCESS, "test-transactions-201706.csv");
+    verifyNoMoreInteractions(mockReporter);
+    assertThat(((FakeResponse) action.response).getPayload())
+        .isEqualTo("OK, attempted uploading 2 reports");
+    verify(emailService)
+        .sendEmail(
+            EmailMessage.create(
+                "ICANN Monthly report upload summary: 1/2 succeeded",
+                "Report Filename - Upload status:\n"
+                    + "test-transactions-201706.csv - SUCCESS\n"
+                    + "a-activity-201706.csv - FAILURE",
+                new InternetAddress("recipient@example.com"),
+                new InternetAddress("sender@example.com")));
+  }
+
   @Test
   public void testFail_FileNotFound() throws Exception {
     IcannReportingUploadAction action = createAction();

core/src/test/java/google/registry/schema/integration/SqlIntegrationMembershipTest.java

@@ -0,0 +1,81 @@
+// Copyright 2018 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.integration;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.common.collect.Sets.SetView;
+import google.registry.model.transaction.JpaTransactionManagerRule;
+import io.github.classgraph.ClassGraph;
+import io.github.classgraph.ScanResult;
+import java.lang.reflect.Field;
+import java.util.stream.Stream;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.junit.runners.Suite.SuiteClasses;
+
+/**
+ * Verifies that all tests that depends on the Cloud SQL schema are included in the project's
+ * sqlIntegrationTest suite. Names of the test classes is set to the 'test.sqlIntergrationTests'
+ * system property as a comma-separated string.
+ *
+ * <p>A test is deemed dependent on the SQL schema iff it has a field with type {@link
+ * JpaTransactionManagerRule}.
+ */
+// TODO(weiminyu): consider generating a TestSuite class instead.
+@RunWith(JUnit4.class)
+public class SqlIntegrationMembershipTest {
+
+  @Test
+  public void sqlIntegrationMembershipComplete() {
+    ImmutableSet<String> sqlDependentTests;
+    try (ScanResult scanResult =
+        new ClassGraph()
+            .enableAnnotationInfo()
+            .whitelistPackages("google.registry")
+            .scan()) {
+      sqlDependentTests =
+          scanResult.getClassesWithAnnotation(RunWith.class.getName()).stream()
+              .filter(clazz -> clazz.getSimpleName().endsWith("Test"))
+              .map(clazz -> clazz.loadClass())
+              .filter(SqlIntegrationMembershipTest::isSqlDependent)
+              .map(Class::getName)
+              .collect(ImmutableSet.toImmutableSet());
+    }
+    ImmutableSet<String> declaredTests =
+        Stream.of(SqlIntegrationTestSuite.class.getAnnotation(SuiteClasses.class).value())
+            .map(Class::getName)
+            .collect(ImmutableSet.toImmutableSet());
+    SetView<String> undeclaredTests = Sets.difference(sqlDependentTests, declaredTests);
+    assertWithMessage(
+            "Undeclared sql-dependent tests found. "
+                + "Please add them to SqlIntegrationTestSuite.java.")
+        .that(undeclaredTests)
+        .isEmpty();
+    SetView<String> unnecessaryDeclarations = Sets.difference(declaredTests, sqlDependentTests);
+    assertWithMessage("Found tests that should not be included in SqlIntegrationTestSuite.java.")
+        .that(unnecessaryDeclarations)
+        .isEmpty();
+  }
+
+  private static boolean isSqlDependent(Class<?> testClass) {
+    return Stream.of(testClass.getDeclaredFields())
+        .map(Field::getType)
+        .anyMatch(JpaTransactionManagerRule.class::equals);
+  }
+}

core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java

@@ -0,0 +1,52 @@
+// Copyright 2018 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.integration;
+
+import google.registry.model.registry.RegistryLockDaoTest;
+import google.registry.model.tmch.ClaimsListDaoTest;
+import google.registry.model.transaction.JpaTransactionManagerImplTest;
+import google.registry.model.transaction.JpaTransactionManagerRuleTest;
+import google.registry.persistence.BloomFilterConverterTest;
+import google.registry.persistence.CreateAutoTimestampConverterTest;
+import google.registry.persistence.UpdateAutoTimestampConverterTest;
+import google.registry.persistence.ZonedDateTimeConverterTest;
+import google.registry.schema.tld.PremiumListDaoTest;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+import org.junit.runners.Suite.SuiteClasses;
+
+/**
+ * Groups all tests that may depends on Cloud SQL schema. They will be run for server-schema
+ * compatibility check.
+ *
+ * <p>Schema dependency is approximated by the use of {@link
+ * google.registry.model.transaction.JpaTransactionManagerRule}.
+ *
+ * @see SqlIntegrationMembershipTest
+ */
+// TODO(weiminyu): refactor JpaTransactionManagerRule to eliminate false positives.
+@RunWith(Suite.class)
+@SuiteClasses({
+  BloomFilterConverterTest.class,
+  ClaimsListDaoTest.class,
+  CreateAutoTimestampConverterTest.class,
+  JpaTransactionManagerImplTest.class,
+  JpaTransactionManagerRuleTest.class,
+  PremiumListDaoTest.class,
+  RegistryLockDaoTest.class,
+  UpdateAutoTimestampConverterTest.class,
+  ZonedDateTimeConverterTest.class
+})
+public class SqlIntegrationTestSuite {}

core/src/test/java/google/registry/testing/AppEngineRule.java

@@ -193,6 +193,7 @@ public final class AppEngineRule extends ExternalResource {
         .setPassword("foo-BAR2")
         .setPhoneNumber("+1.3334445555")
         .setPhonePasscode("12345")
+        .setRegistryLockAllowed(false)
         .build();
   }
 
@@ -206,6 +207,7 @@ public final class AppEngineRule extends ExternalResource {
         .setPassword("password2")
         .setPhoneNumber("+1.2223334444")
         .setPhonePasscode("22222")
+        .setRegistryLockAllowed(true)
         .build();
   }
 
@@ -241,6 +243,19 @@ public final class AppEngineRule extends ExternalResource {
         .build();
   }
 
+  public static RegistrarContact makeRegistrarContact3() {
+    return new RegistrarContact.Builder()
+        .setParent(makeRegistrar2())
+        .setName("Marla Singer")
+        .setEmailAddress("Marla.Singer@crr.com")
+        .setPhoneNumber("+1.2128675309")
+        .setTypes(ImmutableSet.of(RegistrarContact.Type.TECH))
+        .setGaeUserId(THE_REGISTRAR_GAE_USER_ID)
+        .setAllowedToSetRegistryLockPassword(true)
+        .setRegistryLockPassword("hi")
+        .build();
+  }
+
   /** Hack to make sure AppEngineRule is always wrapped in a TemporaryFolder rule. */
   @Override
   public Statement apply(Statement base, Description description) {
@@ -412,6 +427,10 @@ public final class AppEngineRule extends ExternalResource {
   public static void loadInitialData() {
     persistSimpleResources(
         ImmutableList.of(
-            makeRegistrar1(), makeRegistrarContact1(), makeRegistrar2(), makeRegistrarContact2()));
+            makeRegistrar1(),
+            makeRegistrarContact1(),
+            makeRegistrar2(),
+            makeRegistrarContact2(),
+            makeRegistrarContact3()));
   }
 }

core/src/test/java/google/registry/tools/ResaveEnvironmentEntitiesCommandTest.java

@@ -21,6 +21,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.loadRegistrar;
 
+import com.google.common.collect.ImmutableSortedSet;
 import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.CommitLogManifest;
 import google.registry.model.ofy.CommitLogMutation;
@@ -62,13 +63,16 @@ public class ResaveEnvironmentEntitiesCommandTest
         transform(
             ofy().load().type(CommitLogMutation.class).list(),
             mutation -> ofy().load().fromEntity(mutation.getEntity()));
+    ImmutableSortedSet<RegistrarContact> theRegistrarContacts =
+        loadRegistrar("TheRegistrar").getContacts();
     assertThat(savedEntities)
         .containsExactly(
             // The Registrars and RegistrarContacts are created by AppEngineRule.
             loadRegistrar("TheRegistrar"),
             loadRegistrar("NewRegistrar"),
             Registry.get("tld"),
-            getOnlyElement(loadRegistrar("TheRegistrar").getContacts()),
+            theRegistrarContacts.first(),
+            theRegistrarContacts.last(),
             getOnlyElement(loadRegistrar("NewRegistrar").getContacts()));
   }
 

core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java

@@ -188,6 +188,33 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
   public void testSuccess_setRegistryLockPassword() {
+    addPasswordToTechContact();
+    techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH));
+    assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue();
+    assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+  }
+
+  @Test
+  public void testSuccess_setRegistryLockPassword_notOverriddenLater() {
+    addPasswordToTechContact();
+    techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH));
+    assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue();
+
+    techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH));
+    Map<String, Object> techContactMap = techContact.toJsonMap();
+    techContactMap.put("name", "Some Other Name");
+    Map<String, Object> reqJson = loadRegistrar(CLIENT_ID).toJsonMap();
+    reqJson.put(
+        "contacts",
+        ImmutableList.of(AppEngineRule.makeRegistrarContact2().toJsonMap(), techContactMap));
+    Map<String, Object> response =
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson));
+    assertThat(response).containsAtLeastEntriesIn(ImmutableMap.of("status", "SUCCESS"));
+    techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH));
+    assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue();
+  }
+
+  private void addPasswordToTechContact() {
     techContact =
         persistResource(techContact.asBuilder().setAllowedToSetRegistryLockPassword(true).build());
     Map<String, Object> contactMap = techContact.toJsonMap();
@@ -199,9 +226,6 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> response =
         action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson));
     assertThat(response).containsAtLeastEntriesIn(ImmutableMap.of("status", "SUCCESS"));
-    techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH));
-    assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue();
-    assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
   }
 
   @Test
@@ -275,7 +299,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
             "results",
             ImmutableList.of(),
             "message",
-            "Registrar contact not allowed to set registry lock password");
+            "Cannot set isAllowedToSetRegistryLockPassword through UI");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException");
   }
 }

core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java

@@ -35,6 +35,7 @@ import org.junit.Test;
 import org.junit.rules.Timeout;
 import org.junit.runner.RunWith;
 import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
 /** WebDriver tests for Registrar Console UI. */
 @RunWith(RepeatableRunner.class)
@@ -78,6 +79,12 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
     assertThat(driver.waitForElement(By.id(eltId)).isDisplayed()).isFalse();
   }
 
+  /** Checks that searching with the given By produces at least one element with the given text. */
+  void assertEltTextPresent(By by, String toFind) {
+    assertThat(driver.findElements(by).stream().map(WebElement::getText).anyMatch(toFind::equals))
+        .isTrue();
+  }
+
   @Test
   public void testEditButtonsVisibility_owner() throws Throwable {
     driver.get(server.getUrl("/registrar#whois-settings"));
@@ -190,9 +197,11 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
     ImmutableList<RegistrarContact> contacts =
         server.runInAppEngineEnvironment(
             () -> loadRegistrar("TheRegistrar").getContacts().asList());
-    assertEltText("contacts[0].name", contacts.get(0).getName());
-    assertEltText("contacts[0].emailAddress", contacts.get(0).getEmailAddress());
-    assertEltText("contacts[0].phoneNumber", contacts.get(0).getPhoneNumber());
+    for (RegistrarContact contact : contacts) {
+      assertEltTextPresent(By.id("contacts[0].name"), contact.getName());
+      assertEltTextPresent(By.id("contacts[0].emailAddress"), contact.getEmailAddress());
+      assertEltTextPresent(By.id("contacts[0].phoneNumber"), contact.getPhoneNumber());
+    }
   }
 
   @Test

core/src/test/resources/google/registry/model/transaction/cleanup_database.sql

@@ -0,0 +1,16 @@
+-- Copyright 2019 The Nomulus Authors. All Rights Reserved.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+DROP DATABASE IF EXISTS postgres;
+CREATE DATABASE postgres;

core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt

@@ -11,9 +11,10 @@ emailAddress: the.registrar@example.com -> thase@the.registrar
 url: http://my.fake.url -> http://my.new.url
 contacts:
     ADDED:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
     REMOVED:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null},
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Marla Singer, emailAddress=Marla.Singer@crr.com, phoneNumber=+1.2128675309, faxNumber=null, types=[TECH], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
     FINAL CONTENTS:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}

db/build.gradle

@@ -94,6 +94,7 @@ ext {
     def command =
         """gsutil cp \
            gs://domain-registry${env}-cloudsql-credentials/${role}_credential.enc - | \
+           base64 -d | \
            gcloud kms decrypt --location global --keyring nomulus \
            --key sql-credentials-on-gcs-key --plaintext-file=- \
            --ciphertext-file=- \
@@ -118,11 +119,11 @@ artifacts {
 publishing {
   repositories {
     maven {
-      url project.schema_jar_repo
+      url project.schema_publish_repo
     }
   }
   publications {
-    schemaOrmPublication(MavenPublication) {
+    sqlSchemaPublication(MavenPublication) {
       groupId 'google.registry'
       artifactId 'schema'
       version project.schema_version

db/src/main/resources/sql/flyway/V8__registry_lock_registrar_index.sql

@@ -0,0 +1,16 @@
+-- Copyright 2019 The Nomulus Authors. All Rights Reserved.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+create index if not exists idx_registry_lock_registrar_id ON "RegistryLock"
+   using btree (registrar_id);

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -22,6 +22,7 @@
     create table "ClaimsList" (
        revision_id  bigserial not null,
         creation_timestamp timestamptz not null,
+        tmdb_generation_time timestamptz not null,
         primary key (revision_id)
     );
 
@@ -160,6 +161,8 @@
     alter table if exists "Domain_GracePeriod" 
        add constraint UK_4ps2u4y8i5r91wu2n1x2xea28 unique (grace_periods_id);
 create index premiumlist_name_idx on "PremiumList" (name);
+create index idx_registry_lock_verification_code on "RegistryLock" (verification_code);
+create index idx_registry_lock_registrar_id on "RegistryLock" (registrar_id);
 
     alter table if exists "RegistryLock" 
        add constraint idx_registry_lock_repo_id_revision_id unique (repo_id, revision_id);

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -223,6 +223,13 @@ ALTER TABLE ONLY public."RegistryLock"
     ADD CONSTRAINT idx_registry_lock_repo_id_revision_id UNIQUE (repo_id, revision_id);
 
 
+--
+-- Name: idx_registry_lock_registrar_id; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idx_registry_lock_registrar_id ON public."RegistryLock" USING btree (registrar_id);
+
+
 --
 -- Name: idx_registry_lock_verification_code; Type: INDEX; Schema: public; Owner: -
 --

dependencies.gradle

@@ -79,6 +79,7 @@ ext {
       'com.sun.xml.bind:jaxb-xjc:2.2.11',
       'com.thoughtworks.qdox:qdox:1.12.1',
       'dnsjava:dnsjava:2.1.7',
+      'io.github.classgraph:classgraph:4.8.52',
       'io.github.java-diff-utils:java-diff-utils:4.0',
       'io.netty:netty-buffer:4.1.31.Final',
       'io.netty:netty-codec:4.1.31.Final',

gradle.properties

@@ -24,5 +24,5 @@ dbPassword=
 
 # Maven repository of the Cloud SQL schema jar, which contains the
 # SQL DDL scripts.
-schema_jar_repo=
+schema_publish_repo=
 schema_version=

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.2-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.3-all.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

package.json

@@ -21,10 +21,10 @@
   },
   "devDependencies": {
     "jasmine-core": "3.4.0",
-    "karma": "3.0.0",
+    "karma": "^4.4.1",
     "karma-chrome-launcher": "2.2.0",
     "karma-closure": "0.1.3",
     "karma-jasmine": "2.0.1",
-    "puppeteer": "1.17.0"
+    "puppeteer": "1.20.0"
   }
 }

prober/gradle/dependency-locks/testCompile.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

prober/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -130,6 +130,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

prober/gradle/dependency-locks/testRuntime.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

prober/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

proxy/gradle/dependency-locks/testCompile.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

proxy/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -130,6 +130,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

proxy/gradle/dependency-locks/testRuntime.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

proxy/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

util/gradle/dependency-locks/testCompile.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

util/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -130,6 +130,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

util/gradle/dependency-locks/testRuntime.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

util/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -141,6 +141,7 @@ commons-codec:commons-codec:1.11
 commons-logging:commons-logging:1.2
 dnsjava:dnsjava:2.1.7
 io.dropwizard.metrics:metrics-core:3.1.2
+io.github.classgraph:classgraph:4.8.52
 io.grpc:grpc-all:1.17.1
 io.grpc:grpc-alts:1.17.1
 io.grpc:grpc-auth:1.17.1

util/src/main/java/google/registry/util/Retrier.java

@@ -70,32 +70,8 @@ public class Retrier implements Serializable {
    *
    * @return <V> the value returned by the {@link Callable}.
    */
-  private <V> V callWithRetry(
-      Callable<V> callable,
-      FailureReporter failureReporter,
-      Predicate<Throwable> isRetryable) {
-    int failures = 0;
-    while (true) {
-      try {
-        return callable.call();
-      } catch (Throwable e) {
-        if (++failures == attempts || !isRetryable.test(e)) {
-          throwIfUnchecked(e);
-          throw new RuntimeException(e);
-        }
-        failureReporter.beforeRetry(e, failures, attempts);
-        try {
-          // Wait 100ms on the first attempt, doubling on each subsequent attempt.
-          sleeper.sleep(Duration.millis(pow(2, failures) * 100));
-        } catch (InterruptedException e2) {
-          // Since we're not rethrowing InterruptedException, set the interrupt state on the thread
-          // so the next blocking operation will know to abort the thread.
-          Thread.currentThread().interrupt();
-          throwIfUnchecked(e);
-          throw new RuntimeException(e);
-        }
-      }
-    }
+  public <V> V callWithRetry(Callable<V> callable, Predicate<Throwable> isRetryable) {
+    return callWithRetry(callable, LOGGING_FAILURE_REPORTER, isRetryable);
   }
 
   /**
@@ -169,6 +145,34 @@ public class Retrier implements Serializable {
     callWithRetry(callable.asCallable(), failureReporter, retryableError, moreRetryableErrors);
   }
 
+  private <V> V callWithRetry(
+      Callable<V> callable,
+      FailureReporter failureReporter,
+      Predicate<Throwable> isRetryable) {
+    int failures = 0;
+    while (true) {
+      try {
+        return callable.call();
+      } catch (Throwable e) {
+        if (++failures == attempts || !isRetryable.test(e)) {
+          throwIfUnchecked(e);
+          throw new RuntimeException(e);
+        }
+        failureReporter.beforeRetry(e, failures, attempts);
+        try {
+          // Wait 100ms on the first attempt, doubling on each subsequent attempt.
+          sleeper.sleep(Duration.millis(pow(2, failures) * 100));
+        } catch (InterruptedException e2) {
+          // Since we're not rethrowing InterruptedException, set the interrupt state on the thread
+          // so the next blocking operation will know to abort the thread.
+          Thread.currentThread().interrupt();
+          throwIfUnchecked(e);
+          throw new RuntimeException(e);
+        }
+      }
+    }
+  }
+
   private static final FailureReporter LOGGING_FAILURE_REPORTER =
       (thrown, failures, maxAttempts) ->
           logger.atInfo().withCause(thrown).log(

util/src/test/java/google/registry/util/RetrierTest.java

@@ -38,7 +38,7 @@ public class RetrierTest {
     }
   }
 
-  /** Test object that always throws an exception with the current count. */
+  /** Test object that throws CountingExceptions up to a given limit, then succeeds. */
   static class CountingThrower implements Callable<Integer> {
 
     int count = 0;
@@ -125,4 +125,21 @@ public class RetrierTest {
         .isEqualTo(0);
     assertThat(reporter.numBeforeRetry).isEqualTo(0);
   }
+
+  @Test
+  public void testRetryPredicate_succeedsWhenRetries() {
+    // Throws a retryable "1" exception is retryable, and then it succeeds on "1".
+    assertThat(retrier.callWithRetry(new CountingThrower(1), e -> e.getMessage().equals("1")))
+        .isEqualTo(1);
+  }
+
+  @Test
+  public void testRetryPredicate_failsWhenDoesntRetry() {
+    // Throws a retryable "1" exception, then a non-retryable "2" exception, resulting in failure.
+    CountingException ex =
+        assertThrows(
+            CountingException.class,
+            () -> retrier.callWithRetry(new CountingThrower(2), e -> e.getMessage().equals("1")));
+    assertThat(ex).hasMessageThat().isEqualTo("2");
+  }
 }