HSEARCH-4869 Avoid most uses of EntityPersister

hibernate · Sep 25, 2023 · 4ad4fc0 · 4ad4fc0
1 parent 8bc762c
commit 4ad4fc0
Show file tree

Hide file tree

Showing 19 changed files with 190 additions and 229 deletions.
diff --git a/build/config/src/main/resources/forbidden-public.txt b/build/config/src/main/resources/forbidden-public.txt
@@ -63,6 +63,7 @@ java.time.ZonedDateTime#parse(java.lang.CharSequence, java.time.format.DateTimeF
 @defaultMessage Avoid using Hibernate ORM internals
 org.hibernate.internal.**
 org.hibernate.**.internal.**
+org.hibernate.persister.** @ Prefer EntityMappingType to the implicitly internal EntityPersister and related classes; see https://hibernate.zulipchat.com/#narrow/stream/132094-hibernate-orm-dev/topic/persister.20and.20internal
 
 ################################################################################################################
 @defaultMessage Favor EntityManager methods over their Session equivalent

diff --git a/.../java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java b/.../java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java
@@ -7,10 +7,7 @@
 package org.hibernate.search.batch.jsr352.core.massindexing.util.impl;
 
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
 import java.util.List;
-import java.util.ListIterator;
 import java.util.function.BiFunction;
 
 import jakarta.persistence.EmbeddedId;
@@ -21,7 +18,8 @@
 import jakarta.persistence.criteria.Predicate;
 import jakarta.persistence.criteria.Root;
 
