Skip to content

Commit

Permalink
[GEOS-8455] WFS 2.0 LockFeature with embedded StoredQuery fails with NPE
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaime
aaime committed Dec 1, 2017
1 parent ee86f81 commit 201a3e9
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 32 deletions.
53 changes: 50 additions & 3 deletions src/wfs/src/main/java/org/geoserver/wfs/DefaultWebFeatureService20.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.geoserver.wfs;

import net.opengis.fes20.AbstractQueryExpressionType;
import net.opengis.wfs20.CreateStoredQueryResponseType;
import net.opengis.wfs20.CreateStoredQueryType;
import net.opengis.wfs20.DescribeFeatureTypeType;
Expand All @@ -20,25 +21,36 @@
import net.opengis.wfs20.ListStoredQueriesType;
import net.opengis.wfs20.LockFeatureResponseType;
import net.opengis.wfs20.LockFeatureType;
import net.opengis.wfs20.QueryType;
import net.opengis.wfs20.StoredQueryType;
import net.opengis.wfs20.TransactionResponseType;
import net.opengis.wfs20.TransactionType;
import net.opengis.wfs20.ValueCollectionType;
import net.opengis.wfs20.Wfs20Factory;
import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.ecore.util.EcoreUtil;
import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.FeatureTypeInfo;
import org.geoserver.config.GeoServer;
import org.geoserver.feature.TypeNameExtractingVisitor;
import org.geoserver.platform.ServiceException;
import org.geoserver.wfs.request.DescribeFeatureTypeRequest;
import org.geoserver.wfs.request.FeatureCollectionResponse;
import org.geoserver.wfs.request.GetCapabilitiesRequest;
import org.geoserver.wfs.request.GetFeatureRequest;
import org.geoserver.wfs.request.LockFeatureRequest;
import org.geoserver.wfs.request.LockFeatureResponse;
import org.geoserver.wfs.request.Query;
import org.geoserver.wfs.request.TransactionRequest;
import org.geotools.xml.transform.TransformerBase;
import org.opengis.filter.FilterFactory2;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;

import java.util.ArrayList;
import java.util.List;

