-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Persistence hooks not working in v6.2.0-RC7 #304
Comments
Thanks for the report! @PromanSEW what do you think? Can you take a look? |
Hmm... Maybe @rbygrave can help to find where to dig? |
@PromanSEW ah, we're only specifying a play-ebean version, so whatever version the plugin brings in. |
Any chance this change could be related? Just a guess since it's related to annotation processing. |
@bion I only checked 13.6.0+, this change was a fix for another change (692ae03) in RC6 |
Hmm... Ebean was updated from 12.8.1 to 12.16.1, maybe problem is here? |
@rbygrave could |
Well maybe, in that dirty detection of JSON properties changed in 12.11.1 via: We got a bug reported 17 days ago (with a workaround) and fixed 13.6.4 And this looks at least similar: @bion Have you created a failing test case? If so, you can run that failing test with:
@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);
} |
but ...
this was not that case in that the methods were being executed, it's just that the |
@bion see how to update Ebean here: |
Thank you for the responses! I tried
None of them resolved the problem, from what I can tell. I am testing solutions by running |
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. |
@mkurz Ebean can be updated to 13.10.1, but before release this #290 (comment) should be added to README how to update Ebean to latest version properly. Without overriding version in Edit: I was confused with releases and thought |
Ping @bion, could you solve the problem? |
Alright, so let's just release the final version (but will update dependencies before), I think we are good now |
6.2.0 released: https:/playframework/play-ebean/releases/tag/6.2.0 |
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 |
@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. |
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 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. |
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.
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:/civiform/civiform/wiki/Testing#java-unit-tests sudo bin/sbt-test
sbt
$ testOnly repository.UserRepositoryTest |
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. |
@gwendolyngoetz Could you isolate the issue in a standalone project? |
@mkurz Sure, I'll try tomorrow |
@mkurz Finally got this distilled down into three files in a copy of the 2.8 play hello world example. https:/gwendolyngoetz/civiform-persistence-error/ Unless I manually call Setting the project to Play 2.9.0 / Play EBean 7.0.0 result in the same error as 2.8. |
@gwendolyngoetz Great! I will try to take a look asap, thanks! |
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. got merged. So starting with ebean version Following is based on the example project https:/gwendolyngoetz/civiform-persistence-error/ @gwendolyngoetz's comment: I am not an ebean expert, but what I can say is that So, for me it seems what you are expecting is that What changed with #2274 above is, that a variable of Also interesting: see the wording in the
Following line of the pull request is relevevant here:
If the So, my conclusing is, ebean 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.
For me I can safely close this issue and you probably need to get help from some real ebean expert 😉 |
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. |
@mkurz than you very much for the detailed response to this bug, very much appreciated! |
Updating from v6.2.0-RC4 to v6.2.0-RC7 has caused a regression where methods annotated with
javax.persistence.PrePersist
andjavax.persistence.PreUpdate
are not longer called before insertion or update.Example usage: https:/seattle-uat/civiform/blob/main/server/app/models/Applicant.java#L74-L76
The text was updated successfully, but these errors were encountered: