Hi all,
Recently i have come across a few extensions for wicket that I figure would be nice but require wicket 1.4. That and since 1.4 seems to be the current stable release I thought I would take a crack at an upgrade. Not the easiest upgrade by a long shot but I have a working setup in my github repo [1].
Before describing what I did I am curious to hear what people think about an upgrade? Is now the right time? Or should we push off to 2.2? Personally i guess i am +0 on the upgrade. On the one hand there are no crucial issues that will be fixed by the upgrade, if anything we will still have issues as i am sure there are cases where keys are being referenced that do not exist in the property file. we just don’t have the test coverage to catch it. Before this patch goes through every single page will have to visited manually… that or we up test coverage to 100% 
But on the other hand 95% of the work is done and if we don’t upgrade now the patch will become outdated, etc…
Anyways, the patch is sizeable so here is the gist of what I did:
- compile errors
There were a wack of compile errors as described here [2]. But the changes were pretty mechanical, mostly changing getModel() and getModelObject() to getDefaultModel() and getDefaultModelObject() respectively
- gzip compression issues
The gzip compression filter had to be updated to deal with the way that wicket sets the content-length header. Or else two content-length headers get encoded and this throws off most web browsers, only firefox was able to cope
- resource lookups
This was most of the work. It seems that wicket is now not so lenient when using wicket:message elements on a page when the key does not actually exist. And we had a number of pages that were referencing keys that do not exist
- customized resource settings
To obtain the single .properties stuff we have to customize the default wicket application resource settings. And have a custom resource loader. This api has changed slightly (well not the api but the wicket internal implementation) so we had to update the custom geoserver resource loader.
- wicket tester
it seems with 1.4 the wicket tester stuff is a bit more strict to referencing components that do not exist. I had to change some of the paths used in test cases for components and when calling submit specifying an invalid id for the submit button
- LayerGroupEditPageTest
This one i was actually not sure about. It seems the test makes some assumptions about how the layer list is structured and perhaps it changed.
— a/web/core/src/test/java/org/geoserver/web/data/layergroup/LayerGroupEditPageTest.java
+++ b/web/core/src/test/java/org/geoserver/web/data/layergroup/LayerGroupEditPageTest.java
@@ -13,8 +13,9 @@ public class LayerGroupEditPageTest extends LayerGroupBaseTest {
tester.assertRenderedPage(LayerGroupEditPage.class);
// remove the first and second elements
tester.clickLink(“form:layers:layers:listContainer:items:1:itemProperties:4:component:link”);
// the regenerated list will have ids starting from 4
- tester.clickLink(“form:layers:layers:listContainer:items:4:itemProperties:4:component:link”);
- //tester.clickLink(“form:layers:layers:listContainer:items:4:itemProperties:4:component:link”);
// manually regenerate bounds
tester.clickLink(“form:generateBounds”);
// print(page, true, true);
Hopefully the author can chime in.
- CoverageStoreEditPageTest,DataAccessEditPageTest
These pages have custom validator on the form that check the data store name to ensure that it (a) gets set and (b) does not already exist in the catalog. There is also the validator that gets implicitly set since the “name” field is required. It seems the order in which they execute is now different and the latter executes first. For example:
— a/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreEditPageTest.java
+++ b/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreEditPageTest.java
@@ -55,7 +57,7 @@ public class CoverageStoreEditPageTest extends GeoServerWicketTestSupport {
tester.clickLink(“rasterStoreForm:save”);
tester.assertRenderedPage(CoverageStoreEditPage.class);
- tester.assertErrorMessages(new String { “Store name is required” });
- tester.assertErrorMessages(new String { “Field ‘Data Source Name’ is required.” });
}
- WebUtils.localize
In this method there was a workaround for a wicket bug reported by Andrea.
https://issues.apache.org/jira/browse/WICKET-1719
I removed the workaround because (a) it should be fixed and (b) StringResourceModel.setLocalizer() has been made final.
That’s it
-Justin
[1] http://github.com/jdeolive/geoserver/commit/775dd700c10e7c98a31b8956468f35d1b80f6c3f
[2] https://cwiki.apache.org/WICKET/migrating-to-wicket-14.html#MigratingtoWicket1.4-Modelchanges
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.