GSIP 49 - WMS module cleanup and refactoring

compared with
Current by Gabriel Roldán
on Sep 22, 2010 00:29.

These lines were removed. This word was removed.
These lines were added. This word was added.

View page history

There are 2 changes. View first change.

 h2. Overview
 {excerpt}Clean up packages, javadocs and port the seven years old code to the current Dispatcher architecture{excerpt}
 h4. Proposed By
 h4. Assigned to Release
 h4. State
 Choose one of: Under Discussion, *In Progress*, Completed, Rejected, Deferred
  Choose one of: Under Discussion, In Progress, *Completed*, Rejected, Deferred
 h2. Motivation
 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.
 h2. Proposal
 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||
 | !wms_packages.gif! | !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:
 The branch is being reviewed as part of the FOSS4G 2010 code sprint by Andrea, Gabriel and Justin.
 h2. Feedback
 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
 h2. Backwards Compatibility
 Changes will break backwards compatibility with map producers that might have been developed outside of the GeoServer source code.
 h2. Voting
 [~aaime]: +1
 [~jive]: +1
 [~cholmes]: +1
 [~roba]: +1
 [~bencaradocdavies]: +1
 h3. Community support
 [~groldan] +1
 [~jdeolive]: +1
 h2. Links
 [JIRA Task|]
 [Email Discussion|]
 [Wiki Page|]
  [JIRA Task|]
View Attachments (4) Info