save() is calling read-only getter methods during EntityAccess.refresh()

[Aaron Long]

After upgrading to Grails 2.3.4, I've noticed that my non-persistent
properties on objects are being called on save() (things with just getters,
for instance).

Digging around, I see this method:

/**
* Refreshes the object from entity state.
*/
public void refresh() {
final PropertyDescriptor[] descriptors =
beanWrapper.getPropertyDescriptors();
for (PropertyDescriptor descriptor : descriptors) {
final String name = descriptor.getName();
if (EXCLUDED_PROPERTIES.contains(name)) {
continue;
}

if (!beanWrapper.isReadableProperty(name) ||
!beanWrapper.isWritableProperty(name)) {
continue;
}

Object newValue = getProperty(name);
setProperty(name, newValue);
}
}

One of the descriptors in this loop is the property "properties", which is
a DataBindingLazyMetaPropertyMap containing ALL properties, even
non-writable ones. It ends up going through some of the converters and
calling all those getters, even though the refresh() seems to want to avoid
anything that isn't both writable and readable.

Not sure what changed in 2.3 to make this behavior start but it seems like
"properties" should be added to the EXCLUDED_PROPERTIES map.

I'm glad to open a JIRA and submit a patch, just wanted to make sure I
wasn't missing anything...

-Aaron

[Aaron Long]

I opened http://jira.grails.org/browse/GRAILS-10961 for this issue.

Post by Aaron Long
After upgrading to Grails 2.3.4, I've noticed that my non-persistent
properties on objects are being called on save() (things with just getters,
for instance).
/**
* Refreshes the object from entity state.
*/
public void refresh() {
final PropertyDescriptor[] descriptors =
beanWrapper.getPropertyDescriptors();
for (PropertyDescriptor descriptor : descriptors) {
final String name = descriptor.getName();
if (EXCLUDED_PROPERTIES.contains(name)) {
continue;
}
if (!beanWrapper.isReadableProperty(name) ||
!beanWrapper.isWritableProperty(name)) {
continue;
}
Object newValue = getProperty(name);
setProperty(name, newValue);
}
}
One of the descriptors in this loop is the property "properties", which is
a DataBindingLazyMetaPropertyMap containing ALL properties, even
non-writable ones. It ends up going through some of the converters and
calling all those getters, even though the refresh() seems to want to avoid
anything that isn't both writable and readable.
Not sure what changed in 2.3 to make this behavior start but it seems like
"properties" should be added to the EXCLUDED_PROPERTIES map.
I'm glad to open a JIRA and submit a patch, just wanted to make sure I
wasn't missing anything...
-Aaron

[Aaron Long]

I confirmed that adding "properties" to the exclude list fixes this issue.
I can't imagine that it's good for refresh to copy all the properties
essentially twice. I've created a pull request:

https://github.com/grails/grails-data-mapping/pull/4

This is breaking at least 100 test cases for us so it would be nice to see
it in 2.3.5 if possible.

-Aaron

Post by Aaron Long
I opened http://jira.grails.org/browse/GRAILS-10961 for this issue.

Post by Aaron Long
After upgrading to Grails 2.3.4, I've noticed that my non-persistent
properties on objects are being called on save() (things with just getters,
for instance).
/**
* Refreshes the object from entity state.
*/
public void refresh() {
final PropertyDescriptor[] descriptors =
beanWrapper.getPropertyDescriptors();
for (PropertyDescriptor descriptor : descriptors) {
final String name = descriptor.getName();
if (EXCLUDED_PROPERTIES.contains(name)) {
continue;
}
if (!beanWrapper.isReadableProperty(name) ||
!beanWrapper.isWritableProperty(name)) {
continue;
}
Object newValue = getProperty(name);
setProperty(name, newValue);
}
}
One of the descriptors in this loop is the property "properties", which
is a DataBindingLazyMetaPropertyMap containing ALL properties, even
non-writable ones. It ends up going through some of the converters and
calling all those getters, even though the refresh() seems to want to avoid
anything that isn't both writable and readable.
Not sure what changed in 2.3 to make this behavior start but it seems
like "properties" should be added to the EXCLUDED_PROPERTIES map.
I'm glad to open a JIRA and submit a patch, just wanted to make sure I
wasn't missing anything...
-Aaron

[Aaron Long]

I attached a sample app to the ticket. I found that the issue only affects
domain classes that have beforeInsert() and/or beforeUpdate() event
listeners, since they trigger the EntityAccess.refresh() method before
firing the events.

Post by Aaron Long
I confirmed that adding "properties" to the exclude list fixes this issue.
I can't imagine that it's good for refresh to copy all the properties
https://github.com/grails/grails-data-mapping/pull/4
This is breaking at least 100 test cases for us so it would be nice to see
it in 2.3.5 if possible.
-Aaron

Post by Aaron Long
I opened http://jira.grails.org/browse/GRAILS-10961 for this issue.

Post by Aaron Long
After upgrading to Grails 2.3.4, I've noticed that my non-persistent
properties on objects are being called on save() (things with just getters,
for instance).
/**
* Refreshes the object from entity state.
*/
public void refresh() {
final PropertyDescriptor[] descriptors =
beanWrapper.getPropertyDescriptors();
for (PropertyDescriptor descriptor : descriptors) {
final String name = descriptor.getName();
if (EXCLUDED_PROPERTIES.contains(name)) {
continue;
}
if (!beanWrapper.isReadableProperty(name) ||
!beanWrapper.isWritableProperty(name)) {
continue;
}
Object newValue = getProperty(name);
setProperty(name, newValue);
}
}
One of the descriptors in this loop is the property "properties", which
is a DataBindingLazyMetaPropertyMap containing ALL properties, even
non-writable ones. It ends up going through some of the converters and
calling all those getters, even though the refresh() seems to want to avoid
anything that isn't both writable and readable.
Not sure what changed in 2.3 to make this behavior start but it seems
like "properties" should be added to the EXCLUDED_PROPERTIES map.
I'm glad to open a JIRA and submit a patch, just wanted to make sure I
wasn't missing anything...
-Aaron