From f6c10f0334b6259583551ad043785a1492780e91 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Fri, 26 May 2023 17:14:21 +0100 Subject: [PATCH] HHH-16717 Type pollution fix for ExecutableList having to implement Comparable --- .../action/internal/CollectionAction.java | 20 ++++++++--- .../action/internal/EntityAction.java | 20 ++++++++--- .../org/hibernate/engine/spi/ActionQueue.java | 6 ++-- .../engine/spi/ComparableExecutable.java | 35 +++++++++++++++++++ .../hibernate/engine/spi/ExecutableList.java | 26 ++++++-------- .../action/NonSortedExecutableListTest.java | 20 +++++++---- .../action/SortedExecutableListTest.java | 22 ++++++++---- 7 files changed, 108 insertions(+), 41 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/engine/spi/ComparableExecutable.java diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionAction.java index ed58820fa1..3c1ba30737 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/CollectionAction.java @@ -10,11 +10,11 @@ import java.io.Serializable; import org.hibernate.action.spi.AfterTransactionCompletionProcess; import org.hibernate.action.spi.BeforeTransactionCompletionProcess; -import org.hibernate.action.spi.Executable; import org.hibernate.cache.CacheException; import org.hibernate.cache.spi.access.CollectionDataAccess; import org.hibernate.cache.spi.access.SoftLock; import org.hibernate.collection.spi.PersistentCollection; +import org.hibernate.engine.spi.ComparableExecutable; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.internal.FastSessionServices; @@ -27,7 +27,7 @@ import org.hibernate.pretty.MessageHelper; * * @author Gavin King */ -public abstract class CollectionAction implements Executable, Serializable, Comparable { +public abstract class CollectionAction implements ComparableExecutable { private transient CollectionPersister persister; private transient EventSource session; @@ -122,6 +122,16 @@ public abstract class CollectionAction implements Executable, Serializable, Comp return finalKey; } + @Override + public String getPrimarySortClassifier() { + return collectionRole; + } + + @Override + public Object getSecondarySortIndex() { + return key; + } + protected final EventSource getSession() { return session; } @@ -145,15 +155,15 @@ public abstract class CollectionAction implements Executable, Serializable, Comp } @Override - public int compareTo(CollectionAction action) { + public int compareTo(ComparableExecutable o) { // sort first by role name - final int roleComparison = collectionRole.compareTo( action.collectionRole ); + final int roleComparison = collectionRole.compareTo( o.getPrimarySortClassifier() ); if ( roleComparison != 0 ) { return roleComparison; } else { //then by fk - return persister.getAttributeMapping().getKeyDescriptor().compare( key, action.key ); + return persister.getAttributeMapping().getKeyDescriptor().compare( key, o.getSecondarySortIndex() ); // //noinspection unchecked // final JavaType javaType = (JavaType) persister.getAttributeMapping().getKeyDescriptor().getJavaType(); // return javaType.getComparator().compare( key, action.key ); diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java index 429cc29930..203c0f31b3 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java @@ -11,7 +11,7 @@ import java.io.Serializable; import org.hibernate.AssertionFailure; import org.hibernate.action.spi.AfterTransactionCompletionProcess; import org.hibernate.action.spi.BeforeTransactionCompletionProcess; -import org.hibernate.action.spi.Executable; +import org.hibernate.engine.spi.ComparableExecutable; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.event.spi.EventSource; import org.hibernate.internal.FastSessionServices; @@ -26,7 +26,7 @@ import org.hibernate.pretty.MessageHelper; * @author Gavin King */ public abstract class EntityAction - implements Comparable, Executable, Serializable, AfterTransactionCompletionProcess { + implements ComparableExecutable, AfterTransactionCompletionProcess { private final String entityName; private final Object id; @@ -151,13 +151,23 @@ public abstract class EntityAction } @Override - public int compareTo(EntityAction action) { + public int compareTo(ComparableExecutable o) { //sort first by entity name - final int roleComparison = entityName.compareTo( action.getEntityName() ); + final int roleComparison = entityName.compareTo( o.getPrimarySortClassifier() ); return roleComparison != 0 ? roleComparison //then by id - : persister.getIdentifierType().compare( id, action.getId(), session.getSessionFactory() ); + : persister.getIdentifierType().compare( id, o.getSecondarySortIndex(), session.getSessionFactory() ); + } + + @Override + public String getPrimarySortClassifier() { + return entityName; + } + + @Override + public Object getSecondarySortIndex() { + return id; } /** diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index d9a53f42b3..4809e9f15f 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -592,7 +592,7 @@ public class ActionQueue { * @param list The list of Executable elements to be performed * */ - private & Serializable> void executeActions(ExecutableList list) + private void executeActions(ExecutableList list) throws HibernateException { if ( list == null || list.isEmpty() ) { return; @@ -601,7 +601,7 @@ public class ActionQueue { // 1) we explicitly iterate list here to perform Executable#execute() // 2) ExecutableList#getQuerySpaces also iterates the Executables to collect query spaces. try { - for ( E e : list ) { + for ( ComparableExecutable e : list ) { try { e.execute(); } @@ -1354,7 +1354,7 @@ public class ActionQueue { } - private abstract static class ListProvider & Serializable> { + private abstract static class ListProvider { abstract ExecutableList get(ActionQueue instance); abstract ExecutableList init(ActionQueue instance); ExecutableList getOrInit( ActionQueue instance ) { diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ComparableExecutable.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ComparableExecutable.java new file mode 100644 index 0000000000..81398f9d95 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ComparableExecutable.java @@ -0,0 +1,35 @@ +/* + * 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.engine.spi; + +import java.io.Serializable; + +import org.hibernate.action.spi.Executable; + +/** + * We frequently need the union type of Executable, Comparable of ComparableExecutable, Serializable; + * this interface represents such union; this helps to simplify several generic signatures. + * Secondarily, it helps to avoid triggering type pollution by not needing to typecheck + * for a very specific Comparable type; we represent the common needs to resolve sorting + * by exposing primary and secondary sorting attributes. + */ +public interface ComparableExecutable extends Executable, Comparable, Serializable { + + /** + * This affect sorting order of the executables, when sorting is enabled. + * @return the primary sorting attribute; typically the entity name or collection role. + */ + String getPrimarySortClassifier(); + + /** + * This affect sorting order of the executables, when sorting is enabled. + * @return the secondary sorting attribute, applied when getPrimarySortClassifier + * matches during a comparison; typically the entity key or collection key. + */ + Object getSecondarySortIndex(); + +} diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java index 36a776069e..a59f52a4d1 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java @@ -29,24 +29,20 @@ import org.hibernate.internal.util.collections.CollectionHelper; * * @author Steve Ebersole * @author Anton Marsden - * - * @param Intersection type describing {@link Executable} implementations */ -public class ExecutableList & Serializable> +public class ExecutableList implements Serializable, Iterable, Externalizable { public static final int INIT_QUEUE_LIST_SIZE = 5; /** * Provides a sorting interface for {@link ExecutableList}. - * - * @param */ - public interface Sorter { + public interface Sorter { /** * Sorts the list. */ - void sort(List l); + void sort(List l); } private final ArrayList executables; @@ -123,7 +119,7 @@ public class ExecutableList & Seria */ public Set getQuerySpaces() { if ( querySpaces == null ) { - for ( E e : executables ) { + for ( ComparableExecutable e : executables ) { Serializable[] propertySpaces = e.getPropertySpaces(); if ( propertySpaces != null && propertySpaces.length > 0 ) { if( querySpaces == null ) { @@ -132,7 +128,7 @@ public class ExecutableList & Seria Collections.addAll( querySpaces, propertySpaces ); } } - if( querySpaces == null ) { + if ( querySpaces == null ) { return Collections.emptySet(); } } @@ -153,10 +149,10 @@ public class ExecutableList & Seria * * @return the entry that was removed */ - public E remove(int index) { + public ComparableExecutable remove(int index) { // removals are generally safe with regard to sorting... - final E e = executables.remove( index ); + final ComparableExecutable e = executables.remove( index ); // If the executable being removed defined query spaces we need to recalculate the overall query spaces for // this list. The problem is that we don't know how many other executable instances in the list also @@ -187,7 +183,7 @@ public class ExecutableList & Seria public void removeLastN(int n) { if ( n > 0 ) { int size = executables.size(); - for ( Executable e : executables.subList( size - n, size ) ) { + for ( ComparableExecutable e : executables.subList( size - n, size ) ) { if ( e.getPropertySpaces() != null && e.getPropertySpaces().length > 0 ) { // querySpaces could now be incorrect querySpaces = null; @@ -206,7 +202,7 @@ public class ExecutableList & Seria * @return true if the object was added to the list */ public boolean add(E executable) { - final E previousLast = sorter != null || executables.isEmpty() ? null : executables.get( executables.size() - 1 ); + final ComparableExecutable previousLast = sorter != null || executables.isEmpty() ? null : executables.get( executables.size() - 1 ); boolean added = executables.add( executable ); if ( !added ) { @@ -291,7 +287,7 @@ public class ExecutableList & Seria oos.writeBoolean( sorted ); oos.writeInt( executables.size() ); - for ( E e : executables ) { + for ( ComparableExecutable e : executables ) { oos.writeObject( e ); } @@ -347,7 +343,7 @@ public class ExecutableList & Seria * @param session The session with which to associate the {@code Executable}s */ public void afterDeserialize(EventSource session) { - for ( E e : executables ) { + for ( ComparableExecutable e : executables ) { e.afterDeserialize( session ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/NonSortedExecutableListTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/NonSortedExecutableListTest.java index 66ed7c30c1..ccb2cab44f 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/NonSortedExecutableListTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/NonSortedExecutableListTest.java @@ -18,9 +18,8 @@ import java.util.Set; import org.hibernate.HibernateException; import org.hibernate.action.spi.AfterTransactionCompletionProcess; import org.hibernate.action.spi.BeforeTransactionCompletionProcess; -import org.hibernate.action.spi.Executable; +import org.hibernate.engine.spi.ComparableExecutable; import org.hibernate.engine.spi.ExecutableList; -import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.testing.orm.junit.BaseUnitTest; @@ -38,7 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class NonSortedExecutableListTest { // For testing, we need an Executable that is also Comparable and Serializable - private static class AnExecutable implements Executable, Comparable, Serializable { + private static class AnExecutable implements ComparableExecutable { private final int n; private final Serializable[] spaces; @@ -55,8 +54,9 @@ public class NonSortedExecutableListTest { } @Override - public int compareTo(AnExecutable o) { - return Integer.compare( n, o.n ); + public int compareTo(ComparableExecutable o) { + Integer index = (Integer) o.getSecondarySortIndex(); + return Integer.compare( n, index.intValue() ); } @Override @@ -106,6 +106,15 @@ public class NonSortedExecutableListTest { return String.valueOf(n); } + @Override + public String getPrimarySortClassifier() { + return toString(); + } + + @Override + public Object getSecondarySortIndex() { + return Integer.valueOf( n ); + } } private ExecutableList actionList; @@ -303,4 +312,3 @@ public class NonSortedExecutableListTest { assertThat( actionList ).element( 3 ).extracting( AnExecutable::wasAfterDeserializeCalled ).isEqualTo( true ); } } - diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/SortedExecutableListTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/SortedExecutableListTest.java index 724886f685..9a3304dfc6 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/SortedExecutableListTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/engine/action/SortedExecutableListTest.java @@ -18,9 +18,8 @@ import java.util.Set; import org.hibernate.HibernateException; import org.hibernate.action.spi.AfterTransactionCompletionProcess; import org.hibernate.action.spi.BeforeTransactionCompletionProcess; -import org.hibernate.action.spi.Executable; +import org.hibernate.engine.spi.ComparableExecutable; import org.hibernate.engine.spi.ExecutableList; -import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.testing.orm.junit.BaseUnitTest; @@ -37,7 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class SortedExecutableListTest { // For testing, we need an Executable that is also Comparable and Serializable - private static class AnExecutable implements Executable, Comparable, Serializable { + private static class AnExecutable implements ComparableExecutable { private final int n; private Serializable[] spaces; @@ -53,8 +52,9 @@ public class SortedExecutableListTest { } @Override - public int compareTo(AnExecutable o) { - return Integer.compare( n, o.n ); + public int compareTo(ComparableExecutable o) { + Integer index = (Integer) o.getSecondarySortIndex(); + return Integer.compare( n, index.intValue() ); } @Override @@ -101,9 +101,18 @@ public class SortedExecutableListTest { } public String toString() { - return String.valueOf(n); + return String.valueOf( n ); } + @Override + public String getPrimarySortClassifier() { + return toString(); + } + + @Override + public Object getSecondarySortIndex() { + return Integer.valueOf( n ); + } } private ExecutableList actionList; @@ -288,4 +297,3 @@ public class SortedExecutableListTest { assertThat( actionList ).element( 3 ).isEqualTo( action4 ); } } -