Sure, pair review at foss4g seems great. Cheers, Gabriel On Aug 30, 2010, at 11:09 AM, Justin Deoliveira wrote:
> 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 <[email protected]> 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 > [email protected] > https://lists.sourceforge.net/lists/listinfo/geoserver-devel > > > > -- > Justin Deoliveira > OpenGeo - http://opengeo.org > Enterprise support for open source geospatial. > Gabriel Roldan [email protected] Expert service straight from the developers ------------------------------------------------------------------------------ This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd _______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
