[Geoserver-devel] CSW and checking spatial filters

Hi,
as you may know a CSW should check the GetRecord incoming filters for validity, one of
the checks we do is verify that spatial filters are actually hitting spatial properties.

The current code uses a SpatialFilterChecker which works fine for the csw:Record case,
and I guess is working fine for the ISO case too, but it’s definitely not working as expected
for the more complex ebRIM case, where the geometries are nested in a structure
that looks a lot like a hashmap, whose keys are what determine which type of value
should be contained in the properties (long story short, having just the schema won’t cut it).

So I wanted to change things and have the code delegate the check to RecordDescriptor:

RecordDescriptor {
/**

  • Checks the spatial filters, throws service exceptions in case they are referring to non existing,
  • or existing but non spatial, properties
    */
    public void verifySpatialFilters(Filter filter);

    }

and have the common cases use SpatialFilterChecker to implement the above method.

I’d like to commit the change on trunk only, no need to backport to 2.4.x

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


Sounds reasonable to me but I will let Niels chime in.

Regarding the backport is there a reason not to backport it to 2.4.x? Thinking it is best to not diverge unless the code on master is unstable (which doesn’t seem to be the case here) so that future backports remain trivial.

$0.02

···

On Thu, Aug 22, 2013 at 1:48 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
as you may know a CSW should check the GetRecord incoming filters for validity, one of
the checks we do is verify that spatial filters are actually hitting spatial properties.

The current code uses a SpatialFilterChecker which works fine for the csw:Record case,
and I guess is working fine for the ISO case too, but it’s definitely not working as expected
for the more complex ebRIM case, where the geometries are nested in a structure
that looks a lot like a hashmap, whose keys are what determine which type of value
should be contained in the properties (long story short, having just the schema won’t cut it).

So I wanted to change things and have the code delegate the check to RecordDescriptor:

RecordDescriptor {
/**

  • Checks the spatial filters, throws service exceptions in case they are referring to non existing,
  • or existing but non spatial, properties
    */
    public void verifySpatialFilters(Filter filter);

    }

and have the common cases use SpatialFilterChecker to implement the above method.

I’d like to commit the change on trunk only, no need to backport to 2.4.x

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



Introducing Performance Central, a new site from SourceForge and
AppDynamics. Performance Central is your source for news, insights,
analysis and resources for efficient Application Performance Management.
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk


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.

On Thu, Aug 22, 2013 at 9:18 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Sounds reasonable to me but I will let Niels chime in.

Regarding the backport is there a reason not to backport it to 2.4.x?
Thinking it is best to not diverge unless the code on master is unstable
(which doesn't seem to be the case here) so that future backports remain
trivial.

Uh, mostly because it's an API change (new method in an interface), but
generally speaking for the existing code it's a clean refactor

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

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

Sounds good to me.

On 22/08/13 21:18, Justin Deoliveira wrote:

Sounds reasonable to me but I will let Niels chime in.

Regarding the backport is there a reason not to backport it to 2.4.x? Thinking it is best to not diverge unless the code on master is unstable (which doesn't seem to be the case here) so that future backports remain trivial.

$0.02

On Thu, Aug 22, 2013 at 1:48 PM, Andrea Aime <andrea.aime@anonymised.com <mailto:andrea.aime@anonymised.com>> wrote:

    Hi,
    as you may know a CSW should check the GetRecord incoming filters
    for validity, one of
    the checks we do is verify that spatial filters are actually
    hitting spatial properties.

    The current code uses a SpatialFilterChecker which works fine for
    the csw:Record case,
    and I guess is working fine for the ISO case too, but it's
    definitely not working as expected
    for the more complex ebRIM case, where the geometries are nested
    in a structure
    that looks a lot like a hashmap, whose keys are what determine
    which type of value
    should be contained in the properties (long story short, having
    just the schema won't cut it).

    So I wanted to change things and have the code delegate the check
    to RecordDescriptor:

    RecordDescriptor {
       /**
       * Checks the spatial filters, throws service exceptions in case
    they are referring to non existing,
       * or existing but non spatial, properties
       */
       public void verifySpatialFilters(Filter filter);
    ...
    }

    and have the common cases use SpatialFilterChecker to implement
    the above method.

    I'd like to commit the change on trunk only, no need to backport
    to 2.4.x

    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 <tel:%2B39%200584%20962313>
    fax: +39 0584 1660272 <tel:%2B39%200584%201660272>
    mob: +39 339 8844549 <tel:%2B39%20%C2%A0339%208844549>

    http://www.geo-solutions.it
    http://twitter.com/geosolutions_it

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

    ------------------------------------------------------------------------------
    Introducing Performance Central, a new site from SourceForge and
    AppDynamics. Performance Central is your source for news, insights,
    analysis and resources for efficient Application Performance
    Management.
    Visit us today!
    http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
    _______________________________________________
    Geoserver-devel mailing list
    Geoserver-devel@lists.sourceforge.net
    <mailto:Geoserver-devel@lists.sourceforge.net>
    https://lists.sourceforge.net/lists/listinfo/geoserver-devel

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

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and
AppDynamics. Performance Central is your source for news, insights,
analysis and resources for efficient Application Performance Management.
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk

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

On Thu, Aug 22, 2013 at 9:18 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Sounds reasonable to me but I will let Niels chime in.

Regarding the backport is there a reason not to backport it to 2.4.x?
Thinking it is best to not diverge unless the code on master is unstable
(which doesn't seem to be the case here) so that future backports remain
trivial.

Here is the patch applied on trunk:
https://github.com/geoserver/geoserver/commit/01843f8be22e7e30ea1acc77e3bb2de72a552f14

So, shall I backport it to 2.4.x? (no particular objection to the backport
from me, CSW just graduated
and we don't have a release with it included in the wild)

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

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

I would say given that we haven’t officially released it yet I would be +1 on backporting the api change in the interest of keeping the two branches consistent.

···

On Fri, Aug 23, 2013 at 7:06 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Thu, Aug 22, 2013 at 9:18 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:


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

Sounds reasonable to me but I will let Niels chime in.

Regarding the backport is there a reason not to backport it to 2.4.x? Thinking it is best to not diverge unless the code on master is unstable (which doesn’t seem to be the case here) so that future backports remain trivial.

Here is the patch applied on trunk:
https://github.com/geoserver/geoserver/commit/01843f8be22e7e30ea1acc77e3bb2de72a552f14

So, shall I backport it to 2.4.x? (no particular objection to the backport from me, CSW just graduated
and we don’t have a release with it included in the wild)

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