Hi,
today I was following up on the bbox wfs kvp parsing issue cited a few days
ago. So I set myself up a new bbox parser parsing towards a filter,
and rolled out a bbox parser for WMS to parse an envelope instead.

It occurred to me to try out what would have happened if a request came
in without a service, but called from "/wfs?..."...
Boom! Exceptions, because the two bbox parsers are service specific now.
However, before it was working.
So I started to investigate what was happening in the dispatcher and
fixed issues
as I saw them... the changes started to cascade and I soon found myself
in research mode.
Oh well, nothing wrong with that but in the end I came up with a much larger
patch than I desired, and in the end the single blob I came up with is really
a mix of three patches: I'll eventually split it up in three parts but for now
take it as a draft that I'm making public for the sake of discussion.

And now let's talk about the three portions making up the patch (which is
attached).

BBOX parsing
---------------------

Nothing special here. The patch sets up a new bbox parser for WFS that
parses the bbox=... into a BBOX filter preserving the srsName verbatim.
So no need to jump through hoops in order to rebuild the srsName afterwards.
The old bbox parser has been moved to WMS, where it does the right thing,
and made service specific (specific to wms).

KVP/XML parsing, filtering, prioritizing, and heuristics (oh my!)
--------------------------------------------------------------------------------------------

The current dispatcher roughly works like this:
- parse the kvp (bzzt, wrong!)
- get the service, request and version from kvp or xml
- eventually parse the xml (bzzt, wrong!)
- apply heuristics to pick up the service and the version in case
  they are not specified and we're not in cite compliance mode
- ... (lots of other stuff which is not relevant to this discussion)

The heuristics applied at the last step are basically:
- use the last part of the url as indication of the service if the
  actual service is missing (that's the reason why we sometimes
  see an error such as "can't find "ows" service when making
  an invalid request)
- use the highest version if none was specified

Ok, so why the "bzzt, wrong!" thing? Well, because kvp parsers
and xml readers can be service, version, and even request specific.
But the heuristics to handle incomplete requests are handled too
late, when the kvp and xml parsing is long done.

So one big part of this patch moves the heuristics at the first
step in the chain instead, so that the kvp and xml parsing
can work against the chosen service and version, regardless
of how we came to that choice.

This basically means the choice of the Service object is now
done first, whilst before it was done later. No big deal, but to
preserve compatibility with DispatcherCallback, that exposes
the old order, the serviceDispatched() callback is fired exactly
at the same time as before (which is the reason why Request
now has a serviceBean field, to store the Service decided during
the init and provide it back when it's time to use it for callbacks
and dispatch).

Once the service and version are found they are fed back into
the Request and into kvp map so that KVPUtils.parseKVP(Map)
method, which is used in various other places, would not
need changes.

Cite compliance
--------------------------------------------------------------------------

Moving up the heuristics for locating the service and the
version broke the cite compliance
checks because they end up playing with the version and
service inferred from the heuristics.
This first made me add a "native" (or should I have said
"original") service and version to be used for those checks:
if the request did not actually have the service and/or version
that will result in an exception.

Now, looking around I noticed a mismatch, the cite compliance
mode was actually respected only for wfs using the "citecompliancehack"
interceptor, which then put the dispatcher into "strict" mode.

However the cite compliance configuration is exposed for all services,
not just wfs.
One way would have been to have a similar interceptor for the
other services, but why not use the dispatcher callback instead?

So what I did was to move the "citeCompliant" flag from the dispatcher
into the Request, and then create a generic callback that given
the service would look into the configuration and see if the cite
compliance flag was set for that specific service.

This had a repercussion on a WMS specific hack: in that protocol
for backwards compatibility the "wmtver" parameter is equated to the
"version". To have that keep on working I've rolled out another simple
callback that would copy the wmtver value into the service field.

Summing up
---------------------------------------------------

Two questions might arise:
- was all this really necessary?
- did you run the cite tests?

The answer to the second question is no, but if people think
the above changes are an improvement worth committing
I'll do run the cite tests, of course.

As for the first... hum... no, it is not strictly necessary.
However it makes kvp and xml parsers that are service and
version specific actually work as intended even when the request
is incomplete, and make the cite compliance flag work for all services,
not just for wfs.

Plan B
-------------------------------------------------

If changes in the patch are not well received or considered too risky,
is it still possible to
fix the original problem, that is, have the srsName of the bbox filter
preserved?
The answer is yes, a simpler, but hackier path is to have that parser return
a subclass of ReferencedEnvelope, let's call it SRSEnvelope, that also
stores the
srsName as is, and then use it in the wfs GetFeature kvp reader to
build the proper
BBOX filter.
It's probably 100loc total, so very small, quite well contained, but
it's clear it's a workaround
for a non fully working situation in the kvp parsing handling.

Long story short, it's not that I need the patch attached to this mail.
We can consider it an exploration of some of current dispatcher weaknesses and
put it apart as R&D for times where we have more time or we're
interested in taking
more risks.

