Persistence hooks not working in v6.2.0-RC7

[bion]

Updating from v6.2.0-RC4 to v6.2.0-RC7 has caused a regression where methods annotated with javax.persistence.PrePersist and javax.persistence.PreUpdate are not longer called before insertion or update.

Example usage: https://github.com/seattle-uat/civiform/blob/main/server/app/models/Applicant.java#L74-L76

[mkurz]

Thanks for the report! @PromanSEW what do you think? Can you take a look? BTW: This whole -RC versioning thing is a bit of a mess. I did that because someone else who was taking care of this repo before started with RC1 or 2... Maybe it would make sense to start all over with the versioning here...

[PromanSEW]

Hmm... Maybe @rbygrave can help to find where to dig?

[PromanSEW]

@bion which version of Ebean do you use? I assume that 13.6+ I worked for adaptation for Ebean 13.6+ in RC5-RC7, so enhancing can work Also see #290

[bion]

@PromanSEW ah, we're only specifying a play-ebean version, so whatever version the plugin brings in.

[bion]

Any chance this change could be related? Just a guess since it's related to annotation processing.

[PromanSEW]

@bion I only checked 13.6.0+, this change was a fix for another change (https://github.com/playframework/play-ebean/commit/692ae03c00485d1098ef570dffe69c73ba273f22) in RC6 Final changes from RC4...RC7: https://github.com/playframework/play-ebean/compare/6.2.0-RC4...6.2.0-RC7

[PromanSEW]

Hmm... Ebean was updated from 12.8.1 to 12.16.1, maybe problem is here?

[PromanSEW]

@rbygrave could javax.persistence.PreUpdate be broken between 12.8.1 and 12.16.1?

[rbygrave]

Well maybe, in that dirty detection of JSON properties changed in 12.11.1 via: https://github.com/ebean-orm/ebean/pull/2274

We got a bug reported 17 days ago (with a workaround) and fixed 13.6.4 https://github.com/ebean-orm/ebean/issues/2710

And this looks at least similar: https://github.com/seattle-uat/civiform/blob/main/server/app/models/Applicant.java#L81 https://github.com/seattle-uat/civiform/blob/main/server/app/models/Applicant.java#L44 ... as this is also a @DbJson property mutated in a @PreUpdate so pretty likely this is the same issue.

@bion Have you created a failing test case? If so, you can run that failing test with:

1) try against Ebean version 13.6.4 2) or try explicitly marking the property as dirty/changed via:

  @PrePersist
  @PreUpdate
  public void synchronizeObject() {
    this.preferredLocale =
        getApplicantData().hasPreferredLocale()
            ? getApplicantData().preferredLocale().toLanguageTag()
            : null;
    this.object = objectAsJsonString();

   // explicitly mark the "object" property as dirty / changed
  EntityBean entityBean = (EntityBean)this; // bean
  EntityBeanIntercept ebi = entityBean._ebean_getIntercept();
  int pos = ebi.findProperty("object");
  ebi.setChanged(pos);

  }

[rbygrave]

but ...

are not longer called before insertion or update

this was not that case in that the methods were being executed, it's just that the @DbJson properties were not treated as dirty. So in this sense it would be very good @bion if you had a failing test case that we could run (because we have no failing test case where these methods don't run).

[PromanSEW]

@bion see how to update Ebean here: https://github.com/playframework/play-ebean/issues/290#issuecomment-1126729369

[bion]

Thank you for the responses! I tried

1. setting the property to dirty manually, as in the code example
2. setting the EBean version to 13.6.4 using dependencyOverrides in both plugins.sbt and build.sbt
3. both together

None of them resolved the problem, from what I can tell. I am testing solutions by running testOnly repository.UserRepositoryTest. The repository.UserRepositoryTest.updateApplicant test fails.

[mkurz]

Was this problem solved? I want to release Play Ebean 6.2.0 final soon, would be nice if all problems are sorted out before. @PromanSEW Can you please check if all dependencies in the 6.2.x branch are up to date? Thanks!

[PromanSEW]

@mkurz Ebean can be updated to 13.10.1, but before release this https://github.com/playframework/play-ebean/issues/290#issuecomment-1126729369 should be added to README how to update Ebean to latest version properly. Without overriding version in plugins.sbt you got dependency collision with different versions of ebean-agent and maybe broken model enhancing, in case of different Ebean versions for build and runtime. What about current issue - I don't know. I think it can be solved simply by updating Ebean and nothing else. Also, it's OK for using milestones of Play and SBT for releases?

Edit: I was confused with releases and thought 6.2.0 is release for main branch, not 6.2.x So I think it can be released without any edits above

[mkurz]

Ping @bion, could you solve the problem?

[mkurz]

Alright, so let's just release the final version (but will update dependencies before), I think we are good now

[mkurz]

6.2.0 released: https://github.com/playframework/play-ebean/releases/tag/6.2.0 I keep this issue open for a while in case @bion comes back one day to let us know if he still runs into issues :wink:

[bion]

Hi @mkurz, sorry I missed responding this earlier! Still seeing the same behavior as mentioned in my last comment. The hooks are running correctly on 6.2.0-RC4 but not 6.2.0.

[mkurz]

@bion I am sorry I can not not help here, I am not so much into ebean. If you or someone else can provide a fix I am happy to take a look and merge.

[gwendolyngoetz]

Hello,

CiviForm has had this in our backlog for a while. With Play 2.9 and Play EBean 7.0 around the corner I was trying to get 6.2.0 working. We're currently stuck on 6.4.0-RC4

I tried the suggestions by rbygrave again. Just like @bion that didn't work for me.

