Enable filtering across all test tasks (#311)

The segregated test targets in core break the --tests filter.  Fix this by
defining a "testFilter" property and creating the FilteringTest task type that
applies it to the property set by "--tests".

core/build.gradle

@@ -558,10 +558,38 @@ artifacts {
   testRuntime testJar
 }
 
-task fragileTest(type: Test) {
+/**
+ * We have to break out the test suites because some of the tests conflict
+ * with one another, but unfortunately this breaks the "--tests" flag.  The
+ * --tests flag only applies to the task named on the command line (usually
+ * just "test"), not for all tasks of type "Test".
+ *
+ * As a better solution, FilteringTest sets testNameIncludePatterns (the
+ * internal property that --tests sets) from the value of the "testFilter"
+ * property, allowing us to filter across all the tests in core without
+ * explicitly specifying a test task or causing errors because there are no
+ * matching tests in the main task.
+ *
+ * To use it, define "testFilter" to be a comma-separated collection of class
+ * names (wildcards are allowed):
+ *
+ *   ./gradlew test -P testFilter=*.FooBar,google.registry.tools.ShellCommandTest
+ */
+class FilteringTest extends Test {
+
+  void setTests(List<String> tests) {
+    // Common exclude pattern. See README in parent directory for explanation.
+    exclude "**/*TestCase.*", "**/*TestSuite.*"
+    include tests
+    if (project.testFilter) {
+      testNameIncludePatterns = project.testFilter.split(',')
+    }
+  }
+}
+
+task fragileTest(type: FilteringTest) {
   // Common exclude pattern. See README in parent directory for explanation.
-  exclude "**/*TestCase.*", "**/*TestSuite.*"
-  include fragileTestPatterns
+  tests = fragileTestPatterns
 
   if (rootProject.findProperty("skipDockerIncompatibleTests") == "true") {
     exclude dockerIncompatibleTestPatterns
@@ -571,10 +599,8 @@ task fragileTest(type: Test) {
   forkEvery 1
 }
 
-task outcastTest(type: Test) {
-  // Common exclude pattern. See README in parent directory for explanation.
-  exclude "**/*TestCase.*", "**/*TestSuite.*"
-  include outcastTestPatterns
+task outcastTest(type: FilteringTest) {
+  tests = outcastTestPatterns
 
   // Sets the maximum number of test executors that may exist at the same time.
   maxParallelForks 5
@@ -630,10 +656,8 @@ task registryTool(type: JavaExec) {
   }
 }
 
-task generateGoldenImages(type: Test) {
-  // Common exclude pattern. See README in parent directory for explanation.
-  exclude "**/*TestCase.*", "**/*TestSuite.*"
-  include "**/webdriver/*"
+task generateGoldenImages(type: FilteringTest) {
+  tests = ["**/webdriver/*"]
 
   // Sets the maximum number of test executors that may exist at the same time.
   maxParallelForks 5

core/src/main/java/google/registry/model/tmch/ClaimsListDao.java

@@ -0,0 +1,72 @@
+// 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.
+
+package google.registry.model.tmch;
+
+import static google.registry.model.transaction.TransactionManagerFactory.jpaTm;
+
+import com.google.common.flogger.FluentLogger;
+import google.registry.schema.tmch.ClaimsList;
+import javax.persistence.EntityManager;
+
+/** Data access object for {@link ClaimsList}. */
+public class ClaimsListDao {
+
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private static void save(ClaimsList claimsList) {
+    jpaTm().transact(() -> jpaTm().getEntityManager().persist(claimsList));
+  }
+
+  /**
+   * Try to save the given {@link ClaimsList} into Cloud SQL. If the save fails, the error will be
+   * logged but no exception will be thrown.
+   *
+   * <p>This method is used during the dual-write phase of database migration as Datastore is still
+   * the authoritative database.
+   */
+  public static void trySave(ClaimsList claimsList) {
+    try {
+      ClaimsListDao.save(claimsList);
+      logger.atInfo().log(
+          "Inserted %,d claims into Cloud SQL, created at %s",
+          claimsList.getLabelsToKeys().size(), claimsList.getTmdbGenerationTime());
+    } catch (Throwable e) {
+      logger.atSevere().withCause(e).log("Error inserting claims into Cloud SQL");
+    }
+  }
+
+  /**
+   * Returns the current revision of the {@link ClaimsList} in Cloud SQL. Throws exception if there
+   * is no claims in the table.
+   */
+  public static ClaimsList getCurrent() {
+    return jpaTm()
+        .transact(
+            () -> {
+              EntityManager em = jpaTm().getEntityManager();
+              Long revisionId =
+                  em.createQuery("SELECT MAX(revisionId) FROM ClaimsList", Long.class)
+                      .getSingleResult();
+              return em.createQuery(
+                      "FROM ClaimsList cl LEFT JOIN FETCH cl.labelsToKeys WHERE cl.revisionId ="
+                          + " :revisionId",
+                      ClaimsList.class)
+                  .setParameter("revisionId", revisionId)
+                  .getSingleResult();
+            });
+  }
+
+  private ClaimsListDao() {}
+}

core/src/main/java/google/registry/model/tmch/ClaimsListShard.java

@@ -217,8 +217,7 @@ public class ClaimsListShard extends ImmutableObject {
             });
   }
 
-  public static ClaimsListShard create(
-      DateTime creationTime, ImmutableMap<String, String> labelsToKeys) {
+  public static ClaimsListShard create(DateTime creationTime, Map<String, String> labelsToKeys) {
     ClaimsListShard instance = new ClaimsListShard();
     instance.id = allocateId();
     instance.creationTime = checkNotNull(creationTime);

core/src/main/java/google/registry/schema/tmch/ClaimsList.java

@@ -15,7 +15,11 @@
 package google.registry.schema.tmch;
 
 import static com.google.common.base.Preconditions.checkState;
+import static google.registry.util.DateTimeUtils.toJodaDateTime;
+import static google.registry.util.DateTimeUtils.toZonedDateTime;
 
+import google.registry.model.CreateAutoTimestamp;
+import google.registry.model.ImmutableObject;
 import java.time.ZonedDateTime;
 import java.util.Map;
 import java.util.Optional;
@@ -29,6 +33,7 @@ import javax.persistence.Id;
 import javax.persistence.JoinColumn;
 import javax.persistence.MapKeyColumn;
 import javax.persistence.Table;
+import org.joda.time.DateTime;
 
 /**
  * A list of TMCH claims labels and their associated claims keys.
@@ -40,26 +45,29 @@ import javax.persistence.Table;
  * highest {@link #revisionId}.
  */
 @Entity
-@Table(name = "ClaimsList")
-public class ClaimsList {
+@Table
+public class ClaimsList extends ImmutableObject {
   @Id
   @GeneratedValue(strategy = GenerationType.IDENTITY)
-  @Column(name = "revision_id")
+  @Column
   private Long revisionId;
 
-  @Column(name = "creation_timestamp", nullable = false)
-  private ZonedDateTime creationTimestamp;
+  @Column(nullable = false)
+  private CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
+
+  @Column(nullable = false)
+  private ZonedDateTime tmdbGenerationTime;
 
   @ElementCollection
   @CollectionTable(
       name = "ClaimsEntry",
-      joinColumns = @JoinColumn(name = "revision_id", referencedColumnName = "revision_id"))
-  @MapKeyColumn(name = "domain_label", nullable = false)
-  @Column(name = "claim_key", nullable = false)
+      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
+  @MapKeyColumn(name = "domainLabel", nullable = false)
+  @Column(name = "claimKey", nullable = false)
   private Map<String, String> labelsToKeys;
 
-  private ClaimsList(ZonedDateTime creationTimestamp, Map<String, String> labelsToKeys) {
-    this.creationTimestamp = creationTimestamp;
+  private ClaimsList(ZonedDateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
+    this.tmdbGenerationTime = tmdbGenerationTime;
     this.labelsToKeys = labelsToKeys;
   }
 
@@ -67,9 +75,8 @@ public class ClaimsList {
   private ClaimsList() {}
 
   /** Constructs a {@link ClaimsList} object. */
-  public static ClaimsList create(
-      ZonedDateTime creationTimestamp, Map<String, String> labelsToKeys) {
-    return new ClaimsList(creationTimestamp, labelsToKeys);
+  public static ClaimsList create(DateTime creationTimestamp, Map<String, String> labelsToKeys) {
+    return new ClaimsList(toZonedDateTime(creationTimestamp), labelsToKeys);
   }
 
   /** Returns the revision id of this claims list, or throws exception if it is null. */
@@ -79,9 +86,14 @@ public class ClaimsList {
     return revisionId;
   }
 
+  /** Returns the TMDB generation time of this claims list. */
+  public DateTime getTmdbGenerationTime() {
+    return toJodaDateTime(tmdbGenerationTime);
+  }
+
   /** Returns the creation time of this claims list. */
-  public ZonedDateTime getCreationTimestamp() {
-    return creationTimestamp;
+  public DateTime getCreationTimestamp() {
+    return creationTimestamp.getTimestamp();
   }
 
   /** Returns an {@link Map} mapping domain label to its lookup key. */

core/src/main/java/google/registry/tmch/ClaimsListParser.java

@@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
-import google.registry.model.tmch.ClaimsListShard;
+import google.registry.schema.tmch.ClaimsList;
 import java.util.List;
 import org.joda.time.DateTime;
 
@@ -34,11 +34,11 @@ import org.joda.time.DateTime;
 public class ClaimsListParser {
 
   /**
-   * Converts the lines from the DNL CSV file into a {@link ClaimsListShard} object.
+   * Converts the lines from the DNL CSV file into a {@link ClaimsList} object.
    *
    * <p>Please note that this does <b>not</b> insert the object into Datastore.
    */
-  public static ClaimsListShard parse(List<String> lines) {
+  public static ClaimsList parse(List<String> lines) {
     ImmutableMap.Builder<String, String> builder = new ImmutableMap.Builder<>();
 
     // First line: <version>,<DNL List creation datetime>
@@ -74,6 +74,6 @@ public class ClaimsListParser {
       builder.put(label, lookupKey);
     }
 
-    return ClaimsListShard.create(creationTime, builder.build());
+    return ClaimsList.create(creationTime, builder.build());
   }
 }

core/src/main/java/google/registry/tmch/TmchDnlAction.java

@@ -18,9 +18,11 @@ import static google.registry.request.Action.Method.POST;
 
 import com.google.common.flogger.FluentLogger;
 import google.registry.keyring.api.KeyModule.Key;
+import google.registry.model.tmch.ClaimsListDao;
 import google.registry.model.tmch.ClaimsListShard;
 import google.registry.request.Action;
 import google.registry.request.auth.Auth;
+import google.registry.schema.tmch.ClaimsList;
 import java.io.IOException;
 import java.security.SignatureException;
 import java.util.List;
@@ -54,10 +56,14 @@ public final class TmchDnlAction implements Runnable {
     } catch (SignatureException | IOException | PGPException e) {
       throw new RuntimeException(e);
     }
-    ClaimsListShard claims = ClaimsListParser.parse(lines);
-    claims.save();
+    ClaimsList claims = ClaimsListParser.parse(lines);
+    ClaimsListShard claimsListShard =
+        ClaimsListShard.create(claims.getTmdbGenerationTime(), claims.getLabelsToKeys());
+    claimsListShard.save();
     logger.atInfo().log(
         "Inserted %,d claims into Datastore, created at %s",
-        claims.size(), claims.getCreationTime());
+        claimsListShard.size(), claimsListShard.getCreationTime());
+
+    ClaimsListDao.trySave(claims);
   }
 }

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

@@ -21,7 +21,9 @@ import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.common.base.Joiner;
 import com.google.common.io.Files;
+import google.registry.model.tmch.ClaimsListDao;
 import google.registry.model.tmch.ClaimsListShard;
+import google.registry.schema.tmch.ClaimsList;
 import google.registry.tmch.ClaimsListParser;
 import java.io.File;
 import java.io.IOException;
@@ -35,9 +37,14 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
   @Parameter(description = "Claims list filename")
   private List<String> mainParameters = new ArrayList<>();
 
+  @Parameter(
+      names = {"--also_cloud_sql"},
+      description = "Persist claims list to Cloud SQL in addition to Datastore; defaults to false.")
+  boolean alsoCloudSql;
+
   private String claimsListFilename;
 
-  private ClaimsListShard claimsList;
+  private ClaimsList claimsList;
 
   @Override
   protected void init() throws IOException {
@@ -56,7 +63,10 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
 
   @Override
   public String execute() {
-    claimsList.save();
+    ClaimsListShard.create(claimsList.getTmdbGenerationTime(), claimsList.getLabelsToKeys()).save();
+    if (alsoCloudSql) {
+      ClaimsListDao.trySave(claimsList);
+    }
     return String.format("Successfully uploaded claims list %s", claimsListFilename);
   }
 }

core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java

@@ -0,0 +1,95 @@
+// 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.
+
+package google.registry.model.tmch;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.JUnitBackports.assertThrows;
+
+import com.google.common.collect.ImmutableMap;
+import google.registry.model.transaction.JpaTransactionManagerRule;
+import google.registry.schema.tmch.ClaimsList;
+import google.registry.testing.FakeClock;
+import javax.persistence.NoResultException;
+import javax.persistence.PersistenceException;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit tests for {@link ClaimsListDao}. */
+@RunWith(JUnit4.class)
+public class ClaimsListDaoTest {
+
+  private FakeClock fakeClock = new FakeClock();
+
+  @Rule
+  public final JpaTransactionManagerRule jpaTmRule =
+      new JpaTransactionManagerRule.Builder().build();
+
+  @Test
+  public void trySave_insertsClaimsListSuccessfully() {
+    ClaimsList claimsList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListDao.trySave(claimsList);
+    ClaimsList insertedClaimsList = ClaimsListDao.getCurrent();
+    assertClaimsListEquals(claimsList, insertedClaimsList);
+    assertThat(insertedClaimsList.getCreationTimestamp())
+        .isEqualTo(jpaTmRule.getTxnClock().nowUtc());
+  }
+
+  @Test
+  public void trySave_noExceptionThrownWhenSaveFail() {
+    ClaimsList claimsList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListDao.trySave(claimsList);
+    ClaimsList insertedClaimsList = ClaimsListDao.getCurrent();
+    assertClaimsListEquals(claimsList, insertedClaimsList);
+    // Save ClaimsList with existing revisionId should fail because revisionId is the primary key.
+    ClaimsListDao.trySave(insertedClaimsList);
+  }
+
+  @Test
+  public void trySave_claimsListWithNoEntries() {
+    ClaimsList claimsList = ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of());
+    ClaimsListDao.trySave(claimsList);
+    ClaimsList insertedClaimsList = ClaimsListDao.getCurrent();
+    assertClaimsListEquals(claimsList, insertedClaimsList);
+    assertThat(insertedClaimsList.getLabelsToKeys()).isEmpty();
+  }
+
+  @Test
+  public void getCurrent_throwsNoResultExceptionIfTableIsEmpty() {
+    PersistenceException thrown =
+        assertThrows(PersistenceException.class, () -> ClaimsListDao.getCurrent());
+    assertThat(thrown).hasCauseThat().isInstanceOf(NoResultException.class);
+  }
+
+  @Test
+  public void getCurrent_returnsLatestClaims() {
+    ClaimsList oldClaimsList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsList newClaimsList =
+        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4"));
+    ClaimsListDao.trySave(oldClaimsList);
+    ClaimsListDao.trySave(newClaimsList);
+    assertClaimsListEquals(newClaimsList, ClaimsListDao.getCurrent());
+  }
+
+  private void assertClaimsListEquals(ClaimsList left, ClaimsList right) {
+    assertThat(left.getRevisionId()).isEqualTo(right.getRevisionId());
+    assertThat(left.getTmdbGenerationTime()).isEqualTo(right.getTmdbGenerationTime());
+    assertThat(left.getLabelsToKeys()).isEqualTo(right.getLabelsToKeys());
+  }
+}

db/build.gradle

@@ -12,18 +12,45 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import org.gradle.api.internal.tasks.userinput.UserInputHandler
+
 plugins {
   id "org.flywaydb.flyway" version "6.0.1"
   id 'maven-publish'
 }
 
 ext {
+  Set restrictedDbEnv =
+      [ 'sandbox', 'production' ].asUnmodifiable()
+  Set allDbEnv =
+      [ 'alpha', 'crash' ].plus(restrictedDbEnv).asUnmodifiable()
+
   def dbServerProperty = 'dbServer'
   def dbNameProperty = 'dbName'
 
-  def dbServer = findProperty(dbServerProperty)
+  def dbServer = findProperty(dbServerProperty).toString().toLowerCase()
   def dbName = findProperty(dbNameProperty)
 
+  reconfirmRestrictedDbEnv = {
+    if (!restrictedDbEnv.contains(dbServer)) {
+      return
+    }
+    // For restricted environments, ask the user to type again to confirm.
+    // The following statement uses Gradle internal API to get around the
+    // missing console bug when Gradle Daemon is in use. Another option is
+    // to use the ant.input task. For details please refer to
+    // https://github.com/gradle/gradle/issues/1251.
+    def dbServerAgain = services.get(UserInputHandler.class).askQuestion(
+        """\
+           Are you sure? Operating on ${dbServer} from desktop is unsafe.
+           Please type '${dbServer}' again to proceed: """.stripIndent(),
+        '').trim()
+    if (dbServer != dbServerAgain) {
+      throw new RuntimeException(
+          "Failed to confirm for restricted database environment. Operation aborted.")
+    }
+  }
+
   getAccessInfoByHostPort = { hostAndPort ->
     return [
         url: "jdbc:postgresql://${hostAndPort}/${dbName}",
@@ -31,8 +58,8 @@ ext {
         password: findProperty('dbPassword')]
   }
 
-  getSocketFactoryAccessInfo = {
-    def cred = getCloudSqlCredential('alpha', 'superuser').split(' ')
+  getSocketFactoryAccessInfo = { env ->
+    def cred = getCloudSqlCredential(env, 'admin').split(' ')
     def sqlInstance = cred[0]
     return [
         url: """\
@@ -46,11 +73,10 @@ ext {
   }
 
   getJdbcAccessInfo = {
-    switch (dbServer.toString().toLowerCase()) {
-      case 'alpha':
-        return getSocketFactoryAccessInfo()
-      default:
-        return getAccessInfoByHostPort(dbServer)
+    if (allDbEnv.contains(dbServer)) {
+      return getSocketFactoryAccessInfo(dbServer)
+    } else {
+      return getAccessInfoByHostPort(dbServer)
     }
   }
 
@@ -62,13 +88,16 @@ ext {
   // later).
   getCloudSqlCredential = { env, role ->
     env = env == 'production' ? '' : "-${env}"
+    def keyProject = env == '-crash'
+        ? 'domain-registry-crash-kms-keys'
+        : "domain-registry${env}-keys"
     def command =
         """gsutil cp \
-           gs://domain-registry${env}-cloudsql-credentials/${role}.enc - | \
+           gs://domain-registry${env}-cloudsql-credentials/${role}_credential.enc - | \
            gcloud kms decrypt --location global --keyring nomulus \
            --key sql-credentials-on-gcs-key --plaintext-file=- \
            --ciphertext-file=- \
-           --project=domain-registry${env}-keys"""
+           --project=${keyProject}"""
 
     return execInBash(command, '/tmp')
   }
@@ -114,6 +143,13 @@ flyway {
   locations = [ "classpath:sql/flyway" ]
 }
 
+tasks.flywayMigrate.dependsOn(
+    tasks.create('confirmMigrateOnRestrictedDb') {
+      doLast {
+        project.ext.reconfirmRestrictedDbEnv()
+      }
+    })
+
 dependencies {
   def deps = rootProject.dependencyMap
 

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

@@ -0,0 +1,15 @@
+-- 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.
+
+alter table "ClaimsList" add column if not exists tmdb_generation_time timestamp with time zone not null;

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

@@ -50,7 +50,8 @@ CREATE TABLE public."ClaimsEntry" (
 
 CREATE TABLE public."ClaimsList" (
     revision_id bigint NOT NULL,
-    creation_timestamp timestamp with time zone NOT NULL
+    creation_timestamp timestamp with time zone NOT NULL,
+    tmdb_generation_time timestamp with time zone NOT NULL
 );
 
 

db/src/main/resources/sql/user/initialize_roles.sql

@@ -16,7 +16,7 @@
 -- This script should run once under the **'postgres'** user before any other
 -- roles or users are created.
 
-# Prevent backdoor grants through the implicit 'public' role.
+-- Prevent backdoor grants through the implicit 'public' role.
 REVOKE ALL PRIVILEGES ON SCHEMA public from public;
 
 CREATE ROLE readonly;

gradle.properties

@@ -8,6 +8,9 @@ verboseTestOutput=false
 flowDocsFile=
 enableDependencyLocking=true
 
+# Comma separated list of test patterns, if specified run only these.
+testFilter=
+
 # GAE Environment for deployment and staging.
 environment=