GSIP-157 / GEOS-7987 - Track modified properties in CatalogPostModifyEvent

[tbarsballe]

GSIP Here: https://github.com/geoserver/geoserver/wiki/GSIP-157

API Changes:

• Public methods getPropertyNames,getOldValues,getNewValues added to CatalogPostModifyEvent interface.
• Public methods getPropertyNames, setPropertyNames, getNewValues, setNewValues, getOldValues, setOldValues added to CatalogPostModifyEventImpl.
• The arguments List<String> propertyNames, List oldValues, List newValues have been added to the package private method firePostModified of AbstractCatalogDecorator, AbstractFilteredCatalog, CatalogImpl, and SecureCatalogImpl, and to the protected methods beforeSaved and afterSaved of AbstractCatalogFacade.

All invocations of above methods and classes have been updated accordingly.

Given that all the changes are internal (protected or package private) to the Catalog, this seems likely to be a relatively safe change (Although I think it should not be backported, as it is still an API change)

[aaime]

Big API break Batman! There is no such a thing as a package protected method for an interface, they are all public.
http://stackoverflow.com/questions/5376970/protected-in-interfaces

In this current form the change is proposal worthy and non backportable. Any chance you can use default methods to fix the backwards compatibility issues?

[tbarsballe]

There is no such a thing as a package protected method for an interface, they are all public.

Huh, learned something new today. Seems like that should have been obvious in retrospect.

GSIP incoming.

There are a few ways I could see of maintaining API backwards-compatibility. Unfortunately, they all run into the same problem:

By necessity, anything using these methods would not set the changed values. This means that anything listening on CatalogPostModifiedEvent can no longer rely on an empty list of properties meaning nothing has changed.
If we can't rely on the list of changed values being valid, then this substantially reduces the utility of this change.

[aaime]

Master is ready for breaking changes :-)

[tbarsballe]

Thanks for the reminder. I've updated this so it is mergeable again, and sent out a notification to the dev list.

…

On Sat, Mar 18, 2017 at 3:50 AM, Andrea Aime ***@***.***> wrote: Master is ready for breaking changes :-) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2101 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AI14yYhKkhLZ00Ue9jFd1t8B1Dmq8qrCks5rm7cKgaJpZM4L8keA> .

[aaime]

I see this pattern repeated over and over and over... how about a method taking a lambda to do the bit in the middle, after the notication and before commitProxy? Just thinking out loud.

[tbarsballe]

That sounds like a good idea, although I don't know how to implement it (I am still not especially familiar with lambdas in java). If you've got an example of what you are suggesting, I could certainly apply it to the relevant places.