HHH-11058 - NPE in SchemaValidator with DdlTransactionIsolatorJtaImpl

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java

@@ -10,7 +10,6 @@ import java.sql.Connection;
 import java.sql.SQLException;
 
 import org.hibernate.internal.log.ConnectionAccessLogger;
-import org.hibernate.internal.log.ConnectionPoolingLogger;
 import org.hibernate.resource.transaction.spi.DdlTransactionIsolator;
 import org.hibernate.tool.schema.internal.exec.JdbcContext;
 

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/DdlTransactionIsolatorJtaImpl.java

@@ -34,15 +34,7 @@ public class DdlTransactionIsolatorJtaImpl implements DdlTransactionIsolator {
 
 	public DdlTransactionIsolatorJtaImpl(JdbcContext jdbcContext) {
 		this.jdbcContext = jdbcContext;
-	}
 
-	@Override
-	public JdbcContext getJdbcContext() {
-		return jdbcContext;
-	}
-
-	@Override
-	public void prepare() {
 		try {
 			final JtaPlatform jtaPlatform = jdbcContext.getServiceRegistry().getService( JtaPlatform.class );
 			log.tracef( "DdlTransactionIsolatorJtaImpl#prepare: JtaPlatform -> %s", jtaPlatform );
@@ -78,6 +70,15 @@ public class DdlTransactionIsolatorJtaImpl implements DdlTransactionIsolator {
 		}
 	}
 
+	@Override
+	public JdbcContext getJdbcContext() {
+		return jdbcContext;
+	}
+
+	@Override
+	public void prepare() {
+	}
+
 	@Override
 	public Connection getIsolatedConnection() {
 		return jdbcConnection;

hibernate-core/src/main/java/org/hibernate/resource/transaction/spi/DdlTransactionIsolator.java

@@ -20,6 +20,12 @@ import org.hibernate.tool.schema.internal.exec.JdbcContext;
 public interface DdlTransactionIsolator {
 	JdbcContext getJdbcContext();
 
+	/**
+	 * In general a DdlTransactionIsolator should be returned from
+	 * {@link TransactionCoordinatorBuilder#buildDdlTransactionIsolator}
+	 * already prepared for use (until {@link #release} is called).
+	 */
+	@Deprecated
 	void prepare();
 
 	/**

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/DdlTransactionIsolatorSharedImpl.java

@@ -1,46 +0,0 @@
-/*
- * Hibernate, Relational Persistence for Idiomatic Java
- *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
- * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
- */
-package org.hibernate.tool.schema.internal;
-
-import java.sql.Connection;
-
-import org.hibernate.resource.transaction.spi.DdlTransactionIsolator;
-import org.hibernate.tool.schema.internal.exec.JdbcContext;
-
-/**
- * A DdlTransactionIsolator implementations for use in cases where the
- * isolated Connection is shared.
- *
- * @author Steve Ebersole
- */
-public class DdlTransactionIsolatorSharedImpl implements DdlTransactionIsolator {
-	private final DdlTransactionIsolator wrappedIsolator;
-
-	public DdlTransactionIsolatorSharedImpl(DdlTransactionIsolator ddlTransactionIsolator) {
-		this.wrappedIsolator = ddlTransactionIsolator;
-	}
-
-	@Override
-	public JdbcContext getJdbcContext() {
-		return wrappedIsolator.getJdbcContext();
-	}
-
-	@Override
-	public void prepare() {
-		// skip delegating the call to prepare
-	}
-
-	@Override
-	public Connection getIsolatedConnection() {
-		return wrappedIsolator.getIsolatedConnection();
-	}
-
-	@Override
-	public void release() {
-		// skip delegating the call to prepare
-	}
-}

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/HibernateSchemaManagementTool.java

@@ -117,7 +117,7 @@ public class HibernateSchemaManagementTool implements SchemaManagementTool, Serv
 		}
 
 		if ( targetDescriptor.getTargetTypes().contains( TargetType.DATABASE ) ) {
-			targets[index] = new GenerationTargetToDatabase( getDdlTransactionIsolator( jdbcContext ) );
+			targets[index] = new GenerationTargetToDatabase( getDdlTransactionIsolator( jdbcContext ), true );
 		}
 
 		return targets;
@@ -147,7 +147,7 @@ public class HibernateSchemaManagementTool implements SchemaManagementTool, Serv
 		}
 
 		if ( targetDescriptor.getTargetTypes().contains( TargetType.DATABASE ) ) {
-			targets[index] = new GenerationTargetToDatabase( ddlTransactionIsolator );
+			targets[index] = new GenerationTargetToDatabase( ddlTransactionIsolator, false );
 		}
 
 		return targets;

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/SchemaDropperImpl.java

@@ -452,7 +452,8 @@ public class SchemaDropperImpl implements SchemaDropper {
 			final JdbcContext jdbcContext = tool.resolveJdbcContext( settings );
 			targets = new GenerationTarget[] {
 				new GenerationTargetToDatabase(
-						serviceRegistry.getService( TransactionCoordinatorBuilder.class ).buildDdlTransactionIsolator( jdbcContext )
+						serviceRegistry.getService( TransactionCoordinatorBuilder.class ).buildDdlTransactionIsolator( jdbcContext ),
+						true
 				)
 			};
 		}
@@ -523,7 +524,8 @@ public class SchemaDropperImpl implements SchemaDropper {
 
 			final JdbcContext jdbcContext = new JdbcContextDelayedDropImpl( serviceRegistry );
 			final GenerationTargetToDatabase target = new GenerationTargetToDatabase(
-					serviceRegistry.getService( TransactionCoordinatorBuilder.class ).buildDdlTransactionIsolator( jdbcContext )
+					serviceRegistry.getService( TransactionCoordinatorBuilder.class ).buildDdlTransactionIsolator( jdbcContext ),
+					true
 			);
 
 			target.prepare();

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/SchemaMigratorImpl.java

@@ -89,19 +89,15 @@ public class SchemaMigratorImpl implements SchemaMigrator {
 		final DdlTransactionIsolator ddlTransactionIsolator = tool.getDdlTransactionIsolator( jdbcContext );
 
 		try {
-			ddlTransactionIsolator.prepare();
-
-			final DdlTransactionIsolator sharedDdlTransactionIsolator = new DdlTransactionIsolatorSharedImpl( ddlTransactionIsolator );
-
 			final DatabaseInformation databaseInformation = Helper.buildDatabaseInformation(
 					tool.getServiceRegistry(),
-					sharedDdlTransactionIsolator,
+					ddlTransactionIsolator,
 					metadata.getDatabase().getDefaultNamespace().getName()
 			);
 
 			final GenerationTarget[] targets = tool.buildGenerationTargets(
 					targetDescriptor,
-					sharedDdlTransactionIsolator,
+					ddlTransactionIsolator,
 					options.getConfigurationValues()
 			);
 

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/SchemaValidatorImpl.java

@@ -17,6 +17,7 @@ import org.hibernate.dialect.Dialect;
 import org.hibernate.mapping.Column;
 import org.hibernate.mapping.Selectable;
 import org.hibernate.mapping.Table;
+import org.hibernate.resource.transaction.spi.DdlTransactionIsolator;
 import org.hibernate.tool.schema.extract.spi.ColumnInformation;
 import org.hibernate.tool.schema.extract.spi.DatabaseInformation;
 import org.hibernate.tool.schema.extract.spi.SequenceInformation;
@@ -52,9 +53,11 @@ public class SchemaValidatorImpl implements SchemaValidator {
 	public void doValidation(Metadata metadata, ExecutionOptions options) {
 		final JdbcContext jdbcContext = tool.resolveJdbcContext( options.getConfigurationValues() );
 
+		final DdlTransactionIsolator isolator = tool.getDdlTransactionIsolator( jdbcContext );
+
 		final DatabaseInformation databaseInformation = Helper.buildDatabaseInformation(
 				tool.getServiceRegistry(),
-				tool.getDdlTransactionIsolator( jdbcContext ),
+				isolator,
 				metadata.getDatabase().getDefaultNamespace().getName()
 		);
 
@@ -68,6 +71,8 @@ public class SchemaValidatorImpl implements SchemaValidator {
 			catch (Exception e) {
 				log.debug( "Problem releasing DatabaseInformation : " + e.getMessage() );
 			}
+
+			isolator.release();
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/exec/GenerationTargetToDatabase.java

@@ -25,16 +25,21 @@ public class GenerationTargetToDatabase implements GenerationTarget {
 	private static final CoreMessageLogger log = CoreLogging.messageLogger( GenerationTargetToDatabase.class );
 
 	private final DdlTransactionIsolator ddlTransactionIsolator;
+	private final boolean releaseAfterUse;
 
 	private Statement jdbcStatement;
 
 	public GenerationTargetToDatabase(DdlTransactionIsolator ddlTransactionIsolator) {
+		this( ddlTransactionIsolator, true );
+	}
+
+	public GenerationTargetToDatabase(DdlTransactionIsolator ddlTransactionIsolator, boolean releaseAfterUse) {
 		this.ddlTransactionIsolator = ddlTransactionIsolator;
+		this.releaseAfterUse = releaseAfterUse;
 	}
 
 	@Override
 	public void prepare() {
-		ddlTransactionIsolator.prepare();
 	}
 
 	@Override
@@ -81,6 +86,8 @@ public class GenerationTargetToDatabase implements GenerationTarget {
 
 	@Override
 	public void release() {
+		if ( releaseAfterUse ) {
 			ddlTransactionIsolator.release();
 		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/exec/ImprovedExtractionContextImpl.java

@@ -96,7 +96,5 @@ public class ImprovedExtractionContextImpl implements ExtractionContext {
 		if ( jdbcDatabaseMetaData != null ) {
 			jdbcDatabaseMetaData = null;
 		}
-
-		ddlTransactionIsolator.release();
 	}
 }

hibernate-core/src/test/java/org/hibernate/test/connections/SuppliedConnectionTest.java

@@ -118,7 +118,8 @@ public class SuppliedConnectionTest extends ConnectionManagementTestCase {
 
 			try {
 				final GenerationTargetToDatabase target = new GenerationTargetToDatabase(
-						new DdlTransactionIsolatorTestingImpl( serviceRegistry(), conn )
+						new DdlTransactionIsolatorTestingImpl( serviceRegistry(), conn ),
+						true
 				);
 				new SchemaCreatorImpl( serviceRegistry() ).doCreation(
 						metadata(),
@@ -145,7 +146,8 @@ public class SuppliedConnectionTest extends ConnectionManagementTestCase {
 						new DdlTransactionIsolatorTestingImpl(
 								serviceRegistry(),
 								conn
-						)
+						),
+						true
 				);
 				new SchemaDropperImpl( serviceRegistry() ).doDrop( metadata(), false, target );
 			}

hibernate-core/src/test/java/org/hibernate/test/tool/schema/ExecutionOptionsTestImpl.java

@@ -0,0 +1,45 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.test.tool.schema;
+
+import java.util.Collections;
+import java.util.Map;
+
+import org.hibernate.tool.schema.spi.CommandAcceptanceException;
+import org.hibernate.tool.schema.spi.ExceptionHandler;
+import org.hibernate.tool.schema.spi.ExecutionOptions;
+
+/**
+ * @author Steve Ebersole
+ */
+public class ExecutionOptionsTestImpl implements ExecutionOptions, ExceptionHandler {
+	/**
+	 * Singleton access for standard cases.  Returns an empty map of configuration values,
+	 * true that namespaces should be managed and it always re-throws command exceptions
+	 */
+	public static final ExecutionOptionsTestImpl INSTANCE = new ExecutionOptionsTestImpl();
+
+	@Override
+	public Map getConfigurationValues() {
+		return Collections.emptyMap();
+	}
+
+	@Override
+	public boolean shouldManageNamespaces() {
+		return true;
+	}
+
+	@Override
+	public ExceptionHandler getExceptionHandler() {
+		return this;
+	}
+
+	@Override
+	public void handleException(CommandAcceptanceException exception) {
+		throw exception;
+	}
+}

hibernate-core/src/test/java/org/hibernate/test/tool/schema/SchemaToolTransactionHandlingTest.java

@@ -0,0 +1,253 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.test.tool.schema;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.transaction.SystemException;
+import javax.transaction.Transaction;
+
+import org.hibernate.boot.Metadata;
+import org.hibernate.boot.MetadataSources;
+import org.hibernate.boot.registry.StandardServiceRegistry;
+import org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorBuilderImpl;
+import org.hibernate.resource.transaction.spi.TransactionCoordinatorBuilder;
+import org.hibernate.tool.schema.SourceType;
+import org.hibernate.tool.schema.TargetType;
+import org.hibernate.tool.schema.spi.SchemaCreator;
+import org.hibernate.tool.schema.spi.SchemaDropper;
+import org.hibernate.tool.schema.spi.SchemaManagementTool;
+import org.hibernate.tool.schema.spi.ScriptSourceInput;
+import org.hibernate.tool.schema.spi.ScriptTargetOutput;
+import org.hibernate.tool.schema.spi.SourceDescriptor;
+import org.hibernate.tool.schema.spi.TargetDescriptor;
+
+import org.hibernate.testing.jta.TestingJtaBootstrap;
+import org.hibernate.testing.jta.TestingJtaPlatformImpl;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * @author Steve Ebersole
+ */
+public class SchemaToolTransactionHandlingTest extends BaseUnitTestCase {
+	// for each case we want to run these tool delegates in a matrix of:
+	//		1) JTA versus JDBC transaction handling
+	//		2) existing transaction versus not
+	//
+	// cases:
+	//		1) create-drop
+	//		2) update
+	//		3) validate
+	//
+	// so:
+	//		1) create-drop
+	//			1.1) JTA transaction handling
+	//				1.1.1) inside an existing transaction
+	//				1.1.2) outside any transaction
+	//			1.1) JDBC transaction handling
+	//				- there really cannot be an "existing transaction" case...
+
+	@Test
+	public void testDropCreateDropInExistingJtaTransaction() {
+		// test for 1.1.1 - create-drop + JTA handling + existing
+
+		// start a JTA transaction...
+		try {
+			TestingJtaPlatformImpl.INSTANCE.getTransactionManager().begin();
+		}
+		catch (Exception e) {
+			throw new RuntimeException( "Unable to being JTA transaction prior to starting test", e );
+		}
+
+		// hold reference to Transaction object
+		final Transaction jtaTransaction;
+		try {
+			jtaTransaction = TestingJtaPlatformImpl.INSTANCE.getTransactionManager().getTransaction();
+		}
+		catch (SystemException e) {
+			throw new RuntimeException( "Unable to access JTA Transaction prior to starting test", e );
+		}
+
+		// perform the test...
+		try {
+			final StandardServiceRegistry registry = buildJtaStandardServiceRegistry();
+			final SchemaManagementTool smt = registry.getService( SchemaManagementTool.class );
+			final SchemaDropper schemaDropper = smt.getSchemaDropper( Collections.emptyMap() );
+			final SchemaCreator schemaCreator = smt.getSchemaCreator( Collections.emptyMap() );
+
+			final Metadata mappings = buildMappings( registry );
+			try {
+				schemaDropper.doDrop(
+						mappings,
+						ExecutionOptionsTestImpl.INSTANCE,
+						SourceDescriptorImpl.INSTANCE,
+						TargetDescriptorImpl.INSTANCE
+				);
+				schemaCreator.doCreation(
+						mappings,
+						ExecutionOptionsTestImpl.INSTANCE,
+						SourceDescriptorImpl.INSTANCE,
+						TargetDescriptorImpl.INSTANCE
+				);
+			}
+			finally {
+				try {
+					schemaDropper.doDrop(
+							mappings,
+							ExecutionOptionsTestImpl.INSTANCE,
+							SourceDescriptorImpl.INSTANCE,
+							TargetDescriptorImpl.INSTANCE
+					);
+				}
+				catch (Exception ignore) {
+					// ignore
+				}
+			}
+		}
+		finally {
+			try {
+				jtaTransaction.commit();
+			}
+			catch (Exception e) {
+				// not much we can do...
+			}
+		}
+
+	}
+	@Test
+	public void testValidateInExistingJtaTransaction() {
+		// test for 1.1.1 - create-drop + JTA handling + existing
+
+		// start a JTA transaction...
+		try {
+			TestingJtaPlatformImpl.INSTANCE.getTransactionManager().begin();
+		}
+		catch (Exception e) {
+			throw new RuntimeException( "Unable to being JTA transaction prior to starting test", e );
+		}
+
+		// hold reference to Transaction object
+		final Transaction jtaTransaction;
+		try {
+			jtaTransaction = TestingJtaPlatformImpl.INSTANCE.getTransactionManager().getTransaction();
+		}
+		catch (SystemException e) {
+			throw new RuntimeException( "Unable to access JTA Transaction prior to starting test", e );
+		}
+
+		// perform the test...
+		try {
+			final StandardServiceRegistry registry = buildJtaStandardServiceRegistry();
+			final SchemaManagementTool smt = registry.getService( SchemaManagementTool.class );
+
+			final Metadata mappings = buildMappings( registry );
+
+			// first make the schema exist...
+			try {
+				smt.getSchemaCreator( Collections.emptyMap() ).doCreation(
+						mappings,
+						ExecutionOptionsTestImpl.INSTANCE,
+						SourceDescriptorImpl.INSTANCE,
+						TargetDescriptorImpl.INSTANCE
+				);
+			}
+			catch (Exception e) {
+				throw new RuntimeException( "Unable to create schema to validation tests", e );
+			}
+
+			try {
+				smt.getSchemaValidator( Collections.emptyMap() ).doValidation(
+						mappings,
+						ExecutionOptionsTestImpl.INSTANCE
+				);
+			}
+			finally {
+				try {
+					smt.getSchemaDropper( Collections.emptyMap() ).doDrop(
+							mappings,
+							ExecutionOptionsTestImpl.INSTANCE,
+							SourceDescriptorImpl.INSTANCE,
+							TargetDescriptorImpl.INSTANCE
+					);
+				}
+				catch (Exception ignore) {
+					// ignore
+				}
+			}
+		}
+		finally {
+			try {
+				jtaTransaction.commit();
+			}
+			catch (Exception e) {
+				// not much we can do...
+			}
+		}
+
+	}
+
+	private Metadata buildMappings(StandardServiceRegistry registry) {
+		return new MetadataSources( registry )
+				.addAnnotatedClass( MyEntity.class )
+				.buildMetadata();
+	}
+
+	protected StandardServiceRegistry buildJtaStandardServiceRegistry() {
+		StandardServiceRegistry registry = TestingJtaBootstrap.prepare().build();
+		assertThat( registry.getService( TransactionCoordinatorBuilder.class ), instanceOf( JtaTransactionCoordinatorBuilderImpl.class ) );
+		return registry;
+	}
+
+	@Entity( name = "MyEntity" )
+	@Table( name = "MyEntity" )
+	public static class MyEntity {
+		@Id
+		public Integer id;
+		public String name;
+	}
+
+	private static class SourceDescriptorImpl implements SourceDescriptor {
+		/**
+		 * Singleton access
+		 */
+		public static final SourceDescriptorImpl INSTANCE = new SourceDescriptorImpl();
+
+		@Override
+		public SourceType getSourceType() {
+			return SourceType.METADATA;
+		}
+
+		@Override
+		public ScriptSourceInput getScriptSourceInput() {
+			return null;
+		}
+	}
+
+	private static class TargetDescriptorImpl implements TargetDescriptor {
+		/**
+		 * Singleton access
+		 */
+		public static final TargetDescriptorImpl INSTANCE = new TargetDescriptorImpl();
+
+		@Override
+		public EnumSet<TargetType> getTargetTypes() {
+			return EnumSet.of( TargetType.DATABASE );
+		}
+
+		@Override
+		public ScriptTargetOutput getScriptTargetOutput() {
+			return null;
+		}
+	}
+}

hibernate-testing/src/main/java/org/hibernate/testing/jta/TestingJtaBootstrap.java

@@ -8,8 +8,8 @@ package org.hibernate.testing.jta;
 
 import java.util.Map;
 
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
 import org.hibernate.cfg.AvailableSettings;
-import org.hibernate.cfg.Environment;
 
 /**
  * @author Steve Ebersole
@@ -20,10 +20,23 @@ public final class TestingJtaBootstrap {
 	@SuppressWarnings("unchecked")
 	public static void prepare(Map configValues) {
 		configValues.put( AvailableSettings.JTA_PLATFORM, TestingJtaPlatformImpl.INSTANCE );
-		configValues.put( Environment.CONNECTION_PROVIDER, JtaAwareConnectionProviderImpl.class.getName() );
+		configValues.put( AvailableSettings.CONNECTION_PROVIDER, JtaAwareConnectionProviderImpl.class.getName() );
 		configValues.put( "javax.persistence.transactionType", "JTA" );
 	}
 
+	public static void prepare(StandardServiceRegistryBuilder registryBuilder) {
+		registryBuilder.applySetting( AvailableSettings.TRANSACTION_COORDINATOR_STRATEGY, "jta" );
+		registryBuilder.applySetting( AvailableSettings.JTA_PLATFORM, TestingJtaPlatformImpl.INSTANCE );
+		registryBuilder.applySetting( AvailableSettings.CONNECTION_PROVIDER, JtaAwareConnectionProviderImpl.class.getName() );
+		registryBuilder.applySetting( "javax.persistence.transactionType", "JTA" );
+	}
+
+	public static StandardServiceRegistryBuilder prepare() {
+		final StandardServiceRegistryBuilder registryBuilder = new StandardServiceRegistryBuilder();
+		prepare( registryBuilder );
+		return registryBuilder;
+	}
+
 	private TestingJtaBootstrap() {
 	}
 }