Skip to content

Conversation

@codeconsole
Copy link
Contributor

@codeconsole codeconsole commented Oct 14, 2025

With mongodb, anything done during a PreUpdateEvent is lost due to not being marked dirty.

This fix wraps FieldEntityAccess with ModificationTrackingEntityAccess then marks the modified properties as dirty.
Hibernate does the same thing, but it is still not necessary for hibernate to do so in order for properties to be marked dirty.

# clone grails-core with the fix and pTML
git clone --depth 1 --branch 7.0.x-mongo-pre-update-dirty-fix https://github.com/codeconsole/grails-core grails-core-mongo-pre-update-dirty-fix && cd grails-core-mongo-pre-update-dirty-fix && ./gradlew pTML

# Start mongo  and hibernate versions with the fix
git clone --depth 1 --branch mongodb-mavenLocal-7.0.0-SNAPSHOT https://github.com/codeconsole/mongo-test mongo-test-mongo-SNAPSHOT && cd mongo-test-mongo-SNAPSHOT && ./gradlew bootRun --args='--server.port=8081'

git clone --depth 1 --branch h2-mavenLocal-7.0.0-SNAPSHOT https://github.com/codeconsole/mongo-test mongo-test-hibernate-SNAPSHOT && cd mongo-test-hibernate-SNAPSHOT && ./gradlew bootRun --args='--server.port=8082'

Verify fix:
http://localhost:8081/user/create
http://localhost:8082/user/create

@github-actions github-actions bot added the bug label Oct 14, 2025
@codeconsole codeconsole changed the title Fix for properties during PreUpdateEvents not being marked dirty. 7.0.0-RC2 BUG FIX Fix for properties during PreUpdateEvents not being marked dirty. Oct 14, 2025
@codeconsole codeconsole changed the title 7.0.0-RC2 BUG FIX Fix for properties during PreUpdateEvents not being marked dirty. 7.0.0-RC2 BUG FIX - for properties during PreUpdateEvents not being marked dirty. Oct 14, 2025
@codeconsole codeconsole changed the title 7.0.0-RC2 BUG FIX - for properties during PreUpdateEvents not being marked dirty. 7.0.0-RC2 BUG FIX - for properties during PreUpdateEvents not being marked dirty when using mongodb Oct 14, 2025
@codeconsole codeconsole requested a review from matrei October 14, 2025 02:39
@matrei
Copy link
Contributor

matrei commented Oct 14, 2025

I think this is a possible candidate for 7.0.1.
We need to ship 7.0.0 to lay some stable ground so we can continue moving from there.

@codeconsole
Copy link
Contributor Author

@matrei but that is the reason behind this PR. Mongo is broken. This PR only affects mongo. Should 7.0.0 really go out broken?

@matrei
Copy link
Contributor

matrei commented Oct 14, 2025

@matrei but that is the reason behind this PR. Mongo is broken. This PR only affects mongo. Should 7.0.0 really go out broken?

Yes, I'm afraid that would be my suggestion. I don't think we can let any more time go by. I think we need to ship 7 now.
It's almost a year since I wrote this comment apache/grails-data-mapping#1855 (comment), and my gut feeling seems to have been right at that time.

@codeconsole codeconsole force-pushed the 7.0.x-mongo-pre-update-dirty-fix branch from 2d40bc0 to e718216 Compare October 14, 2025 16:19
@codeconsole codeconsole marked this pull request as draft October 14, 2025 16:29
@jdaugherty
Copy link
Contributor

Since this is affecting mongo only now, I do not have a concern with making this change if the tests pass. I'm ok with it also going in 7.0.1 since it's limited in scope / wont' affect hibernate.

@codeconsole
Copy link
Contributor Author

Since this is affecting mongo only now, I do not have a concern with making this change if the tests pass. I'm ok with it also going in 7.0.1 since it's limited in scope / wont' affect hibernate.

@jdaugherty This will not fix beforeUpdate() {}. This will fix EventListeners.

@codeconsole
Copy link
Contributor Author

Yes, I'm afraid that would be my suggestion. I don't think we can let any more time go by. I think we need to ship 7 now. It's almost a year since I wrote this comment apache/grails-data-mapping#1855 (comment), and my gut feeling seems to have been right at that time.

@matrei what does your previous comment have to do with mongo being broken? There is no issue with the @AutoTimestamp code. It has worked fine with hibernate. The issue is due to a specific bug with mongo.

This PR has nothing to do with @AutoTimestamp.

@matrei
Copy link
Contributor

matrei commented Oct 14, 2025

what does your previous comment have to do with mongo being broken? There is no issue with the @AutoTimestamp code. It has worked fine with hibernate. The issue is due to a specific bug with mongo.

Sorry, my mistake. I may have misunderstood or mixed up your PRs.
Thank you for creating tests, it saves so much time understanding the bug and reviewing the bug fix.

@jdaugherty
Copy link
Contributor

@codeconsole I believe this can be closed now that the other PR was merged?

@codeconsole
Copy link
Contributor Author

no longer needed due to #15143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants