[Geoserver-devel] Sanitizing catalog objects during resolve

GeoServer GeoServer Developer
1

Hi,
I was looking into this one
http://jira.codehaus.org/browse/GEOS-6320

and it seems to me the issue can be caused by a group whose layers have
been removed from the configuration manually (e.g., deleted from disk): if you reload
the configuration after that, the layer references are ResovingProxy, and they will
return a null if the layer they point to is not more there, causing the above configuration
to be saved.

Now, my first take on this one is to sanitize the layer list in the “resolve” method, and if
missing, clear them:
https://github.com/geoserver/geoserver/pull/477

This should fix the issue if the layer group has been already saved on disk like Mauro
reported, but I guess it won’t help if the ResolvingProxy is still around…
I guess a different take would be to modify resolve(LayerGroupInfo layerGroup)
in DefaultCatalogFacade, but this would make it facade dependent (e.g., this would not
work in the jdbc config)

Or a separate method in CatalogImpl could be used to sanitize these configuration issues,
to be used after the object has been added to the facade.

Mind, this is not the only issue we have with dangling references, this one is also
a headache, which is caused by layers that have lost the default style they referenced to.
http://jira.codehaus.org/browse/GEOS-6318

So… it would be nice to have some general mechanism to handle these dangling references
in a way that still allows GeoServer to start up, and the config objects in question to
be fixed enough to make them usable in the config UI.

Suggestions?

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

2

On Sat, Feb 8, 2014 at 4:25 PM, Andrea Aime <andrea.aime@anonymised.com>wrote:

Mind, this is not the only issue we have with dangling references, this
one is also
a headache, which is caused by layers that have lost the default style
they referenced to.
http://jira.codehaus.org/browse/GEOS-6318

And this one is probably relevant too, despite its title:
http://jira.codehaus.org/browse/GEOS-5893

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 Sat, Feb 8, 2014 at 5:04 PM, Andrea Aime <andrea.aime@anonymised.com>wrote:

On Sat, Feb 8, 2014 at 4:25 PM, Andrea Aime <andrea.aime@anonymised.com>wrote:

Mind, this is not the only issue we have with dangling references, this
one is also
a headache, which is caused by layers that have lost the default style
they referenced to.
http://jira.codehaus.org/browse/GEOS-6318

And this one is probably relevant too, despite its title:
http://jira.codehaus.org/browse/GEOS-5893

And this one too, actually caused by a layer whose default style is null:
http://jira.codehaus.org/browse/GEOS-6106

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

A couple of thoughts.

  • Agree that ideally whatever we do lives in CatalogImpl so it can be utilized by all facade implementations

  • I wonder if the Catalog validate methods are a better place for this check? At the moment when adding a new object to the catalog validate is called before resolve. I can’t think off hand of any reason why we can’t reverse the order but i am not sure.

  • If any of the checks are going to be expensive (thinking about large catalogs here) then we may want to consider only enabling these referential integrity checks on startup, or when the catalog is being reloaded and forgo them during normal operation.

···

On Sat, Feb 8, 2014 at 10:44 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk

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

–

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

On Sat, Feb 8, 2014 at 5:04 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

And this one too, actually caused by a layer whose default style is null:
http://jira.codehaus.org/browse/GEOS-6106

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

On Sat, Feb 8, 2014 at 4:25 PM, Andrea Aime <andrea.aime@anonymised.com…> wrote:

Mind, this is not the only issue we have with dangling references, this one is also

a headache, which is caused by layers that have lost the default style they referenced to.
http://jira.codehaus.org/browse/GEOS-6318

And this one is probably relevant too, despite its title:
http://jira.codehaus.org/browse/GEOS-5893

5

On Mon, Feb 10, 2014 at 4:53 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

A couple of thoughts.

- Agree that ideally whatever we do lives in CatalogImpl so it can be
utilized by all facade implementations

- I wonder if the Catalog validate methods are a better place for this
check? At the moment when adding a new object to the catalog validate is
called before resolve. I can't think off hand of any reason why we can't
reverse the order but i am not sure.

The thing is, I believe I need the resolving proxy out of the way before I
can do the checks. So yeah, that could be done, but we need to reverse the
calls to validate and resolve.
Actually... I believe calling resolve first is a good idea, since
resolutions to null can trigger validation checks.

- If any of the checks are going to be expensive (thinking about large
catalogs here) then we may want to consider only enabling these referential
integrity checks on startup, or when the catalog is being reloaded and
forgo them during normal operation.

Eh, I'm not sure about their cost. In both cases it's a matter of scanning
the contents of a single layer, or sigle layer group, and see
if any of the pointers is dangling. Is that expensive in JDBC config?

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

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

6

On Tue, Feb 11, 2014 at 3:17 AM, Andrea Aime
<andrea.aime@anonymised.com>wrote:

On Mon, Feb 10, 2014 at 4:53 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

A couple of thoughts.

- Agree that ideally whatever we do lives in CatalogImpl so it can be
utilized by all facade implementations

- I wonder if the Catalog validate methods are a better place for this
check? At the moment when adding a new object to the catalog validate is
called before resolve. I can't think off hand of any reason why we can't
reverse the order but i am not sure.

The thing is, I believe I need the resolving proxy out of the way before I
can do the checks. So yeah, that could be done, but we need to reverse the
calls to validate and resolve.
Actually... I believe calling resolve first is a good idea, since
resolutions to null can trigger validation checks.

