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

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

longwa
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

Reply | Threaded
Open this post in threaded view
|

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

longwa


On Mon, Jan 6, 2014 at 5:00 PM, Aaron Long <[hidden email]> wrote:
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


Reply | Threaded
Open this post in threaded view
|

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

longwa
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:


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


On Mon, Jan 6, 2014 at 5:13 PM, Aaron Long <[hidden email]> wrote:


On Mon, Jan 6, 2014 at 5:00 PM, Aaron Long <[hidden email]> wrote:
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



Reply | Threaded
Open this post in threaded view
|

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

longwa
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.


On Mon, Jan 6, 2014 at 6:19 PM, Aaron Long <[hidden email]> wrote:
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:


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


On Mon, Jan 6, 2014 at 5:13 PM, Aaron Long <[hidden email]> wrote:


On Mon, Jan 6, 2014 at 5:00 PM, Aaron Long <[hidden email]> wrote:
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