public class DefaultWebFeatureService20 implements WebFeatureService20, ApplicationContextAware {

/**
Expand Down Expand Up @@ -110,18 +122,53 @@ public ValueCollectionType getPropertyValue(GetPropertyValueType request) throws

public LockFeatureResponseType lockFeature(LockFeatureType request) throws WFSException {
LockFeature lockFeature = new LockFeature(getServiceInfo(), getCatalog(), filterFactory);
LockFeatureRequest.WFS20 requestWrapper = new LockFeatureRequest.WFS20(request);
if (request.getLockId() != null) {
lockFeature.refresh(request.getLockId());
LockFeatureResponse response = new LockFeatureRequest.WFS20(request).createResponse();
LockFeatureResponse response = requestWrapper.createResponse();
response.setLockId(request.getLockId());

return (LockFeatureResponseType) response.getAdaptee();
} else {
return (LockFeatureResponseType) lockFeature.lockFeature(new LockFeatureRequest.WFS20(request))
// Need to perform some of the same Stored Query handling as GetFeature
// ... expand eventual stored queries
boolean getFeatureById = GetFeature.expandStoredQueries(requestWrapper,
request.getAbstractQueryExpression(), getStoredQueryProvider());
// ... expand the typenames from feature id filters (the wrappers will modify the underlying object
List<Query> queries = GetFeatureRequest.WFS20.getQueries(request.getAbstractQueryExpression());
GetFeature.expandTypeNames(requestWrapper, queries, getFeatureById, getCatalog());
// ... lock cannot handle queries with multiple target typenames, need to expand them into separate queries
fixQueriesForLock(request.getAbstractQueryExpression());

// run the lock
return (LockFeatureResponseType) lockFeature.lockFeature(requestWrapper)
.getAdaptee();
}
}


private void fixQueriesForLock(EList<AbstractQueryExpressionType> queries) {
for (int i = 0; i < queries.size(); i++) {
Object obj = queries.get(i);
if (obj instanceof QueryType) {
QueryType query = (QueryType) queries.get(0);


if(query.getTypeNames().size() > 1) {
List<QueryType> expanded = new ArrayList<>();
for (Object typeName : query.getTypeNames()) {
QueryType copy = EcoreUtil.copy(query);
copy.getTypeNames().clear();
copy.getTypeNames().add(typeName);
expanded.add(copy);
}
queries.remove(i);
queries.addAll(i, expanded);
i+= expanded.size();
}
}
}
}

public TransactionResponseType transaction(TransactionType request) throws WFSException {
Transaction tx = new Transaction(getServiceInfo(), getCatalog(), context);
tx.setFilterFactory(filterFactory);
Expand Down
66 changes: 42 additions & 24 deletions src/wfs/src/main/java/org/geoserver/wfs/GetFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.geoserver.wfs.request.LockFeatureRequest;
import org.geoserver.wfs.request.LockFeatureResponse;
import org.geoserver.wfs.request.Query;
import org.geoserver.wfs.request.RequestObject;
import org.geotools.data.DataUtilities;
import org.geotools.data.FeatureSource;
import org.geotools.data.Join;
Expand Down Expand Up @@ -250,27 +251,7 @@ public FeatureCollectionResponse run(GetFeatureRequest request)
queries = request.getQueries();

if (request.isQueryTypeNamesUnset()) {
// do a check for FeatureId filters in the queries and update the type names for the
// queries accordingly
for (Query q : queries) {
if (!q.getTypeNames().isEmpty()) continue;

if (q.getFilter() != null) {
TypeNameExtractingVisitor v = new TypeNameExtractingVisitor(catalog);
q.getFilter().accept(v, null);
q.getTypeNames().addAll(v.getTypeNames());
}

if (q.getTypeNames().isEmpty()) {
if (getFeatureById) {
// by spec, a 404 should be returned in this case
throw new WFSException(request, "Could not find feature with specified id", WFSException.NOT_FOUND);
} else {
String msg = "No feature types specified";
throw new WFSException(request, msg, ServiceException.INVALID_PARAMETER_VALUE);
}
}
}
expandTypeNames(request, queries, getFeatureById, getCatalog());
}

// Optimization Idea
Expand Down Expand Up @@ -720,6 +701,30 @@ public Object loadObject() throws Exception {
return buildResults(request, totalOffset, maxFeatures, count, totalCount, results, lockId, getFeatureById);
}

static void expandTypeNames(RequestObject request, List<Query> queries, boolean getFeatureById, Catalog catalog) {
// do a check for FeatureId filters in the queries and update the type names for the
// queries accordingly
for (Query q : queries) {
if (!q.getTypeNames().isEmpty()) continue;

if (q.getFilter() != null) {
TypeNameExtractingVisitor v = new TypeNameExtractingVisitor(catalog);
q.getFilter().accept(v, null);
q.getTypeNames().addAll(v.getTypeNames());
}

if (q.getTypeNames().isEmpty()) {
if (getFeatureById) {
// by spec, a 404 should be returned in this case
throw new WFSException(request, "Could not find feature with specified id", WFSException.NOT_FOUND);
} else {
String msg = "No feature types specified";
throw new WFSException(request, msg, ServiceException.INVALID_PARAMETER_VALUE);
}
}
}
}


/**
* Expands the stored queries, returns true if a single GetFeatureById stored query was found (as a
Expand All @@ -730,6 +735,20 @@ public Object loadObject() throws Exception {
*/
protected boolean processStoredQueries(GetFeatureRequest request) {
List queries = request.getAdaptedQueries();
boolean foundGetFeatureById = expandStoredQueries(request, queries, storedQueryProvider);

return queries.size() == 1 && foundGetFeatureById;
}

/**
* Replaces stored queries with actual ad-hoc queries
* @param request
* @param queries
* @param foundGetFeatureById
* @param storedQueryProvider
* @return
*/
static boolean expandStoredQueries(RequestObject request, List queries, StoredQueryProvider storedQueryProvider) {
boolean foundGetFeatureById = false;
for (int i = 0; i < queries.size(); i++) {
Object obj = queries.get(i);
Expand Down Expand Up @@ -758,10 +777,9 @@ protected boolean processStoredQueries(GetFeatureRequest request) {
i += compiled.size();
}
}

return queries.size() == 1 && foundGetFeatureById;
return foundGetFeatureById;
}

/**
* Allows subclasses to alter the result generation
*/
Expand Down
12 changes: 9 additions & 3 deletions src/wfs/src/main/java/org/geoserver/wfs/request/GetFeatureRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,20 @@ public WFS20(EObject adaptee) {

@Override
public List<Query> getQueries() {
List<Object> adaptedQueries = getAdaptedQueries();

return getQueries(adaptedQueries);
}

public static List<Query> getQueries(List<?> adaptedQueries) {
List<Query> list = new ArrayList<Query>();
for (Object o : getAdaptedQueries()) {
for (Object o : adaptedQueries) {
list.add(new Query.WFS20((EObject)o));
}

return list;
}

@Override
public List<Object> getAdaptedQueries() {
return eGet(adaptee, "abstractQueryExpression", List.class);
Expand Down
5 changes: 4 additions & 1 deletion src/wfs/src/main/java/org/geoserver/wfs/request/Lock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ public QName getTypeName() {
List typeNames = eGet(adaptee, "typeNames", List.class);
if (typeNames.size() == 1) {
return (QName) typeNames.get(0);
} else if(typeNames.size() > 0) {
throw new IllegalArgumentException("Multiple type names on single lock not supported");
}
throw new IllegalArgumentException("Multiple type names on single lock not supported");
// no typenames found, happens with GetFeatureById stored query for example
return null;
}

@Override
Expand Down
22 changes: 21 additions & 1 deletion src/wfs/src/test/java/org/geoserver/wfs/v2_0/LockFeatureTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void testLock() throws Exception {
assertEquals(5, dom.getElementsByTagNameNS(FES.NAMESPACE, "ResourceId").getLength());

// release the lock
print(dom);
// print(dom);
String lockId = dom.getDocumentElement().getAttribute("lockId");
get("wfs?request=ReleaseLock&version=2.0&lockId=" + lockId);
}
Expand Down Expand Up @@ -127,4 +127,24 @@ public void testRenewUnknownLock() throws Exception {

}

@Test
public void testLockWithStoredQuery() throws Exception {
String xml = "<wfs:LockFeature xmlns:wfs=\"http://www.opengis.net/wfs/2.0\" expiry=\"1\" service=\"WFS\"\n" +
" version=\"2.0.0\">\n" +
" <wfs:StoredQuery id=\"urn:ogc:def:query:OGC-WFS::GetFeatureById\">\n" +
" <wfs:Parameter name=\"id\">AggregateGeoFeature.f005</wfs:Parameter>\n" +
" </wfs:StoredQuery>\n" +
"</wfs:LockFeature>";

Document dom = postAsDOM("wfs", xml);
assertEquals("wfs:LockFeatureResponse", dom.getDocumentElement().getNodeName());
assertEquals(1, dom.getElementsByTagNameNS(FES.NAMESPACE, "ResourceId").getLength());

// release the lock
// print(dom);
String lockId = dom.getDocumentElement().getAttribute("lockId");
get("wfs?request=ReleaseLock&version=2.0&lockId=" + lockId);
}


}

0 comments on commit 201a3e9

Please sign in to comment.