[Geoserver-devel] Two properties that get out of synch between layers and resources: enabled, advertised

GeoServer GeoServer Developer
1

Hi,

in the current code base we have three properties that LayerInfoImpl tries to keep in synch between
the layer and the resource:

  • name
  • enabled
  • advertised

Having them in synch is pretty important, as LayerInfo is used only by WMS, whilst WFS and WCS
use the underlying resource.
Also, the AdvertisedCatalog (which used to be a filter in previous releases) use the advertised
flag of the resources to decide whether to return them.

Unfortunately it seems that any attempt to edit enabled and advertised will result in the data stored
on disk to get inconsistent, with the layer.xml containing what we edited on the GUI, but the
resource xml file (featuretype.xml, wmsLayer.xml, coverage.xml) will have a enabled and advertised
flags that almost seem random, but are never consistent with the layer ones.

Debugged the issue, and tracked down the problem to how we save stuff:

  • the resource get saved, and stored on disk
  • the layer gets saved on disk, then the modification proxy does the commit,
    LayerInfoImpl tries to synch the resource, but it’s too late

Tried to invert the way things are saved, first layer then resource, but that does not change a thing,
since the layer has a direct reference to ResourceInfo (no modification proxy),
while the save subsystem looks at the properties modified via a ModificationProxy.

Then I looked at why name is always in synch instead… and saw it’s because:

  • the name is edited directly against the resource
  • the layer does not have a name attribute, it just reflects the resource one

Well, I guess we could do the same for enabled and advertised…

This is an old bug (at least for the enabled property, it dates back to the layer/resource split),
but it basically make enabled/advertised unusable, so I’d like to fix it in 2.3.x before releasing it.

I’ll come up with a patch shortly, can someone follow up and review?

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.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

–

==
Our support, Your Success! Visit http://opensdi.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

On Tue, Mar 12, 2013 at 7:30 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

I’ll come up with a patch shortly, can someone follow up and review?

Here is a patch:
https://github.com/geoserver/geoserver/pull/187

Key points:

  • enabled and advertised are now treated like name in LayerInfo
  • as a consequence, setEnabled and setAdvertised need a resource to be set before
    being called (which requires some changes in the tests)
  • advertised and enabled have been moved to the basic resource panel info
  • the BasicLayerConfig panel remained with no editable items. It had name (read only)
    advertised and enabledl which are no gone… so I just removed it

Tried to think about making additional tests, but the structure changed so it’s not possible
to make a test that would fail anymore.

In any case, the patch is large and has a GUI change, so I’d commit it this one on
trunk only.

For 2.3.x and 2.2.x I’d go for the cheesy solution instead: modify the GUI code so that
it copies over the two flags from layer to resource before saving.
Two ugly lines of code and done, but they should be pretty safe.

Opinions?

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.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

3

On Tue, Mar 12, 2013 at 7:37 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

For 2.3.x and 2.2.x I’d go for the cheesy solution instead: modify the GUI code so that

it copies over the two flags from layer to resource before saving.
Two ugly lines of code and done, but they should be pretty safe.

Here are the ugly lines:
https://github.com/aaime/geoserver/commit/eeb08b8c540985be9f10ca3562a0ad069868903e

Tested that this works interactively.
Theoretically this one can be tested also with a junit test… but I did not manage to make
the wicket tester switch tabs in order to check the enabled/advertised checkboxes…

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.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

On Tue, Mar 12, 2013 at 8:06 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Here are the ugly lines:

https://github.com/aaime/geoserver/commit/eeb08b8c540985be9f10ca3562a0ad069868903e

Tested that this works interactively.
Theoretically this one can be tested also with a junit test… but I did not manage to make
the wicket tester switch tabs in order to check the enabled/advertised checkboxes…

Unless I hear otherwise I’m going to merge these patches tomorrow… but I would still appreciate
if someone else had a look at them

Cheers
andrea

–

==
Our support, Your Success! Visit http://opensdi.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

5

Patch looks ok to me. No issue with committing it as is.

Hopefully at some point we can move forward with moving all publishing stuff to LayerInfo and avoid having to jump through hoops like this.

···

On Wed, Mar 13, 2013 at 5:48 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Mar 12, 2013 at 8:06 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Here are the ugly lines:

https://github.com/aaime/geoserver/commit/eeb08b8c540985be9f10ca3562a0ad069868903e

Tested that this works interactively.
Theoretically this one can be tested also with a junit test… but I did not manage to make
the wicket tester switch tabs in order to check the enabled/advertised checkboxes…

Unless I hear otherwise I’m going to merge these patches tomorrow… but I would still appreciate
if someone else had a look at them

Cheers

andrea

–

==
Our support, Your Success! Visit http://opensdi.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

Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar

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.

6

On Wed, Mar 13, 2013 at 4:04 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Patch looks ok to me. No issue with committing it as is.

Thank you :slight_smile:

Hopefully at some point we can move forward with moving all publishing stuff to LayerInfo and avoid having to jump through hoops like this.

Eh… to get there we’d have to rewrite all the WFS and WCS code to use LayerInto instead of FeatureTypeInfo and CoverageInfo.
Not quick I’m afraid. Plus, the advertised catalog and the like should stop checking ResourceInfo, or find the corresponding
LayerInfo to decide whether to return a certain resource.
Sounds like quite the sizable amount of work.

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.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

7

On Wed, Mar 13, 2013 at 9:52 AM, Andrea Aime
<andrea.aime@anonymised.com>wrote:

On Wed, Mar 13, 2013 at 4:04 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Patch looks ok to me. No issue with committing it as is.

Thank you :slight_smile:

Hopefully at some point we can move forward with moving all publishing
stuff to LayerInfo and avoid having to jump through hoops like this.

Eh.. to get there we'd have to rewrite all the WFS and WCS code to use
LayerInto instead of FeatureTypeInfo and CoverageInfo.
Not quick I'm afraid. Plus, the advertised catalog and the like should
stop checking ResourceInfo, or find the corresponding
LayerInfo to decide whether to return a certain resource.
Sounds like quite the sizable amount of work.

For sure, wasn't implying that it should be done now or anything. Something
that probably can't be done until a major version upgrade, Geoserver 3.0
perhaps.

Cheers
Andrea

--

Our support, Your Success! Visit http://opensdi.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

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

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