[Geoserver-devel] WMS 1.3 cascading broken, how to test so that it does not happen again

GeoServer GeoServer Developer
1

Hi,
today I’ve discovered that WMS 1.3 cascading got broken in GeoServer as a result of Jody’s patch relative to GEOT-4283.

Basically GeoServer relied on some higher level checks for WMS 1.3, while Jody did similar checks and patches right inside the WMS classes.
What happens is, the two flipping logics trigger at the same time and they flip the axis twice… you may imagine the results.

Now, I did add some tests back in GeoTools, but they did not trigger Jody’s flipping logic due to a CRS setting, so the change broke GeoServer without anybody noticing.

Now, I’m working on stripping out the code that GeoServer relied onto, Jody’s approach is cleaner, but I also want to add some end to end tests so that we are not going to get bitten by this anymore.

Doing such kind of test requires to have some test server though, which can be obtained:

a) by using a fixed remote server
b) by standing up a local server during the test
c) by creating a WMSStore that uses a mock http client of our choice, that returns fixed responses

Ideally I’d like c), however I don’t see a way to get there without hacking ResourcePool in some ugly way…

a) is used in some places already, however those tests are not run in a normal build, nor on Hudson, so in the end not such a good thing.

b) is interesting, and there are some nice libraries to setup such kind of tests, one that looks particularly nice is http://wiremock.org/ , and https://github.com/NanoHttpd/nanohttpd looks nice too.

Wiremock is really born to perform testing, has a nice fluent api to build mocks and expectations, and can be also used to record and replay conversations with another server. Downside, it’s large-ish (has quite a bit of dependencies, or comes with a 5MB standalone version), I don’t see a mailing list for the project, and some of its dependencies are XML related, which worries me a bit (we already had issues with geotools xml parsers/encoders when adding extra xml related stuff in the classpath).

Nanohttp is nice in that it’s really small (under 30KB), however it does not seem to be available on maven repos (could not find it at least, but not the end of the world), and by the docs only supports GET/POST with a bit of PUT, so we could not use it in the future if we want to test communications with a remote RESTful service (something we don’t need now, but…)

An issue with tests that do open a HTTP port is that the port could be busy, and the test need to be hardened against that, with some test and retry logic (e.g, try 100 random ports above 8080, fail if they are all busy). This does not seem to hard to code though, and in fact a server socket can be bound to port 0 to make it found a free port by itself… managed to patch Nanohttp in 5 minutes to support that and return the port it actually bound to.

Anyways… suggestions?

Cheers
Andrea

–

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

2

I agree mocking would be the ideal approach. Can you be more specific about the issues in resourcepool? Imo an ugly hack, as long as its not spread out in too many places for testing purposes , would be worth it to avoid adding more moving parts to the build.

···

On Sun, May 19, 2013 at 9:56 AM, Andrea Aime <andrea.aime@anonymised.com…> wrote:

Hi,
today I’ve discovered that WMS 1.3 cascading got broken in GeoServer as a result of Jody’s patch relative to GEOT-4283.

Basically GeoServer relied on some higher level checks for WMS 1.3, while Jody did similar checks and patches right inside the WMS classes.
What happens is, the two flipping logics trigger at the same time and they flip the axis twice… you may imagine the results.

Now, I did add some tests back in GeoTools, but they did not trigger Jody’s flipping logic due to a CRS setting, so the change broke GeoServer without anybody noticing.

Now, I’m working on stripping out the code that GeoServer relied onto, Jody’s approach is cleaner, but I also want to add some end to end tests so that we are not going to get bitten by this anymore.

Doing such kind of test requires to have some test server though, which can be obtained:

a) by using a fixed remote server
b) by standing up a local server during the test
c) by creating a WMSStore that uses a mock http client of our choice, that returns fixed responses

Ideally I’d like c), however I don’t see a way to get there without hacking ResourcePool in some ugly way…

a) is used in some places already, however those tests are not run in a normal build, nor on Hudson, so in the end not such a good thing.

b) is interesting, and there are some nice libraries to setup such kind of tests, one that looks particularly nice is http://wiremock.org/ , and https://github.com/NanoHttpd/nanohttpd looks nice too.

Wiremock is really born to perform testing, has a nice fluent api to build mocks and expectations, and can be also used to record and replay conversations with another server. Downside, it’s large-ish (has quite a bit of dependencies, or comes with a 5MB standalone version), I don’t see a mailing list for the project, and some of its dependencies are XML related, which worries me a bit (we already had issues with geotools xml parsers/encoders when adding extra xml related stuff in the classpath).

Nanohttp is nice in that it’s really small (under 30KB), however it does not seem to be available on maven repos (could not find it at least, but not the end of the world), and by the docs only supports GET/POST with a bit of PUT, so we could not use it in the future if we want to test communications with a remote RESTful service (something we don’t need now, but…)

An issue with tests that do open a HTTP port is that the port could be busy, and the test need to be hardened against that, with some test and retry logic (e.g, try 100 random ports above 8080, fail if they are all busy). This does not seem to hard to code though, and in fact a server socket can be bound to port 0 to make it found a free port by itself… managed to patch Nanohttp in 5 minutes to support that and return the port it actually bound to.

Anyways… suggestions?

Cheers
Andrea

–

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

3

On Tue, May 21, 2013 at 2:34 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

I agree mocking would be the ideal approach. Can you be more specific
about the issues in resourcepool? Imo an ugly hack, as long as its not
spread out in too many places for testing purposes , would be worth it to
avoid adding more moving parts to the build.

