Author: adamw Date: 2009-12-15 07:52:20 -0500 (Tue, 15 Dec 2009) New Revision: 18226 Modified: core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/ToOneRelationMetadataGenerator.java core/trunk/envers/src/main/java/org/hibernate/envers/entities/EntityConfiguration.java core/trunk/envers/src/main/java/org/hibernate/envers/entities/RelationDescription.java core/trunk/envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java core/trunk/envers/src/main/java/org/hibernate/envers/event/AuditEventListener.java core/trunk/envers/src/test/java/org/hibernate/envers/test/integration/onetomany/detached/JoinColumnBidirectionalList.java Log: HHH-4694: - if a field is non-insertable in a "fake" bidirectional relation, not storing modifications made on the non-insertable side - also, not generating bidirectional collection changes in such case - updating test Modified: core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -224,7 +224,7 @@ // The mapper will only be used to map from entity to map, so no need to provide other details // when constructing the PropertyData. new PropertyData(auditMappedBy, null, null, null), - referencedEntityName); + referencedEntityName, false); } else { fakeBidirectionalRelationMapper = null; } Modified: core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/ToOneRelationMetadataGenerator.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/ToOneRelationMetadataGenerator.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/configuration/metadata/ToOneRelationMetadataGenerator.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -70,7 +70,7 @@ // Storing information about this relation mainGenerator.getEntitiesConfigurations().get(entityName).addToOneRelation( - propertyAuditingData.getName(), referencedEntityName, relMapper); + propertyAuditingData.getName(), referencedEntityName, relMapper, insertable); // If the property isn't insertable, checking if this is not a "fake" bidirectional many-to-one relationship, // that is, when the one side owns the relation (and is a collection), and the many side is non insertable. @@ -79,6 +79,7 @@ // the entity that didn't involve the relation, it's value will then be stored properly. In case of changes // to the entity that did involve the relation, it's the responsibility of the collection side to store the // proper data. + boolean nonInsertableFake = false; if (!insertable) { ClassAuditingData referencedAuditingData = mainGenerator.getClassesAuditingData().getClassAuditingData(referencedEntityName); @@ -91,6 +92,7 @@ referencedEntityName + " entity."); insertable = true; + nonInsertableFake = true; break; } } @@ -106,7 +108,7 @@ // Adding mapper for the id PropertyData propertyData = propertyAuditingData.getPropertyData(); - mapper.addComposite(propertyData, new ToOneIdMapper(relMapper, propertyData, referencedEntityName)); + mapper.addComposite(propertyData, new ToOneIdMapper(relMapper, propertyData, referencedEntityName, nonInsertableFake)); } @SuppressWarnings({"unchecked"}) Modified: core/trunk/envers/src/main/java/org/hibernate/envers/entities/EntityConfiguration.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/entities/EntityConfiguration.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/entities/EntityConfiguration.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -51,31 +51,31 @@ this.relations = new HashMap<String, RelationDescription>(); } - public void addToOneRelation(String fromPropertyName, String toEntityName, IdMapper idMapper) { + public void addToOneRelation(String fromPropertyName, String toEntityName, IdMapper idMapper, boolean insertable) { relations.put(fromPropertyName, new RelationDescription(fromPropertyName, RelationType.TO_ONE, - toEntityName, null, idMapper, null)); + toEntityName, null, idMapper, null, insertable)); } public void addToOneNotOwningRelation(String fromPropertyName, String mappedByPropertyName, String toEntityName, IdMapper idMapper) { relations.put(fromPropertyName, new RelationDescription(fromPropertyName, RelationType.TO_ONE_NOT_OWNING, - toEntityName, mappedByPropertyName, idMapper, null)); + toEntityName, mappedByPropertyName, idMapper, null, true)); } public void addToManyNotOwningRelation(String fromPropertyName, String mappedByPropertyName, String toEntityName, IdMapper idMapper, PropertyMapper fakeBidirectionalRelationMapper) { relations.put(fromPropertyName, new RelationDescription(fromPropertyName, RelationType.TO_MANY_NOT_OWNING, - toEntityName, mappedByPropertyName, idMapper, fakeBidirectionalRelationMapper)); + toEntityName, mappedByPropertyName, idMapper, fakeBidirectionalRelationMapper, true)); } public void addToManyMiddleRelation(String fromPropertyName, String toEntityName) { relations.put(fromPropertyName, new RelationDescription(fromPropertyName, RelationType.TO_MANY_MIDDLE, - toEntityName, null, null, null)); + toEntityName, null, null, null, true)); } public void addToManyMiddleNotOwningRelation(String fromPropertyName, String mappedByPropertyName, String toEntityName) { relations.put(fromPropertyName, new RelationDescription(fromPropertyName, RelationType.TO_MANY_MIDDLE_NOT_OWNING, - toEntityName, mappedByPropertyName, null, null)); + toEntityName, mappedByPropertyName, null, null, true)); } public boolean isRelation(String propertyName) { Modified: core/trunk/envers/src/main/java/org/hibernate/envers/entities/RelationDescription.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/entities/RelationDescription.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/entities/RelationDescription.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -36,17 +36,19 @@ private final String mappedByPropertyName; private final IdMapper idMapper; private final PropertyMapper fakeBidirectionalRelationMapper; + private final boolean insertable; private boolean bidirectional; public RelationDescription(String fromPropertyName, RelationType relationType, String toEntityName, String mappedByPropertyName, IdMapper idMapper, - PropertyMapper fakeBidirectionalRelationMapper) { + PropertyMapper fakeBidirectionalRelationMapper, boolean insertable) { this.fromPropertyName = fromPropertyName; this.relationType = relationType; this.toEntityName = toEntityName; this.mappedByPropertyName = mappedByPropertyName; this.idMapper = idMapper; this.fakeBidirectionalRelationMapper = fakeBidirectionalRelationMapper; + this.insertable = insertable; this.bidirectional = false; } @@ -75,6 +77,10 @@ return fakeBidirectionalRelationMapper; } + public boolean isInsertable() { + return insertable; + } + public boolean isBidirectional() { return bidirectional; } Modified: core/trunk/envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -49,20 +49,26 @@ private final IdMapper delegate; private final PropertyData propertyData; private final String referencedEntityName; + private final boolean nonInsertableFake; - public ToOneIdMapper(IdMapper delegate, PropertyData propertyData, String referencedEntityName) { + public ToOneIdMapper(IdMapper delegate, PropertyData propertyData, String referencedEntityName, boolean nonInsertableFake) { this.delegate = delegate; this.propertyData = propertyData; this.referencedEntityName = referencedEntityName; + this.nonInsertableFake = nonInsertableFake; } public boolean mapToMapFromEntity(SessionImplementor session, Map<String, Object> data, Object newObj, Object oldObj) { HashMap<String, Object> newData = new HashMap<String, Object>(); data.put(propertyData.getName(), newData); - delegate.mapToMapFromEntity(newData, newObj); + // If this property is originally non-insertable, but made insertable because it is in a many-to-one "fake" + // bi-directional relation, we always store the "old", unchaged data, to prevent storing changes made + // to this field. It is the responsibility of the collection to properly update it if it really changed. + delegate.mapToMapFromEntity(newData, nonInsertableFake ? oldObj : newObj); - return !Tools.entitiesEqual(session, newObj, oldObj); + //noinspection SimplifiableConditionalExpression + return nonInsertableFake ? false : !Tools.entitiesEqual(session, newObj, oldObj); } public void mapToEntityFromMap(AuditConfiguration verCfg, Object obj, Map data, Object primaryKey, Modified: core/trunk/envers/src/main/java/org/hibernate/envers/event/AuditEventListener.java =================================================================== --- core/trunk/envers/src/main/java/org/hibernate/envers/event/AuditEventListener.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/main/java/org/hibernate/envers/event/AuditEventListener.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -84,7 +84,8 @@ for (int i=0; i<propertyNames.length; i++) { String propertyName = propertyNames[i]; RelationDescription relDesc = verCfg.getEntCfg().getRelationDescription(entityName, propertyName); - if (relDesc != null && relDesc.isBidirectional() && relDesc.getRelationType() == RelationType.TO_ONE) { + if (relDesc != null && relDesc.isBidirectional() && relDesc.getRelationType() == RelationType.TO_ONE && + relDesc.isInsertable()) { // Checking for changes Object oldValue = oldState == null ? null : oldState[i]; Object newValue = newState == null ? null : newState[i]; @@ -144,11 +145,14 @@ if (verCfg.getEntCfg().isVersioned(entityName)) { AuditSync verSync = verCfg.getSyncManager().get(event.getSession()); - verSync.addWorkUnit(new AddWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, - event.getId(), event.getPersister(), event.getState())); + AuditWorkUnit workUnit = new AddWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, + event.getId(), event.getPersister(), event.getState()); + verSync.addWorkUnit(workUnit); - generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, event.getState(), - null, event.getSession()); + if (workUnit.containsWork()) { + generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, event.getState(), + null, event.getSession()); + } } } @@ -158,11 +162,14 @@ if (verCfg.getEntCfg().isVersioned(entityName)) { AuditSync verSync = verCfg.getSyncManager().get(event.getSession()); - verSync.addWorkUnit(new ModWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, - event.getId(), event.getPersister(), event.getState(), event.getOldState())); + AuditWorkUnit workUnit = new ModWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, + event.getId(), event.getPersister(), event.getState(), event.getOldState()); + verSync.addWorkUnit(workUnit); - generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, event.getState(), - event.getOldState(), event.getSession()); + if (workUnit.containsWork()) { + generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, event.getState(), + event.getOldState(), event.getSession()); + } } } @@ -172,11 +179,14 @@ if (verCfg.getEntCfg().isVersioned(entityName)) { AuditSync verSync = verCfg.getSyncManager().get(event.getSession()); - verSync.addWorkUnit(new DelWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, - event.getId(), event.getPersister(), event.getDeletedState())); + AuditWorkUnit workUnit = new DelWorkUnit(event.getSession(), event.getPersister().getEntityName(), verCfg, + event.getId(), event.getPersister(), event.getDeletedState()); + verSync.addWorkUnit(workUnit); - generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, null, - event.getDeletedState(), event.getSession()); + if (workUnit.containsWork()) { + generateBidirectionalCollectionChangeWorkUnits(verSync, event.getPersister(), entityName, null, + event.getDeletedState(), event.getSession()); + } } } Modified: core/trunk/envers/src/test/java/org/hibernate/envers/test/integration/onetomany/detached/JoinColumnBidirectionalList.java =================================================================== --- core/trunk/envers/src/test/java/org/hibernate/envers/test/integration/onetomany/detached/JoinColumnBidirectionalList.java 2009-12-15 12:27:01 UTC (rev 18225) +++ core/trunk/envers/src/test/java/org/hibernate/envers/test/integration/onetomany/detached/JoinColumnBidirectionalList.java 2009-12-15 12:52:20 UTC (rev 18226) @@ -86,12 +86,27 @@ em.getTransaction().commit(); em.clear(); + // No revision - no changes + em.getTransaction().begin(); + + ing1 = em.find(ListJoinColumnBidirectionalRefIngEntity.class, ing1.getId()); + ing2 = em.find(ListJoinColumnBidirectionalRefIngEntity.class, ing2.getId()); + ed1 = em.find(ListJoinColumnBidirectionalRefEdEntity.class, ed1.getId()); + ed2 = em.find(ListJoinColumnBidirectionalRefEdEntity.class, ed2.getId()); + + ed2.setOwner(ing2); + + em.getTransaction().commit(); + em.clear(); + // Revision 3 (ing1: ed1, ed2) em.getTransaction().begin(); ed1 = em.find(ListJoinColumnBidirectionalRefEdEntity.class, ed1.getId()); ed1.setData("ed1 bis"); + // Shouldn't get written + ed1.setOwner(ing2); em.getTransaction().commit(); em.clear();