Oh well, this mail has been long enough. I'll let you decide whether
the attached patch
falls into the "trick" or in the "treat" category :-p
Let me know!

Cheers
Andrea

-----------------------------------------------------
Ing. Andrea Aime
Senior Software Engineer

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy

phone: +39 0584962313
fax:     +39 0584962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-----------------------------------------------------
diff --git a/src/extension/wfsv/src/main/java/applicationContext.xml b/src/extension/wfsv/src/main/java/applicationContext.xml
index ae211e3..675f202 100644
--- a/src/extension/wfsv/src/main/java/applicationContext.xml
+++ b/src/extension/wfsv/src/main/java/applicationContext.xml
@@ -83,11 +83,6 @@
     class="org.geoserver.ows.OWSHandlerMapping">
     <constructor-arg ref="catalog"/>
     <property name="alwaysUseFullPath" value="true"/>    
-    <property name="interceptors">
-      <list>
-        <ref bean="citeComplianceHack" />
-      </list>
-    </property>
 
     <property name="mappings">
       <props>
diff --git a/src/main/src/main/java/applicationContext.xml b/src/main/src/main/java/applicationContext.xml
index d79c227..400eb0b 100644
--- a/src/main/src/main/java/applicationContext.xml
+++ b/src/main/src/main/java/applicationContext.xml
@@ -109,6 +109,10 @@
   
   <bean id="disabledServiceChecker" class="org.geoserver.ows.DisabledServiceCheck"/>
   
+  <bean id="citeComplianceCallback" class="org.geoserver.ows.CiteComplianceCallback">
+    <constructor-arg index="0" ref="geoServer" />
+  </bean>
+  
   <bean id="dispatcherMapping" 
     class="org.geoserver.ows.OWSHandlerMapping">
     <constructor-arg ref="catalog"/>
