Fix show-sql which stopped working (#596)

* Fix show-sql which stopped working

Made show-sql property configurable in JpaUnitTestRules.

Added a few comments on foreign key constraint behavior.

core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java

@@ -119,7 +119,8 @@ public class DomainBaseSqlTest {
         .transact(
             () -> {
               // Persist the contacts.  Note that these need to be persisted before the domain
-              // otherwise we get a foreign key constraint error.
+              // otherwise we get a foreign key constraint error.  If we ever decide to defer the
+              // relevant foreign key checks to commit time, then the order would not matter.
               jpaTm().saveNew(contact);
               jpaTm().saveNew(contact2);
 
@@ -127,7 +128,8 @@ public class DomainBaseSqlTest {
               jpaTm().saveNew(domain);
 
               // Persist the host.  This does _not_ need to be persisted before the domain,
-              // presumably because its relationship is stored in a join table.
+              // because only the row in the join table (DomainHost) is subject to foreign key
+              // constraints, and Hibernate knows to insert it after domain and host.
               jpaTm().saveNew(host);
             });
 

core/src/test/java/google/registry/persistence/transaction/JpaTestRules.java

@@ -28,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import org.hibernate.cfg.Environment;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
@@ -180,6 +181,17 @@ public class JpaTestRules {
       return this;
     }
 
+    /**
+     * Enables logging of SQL statements.
+     *
+     * <p>SQL logging is very noisy and disabled by default. This method maybe useful when
+     * troubleshooting a specific test.
+     */
+    public Builder withSqlLogging() {
+      withProperty(Environment.SHOW_SQL, "true");
+      return this;
+    }
+
     /** Builds a {@link JpaIntegrationTestRule} instance. */
     public JpaIntegrationTestRule buildIntegrationTestRule() {
       return new JpaIntegrationTestRule(

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

@@ -41,6 +41,8 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Properties;
 import java.util.stream.Collectors;
@@ -146,13 +148,15 @@ abstract class JpaTransactionManagerRule extends ExternalResource {
     ImmutableMap properties = PersistenceModule.providesDefaultDatabaseConfigs();
     if (!userProperties.isEmpty()) {
       // If there are user properties, create a new properties object with these added.
-      ImmutableMap.Builder builder = properties.builder();
+      Map<String, String> mergedProperties = Maps.newHashMap();
-      builder.putAll(userProperties);
+      mergedProperties.putAll(properties);
-      // Forbid Hibernate push to stay consistent with flyway-based schema management.
+      mergedProperties.putAll(userProperties);
-      builder.put(Environment.HBM2DDL_AUTO, "none");
+      properties = ImmutableMap.copyOf(mergedProperties);
-      builder.put(Environment.SHOW_SQL, "true");
-      properties = builder.build();
     }
+    // Forbid Hibernate push to stay consistent with flyway-based schema management.
+    checkState(
+        Objects.equals(properties.get(Environment.HBM2DDL_AUTO), "none"),
+        "The HBM2DDL_AUTO property must be 'none'.");
     assertReasonableNumDbConnections();
     emf =
         createEntityManagerFactory(