Thanks for the review Andrea.
I have found xstream to be very flexible and straight forward. Tweaking the default settings and adding in some custom stuff I have been able to output xml which looks pretty close to what we actually output today.
Comments/concerns/feedback welcome.
The code looks nice and clean, the effort to extend it seems low,
it seems possible to achieve extension by adding extensions points
so that other plugins do configure the XStream instance for their
needs. I like it.
Yeah I thought about extensibility. We could have the class process an extension point. However, my thought was that if some code needs to customize the xstream in some way (adding a converter or omitting a field or something) they can just grab the reference to the XStream object and modify it.
Questions:
* how is the final layout of the xml files like?
With all the converters in that class similar to what we have today. Example:
<catalog>
<stores>
<dataStore>
<id>foo</id>
<name>foo</name>
<enabled>false</enabled>
<workspace>foo</workspace>
<connectionParameters/>
<metadata/>
</dataStore>
</stores>
<namespaces>
<namespace default="true">
<id>acme</id>
<prefix>acme</prefix>
<uri>http://acme.org</uri>
</namespace>
</namespaces>
<workspaces>
<workspace default="true">
<id>foo</id>
<name>foo</name>
</workspace>
</workspaces>
<layerGroups/>
<styles>
<style>
<id>style</id>
<name>style</name>
<filename>style.sld</filename>
</style>
</styles>
</catalog>
We could even do more to make it look the same.
* is this limited to save just the services for the moment, and in
particular only wfs?
* what about the catalog?
Yeah at the moment only wfs is hooked up to xstream stuff. But hooking up the otehr stuff is pretty easy. Same goes for catalog and feature type / coverage infos. Not hooked up but not too much work.
* why are you not persisting the client properties out of the GeoServer
object? (xs.omitField(GeoServerInfoImpl.class, "clientProperties")
The idea here was that "metadata" would be persisted and "clientProperties" would be transient. We could just remove "clientProperties" all together. I cant think of a good use case for them at the moment.
Cheers
Andrea
!DSPAM:4007,486374503595219720167!
--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com