[Geoserver-devel] Catalog validation, UI validation, and duplicate names

GeoServer GeoServer Developer
1

Hi,
I'm a bit in a bind for a number of issues related to resource
validation and resource names.

Say I'm adding again an existing dataset, without aliasing it,
and I press submit to add the "clone" [1]. What I get is, rightfully,
the following stack trace:

java.lang.IllegalArgumentException: Resource named 'mosaic' already exists in store: 'mosaic'
      at org.geoserver.catalog.impl.CatalogImpl.validate(CatalogImpl.java:349)
      at org.geoserver.catalog.impl.CatalogImpl.add(CatalogImpl.java:328)
      at org.geoserver.security.SecureCatalogImpl.add(SecureCatalogImpl.java:845)
      at org.geoserver.web.data.ResourceConfigurationPage$4.onSubmit(ResourceConfigurationPage.java:115)

Ok. So I want to add a validation in the UI that prevents this.
Mumble... how to do this? The validation will trigger before the
resources are updated, so in the validator I have in my hands just
the old state of the ResourceInfo, and the new name.
I would like to call Catalog.validate(resource) to avoid duplicating
the validation logic, but to do so I'd need to clone the resource first
and force the new name onto it. Is there any facility to duplicate
a ResourceInfo (maybe I could just make a method that serializes and
un-serializes it, this would effectively deep clone it).

And even if I do, the validator will have to return a proper error
message... say tomorrow you add a new validation, how do I associate
the catalog validation exception with a meaningful message (eventually
translated?). It seems that if I want to have i18n error messages
I have to replicate the validation logic anyways?

While looking into CatalogImpl.validate(ResourceInfo) I also noticed
the name validation occurs only if the layer is new... which does
not look like a good idea, as people can change the name afterwards
as well, and make it collide with another layer. The code should
check if the catalog already contains a resource with the same name
in the same workspace, and pass only if none is found, or if the
resource found is different from the one at hand (which requires
some id based comparison, with an id different than the name)...

Sooo... suggestions?

Cheers
Andrea

[1]: for those that do not know, since 1.6.x it's possible to add the
      same shapefile/table multiple times, and have it act as a different
      layer, with different styles and parameters, provided each
      "instance" is given a different "alias"

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

2

Andrea Aime wrote:

Hi,
I'm a bit in a bind for a number of issues related to resource
validation and resource names.

Say I'm adding again an existing dataset, without aliasing it,
and I press submit to add the "clone" [1]. What I get is, rightfully,
the following stack trace:

java.lang.IllegalArgumentException: Resource named 'mosaic' already exists in store: 'mosaic'
      at org.geoserver.catalog.impl.CatalogImpl.validate(CatalogImpl.java:349)
      at org.geoserver.catalog.impl.CatalogImpl.add(CatalogImpl.java:328)
      at org.geoserver.security.SecureCatalogImpl.add(SecureCatalogImpl.java:845)
      at org.geoserver.web.data.ResourceConfigurationPage$4.onSubmit(ResourceConfigurationPage.java:115)

Ok. So I want to add a validation in the UI that prevents this.
Mumble... how to do this? The validation will trigger before the
resources are updated, so in the validator I have in my hands just
the old state of the ResourceInfo, and the new name.
I would like to call Catalog.validate(resource) to avoid duplicating
the validation logic, but to do so I'd need to clone the resource first
and force the new name onto it. Is there any facility to duplicate
a ResourceInfo (maybe I could just make a method that serializes and
un-serializes it, this would effectively deep clone it).

There are methods in CatalogBuilder of the form update* which essentially creates a clone. It might work in this case.

This is a tough issue. The make a copy and validate the copy seems hacky to me... and relies on how the in memory catalog is implemented internally. Also the validate() methods are not public api, which means we would have ot make them public api, which raises the bar to implement the Catalog interface (not an objection, just an observation).

I think it makes more sense to try to make the change, and catch the exception, but the problem (as you noted) is that we have no way to i18nize those exception messages. More on this below.

And even if I do, the validator will have to return a proper error
message... say tomorrow you add a new validation, how do I associate
the catalog validation exception with a meaningful message (eventually
translated?). It seems that if I want to have i18n error messages
I have to replicate the validation logic anyways?

This problem is not really constrained to the catalog imo. Any code the UI calls runs the risk of throwing a runtime exception that needs to be reported somehow. What are the thoughts about trying to come up with exceptions which the UI can i18nize. Either coming up with a special exception class that we can associate a i18n key with, or perhaps just throwing exceptions in which the message itself is treated as the key.

While looking into CatalogImpl.validate(ResourceInfo) I also noticed
the name validation occurs only if the layer is new... which does
not look like a good idea, as people can change the name afterwards
as well, and make it collide with another layer. The code should
check if the catalog already contains a resource with the same name
in the same workspace, and pass only if none is found, or if the
resource found is different from the one at hand (which requires
some id based comparison, with an id different than the name)...

Yup, makes sense, this is a bug.

Sooo... suggestions?

Cheers
Andrea

