@DiffIgnore Annotation on Subclass Fields Sometimes Ignored

[clarkhan]

Clear description of my expectations versus reality
The @DiffIgnore annotation on fields within subclasses seems to be ignored in certain scenarios. For instance, when retrieving the TypeMapping of a superclass before accessing the TypeMapping of a subclass, the ignored fields in the subclass are not treated properly.

Steps To Reproduce
I have a **runnable test case ** which isolates the bug and allows Javers Core Team to easily reproduce it. I have pushed this test case to my fork of this repository:

https://github.com/clarkhan/javers/blob/master/javers-core/src/test/groovy/org/javers/core/cases/CaseInheritanceWithIgnoredField.groovy

package org.javers.core.cases
import jakarta.persistence.Id
import org.javers.core.JaversBuilder
import org.javers.core.metamodel.annotation.DiffIgnore
import org.javers.core.metamodel.annotation.TypeName
import org.javers.core.metamodel.type.EntityType
import spock.lang.Specification

class CaseInheritanceWithIgnoredField extends Specification {
    @TypeName("A")
    class A {
        @Id
        String id
        String field
    }

    @TypeName("B")
    class B extends A {
        @DiffIgnore
        String someOtherField
    }

    @TypeName("C")
    class C extends A {
        @DiffIgnore
        String someOtherField
    }

    def "should ignore fields on sub Class that are annotated with DiffIgnore"(){
        given:
        def javers = JaversBuilder.javers().build()

        when:
        EntityType bType = javers.getTypeMapping(B)
        EntityType aType = javers.getTypeMapping(A)
        EntityType cType = javers.getTypeMapping(C)

        then:
        assert bType.findProperty("someOtherField").isEmpty() // passed
        assert cType.findProperty("someOtherField").isEmpty() // failed
    }
}

result:

EntityType{
  baseType: class org.javers.core.cases.CaseInheritanceWithIgnoredField$A
  typeName: A
  managedProperties:
    Field id String, javersType: ValueType, declared in: A
    Field field String, javersType: ValueType, declared in: A
  idProperties: [id]
}
EntityType{
  baseType: class org.javers.core.cases.CaseInheritanceWithIgnoredField$B
  typeName: B
  managedProperties:
    Field id String, javersType: ValueType, declared in: A
    Field field String, javersType: ValueType, declared in: A
  idProperties: [id]
}
EntityType{
  baseType: class org.javers.core.cases.CaseInheritanceWithIgnoredField$C
  typeName: C
  managedProperties:
    Field someOtherField String, javersType: ValueType, declared in: C
    Field id String, javersType: ValueType, declared in: A
    Field field String, javersType: ValueType, declared in: A
  idProperties: [id]
}


Condition not satisfied:

cType.findProperty("someOtherField").isEmpty()
|     |                              |
|     |                              false
|     Optional[Field someOtherField String, javersType: ValueType, declared in: C]
EntityType{ baseType: 'class org.javers.core.cases.CaseInheritanceWithIgnoredField$C', id: '['id']' }

Javers' Version
7.4.3

Additional context
Upon further examination, I suspect that the underlying issue may stem from the behavior of the ManagedClassFactory#createFromPrototype method within the org.javers.core.metamodel.type package. It appears that when creating the ManagedClass, the method directly utilizes the ManagedPropertiesFilter object of the prototype, inadvertently neglecting the filtering options for properties in the subclass.

This could potentially affect not only the handling of the @DiffIgnore annotation but also the treatment of includedProperties and shallowProperties?
...

[bartoszwalacik]

Hi @clarkhan , thanks for the high quality bug report. I'm triaging this issue as bug, you are encouraged to contribute a PR with the fix. If you decide to contribute, please let me know if you need any support from me.

[clarkhan]

hi, @bartoszwalacik, tks for considering my involvement in addressing this issue.
I've taken the initiative to create a Pull Request (#1387) in an attempt to address the issue we discussed.

Although the changes have successfully passed my local test cases, I'm still cautious about the implications of applying the prototype's PropertiesFilter directly to its subclasses.

As a relative newcomer to the project, I lack the broader perspective that comes with extensive experience within this codebase. Therefore, I humbly request your review of the submitted changes.

[bartoszwalacik]

Hi @clarkhan , thanks for your beautiful contribution, I did some changes in your code to make it more javers way.
See https://github.com/javers/javers/pull/1388/commits, I will merge it in this week

[clarkhan]

Hi @clarkhan , thanks for your beautiful contribution, I did some changes in your code to make it more javers way. See https://github.com/javers/javers/pull/1388/commits, I will merge it in this week

Thank you for the update and for refining the PR to align better with the Javers way – it's greatly appreciated! I've reviewed the changes, and I'm learning a lot from this process.

I'm thrilled to see my contribution being merged and to be a part of improving Javers.

Thanks again for the guidance and for welcoming my contribution to the project.

[bartoszwalacik]

merged #1388