Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected dirty checking behavior on collections of POJOs mapped with JsonBinaryType #138

Closed
mojgh opened this issue Oct 7, 2019 · 16 comments

Comments

@mojgh
Copy link

mojgh commented Oct 7, 2019

Description

When reading entities with collections of POJOs such as TestEntity (see Example 1), updates are performed on the database without the objects being changed.

Example 1

@TypeDefs({
        @TypeDef(name = "jsonb", typeClass = JsonBinaryType.class)
})
@Entity
public class TestEntity {

    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Id
    public int id;

    @Type(type = "jsonb")
    public Collection<TestElement> testElements;


    public static class TestElement implements Serializable {

        static final long serialVersionUID = -8485895696327472738L;

        public String testProperty;

    }

}

Analysis

It appears as if the dirty check on JsonBinaryType properties for collections of POJOs does no longer work as expected.
The problem occurs since [commit-1]. This commit introduces a new dirty checking behavior for JsonBinaryType, when used with collections:

public boolean areEqual(Object one, Object another) {
// ...
	if (one instanceof Collection && another instanceof Collection) {
		return Objects.equals(one, another);
	}

JsonTypeDescriptor

I would not expect the usage of java.lang.Object#equals at this point. In my understanding it creates an inconsistency compared to the dirty checking behavior used for non-collection types.
The recursive field by field comparison before [commit-1] is appropriate in my opinion.

Workaround

Downgrade to the version before [commit-1]: version 2.4.2.

Links

commit-1

@vladmihalcea
Copy link
Owner

vladmihalcea commented Oct 7, 2019

Thanks for the analysis. It would be great if you proposed a Pull Request as well. I'll review it once it's ready.

Also, you need to add a replicating test case that proves the issue as well.

@mojgh
Copy link
Author

mojgh commented Oct 8, 2019

A small project to prove the problem: hibernate_types_138.zip

@vladmihalcea
Copy link
Owner

I think it's better to fork the project and create a replicating test case from an existing test case. This way, the test can prevent the issue from occurring in future as well.

@vladmihalcea
Copy link
Owner

This commit proves that everything works as expected. The only case where I found the issue that you are describing is if you are using a Collection of Objects that don't implement equals or hashCode based on the inner properties, so the default java.lang.Object implementations are used.

@cosminseceleanu
Copy link
Contributor

@vladmihalcea after 4 days of debugging and a lot of logs(including a hot fix) I found that my issue was caused by a similar case.

The only case where I found the issue that you are describing is if you are using a Collection of Objects that don't implement equals or hashCode

Is this documented somewhere? Thanks in advance

@vladmihalcea
Copy link
Owner

@cosminseceleanu I think it's useful to add it to the README.md doc. Send me a Pull Request with the change. Thanks.

@cosminseceleanu
Copy link
Contributor

@vladmihalcea I updated the docs. Please review.

Off topic: I think this project is missing a contribution guide 😃

@vladmihalcea
Copy link
Owner

Thanks. I'll review it when I have some time. It would be great if there were a Contribution Guide too. You can supply one if you like.

@62mkv
Copy link

62mkv commented Nov 26, 2021

another half a day spent on hair-pulling due to this issue.. :)

I decided to bump many things at once - my bad. eventually settled down on this problem. Works fine with com.vladmihalcea:hibernate-types-52:2.9.0, dirty UPDATES manifest themselves with 2.10.0.

However, I still can't understand what is the root cause, will have to check further. Right now it seems that all of the following:

  • entity itself
  • POJO that is wrapped in JSON
  • and types used in collections of this POJO,
    all of it has equals and hashCode implementation (entity - the one suggested by JPABuddy IntelliJ plugin, the other two - implemented with Lombok)

And no warnings are given in 2.10.1 version either, I guess because validatePropertyType only checks if the annotated field type is a Collection (which it isn't, in my case) .

I'll see if I can submit a reproducible test project soon.

@vladmihalcea
Copy link
Owner

Usually, this is caused by bad equals and hashCode implementation in the POJO object that maps the JSON column.

Most likely you added equals and hashCode at the entity level only, not for the POJO attributes. This is a standard requirement by the JPA spec too.

@62mkv
Copy link

62mkv commented Nov 26, 2021

to clarify - by "entity" we mean JPA Entity, by POJO - the type, that is persisted as JSON column, right ?

So, in this case:

@Entity
@Table(name = "RA_OUTBOX_RECORDS")
@Getter @Setter @ToString
@TypeDef(
        name = "jsonb",
        typeClass = JsonBlobType.class
)
public class DocumentOutboxRecord {
///
    @Column(name = "PAYLOAD")
    @Type(type = "jsonb")
    DocumentPayloadV10 payload;
///

DocumentOutboxRecord would be an "entity" and DocumentPayloadV10 - a POJO?

and by "implementing equals/hashCode for POJO attributes" we mean that all custom (non-JDK) types, used inside the POJO type, and their attributes in turn, all should have equals/hashCode implemented (i.e. not inherited from Object) ?

well, if that is correct, then "POJO" and the only custom type used in the POJO definition, they both have equals and hashCode implementations, generated with Lombok.

And by the way, I have narrowed down the problem starting point: it was in 2.9.3 version of "hibernate-types-52" artifact. So, with 2.9.0/2.9.1/2.9.2 all is fine (just as with 2.5.2, for example), and with 2.9.3. and all subsequent versions - all goes bust.. Version number being the only difference

@vladmihalcea
Copy link
Owner

to clarify - by "entity" we mean JPA Entity, by POJO - the type, that is persisted as JSON column, right ?

yes

DocumentOutboxRecord would be an "entity" and DocumentPayloadV10 - a POJO?

yes

and by "implementing equals/hashCode for POJO attributes" we mean that all custom (non-JDK) types, used inside the POJO type, and their attributes in turn, all should have equals/hashCode implemented (i.e. not inherited from Object) ?

It depends on the structure of the DocumentPayloadV10 object. The equals and hashCode should determine whether that object state has changed or not.

And by the way, I have narrowed down the problem starting point: it was in 2.9.3 version of "hibernate-types-52" artifact. So, with 2.9.0/2.9.1/2.9.2 all is fine (just as with 2.5.2, for example), and with 2.9.3. and all subsequent versions - all goes bust.. Version number being the only difference

You can track the change log to see what caused the issue. At the moment, I'm very busy with other project and won't have the time to investigate issues or do any development for this framework. But, as soon as I have more time (Jan-March 2022) I'll take a look at your fndings.

@62mkv
Copy link

62mkv commented Nov 26, 2021

Sure, Vlad, this is not to somehow grab your attention or whatever, just for some other future poor soul who might end up in this issue (and most likely myself!)

I'll check the diff, thanks!

@vladmihalcea
Copy link
Owner

There's this section in the README file that should prevent people from bumping into this issue.

Looking forward to checking your analysis.

@62mkv
Copy link

62mkv commented Dec 1, 2021

(I've posted a big comment but now it seems that the problem was with something else.. I will investigate further)

EDIT: I've added a @Data annotation on one of the POJO custom attribute types, and a @Jacksonized to another, and now it seems to work fine with 2.9.3 as well! 🤷 So it looks like in previous iteration it was not even serializing them properly in the first place. oh my...

@vladmihalcea
Copy link
Owner

I don't know what the @Data or @Jacksonized annotations are. I'm glad you found a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants