On Tue, May 28, 2013 at 3:03 PM, Justin Deoliveira <[email protected]>wrote:
> Thinking about it... the metadata map already requires that the objects be
> put in there are Serializable,
>
>> so maybe the easiest thing to do is to deep clone them by serialization
>> (maybe using XStream?) and
>> be done with it.
>>
>
> True, if this works then it i am all for it.
>
I went down this path, trying to keep cloning to the bare minimum.
The wrap/clone logic is here, adopted several strategies to keep the
overhead low and try to wrap
what we know is wrappable using just ModificationProxy, and to copy by
clone() or
copy constructor when possible (also added copy constructors to a couple of
objects that I saw were
being copied over and over). Also for the copy by serialization I tried to
use binary serialization
(which gives a true copy) when possible, falling back on XStream only as a
last resort:
https://github.com/geoserver/geoserver/pull/245/files#L4R46
Changes to ModificationProxy are minimal, although I'm not sure about the
shallow/deep cloning
choices, in this case, deep for the collection that's returned, shallow for
the one that's kept as the original state:
https://github.com/geoserver/geoserver/pull/245/files#L3L106
In the process I've found a couple of things in WFS that basically relied
on the collections being modified
directly, and just kept a permanent reference to the config objects:
https://github.com/geoserver/geoserver/pull/245/files#L7L76
https://github.com/geoserver/geoserver/pull/245/files#L8R254
With these changes I can no longer reproduce the GUI editing issues (and
oh, added a test to cover
the unwanted change to metadata links contents).
I'd like to commit this on trunk. On 2.3.x, don't know....
For sure several bits of the GUI are broken with the current situation, but
I'm not sure what side
effects might happen by switching to wrapping/cloning. As the patch shows,
some bits of code
existed that relied on that bug, those are fixed, but there might be
others...
Screwed if you do, screwed if you don't kind of thing. Given that I
reported the issue and users
have not complained about it yet, I'd keep it on trunk only.
What do you think?
Cheers
Andrea
--
==
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
-------------------------------------------------------
------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite
It's a free troubleshooting tool designed for production
Get down to code-level detail for bottlenecks, with <2% overhead.
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap2
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
