GSIP 49 WMS module cleanup and refactoring
Clean up packages, javadocs and port the seven years old code to the current Dispatcher architecture
Gabriel Roldan
2.1.0
Choose one of: Under Discussion, In Progress, Completed, Rejected, Deferred
Current code is based on the GeoServer 1.1–1.5 architecture and adapted to the GeoServer 1.6+ one. We have a mix of old and new package names, inconsistent javadocs, hard to follow execution path (e.g., maps are being computed when the mime type is requested by the Dispatcher) and follows a package structure that spreads out highly functionally related classes into a bunch of packages.
The need to fix this situation has been known from a long time, so it should be time to finally do it.
We propose a deep refactoring of the existing module providing the following benefits:
- all code is stored in “org.geoserver” subpackages removing all usage of the old “org.vfny.geoserver” one, in an effort to get closer to functional cohesion.
- the code is refactored towards the Dispatcher architecture separating the concerns of generating and encoding WMS results
- all the execution chain is based on Spring singletons removing the need for prototype bean instantiation
- removal of dead code
- javadoc cleanups
- 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.
| Current package structure | Proposed package structure |
|---|---|
 ] |
( ![image)(wms_packages_cleanup.gif) |
The set of changes is difficult to explain in detail, but it’s distributed among about 80 separate commits in a GitHub branch: http://github.com/groldan/geoserver_trunk/tree/wmscleanup
The branch is being reviewed as part of the FOSS4G 2010 code sprint by Andrea, Gabriel and Justin.
During the code review the following items have been raised:
- (/) done GetMapOutputFormat.enabled () can be replaced by an ExtensionFilter (a generic extension blacklisting mechanism that we added a few months ago)
- (/) done Currently some map producers also implement the Dispatcher Response interface. Consider breaking them apart in two classes so that one is concerned with only generating the buffered image and another only concerned with encoding the image into some external format (we’ll still have a JPEG and GIF custom producers to deal with the detailed image setup they need to handle transparency and palettes)
- (/) done Finish porting the ImageMap extension module to the new architecture (unit tests missing)
- (/) done add missing copyright headers and class javadocs for the new objects
- (/) done have a legend graphic response set of objects just like in GetMap (atm there is just one object looking for delegates in the Spring context)
- (/) done Need to run the WMS 1.1 CITE tests against the new branch
Changes will break backwards compatibility with map producers that might have been developed outside of the GeoServer source code.
Andrea Aime: +1 Alessio Fabiani Jody Garnett: +1 Chris Holmes: +1 Rob Atkinson: +1 Ben Caradoc Davies: +1 Mark Leslie:
Gabriel Roldan +1 Justin Deoliveira: +1