Re: [Geoserver-devel] WMS port to Dispatcher framework, please comment

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

Awesome work man. I hope to check it out as soon as possible. And I agree
with Andrea great material for a code sprint.

On Mon, Aug 30, 2010 at 12:40 AM, Andrea Aime <aa...@op...> wrote:

> Gabriel Roldan ha scritto:
> > Hi developers,
> >
> > I would like to ask for review and comments (and subsequently
> > approving once any concern that could raise is acted upon), of the
> > following GeoServer branch on my git repository that addresses the
> > long standing and much annoying WMS port to the same architecture
> > than the other OWS implementations:
> > <http://github.com/groldan/geoserver_trunk/tree/wmscleanup>
> >
> > If you ever had to deal with the WMS module source code, you'll be
> > amazed that you won't ever have (anymore) to: - wonder where to look
> > for some class related to certain specific functionality: now
> > everything's grouped into functionally cohesive packages, rather than
> > coincidentally or logically (please see the attached before/after
> > package screenshots too see what I mean). - figure out how the
> > awkward GetMapProducer API is turned into something the Dispatcher
> > can actually handle, nor get shocked when you find the only way is to
> > actually produce the map when the GetMapProducer.getMimeType() method
> > is called, nor wonder why a GetMapProducer needs to create a map and
> > write it, when the  Dispacher has operations and responses to
> > separate out those responsibilities. - freak out about the 2003/2004
> > Jalopy days DOCUMENT ME! anti-comments left overs
> >
> > In a word, this work is not only a package reorganization, but an
> > attempt to _finally_ bring the old servlet based, inheritance abusive
> > and too wired spaghetti WMS code into 2010's GeoServer standards.
> >
> > Some technical comments to guide you in your assessment: -
> > GetMapProducer refactored as GetMapOutputFormat with the sole
> > responsibility of producing a Map for a specific format - The
> > org.geoserver.wms.Map abstract class is the result of a GetMap
> > operation, and what a GetMapOutputFormat produces. The class name is
> > open to debate as I acknowledge it may be weird. -
> > org.geoserver.ows.Response concrete subclasses shall be registered as
> > Spring beans to deal with writing each specific org.geoserver.wms.Map
> > specialization (for example, BufferedImageMap, XMLTrasnsformerMap,
> > RawMap). So more than one GetMapOutputFormat implementation can
> > produce the same kind of Map (ie, XMLTransformerMap) which then will
> > be written down by a single Response implementation. - Contrary to
> > GetMapProducer, GetMapOutputFormats can be (and are recommended to
> > be) Spring singletons, meaning they're stateless by nature as the
> > state needed to further encode the map into the desired output format
> > is encapsulated in the returned org.geoserver.wms.Map. Besides more
> > programming simplicity and adherence to the Dispatcher framework,
> > this might lead to a little performance benefit since no new
> > instances need to be created for each and every GetMap request? -
> > Same thing goes for GetFeatureInfo and GetLegendGraphic output
> > formats, no more SPI mechanism, plain Spring beans lookup. For
> > example, the GetFeatureInfo operation returns a FeatureCollectionType
> > (like a WFS GetFeature one) and more response handlers can be easily
> > plugged in to deal with alternate output formats.
> >
> > Sorry it's becoming a long mail, please feel free to raise any
> > concerns. I don't expect the branch to be perfect yet but it
> > certainly looks to me like it's going in the right direction.
> >
> > The final goal is incorporating these changes into trunk asap.
> > Thankfully with the marvels of git that part is easy and conserves
> > full history.
>
> Gabriel,
> thanks for doing this work, it is indeed very much needed
> and appreciated.
>
> The amount of changes is staggering (80 commits) and it will
> take me a few days to go through them: the testing coverage
> in the wms module is light so a line by line visual inspection
> is needed before we land this patch.
> Even after a careful inspection I expect there will be numerous side
> effects, but the cleanup has long term benefits and
> it's worth some short term pain.
>
> Given FOSS4G is approaching I have no time to look into this
> for the next two weeks, but I hope to be able to come back
> to it once the FOSS4G storm calms down.
>
> Or maybe, if we find enough time,  we can go through the changes
> togheter in Barcelona... uh, we could use the code sprint for
> this :-)
>
> Cheers
> Andrea
>
>
>
> --
> Andrea Aime
> OpenGeo - http://opengeo.org
> Expert service straight from the developers.
>
>
> ------------------------------------------------------------------------------
> Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
> Be part of this innovative community and reach millions of netbook users
> worldwide. Take advantage of special opportunities to increase revenue and
> speed time-to-market. Join now, and jumpstart your future.
> http://p.sf.net/sfu/intel-atom-d2d
> _______________________________________________
> Geoserver-devel mailing list
> Geo...@li...
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>



-- 
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.