Hmmm... right.

- If any of the checks are going to be expensive (thinking about large
catalogs here) then we may want to consider only enabling these referential
integrity checks on startup, or when the catalog is being reloaded and
forgo them during normal operation.

Eh, I'm not sure about their cost. In both cases it's a matter of scanning
the contents of a single layer, or sigle layer group, and see
if any of the pointers is dangling. Is that expensive in JDBC config?

Should be fine as long as it doesn't trigger a full scan, which it doesn't
sound like it will.

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*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

7

Hi,
so I’ve reversed validate and resolve, so that resolve is done first, and resolve
also fixes resource references problems (at least for layer groups and styles).

Full build works, here is the full patch:
https://github.com/geoserver/geoserver/pull/477

Ok to merge before the RC?

Cheers
Andrea

···

On Tue, Feb 11, 2014 at 4:23 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

–

== 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

On Tue, Feb 11, 2014 at 3:17 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hmmm… right.

Should be fine as long as it doesn’t trigger a full scan, which it doesn’t sound like it will.

–

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

On Mon, Feb 10, 2014 at 4:53 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

A couple of thoughts.

  • Agree that ideally whatever we do lives in CatalogImpl so it can be utilized by all facade implementations

  • I wonder if the Catalog validate methods are a better place for this check? At the moment when adding a new object to the catalog validate is called before resolve. I can’t think off hand of any reason why we can’t reverse the order but i am not sure.

The thing is, I believe I need the resolving proxy out of the way before I can do the checks. So yeah, that could be done, but we need to reverse the calls to validate and resolve.
Actually… I believe calling resolve first is a good idea, since resolutions to null can trigger validation checks.

  • If any of the checks are going to be expensive (thinking about large catalogs here) then we may want to consider only enabling these referential integrity checks on startup, or when the catalog is being reloaded and forgo them during normal operation.

Eh, I’m not sure about their cost. In both cases it’s a matter of scanning the contents of a single layer, or sigle layer group, and see
if any of the pointers is dangling. Is that expensive in JDBC config?

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

8

Not sure if I missed the deadline but no objection here.

···

On Sat, Feb 15, 2014 at 4:24 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
so I’ve reversed validate and resolve, so that resolve is done first, and resolve
also fixes resource references problems (at least for layer groups and styles).

Full build works, here is the full patch:
https://github.com/geoserver/geoserver/pull/477

Ok to merge before the RC?

Cheers

Andrea

–

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

On Tue, Feb 11, 2014 at 4:23 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

–

== 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

On Tue, Feb 11, 2014 at 3:17 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hmmm… right.

Should be fine as long as it doesn’t trigger a full scan, which it doesn’t sound like it will.

–

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

On Mon, Feb 10, 2014 at 4:53 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

A couple of thoughts.

  • Agree that ideally whatever we do lives in CatalogImpl so it can be utilized by all facade implementations

  • I wonder if the Catalog validate methods are a better place for this check? At the moment when adding a new object to the catalog validate is called before resolve. I can’t think off hand of any reason why we can’t reverse the order but i am not sure.

The thing is, I believe I need the resolving proxy out of the way before I can do the checks. So yeah, that could be done, but we need to reverse the calls to validate and resolve.
Actually… I believe calling resolve first is a good idea, since resolutions to null can trigger validation checks.

  • If any of the checks are going to be expensive (thinking about large catalogs here) then we may want to consider only enabling these referential integrity checks on startup, or when the catalog is being reloaded and forgo them during normal operation.

Eh, I’m not sure about their cost. In both cases it’s a matter of scanning the contents of a single layer, or sigle layer group, and see
if any of the pointers is dangling. Is that expensive in JDBC config?

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

9

On Tue, Feb 18, 2014 at 5:02 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Not sure if I missed the deadline but no objection here.

Hmm... well... it seems Jody still has not built 2.5-RC1, I guess I can
merge now and
cross fingers?

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

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

10

On Tue, Feb 18, 2014 at 5:05 PM, Andrea Aime
<andrea.aime@anonymised.com>wrote:

On Tue, Feb 18, 2014 at 5:02 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Not sure if I missed the deadline but no objection here.

Hmm... well... it seems Jody still has not built 2.5-RC1, I guess I can
merge now and
cross fingers?

Getting late here, so I just went ahead and merged it.
Jody, your call about the release, if you feel the changes are risky just
cut the build and
2.5.x from revision 1aa2e57ab6878d5d04f05d8c5711cbe2f142e31a (
https://github.com/geoserver/geoserver/commit/1aa2e57ab6878d5d04f05d8c5711cbe2f142e31a
),
the last commit before this pull request merge

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

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

11

I will give it a go, sorry the geotools release is held up on release script failures.

···

On Tue, Feb 18, 2014 at 5:05 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Getting late here, so I just went ahead and merged it.
Jody, your call about the release, if you feel the changes are risky just cut the build and
2.5.x from revision 1aa2e57ab6878d5d04f05d8c5711cbe2f142e31a (https://github.com/geoserver/geoserver/commit/1aa2e57ab6878d5d04f05d8c5711cbe2f142e31a),
the last commit before this pull request merge

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

On Tue, Feb 18, 2014 at 5:02 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Not sure if I missed the deadline but no objection here.

Hmm… well… it seems Jody still has not built 2.5-RC1, I guess I can merge now and
cross fingers?