Commit 0078c855 authored by Andrea Aime's avatar Andrea Aime
Browse files

[GEOS-7150] Features counted twice for WFS queries with GeoJSON responses

parent 45921137
...@@ -41,6 +41,10 @@ ...@@ -41,6 +41,10 @@
<groupId>org.geotools</groupId> <groupId>org.geotools</groupId>
<artifactId>gt-shapefile</artifactId> <artifactId>gt-shapefile</artifactId>
</dependency> </dependency>
<dependency>
<groupId>cglib</groupId>
<artifactId>cglib-nodep</artifactId>
</dependency>
<dependency> <dependency>
<groupId>org.geoserver</groupId> <groupId>org.geoserver</groupId>
<artifactId>gs-main</artifactId> <artifactId>gs-main</artifactId>
......
...@@ -40,7 +40,9 @@ class CountExecutor { ...@@ -40,7 +40,9 @@ class CountExecutor {
if(providedCount != COUNT_UNSET) { if(providedCount != COUNT_UNSET) {
return providedCount; return providedCount;
} else { } else {
return source.getCount(query); // make sure we get a count by getting a feature colleciton
// FeatureSource.getCount(...) can return -1
return source.getFeatures(query).size();
} }
} }
......
...@@ -22,6 +22,8 @@ import javax.xml.namespace.QName; ...@@ -22,6 +22,8 @@ import javax.xml.namespace.QName;
import net.opengis.wfs.XlinkPropertyNameType; import net.opengis.wfs.XlinkPropertyNameType;
import net.opengis.wfs20.ResultTypeType; import net.opengis.wfs20.ResultTypeType;
import net.opengis.wfs20.StoredQueryType; import net.opengis.wfs20.StoredQueryType;
import net.sf.cglib.proxy.Enhancer;
import net.sf.cglib.proxy.LazyLoader;
import org.geoserver.catalog.AttributeTypeInfo; import org.geoserver.catalog.AttributeTypeInfo;
import org.geoserver.catalog.Catalog; import org.geoserver.catalog.Catalog;
...@@ -296,7 +298,7 @@ public class GetFeature { ...@@ -296,7 +298,7 @@ public class GetFeature {
boolean isNumberMatchedSkipped = false; boolean isNumberMatchedSkipped = false;
int count = 0; // should probably be long int count = 0; // should probably be long
int totalCount = 0; BigInteger totalCount = BigInteger.ZERO;
//offset into result set in which to return features //offset into result set in which to return features
int totalOffset = request.getStartIndex() != null ? request.getStartIndex().intValue() : -1; int totalOffset = request.getStartIndex() != null ? request.getStartIndex().intValue() : -1;
...@@ -309,9 +311,15 @@ public class GetFeature { ...@@ -309,9 +311,15 @@ public class GetFeature {
totalOffset = 0; totalOffset = 0;
} }
int offset = totalOffset; int offset = totalOffset;
// feature collection size, we may need to calculate it
// optimization: WFS 1.0 does not require count unless we have multiple query elements
// and we are asked to perform a global limit on the results returned
boolean calculateSize = !(("1.0".equals(request.getVersion()) || "1.0.0".equals(request.getVersion())) &&
(queries.size() == 1 || maxFeatures == Integer.MAX_VALUE));
List results = new ArrayList(); List results = new ArrayList();
List<CountExecutor> totalCountExecutors = new ArrayList<CountExecutor>(); final List<CountExecutor> totalCountExecutors = new ArrayList<CountExecutor>();
try { try {
for (int i = 0; (i < queries.size()) && (count < maxFeatures); i++) { for (int i = 0; (i < queries.size()) && (count < maxFeatures); i++) {
...@@ -505,14 +513,6 @@ public class GetFeature { ...@@ -505,14 +513,6 @@ public class GetFeature {
features.getSchema().getUserData().put("targetVersion", request.getVersion()); features.getSchema().getUserData().put("targetVersion", request.getVersion());
} }
//feature collection size, we may need to calculate it
boolean calculateSize = true;
// optimization: WFS 1.0 does not require count unless we have multiple query elements
// and we are asked to perform a global limit on the results returned
calculateSize = !(("1.0".equals(request.getVersion()) || "1.0.0".equals(request.getVersion())) &&
(queries.size() == 1 || maxFeatures == Integer.MAX_VALUE));
if (!calculateSize) { if (!calculateSize) {
//if offset was specified and we have more queries left in this request then we //if offset was specified and we have more queries left in this request then we
// must calculate size in order to adjust the offset // must calculate size in order to adjust the offset
...@@ -553,8 +553,8 @@ public class GetFeature { ...@@ -553,8 +553,8 @@ public class GetFeature {
// collect queries required to return numberMatched/totalSize // collect queries required to return numberMatched/totalSize
// check maxFeatures and offset, if they are unset we can use the size we // check maxFeatures and offset, if they are unset we can use the size we
// calculated above // calculated above
isNumberMatchedSkipped = meta.getSkipNumberMatched() isNumberMatchedSkipped = meta.getSkipNumberMatched()
&& !request.isResultTypeHits(); && !request.isResultTypeHits();
if (!isNumberMatchedSkipped) { if (!isNumberMatchedSkipped) {
if (calculateSize && queryMaxFeatures == Integer.MAX_VALUE && offset == 0) { if (calculateSize && queryMaxFeatures == Integer.MAX_VALUE && offset == 0) {
totalCountExecutors.add(new CountExecutor(size)); totalCountExecutors.add(new CountExecutor(size));
...@@ -604,29 +604,26 @@ public class GetFeature { ...@@ -604,29 +604,26 @@ public class GetFeature {
} }
} }
// total count represents the total count of the features matched for this query in cases // total count represents the total count of the features matched for this query in cases
// where the client has limited the result set size, as an optimization we only calculate // where the client has limited the result set size, so we compute it lazily
// this if the following conditions hold if (isNumberMatchedSkipped) {
// 1. the request is wfs 2.0 totalCount = BigInteger.valueOf(-1);
// 2. maxFeatures != Integer.MAX_VALUE totalOffset = 0;
//TODO: we could actually add a third a optimization that when the count of features is } else if(count < maxFeatures && calculateSize) {
// less than maxFeatures we don't have to calculate it since it is the same as count, but // optimization: if count < max features then total count == count
// this requires that we do that check post query loop which requires a bit of code totalCount = BigInteger.valueOf(count);
// refactoring
// we need the total count only for WFS 2.0
if (!request.getVersion().startsWith("2")) {
totalCount = -1;
} else { } else {
if (isNumberMatchedSkipped) { // ok, in this case we're forced to run the queries to discover the actual total count
totalCount = -1; // We do so lazily, not all output formats need it, leveraging the fact that BigInteger
totalOffset = 0; // is not final to wrap it in a lazy loading proxy
} else { Enhancer enhancer = new Enhancer();
// optimization: if count < max features then total count == count enhancer.setSuperclass(BigInteger.class);
if(count < maxFeatures) { enhancer.setCallback(new LazyLoader() {
totalCount = count;
} else { @Override
// ok, in this case we're forced to run the queries to discover the actual total count public Object loadObject() throws Exception {
long totalCount = 0;
for (CountExecutor q : totalCountExecutors) { for (CountExecutor q : totalCountExecutors) {
int result = q.getCount(); int result = q.getCount();
// if the count is unknown for one, we don't know the total, period // if the count is unknown for one, we don't know the total, period
...@@ -637,10 +634,11 @@ public class GetFeature { ...@@ -637,10 +634,11 @@ public class GetFeature {
totalCount += result; totalCount += result;
} }
} }
return BigInteger.valueOf(totalCount);
} }
} });
totalCount = (BigInteger) enhancer.create(new Class[] {String.class}, new Object[] {"0"});
} }
} catch (IOException e) { } catch (IOException e) {
throw new WFSException(request, "Error occurred getting features", e, request.getHandle()); throw new WFSException(request, "Error occurred getting features", e, request.getHandle());
} catch (SchemaException e) { } catch (SchemaException e) {
...@@ -711,11 +709,11 @@ public class GetFeature { ...@@ -711,11 +709,11 @@ public class GetFeature {
* Allows subclasses to alter the result generation * Allows subclasses to alter the result generation
*/ */
protected FeatureCollectionResponse buildResults(GetFeatureRequest request, int offset, int maxFeatures, protected FeatureCollectionResponse buildResults(GetFeatureRequest request, int offset, int maxFeatures,
int count, int total, List results, String lockId) { int count, BigInteger total, List results, String lockId) {
FeatureCollectionResponse result = request.createResponse(); FeatureCollectionResponse result = request.createResponse();
result.setNumberOfFeatures(BigInteger.valueOf(count)); result.setNumberOfFeatures(BigInteger.valueOf(count));
result.setTotalNumberOfFeatures(BigInteger.valueOf(total)); result.setTotalNumberOfFeatures(total);
result.setTimeStamp(Calendar.getInstance()); result.setTimeStamp(Calendar.getInstance());
result.setLockId(lockId); result.setLockId(lockId);
result.getFeature().addAll(results); result.getFeature().addAll(results);
...@@ -759,7 +757,7 @@ public class GetFeature { ...@@ -759,7 +757,7 @@ public class GetFeature {
//next //next
//calculate the count of the next result set //calculate the count of the next result set
int nextCount = total - (offset + count); int nextCount = total.intValue() - (offset + count);
if (nextCount > 0) { if (nextCount > 0) {
kvp.put("startIndex", String.valueOf(offset > 0 ? offset + count : count)); kvp.put("startIndex", String.valueOf(offset > 0 ? offset + count : count));
//kvp.put("count", String.valueOf(nextCount)); //kvp.put("count", String.valueOf(nextCount));
......
...@@ -13,16 +13,10 @@ import java.io.Writer; ...@@ -13,16 +13,10 @@ import java.io.Writer;
import java.math.BigInteger; import java.math.BigInteger;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.logging.Level; import java.util.logging.Level;
import java.util.logging.Logger; import java.util.logging.Logger;
import javax.xml.namespace.QName;
import org.eclipse.emf.common.util.EList;
import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.FeatureTypeInfo;
import org.geoserver.config.GeoServer; import org.geoserver.config.GeoServer;
import org.geoserver.ows.Dispatcher; import org.geoserver.ows.Dispatcher;
import org.geoserver.ows.Request; import org.geoserver.ows.Request;
...@@ -32,34 +26,22 @@ import org.geoserver.platform.ServiceException; ...@@ -32,34 +26,22 @@ import org.geoserver.platform.ServiceException;
import org.geoserver.wfs.WFSGetFeatureOutputFormat; import org.geoserver.wfs.WFSGetFeatureOutputFormat;
import org.geoserver.wfs.WFSInfo; import org.geoserver.wfs.WFSInfo;
import org.geoserver.wfs.request.FeatureCollectionResponse; import org.geoserver.wfs.request.FeatureCollectionResponse;
import org.geoserver.wfs.request.GetFeatureRequest;
import org.geotools.data.FeatureSource;
import org.geotools.data.Query;
import org.geotools.factory.CommonFactoryFinder;
import org.geotools.factory.Hints;
import org.geotools.feature.FeatureCollection; import org.geotools.feature.FeatureCollection;
import org.geotools.feature.FeatureIterator; import org.geotools.feature.FeatureIterator;
import org.geotools.filter.spatial.ReprojectingFilterVisitor;
import org.geotools.geometry.jts.ReferencedEnvelope; import org.geotools.geometry.jts.ReferencedEnvelope;
import org.geotools.gml2.SrsSyntax; import org.geotools.gml2.SrsSyntax;
import org.geotools.referencing.CRS; import org.geotools.referencing.CRS;
import org.geotools.referencing.NamedIdentifier; import org.geotools.referencing.NamedIdentifier;
import org.opengis.feature.Feature;
import org.opengis.feature.simple.SimpleFeature; import org.opengis.feature.simple.SimpleFeature;
import org.opengis.feature.simple.SimpleFeatureType; import org.opengis.feature.simple.SimpleFeatureType;
import org.opengis.feature.type.AttributeDescriptor; import org.opengis.feature.type.AttributeDescriptor;
import org.opengis.feature.type.FeatureType;
import org.opengis.feature.type.GeometryDescriptor; import org.opengis.feature.type.GeometryDescriptor;
import org.opengis.filter.Filter;
import org.opengis.filter.FilterFactory2;
import org.opengis.referencing.FactoryException; import org.opengis.referencing.FactoryException;
import org.opengis.referencing.ReferenceIdentifier; import org.opengis.referencing.ReferenceIdentifier;
import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.opengis.referencing.crs.CoordinateReferenceSystem;
import com.vividsolutions.jts.geom.Geometry; import com.vividsolutions.jts.geom.Geometry;
import net.opengis.wfs.GetFeatureType;
import net.opengis.wfs.QueryType;
import net.sf.json.JSONException; import net.sf.json.JSONException;
/** /**
...@@ -130,20 +112,10 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat { ...@@ -130,20 +112,10 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
boolean hasGeom = false; boolean hasGeom = false;
// get feature count for request // get feature count for request
BigInteger featureCount = null; BigInteger totalNumberOfFeatures = featureCollection.getTotalNumberOfFeatures();
// for WFS 1.0.0 and WFS 1.1.0 a request with the query must be executed BigInteger featureCount = (totalNumberOfFeatures != null && totalNumberOfFeatures.longValue() < 0)
if(describeFeatureType != null) { ? null : totalNumberOfFeatures;
if (describeFeatureType.getParameters()[0] instanceof GetFeatureType) {
featureCount = BigInteger.valueOf(getFeatureCountFromWFS11Request(describeFeatureType, wfs));
}
// for WFS 2.0.0 the total number of features is stored in the featureCollection
else if (describeFeatureType.getParameters()[0] instanceof net.opengis.wfs20.GetFeatureType){
BigInteger totalNumberOfFeatures = featureCollection.getTotalNumberOfFeatures();
featureCount = (totalNumberOfFeatures != null && totalNumberOfFeatures.longValue() < 0)
? null : totalNumberOfFeatures;
}
}
try { try {
osw = new OutputStreamWriter(output, gs.getGlobal().getSettings().getCharset()); osw = new OutputStreamWriter(output, gs.getGlobal().getSettings().getCharset());
outWriter = new BufferedWriter(osw); outWriter = new BufferedWriter(osw);
...@@ -392,67 +364,6 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat { ...@@ -392,67 +364,6 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
} }
/**
* getFeatureCountFromWFS11Request
*
* Function gets the total number of features from a WFS 1.0.0 or WFS 1.1.0 request and returns it.
*
* @param Operation describeFeatureType
* @param WFSInfo wfs
* @return int featurecount
* @throws IOException
*/
@SuppressWarnings("unchecked")
private int getFeatureCountFromWFS11Request(Operation operation, WFSInfo wfs)
throws IOException {
final FilterFactory2 FF = CommonFactoryFinder.getFilterFactory2(null);
int totalCount = 0;
Catalog catalog = wfs.getGeoServer().getCatalog();
GetFeatureType request = (GetFeatureType) operation.getParameters()[0];
List<Map<String, String>> viewParams = new GetFeatureRequest.WFS11(request).getViewParams();
int idx = 0;
for (QueryType query : (EList<QueryType>) request.getQuery()) {
QName typeName = (QName) query.getTypeName().get(0);
FeatureTypeInfo meta = catalog.getFeatureTypeByName(typeName.getNamespaceURI(),
typeName.getLocalPart());
FeatureSource<? extends FeatureType, ? extends Feature> source = meta.getFeatureSource(
null, null);
Filter filter = query.getFilter();
if (filter == null) {
filter = Filter.INCLUDE;
} else {
// reproject spatial filter to the native crs
ReprojectingFilterVisitor reprojector = new ReprojectingFilterVisitor(FF, source.getSchema());
filter = (Filter) filter.accept(reprojector, null);
}
Query countQuery = new Query(typeName.getLocalPart(), filter);
Map<String, String> viewParam = viewParams != null && viewParams.size() > idx ? viewParams
.get(idx) : null;
if (viewParam != null) {
final Hints hints = new Hints();
hints.put(Hints.VIRTUAL_TABLE_PARAMETERS, viewParam);
countQuery.setHints(hints);
}
int count = 0;
count = source.getCount(countQuery);
if (count == -1) {
// information was not available in the header!
org.geotools.data.Query gtQuery = new org.geotools.data.Query(countQuery);
FeatureCollection<? extends FeatureType, ? extends Feature> features = source
.getFeatures(gtQuery);
count = features.size();
}
totalCount +=count;
}
return totalCount;
}
@Override @Override
public String getCharset(Operation operation){ public String getCharset(Operation operation){
return gs.getGlobal().getSettings().getCharset(); return gs.getGlobal().getSettings().getCharset();
......
...@@ -76,6 +76,8 @@ public abstract class FeatureCollectionResponse extends RequestObject { ...@@ -76,6 +76,8 @@ public abstract class FeatureCollectionResponse extends RequestObject {
} }
public static class WFS11 extends FeatureCollectionResponse { public static class WFS11 extends FeatureCollectionResponse {
BigInteger totalNumberOfFeatures;
public WFS11(EObject adaptee) { public WFS11(EObject adaptee) {
super(adaptee); super(adaptee);
} }
...@@ -96,12 +98,11 @@ public abstract class FeatureCollectionResponse extends RequestObject { ...@@ -96,12 +98,11 @@ public abstract class FeatureCollectionResponse extends RequestObject {
@Override @Override
public BigInteger getTotalNumberOfFeatures() { public BigInteger getTotalNumberOfFeatures() {
//noop return totalNumberOfFeatures;
return null;
} }
@Override @Override
public void setTotalNumberOfFeatures(BigInteger n) { public void setTotalNumberOfFeatures(BigInteger n) {
//noop this.totalNumberOfFeatures = n;
} }
@Override @Override
......
...@@ -317,27 +317,39 @@ public class GeoJSONTest extends WFSTestSupport { ...@@ -317,27 +317,39 @@ public class GeoJSONTest extends WFSTestSupport {
} }
@Test @Test
public void testGetFeatureCount() throws Exception { public void testGetFeatureCountNoFilter() throws Exception {
//request without filter //request without filter
String out = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=10&outputformat="+JSONType.json); String out = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=10&outputformat="+JSONType.json);
JSONObject rootObject = JSONObject.fromObject( out ); JSONObject rootObject = JSONObject.fromObject( out );
assertEquals(rootObject.get("totalFeatures"),5); assertEquals(rootObject.get("totalFeatures"),5);
}
@Test
public void testGetFeatureCountFilter() throws Exception {
//request with filter (featureid=PrimitiveGeoFeature.f001) //request with filter (featureid=PrimitiveGeoFeature.f001)
String out2 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=10&outputformat="+JSONType.json+"&featureid=PrimitiveGeoFeature.f001"); String out2 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=10&outputformat="+JSONType.json+"&featureid=PrimitiveGeoFeature.f001");
JSONObject rootObject2 = JSONObject.fromObject( out2 ); JSONObject rootObject2 = JSONObject.fromObject( out2 );
assertEquals(rootObject2.get("totalFeatures"),1); assertEquals(rootObject2.get("totalFeatures"),1);
}
@Test
public void testGetFeatureCountMaxFeatures() throws Exception {
//check if maxFeatures doesn't affect totalFeatureCount; set Filter and maxFeatures //check if maxFeatures doesn't affect totalFeatureCount; set Filter and maxFeatures
String out3 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=1&outputformat="+JSONType.json+"&featureid=PrimitiveGeoFeature.f001,PrimitiveGeoFeature.f002"); String out3 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature&maxfeatures=1&outputformat="+JSONType.json+"&featureid=PrimitiveGeoFeature.f001,PrimitiveGeoFeature.f002");
JSONObject rootObject3 = JSONObject.fromObject( out3 ); JSONObject rootObject3 = JSONObject.fromObject( out3 );
assertEquals(rootObject3.get("totalFeatures"),2); assertEquals(rootObject3.get("totalFeatures"),2);
}
@Test
public void testGetFeatureCountMultipleFeatureTypes() throws Exception {
//request with multiple featureTypes and Filter //request with multiple featureTypes and Filter
String out4 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature,sf:AggregateGeoFeature&outputformat="+JSONType.json + "&featureid=PrimitiveGeoFeature.f001,PrimitiveGeoFeature.f002,AggregateGeoFeature.f009"); String out4 = getAsString("wfs?request=GetFeature&version=1.0.0&typename=sf:PrimitiveGeoFeature,sf:AggregateGeoFeature&outputformat="+JSONType.json + "&featureid=PrimitiveGeoFeature.f001,PrimitiveGeoFeature.f002,AggregateGeoFeature.f009");
JSONObject rootObject4 = JSONObject.fromObject( out4 ); JSONObject rootObject4 = JSONObject.fromObject( out4 );
assertEquals(rootObject4.get("totalFeatures"),3); assertEquals(rootObject4.get("totalFeatures"),3);
}
@Test
public void testGetFeatureCountSpatialFilter() throws Exception {
//post with spatial-filter in another projection than layer-projection //post with spatial-filter in another projection than layer-projection
String xml = "<wfs:GetFeature " + "service=\"WFS\" " + "outputFormat=\""+JSONType.json+"\" " String xml = "<wfs:GetFeature " + "service=\"WFS\" " + "outputFormat=\""+JSONType.json+"\" "
+ "version=\"1.1.0\" " + "version=\"1.1.0\" "
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment