Prevent test from changing golden schema (#337)

When we add the extra test entity to the current JpaTransactionManagerRule by calling jpaRule.withEntityClass(TestEntity.class) and jpaRule.withProperty(Environment.HBM2DDL_AUTO, "update"), the rule would override the golden database scheme with the schema derived from the current entity class(This is an expected behavior by enabling HBM2DDL_AUTO). This behavior prevents us from detecting breaking changes in ORM classes.

This PR fixed the issue by adding HibernateSchemaExporter to export the DDL script for given extra entity class, and make JpaTransactionManagerRule execute the DDL script to create the corresponding table for the extra entity when initializing the database. This PR also simplified the code when adding extra entity class for testing. For now, you don't need to(and shouldn't) call jpaRule.withProperty(Environment.HBM2DDL_AUTO, "update").

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

@@ -0,0 +1,104 @@
+// 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.persistence;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import java.io.File;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.stream.Stream;
+import javax.persistence.AttributeConverter;
+import org.hibernate.boot.MetadataSources;
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
+import org.hibernate.cfg.Environment;
+import org.hibernate.jpa.boot.internal.ParsedPersistenceXmlDescriptor;
+import org.hibernate.jpa.boot.internal.PersistenceXmlParser;
+import org.hibernate.tool.hbm2ddl.SchemaExport;
+import org.hibernate.tool.schema.TargetType;
+
+/** Utility class to export DDL script for given {@link javax.persistence.Entity} classes. */
+public class HibernateSchemaExporter {
+  private final String jdbcUrl;
+  private final String username;
+  private final String password;
+
+  private HibernateSchemaExporter(String jdbcUrl, String username, String password) {
+    this.jdbcUrl = jdbcUrl;
+    this.username = username;
+    this.password = password;
+  }
+
+  /** Constructs a {@link HibernateSchemaExporter} instance. */
+  public static HibernateSchemaExporter create(String jdbcUrl, String username, String password) {
+    return new HibernateSchemaExporter(jdbcUrl, username, password);
+  }
+
+  /** Exports DDL script to the {@code outputFile} for the given {@code entityClasses}. */
+  public void export(ImmutableList<Class> entityClasses, File outputFile) {
+    // Configure Hibernate settings.
+    Map<String, String> settings = Maps.newHashMap();
+    settings.put(Environment.DIALECT, NomulusPostgreSQLDialect.class.getName());
+    settings.put(Environment.URL, jdbcUrl);
+    settings.put(Environment.USER, username);
+    settings.put(Environment.PASS, password);
+    settings.put(Environment.HBM2DDL_AUTO, "none");
+    settings.put(Environment.SHOW_SQL, "true");
+    settings.put(
+        Environment.PHYSICAL_NAMING_STRATEGY, NomulusNamingStrategy.class.getCanonicalName());
+
+    MetadataSources metadata =
+        new MetadataSources(new StandardServiceRegistryBuilder().applySettings(settings).build());
+
+    // Note that we need to also add all converters to the Hibernate context because
+    // the entity class may use the customized type.
+    Stream.concat(entityClasses.stream(), findAllConverters().stream())
+        .forEach(metadata::addAnnotatedClass);
+
+    SchemaExport export = new SchemaExport();
+    export.setHaltOnError(true);
+    export.setFormat(true);
+    export.setDelimiter(";");
+    export.setOutputFile(outputFile.getAbsolutePath());
+    export.createOnly(EnumSet.of(TargetType.SCRIPT), metadata.buildMetadata());
+  }
+
+  private ImmutableList<Class> findAllConverters() {
+    ParsedPersistenceXmlDescriptor descriptor =
+        PersistenceXmlParser.locatePersistenceUnits(new Properties()).stream()
+            .filter(unit -> PersistenceModule.PERSISTENCE_UNIT_NAME.equals(unit.getName()))
+            .findFirst()
+            .orElseThrow(
+                () ->
+                    new IllegalArgumentException(
+                        String.format(
+                            "Could not find persistence unit with name %s",
+                            PersistenceModule.PERSISTENCE_UNIT_NAME)));
+    return descriptor.getManagedClassNames().stream()
+        .map(
+            className -> {
+              try {
+                return Class.forName(className);
+              } catch (ClassNotFoundException e) {
+                throw new RuntimeException(e);
+              }
+            })
+        .filter(AttributeConverter.class::isAssignableFrom)
+        .collect(toImmutableList());
+  }
+}

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

