HHH-19897 Envers - check mapped-by side is audited for associations

Author: mbellade

hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/EnversMetadataBuildingContextImpl.java

@@ -12,6 +12,7 @@
 import org.hibernate.boot.spi.MetadataBuildingOptions;
 import org.hibernate.envers.boot.spi.EnversMetadataBuildingContext;
 import org.hibernate.envers.configuration.Configuration;
+import org.hibernate.envers.configuration.internal.ClassesAuditingData;
 import org.hibernate.envers.configuration.internal.MappingCollector;
 import org.hibernate.envers.configuration.internal.metadata.AuditEntityConfigurationRegistry;
 import org.hibernate.envers.configuration.internal.metadata.AuditEntityNameRegister;
@@ -33,6 +34,7 @@ public class EnversMetadataBuildingContextImpl implements EnversMetadataBuilding
 	private final ObjectNameNormalizer objectNameNormalizer;
 	private final AuditEntityNameRegister auditEntityNameRegistry;
 	private final AuditEntityConfigurationRegistry auditEntityConfigurationRegistry;
+	private final ClassesAuditingData classesAuditingData;
 
 	public EnversMetadataBuildingContextImpl(
 			Configuration configuration,
@@ -45,6 +47,7 @@ public EnversMetadataBuildingContextImpl(
 		this.mappingCollector = mappingCollector;
 		this.auditEntityNameRegistry = new AuditEntityNameRegister();
 		this.auditEntityConfigurationRegistry = new AuditEntityConfigurationRegistry();
+		this.classesAuditingData = new ClassesAuditingData();
 
 		this.objectNameNormalizer = new ObjectNameNormalizer(this);
 	}
@@ -118,4 +121,9 @@ public AuditEntityNameRegister getAuditEntityNameRegistry() {
 	public AuditEntityConfigurationRegistry getAuditEntityConfigurationRegistry() {
 		return auditEntityConfigurationRegistry;
 	}
+
+	@Override
+	public ClassesAuditingData getClassesAuditingData() {
+		return classesAuditingData;
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/EnversMetadataBuildingContext.java

@@ -6,6 +6,7 @@
 
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.envers.configuration.Configuration;
+import org.hibernate.envers.configuration.internal.ClassesAuditingData;
 import org.hibernate.envers.configuration.internal.MappingCollector;
 import org.hibernate.envers.configuration.internal.metadata.AuditEntityConfigurationRegistry;
 import org.hibernate.envers.configuration.internal.metadata.AuditEntityNameRegister;
@@ -32,4 +33,6 @@ public interface EnversMetadataBuildingContext extends MetadataBuildingContext {
 	AuditEntityNameRegister getAuditEntityNameRegistry();
 
 	AuditEntityConfigurationRegistry getAuditEntityConfigurationRegistry();
+
+	ClassesAuditingData getClassesAuditingData();
 }

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/ClassesAuditingData.java

@@ -61,6 +61,10 @@ public Collection<ClassAuditingData> getAllClassAuditedData() {
 		return persistentClassToAuditingData.values();
 	}
 
+	public ClassAuditingData getClassAuditingData(String entityName) {
+		return entityNameToAuditingData.get( entityName );
+	}
+
 	/**
 	 * After all meta-data is read, updates calculated fields. This includes:
 	 * <ul>

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/EntitiesConfigurator.java

@@ -38,7 +38,7 @@ public EntitiesConfigurations configure(EnversMetadataBuildingContext metadataBu
 		final Iterator<PersistentClass> classes = GraphTopologicalSort.sort( new PersistentClassGraphDefiner( metadata ) )
 				.iterator();
 
-		final ClassesAuditingData classesAuditingData = new ClassesAuditingData();
+		final ClassesAuditingData classesAuditingData = metadataBuildingContext.getClassesAuditingData();
 
 		// Reading metadata from annotations
 		final AnnotationsMetadataReader reader = new AnnotationsMetadataReader( metadataBuildingContext );

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMappedByResolver.java

@@ -4,26 +4,27 @@
  */
 package org.hibernate.envers.configuration.internal.metadata;
 
-import java.lang.invoke.MethodHandles;
-import java.util.List;
-import java.util.Locale;
-import java.util.Objects;
-
 import org.hibernate.envers.boot.EnversMappingException;
+import org.hibernate.envers.boot.spi.EnversMetadataBuildingContext;
+import org.hibernate.envers.configuration.internal.metadata.reader.AuditedPropertiesHolder;
+import org.hibernate.envers.configuration.internal.metadata.reader.ClassAuditingData;
+import org.hibernate.envers.configuration.internal.metadata.reader.ComponentAuditingData;
 import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData;
 import org.hibernate.envers.internal.EnversMessageLogger;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Component;
 import org.hibernate.mapping.KeyValue;
-import org.hibernate.mapping.ManyToOne;
-import org.hibernate.mapping.OneToMany;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
 import org.hibernate.mapping.Selectable;
 import org.hibernate.mapping.Table;
-
 import org.jboss.logging.Logger;
 
+import java.lang.invoke.MethodHandles;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
 /**
  * Helper class that provides a way to resolve the {@code mappedBy} attribute for collections.
  *
@@ -37,26 +38,51 @@ public class CollectionMappedByResolver {
 			CollectionMappedByResolver.class.getName()
 	);
 
-	public static String resolveMappedBy(Collection collection, PropertyAuditingData propertyAuditingData) {
-		final PersistentClass referencedClass = getReferenceCollectionClass( collection );
+	public static String resolveMappedBy(
+			String entityName,
+			Collection collection,
+			PropertyAuditingData propertyAuditingData,
+			String referencedEntityName,
+			EnversMetadataBuildingContext buildingContext) {
+		final var referencedClass = getReferencedClass( referencedEntityName, buildingContext );
 		final ResolverContext resolverContext = new ResolverContext( collection, propertyAuditingData );
-		return getMappedBy( referencedClass, resolverContext );
+		return getMappedBy( entityName, referencedClass, resolverContext, buildingContext );
 	}
 
-	public static String resolveMappedBy(Table collectionTable, PersistentClass referencedClass, PropertyAuditingData propertyAuditingData) {
-		return getMappedBy( referencedClass, new ResolverContext( collectionTable, propertyAuditingData ) );
+	public static String resolveMappedBy(
+			String entityName,
+			Table collectionTable,
+			PropertyAuditingData propertyAuditingData,
+			String referencedEntityName,
+			EnversMetadataBuildingContext buildingContext) {
+		final var referencedClass = getReferencedClass( referencedEntityName, buildingContext );
+		return getMappedBy(
+				entityName,
+				referencedClass,
+				new ResolverContext( collectionTable, propertyAuditingData ),
+				buildingContext
+		);
 	}
 
-	public static boolean isMappedByKey(Collection collection, String mappedBy) {
-		final PersistentClass referencedClass = getReferenceCollectionClass( collection );
+	public static boolean isMappedByKey(
+			Collection collection,
+			String mappedBy,
+			String referencedEntityName,
+			EnversMetadataBuildingContext buildingContext) {
+		final PersistentClass referencedClass = getReferencedClass( referencedEntityName,
+				buildingContext ).getPersistentClass();
 		if ( referencedClass != null ) {
 			final String keyMappedBy = searchMappedByKey( referencedClass, collection );
 			return mappedBy.equals( keyMappedBy );
 		}
 		return false;
 	}
 
-	private static String getMappedBy(PersistentClass referencedClass, ResolverContext resolverContext) {
+	private static String getMappedBy(
+			String entityName,
+			ClassAuditingData referencedClass,
+			ResolverContext resolverContext,
+			EnversMetadataBuildingContext buildingContext) {
 		// If there's an @AuditMappedBy specified, returning it directly.
 		final String auditMappedBy = resolverContext.propertyAuditingData.getAuditMappedBy();
 		if ( auditMappedBy != null ) {
@@ -70,74 +96,93 @@ private static String getMappedBy(PersistentClass referencedClass, ResolverConte
 			LOG.debugf(
 					"Going to search the mapped by attribute for %s in superclasses of entity: %s",
 					resolverContext.propertyAuditingData.getName(),
-					referencedClass.getClassName()
+					referencedClass.getEntityName()
 			);
 
-			PersistentClass tempClass = referencedClass;
-			while ( mappedBy == null && tempClass.getSuperclass() != null ) {
-				LOG.debugf( "Searching in superclass: %s", tempClass.getSuperclass().getClassName() );
-				mappedBy = searchMappedBy( tempClass.getSuperclass(), resolverContext );
+			PersistentClass tempClass = referencedClass.getPersistentClass().getSuperclass();
+			while ( mappedBy == null && tempClass != null ) {
+				final var superclassName = tempClass.getEntityName();
+				LOG.debugf( "Searching in superclass: %s", superclassName );
+				final var auditingData = buildingContext.getClassesAuditingData().getClassAuditingData( superclassName );
+				mappedBy = searchMappedBy( auditingData, resolverContext );
 				tempClass = tempClass.getSuperclass();
 			}
 		}
 
 		if ( mappedBy == null ) {
 			throw new EnversMappingException(
 					String.format(
-							Locale.ENGLISH,
-							"Unable to read mapped by attribute for %s in %s!",
+							Locale.ROOT,
+							"Could not resolve mapped by property for association [%s.%s] in the referenced entity [%s],"
+							+ " please ensure that the association is audited on both sides.",
+							entityName,
 							resolverContext.propertyAuditingData.getName(),
-							referencedClass.getClassName()
+							referencedClass.getEntityName()
 					)
 			);
 		}
 
 		return mappedBy;
 	}
 
-	private static String searchMappedBy(PersistentClass persistentClass, ResolverContext resolverContext) {
+	private static String searchMappedBy(ClassAuditingData referencedClass, ResolverContext resolverContext) {
 		if ( resolverContext.getCollection() != null ) {
-			return searchMappedBy( persistentClass, resolverContext.getCollection() );
+			return searchMappedBy( referencedClass, resolverContext.getCollection() );
 		}
-		return searchMappedBy( persistentClass, resolverContext.getTable() );
+		return searchMappedBy( referencedClass, resolverContext.getTable() );
 	}
 
-	private static String searchMappedBy(PersistentClass referencedClass, Collection collectionValue) {
-		final List<Property> assocClassProps = referencedClass.getProperties();
+	private static String searchMappedBy(ClassAuditingData referencedClass, Collection collectionValue) {
+		final var persistentClass = referencedClass.getPersistentClass();
+		final List<Property> assocClassProps = referencedClass.getPersistentClass().getProperties();
 		for ( Property property : assocClassProps ) {
 			final List<Selectable> assocClassSelectables = property.getValue().getSelectables();
 			final List<Selectable> collectionKeySelectables = collectionValue.getKey().getSelectables();
 			if ( Objects.equals( assocClassSelectables, collectionKeySelectables ) ) {
-				return property.getName();
+				final var propertyName = property.getName();
+				// We need to check if the property is audited as well
+				return referencedClass.contains( propertyName ) ? propertyName : null;
 			}
 		}
 		// HHH-7625
 		// Support ToOne relations with mappedBy that point to an @IdClass key property.
-		return searchMappedByKey( referencedClass, collectionValue );
+		return searchMappedByKey( persistentClass, collectionValue );
 	}
 
-	private static String searchMappedBy(PersistentClass referencedClass, Table collectionTable) {
-		return searchMappedBy( referencedClass.getProperties(), collectionTable );
+	private static String searchMappedBy(ClassAuditingData referencedClass, Table collectionTable) {
+		return searchMappedBy( referencedClass, referencedClass.getPersistentClass().getProperties(), collectionTable );
 	}
 
-	private static String searchMappedBy(List<Property> properties, Table collectionTable) {
+	private static String searchMappedBy(AuditedPropertiesHolder propertiesHolder, List<Property> properties, Table collectionTable) {
 		for ( Property property : properties ) {
 			if ( property.getValue() instanceof Collection ) {
 				// The equality is intentional. We want to find a collection property with the same collection table.
 				//noinspection ObjectEquality
-				if ( ( (Collection) property.getValue() ).getCollectionTable() == collectionTable ) {
-					return property.getName();
+				if ( ((Collection) property.getValue()).getCollectionTable() == collectionTable ) {
+					final var propertyName = property.getName();
+					// We need to check if the property is audited as well
+					return propertiesHolder.contains( propertyName ) ? propertyName : null;
 				}
 			}
-			else if ( property.getValue() instanceof Component ) {
+			else if ( property.getValue() instanceof Component component ) {
 				// HHH-12240
 				// Should we find an embeddable, we should traverse it as well to see if the collection table
 				// happens to be an attribute inside the embeddable rather than directly on the entity.
-				final Component component = (Component) property.getValue();
-
-				final String mappedBy = searchMappedBy( component.getProperties(), collectionTable );
-				if ( mappedBy != null ) {
-					return property.getName() + "_" + mappedBy;
+				final var componentName = property.getName();
+				final var componentData = propertiesHolder.getPropertyAuditingData( componentName );
+				if ( componentData == null ) {
+					// If the component is not audited, no need to check sub-properties
+					return null;
+				}
+				else {
+					final String mappedBy = searchMappedBy(
+							(ComponentAuditingData) componentData,
+							component.getProperties(),
+							collectionTable
+					);
+					if ( mappedBy != null ) {
+						return property.getName() + "_" + mappedBy;
+					}
 				}
 			}
 		}
@@ -161,18 +206,8 @@ private static String searchMappedByKey(PersistentClass referencedClass, Collect
 		return null;
 	}
 
-	private static PersistentClass getReferenceCollectionClass(Collection collectionValue) {
-		PersistentClass referencedClass = null;
-		if ( collectionValue.getElement() instanceof OneToMany ) {
-			final OneToMany oneToManyValue = (OneToMany) collectionValue.getElement();
-			referencedClass = oneToManyValue.getAssociatedClass();
-		}
-		else if ( collectionValue.getElement() instanceof ManyToOne ) {
-			// Case for bidirectional relation with @JoinTable on the owning @ManyToOne side.
-			final ManyToOne manyToOneValue = (ManyToOne) collectionValue.getElement();
-			referencedClass = manyToOneValue.getMetadata().getEntityBinding( manyToOneValue.getReferencedEntityName() );
-		}
-		return referencedClass;
+	private static ClassAuditingData getReferencedClass(String className, EnversMetadataBuildingContext buildingContext) {
+		return buildingContext.getClassesAuditingData().getClassAuditingData( className );
 	}
 
 	private static class ResolverContext {

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/JoinColumnCollectionMetadataGenerator.java

@@ -54,19 +54,27 @@ public JoinColumnCollectionMetadataGenerator(
 
 	@Override
 	public void addCollection(CollectionMetadataContext context) {
+		final var referencingEntityName = context.getReferencingEntityName();
 		LOG.debugf(
 				"Adding audit mapping for property %s.%s: one-to-many collection, using a join column on the referenced entity",
-				context.getReferencingEntityName(),
+				referencingEntityName,
 				context.getPropertyName()
 		);
 
 		final Collection collection = context.getCollection();
 		final PropertyAuditingData propertyAuditingData = context.getPropertyAuditingData();
-		final String mappedBy = CollectionMappedByResolver.resolveMappedBy( collection, propertyAuditingData );
+		final var referencedEntityName = context.getReferencedEntityName();
+		final String mappedBy = CollectionMappedByResolver.resolveMappedBy(
+				referencingEntityName,
+				collection,
+				propertyAuditingData,
+				referencedEntityName,
+				getMetadataBuildingContext()
+		);
 
 		final IdMappingData referencedIdMapping = getReferencedIdMappingData(
-				context.getReferencingEntityName(),
-				context.getReferencedEntityName(),
+				referencingEntityName,
+				referencedEntityName,
 				propertyAuditingData,
 				false
 		);
@@ -78,15 +86,15 @@ public void addCollection(CollectionMetadataContext context) {
 		final MiddleIdData referencingIdData = createMiddleIdData(
 				referencingIdMapping,
 				mappedBy + "_",
-				context.getReferencingEntityName()
+				referencingEntityName
 		);
 
 		// And for the referenced side. The prefixed mapper won't be used (as this collection isn't persisted
 		// in a join table, so the prefix value is arbitrary).
 		final MiddleIdData referencedIdData = createMiddleIdData(
 				referencedIdMapping,
 				null,
-				context.getReferencedEntityName()
+				referencedEntityName
 		);
 
 		// Generating the element mapping.
@@ -102,18 +110,18 @@ public void addCollection(CollectionMetadataContext context) {
 		final RelationQueryGenerator queryGenerator = new OneAuditEntityQueryGenerator(
 				getMetadataBuildingContext().getConfiguration(),
 				referencingIdData,
-				context.getReferencedEntityName(),
+				referencedEntityName,
 				referencedIdData,
 				context.getCollection().getElement() instanceof ComponentType,
 				mappedBy,
-				CollectionMappedByResolver.isMappedByKey( collection, mappedBy ),
+				CollectionMappedByResolver.isMappedByKey( collection, mappedBy, referencedEntityName, getMetadataBuildingContext() ),
 				getOrderByCollectionRole( collection, collection.getOrderBy() )
 		);
 
 		// Creating common mapper data.
 		final CommonCollectionMapperData commonCollectionMapperData = createCommonCollectionMapperData(
 				context,
-				context.getReferencedEntityName(),
+				referencedEntityName,
 				referencingIdData,
 				queryGenerator
 		);
@@ -126,7 +134,7 @@ public void addCollection(CollectionMetadataContext context) {
 			final String auditMappedBy = getAddOneToManyAttachedAuditMappedBy( context );
 
 			fakeBidirectionalRelationMapper = getBidirectionalRelationMapper(
-					context.getReferencingEntityName(),
+					referencingEntityName,
 					referencingIdMapping,
 					auditMappedBy
 			);
@@ -153,7 +161,7 @@ public void addCollection(CollectionMetadataContext context) {
 		referencingEntityConfiguration.addToManyNotOwningRelation(
 				context.getPropertyName(),
 				mappedBy,
-				context.getReferencedEntityName(),
+				referencedEntityName,
 				referencingIdData.getPrefixedMapper(),
 				fakeBidirectionalRelationMapper,
 				fakeBidirectionalRelationIndexMapper,