-import org.hibernate.type.ComponentType;
+import org.hibernate.metamodel.mapping.EmbeddableMappingType;
+import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
 
 /**
  * Order over multiple ID attributes.
@@ -49,32 +47,12 @@
  */
 public class CompositeIdOrder implements IdOrder {
 
-	private final ComponentType componentType;
+	private final EntityIdentifierMapping mapping;
+	private final EmbeddableMappingType mappingType;
 
-	private final List<String> propertyPaths;
-
-	private final List<Integer> propertyIndices;
-
-	public CompositeIdOrder(String componentPath, ComponentType componentType) {
-		super();
-		this.componentType = componentType;
-
-		// Initialize with relative paths, but prepend a prefix below
-		this.propertyPaths = new ArrayList<>( Arrays.asList( componentType.getPropertyNames() ) );
-		this.propertyPaths.sort( Comparator.naturalOrder() );
-
-		String pathPrefix = componentPath == null ? "" : componentPath + ".";
-		this.propertyIndices = new ArrayList<>( propertyPaths.size() );
-		ListIterator<String> iterator = this.propertyPaths.listIterator();
-		while ( iterator.hasNext() ) {
-			String propertyName = iterator.next();
-
-			// We need the relative path of the property here
-			propertyIndices.add( componentType.getPropertyIndex( propertyName ) );
-
-			// Prepend the path prefix to each property; we will only use absolute path from now on
-			iterator.set( pathPrefix + propertyName );
-		}
+	public CompositeIdOrder(EntityIdentifierMapping mapping, EmbeddableMappingType mappingType) {
+		this.mapping = mapping;
+		this.mappingType = mappingType;
 	}
 
 	@Override
@@ -107,54 +85,51 @@ public Predicate idLesser(CriteriaBuilder builder, Root<?> root, Object idObj) {
 
 	@Override
 	public void addAscOrder(CriteriaBuilder builder, CriteriaQuery<?> criteria, Root<?> root) {
-		ArrayList<Order> orders = new ArrayList<>( propertyPaths.size() );
-		for ( String path : propertyPaths ) {
-			orders.add( builder.asc( root.get( path ) ) );
-		}
+		ArrayList<Order> orders = new ArrayList<>();
+		mapping.forEachSelectable( (i, selectable) ->
+				orders.add( builder.asc( root.get( selectable.getSelectablePath().getFullPath() ) ) ) );
 		criteria.orderBy( orders );
 	}
 
 	private Predicate restrictLexicographically(BiFunction<String, Object, Predicate> strictOperator,
 			CriteriaBuilder builder, Root<?> root, Object idObj, boolean orEquals) {
-		int propertyPathsSize = propertyPaths.size();
-		int expressionsInOr = propertyPathsSize + ( orEquals ? 1 : 0 );
+		Object[] selectableValues = mappingType.getValues( idObj );
+		int selectablesSize = selectableValues.length;
 
-		Predicate[] or = new Predicate[expressionsInOr];
+		List<Predicate> or = new ArrayList<>();
 
-		for ( int i = 0; i < propertyPathsSize; i++ ) {
+		mapping.forEachSelectable( (i, selectable) -> {
 			// Group expressions together in a single conjunction (A and B and C...).
 			Predicate[] and = new Predicate[i + 1];
-			int j = 0;
-			for ( ; j < and.length - 1; j++ ) {
-				// The first N-1 expressions have symbol `=`
-				String path = propertyPaths.get( j );
-				Object val = getPropertyValue( idObj, j );
-				and[j] = builder.equal( root.get( path ), val );
-			}
+
+			mapping.forEachSelectable( (j, previousSelectable) -> {
+				if ( j < i ) {
+					// The first N-1 expressions have symbol `=`
+					String path = previousSelectable.getSelectablePath().getFullPath();
+					Object val = selectableValues[j];
+					and[j] = builder.equal( root.get( path ), val );
+				}
+			} );
 			// The last expression has whatever symbol is defined by "strictOperator"
-			String path = propertyPaths.get( j );
-			Object val = getPropertyValue( idObj, j );
-			and[j] = strictOperator.apply( path, val );
+			String path = selectable.getSelectablePath().getFullPath();
+			Object val = selectableValues[i];
+			and[i] = strictOperator.apply( path, val );
 
-			or[i] = builder.and( and );
-		}
+			or.add( builder.and( and ) );
+		} );
 
 		if ( orEquals ) {
-			Predicate[] and = new Predicate[propertyPathsSize];
-			for ( int i = 0; i < propertyPathsSize; i++ ) {
-				String path = propertyPaths.get( i );
-				Object val = getPropertyValue( idObj, i );
+			Predicate[] and = new Predicate[selectablesSize];
+			mapping.forEachSelectable( (i, previousSelectable) -> {
+				String path = previousSelectable.getSelectablePath().getFullPath();
+				Object val = selectableValues[i];
 				and[i] = builder.equal( root.get( path ), val );
-			}
-			or[or.length - 1] = builder.and( and );
+			} );
+			or.add( builder.and( and ) );
 		}
 
 		// Group the disjunction of multiple expressions (X or Y or Z...).
-		return builder.or( or );
+		return builder.or( or.toArray( new Predicate[0] ) );
 	}
 
-	private Object getPropertyValue(Object obj, int ourIndex) {
-		int theirIndex = propertyIndices.get( ourIndex );
-		return componentType.getPropertyValue( obj, theirIndex );
-	}
 }
diff --git a/...n/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/PersistenceUtil.java b/...n/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/PersistenceUtil.java
@@ -20,11 +20,11 @@
 import org.hibernate.StatelessSessionBuilder;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.metamodel.MappingMetamodel;
-import org.hibernate.persister.entity.EntityPersister;
+import org.hibernate.metamodel.mapping.EmbeddableMappingType;
+import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
+import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.search.batch.jsr352.core.massindexing.step.impl.IndexScope;
 import org.hibernate.search.util.common.impl.StringHelper;
-import org.hibernate.type.ComponentType;
-import org.hibernate.type.Type;
 
 /**
  * Internal utility class for persistence usage.
@@ -104,20 +104,19 @@ public static List<EntityTypeDescriptor> createDescriptors(EntityManagerFactory
 	}
 
 	private static <T> EntityTypeDescriptor createDescriptor(MappingMetamodel metamodel, Class<T> type) {
-		EntityPersister entityPersister = metamodel.findEntityDescriptor( type );
-		IdOrder idOrder = createIdOrder( entityPersister );
+		EntityMappingType entityMappingType = metamodel.findEntityDescriptor( type );
+		IdOrder idOrder = createIdOrder( entityMappingType );
 		return new EntityTypeDescriptor( type, idOrder );
 	}
 
-	private static IdOrder createIdOrder(EntityPersister entityPersister) {
-		final String identifierPropertyName = entityPersister.getIdentifierPropertyName();
-		final Type identifierType = entityPersister.getIdentifierType();
-		if ( identifierType instanceof ComponentType ) {
-			final ComponentType componentType = (ComponentType) identifierType;
-			return new CompositeIdOrder( identifierPropertyName, componentType );
+	private static IdOrder createIdOrder(EntityMappingType entityMappingType) {
+		EntityIdentifierMapping identifierMapping = entityMappingType.getIdentifierMapping();
+		if ( identifierMapping.getPartMappingType() instanceof EmbeddableMappingType ) {
+			return new CompositeIdOrder( identifierMapping,
+					(EmbeddableMappingType) identifierMapping.getPartMappingType() );
 		}
 		else {
-			return new SingularIdOrder( identifierPropertyName );
+			return new SingularIdOrder( identifierMapping.getAttributeName() );
 		}
 	}
 

diff --git a/mapper/orm/src/main/java/org/hibernate/search/mapper/orm/common/impl/HibernateOrmUtils.java b/mapper/orm/src/main/java/org/hibernate/search/mapper/orm/common/impl/HibernateOrmUtils.java
@@ -17,15 +17,14 @@
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.EntityManagerFactory;
 
-import org.hibernate.AssertionFailure;
 import org.hibernate.Session;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.mapping.Property;
 import org.hibernate.metamodel.MappingMetamodel;
 import org.hibernate.metamodel.mapping.EntityMappingType;
-import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.search.mapper.orm.logging.impl.Log;
+import org.hibernate.search.util.common.AssertionFailure;
 import org.hibernate.search.util.common.annotation.impl.SuppressForbiddenApis;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 import org.hibernate.service.Service;
@@ -67,51 +66,34 @@ public static SessionImplementor toSessionImplementor(EntityManager entityManage
 		}
 	}
 
-	private static boolean isSuperTypeOf(EntityPersister type1, EntityPersister type2) {
-		return type1.isSubclassEntityName( type2.getEntityName() );
+	private static boolean isSuperTypeOf(EntityMappingType type1, EntityMappingType type2) {
+		return type1.getSubclassEntityNames().contains( type2.getEntityName() );
 	}
 
-	public static EntityPersister toRootEntityType(SessionFactoryImplementor sessionFactory,
-			EntityPersister entityType) {
-		/*
-		 * We need to rely on Hibernate ORM's SPIs: this is complex stuff.
-		 * For example there may be class hierarchies such as A > B > C
-		 * where A and C are entity types and B is a mapped superclass.
-		 * So we need to exclude non-entity types, and for that we need the Hibernate ORM metamodel.
-		 */
-		MappingMetamodel metamodel = sessionFactory.getMappingMetamodel();
-		String rootEntityName = entityType.getRootEntityName();
-		return metamodel.getEntityDescriptor( rootEntityName );
-	}
-
-	public static EntityPersister toMostSpecificCommonEntitySuperType(EntityPersister type1, EntityPersister type2) {
+	public static EntityMappingType toMostSpecificCommonEntitySuperType(EntityMappingType type1, EntityMappingType type2) {
 		/*
 		 * We need to rely on Hibernate ORM's SPIs: this is complex stuff.
 		 * For example there may be class hierarchies such as A > B > C
 		 * where A and C are entity types and B is a mapped superclass.
 		 * So even if we know the two types have a common superclass,
 		 * we need to skip non-entity superclasses, and for that we need the Hibernate ORM metamodel.
 		 */
-		EntityPersister superTypeCandidate = type1;
+		EntityMappingType superTypeCandidate = type1;
 		while ( superTypeCandidate != null && !isSuperTypeOf( superTypeCandidate, type2 ) ) {
-			EntityMappingType superSuperType = superTypeCandidate.getSuperMappingType();
-			superTypeCandidate = superSuperType == null
-					? null : superSuperType.getEntityPersister();
+			superTypeCandidate = superTypeCandidate.getSuperMappingType();
 		}
 		if ( superTypeCandidate == null ) {
 			throw new AssertionFailure(
 					"Cannot find a common entity supertype for " + type1.getEntityName()
 							+ " and " + type2.getEntityName() + "."
-							+ " There is a bug in Hibernate Search, please report it."
 			);
 		}
 		return superTypeCandidate;
 	}
 
 	public static boolean targetsAllConcreteSubTypes(SessionFactoryImplementor sessionFactory,
-			EntityPersister parentType, Collection<?> targetConcreteSubTypes) {
-		@SuppressWarnings("unchecked")
-		Set<String> subClassEntityNames = parentType.getEntityMetamodel().getSubclassEntityNames();
+			EntityMappingType parentType, Collection<?> targetConcreteSubTypes) {
+		Set<String> subClassEntityNames = parentType.getSubclassEntityNames();
 		// Quick check to return true immediately if all subtypes are concrete
 		if ( subClassEntityNames.size() == targetConcreteSubTypes.size() ) {
 			return true;
@@ -120,7 +102,8 @@ public static boolean targetsAllConcreteSubTypes(SessionFactoryImplementor sessi
 		MappingMetamodel metamodel = sessionFactory.getMappingMetamodel();
 		int concreteSubTypesCount = 0;
 		for ( String subClassEntityName : subClassEntityNames ) {
-			if ( !metamodel.getEntityDescriptor( subClassEntityName ).isAbstract() ) {
+			EntityMappingType subclassType = metamodel.getEntityDescriptor( subClassEntityName );
+			if ( !subclassType.isAbstract() ) {
 				++concreteSubTypesCount;
 			}
 		}
@@ -132,7 +115,7 @@ public static <T extends Service> T getServiceOrFail(ServiceRegistry serviceRegi
 			Class<T> serviceClass) {
 		T service = serviceRegistry.getService( serviceClass );
 		if ( service == null ) {
-			throw new org.hibernate.search.util.common.AssertionFailure(
+			throw new AssertionFailure(
 					"A required service was missing. Missing service: " + serviceClass );
 		}
 		return service;

diff --git a/...rc/main/java/org/hibernate/search/mapper/orm/event/impl/HibernateSearchEventListener.java b/...rc/main/java/org/hibernate/search/mapper/orm/event/impl/HibernateSearchEventListener.java
@@ -35,11 +35,13 @@
 import org.hibernate.event.spi.PostInsertEventListener;
 import org.hibernate.event.spi.PostUpdateEvent;
 import org.hibernate.event.spi.PostUpdateEventListener;
+import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.search.mapper.orm.common.impl.HibernateOrmUtils;
 import org.hibernate.search.mapper.orm.logging.impl.Log;
 import org.hibernate.search.mapper.pojo.work.spi.PojoIndexingPlan;
 import org.hibernate.search.mapper.pojo.work.spi.PojoTypeIndexingPlan;
+import org.hibernate.search.util.common.annotation.impl.SuppressForbiddenApis;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 /**
@@ -290,8 +292,8 @@ private PojoIndexingPlan getCurrentIndexingPlanIfExisting(SessionImplementor ses
 		return contextProvider.currentIndexingPlanIfExisting( sessionImplementor );
 	}
 
-	private HibernateOrmListenerTypeContext getTypeContextOrNull(EntityPersister entityPersister) {
-		String entityName = entityPersister.getEntityName();
+	private HibernateOrmListenerTypeContext getTypeContextOrNull(EntityMappingType entityMappingType) {
+		String entityName = entityMappingType.getEntityName();
 		return contextProvider.typeContextProvider().byHibernateOrmEntityName().getOrNull( entityName );
 	}
 
@@ -363,7 +365,7 @@ private void processCollectionEvent(AbstractCollectionEvent event) {
 	 * Required since Hibernate ORM 4.3
 	 */
 	@Override
-	@SuppressWarnings("deprecation") // Deprecated but abstract, so we have to implement it...
+	@SuppressForbiddenApis(reason = "We are forced to implement this method and it requires accepting an EntityPersister")
 	public boolean requiresPostCommitHandling(EntityPersister persister) {
 		// TODO Tests seem to pass using _false_ but we might be able to take
 		// advantage of this new hook?

diff --git a/...ava/org/hibernate/search/mapper/orm/loading/impl/AbstractHibernateOrmLoadingStrategy.java b/...ava/org/hibernate/search/mapper/orm/loading/impl/AbstractHibernateOrmLoadingStrategy.java
@@ -13,7 +13,7 @@
 import java.util.Set;
 
 import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.persister.entity.EntityPersister;
+import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.search.mapper.orm.common.impl.HibernateOrmUtils;
 import org.hibernate.search.mapper.orm.massindexing.impl.ConditionalExpression;
 import org.hibernate.search.util.common.AssertionFailure;
@@ -22,21 +22,21 @@ public abstract class AbstractHibernateOrmLoadingStrategy<E, I>
 		implements HibernateOrmEntityLoadingStrategy<E, I> {
 
 	private final SessionFactoryImplementor sessionFactory;
-	private final EntityPersister rootEntityPersister;
+	private final EntityMappingType rootEntityMappingType;
 	private final TypeQueryFactory<E, I> queryFactory;
 
 	AbstractHibernateOrmLoadingStrategy(SessionFactoryImplementor sessionFactory,
-			EntityPersister rootEntityPersister, TypeQueryFactory<E, I> queryFactory) {
+			EntityMappingType rootEntityMappingType, TypeQueryFactory<E, I> queryFactory) {
 		this.sessionFactory = sessionFactory;
-		this.rootEntityPersister = rootEntityPersister;
+		this.rootEntityMappingType = rootEntityMappingType;
 		this.queryFactory = queryFactory;
 	}
 
 	@Override
 	public HibernateOrmQueryLoader<E, I> createQueryLoader(
 			List<LoadingTypeContext<? extends E>> typeContexts, Optional<ConditionalExpression> conditionalExpression) {
 		Set<Class<? extends E>> includedTypesFilter;
-		if ( HibernateOrmUtils.targetsAllConcreteSubTypes( sessionFactory, rootEntityPersister, typeContexts ) ) {
+		if ( HibernateOrmUtils.targetsAllConcreteSubTypes( sessionFactory, rootEntityMappingType, typeContexts ) ) {
 			// All concrete types are included, no need to filter by type.
 			includedTypesFilter = Collections.emptySet();
 		}
@@ -52,9 +52,9 @@ public HibernateOrmQueryLoader<E, I> createQueryLoader(
 				throw new AssertionFailure( "conditional expression is always defined on a single type" );
 			}
 
-			EntityPersister entityPersister = typeContexts.get( 0 ).entityPersister();
+			EntityMappingType entityMappingType = typeContexts.get( 0 ).entityMappingType();
 			return new HibernateOrmQueryLoader<>(
-					queryFactory, entityPersister, includedTypesFilter, conditionalExpression.get() );
+					queryFactory, entityMappingType, includedTypesFilter, conditionalExpression.get() );
 		}
 		return new HibernateOrmQueryLoader<>( queryFactory, includedTypesFilter );
 	}