Envers - Proposal fix for HHH-4646 and HHH-5177 #3

hchanfreau · 2010-10-19T17:09:52Z

Hi!

It's my first pull request. I hope to be doing well.
As I mencioned on envers forum I've been working on this issuues.

Regards. Hernán.

Deleting obsolete tests

added methods isEmpty() and contains(key)

- Read audited properties from subclasses first, then check recursively on superclasses. - Chech on superclasses only if a class is audited - Calculate if the "current" class of the recursion is audited at class level. - Created ComponentAuditedPropertiesReader subclass for AuditedPropertiesReader to use for reading component audited properties inside classes - Created a protected method in AuditedPropertiesReader, letting the subclass do their job. - Ordered the code to be more understandable.

Adding tests cases: - MappedSuperclass audited at class level with a NotAuditedSubclass and: - Audited subclass at class level - Audited subclass at method/field level - MappedSuperclass audited at method/field level with a NotAuditedSubclass and: - Audited subclass at class level - Audited subclass at method/field level

adamw · 2010-10-21T05:58:50Z

I looked at the patch and I've got one question: why do the the methods addPropertiesFromClass and addPropertiesFromClassRec differ? Shouldn't properties be always read first, and then superclasses?

hchanfreau · 2010-10-21T12:20:52Z

Oh yes! You're right. The method addPropertiesFromClassRec must be:

private void addPropertiesFromClassRec(XClass clazz)  {

    Audited allClassAudited = clazz.getAnnotation(Audited.class);

    addFromProperties(clazz.getDeclaredProperties("field"), "field", fieldAccessedPersistentProperties, allClassAudited);
    addFromProperties(clazz.getDeclaredProperties("property"), "property", propertyAccessedPersistentProperties, allClassAudited);

    XClass superclazz = clazz.getSuperclass();
    if (!clazz.isInterface() && !"java.lang.Object".equals(superclazz.getName())) {
        addPropertiesFromClassRec(superclazz);
    }

}

adamw · 2010-10-21T13:46:05Z

So then the addPropertiesFromClassRec method can be removed compeltely. Trying :)

hchanfreau · 2010-10-21T14:02:25Z

Yes! sorry for that mistake.

adamw · 2010-10-21T14:23:53Z

Pushed into 3.6

adamw · 2010-10-22T12:45:46Z

Merging these changes to the master branch isn't that easy. If you've got an idea on how to do it, let me know :)

hchanfreau · 2010-10-22T17:40:46Z

What's the problem? Is there a refactor or something on sources on master?
I'll try to send another pull req for master if I can figure it out.

adamw · 2010-10-23T08:26:15Z

Yes, the modules are renamed from envers to hibernate-envers. In fact I'm looking for a generic solution on how to merge some commits from 3.6 to master. So if you find a way, please share :)

hchanfreau · 2010-10-25T15:21:51Z

Oh! I see... All modules were renamed, so it's an issue for all components.
The way master -> 3.6 would be easy or it's the same?

adamw · 2010-10-25T15:26:29Z

I think it would be the same - the problem is that git doesn't recognize the rename. I'm planning on creating a create patch - rename paths with sed - apply patch script which takes git commit hashes, but didn't have time yet ;)

…lizedAssociationTest This test accesses a field of an entity directly and expects it to be automatically initialized; this cannot work without extended bytecode enhancement. This used to work with Java 8 bytecode, but only by chance. It seems that Java 8 bytecode relies on "synthetic", static access methods inserted by the compiler to access the fields of entities in this test: any access to the field is done through this access method instead of through a direct field access. Since we apply bytecode enhancement to all methods of entities, this means that access to fields triggers initialization, without any bytecode enhancement in the caller class. I believe this is specific to nested classes, but couldn't find a source. For reference, the bytecode of access methods looks like this: static int access$002(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, int); Code: 0: aload_0 1: iload_1 2: dup_x1 3: putfield hibernate#3 // Field id:I 6: ireturn static org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId access$102(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId); Code: 0: aload_0 1: aload_1 2: dup_x1 3: putfield hibernate#2 // Field entityImmutableNaturalId:Lorg/hibernate/test/bytecode/enhancement/lazy/NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId; 6: areturn With Java 11, however, access to fields of entities is done directly, even for nested classes. So the access methods no longer exist, and we don't get automatic initialization upon field access. We need extended bytecode enhancement, like we would in any other case of field access (in particular accessing fields of non-nested classes).

