From f1dedbe21e43eba13ca83ec43fcc44dd53ec0afd Mon Sep 17 00:00:00 2001 From: Shicong Huang Date: Tue, 5 Nov 2019 11:36:03 -0500 Subject: [PATCH] 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"). --- .../persistence/HibernateSchemaExporter.java | 104 ++++++++++++++++++ .../JpaTransactionManagerRule.java | 44 ++++++-- .../JpaTransactionManagerRuleTest.java | 2 - .../persistence/BloomFilterConverterTest.java | 2 - .../CreateAutoTimestampConverterTest.java | 2 - .../CurrencyUnitConverterTest.java | 7 +- .../HibernateSchemaExporterTest.java | 72 ++++++++++++ .../UpdateAutoTimestampConverterTest.java | 2 - .../ZonedDateTimeConverterTest.java | 2 - 9 files changed, 214 insertions(+), 23 deletions(-) create mode 100644 core/src/main/java/google/registry/persistence/HibernateSchemaExporter.java create mode 100644 core/src/test/java/google/registry/persistence/HibernateSchemaExporterTest.java diff --git a/core/src/main/java/google/registry/persistence/HibernateSchemaExporter.java b/core/src/main/java/google/registry/persistence/HibernateSchemaExporter.java new file mode 100644 index 000000000..23a1e62a2 --- /dev/null +++ b/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 entityClasses, File outputFile) { + // Configure Hibernate settings. + Map 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 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()); + } +} diff --git a/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRule.java b/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRule.java index 738c7f61f..06ec0cab8 100644 --- a/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRule.java +++ b/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, diff --git a/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRuleTest.java b/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRuleTest.java index cef58c5a4..5ca719707 100644 --- a/core/src/test/java/google/registry/model/transaction/JpaTransactionManagerRuleTest.java +++ b/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 diff --git a/core/src/test/java/google/registry/persistence/BloomFilterConverterTest.java b/core/src/test/java/google/registry/persistence/BloomFilterConverterTest.java index 51fc756bd..1fa904734 100644 --- a/core/src/test/java/google/registry/persistence/BloomFilterConverterTest.java +++ b/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 diff --git a/core/src/test/java/google/registry/persistence/CreateAutoTimestampConverterTest.java b/core/src/test/java/google/registry/persistence/CreateAutoTimestampConverterTest.java index 6fd80c844..96801e6c0 100644 --- a/core/src/test/java/google/registry/persistence/CreateAutoTimestampConverterTest.java +++ b/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 diff --git a/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java b/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java index cca8315ed..18d455628 100644 --- a/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java +++ b/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( diff --git a/core/src/test/java/google/registry/persistence/HibernateSchemaExporterTest.java b/core/src/test/java/google/registry/persistence/HibernateSchemaExporterTest.java new file mode 100644 index 000000000..93ce1c645 --- /dev/null +++ b/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; + } +} diff --git a/core/src/test/java/google/registry/persistence/UpdateAutoTimestampConverterTest.java b/core/src/test/java/google/registry/persistence/UpdateAutoTimestampConverterTest.java index 885facbd8..f241cfccc 100644 --- a/core/src/test/java/google/registry/persistence/UpdateAutoTimestampConverterTest.java +++ b/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 diff --git a/core/src/test/java/google/registry/persistence/ZonedDateTimeConverterTest.java b/core/src/test/java/google/registry/persistence/ZonedDateTimeConverterTest.java index b0ef1fe21..a9b823d2c 100644 --- a/core/src/test/java/google/registry/persistence/ZonedDateTimeConverterTest.java +++ b/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();