diff --git a/src/main/src/main/java/org/geoserver/ows/CiteComplianceCallback.java b/src/main/src/main/java/org/geoserver/ows/CiteComplianceCallback.java
index e4a9cbf..b491d3d 100644
--- a/src/main/src/main/java/org/geoserver/ows/CiteComplianceCallback.java
+++ b/src/main/src/main/java/org/geoserver/ows/CiteComplianceCallback.java
@@ -10,6 +10,10 @@ public class CiteComplianceCallback implements DispatcherCallback {
 
     GeoServer geoServer;
 
+    public CiteComplianceCallback(GeoServer geoServer) {
+        this.geoServer = geoServer;
+    }
+
     public void finished(Request request) {
         // nothing to do
     }
diff --git a/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java b/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java
index 4a8888f..384aec7 100644
--- a/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java
+++ b/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java
@@ -112,9 +112,6 @@ public class Dispatcher extends AbstractController {
      */
     static Logger logger = org.geotools.util.logging.Logging.getLogger("org.geoserver.ows");
 
-    /** flag to control wether the dispatcher is cite compliant */
-    boolean citeCompliant = false;
-
     /** thread local variable for the request */
     public static final ThreadLocal<Request> REQUEST = new ThreadLocal<Request>();
     
@@ -130,26 +127,6 @@ public class Dispatcher extends AbstractController {
      */
     List<DispatcherCallback> callbacks = Collections.EMPTY_LIST;
     
-    /**
-     * Sets the flag to control wether the dispatcher is cite compliante.
-     * <p>
-     * If set to <code>true</code>, the dispatcher with throw exceptions when
-     * it encounters something that is not 100% compliant with CITE standards.
-     * An example would be a request which specifies the servce in the context
-     * path: '.../geoserver/wfs?request=...' and not with the kvp '&service=wfs'.
-     * </p>
-     *
-     * @param citeCompliant <code>true</code> to set compliance,
-     *         <code>false</code> to unset it.
-     */
-    public void setCiteCompliant(boolean citeCompliant) {
-        this.citeCompliant = citeCompliant;
-    }
-
-    public boolean isCiteCompliant() {
-        return citeCompliant;
-    }
-    
     @Override
     protected void initApplicationContext(ApplicationContext context) {
         //load life cycle callbacks
@@ -200,24 +177,28 @@ public class Dispatcher extends AbstractController {
         request.setHttpResponse(httpResponse);
 
         Service service = null;
-
         try {
             // initialize the request and allow callbacks to override it
             request = init(request);
+            if(request == null) {
+                return null;
+            }
+            
+            // the service is now determined during the init for proper filtering and
+            // prioritizing of kvp parsers, but we fire the callback now for backwards
+            // compatibility with the DispatcherCallback interface
+            service = request.getServiceBean();
+            if (service == null) {
+                String msg = "No service: ( " + request.getService() + " )";
+                throw new ServiceException(msg, "InvalidParameterValue", "service");    
+            }
+            service = fireServiceDispatchedCallback(request, service);
+            request.setServiceBean(service);
 
             // store it in the thread local
             REQUEST.set(request);
             
-            //find the service
-            try {
-                service = service(request);
-            } catch (Throwable t) {
-                exception(t, null, request);
-
-                return null;
-            }
-            
-            //throw any outstanding errors
+            // throw any outstanding errors
             if (request.getError() != null) {
                 throw request.getError();
             }
@@ -257,9 +238,6 @@ public class Dispatcher extends AbstractController {
         //figure out method
         request.setGet("GET".equalsIgnoreCase(httpRequest.getMethod())
             || "application/x-www-form-urlencoded".equals(httpRequest.getContentType()));
-
-        //create the kvp map
-        parseKVP(request);
         
         if ( !request.isGet() ) { // && httpRequest.getInputStream().available() > 0) {
             //wrap the input stream in a buffered input stream
@@ -316,6 +294,18 @@ public class Dispatcher extends AbstractController {
         request.setContext(context);
         request.setPath(path);
         
+        // initialize the basics
+        try {
+            initServiceVersionRequest(request);
+        } catch (Throwable t) {
+            exception(t, null, request);
+
+            return null;
+        }
+
+        //create the kvp map
+        parseKVP(request);
+        
         return fireInitCallback(request);
     }
 
@@ -353,15 +343,23 @@ public class Dispatcher extends AbstractController {
         return new BufferedReader(reader);
     }
 
-    Service service(Request req) throws Exception {
+    /**
+     * Looks up for the core three features of an OGC request, service, version and request,
+     * and sets them in the request and in the kvp map (for proper filtering and prioritizing
+     * of kvp parsers)
+     * @param req
+     * @throws Exception
+     */
+    void initServiceVersionRequest(Request req) throws Exception {
+        // normalize the kvp maps
+        preParseKVP(req);
+        
         //check kvp
-        if (req.getKvp() != null) {
+        req.setService(normalize((String) req.getKvp().get("service")));
+        req.setVersion(normalize((String) req.getKvp().get("version")));
+        req.setRequest(normalize((String) req.getKvp().get("request")));
+        req.setOutputFormat(normalize((String) req.getKvp().get("outputFormat")));
 
-            req.setService(normalize((String) req.getKvp().get("service")));
-            req.setVersion(normalize((String) req.getKvp().get("version")));
-            req.setRequest(normalize((String) req.getKvp().get("request")));
-            req.setOutputFormat(normalize((String) req.getKvp().get("outputFormat")));
-        } 
         //check the body
         if (req.getInput() != null) {
             Map xml = readOpPost(req.getInput());
@@ -378,9 +376,15 @@ public class Dispatcher extends AbstractController {
                 req.setOutputFormat(normalize((String) xml.get("outputFormat")));    
             }
         }
+        
+        // setup the native service/version before we apply heuristics that might not
+        // be valid in pure cite compliance mode
+        req.nativeService = req.getService();
+        req.nativeVersion = req.getVersion();
+        
 
         //try to infer from context
-        //JD: for cite compliance, a service *must* be specified explicitley by 
+        //JD: for cite compliance, a service *must* be specified explicitly by 
         // either a kvp, or an xml attribute, however in reality the context 
         // is often a good way to infer the service or request 
         String service = req.getService();
@@ -391,7 +395,7 @@ public class Dispatcher extends AbstractController {
             if (service == null) {
                 service = normalize((String) map.get("service"));
 
-                if ((service != null) && !citeCompliant) {
+                if ((service != null) && !req.isCiteCompliant()) {
                     req.setService(service);
                 }
             }
@@ -406,15 +410,15 @@ public class Dispatcher extends AbstractController {
             throw new ServiceException("Could not determine service", "MissingParameterValue",
                 "service");
         }
-
-        //load from teh context
-        Service serviceDescriptor = findService(service, req.getVersion());
-        if (serviceDescriptor == null) {
-            //hack for backwards compatability, try finding the service with the context instead 
+        
+        // lookup the actual service bean (here more version heuristics are going to happen) 
+        Service serviceBean = findService(req.getService(), req.getVersion());
+        if (serviceBean == null) {
+            //hack for backwards compatibility, try finding the service with the context instead 
             // of the service
             if (req.getContext() != null) {
-                serviceDescriptor = findService(req.getContext(), req.getVersion());
-                if (serviceDescriptor != null) {
+                serviceBean = findService(req.getContext(), req.getVersion());
+                if (serviceBean != null) {
                     //found, assume that the client is using <service>/<request>
                     if (req.getRequest() == null) {
                         req.setRequest(req.getService());
@@ -423,12 +427,30 @@ public class Dispatcher extends AbstractController {
                     req.setContext(null);
                 }
             }
-            if (serviceDescriptor == null) {
-                String msg = "No service: ( " + service + " )";
-                throw new ServiceException(msg, "InvalidParameterValue", "service");    
+        }
+        if(serviceBean != null) {
+            req.setServiceBean(serviceBean);
+            if(req.getVersion() == null) {
+                req.setVersion(serviceBean.getVersion().toString());
             }
         }
-        return fireServiceDispatchedCallback(req,serviceDescriptor);
+        
+        // set the information back so that service or version specific kvp parser
+        // can be properly filtered and prioritized
+        if(req.getService() != null) {
+            req.getKvp().put("service", req.getService());
+        }
+        if(req.getRequest() != null) {
+            req.getKvp().put("request", req.getRequest());
+        }
+        // only force in the inferred version if the request is not getcapabilities:
+        // for that one we want the negotiation mechanism to establish the "right" version to use
+        if(req.getVersion() != null && !"getCapabilities".equalsIgnoreCase(req.getRequest())) {
+            req.getKvp().put("version", req.getVersion());
+        }
+        if(req.getOutputFormat() != null) {
+            req.getKvp().put("outputFormat", req.getOutputFormat());
+        }
     }
     
     Service fireServiceDispatchedCallback(Request req, Service service ) {
@@ -560,22 +582,22 @@ public class Dispatcher extends AbstractController {
         //if we are in cite compliant mode, do some additional checks to make
         // sure the "mandatory" parameters are specified, even though we 
         // succesfully dispatched the request.
-        if (citeCompliant) {
+        if (req.isCiteCompliant()) {
             if (!"GetCapabilities".equalsIgnoreCase(req.getRequest())) {
-                if (req.getVersion() == null) {
+                if (req.getNativeVersion() == null) {
                     //must be a version on non-capabilities requests
                     throw new ServiceException("Could not determine version",
                         "MissingParameterValue", "version");
                 } else {
                     //version must be valid
-                    if (!req.getVersion().matches("[0-99].[0-99].[0-99]")) {
+                    if (!req.getNativeVersion().matches("[0-99].[0-99].[0-99]")) {
                         throw new ServiceException("Invalid version: " + req.getVersion(),
                             "InvalidParameterValue", "version");
                     }
 
-                    //make sure the versoin actually exists
+                    //make sure the version actually exists
                     boolean found = false;
-                    Version version = new Version(req.getVersion());
+                    Version version = new Version(req.getNativeVersion());
 
                     for (Iterator s = loadServices().iterator(); s.hasNext();) {
                         Service service = (Service) s.next();
@@ -588,12 +610,12 @@ public class Dispatcher extends AbstractController {
                     }
 
                     if (!found) {
-                        throw new ServiceException("Invalid version: " + req.getVersion(),
+                        throw new ServiceException("Invalid version: " + req.nativeVersion,
                             "InvalidParameterValue", "version");
                     }
                 }
 
-                if (req.getService() == null) {
+                if (req.getNativeService() == null) {
                     //give up 
                     throw new ServiceException("Could not determine service",
                         "MissingParameterValue", "service");
@@ -1130,7 +1152,7 @@ public class Dispatcher extends AbstractController {
         Map kvp = request.getParameterMap();
 
         if (kvp == null || kvp.isEmpty()) {
-            req.setKvp(Collections.EMPTY_MAP);
+            req.setKvp(new HashMap<String, String>());
             //req.kvp = null;
             return;
         }
@@ -1144,8 +1166,6 @@ public class Dispatcher extends AbstractController {
     }
     
     void parseKVP(Request req) throws ServiceException {
-        
-        preParseKVP( req );
         List<Throwable> errors = KvpUtils.parse( req.getKvp() );
         if ( !errors.isEmpty() ) {
             req.setError(errors.get(0));
diff --git a/src/ows/src/main/java/org/geoserver/ows/Request.java b/src/ows/src/main/java/org/geoserver/ows/Request.java
index 20709ea..6f874e6 100644
--- a/src/ows/src/main/java/org/geoserver/ows/Request.java
+++ b/src/ows/src/main/java/org/geoserver/ows/Request.java
@@ -11,6 +11,9 @@ import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.geoserver.platform.Service;
+
+
 /**
  * A collection of the informations collected and parsed by the
  * {...@link Dispatcher} while doing its dispatching work. In case of dispatching
@@ -49,13 +52,20 @@ public class Request {
     protected BufferedReader input;
 
     /**
-     * The ows service,request,version
+     * The ows service,request,version (eventually inferred from heuristics)
      */
     protected String service;
 
     protected String request;
 
     protected String version;
+    
+    /**
+     * The real thing 
+     */
+    protected String nativeService;
+    
+    protected String nativeVersion;
 
     /**
      * Pre context of the url path
@@ -81,6 +91,13 @@ public class Request {
      */
     protected Date timestamp;
     
+    /**
+     * The actual service that we're going to hit
+     */
+    Service serviceBean;
+
+    boolean citeCompliant;
+    
     public Request() {
         timestamp = new Date(); 
     }
@@ -325,4 +342,56 @@ public class Request {
     public void setTimestamp(Date timestamp) {
         this.timestamp = timestamp;
     }
+    
+    /**
+     * The Service that the request will be dispatched to
+     * @return
+     */
+    public Service getServiceBean() {
+        return serviceBean;
+    }
+
+    /**
+     * Sets the service the request will be dispatched to
+     * @param serviceBean
+     */
+    public void setServiceBean(Service serviceBean) {
+        this.serviceBean = serviceBean;
+    }
+    
+    /**
+     * Sets the flag to control wether the dispatcher is cite compliante.
+     * <p>
+     * If set to <code>true</code>, the dispatcher with throw exceptions when
+     * it encounters something that is not 100% compliant with CITE standards.
+     * An example would be a request which specifies the servce in the context
+     * path: '.../geoserver/wfs?request=...' and not with the kvp '&service=wfs'.
+     * </p>
+     *
+     * @param citeCompliant <code>true</code> to set compliance,
+     *         <code>false</code> to unset it.
+     */
+    public void setCiteCompliant(boolean citeCompliant) {
+        this.citeCompliant = citeCompliant;
+    }
+
+    public boolean isCiteCompliant() {
+        return citeCompliant;
+    }
+    
+    public String getNativeService() {
+        return nativeService;
+    }
+
+    public void setNativeService(String nativeService) {
+        this.nativeService = nativeService;
+    }
+
+    public String getNativeVersion() {
+        return nativeVersion;
+    }
+
+    public void setNativeVersion(String nativeVersion) {
+        this.nativeVersion = nativeVersion;
+    }
 }
\ No newline at end of file
diff --git a/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java b/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java
index b41f5c0..ca134c8 100644
--- a/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java
+++ b/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java
@@ -127,6 +127,7 @@ public class DispatcherTest extends TestCase {
         Request req = new Request();
         req.setHttpRequest(request);
 
+        dispatcher.preParseKVP(req);
         dispatcher.parseKVP(req);
 
         Message message = (Message) dispatcher.parseRequestKVP(Message.class, req);
diff --git a/src/wcs1_0/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java b/src/wcs1_0/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
index f8a9fab..e229a22 100644
--- a/src/wcs1_0/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
+++ b/src/wcs1_0/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
@@ -74,7 +74,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
 
     public void testUpdateSequenceEqualsGet() throws Exception {
         Document dom = getAsDOM(BASEPATH
-                + "?request=GetCapabilities&service=WCS&version=1.0.0&updateSequence=0");
+                + "?request=GetCapabilities&service=WCS&version=1.0.0&updateSequence=1");
         // print(dom);
         final Node root = dom.getFirstChild();
         assertEquals("ServiceExceptionReport", root.getNodeName());
@@ -87,7 +87,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
                 + "<wcs:GetCapabilities service=\"WCS\" xmlns:ows=\"http://www.opengis.net/ows/1.1\"";
                 + " xmlns:wcs=\"http://www.opengis.net/wcs\"";
                 + " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"";
-                + " updateSequence=\"0\"/>";
+                + " updateSequence=\"1\"/>";
         Document dom = postAsDOM(BASEPATH, request);
         // print(dom);
         final Node root = dom.getFirstChild();
@@ -98,7 +98,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
 
     public void testUpdateSequenceSuperiorGet() throws Exception {
         Document dom = getAsDOM(BASEPATH
-                + "?request=GetCapabilities&service=WCS&version=1.0.0&updateSequence=1");
+                + "?request=GetCapabilities&service=WCS&version=1.0.0&updateSequence=2");
         // print(dom);
         checkOws11Exception(dom);
     }
@@ -108,7 +108,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
                 + "<wcs:GetCapabilities service=\"WCS\" xmlns:ows=\"http://www.opengis.net/ows/1.1\"";
                 + " xmlns:wcs=\"http://www.opengis.net/wcs\"";
                 + " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"";
-                + " updateSequence=\"1\" version=\"1.0.0\"/>";
+                + " updateSequence=\"2\" version=\"1.0.0\"/>";
         Document dom = postAsDOM(BASEPATH, request);
         checkOws11Exception(dom);
     }
diff --git a/src/wcs1_0/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java b/src/wcs1_0/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
index b1ed5d4..ae8cff8 100644
--- a/src/wcs1_0/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
+++ b/src/wcs1_0/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
@@ -17,6 +17,7 @@ import javax.xml.validation.SchemaFactory;
 import org.custommonkey.xmlunit.SimpleNamespaceContext;
 import org.custommonkey.xmlunit.XMLUnit;
 import org.custommonkey.xmlunit.XpathEngine;
+import org.geoserver.wcs.WCSInfo;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 
@@ -83,6 +84,11 @@ public abstract class WCSTestSupport extends CoverageTestSupport {
         namespaces.put("xlink", "http://www.w3.org/1999/xlink";);
         XMLUnit.setXpathNamespaceContext(new SimpleNamespaceContext(namespaces));
         xpath = XMLUnit.newXpathEngine();
+        
+        // set the service into cite compliance mode
+        WCSInfo info = getGeoServer().getService(WCSInfo.class);
+        info.setCiteCompliant(true);
+        getGeoServer().save(info);
     }
     
     @Override
diff --git a/src/wcs1_1/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java b/src/wcs1_1/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
index 8697e3f..8be2642 100644
--- a/src/wcs1_1/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
+++ b/src/wcs1_1/src/test/java/org/geoserver/wcs/GetCapabilitiesTest.java
@@ -153,7 +153,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
     }
     
     public void testUpdateSequenceEqualsGet() throws Exception {
-        Document dom = getAsDOM(BASEPATH + "?request=GetCapabilities&service=WCS&updateSequence=0");
+        Document dom = getAsDOM(BASEPATH + "?request=GetCapabilities&service=WCS&updateSequence=1");
         checkValidationErrors(dom, WCS11_SCHEMA);
         final Node root = dom.getFirstChild();
         assertEquals("wcs:Capabilities", root.getNodeName());
@@ -165,7 +165,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
                 + "<wcs:GetCapabilities service=\"WCS\" xmlns:ows=\"http://www.opengis.net/ows/1.1\"";
                 + " xmlns:wcs=\"http://www.opengis.net/wcs/1.1.1\"";
                 + " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"";
-                + " updateSequence=\"0\"/>";
+                + " updateSequence=\"1\"/>";
         Document dom = postAsDOM(BASEPATH, request);
         checkValidationErrors(dom, WCS11_SCHEMA);
         final Node root = dom.getFirstChild();
@@ -174,7 +174,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
     }
     
     public void testUpdateSequenceSuperiorGet() throws Exception {
-        Document dom = getAsDOM(BASEPATH + "?request=GetCapabilities&service=WCS&updateSequence=1");
+        Document dom = getAsDOM(BASEPATH + "?request=GetCapabilities&service=WCS&updateSequence=2");
         checkValidationErrors(dom, WCS11_SCHEMA);
 //        print(dom);
         checkOws11Exception(dom);
@@ -185,7 +185,7 @@ public class GetCapabilitiesTest extends WCSTestSupport {
                 + "<wcs:GetCapabilities service=\"WCS\" xmlns:ows=\"http://www.opengis.net/ows/1.1\"";
                 + " xmlns:wcs=\"http://www.opengis.net/wcs/1.1.1\"";
                 + " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"";
-                + " updateSequence=\"1\"/>";
+                + " updateSequence=\"2\"/>";
         Document dom = postAsDOM(BASEPATH, request);
         checkValidationErrors(dom, WCS11_SCHEMA);
 //        print(dom);
diff --git a/src/wcs1_1/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java b/src/wcs1_1/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
index 13b7a0a..d7a704a 100644
--- a/src/wcs1_1/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
+++ b/src/wcs1_1/src/test/java/org/geoserver/wcs/test/WCSTestSupport.java
@@ -64,6 +64,11 @@ public abstract class WCSTestSupport extends CoverageTestSupport {
         namespaces.put("xlink", "http://www.w3.org/1999/xlink";);
         XMLUnit.setXpathNamespaceContext(new SimpleNamespaceContext(namespaces));
         xpath = XMLUnit.newXpathEngine();
+        
+        // set the service into cite compliance mode
+        WCSInfo info = getGeoServer().getService(WCSInfo.class);
+        info.setCiteCompliant(true);
+        getGeoServer().save(info);
     }
     
     @Override
diff --git a/src/wfs/src/main/java/applicationContext.xml b/src/wfs/src/main/java/applicationContext.xml
index 6255b9f..5a06c42 100644
--- a/src/wfs/src/main/java/applicationContext.xml
+++ b/src/wfs/src/main/java/applicationContext.xml
@@ -149,11 +149,6 @@
 		<constructor-arg ref="xmlConfiguration-1.1"/>
 	</bean>
 	
-	<!-- cite compliance hack -->
-	<bean id="citeComplianceHack" class="org.geoserver.wfs.CiteComplianceHack">
-		<constructor-arg ref="geoServer"/>
-	</bean>
-	
 	<!-- test servlet -->
 	<bean id="wfsTestServlet" class="org.springframework.web.servlet.mvc.ServletWrappingController">
 	  <property name="servletClass" value="org.vfny.geoserver.wfs.servlets.TestWfsPost"/>
@@ -165,11 +160,6 @@
         class="org.geoserver.ows.OWSHandlerMapping">
         <constructor-arg ref="catalog"/> 
 		<property name="alwaysUseFullPath" value="true"/>
-		<property name="interceptors">
-			<list>
-				<ref bean="citeComplianceHack"/>
-			</list>
-		</property>
 	
 		<property name="mappings">
 			<props>
diff --git a/src/wfs/src/main/java/org/geoserver/wfs/kvp/BBoxKvpParser.java b/src/wfs/src/main/java/org/geoserver/wfs/kvp/BBoxKvpParser.java
index 5b891fb..f3c568b 100644
--- a/src/wfs/src/main/java/org/geoserver/wfs/kvp/BBoxKvpParser.java
+++ b/src/wfs/src/main/java/org/geoserver/wfs/kvp/BBoxKvpParser.java
@@ -6,20 +6,23 @@ package org.geoserver.wfs.kvp;
 
 import java.util.List;
 
-import org.apache.commons.lang.StringUtils;
 import org.geoserver.ows.KvpParser;
 import org.geoserver.ows.util.KvpUtils;
 import org.geoserver.platform.ServiceException;
-import org.geotools.geometry.jts.ReferencedEnvelope;
-import org.geotools.referencing.CRS;
-import org.opengis.referencing.crs.CoordinateReferenceSystem;
-
-import com.vividsolutions.jts.geom.Envelope;
-
+import org.geotools.factory.CommonFactoryFinder;
+import org.opengis.filter.Filter;
+import org.opengis.filter.FilterFactory;
+import org.opengis.filter.spatial.BBOX;
 
+/**
+ * Parses the BBOX in a WFS GetFeature request to a {...@link BBOX} filter
+ */
 public class BBoxKvpParser extends KvpParser {
+    FilterFactory ff = CommonFactoryFinder.getFilterFactory(null);
+    
     public BBoxKvpParser() {
-        super("bbox", Envelope.class);
+        super("bbox", Filter.class);
+        setService("WFS");
     }
 
     public Object parse(String value) throws Exception {
@@ -60,8 +63,7 @@ public class BBoxKvpParser extends KvpParser {
         }
 
         //check for crs
-        CoordinateReferenceSystem crs = null;
-
+        String srsName = null;
         if (unparsed.size() > 4) {
             // merge back the CRS definition, in case it is an AUTO one
             StringBuilder sb = new StringBuilder();
@@ -71,11 +73,9 @@ public class BBoxKvpParser extends KvpParser {
                     sb.append(",");
                 }
             }
-            crs = CRS.decode(sb.toString());
-        } else {
-            //TODO: use the default crs of the system
-        }
+            srsName = sb.toString();
+        } 
 
-        return new ReferencedEnvelope(minx,maxx,miny,maxy,crs);
+        return ff.bbox("", minx, miny, maxx, maxy, srsName);
     }
 }
diff --git a/src/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java b/src/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java
index dd2c8f9..2f3bfee 100644
--- a/src/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java
+++ b/src/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java
@@ -197,7 +197,7 @@ public class GetFeatureKvpRequestReader extends WFSKvpRequestReader {
             querySet(eObject, "filter", filters);
         } else if (kvp.containsKey("bbox")) {
             //set filter from bbox 
-            Envelope bbox = (Envelope) kvp.get("bbox");
+            BBOX bbox = (BBOX) kvp.get("bbox");
 
             List queries = (List) EMFUtils.get(eObject, "query");
             List filters = new ArrayList();
@@ -212,12 +212,12 @@ public class GetFeatureKvpRequestReader extends WFSKvpRequestReader {
                     List and = new ArrayList(typeName.size());
 
                     for (Iterator t = typeName.iterator(); t.hasNext();) {
-                        and.add(bboxFilter((QName) t.next(), bbox));
+                        and.add(bbox);
                     }
 
                     filter = filterFactory.and(and);
                 } else {
-                    filter = bboxFilter((QName) typeName.get(0), bbox);
+                    filter = bbox;
                 }
 
                 filters.add(filter);
@@ -283,28 +283,6 @@ public class GetFeatureKvpRequestReader extends WFSKvpRequestReader {
         }
     }
 
-    BBOX bboxFilter(QName typeName, Envelope bbox) throws Exception {
-        FeatureTypeInfo featureTypeInfo = 
-            catalog.getFeatureTypeByName(typeName.getNamespaceURI(), typeName.getLocalPart());
-
-        //JD: should this be applied to all geometries?
-        //String name = featureType.getDefaultGeometry().getLocalName();
-        //JD: changing to "" so it is
-        String name = "";
-        
-        //get the epsg code
-        String epsgCode = null;
-        if ( bbox instanceof ReferencedEnvelope ) {
-            CoordinateReferenceSystem crs = ((ReferencedEnvelope)bbox).getCoordinateReferenceSystem();
-            if ( crs != null ) {
-                epsgCode = GML2EncodingUtils.crs(crs);
-            }
-        }
-        
-        return filterFactory.bbox(name, bbox.getMinX(), bbox.getMinY(), bbox.getMaxX(),
-            bbox.getMaxY(), epsgCode);
-    }
-
     protected void querySet(EObject request, String property, List values)
         throws WFSException {
         //no values specified, do nothing
diff --git a/src/wms/src/main/java/applicationContext.xml b/src/wms/src/main/java/applicationContext.xml
index 035a50e..7f34346 100644
--- a/src/wms/src/main/java/applicationContext.xml
+++ b/src/wms/src/main/java/applicationContext.xml
@@ -187,9 +187,10 @@
 		<property name="request" value="GetStyles"/>
 		<property name="service" value="WMS"/>
  	</bean>
-    <bean id="dpiKvpParser" class="org.geoserver.ows.kvp.IntegerKvpParser">
-        <constructor-arg value="dpi"/>
-    </bean>
+  <bean id="dpiKvpParser" class="org.geoserver.ows.kvp.IntegerKvpParser">
+      <constructor-arg value="dpi"/>
+  </bean>
+  <bean id="wmsBboxKvpParser" class="org.geoserver.wms.kvp.BBoxKvpParser"/>
  	
  	
  	<!-- kvp request readers -->
@@ -561,4 +562,6 @@
     <bean id="wmsLifecycleHandler" class="org.geoserver.wms.WMSLifecycleHandler">
       <constructor-arg index="0" ref="dataDirectory"/>
     </bean>
+    
+    <bean id="wmtVerCallback" class="org.geoserver.wms.WmtVerCallback"/>
 </beans>
diff --git a/src/wms/src/main/java/org/geoserver/wms/kvp/BBoxKvpParser.java b/src/wms/src/main/java/org/geoserver/wms/kvp/BBoxKvpParser.java
index 9e778f5..4c96956 100644
--- a/src/wms/src/main/java/org/geoserver/wms/kvp/BBoxKvpParser.java
+++ b/src/wms/src/main/java/org/geoserver/wms/kvp/BBoxKvpParser.java
@@ -21,7 +21,6 @@ import com.vividsolutions.jts.geom.Envelope;
 public class BBoxKvpParser extends KvpParser {
     public BBoxKvpParser() {
         super("bbox", Envelope.class);
-        setService("WMS");
     }
 
     public Object parse(String value) throws Exception {
diff --git a/src/wms/src/test/java/org/geoserver/wms/WMSDisabledTest.java b/src/wms/src/test/java/org/geoserver/wms/WMSDisabledTest.java
index 18f440f..37485db 100644
--- a/src/wms/src/test/java/org/geoserver/wms/WMSDisabledTest.java
+++ b/src/wms/src/test/java/org/geoserver/wms/WMSDisabledTest.java
@@ -18,7 +18,7 @@ public class WMSDisabledTest extends WMSTestSupport {
         getGeoServer().save(wms);
         
         Document doc = getAsDOM("wms?service=WMS&request=getCapabilities");
-        assertEquals("ows:ExceptionReport", doc.getDocumentElement()
+        assertEquals("ServiceExceptionReport", doc.getDocumentElement()
                 .getNodeName());
     }
     
diff --git a/src/wms/src/test/java/org/geoserver/wms/map/GetMapKvpRequestReaderTest.java b/src/wms/src/test/java/org/geoserver/wms/map/GetMapKvpRequestReaderTest.java
index 078d3f7..1ce6c17 100644
--- a/src/wms/src/test/java/org/geoserver/wms/map/GetMapKvpRequestReaderTest.java
+++ b/src/wms/src/test/java/org/geoserver/wms/map/GetMapKvpRequestReaderTest.java
@@ -29,6 +29,7 @@ import org.geoserver.test.ows.KvpRequestReaderTestSupport;
 import org.geoserver.wms.GetMapRequest;
 import org.geoserver.wms.MapLayerInfo;
 import org.geoserver.wms.WMS;
+import org.geoserver.wms.WMSInfo;
 import org.geoserver.wms.kvp.PaletteManager;
 import org.geotools.factory.CommonFactoryFinder;
 import org.geotools.styling.Style;
@@ -61,6 +62,10 @@ public class GetMapKvpRequestReaderTest extends KvpRequestReaderTestSupport {
         gi.getStyles().add(getCatalog().getStyleByName("polygon"));
         cb.calculateLayerGroupBounds(gi);
         getCatalog().add(gi);
+        
+        WMSInfo info = getGeoServer().getService(WMSInfo.class);
+        info.setCiteCompliant(true);
+        getGeoServer().save(info);
     }
 
     protected void setUpInternal() throws Exception {
@@ -349,7 +354,7 @@ public class GetMapKvpRequestReaderTest extends KvpRequestReaderTestSupport {
      * @throws Exception
      */
     public void testWmtVer() throws Exception {
-        dispatcher.setCiteCompliant(true);
+        
         String request = "wms?SERVICE=WMS&&WiDtH=200&FoRmAt=image/png&LaYeRs=cite:Lakes&StYlEs=&BbOx=0,-0.0020,0.0040,0&ReQuEsT=GetMap&HeIgHt=100&SrS=EPSG:4326&WmTvEr=1.1.1";
         assertEquals("image/png", getAsServletResponse(request).getContentType());
     }
------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest
Create new apps & games for the Nokia N8 for consumers in  U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store 
http://p.sf.net/sfu/nokia-dev2dev
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to