Hey Ian,
On Wed, Jul 27, 2011 at 4:23 PM, Ian Schneider <[email protected]>wrote:
> Hey Justin,
>
> I'm not sure I understand what you don't agree with ;)
>
> Just that a "mostly backward compatible" approach will suffice when a
completely backward compatible one is possible. Not saying you were
advocating for that so "don't agree" was poor phrasing on my part.
As for implementing the check for the headers, this seems like a
> reasonable approach, though at least for code in the tree, only one
> implementation remains that uses the headers. I hesitate here because
> it involves more dispatcher code that will mostly not be used.
>
> As I said output formats are the most common way that geoserver is extended
so "out of tree" implementations should be considered. During my time people
trying to implement custom output formats have come up a number of times...
I imagine many of them don't get contributed back, often with good reason
because they are very custom in nature. So I generally operate under the
assumption that there lots of them out there.
> The other approach I offered was to make the default implementation
> opt-out of the new behavior (100% backwards compatible) and adjust the
> current implementors to opt-in (remove getHeaders implementation and
> use the new API).
>
Right, which is why I think opting out of the new behaviour (by not calling
the new methods) iff the header has been set by getHeaders() is a
reasonable middle ground.
>
> We _do_ agree that the current solution is not optimal for out-of-tree
> Response subclasses, 100% backwards compatibility is to be strived
> for, and that changes to the API should not be taken lightly.
>
> -Ian
>
> On Wed, Jul 27, 2011 at 2:58 PM, Justin Deoliveira <[email protected]>
> wrote:
> > Hmmm... not sure I agree. I tend to always prefer 100%
> > backward compatibility when its possible. I know in GeoServer we don't
> > publish formal api at all but the output format is the one that is most
> > commonly implemented. So I think updating the api should not be taken
> > lightly.
> > What about having the dispatcher check the result of
> Response.getHeaders()
> > and if the content-disposition header was specified there don't worry
> about
> > calling the specific methods to try and determine it?
> >
> > On Wed, Jul 27, 2011 at 9:52 AM, Ian Schneider <[email protected]>
> > wrote:
> >>
> >> Hey Justin,
> >>
> >> In the case you mention, it is mostly backwards compatible - the
> >> original preferred headers are respected if the Response returns null
> >> from getAttachmentFileName. This has to be implemented in the Response
> >> subclass, though, since the default is to compute a filename based on
> >> operation and mimetype. The latter could be changed, but it seemed
> >> like a balance between getting useful default behavior or preserving
> >> the old behavior - which in almost all of the cases was trivial to
> >> upgrade. In other words, change the Response base class behavior to
> >> opt-in versus opt-out.
> >>
> >> As for the client overriding the preferred type (or headers via old
> >> API), that was the intention of the changes.
> >>
> >> On Wed, Jul 27, 2011 at 9:11 AM, Justin Deoliveira <
> [email protected]>
> >> wrote:
> >> > Hi Ian,
> >> > So are you saying that the change is backward compatible with existing
> >> > Responses/output formats that use getHeaders() to set the content
> >> > dispotiion. Looking at the patch (and maybe I am not looking close
> >> > enough)
> >> > it seems like the new api will overwrite in the case where the client
> >> > specifies the content-disposition flag?
> >> > Excuse the noise if i am missing something.
> >> > On Tue, Jul 26, 2011 at 8:58 PM, Ian Schneider <
> [email protected]>
> >> > wrote:
> >> >>
> >> >> Hi everyone,
> >> >>
> >> >> While committing my patches for
> >> >> http://jira.codehaus.org/browse/GEOS-4683, I broke the build. This
> was
> >> >> resolved fairly quickly (note to self - run maven just like hudson
> >> >> does) but made me realize these changes were worth a heads up to
> >> >> anyone authoring OWS Response subclasses.
> >> >>
> >> >> For details see the OWS Dispatcher and the Response javadoc, but the
> >> >> gist is that previously each Response subclass was handling
> >> >> Content-Disposition headers via the getHeaders method. The new
> >> >> behavior is that the Dispatcher uses the new API to determine a
> >> >> filename, and if not null, the preferred disposition type (attachment
> >> >> or the default, inline). If the request provides the
> >> >> content-disposition header parameter (either inline or attachment
> with
> >> >> header injection checking) then this is respected over the Response
> >> >> preference.
> >> >>
> >> >> I handled most of the Response subclasses and the tests. This is a
> >> >> heads up for existing maintainers on the changes as well as for new
> >> >> implementations.
> >> >>
> >> >> On a related note, it is not clear whether the filename should be
> >> >> quoted or encoded, but this patch will at least make that
> >> >> cross-cutting concern easier to address at some point (though it
> >> >> requires a number of test case adjustments).
> >> >>
> >> >> Thanks for bearing with me and comments much appreciated,
> >> >> -Ian
> >> >>
> >> >> --
> >> >> Ian Schneider
> >> >> OpenGeo - http://opengeo.org
> >> >> Enterprise support for open source geospatial.
> >> >>
> >> >>
> >> >>
> >> >>
> ------------------------------------------------------------------------------
> >> >> Got Input? Slashdot Needs You.
> >> >> Take our quick survey online. Come on, we don't ask for help often.
> >> >> Plus, you'll get a chance to win $100 to spend on ThinkGeek.
> >> >> http://p.sf.net/sfu/slashdot-survey
> >> >> _______________________________________________
> >> >> Geoserver-devel mailing list
> >> >> [email protected]
> >> >> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
> >> >
> >> >
> >> >
> >> > --
> >> > Justin Deoliveira
> >> > OpenGeo - http://opengeo.org
> >> > Enterprise support for open source geospatial.
> >> >
> >>
> >>
> >>
> >> --
> >> Ian Schneider
> >> OpenGeo - http://opengeo.org
> >> Enterprise support for open source geospatial.
> >
> >
> >
> > --
> > Justin Deoliveira
> > OpenGeo - http://opengeo.org
> > Enterprise support for open source geospatial.
> >
>
>
>
> --
> Ian Schneider
> OpenGeo - http://opengeo.org
> Enterprise support for open source geospatial.
>
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
------------------------------------------------------------------------------
Got Input? Slashdot Needs You.
Take our quick survey online. Come on, we don't ask for help often.
Plus, you'll get a chance to win $100 to spend on ThinkGeek.
http://p.sf.net/sfu/slashdot-survey
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