@@ -23,11 +23,18 @@ 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.HibernateSchemaExporter;
 import google.registry.persistence.PersistenceModule;
 import google.registry.testing.FakeClock;
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.sql.Connection;
 import java.sql.Driver;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -52,8 +59,8 @@ import org.testcontainers.containers.PostgreSQLContainer;
  * PostgreSQLContainer} to achieve test purpose.
  */
 public class JpaTransactionManagerRule extends ExternalResource {
-  private static final String SCHEMA_GOLDEN_SQL = "sql/schema/nomulus.golden.sql";
-  private static final String DB_CLEANUP_SQL =
+  private static final String GOLDEN_SCHEMA_SQL_PATH = "sql/schema/nomulus.golden.sql";
+  private static final String DB_CLEANUP_SQL_PATH =
       "google/registry/model/transaction/cleanup_database.sql";
   private static final String MANAGEMENT_DB_NAME = "management";
   private static final String POSTGRES_DB_NAME = "postgres";
@@ -65,6 +72,9 @@ public class JpaTransactionManagerRule extends ExternalResource {
   private final ImmutableMap userProperties;
 
   private static final JdbcDatabaseContainer database = create();
+  private static final HibernateSchemaExporter exporter =
+      HibernateSchemaExporter.create(
+          database.getJdbcUrl(), database.getUsername(), database.getPassword());
   private EntityManagerFactory emf;
   private JpaTransactionManager cachedTm;
 
@@ -86,8 +96,16 @@ public class JpaTransactionManagerRule extends ExternalResource {
 
   @Override
   public void before() throws Exception {
-    executeSql(MANAGEMENT_DB_NAME, DB_CLEANUP_SQL);
-    executeSql(POSTGRES_DB_NAME, initScriptPath);
+    executeSql(MANAGEMENT_DB_NAME, readSqlInClassPath(DB_CLEANUP_SQL_PATH));
+    executeSql(POSTGRES_DB_NAME, readSqlInClassPath(initScriptPath));
+    if (!extraEntityClasses.isEmpty()) {
+      File tempSqlFile = File.createTempFile("tempSqlFile", ".sql");
+      tempSqlFile.deleteOnExit();
+      exporter.export(extraEntityClasses, tempSqlFile);
+      executeSql(
+          POSTGRES_DB_NAME,
+          new String(Files.readAllBytes(tempSqlFile.toPath()), StandardCharsets.UTF_8));
+    }
 
     ImmutableMap properties = PersistenceModule.providesDefaultDatabaseConfigs();
     if (!userProperties.isEmpty()) {
@@ -118,10 +136,18 @@ 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);
+  private static String readSqlInClassPath(String sqlScriptPath) {
+    try {
+      return Resources.toString(Resources.getResource(sqlScriptPath), Charsets.UTF_8);
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    }
+  }
+
+  private void executeSql(String dbName, String sqlScript) {
+    try (Connection conn = createConnection(dbName);
+        Statement statement = conn.createStatement()) {
+      statement.execute(sqlScript);
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
@@ -212,7 +238,7 @@ public class JpaTransactionManagerRule extends ExternalResource {
     /** Builds a {@link JpaTransactionManagerRule} instance. */
     public JpaTransactionManagerRule build() {
       if (initScript == null) {
-        initScript = SCHEMA_GOLDEN_SQL;
+        initScript = GOLDEN_SCHEMA_SQL_PATH;
       }
       return new JpaTransactionManagerRule(
           initScript,

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

@@ -23,7 +23,6 @@ import java.util.List;
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import javax.persistence.PersistenceException;
-import org.hibernate.cfg.Environment;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -37,7 +36,6 @@ public class JpaTransactionManagerRuleTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   @Test

core/src/test/java/google/registry/persistence/BloomFilterConverterTest.java

@@ -24,7 +24,6 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.transaction.JpaTransactionManagerRule;
 import javax.persistence.Entity;
 import javax.persistence.Id;
-import org.hibernate.cfg.Environment;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -38,7 +37,6 @@ public class BloomFilterConverterTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   @Test

core/src/test/java/google/registry/persistence/CreateAutoTimestampConverterTest.java

@@ -21,7 +21,6 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.transaction.JpaTransactionManagerRule;
 import javax.persistence.Entity;
 import javax.persistence.Id;
-import org.hibernate.cfg.Environment;
 import org.joda.time.DateTime;
 import org.junit.Rule;
 import org.junit.Test;
@@ -36,7 +35,6 @@ public class CreateAutoTimestampConverterTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   @Test

core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java

@@ -22,7 +22,6 @@ import google.registry.model.transaction.JpaTransactionManagerRule;
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import javax.persistence.PersistenceException;
-import org.hibernate.cfg.Environment;
 import org.joda.money.CurrencyUnit;
 import org.junit.Rule;
 import org.junit.Test;
@@ -37,7 +36,6 @@ public class CurrencyUnitConverterTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   @Test
@@ -50,7 +48,8 @@ public class CurrencyUnitConverterTest {
                     () ->
                         jpaTm()
                             .getEntityManager()
-                            .createNativeQuery("SELECT currency FROM TestEntity WHERE name = 'id'")
+                            .createNativeQuery(
+                                "SELECT currency FROM \"TestEntity\" WHERE name = 'id'")
                             .getResultList()))
         .containsExactly("EUR");
     TestEntity persisted =
@@ -66,7 +65,7 @@ public class CurrencyUnitConverterTest {
                 jpaTm()
                     .getEntityManager()
                     .createNativeQuery(
-                        "INSERT INTO TestEntity (name, currency) VALUES('id', 'XXXX')")
+                        "INSERT INTO \"TestEntity\" (name, currency) VALUES('id', 'XXXX')")
                     .executeUpdate());
     PersistenceException thrown =
         assertThrows(

core/src/test/java/google/registry/persistence/HibernateSchemaExporterTest.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.persistence;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import org.joda.money.CurrencyUnit;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.testcontainers.containers.PostgreSQLContainer;
+
+/** Unit tests for {@link HibernateSchemaExporter}. */
+@RunWith(JUnit4.class)
+public class HibernateSchemaExporterTest {
+  @ClassRule public static final PostgreSQLContainer database = new PostgreSQLContainer();
+  private static HibernateSchemaExporter exporter;
+
+  @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @BeforeClass
+  public static void init() {
+    exporter =
+        HibernateSchemaExporter.create(
+            database.getJdbcUrl(), database.getUsername(), database.getPassword());
+  }
+
+  @Test
+  public void export_succeeds() throws IOException {
+    File sqlFile = tempFolder.newFile();
+    exporter.export(ImmutableList.of(TestEntity.class), sqlFile);
+    assertThat(Files.readAllBytes(sqlFile.toPath()))
+        .isEqualTo(
+            ("\n"
+                    + "    create table \"TestEntity\" (\n"
+                    + "       name text not null,\n"
+                    + "        cu text,\n"
+                    + "        primary key (name)\n"
+                    + "    );\n")
+                .getBytes(StandardCharsets.UTF_8));
+  }
+
+  @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference.
+  private static class TestEntity {
+    @Id String name;
+
+    CurrencyUnit cu;
+  }
+}

core/src/test/java/google/registry/persistence/UpdateAutoTimestampConverterTest.java

@@ -21,7 +21,6 @@ import google.registry.model.UpdateAutoTimestamp;
 import google.registry.model.transaction.JpaTransactionManagerRule;
 import javax.persistence.Entity;
 import javax.persistence.Id;
-import org.hibernate.cfg.Environment;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,7 +34,6 @@ public class UpdateAutoTimestampConverterTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   @Test

core/src/test/java/google/registry/persistence/ZonedDateTimeConverterTest.java

@@ -24,7 +24,6 @@ import java.time.Instant;
 import java.time.ZonedDateTime;
 import javax.persistence.Entity;
 import javax.persistence.Id;
-import org.hibernate.cfg.Environment;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -38,7 +37,6 @@ public class ZonedDateTimeConverterTest {
   public final JpaTransactionManagerRule jpaTmRule =
       new JpaTransactionManagerRule.Builder()
           .withEntityClass(TestEntity.class)
-          .withProperty(Environment.HBM2DDL_AUTO, "update")
           .build();
 
   private final ZonedDateTimeConverter converter = new ZonedDateTimeConverter();