Hi Andrea, thank you for the great feedback!
As I am very fresh with wickets and geoserver internals this was basically my first real patch, so your comments mean a lot.
In the meanwhile I also noticed some bugs with the patch, and Ive already fixed some. But some of them I cant reproduce:
If one layer had been published manually already it will fail
I cannot reproduce this behaviour, as I check if layer is allready published and then I update it (at least now its like that, but I guess It should be configurable to skip/reconfigure layer)
WMSLayerImporterPage line #133
if (exists != null) {
// TODO what to do if layer allready exists?
// continue;
builder.updateWMSLayer(exists, wli);
If a layer with the same name exists it will fail too
Again, it will only update the layer with same name. But I agree, we have to handle somehow layers with same name. Problem here is that we strip namespace from original layer, so we lost that info in the process. Should we maybe keep the original namespace somehow, but not expose it? Or renaming the layer?
It seems to stop at the first issue instead of carrying on and providing a report of failures later,
and there is no report of what happened. Better continue and have a report page at the end.
Indeed, thats how I imagined it also. But I never had any layer fail until now so I didnt see this behaviour
No option in the "add layer" page that we get right past the "new store" one
I also noticed this and Im fixing it already. It is the same case with createSQLViewContainer and createTypeContainer elements inside NewLayerPage. This happens because its state is only changing on DropDownChoice onUpdate event. We have to modify this so the change occurs in NewLayerPage constructor also.
Trying to sort on "published" does not work
Switching between pages does not work either (this might require some patch to the geoserver table panel)
I dont have problems with this, its working fine for me.
I do have problems only when I first use filter, and after that select layer by checkbox. Although checkboxes are checked, my selection comes out empty (but thats some wicket issue, ill figure it out)
Andrea, do you maybe have some error infos about these previous issues? Id help a lot to see why is this happening to you.
When the process succeeds it imports anyways just the layers in the first page (try to cascade demo.opengeo.org that has more than 25 layers)
Indeed this is the behaviour now. But I have tested it before with 700+ layers and it imported without error. I might have messed up something when I changed custom Resource for one in org.geoserver.web.data.layer. I will investigate why is this happening.
separate page with a summary & progress indicator
This functionality would be indeed desirable, but as this is already inside geosuite importer, Id wait for it and implement it for MWSCascade once importer is in geoserver.
Thank you again and regards,
Ivan
Hi Ivan, the patch is headed in the right direction but I think it might need some more work:
and there is no report of what happened. Better continue and have a report page at the end.
demo.opengeo.org that has more than 25 layers)
page with a summary of the import process result.
ask Simone Giannecchini to get a private reference to a WMS server that has that many layers.
You can try out this one that has 1000: http://giswebservices.massgis.state.ma.us/geoserver/web/?wicket:bookmarkablePage=:org.geoserver.web.demo.MapPreviewPage
(btw, before Justin starts wondering about me providing a ton of suggestions/extra work onto people providing patches, Ivan is working with us
)
One final note, I don't see any test, some would be needed. I guess you can build some tests around the importer by manually configuring a WMS store, which
you can see in src/wms/src/test/java/org/geoserver/wms/ResourceAccessManagerWMSTest.java, and then use the wicket tester machinery to try and make the workflow
go through the various steps (hint, to figure out a compontent full path just have the page you're testing be printed using GeoServerWicketTestSupport.print(component, false, true)
Generally speaking in the long term I would like to have something like this be the main import interface, but I guess we'll have to wait for the OpenGeo importer work to be folded into the main path before doing that.