Remove more tests from dockerIncompatibleGroup

We don't want to override toDiffableFieldMap because (per the javadoc)
that is supposed to contain sensitive information. So, we should just
remove it before sending it out.

core/build.gradle

@@ -46,17 +46,15 @@ def outcastTestPatterns = [
 // Tests that fail when running Gradle in a docker container, e. g. when
 // building the release artifacts in Google Cloud Build.
 def dockerIncompatibleTestPatterns = [
-    // The webdriver tests start headless Chrome in a Docker container,
-    // resulting in Docker-in-Docker complications.  Likewise,
-    // GenerateSqlSchemaCommandTest launches postgresql in a docker container.
-    "google/registry/webdriver/*",
-    "google/registry/tools/GenerateSqlSchemaCommandTest.*",
-    // PathParameterTest includes tests which validate that file permissions are
-    // respected. However when running in Docker the user is root by default, so
-    // every file is read/write-able. There is no way to exclude specific test
-    // methods, so we exclude the whole test class.
-    "google/registry/tools/params/PathParameterTest.*",
-    "google/registry/persistence/PersistenceModuleTest.*",
+        // The webdriver tests start headless Chrome in a Docker container,
+        // resulting in Docker-in-Docker complications.  Likewise,
+        // GenerateSqlSchemaCommandTest launches postgresql in a docker container.
+        "google/registry/webdriver/*",
+        // PathParameterTest includes tests which validate that file permissions are
+        // respected. However when running in Docker the user is root by default, so
+        // every file is read/write-able. There is no way to exclude specific test
+        // methods, so we exclude the whole test class.
+        "google/registry/tools/params/PathParameterTest.*",
 ]
 
 // Tests that conflict with members of both the main test suite and the

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/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/ui/server/registrar/RegistrarSettingsAction.java

@@ -240,11 +240,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());

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/resources/google/registry/ui/server/registrar/update_registrar_email.txt

@@ -11,9 +11,9 @@ 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=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/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: -
 --

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"
   }
 }