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 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||POINT(39.73245 2.00342)||155|http://www.opengeospatial.org/|12765||2006-10-25|5.03|true
PrimitiveGeoFeature.f002=description-f002|name-f002||POINT(59.41276 0.22601)||154|http://www.opengeospatial.org/|12769||2006-10-23|4.02|false
PrimitiveGeoFeature.f003=description-f003|name-f003|||LINESTRING(46.074 9.799,46.652 10.466,47.114 11.021)|180||672.1||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))|||300||783.5|2006-06-27 22:08:00-07|2006-12-12|18.92|true
PrimitiveGeoFeature.f015=|||POINT(34.94 -10.52)||-900||2.4|||7.9|
···
On Mon, Dec 22, 2014 at 3:26 PM, Jody Garnett <jgarnett@anonymised.com> 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:Notfes: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 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
On 22 December 2014 at 14:12, Torben Barsballe <tbarsballe@anonymised.com> 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:
- 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();
- 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