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.
Best regards,
Gabriel.
Gabriel Roldan
groldan@anonymised.com
Expert service straight from the developers
(attachments)
