Re: [Geoserver-devel] Upgrade Jettison dependency version

[Justin D. <jde...@op...>]

No time line really but if you plan to push on this I would be more
inclined to push on this.

On Dec 26, 2011 10:42 AM, "Gabriel Roldan" <gr...@op...> wrote:
>
> On Mon, Dec 26, 2011 at 2:01 PM, Justin Deoliveira <jde...@op...>
wrote:
> > Hey gabriel,
> >
> > Thanks for the detailed explanation. Unfortunately I don't think it is
an
> > option to change the rest API serialization format on the stable branch.
> > There is just too much code out there written against it. I would be OK
with
> > doing it on the trunk though. Actually this sort of aligns with another
api
> > change I have been thinking about and that is to have then root node of
an
> > object dropped in a collection:
> >
> > http://xstream.codehaus.org/json-tutorial.html#json-dropping-root
> >
> > So I guess to upgrade on the stable branch if we upgrade we would have
to
> > tweak converters to maintain the old format. I know it seems a bit
silly to
> > maintain a bug but I think its the safest way forward.
>
> Yeah I agree on stable it's not an option. Hopefully the rest
> collection converter seems to be isolated enough as to make the
> backwards compatible change in there, if we had to upgrade jettison on
> stable too.
> Personally I'm not in a rush, would just need to upgrade jettison when
> the time comes to backport the gwc integration improvements. By that
> time I'll make sure to propose a patch for review.
> Back on to trunk, do you have a timeline for the root element name drop?
>
> Cheers,
> Gabriel
>
> >
> > $0.02
> >
> > -Justin
> >
> > On Dec 26, 2011 12:00 AM, "Gabriel Roldan" <gr...@op...> wrote:
> >>
> >> Due to a bug in Jettison 1.0.1,  serializing and deserializing
> >> collections is broken <http://jira.codehaus.org/browse/JETTISON-48>
> >>
> >> Here's a test that demonstrates the issue:
> >>
> >> package org.geoserver.rest.format;
> >> ...
> >> public class ReflectiveJSONFormatTest extends TestCase {
> >>
> >>    @SuppressWarnings({ "unchecked", "rawtypes" })
> >>    public void testFoo() throws Exception {
> >>
> >>        ReflectiveJSONFormat format = new ReflectiveJSONFormat();
> >>
> >>        Foo foo = new Foo("list", 2, 3.0, new
> >> ArrayList(Arrays.asList(1L, 2.5D)));
> >>
> >>        ByteArrayOutputStream output = new ByteArrayOutputStream();
> >>        format.write(foo, output);
> >>
> >>        Object read = format.read(new
> >> ByteArrayInputStream(output.toByteArray()));
> >>
> >>        assertTrue(read instanceof Foo);
> >>        Foo foo2 = (Foo) read;
> >>        assertEquals(foo.getList(), foo2.getList());
> >>        assertEquals(foo.getProp1(), foo2.getProp1());
> >>        assertEquals(foo.getProp2(), foo2.getProp2());
> >>        assertEquals(foo.getProp3(), foo2.getProp3());
> >>    }
> >> }
> >>
> >> The test fails at the first assert. (Have added a list property to the
> >> org.geoserver.rest.Foo test object).
> >>
> >> The generated JSON with the current Jettison version is
> >> Jettison 1.0.1:
> >>
> >>
{"org.geoserver.rest.Foo":{"list":{"int":[1],"double":2.5},"prop1":"list","prop2":2,"prop3":3}}
> >>
> >> The generated JSON with Jettison 1.3 (latest release) is:
> >> Jettison 1.3:
> >>
> >>
{"org.geoserver.rest.Foo":{"list":[{"int":1,"double":2.5}],"prop1":"list","prop2":2,"prop3":3}}
> >>
> >> As you can see, the currently generated value of the "list" attribute
> >> is not a json array as it should be, whilst the Jettison 1.3 one is.
> >>
> >> Upgrading to Jettison 1.3 breaks some restconfig tests though, because
> >> they're asserting the "incorrect" format is being generated.
> >> For instance:
> >> org.geoserver.catalog.rest.WorkspaceTest.testGetAllAsJSON
> >> org.geoserver.catalog.rest.DataStoreTest.testGetAllAsJSON
> >> org.geoserver.catalog.rest.CoverageStoreTest.testGetAllAsJSON
> >> org.geoserver.catalog.rest.NamespaceTest.testGetAllAsJSON
> >> org.geoserver.catalog.rest.WMSStoreTest.testGetAllAsJSON
> >> org.geoserver.catalog.rest.StyleTest.testGetAllAsJSON
> >>
> >> Take for example WorkspaceTest.testGetAllAsJSON
> >> The GET request produces:
> >>
> >>
> >>
 {"workspaces":{"workspace":[{"name":"cite","..."},{"name":"cgf","..."},{"name":"sf","..."},{"name":"gs","..."},{"name":"cdf","..."}]}}
> >>
> >> while Jettison 1.3 produces:
> >>
> >>
{"workspaces":[{"workspace":[{"name":"cite","..."},{"name":"cgf","..."},{"name":"sf","..."},{"name":"gs","..."},{"name":"cdf","..."}]}]}
> >>
> >> In the second case, "workspaces" is in itself an array. And that's
> >> correct because the object being encoded is a List<WorkspaceInfo>.
> >>
> >> So the question is, is there any chance we can upgrade to Jettison 1.3
> >> and fix those tests? Would it be too risky a REST api breakage?
> >> I would like to upgrade and get rid of that Jettison bug because it is
> >> very much needed in order to work with the gwc configuration that
> >> collection properties are properly encoded and decoded when they
> >> contain polymorphic elements, which currently is impossible.
> >>
> >> TIA,
> >> Gabriel
> >>
> >> --
> >> Gabriel Roldan
> >> OpenGeo - http://opengeo.org
> >> Expert service straight from the developers.
>
>
>
> --
> Gabriel Roldan
> OpenGeo - http://opengeo.org
> Expert service straight from the developers.