Hi,
I was playing around with the INSPIRE extension and I’ve noticed this issue.
Basically, ModificationProxy fails to guard against modification of collections, when the modification hits a mutable item contained in it.
ModificationProxy collection handling basically clones then collection before returning it, so that the caller works against a copy, but fails to deep clone it,
as a result:
- if the client adds/removes items from the returned collection nothing bad happens
- if the client grabs an item from the collection and alters it, it’s actually modifying the same item referred by the original collection
One example where this is happening in our GUI is the metadata links.
Here is a little experiment:
- Go to a layer, add a new metadata link with url “http://www.test.com”, save the layer.
- Open the same layer, modify the url to be “http://www.test.com/123”
- Add a new link (this will force wicket to partially submit the form and thus change the state of the objects behind the GUI, which are coming from the ModificationProxy anyways)
- Exit by pressing cancel
- Open back the same layer and observe the results. The new link is not there, but the url of the first one got modified.
I haven’t tried, but guess anything that’s mutable and stuck in the metadata map will follow the same destiny (e.g., SQL view definitions).
Now… what to do about it.
We cannot wrap the collection contents in other ModificationProxies, because we don’t have a target interface for them.
I guess we can deep clone them. Deep cloning is not simple, and one might need some way to control how to stop the cloning at some point (so cloning by serialization is out of the question).
I’ve found this library that seems to be doing a decent job at deep cloning in memory with some cloning control: https://code.google.com/p/cloning/wiki/Usage
The library is also quite small. The idea is that we’d use a Spring registered Cloner object to deep clone the contents of all collections before returning them out of a modification proxy.
What do you think?
Cheers
Andrea
PS: I know this is yet another band aid, however I don’t see any good solution in sight until we can replace the whole config subsystem with a storage that’s not memory based and understands transactions.
At the moment I’m just trying to get the INSPIRE piugin going, not trying to start a crusade to improve the config subsytem…
–
==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it