From ce195a5e9ba3effefbdc8bef6b0cadd073be1671 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Fri, 4 Jul 2025 14:03:42 +0100 Subject: [PATCH 1/2] Avoid overhead from excessive trace level logging in JdbcCoordinatorImpl --- .../engine/jdbc/internal/JdbcCoordinatorImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java index f4587a1e4605..f85b2caefff1 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java @@ -49,6 +49,7 @@ */ public class JdbcCoordinatorImpl implements JdbcCoordinator { private static final CoreMessageLogger LOG = CoreLogging.messageLogger( JdbcCoordinatorImpl.class ); + private static final boolean TRACE_ENABLED = LOG.isTraceEnabled(); private transient final LogicalConnectionImplementor logicalConnection; private transient final JdbcSessionOwner owner; @@ -141,7 +142,7 @@ public void flushEnding() { @Override public Connection close() { - LOG.tracev( "Closing JDBC container [{0}]", this ); + if ( TRACE_ENABLED ) LOG.tracev( "Closing JDBC container [{0}]", this ); Connection connection; try { if ( currentBatch != null ) { @@ -264,7 +265,7 @@ public int determineRemainingTransactionTimeOutPeriod() { @Override public void afterStatementExecution() { final ConnectionReleaseMode connectionReleaseMode = connectionReleaseMode(); - LOG.tracev( "Starting after statement execution processing [{0}]", connectionReleaseMode ); + if ( TRACE_ENABLED ) LOG.tracev( "Starting after statement execution processing [{0}]", connectionReleaseMode ); if ( connectionReleaseMode == AFTER_STATEMENT ) { if ( ! releasesEnabled ) { LOG.debug( "Skipping aggressive release due to manual disabling" ); @@ -320,7 +321,7 @@ public boolean isReadyForSerialization() { @Override @SuppressWarnings("unchecked") public void registerLastQuery(Statement statement) { - LOG.tracev( "Registering last query statement [{0}]", statement ); + if ( TRACE_ENABLED ) LOG.tracev( "Registering last query statement [{0}]", statement ); if ( statement instanceof JdbcWrapper ) { final JdbcWrapper wrapper = (JdbcWrapper) statement; registerLastQuery( wrapper.getWrappedObject() ); From 3152a064a0e10c9f52f2d96627e6ced62b42ea39 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Wed, 2 Jul 2025 16:21:43 +0100 Subject: [PATCH 2/2] HHH-19593 ResourceRegistryStandardImpl triggers identity hashcode on Statements and ResultSets This reverts commit ef2207f17d328d00ead2c372a6d311fe1b26a432. --- .../ResourceRegistryStandardImpl.java | 57 +--- .../resource/jdbc/internal/ResultSetsSet.java | 94 ++++++ .../internal/ResultsetsTrackingContainer.java | 206 +++++++++++++ .../jdbc/internal/ResultSetsSetTest.java | 279 ++++++++++++++++++ .../ResultsetsTrackingContainerTest.java | 182 ++++++++++++ 5 files changed, 777 insertions(+), 41 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultSetsSet.java create mode 100644 hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainer.java create mode 100644 hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultSetsSetTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainerTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java index 2baeb3d30fc4..d5bc10b47d5d 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java @@ -11,9 +11,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; -import java.util.HashMap; -import org.hibernate.HibernateException; import org.hibernate.JDBCException; import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreMessageLogger; @@ -40,17 +38,9 @@ public final class ResourceRegistryStandardImpl implements ResourceRegistry { private static final CoreMessageLogger log = CoreLogging.messageLogger( ResourceRegistryStandardImpl.class ); private static final boolean IS_TRACE_ENABLED = log.isTraceEnabled(); - // Dummy value to associate with an Object in the backing Map when we use it as a set: - private static final Object PRESENT = new Object(); - - //Used instead of Collections.EMPTY_SET to avoid polymorphic calls on xref; - //Also, uses an HashMap as it were an HashSet, as technically we just need the Set semantics - //but in this case the overhead of HashSet is not negligible. - private static final HashMap EMPTY = new HashMap<>( 1, 0.2f ); - private final JdbcEventHandler jdbcEventHandler; - private final HashMap> xref = new HashMap<>(); + private final ResultsetsTrackingContainer xref = new ResultsetsTrackingContainer(); private ExtendedState ext; private Statement lastQuery; @@ -65,7 +55,7 @@ public ResourceRegistryStandardImpl(JdbcEventHandler jdbcEventHandler) { @Override public boolean hasRegisteredResources() { - return hasRegistered( xref ) + return xref.hasRegisteredResources() || ext != null && ext.hasRegisteredResources(); } @@ -73,10 +63,7 @@ public boolean hasRegisteredResources() { public void register(Statement statement, boolean cancelable) { if ( IS_TRACE_ENABLED ) log.tracef( "Registering statement [%s]", statement ); - HashMap previousValue = xref.putIfAbsent( statement, EMPTY ); - if ( previousValue != null ) { - throw new HibernateException( "JDBC Statement already registered" ); - } + xref.registerExpectingNew( statement ); if ( cancelable ) { lastQuery = statement; @@ -87,7 +74,7 @@ public void register(Statement statement, boolean cancelable) { public void release(Statement statement) { if ( IS_TRACE_ENABLED ) log.tracev( "Releasing statement [{0}]", statement ); - final HashMap resultSets = xref.remove( statement ); + final ResultSetsSet resultSets = xref.remove( statement ); if ( resultSets != null ) { closeAll( resultSets ); } @@ -117,12 +104,12 @@ public void release(ResultSet resultSet, Statement statement) { } } if ( statement != null ) { - final HashMap resultSets = xref.get( statement ); + final ResultSetsSet resultSets = xref.getForResultSetRemoval( statement ); if ( resultSets == null ) { log.unregisteredStatement(); } else { - resultSets.remove( resultSet ); + resultSets.removeResultSet( resultSet ); if ( resultSets.isEmpty() ) { try { if ( statement.isClosed() ) { @@ -143,15 +130,14 @@ public void release(ResultSet resultSet, Statement statement) { close( resultSet ); } - private static void closeAll(final HashMap resultSets) { + private static void closeAll(final ResultSetsSet resultSets) { if ( resultSets == null ) { return; } - resultSets.forEach( (resultSet, o) -> close( resultSet ) ); - resultSets.clear(); + resultSets.forEachResultSet( ResourceRegistryStandardImpl::close ); } - private static void releaseXref(final Statement s, final HashMap r) { + private static void releaseXref(final Statement s, final ResultSetsSet r) { closeAll( r ); close( s ); } @@ -219,19 +205,7 @@ public void register(ResultSet resultSet, Statement statement) { } } if ( statement != null ) { - HashMap resultSets = xref.get( statement ); - - // Keep this at DEBUG level, rather than warn. Numerous connection pool implementations can return a - // proxy/wrapper around the JDBC Statement, causing excessive logging here. See HHH-8210. - if ( resultSets == null ) { - log.debug( "ResultSet statement was not registered (on register)" ); - } - - if ( resultSets == null || resultSets == EMPTY ) { - resultSets = new HashMap<>(); - xref.put( statement, resultSets ); - } - resultSets.put( resultSet, PRESENT ); + xref.storeAssociatedResultset( statement, resultSet ); } else { getExtendedStateForWrite().storeUnassociatedResultset( resultSet ); @@ -328,7 +302,7 @@ public void releaseResources() { } } - private static boolean hasRegistered(final HashMap resource) { + private static boolean hasRegistered(final ResultSetsSet resource) { return resource != null && !resource.isEmpty(); } @@ -344,7 +318,7 @@ private static boolean hasRegistered(final ArrayList resource) { */ private static class ExtendedState { //All fields lazily initialized as well: - private HashMap unassociatedResultSets; + private ResultSetsSet unassociatedResultSets; private ArrayList blobs; private ArrayList clobs; private ArrayList nclobs; @@ -357,7 +331,7 @@ public boolean hasRegisteredResources() { } public void releaseUnassociatedResult(final ResultSet resultSet) { - final Object removed = unassociatedResultSets == null ? null : unassociatedResultSets.remove( resultSet ); + final Object removed = unassociatedResultSets == null ? null : unassociatedResultSets.removeResultSet( resultSet ); if ( removed == null ) { log.unregisteredResultSetWithoutStatement(); } @@ -365,9 +339,9 @@ public void releaseUnassociatedResult(final ResultSet resultSet) { public void storeUnassociatedResultset(ResultSet resultSet) { if ( unassociatedResultSets == null ) { - this.unassociatedResultSets = new HashMap<>(); + this.unassociatedResultSets = new ResultSetsSet(); } - unassociatedResultSets.put( resultSet, PRESENT ); + unassociatedResultSets.storeResultSet( resultSet ); } public void registerBlob(final Blob blob) { @@ -394,6 +368,7 @@ public void registerNClob(final NClob nclob) { public void releaseResources() { closeAll( unassociatedResultSets ); + unassociatedResultSets.clear(); if ( blobs != null ) { blobs.forEach( blob -> { diff --git a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultSetsSet.java b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultSetsSet.java new file mode 100644 index 000000000000..177133bbb5de --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultSetsSet.java @@ -0,0 +1,94 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.resource.jdbc.internal; + +import java.sql.ResultSet; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.function.Consumer; + +/** + * Similar goal as in {@link ResultsetsTrackingContainer}: make sure to + * be very efficient when handling a single {@link ResultSet} + * as it's a very common case, and try to avoid needing the hashcodes. + * Yet we want to allow scaling to multiple instances as well. + */ +final class ResultSetsSet { + + //Implementation notes: + // # if first is null, then the Map in field 'more' is guaranteed to be empty + // # The 'more' Map is lazily initialized, but when emptied it's not guaranteed to be made null + + private ResultSet first; + //This could have been a set, but we intentionally use a Map instead to avoid the wrapping done in + //HashSet. + private HashMap more; + + void forEachResultSet(final Consumer action) { + if ( first != null ) { + action.accept( first ); + if ( more != null ) { + more.keySet().forEach( action ); + } + } + } + + void storeResultSet(final ResultSet resultSet) { + if ( first == null ) { + //no need for further checks as we guarantee "more" to be empty in this case + first = resultSet; + } + else if ( first == resultSet ) { + //no-op for this special case + } + else { + if ( more == null ) { + more = new HashMap<>(); + } + more.put( resultSet, resultSet ); + } + } + + boolean isEmpty() { + return first == null; + } + + ResultSet removeResultSet(final ResultSet resultSet) { + if ( first == resultSet ) { + ResultSet v = first; + first = null; + scaleDown(); + return v; + } + else if ( more != null ) { + return more.remove( resultSet ); + } + else { + return null; + } + } + + //When slot "first" is made available, make sure to move an entry from "more" into that field. + //Any entry will do, so we take the first one if there's any. + private void scaleDown() { + if ( more != null && !more.isEmpty() ) { + Iterator> iterator = more.entrySet().iterator(); + Map.Entry entry = iterator.next(); + final ResultSet resultSet = entry.getKey(); + iterator.remove(); + first = resultSet; + } + } + + void clear() { + first = null; + if ( more != null ) { + more.clear(); + this.more = null; + } + } + +} diff --git a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainer.java b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainer.java new file mode 100644 index 000000000000..a705e362db9e --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainer.java @@ -0,0 +1,206 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.resource.jdbc.internal; + +import org.hibernate.internal.CoreLogging; +import org.hibernate.internal.CoreMessageLogger; + +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.function.BiConsumer; + +/** + * We used to record Statement(s) and their associated ResultSet(s) in a Map + * to guarantee proper resource cleanup, however such types will commonly + * be implemented in such a way to require identity hashcode calculations, which has + * been shown to become a drag on overall system efficiency + * (There are JVM tunables one can use to improve on the default, but they have + * system wide impact which in turn could have undesirable impact on other libraries). + * Since in the most common case we process a single statement at a time, we + * trade some code complexity here to keep track of such resources via direct + * fields only, and overflow to the normal Map usage for the remaining cases, which + * are typically less frequent. + */ +final class ResultsetsTrackingContainer { + + //Implementation notes: + // #1. if key_1 is non-null, then value_1 is the value it maps to. + // #2. if key_1 is null, then the Map in xref is guaranteed to be empty + // #3. The Map in xref is lazily initialized, but when emptied it's not guaranteed to be made null + + private static final CoreMessageLogger log = CoreLogging.messageLogger( ResourceRegistryStandardImpl.class ); + + private static final ResultSetsSet EMPTY = new ResultSetsSet(); + + private Statement key_1; + private ResultSetsSet value_1; + + //Additional pairs, for the case in which we need more: + private HashMap xref; + + public boolean hasRegisteredResources() { + return key_1 != null; //No need to check the content of xref because of implementation rule #1. + } + + public void registerExpectingNew(final Statement statement) { + //We use an assert here as it's a relatively expensive check and I'm fairly confident this would never happen at runtime, + //yet we keep the check as an assertion to have the testsuite help us ensure this confidence is maintained in the future. + assert statementNotExisting( statement ) : "JDBC Statement already registered"; + if ( key_1 == null ) { + //this is the fast-path: most likely case and most efficient as we avoid accessing xref altogether + key_1 = statement; + value_1 = EMPTY; + } + else { + getXrefForWriting().put( statement, EMPTY ); + } + } + + //Assertion helper only: + private boolean statementNotExisting(final Statement statement) { + if ( key_1 == statement ) { + return false; + } + else if ( xref != null ) { + return ! xref.containsKey( statement ); + } + else { + return true; + } + } + + private HashMap getXrefForWriting() { + if ( this.xref == null ) { + this.xref = new HashMap<>(); + } + return this.xref; + } + + private void trickleDown() { + //Moves the first entry from the xref map into the fields, if any entry exists in it. + if ( xref != null ) { + Iterator> iterator = xref.entrySet().iterator(); { + if ( iterator.hasNext() ) { + Map.Entry entry = iterator.next(); + key_1 = entry.getKey(); + value_1 = entry.getValue(); + iterator.remove(); + } + } + } + } + + public void storeAssociatedResultset(Statement statement, ResultSet resultSet) { + if ( key_1 == null ) { + key_1 = statement; + value_1 = new ResultSetsSet(); + //A debug warning wrapped in an assertion to avoid its overhead in production systems + assert warnOnNotNull( null ); + value_1.storeResultSet( resultSet ); + } + else if ( key_1 == statement ) { + value_1 = ensureWriteable( value_1 ); + value_1.storeResultSet( resultSet ); + } + else { + ResultSetsSet resultSetsSet = null; + if ( xref == null ) { + xref = new HashMap<>(); + } + else { + resultSetsSet = xref.get( statement ); + } + //A debug warning wrapped in an assertion to avoid its overhead in production systems + assert warnOnNotNull( resultSetsSet ); + if ( resultSetsSet == null || resultSetsSet == EMPTY ) { + resultSetsSet = new ResultSetsSet(); + xref.put( statement, resultSetsSet ); + } + resultSetsSet.storeResultSet( resultSet ); + } + } + + private ResultSetsSet ensureWriteable(final ResultSetsSet value) { + if ( value == null || value == EMPTY) { + return new ResultSetsSet(); + } + else { + return value; + } + } + + //As it's a get "for removal" we won't be wrapping an EMPTY set + + /** + * This gets the {@link ResultSetsSet} associated to a particular statement, + * but should only be used for read operations or to remove resultsets. + * Performing an "add" operation could result in tainting constants. + * This is NOT removing the statement - use {@link #remove(Statement)} for that purpose. + * @param statement + * @return + */ + public ResultSetsSet getForResultSetRemoval(final Statement statement) { + final ResultSetsSet existingEntry; + if ( key_1 == statement ) { + existingEntry = value_1; + } + else if ( key_1 != null && xref != null ) { + existingEntry = xref.get( statement ); + } + else { + existingEntry = null; + } + + //A debug warning wrapped in an assertion to avoid its overhead in production systems + assert warnOnNotNull( existingEntry ); + + return existingEntry; + } + + public ResultSetsSet remove(final Statement statement) { + if ( key_1 == statement ) { + final ResultSetsSet v = value_1; + key_1 = null; + value_1 = null; + trickleDown(); //most expensive operation, but necessary to guarantee the invariants which allow the other optimisations + return v; + } + else if ( xref != null ) { + return xref.remove( statement ); + } + return null; + } + + private boolean warnOnNotNull(ResultSetsSet existingEntry) { + // Keep this at DEBUG level, rather than warn. Numerous connection pool implementations can return a + // proxy/wrapper around the JDBC Statement, causing excessive logging here. See HHH-8210. + if ( existingEntry == null ) { + log.debug( "ResultSet statement was not registered (on register)" ); + } + return true; + } + + public void forEach(final BiConsumer action) { + if ( key_1 != null ) { + action.accept( key_1, value_1 ); + if ( xref != null ) { + xref.forEach( action ); + } + } + } + + public void clear() { + key_1 = null; + value_1 = null; + if ( xref != null ) { + xref.clear(); + xref = null; + } + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultSetsSetTest.java b/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultSetsSetTest.java new file mode 100644 index 000000000000..b0a3afdfd0b5 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultSetsSetTest.java @@ -0,0 +1,279 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.resource.jdbc.internal; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.sql.ResultSet; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; + +import static org.junit.Assert.assertFalse; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +/** + * See {@link ResultSetsSet}: a custom data container, + * we should be able to fully test this via simple unit testing. + */ +public class ResultSetsSetTest { + + private ResultSetsSet resultSetsSet; + private ResultSet mockResultSet1; + private ResultSet mockResultSet2; + private ResultSet mockResultSet3; + + @BeforeEach + void setUp() { + resultSetsSet = new ResultSetsSet(); + mockResultSet1 = mock(ResultSet.class); + mockResultSet2 = mock(ResultSet.class); + mockResultSet3 = mock(ResultSet.class); + } + + @Test + void testInitialStateIsEmpty() { + assertTrue(resultSetsSet.isEmpty()); + } + + @Test + void testStoreFirstResultSet() { + resultSetsSet.storeResultSet(mockResultSet1); + assertFalse(resultSetsSet.isEmpty()); + } + + @Test + void testStoreDuplicateResultSet() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet1); + + // Store same ResultSet twice should be a no-op + List stored = new ArrayList<>(); + resultSetsSet.forEachResultSet(stored::add); + + assertEquals(1, stored.size()); + assertSame(mockResultSet1, stored.get(0)); + + //The same after the first element is inserted: + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet2); + + stored.clear(); + resultSetsSet.forEachResultSet(stored::add); + assertEquals(2, stored.size()); + assertTrue(stored.contains(mockResultSet1)); + + } + + @Test + void testStoreMultipleResultSets() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet3); + + List stored = new ArrayList<>(); + resultSetsSet.forEachResultSet(stored::add); + + assertEquals(3, stored.size()); + assertTrue(stored.contains(mockResultSet1)); + assertTrue(stored.contains(mockResultSet2)); + assertTrue(stored.contains(mockResultSet3)); + } + + @Test + void testRemoveExistingResultSet() { + resultSetsSet.storeResultSet(mockResultSet1); + ResultSet removed = resultSetsSet.removeResultSet(mockResultSet1); + + assertSame(mockResultSet1, removed); + assertTrue(resultSetsSet.isEmpty()); + } + + @Test + void testRemoveNonExistingResultSet() { + resultSetsSet.storeResultSet(mockResultSet1); + ResultSet removed = resultSetsSet.removeResultSet(mockResultSet2); + + assertNull(removed); + assertFalse(resultSetsSet.isEmpty()); + } + + @Test + void testClear() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + + resultSetsSet.clear(); + assertTrue(resultSetsSet.isEmpty()); + } + + @Test + void testForEachResultSet() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet3); + + @SuppressWarnings("unchecked") + Consumer mockConsumer = mock(Consumer.class); + resultSetsSet.forEachResultSet(mockConsumer); + + verify(mockConsumer).accept(mockResultSet1); + verify(mockConsumer).accept(mockResultSet2); + verify(mockConsumer).accept(mockResultSet3); + verifyNoMoreInteractions(mockConsumer); + } + + @Test + void testScaleDownWhenRemovingFirst() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + + // Remove first ResultSet + resultSetsSet.removeResultSet(mockResultSet1); + + // Verify second ResultSet got moved to first position + List remaining = new ArrayList<>(); + resultSetsSet.forEachResultSet(remaining::add); + + assertEquals(1, remaining.size()); + assertSame(mockResultSet2, remaining.get(0)); + } + + @Test + void testScaleUpDown() { + assertTrue(resultSetsSet.isEmpty()); + resultSetsSet.storeResultSet(mockResultSet1); + assertFalse(resultSetsSet.isEmpty()); + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet3); + resultSetsSet.storeResultSet(mockResultSet3);//intentional duplicate + + resultSetsSet.removeResultSet(mockResultSet1); + resultSetsSet.removeResultSet(mockResultSet2); + assertFalse(resultSetsSet.isEmpty()); + + // Now we should have only mockResultSet3 + List remaining = new ArrayList<>(); + resultSetsSet.forEachResultSet(remaining::add); + + assertEquals(1, remaining.size()); + assertSame(mockResultSet3, remaining.get(0)); + + resultSetsSet.storeResultSet(mockResultSet3);//another duplicate, different internal slot + resultSetsSet.removeResultSet(mockResultSet3); + assertTrue(resultSetsSet.isEmpty()); + remaining.clear(); + resultSetsSet.forEachResultSet(remaining::add); + assertEquals(0, remaining.size()); + } + + @Test + void testRemoveFromEmptySet() { + ResultSet removed = resultSetsSet.removeResultSet(mockResultSet1); + assertNull(removed); + assertTrue(resultSetsSet.isEmpty()); + } + + @Test + void testForEachOnEmptySet() { + @SuppressWarnings("unchecked") + Consumer mockConsumer = mock(Consumer.class); + resultSetsSet.forEachResultSet(mockConsumer); + + verifyNoInteractions(mockConsumer); + } + + @Test + void testScaleDownWithMultipleRemovals() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet3); + + // Remove first ResultSet twice + ResultSet removed1 = resultSetsSet.removeResultSet(mockResultSet1); + ResultSet removed2 = resultSetsSet.removeResultSet(mockResultSet1); + + assertSame(mockResultSet1, removed1); + assertNull(removed2); + + List remaining = new ArrayList<>(); + resultSetsSet.forEachResultSet(remaining::add); + assertEquals(2, remaining.size()); + } + + @Test + void testClearAndReuse() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + + resultSetsSet.clear(); + assertTrue(resultSetsSet.isEmpty()); + + // Test that we can reuse the set after clearing + resultSetsSet.storeResultSet(mockResultSet3); + assertFalse(resultSetsSet.isEmpty()); + + List stored = new ArrayList<>(); + resultSetsSet.forEachResultSet(stored::add); + assertEquals(1, stored.size()); + assertSame(mockResultSet3, stored.get(0)); + } + + @Test + void testRemoveAndRestore() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + + ResultSet removed = resultSetsSet.removeResultSet(mockResultSet1); + assertSame(mockResultSet1, removed); + + // Restore the removed ResultSet + resultSetsSet.storeResultSet(mockResultSet1); + + List stored = new ArrayList<>(); + resultSetsSet.forEachResultSet(stored::add); + assertEquals(2, stored.size()); + assertTrue(stored.contains(mockResultSet1)); + assertTrue(stored.contains(mockResultSet2)); + } + + @Test + void testStoreRemoveStore() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.removeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + + List stored = new ArrayList<>(); + resultSetsSet.forEachResultSet(stored::add); + + assertEquals(1, stored.size()); + assertSame(mockResultSet2, stored.get(0)); + } + + @Test + void testRemoveNonFirstWhenMultiple() { + resultSetsSet.storeResultSet(mockResultSet1); + resultSetsSet.storeResultSet(mockResultSet2); + resultSetsSet.storeResultSet(mockResultSet3); + + ResultSet removed = resultSetsSet.removeResultSet(mockResultSet2); + assertSame(mockResultSet2, removed); + + List remaining = new ArrayList<>(); + resultSetsSet.forEachResultSet(remaining::add); + assertEquals(2, remaining.size()); + assertTrue(remaining.contains(mockResultSet1)); + assertTrue(remaining.contains(mockResultSet3)); + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainerTest.java b/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainerTest.java new file mode 100644 index 000000000000..fbdda6fd14be --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/resource/jdbc/internal/ResultsetsTrackingContainerTest.java @@ -0,0 +1,182 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.resource.jdbc.internal; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.function.BiConsumer; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class ResultsetsTrackingContainerTest { + + private ResultsetsTrackingContainer container; + + @Mock + private Statement statement1; + @Mock + private Statement statement2; + @Mock + private Statement statement3; + @Mock + private ResultSet resultSet1; + @Mock + private ResultSet resultSet2; + @Mock + private ResultSet resultSet3; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + container = new ResultsetsTrackingContainer(); + } + + @Test + void initialStateHasNoRegisteredResources() { + assertFalse(container.hasRegisteredResources()); + } + + @Test + void registeringSingleStatementShouldWork() { + container.registerExpectingNew(statement1); + assertTrue(container.hasRegisteredResources()); + } + + @Test + void storingAssociatedResultSetForUnregisteredStatementShouldWork() { + container.storeAssociatedResultset(statement1, resultSet1); + assertTrue(container.hasRegisteredResources()); + } + + @Test + void storingMultipleResultSetsForSameStatement() { + container.registerExpectingNew(statement1); + container.storeAssociatedResultset(statement1, resultSet1); + container.storeAssociatedResultset(statement1, resultSet2); + + ResultSetsSet resultSets = container.getForResultSetRemoval(statement1); + assertNotNull(resultSets); + + List stored = new ArrayList<>(); + resultSets.forEachResultSet(stored::add); + assertEquals(2, stored.size()); + assertTrue(stored.contains(resultSet1)); + assertTrue(stored.contains(resultSet2)); + } + + @Test + void removingStatementShouldReturnAssociatedResultSets() { + container.registerExpectingNew(statement1); + container.storeAssociatedResultset(statement1, resultSet1); + + ResultSetsSet removed = container.remove(statement1); + assertNotNull(removed); + + List stored = new ArrayList<>(); + removed.forEachResultSet(stored::add); + assertEquals(1, stored.size()); + assertTrue(stored.contains(resultSet1)); + assertFalse(container.hasRegisteredResources()); + } + + @Test + void removingNonExistentStatementShouldReturnNull() { + assertNull(container.remove(statement1)); + } + + @Test + void handlingMultipleStatements() { + container.registerExpectingNew(statement1); + container.registerExpectingNew(statement2); + container.storeAssociatedResultset(statement1, resultSet1); + container.storeAssociatedResultset(statement2, resultSet2); + + assertTrue(container.hasRegisteredResources()); + + List processedStatements = new ArrayList<>(); + List processedResultSets = new ArrayList<>(); + + container.forEach((stmt, results) -> { + processedStatements.add(stmt); + processedResultSets.add(results); + }); + + assertEquals(2, processedStatements.size()); + assertTrue(processedStatements.contains(statement1)); + assertTrue(processedStatements.contains(statement2)); + } + + @Test + void clearingShouldRemoveAllResources() { + container.registerExpectingNew(statement1); + container.registerExpectingNew(statement2); + container.storeAssociatedResultset(statement1, resultSet1); + container.storeAssociatedResultset(statement2, resultSet2); + + container.clear(); + + assertFalse(container.hasRegisteredResources()); + assertNull(container.getForResultSetRemoval(statement1)); + assertNull(container.getForResultSetRemoval(statement2)); + } + + @Test + void trickleDownShouldMoveEntryFromXrefToMainSlot() { + container.registerExpectingNew(statement1); + container.registerExpectingNew(statement2); + container.storeAssociatedResultset(statement1, resultSet1); + container.storeAssociatedResultset(statement2, resultSet2); + + container.remove(statement1); // This should trigger trickle down + + assertTrue(container.hasRegisteredResources()); + assertNotNull(container.getForResultSetRemoval(statement2)); + + List processedStatements = new ArrayList<>(); + container.forEach((stmt, results) -> processedStatements.add(stmt)); + + assertEquals(1, processedStatements.size()); + assertEquals(statement2, processedStatements.get(0)); + } + + @Test + void getForRemovalShouldReturnCorrectResultSets() { + container.registerExpectingNew(statement1); + container.storeAssociatedResultset(statement1, resultSet1); + + ResultSetsSet resultSets = container.getForResultSetRemoval(statement1); + assertNotNull(resultSets); + + List stored = new ArrayList<>(); + resultSets.forEachResultSet(stored::add); + assertEquals(1, stored.size()); + assertTrue(stored.contains(resultSet1)); + } + + @Test + void forEachShouldProcessAllEntries() { + container.registerExpectingNew(statement1); + container.registerExpectingNew(statement2); + container.storeAssociatedResultset(statement1, resultSet1); + container.storeAssociatedResultset(statement2, resultSet2); + + @SuppressWarnings("unchecked") + BiConsumer mockConsumer = mock(BiConsumer.class); + + container.forEach(mockConsumer); + + verify(mockConsumer, times(2)).accept(any(), any()); + verify(mockConsumer).accept(eq(statement1), any()); + verify(mockConsumer).accept(eq(statement2), any()); + } +}