…lizedAssociationTest This test accesses a field of an entity directly and expects it to be automatically initialized; this cannot work without extended bytecode enhancement. This used to work with Java 8 bytecode, but only by chance. It seems that Java 8 bytecode relies on "synthetic", static access methods inserted by the compiler to access the fields of entities in this test: any access to the field is done through this access method instead of through a direct field access. Since we apply bytecode enhancement to all methods of entities, this means that access to fields triggers initialization, without any bytecode enhancement in the caller class. I believe this is specific to nested classes, but couldn't find a source. For reference, the bytecode of access methods looks like this: static int access$002(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, int); Code: 0: aload_0 1: iload_1 2: dup_x1 3: putfield #3 // Field id:I 6: ireturn static org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId access$102(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId); Code: 0: aload_0 1: aload_1 2: dup_x1 3: putfield #2 // Field entityImmutableNaturalId:Lorg/hibernate/test/bytecode/enhancement/lazy/NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId; 6: areturn With Java 11, however, access to fields of entities is done directly, even for nested classes. So the access methods no longer exist, and we don't get automatic initialization upon field access. We need extended bytecode enhancement, like we would in any other case of field access (in particular accessing fields of non-nested classes).

…lizedAssociationTest This test accesses a field of an entity directly and expects it to be automatically initialized; this cannot work without extended bytecode enhancement. This used to work with Java 8 bytecode, but only by chance. It seems that Java 8 bytecode relies on "synthetic", static access methods inserted by the compiler to access the fields of entities in this test: any access to the field is done through this access method instead of through a direct field access. Since we apply bytecode enhancement to all methods of entities, this means that access to fields triggers initialization, without any bytecode enhancement in the caller class. I believe this is specific to nested classes, but couldn't find a source. For reference, the bytecode of access methods looks like this: static int access$002(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, int); Code: 0: aload_0 1: iload_1 2: dup_x1 3: putfield hibernate#3 // Field id:I 6: ireturn static org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId access$102(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId); Code: 0: aload_0 1: aload_1 2: dup_x1 3: putfield hibernate#2 // Field entityImmutableNaturalId:Lorg/hibernate/test/bytecode/enhancement/lazy/NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId; 6: areturn With Java 11, however, access to fields of entities is done directly, even for nested classes. So the access methods no longer exist, and we don't get automatic initialization upon field access. We need extended bytecode enhancement, like we would in any other case of field access (in particular accessing fields of non-nested classes).

…lizedAssociationTest This test accesses a field of an entity directly and expects it to be automatically initialized; this cannot work without extended bytecode enhancement. This used to work with Java 8 bytecode, but only by chance. It seems that Java 8 bytecode relies on "synthetic", static access methods inserted by the compiler to access the fields of entities in this test: any access to the field is done through this access method instead of through a direct field access. Since we apply bytecode enhancement to all methods of entities, this means that access to fields triggers initialization, without any bytecode enhancement in the caller class. I believe this is specific to nested classes, but couldn't find a source. For reference, the bytecode of access methods looks like this: static int access$002(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, int); Code: 0: aload_0 1: iload_1 2: dup_x1 3: putfield #3 // Field id:I 6: ireturn static org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId access$102(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId); Code: 0: aload_0 1: aload_1 2: dup_x1 3: putfield #2 // Field entityImmutableNaturalId:Lorg/hibernate/test/bytecode/enhancement/lazy/NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId; 6: areturn With Java 11, however, access to fields of entities is done directly, even for nested classes. So the access methods no longer exist, and we don't get automatic initialization upon field access. We need extended bytecode enhancement, like we would in any other case of field access (in particular accessing fields of non-nested classes). (cherry picked from commit 1060baf)

hchanfreau added 4 commits October 19, 2010 13:08

HHH-4646 and HHH-5177

b5dcff0

Deleting obsolete tests

Fix for HHH-4646 and HHH-5177

d05f2d8

added methods isEmpty() and contains(key)

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Envers - Proposal fix for HHH-4646 and HHH-5177 #3

Envers - Proposal fix for HHH-4646 and HHH-5177 #3

hchanfreau commented Oct 19, 2010

adamw commented Oct 21, 2010

hchanfreau commented Oct 21, 2010

adamw commented Oct 21, 2010

hchanfreau commented Oct 21, 2010

adamw commented Oct 21, 2010

adamw commented Oct 22, 2010

hchanfreau commented Oct 22, 2010

adamw commented Oct 23, 2010

hchanfreau commented Oct 25, 2010

adamw commented Oct 25, 2010

Envers - Proposal fix for HHH-4646 and HHH-5177 #3

Envers - Proposal fix for HHH-4646 and HHH-5177 #3

Conversation

hchanfreau commented Oct 19, 2010

adamw commented Oct 21, 2010

hchanfreau commented Oct 21, 2010

adamw commented Oct 21, 2010

hchanfreau commented Oct 21, 2010

adamw commented Oct 21, 2010

adamw commented Oct 22, 2010

hchanfreau commented Oct 22, 2010

adamw commented Oct 23, 2010

hchanfreau commented Oct 25, 2010

adamw commented Oct 25, 2010