Ok, followed the suggestion and had a second look to ResourcePool and the
classes that are talking to external http services,
in particular WebMapServer and the WFS store.
In both cases there is an option to ingest a user provided http client, so
we can inject a mock driven one like the MockHttpClient
that we have in GeoTools in the gt-wfs module.

Here is how I believe things could be setup. In order to identify that we
actually want to talk to a mock client
we could use some special URLs, for example, anything starting with
http://mockserver/… would automatically
be wired up with a mock http client.

Now, the other issue is how to setup the mock http client, some non trivial
tests would need more than one.
For this case, I guess we can use the path in the url to make a different,
eg.., mock1 bound to http://mockserver/mock1/…
and mock2 bound to http://mockserver/mock2.
There would be a MockHttpFactory that has static methods to bind the mock
http client to a path and retrieve them,
e.g.:

MockHttpFactory {

  public static void bind(HttpClient client, String path);

  public HttpClient get(String path);

  public void clear();

}

The clear() method would remove all bindings to inadvertent cross-test
contamination, and I would be tempted
to call it from GeoServerBaseTest automatically in an @After method.

The tests would be then free to setup their mocks. Having one implementtion
with a WireMock like stubbing API would
be nice of course (http://wiremock.org/stubbing.html) but I'm not
considering writing one right away.

Opinions?

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

4

Works for me.

···

On Sat, May 25, 2013 at 3:54 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, May 21, 2013 at 2:34 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

I agree mocking would be the ideal approach. Can you be more specific about the issues in resourcepool? Imo an ugly hack, as long as its not spread out in too many places for testing purposes , would be worth it to avoid adding more moving parts to the build.

Ok, followed the suggestion and had a second look to ResourcePool and the classes that are talking to external http services,
in particular WebMapServer and the WFS store.
In both cases there is an option to ingest a user provided http client, so we can inject a mock driven one like the MockHttpClient
that we have in GeoTools in the gt-wfs module.

Here is how I believe things could be setup. In order to identify that we actually want to talk to a mock client
we could use some special URLs, for example, anything starting with http://mockserver/.… would automatically
be wired up with a mock http client.

Now, the other issue is how to setup the mock http client, some non trivial tests would need more than one.
For this case, I guess we can use the path in the url to make a different, eg…, mock1 bound to http://mockserver/mock1/.…
and mock2 bound to http://mockserver/mock2.
There would be a MockHttpFactory that has static methods to bind the mock http client to a path and retrieve them,
e.g.:

MockHttpFactory {

public static void bind(HttpClient client, String path);

public HttpClient get(String path);

public void clear();

}

–

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

The clear() method would remove all bindings to inadvertent cross-test contamination, and I would be tempted
to call it from GeoServerBaseTest automatically in an @After method.

The tests would be then free to setup their mocks. Having one implementtion with a WireMock like stubbing API would
be nice of course (http://wiremock.org/stubbing.html) but I’m not considering writing one right away.

Opinions?

Cheers
Andrea

5

Hi all,
followed this trail and created a simple http client mocking for wms
testing, also added tests for cascading GetMap
(still have to add tests for building the wms layer, the changes in
GEOT-4283 also broke the code that sets up the bounds).

https://github.com/geoserver/geoserver/pull/246

What do you think?
Suitability for a backport on 2.3.x?
Or maybe we should just backport the fixes, but not backport the tests and
the testing framework changes?

Cheers
Andrea

6

Looks good to me. No problem back porting to 2.3.x (fix + tests) for me.

···

On Sun, Jun 2, 2013 at 11:59 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Hi all,
followed this trail and created a simple http client mocking for wms testing, also added tests for cascading GetMap
(still have to add tests for building the wms layer, the changes in GEOT-4283 also broke the code that sets up the bounds).

https://github.com/geoserver/geoserver/pull/246

What do you think?
Suitability for a backport on 2.3.x?
Or maybe we should just backport the fixes, but not backport the tests and the testing framework changes?

Cheers
Andrea

7

Glad the testing storying has come together.
I am curious if the code that sets up the bounds is open to the ReferencedEnvelope3D trouble (it BBOX with a 3D CRS)

–
Jody Garnett

On Monday, 3 June 2013 at 3:59 AM, Andrea Aime wrote:

Hi all,
followed this trail and created a simple http client mocking for wms testing, also added tests for cascading GetMap
(still have to add tests for building the wms layer, the changes in GEOT-4283 also broke the code that sets up the bounds).

https://github.com/geoserver/geoserver/pull/246

What do you think?
Suitability for a backport on 2.3.x?
Or maybe we should just backport the fixes, but not backport the tests and the testing framework changes?

Cheers
Andrea

Get 100% visibility into Java/.NET code with AppDynamics Lite
It’s a free troubleshooting tool designed for production
Get down to code-level detail for bottlenecks, with <2% overhead.
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap2

Geoserver-devel mailing list
Geoserver-devel@anonymised.comts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

8

On Sun, Jun 9, 2013 at 1:20 PM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Glad the testing storying has come together.
I am curious if the code that sets up the bounds is open to the
ReferencedEnvelope3D trouble (it BBOX with a 3D CRS)

As far as I know WMS has no notion of 3D anything so... why bother?
(elevation might be thought as a way to add
the 3rd dimension, but it does not affect the bounding boxes).

WFS on the other side... but it's a datastore, I guess if the code works
with Oracle and PostGIS 3d boxes, it should
work with an eventual WF one (my bet: the WFS data store code is not ready
to handle 3D bounds in WFS caps
document to start with, I guess an analysis might have to start there).

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------