This page last changed on Sep 22, 2010 by groldan.

Overview

Clean up packages, javadocs and port the seven years old code to the current Dispatcher architecture

Proposed By

Gabriel Roldán

Assigned to Release

2.1.0

State

Choose one of: Under Discussion, In Progress, Completed, Rejected, Deferred

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.

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

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.

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)
  • doneFinish 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

Backwards Compatibility

Changes will break backwards compatibility with map producers that might have been developed outside of the GeoServer source code.

Voting

Andrea Aime: +1
Alessio Fabiani
Jody Garnett: +1
Chris Holmes: +1
[~roba]: +1
Ben Caradoc-Davies: +1
Mark Leslie:

Community support

Gabriel Roldán +1
Justin Deoliveira: +1

Links

JIRA Task


before.png (image/png)
after.png (image/png)
wms_packages.gif (image/gif)
wms_packages_cleanup.gif (image/gif)
Document generated by Confluence on May 14, 2014 23:00