From b09e2729c0e63491d794544a738ae4133dceeaf4 Mon Sep 17 00:00:00 2001 From: Shawn Clowater Date: Wed, 21 Mar 2012 16:25:32 -0600 Subject: [PATCH] HHH-7193 Added clearBatch() to the releaseStatements() of AbstractBatchImpl in order to prevent batches from rolled back transactions being applied. Added StatementCacheTest to illustrate the issue. Added a validator dependency for c3p0 tests since I needed to have the batch semi aborted. --- hibernate-c3p0/hibernate-c3p0.gradle | 5 ++ .../hibernate/test/c3p0/IrrelevantEntity.java | 60 +++++++++++++ .../test/c3p0/StatementCacheTest.java | 89 +++++++++++++++++++ .../src/test/resources/hibernate.properties | 1 + .../batch/internal/AbstractBatchImpl.java | 1 + 5 files changed, 156 insertions(+) create mode 100644 hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/IrrelevantEntity.java create mode 100644 hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/StatementCacheTest.java diff --git a/hibernate-c3p0/hibernate-c3p0.gradle b/hibernate-c3p0/hibernate-c3p0.gradle index 403554caaf..cff4261a00 100644 --- a/hibernate-c3p0/hibernate-c3p0.gradle +++ b/hibernate-c3p0/hibernate-c3p0.gradle @@ -1,8 +1,13 @@ apply plugin: 'java' dependencies { + provided( libraries.validation ) compile project( ':hibernate-core' ) compile "c3p0:c3p0:0.9.1" + testCompile( libraries.validator ) { + // for test runtime + transitive = true + } testCompile project( ':hibernate-testing' ) } \ No newline at end of file diff --git a/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/IrrelevantEntity.java b/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/IrrelevantEntity.java new file mode 100644 index 0000000000..2279347d9d --- /dev/null +++ b/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/IrrelevantEntity.java @@ -0,0 +1,60 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2012, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.c3p0; + +import org.hibernate.annotations.GenericGenerator; +import org.hibernate.validator.constraints.NotBlank; + +import javax.persistence.*; + +/** + * A testing entity for cases where the entity definition itself is irrelevant (testing JDBC connection semantics, etc). + * + * @author Shawn Clowater + */ +@Entity +public class IrrelevantEntity { + private Integer id; + private String name; + + @Id + @GeneratedValue( generator = "increment" ) + @GenericGenerator( name = "increment", strategy = "increment" ) + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + @NotBlank + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/StatementCacheTest.java b/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/StatementCacheTest.java new file mode 100644 index 0000000000..04ee7bfbd1 --- /dev/null +++ b/hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/StatementCacheTest.java @@ -0,0 +1,89 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2012, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.c3p0; + +import org.hibernate.Criteria; +import org.hibernate.Session; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.*; + +import java.util.List; + +/** + * Tests that when using cached prepared statement with batching enabled doesn't bleed over into new transactions. + * + * @author Shawn Clowater + */ +public class StatementCacheTest extends BaseCoreFunctionalTestCase { + @Test + @TestForIssue( jiraKey = "HHH-7193" ) + public void testStatementCaching() { + Session session = openSession(); + session.beginTransaction(); + + //save 2 new entities, one valid, one invalid (neither should be persisted) + IrrelevantEntity irrelevantEntity = new IrrelevantEntity(); + irrelevantEntity.setName( "valid 1" ); + session.save( irrelevantEntity ); + //name is required + irrelevantEntity = new IrrelevantEntity(); + session.save( irrelevantEntity ); + try { + session.flush(); + Assert.fail( "Validation exception did not occur" ); + } + catch (Exception e) { + //this is expected roll the transaction back + session.getTransaction().rollback(); + } + session.close(); + + session = openSession(); + session.beginTransaction(); + + //save a new entity and commit it + irrelevantEntity = new IrrelevantEntity(); + irrelevantEntity.setName( "valid 2" ); + session.save( irrelevantEntity ); + session.flush(); + session.getTransaction().commit(); + session.close(); + + //only one entity should have been inserted to the database (if the statement in the cache wasn't cleared then it would have inserted both entities) + session = openSession(); + session.beginTransaction(); + Criteria criteria = session.createCriteria( IrrelevantEntity.class ); + List results = criteria.list(); + session.getTransaction().commit(); + session.close(); + + Assert.assertEquals( 1, results.size() ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[]{ IrrelevantEntity.class }; + } +} diff --git a/hibernate-c3p0/src/test/resources/hibernate.properties b/hibernate-c3p0/src/test/resources/hibernate.properties index 3df063d7f4..5de5219c39 100644 --- a/hibernate-c3p0/src/test/resources/hibernate.properties +++ b/hibernate-c3p0/src/test/resources/hibernate.properties @@ -30,6 +30,7 @@ hibernate.connection.pool_size 5 hibernate.c3p0.min_size 50 hibernate.c3p0.max_size 800 hibernate.c3p0.max_statements 50 +hibernate.jdbc.batch_size 10 hibernate.c3p0.timeout 300 hibernate.c3p0.idle_test_period 3000 hibernate.c3p0.testConnectionOnCheckout true diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/AbstractBatchImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/AbstractBatchImpl.java index 1f7454a41d..2a00e265bd 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/AbstractBatchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/AbstractBatchImpl.java @@ -172,6 +172,7 @@ public abstract class AbstractBatchImpl implements Batch { private void releaseStatements() { for ( PreparedStatement statement : getStatements().values() ) { try { + statement.clearBatch(); statement.close(); } catch ( SQLException e ) {