Your suggested fix seems to have gotten rid of most of the bounding box
problems (Although there are some other tests that assume the first
geometry is the default one that will have to be changed). However, I am a
bit concerned about how reliable this change is. As you can see below, the
dataset has 3 different geometry columns. Each of these columns is <null>
for at least one line. Given this, I am surprised that setting the point
column to be the default actually works.
The other question would be is how often do we actually have multiple
geometries in a single feature type? If this is somewhat comen, then som
proper support might be a good idea. If this rarely occurs, then it might
be advisable to change the test data so it isn't using multiple geometry
features in one feature type.
This is the dataset used for the WFS tests:
_=description:String,name:
String,surfaceProperty:Polygon:srid=4326,pointProperty:Point:srid=4326,curveProperty:LineString:srid=4326,intProperty:Integer,uriProperty:java.net.URI,measurand:String,dateTimeProperty:java.sql.Timestamp,dateProperty:java.sql.Date,decimalProperty:Double,booleanProperty:Boolean
PrimitiveGeoFeature.f001=description-f001|name-f001|<null>|POINT(39.73245
2.00342)|<null>|155|http://www.opengeospatial.org/|12765|
<null>|2006-10-25|5.03|true
PrimitiveGeoFeature.f002=description-f002|name-f002|<null>|POINT(59.41276
0.22601)|<null>|154|http://www.opengeospatial.org/|12769|
<null>|2006-10-23|4.02|false
PrimitiveGeoFeature.f003=description-f003|name-f003|<null>|<null>|LINESTRING(46.074
9.799,46.652 10.466,47.114
11.021)|180|<null>|672.1|<null>|2006-09-01|12.92|true
PrimitiveGeoFeature.f008=description-f008|name-f008|POLYGON((45.174
30.899,45.652 30.466,45.891 30.466,45.174
30.899))|<null>|<null>|300|<null>|783.5|2006-06-27
22:08:00-07|2006-12-12|18.92|true
PrimitiveGeoFeature.f015=<null>|<null>|<null>|POINT(34.94
-10.52)|<null>|-900|<null>|2.4|<null>|<null>|7.9|<null>
On Mon, Dec 22, 2014 at 3:26 PM, Jody Garnett <[email protected]>
wrote:
> Good debugging Torben, you have hit on an oddness in the WFS
> specification. Much of the specification is written using a geometry
> expression against a feature ... and then it becomes a bit murky. As an web
> / xml specification it was written with the idea we would explore all the
> attributes (each expressed as an XML snippet). Since we are boiling this
> down into Filter and SQL queries we like the idea of a specific geometry to
> test against for speed / indexing / sanity.
>
> So the "correct" thing to do would be to look at the feature type
> definition and determine which attributes are "geometry" attributes and
> test against both.
> The "quick" thing to do is guess a default geometry (often the first one
> unless the user says different) and only test against that one.
>
> As you can see AbstractDataStore and ContentDataStore made a different
> choice!
>
> Aside: The WFS Specification is split into sections, the filter stuff is
> factored out so it can be used in a couple specifications (such as WFS and
> SLD).
>
> From the WFS Tutorial[1]: *This is known as the "bounding box" or BBOX.
> The BBOX allows us to ask for only such Features that are contained (or
> partially contained) inside a box of the coordinates we specify. The form
> of the bbox query is "bbox=a1,b1,a2,b2" where a, b, c, and d refer to
> coordinates.*
>
> The Filter [2] specification actually fixed things up to be clear this
> time: BBOX operator
>
> The fes:BBOX element is defined as a convenient and more compact way of
> encoding the very common bounding box constraint based on the gml:Envelope
> geometry. It is equivalent to the spatial operation <fes:Not><fes:Disjoint>
> … </fes:Disjoint></fes:Not> meaning that the fes:BBOX operator shall
> identify all geometries that spatially interact with the box. If there is
> only one argument (i.e. one child element specified for the BBOX operator,
> the calling service shall apply the operator to the geometric values of all
> the spatial properties of the resource. In this case, the operator shall
> evaluate to true if all tested spatial property values fulfill the spatial
> relationship implied by the operator. Otherwise, the BBOX operator shall
> evaluate the specified arguments (i.e. the two child elements) and test
> whether their geometric values satisfy the implied spatial relationship.
>
> I also note that Filter 2.0.2 matches our GeoTools understanding of Filter
> - i.e. no longer expects one of the expressions to be a propertyName.
>
> Links:
> [1] http://www.ogcnetwork.net/wfstutorial
> [2] http://docs.opengeospatial.org/is/09-026r2/09-026r2.html#60 <--
> finally html welcome to 1999 OGC you made it!
>
> To return the matter at hand, we may be able to mark which geometry is the
> default in the property file, and enable these tests to pass while still
> maintaining the ContendDataStore design decisions (focused on being quick
> and clear).
>
> The DataUtilities method createType
> <http://docs.geotools.org/latest/javadocs/org/geotools/data/DataUtilities.html#createType(java.lang.String,%20java.lang.String)>
> has
> the following javadoc examples:
>
> Examples:
>
> name:"",age:0,geom:Geometry,centroid:Point,url:java.io.URL
> id:String,polygonProperty:Polygon:srid=32615
> identifier:UUID,location:Point,*area:MultiPolygon,created:Date
> uuid:UUID,name:String,description:String,time:java.sql.Timestamp
>
>
>
> See how the first one has both geom and centroid attributes? The javadocs
> also indicate you can use a * to mark one of these as a default geometry.
>
> The following would mark centroid as the default geometry:
> name:"",age:0,geom:Geometry,*centroid:Point,url:java.io.URL
>
> Making a similar change to the dataset used for your failed
> geoserver-devel test would allow it to pass.
> --
> Jody Garnett
> Senior Software Engineer | Boundless <http://boundlessgeo.com/>
>
> On 22 December 2014 at 14:12, Torben Barsballe <
> [email protected]> wrote:
>
>> While attempting to migrate geoserver to use gt-property-ng, I have ran
>> into a bit of an issue with the ContentFeatureSource.resolvePropertyNames()
>> method, when it is called by ContentFeatureCollection.size().
>>
>> Geoserver is running a BBox query against a FeatureCollection containing
>> multiple geometries. expression1 of the BBox is a blank attribute,
>> expression2 is the BBox.
>>
>> When using gt-property and AbstractFeatureStore, expression1 remains
>> blank throught execution, and the BBox query is tested against each
>> geometry in the featureCollection
>>
>> When using gt-property-ng and ContentFeatureStore, the
>> resolvePropertyNames() method is called, and results in expression1 being
>> set to the first geometry. Then, when ContentFeatureCollection.size() runs,
>> the BBox query is only tested against this first geometry, and fails to
>> return any results.
>>
>> It seems that the implementation of
>> DefaultGeometrySimpleFeaturePropertyAccessorFactory is only intended to
>> return a single geometry.
>> Upon analysis of the code, I feel that there are two places this could
>> probably be fixed:
>>
>> 1. Change DefaultGeometrySimpleFeaturePropertyAccessorFactory.get() to
>> handle multiple geometries properly, such that it tryAccessor should set
>> value to an AttributeDescriptor of "" when xpath is "". However, I am not
>> sure exactly what this class interacts with, and it looks like it is doing
>> what it should be doing right now, so I am hesitant to change anything.
>>
>> Looking at how accessor caching and accessor factories are handled, it is
>> also possible that the Accessor caching implementation in
>> AttributeExpressionImpl.evaluate() is returning the wrong type of accessor
>> (DefaultGeometrySimpleFeaturePropertyAccessorFactory) but this is not being
>> caught because the value of "target" being tested against is null (causing
>> the wrong accessor to (incorectly) return true for accessor.canHandle();
>>
>> 2. Skip the resolvePropertyNames in ContentFeatureSource.getCount().
>> However, this seems to set some values that are required by later calls,
>> and may require more significant rewriting.
>>
>> Alternatively, I could add a special check prior to resolvePropertyNames
>> such that it is not called in this specific circumstance (Blank xpath,
>> etc.) but this seems a bit hacky.
>>
>> For now, I will continue to compare the new implementation with the
>> implementation used by AbstractFeatureSource to see if I can come up with
>> any ideas.
>>
>> Torben Barsballe
>>
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
