From f379d51504388a302b0fa4241b9f5b8c21f05e76 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Fri, 26 Nov 2021 01:37:33 -0500 Subject: [PATCH] HHH-9228 HHH-9229 Fix audited/auditoverride for embeddables and mappedsuperclass --- .../metadata/reader/AuditOverrideData.java | 6 + .../reader/AuditedPropertiesReader.java | 22 +- .../ComponentAuditedPropertiesReader.java | 63 +++ .../auditoverride/EmbeddableTest.java | 463 ++++++++++++++++++ 4 files changed, 553 insertions(+), 1 deletion(-) create mode 100644 hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/superclass/auditoverride/EmbeddableTest.java diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditOverrideData.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditOverrideData.java index e01c99a006..9802c30f7d 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditOverrideData.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditOverrideData.java @@ -17,11 +17,13 @@ public class AuditOverrideData { private final String name; private final boolean audited; + private final Class forClass; private final AuditJoinTableData auditJoinTableData; public AuditOverrideData(AuditOverride auditOverride) { this.name = auditOverride.name(); this.audited = auditOverride.isAudited(); + this.forClass = auditOverride.forClass(); this.auditJoinTableData = new AuditJoinTableData( auditOverride.auditJoinTable() ); } @@ -33,6 +35,10 @@ public class AuditOverrideData { return audited; } + public Class getForClass() { + return forClass; + } + public AuditJoinTableData getAuditJoinTableData() { return auditJoinTableData; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java index b2ee488109..accf97c9df 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java @@ -369,7 +369,7 @@ public class AuditedPropertiesReader { allClassAudited ); - if ( allClassAudited != null || !auditedPropertiesHolder.isEmpty() ) { + if ( isClassHierarchyTraversalNeeded( allClassAudited ) ) { final XClass superclazz = clazz.getSuperclass(); if ( !clazz.isInterface() && !"java.lang.Object".equals( superclazz.getName() ) ) { addPropertiesFromClass( superclazz ); @@ -377,6 +377,10 @@ public class AuditedPropertiesReader { } } + protected boolean isClassHierarchyTraversalNeeded(Audited allClassAudited) { + return allClassAudited != null || !auditedPropertiesHolder.isEmpty(); + } + private void addFromProperties( Iterable properties, Function accessTypeProvider, @@ -715,6 +719,22 @@ public class AuditedPropertiesReader { return true; } + protected boolean isOverriddenNotAudited(XProperty property) { + return overriddenNotAuditedProperties.contains( property ); + } + + protected boolean isOverriddenNotAudited(XClass clazz) { + return overriddenNotAuditedClasses.contains( clazz ); + } + + protected boolean isOverriddenAudited(XProperty property) { + return overriddenAuditedProperties.contains( property ); + } + + protected boolean isOverriddenAudited(XClass clazz) { + return overriddenAuditedClasses.contains( clazz ); + } + private static final Audited DEFAULT_AUDITED = new Audited() { @Override public RelationTargetAuditMode targetAuditMode() { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java index 55bb1b3bf4..0f4d249d90 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java @@ -6,12 +6,16 @@ */ package org.hibernate.envers.configuration.internal.metadata.reader; +import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.XProperty; +import org.hibernate.envers.AuditOverride; import org.hibernate.envers.Audited; import org.hibernate.envers.boot.internal.ModifiedColumnNameResolver; import org.hibernate.envers.boot.spi.EnversMetadataBuildingContext; import org.hibernate.internal.util.StringHelper; +import jakarta.persistence.MappedSuperclass; + /** * Reads the audited properties for components. * @@ -21,6 +25,8 @@ import org.hibernate.internal.util.StringHelper; */ public class ComponentAuditedPropertiesReader extends AuditedPropertiesReader { + private final ComponentAuditingData componentAuditingData; + public ComponentAuditedPropertiesReader( EnversMetadataBuildingContext metadataBuildingContext, PersistentPropertiesSource persistentPropertiesSource, @@ -39,6 +45,13 @@ public class ComponentAuditedPropertiesReader extends AuditedPropertiesReader { auditedPropertiesHolder, propertyNamePrefix ); + this.componentAuditingData = (ComponentAuditingData) auditedPropertiesHolder; + } + + @Override + protected boolean isClassHierarchyTraversalNeeded(Audited allClassAudited) { + // we always traverse the hierarchy for components + return true; } @Override @@ -58,6 +71,56 @@ public class ComponentAuditedPropertiesReader extends AuditedPropertiesReader { propertyData.setExplicitModifiedFlagName( aud.modifiedColumnName() ); } } + else { + + // get declaring class for property + final XClass declaringClass = property.getDeclaringClass(); + + // check component data to make sure that no audit overrides were not defined at + // the parent class or the embeddable at the property level. + boolean classAuditedOverride = false; + boolean classNotAuditedOverride = false; + for ( AuditOverrideData auditOverride : componentAuditingData.getAuditingOverrides() ) { + if ( auditOverride.getForClass() != void.class ) { + final String className = auditOverride.getForClass().getName(); + if ( declaringClass.getName().equals( className ) ) { + if ( !auditOverride.isAudited() ) { + if ( "".equals( auditOverride.getName() ) ) { + classNotAuditedOverride = true; + } + if ( property.getName().equals( auditOverride.getName() ) ) { + return false; + } + } + else { + if ( "".equals( auditOverride.getName() ) ) { + classAuditedOverride = true; + } + if ( property.getName().equals( auditOverride.getName() ) ) { + return true; + } + } + } + } + } + + // make sure taht if the class or property are explicitly 'isAudited=false', use that. + if ( classNotAuditedOverride || isOverriddenNotAudited( property) || isOverriddenNotAudited( declaringClass ) ) { + return false; + } + + // make sure that if the class or property are explicitly 'isAudited=true', use that. + if ( classAuditedOverride || isOverriddenAudited( property ) || isOverriddenAudited( declaringClass ) ) { + return true; + } + + // assumption here is if a component reader is looking at a @MappedSuperclass, it should be treated + // as not being audited if we have reached htis point; allowing components and any @Embeddable + // class being audited by default. + if ( declaringClass.isAnnotationPresent( MappedSuperclass.class ) ) { + return false; + } + } return true; } diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/superclass/auditoverride/EmbeddableTest.java b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/superclass/auditoverride/EmbeddableTest.java new file mode 100644 index 0000000000..edd490680e --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/superclass/auditoverride/EmbeddableTest.java @@ -0,0 +1,463 @@ +/* + * 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 . + */ +package org.hibernate.orm.test.envers.integration.superclass.auditoverride; + +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.EntityManager; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.MappedSuperclass; + +import org.hibernate.envers.AuditOverride; +import org.hibernate.envers.AuditOverrides; +import org.hibernate.envers.Audited; +import org.hibernate.envers.NotAudited; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.testing.TestForIssue; +import org.hibernate.orm.test.envers.BaseEnversJPAFunctionalTestCase; +import org.hibernate.orm.test.envers.Priority; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * This class addresses numerous issues with Embeddable annotated classes + * for issues {@code HHH-9228} and {@code HHH-9229}. + * + * @author Chris Cranford + */ +public class EmbeddableTest extends BaseEnversJPAFunctionalTestCase { + private Integer simpleId; + private Integer simpleOverrideId; + private Integer simplePropertyId; + private Integer fullOverrideId; + private Integer notAuditedId; + private Integer overridedId; + private Integer auditedId; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { + AuditedEmbeddable.class, + AuditedEmbeddableOverrideEntity.class, + FullOverrideEmbeddable.class, + FullOverrideEmbeddableEntity.class, + NotAuditedEmbeddableEntity.class, + OverrideEmbeddable.class, + OverrideEmbeddableEntity.class, + SimpleAbstractMappedSuperclass.class, + SimpleEmbeddable.class, + SimpleEmbeddableEntity.class, + SimpleEmbeddableWithOverrideEntity.class, + SimpleEmbeddableWithPropertyOverrideEntity.class + }; + } + + @Test + @Priority( 10 ) + public void initData() { + EntityManager entityManager = getEntityManager(); + try { + + // entity 1 + SimpleEmbeddableEntity simple = new SimpleEmbeddableEntity(); + simple.setEmbeddable( new SimpleEmbeddable() ); + simple.getEmbeddable().setStrValue( "strValueSimple" ); + simple.getEmbeddable().setIntValue( 5 ); + + // entity 2 + SimpleEmbeddableWithOverrideEntity simpleOverride = new SimpleEmbeddableWithOverrideEntity(); + simpleOverride.setEmbeddable( new SimpleEmbeddable() ); + simpleOverride.getEmbeddable().setStrValue( "strValueSimpleOverride" ); + simpleOverride.getEmbeddable().setIntValue( 10 ); + + // entity 3 + SimpleEmbeddableWithPropertyOverrideEntity simpleProperty = new SimpleEmbeddableWithPropertyOverrideEntity(); + simpleProperty.setEmbeddable( new SimpleEmbeddable() ); + simpleProperty.getEmbeddable().setStrValue( "strValueSimpleMapped" ); + simpleProperty.getEmbeddable().setIntValue( 15 ); + + // entity 4 + FullOverrideEmbeddableEntity fullOverride = new FullOverrideEmbeddableEntity(); + fullOverride.setEmbeddable( new FullOverrideEmbeddable() ); + fullOverride.getEmbeddable().setStrValue( "strValueFull" ); + fullOverride.getEmbeddable().setIntValue( 20 ); + + // entity 5 + NotAuditedEmbeddableEntity notAudited = new NotAuditedEmbeddableEntity(); + notAudited.setEmbeddable( new FullOverrideEmbeddable() ); + notAudited.getEmbeddable().setStrValue( "strValueNotAudited" ); + notAudited.getEmbeddable().setIntValue( 25 ); + + // entity 6 + OverrideEmbeddableEntity overrided = new OverrideEmbeddableEntity(); + overrided.setEmbeddable( new OverrideEmbeddable() ); + overrided.getEmbeddable().setStrValue( "strValueOver" ); + overrided.getEmbeddable().setIntValue( 30 ); + + // entity 7 + AuditedEmbeddableOverrideEntity audited = new AuditedEmbeddableOverrideEntity(); + audited.setEmbeddable( new AuditedEmbeddable() ); + audited.getEmbeddable().setStrValue( "strValueAudited" ); + audited.getEmbeddable().setIntValue( 35 ); + audited.getEmbeddable().setValue( 1024 ); + + entityManager.getTransaction().begin(); + entityManager.persist( simple ); + entityManager.persist( simpleOverride ); + entityManager.persist( simpleProperty ); + entityManager.persist( fullOverride ); + entityManager.persist( notAudited ); + entityManager.persist( overrided ); + entityManager.persist( audited ); + entityManager.getTransaction().commit(); + + this.simpleId = simple.getId(); + this.simpleOverrideId = simpleOverride.getId(); + this.simplePropertyId = simpleProperty.getId(); + this.fullOverrideId = fullOverride.getId(); + this.notAuditedId = notAudited.getId(); + this.overridedId = overrided.getId(); + this.auditedId = audited.getId(); + } + finally { + entityManager.close(); + } + } + + @Test + @TestForIssue( jiraKey = "HHH-9228" ) + public void testAuditOverrideOnAuditedEmbeddable() { + final PersistentClass clazz = getPersistentClass( AuditedEmbeddableOverrideEntity.class, auditedId, 1 ); + assertTrue( clazz.hasProperty( "name" ) ); + // verify non-audited fields are excluded from mappings. + assertFalse( clazz.hasProperty( "embeddable_value" ) ); + assertFalse( clazz.hasProperty( "embeddable_intValue" ) ); + assertFalse( clazz.hasProperty( "embeddable_strValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableWithFullAudit() { + final PersistentClass clazz = getPersistentClass( FullOverrideEmbeddableEntity.class, fullOverrideId, 1 ); + assertTrue( clazz.hasProperty( "embeddable_intValue" ) ); + assertTrue( clazz.hasProperty( "embeddable_strValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableWithNoAudited() { + final PersistentClass clazz = getPersistentClass( NotAuditedEmbeddableEntity.class, notAuditedId, 1 ); + // not mapped because @NotAudited should override any other behavior. + assertFalse( clazz.hasProperty( "embeddable_intValue" ) ); + assertFalse( clazz.hasProperty( "embeddable_strValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableWithAuditOverride() { + final PersistentClass clazz = getPersistentClass( OverrideEmbeddableEntity.class, overridedId, 1 ); + assertFalse( clazz.hasProperty( "embeddable_strValue" ) ); + assertTrue( clazz.hasProperty( "embeddable_intValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableNoAuditOverrides() { + final PersistentClass clazz = getPersistentClass( SimpleEmbeddableEntity.class, simpleId, 1 ); + assertFalse( clazz.hasProperty( "embeddable_strValue" ) ); + assertFalse( clazz.hasProperty( "embeddable_intValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableWithAuditOverrideForMappedSuperclass() { + final PersistentClass clazz = getPersistentClass( + SimpleEmbeddableWithOverrideEntity.class, + simpleOverrideId, + 1 + ); + assertTrue( clazz.hasProperty( "embeddable_strValue" ) ); + assertTrue( clazz.hasProperty( "embeddable_intValue" ) ); + } + + @Test + @TestForIssue( jiraKey = "HHH-9229" ) + public void testEmptyEmbeddableWithPropertyAuditOverride() { + final PersistentClass clazz = getPersistentClass( + SimpleEmbeddableWithPropertyOverrideEntity.class, + simplePropertyId, + 1 + ); + assertFalse( clazz.hasProperty( "embeddable_strValue" ) ); + assertTrue( clazz.hasProperty( "embeddable_intValue" ) ); + } + + // represents a @MappedSuperclass with no overrides + @MappedSuperclass + public static abstract class SimpleAbstractMappedSuperclass { + private String strValue; + private Integer intValue; + + public Integer getIntValue() { + return intValue; + } + + public void setIntValue(Integer intValue) { + this.intValue = intValue; + } + + public String getStrValue() { + return strValue; + } + + public void setStrValue(String strValue) { + this.strValue = strValue; + } + } + + // an embeddable that should introduce no audited properties + @Embeddable + public static class SimpleEmbeddable extends SimpleAbstractMappedSuperclass { + + } + + // an embeddable that should introduce 'intValue' as audited based on audit overrides locally + @Embeddable + @AuditOverride(forClass = SimpleAbstractMappedSuperclass.class, name = "intValue") + public static class OverrideEmbeddable extends SimpleAbstractMappedSuperclass { + + } + + // an embedddable that introduces all audited values base don audit overrides locally. + @Embeddable + @AuditOverride(forClass = SimpleAbstractMappedSuperclass.class) + public static class FullOverrideEmbeddable extends SimpleAbstractMappedSuperclass { + + } + + @Embeddable + @Audited + public static class AuditedEmbeddable extends SimpleAbstractMappedSuperclass { + private Integer value; + + public Integer getValue() { + return value; + } + + public void setValue(Integer value) { + this.value = value; + } + } + + @Entity + @Audited + public static class AuditedEmbeddableOverrideEntity { + @Id + @GeneratedValue + private Integer id; + private String name; + @Embedded + @AuditOverrides({ + @AuditOverride(name = "value", isAudited = false), + @AuditOverride(forClass = SimpleAbstractMappedSuperclass.class, isAudited = false) + }) + private AuditedEmbeddable embeddable; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public AuditedEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(AuditedEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + @Entity + @Audited + public static class NotAuditedEmbeddableEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + @NotAudited + private FullOverrideEmbeddable embeddable; + + public FullOverrideEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(FullOverrideEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + @Entity + @Audited + public static class FullOverrideEmbeddableEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + private FullOverrideEmbeddable embeddable; + + public FullOverrideEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(FullOverrideEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + @Entity + @Audited + public static class OverrideEmbeddableEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + private OverrideEmbeddable embeddable; + + public OverrideEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(OverrideEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + @Entity + @Audited + public static class SimpleEmbeddableWithPropertyOverrideEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + @AuditOverride(name = "intValue", forClass = SimpleAbstractMappedSuperclass.class) + private SimpleEmbeddable embeddable; + + public SimpleEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(SimpleEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + @Entity + @Audited + public static class SimpleEmbeddableEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + private SimpleEmbeddable embeddable; + + public SimpleEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(SimpleEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + + @Entity + @Audited + public static class SimpleEmbeddableWithOverrideEntity { + @Id + @GeneratedValue + private Integer id; + @Embedded + @AuditOverride(forClass = SimpleAbstractMappedSuperclass.class) + private SimpleEmbeddable embeddable; + + public SimpleEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(SimpleEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } + + private PersistentClass getPersistentClass(Class clazz, Object id, Number revision) { + final Object entity = getAuditReader().find( clazz, id, revision ); + return metadata().getEntityBinding( getAuditReader().getEntityName( id, revision, entity ) + "_AUD" ); + } +}