[1]: for those that do not know, since 1.6.x it's possible to add the
      same shapefile/table multiple times, and have it act as a different
      layer, with different styles and parameters, provided each
      "instance" is given a different "alias"

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

3

Andrea Aime wrote:

Hi,
I'm a bit in a bind for a number of issues related to resource
validation and resource names.

Say I'm adding again an existing dataset, without aliasing it,
and I press submit to add the "clone" [1]. What I get is, rightfully,
the following stack trace:

java.lang.IllegalArgumentException: Resource named 'mosaic' already exists in store: 'mosaic'
      at org.geoserver.catalog.impl.CatalogImpl.validate(CatalogImpl.java:349)
      at org.geoserver.catalog.impl.CatalogImpl.add(CatalogImpl.java:328)
      at org.geoserver.security.SecureCatalogImpl.add(SecureCatalogImpl.java:845)
      at org.geoserver.web.data.ResourceConfigurationPage$4.onSubmit(ResourceConfigurationPage.java:115)

Ok. So I want to add a validation in the UI that prevents this.
Mumble... how to do this? The validation will trigger before the
resources are updated, so in the validator I have in my hands just
the old state of the ResourceInfo, and the new name.
I would like to call Catalog.validate(resource) to avoid duplicating
the validation logic, but to do so I'd need to clone the resource first
and force the new name onto it. Is there any facility to duplicate
a ResourceInfo (maybe I could just make a method that serializes and
un-serializes it, this would effectively deep clone it).

There are methods in CatalogBuilder of the form update* which essentially creates a clone. It might work in this case.

This is a tough issue. The make a copy and validate the copy seems hacky to me... and relies on how the in memory catalog is implemented internally. Also the validate() methods are not public api, which means we would have ot make them public api, which raises the bar to implement the Catalog interface (not an objection, just an observation).

I think it makes more sense to try to make the change, and catch the exception, but the problem (as you noted) is that we have no way to i18nize those exception messages. More on this below.

And even if I do, the validator will have to return a proper error
message... say tomorrow you add a new validation, how do I associate
the catalog validation exception with a meaningful message (eventually
translated?). It seems that if I want to have i18n error messages
I have to replicate the validation logic anyways?

This problem is not really constrained to the catalog imo. Any code the UI calls runs the risk of throwing a runtime exception that needs to be reported somehow. What are the thoughts about trying to come up with exceptions which the UI can i18nize. Either coming up with a special exception class that we can associate a i18n key with, or perhaps just throwing exceptions in which the message itself is treated as the key.

While looking into CatalogImpl.validate(ResourceInfo) I also noticed
the name validation occurs only if the layer is new... which does
not look like a good idea, as people can change the name afterwards
as well, and make it collide with another layer. The code should
check if the catalog already contains a resource with the same name
in the same workspace, and pass only if none is found, or if the
resource found is different from the one at hand (which requires
some id based comparison, with an id different than the name)...

Yup, makes sense, this is a bug.

Sooo... suggestions?

Cheers
Andrea

[1]: for those that do not know, since 1.6.x it's possible to add the
      same shapefile/table multiple times, and have it act as a different
      layer, with different styles and parameters, provided each
      "instance" is given a different "alias"

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

4

Justin Deoliveira ha scritto:

This is a tough issue. The make a copy and validate the copy seems hacky to me... and relies on how the in memory catalog is implemented internally. Also the validate() methods are not public api, which means we would have ot make them public api, which raises the bar to implement the Catalog interface (not an objection, just an observation).

I think it makes more sense to try to make the change, and catch the exception, but the problem (as you noted) is that we have no way to i18nize those exception messages. More on this below.

Meh, this goes against the way wicket works. Validators are there
to prevent the Form to write the fields into the model.
Thought yeah, we can force our way, forget about validation, and handle
the exception while trying to save.

This problem is not really constrained to the catalog imo. Any code the UI calls runs the risk of throwing a runtime exception that needs to be reported somehow. What are the thoughts about trying to come up with exceptions which the UI can i18nize. Either coming up with a special exception class that we can associate a i18n key with, or perhaps just throwing exceptions in which the message itself is treated as the key.

Hmmm.. yeah... given that many exceptions may come up of GeoTools
this may not be much of an option...
So far every time I worked in a UI I basically made sure no
garbage could go down into the lower levels, and as noticed,
this ends up duplicating the checks.

Anyways, for the exceptions we can control we could create a
GeoServerException base class that has a i18n key, and if that
handling does not work, fallback on a generic error message.
Something like:

GeoServerApplication.handleError(exception, "fallback message");
GeoServerApplication.handleError(exception, StringResource);

While looking into CatalogImpl.validate(ResourceInfo) I also noticed
the name validation occurs only if the layer is new... which does
not look like a good idea, as people can change the name afterwards
as well, and make it collide with another layer. The code should
check if the catalog already contains a resource with the same name
in the same workspace, and pass only if none is found, or if the
resource found is different from the one at hand (which requires
some id based comparison, with an id different than the name)...

Yup, makes sense, this is a bug.

http://jira.codehaus.org/browse/GEOS-2812

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.