I tried the pre-release of 7.0.0-RC2 on Play 2.8 which also did not work. We're working through upgrade issues on Play 2.9 so it wasn't feasible to try there yet.

I also tried setting the Json MutationDetection to different options on the DatabaseConfig, but that too did not work.

My notes and those of other team members are documented in this issue.

[mkurz]

Not sure why this is not working, I tried but couldn't figure out the problem. I do think it has something to do with your setup and/or classloading. Using the @Pre.. annotations does work when using the play-ebean sample in the play-samples repo. I set up a branch and it works: https://github.com/mkurz/play-samples/tree/play-ebean-304

• https://github.com/mkurz/play-samples/commit/4a3cbfc8b80229082a5ea9dd144bd04aa159bbca
• Also when switching back to Play 2.8.20 and older play-ebean version: https://github.com/mkurz/play-samples/commit/ba55b6bfb0da51effb55a027e91bf3527d8ed5b7

I will try to take a look at this next week, this needs some deep debugging to figure out what's going on.

Note to myself, need to run the project like described in https://github.com/civiform/civiform/wiki/Testing#java-unit-tests

sudo bin/sbt-test
sbt
$ testOnly repository.UserRepositoryTest

[gwendolyngoetz]

Thanks @mkurz! I'll try to find time to see if I can get the sample configured in a way that will repo the issue.

[mkurz]

@gwendolyngoetz Could you isolate the issue in a standalone project?

[gwendolyngoetz]

@mkurz Sure, I'll try tomorrow

[gwendolyngoetz]

@mkurz Finally got this distilled down into three files in a copy of the 2.8 play hello world example. https://github.com/gwendolyngoetz/civiform-persistence-error/

Unless I manually call io.ebean.DB.markAsDirty(), the @PreUpdate doesn't fire in releases after 6.2.0-RC4.

Setting the project to Play 2.9.0 / Play EBean 7.0.0 result in the same error as 2.8.

[mkurz]

@gwendolyngoetz Great! I will try to take a look asap, thanks!

[mkurz]

This issue was bothering me along time and I finally can say this is not a play-ebean bug, and very likely not even a ebean bug itself. @bion @gwendolyngoetz it seems you guys were using ebean incorrectly until

• https://github.com/ebean-orm/ebean/pull/2274

got merged. So starting with ebean version 12.11.0 your code will fail.

Following is based on the example project https://github.com/gwendolyngoetz/civiform-persistence-error/ @gwendolyngoetz's comment:

I am not an ebean expert, but what I can say is that @PrePersist is called correctly. Also @PreUpdate gets called correctly if ebean thinks there is the need to have it called. I recommend for testing purposes that you use two separate methods, one for each annotation to better distinct when and if a method gets fired.

So, for me it seems what you are expecting is that @PreUpdate gets called as soon as you call something like database.update(applicantNew). However, this is only true when a mutable member of the entity you want to update actually changed (meaning such a mutable member is "dirty"). If ebean does not detect a mutable member as dirty, it thinks nothing needs to be updated and also will not fire @PreUpdate. However, you use @PreUpdate to update the object variable so its new value gets written to the database. However, starting with 12.11.0 @PreUpdate will not even get fired anymore (because nothing is dirty in your entity when you call update) so there is no chance that you can set a value to object.

What changed with #2274 above is, that a variable of String, which is annotated with @DbJson (or @DbJsonB), will now be treated as immutable. Basically ebean says now that when a postgres JSON or JSONB is mapped to a String it is immutable and can not be changed, therefore it does not care about it when calling ebean's update method when checking mutable elements (ebean maintains an internal list of mutable properties and does not add postgres jsonb anymore).

Also interesting: see the wording in the DbJson docs:

If the mapping is to String, Long or Map<String,Object> types then Ebean will use it's built in JSON support to handle the marshalling to/from JSON.

Following line of the pull request is relevevant here:

• https://github.com/ebean-orm/ebean/pull/2274/files#diff-b238b681c2db78a92345995cbfdec16c29c1562f51b35f9f0303cf1dd907c104R221 (moves prop.setDbType(dbType) before typeManager.getJsonScalarType(...) gets called, so that method is now aware it deals with a postgres json(b) type and therefore ebean uses it's internal postgres json(b) mapper, which treats values as immutable, you can see that it does not override the mutable method which defaults to false).

If the getJsonScalarType can not determine a internal json mapper instance (which it couldn't before #2274, because it didn't know with which db engine it was dealing with), it falls back to a generic json object mapper which actually is marked as mutable (!), so that's why your code was working until now: ebean treated the object member of your entity as mutable and therefore did check if it was dirty and eventually started the update process because it thought the entities needs to be updated in the database.

So, my conclusing is, ebean 12.11.0 improved it's behaviour (?) or better: fixed it? Not sure if it is 100% correct to assume a postgres JSON(B) is immutable or not, but this is out of scope for this issue.

What I can say for 100% is, that this is not a bug in play-ebean, but a result of a change in behaviour of ebean itself. Therefore you 1) either have to reach out to the ebean community (https://ebean.io/support) to find out if what you are doing is assumed to be correct and ebean has to change here or 2) use another approach of how you design your data layer as it seems working directly with JSON(B) mapped to a String and relying on (probably) wrong assumptions when @PreUpdate is fired does obviously not work anymore.

For me I can safely close this issue and you probably need to get help from some real ebean expert :wink:

[gwendolyngoetz]

Thank you so much @mkurz for looking in to this. Much appreciated. I'll check with Bion if there was a historical reason it was done this way and we'll look into some way to fix it.

[bion]

@mkurz than you very much for the detailed response to this bug, very much appreciated!