You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2025/10/08 18:08:13 UTC
[PR] 7.0.0-RC2 - @Autotimestamp Persistence Bug Fix [grails-core]
codeconsole opened a new pull request, #15122:
URL: https://github.com/apache/grails-core/pull/15122
This is the isolated part of PR #15118 that fixes #15120
All it does is mark properties annotated with @AutoTimestamp dirty so they are guaranteed to be saved during updates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2426907485
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java:
##########
@@ -148,10 +150,25 @@ private void initializeIfNecessary(PersistentEntity entity, String name) {
public boolean beforeUpdate(PersistentEntity entity, EntityAccess ea) {
Set<String> props = getLastUpdatedPropertyNames(entity.getName());
if (props != null) {
+ Object entityObject = ea.getEntity();
+ boolean isDirtyCheckable = entityObject instanceof DirtyCheckable;
Review Comment:
instanceof is incompatible with Hibernate Proxies
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2426911227
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java:
##########
@@ -148,10 +150,25 @@ private void initializeIfNecessary(PersistentEntity entity, String name) {
public boolean beforeUpdate(PersistentEntity entity, EntityAccess ea) {
Set<String> props = getLastUpdatedPropertyNames(entity.getName());
if (props != null) {
+ Object entityObject = ea.getEntity();
+ boolean isDirtyCheckable = entityObject instanceof DirtyCheckable;
+
+ // For dirty-checking datastores (e.g., MongoDB), only set autotimestamp if entity has dirty properties
+ if (isDirtyCheckable) {
+ List<String> dirtyPropertyNames = ((DirtyCheckable) entityObject).listDirtyPropertyNames();
+ if (dirtyPropertyNames.isEmpty()) {
+ return true;
+ }
+ }
+
for (String prop : props) {
Class<?> lastUpdateType = ea.getPropertyType(prop);
Object timestamp = timestampProvider.createTimestamp(lastUpdateType);
ea.setProperty(prop, timestamp);
+ // Mark property as dirty for datastores that use dirty checking (e.g., MongoDB)
Review Comment:
I understand this is a mongo specific, but hibernate does have dirty checking and works. What makes hibernate different here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2427526344
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/codecs/PersistentEntityCodec.groovy:
##########
@@ -249,12 +248,6 @@ class PersistentEntityCodec extends BsonPersistentEntityCodec {
}
}
- else {
- // schedule lastUpdated if necessary
- if (entity.getPropertyByName(GormProperties.LAST_UPDATED) != null) {
Review Comment:
@jdaugherty yes, for the merge of the next phase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @Autotimestamp Mongo Persistence [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#issuecomment-3386850485
# In Summary
All this does is make @Autotimestamp properties dirty if there are changes so they will be persisted. It doesn't affect anything else. I spent about an hour making a test app that you can verify everything working on both hibernate and mongo in just 3 commands.
### Publish fix to mavenLocal()
```
git clone --depth 1 --branch 7.0.x-autotimestamp-mongo-fix https://github.com/codeconsole/grails-core grails-core-autotimestamp-mongo-fix && cd grails-core-autotimestamp-mongo-fix && ./gradlew pTML
```
### Run fix on port 8081 for mongo and 8082 for hibernate in 2 command
```
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'
```
http://localhost:8081/user/create
http://localhost:8082/user/create
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole closed pull request #15122: 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence
URL: https://github.com/apache/grails-core/pull/15122
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2426913237
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java:
##########
@@ -190,13 +195,15 @@ protected void storeDateCreatedAndLastUpdatedInfo(PersistentEntity persistentEnt
} else if (property.getName().equals(DATE_CREATED_PROPERTY)) {
storeTimestampAvailability(entitiesWithDateCreated, persistentEntity, property);
} else {
- Field field = getFieldFromHierarchy(persistentEntity.getJavaClass(), property.getName());
- if (field != null && field.isAnnotationPresent(AutoTimestamp.class)) {
- AutoTimestamp autoTimestamp = field.getAnnotation(AutoTimestamp.class);
- if (autoTimestamp.value() == AutoTimestamp.EventType.UPDATED) {
- storeTimestampAvailability(entitiesWithLastUpdated, persistentEntity, property);
- } else {
- storeTimestampAvailability(entitiesWithDateCreated, persistentEntity, property);
+ Field field = ReflectionUtils.findField(persistentEntity.getJavaClass(), property.getName());
Review Comment:
If we're trying to target a bug fix, why switch to a different method of resolution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#issuecomment-3400330269
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2427526344
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/codecs/PersistentEntityCodec.groovy:
##########
@@ -249,12 +248,6 @@ class PersistentEntityCodec extends BsonPersistentEntityCodec {
}
}
- else {
- // schedule lastUpdated if necessary
- if (entity.getPropertyByName(GormProperties.LAST_UPDATED) != null) {
Review Comment:
@jdaugherty yes, updates should be uniform in 1 location.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2427525226
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java:
##########
@@ -190,13 +195,15 @@ protected void storeDateCreatedAndLastUpdatedInfo(PersistentEntity persistentEnt
} else if (property.getName().equals(DATE_CREATED_PROPERTY)) {
storeTimestampAvailability(entitiesWithDateCreated, persistentEntity, property);
} else {
- Field field = getFieldFromHierarchy(persistentEntity.getJavaClass(), property.getName());
- if (field != null && field.isAnnotationPresent(AutoTimestamp.class)) {
- AutoTimestamp autoTimestamp = field.getAnnotation(AutoTimestamp.class);
- if (autoTimestamp.value() == AutoTimestamp.EventType.UPDATED) {
- storeTimestampAvailability(entitiesWithLastUpdated, persistentEntity, property);
- } else {
- storeTimestampAvailability(entitiesWithDateCreated, persistentEntity, property);
+ Field field = ReflectionUtils.findField(persistentEntity.getJavaClass(), property.getName());
Review Comment:
@jdaugherty because it is does exactly same thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 7.0.0-RC2 BUG FIX - @AutoTimestamp Mongo Persistence [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15122:
URL: https://github.com/apache/grails-core/pull/15122#discussion_r2426905633
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/codecs/PersistentEntityCodec.groovy:
##########
@@ -249,12 +248,6 @@ class PersistentEntityCodec extends BsonPersistentEntityCodec {
}
}
- else {
- // schedule lastUpdated if necessary
- if (entity.getPropertyByName(GormProperties.LAST_UPDATED) != null) {
Review Comment:
Are you just trying to minimize updates by removing this statement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org