[GEOS-8506] Async WPS does not report wrong output parameters

geoserver · Feb 25, 2018 · 7cd240f · 7cd240f
1 parent a962d5a
commit 7cd240f
Show file tree

Hide file tree

Showing 7 changed files with 199 additions and 73 deletions.
diff --git a/src/extension/wps/wps-core/src/main/java/org/geoserver/wps/executor/ExecuteRequest.java b/src/extension/wps/wps-core/src/main/java/org/geoserver/wps/executor/ExecuteRequest.java
@@ -5,23 +5,15 @@
  */
 package org.geoserver.wps.executor;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-
 import net.opengis.wps10.DocumentOutputDefinitionType;
 import net.opengis.wps10.ExecuteType;
 import net.opengis.wps10.InputType;
 import net.opengis.wps10.OutputDefinitionType;
 import net.opengis.wps10.ResponseDocumentType;
 import net.opengis.wps10.ResponseFormType;
-
 import org.eclipse.emf.common.util.EList;
 import org.geoserver.ows.Ows11Util;
+import org.geoserver.platform.ServiceException;
 import org.geoserver.wps.WPSException;
 import org.geoserver.wps.ppio.ProcessParameterIO;
 import org.geoserver.wps.process.AbstractRawData;
@@ -34,6 +26,15 @@
 import org.opengis.feature.type.Name;
 import org.springframework.validation.Validator;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * Centralizes some common request parsing activities
  * 
@@ -42,12 +43,20 @@
  */
 public class ExecuteRequest {
 
-    ExecuteType request;
+    private final Name processName;
+    private final ProcessFactory pf;
+    private ExecuteType request;
 
-    LazyInputMap inputs;
+    private LazyInputMap inputs;
 
     public ExecuteRequest(ExecuteType request) {
         this.request = request;
+
+        processName = Ows11Util.name(request.getIdentifier());
+        pf = GeoServerProcessors.createProcessFactory(processName, true);
+        if(pf == null) {
+            throw new WPSException("Unknown process " + processName);
+        }
     }
 
     /**
@@ -96,19 +105,13 @@ public Name getProcessName() {
      */
     public LazyInputMap getProcessInputs(WPSExecutionManager manager) {
         if (inputs == null) {
-            return getInputsInternal(manager);
+            inputs = getInputsInternal(manager);
         }
         return inputs;
     }
 
     LazyInputMap getInputsInternal(WPSExecutionManager manager) {
         // get the input descriptors
-        Name processName = Ows11Util.name(request.getIdentifier());
-        ProcessFactory pf = GeoServerProcessors.createProcessFactory(processName, true);
-        if(pf == null) {
-            throw new WPSException("Unknown process " + processName);
-        }
-
         final Map<String, Parameter<?>> parameters = pf.getParameterInfo(processName);
         Map<String, InputProvider> providers = new LinkedHashMap<String, InputProvider>();
 
@@ -233,20 +236,47 @@ public boolean isLineageRequested() {
      * Returns null if nothing specific was requested, the list otherwise
      *
      */
-    public List<DocumentOutputDefinitionType> getRequestedOutputs() {
+    public List<OutputDefinitionType> getRequestedOutputs() {
         // in case nothing specific was requested
-        if (request.getResponseForm() == null
+        ResponseFormType responseForm = request.getResponseForm();
-                || request.getResponseForm().getResponseDocument() == null
+        if (responseForm == null) {
-                || request.getResponseForm().getResponseDocument().getOutput() == null) {
             return null;
         }
-
+
-        List<DocumentOutputDefinitionType> result = new ArrayList<DocumentOutputDefinitionType>();
+        if (responseForm.getRawDataOutput() != null) {
-        EList outputs = request.getResponseForm().getResponseDocument().getOutput();
+            return Collections.singletonList(responseForm.getRawDataOutput());
-        for (Object output : outputs) {
+        } else if (responseForm.getResponseDocument() != null
-            result.add((DocumentOutputDefinitionType) output);
+                && responseForm.getResponseDocument().getOutput() != null) {
+            List<OutputDefinitionType> result = new ArrayList<>();
+            EList outputs = responseForm.getResponseDocument().getOutput();
+            for (Object output : outputs) {
+                result.add((DocumentOutputDefinitionType) output);
+            }
+
+            return result;
         }
-        return result;
+
+        return null;
     }
 
+    /**
+     * Ensures the requested output are valid
+     * @param inputs
+     */
+    public void validateOutputs(Map inputs) {
+        Map<String, Parameter<?>> resultInfo = pf.getResultInfo(getProcessName(), inputs);
+
+        List<OutputDefinitionType> requestedOutputs = getRequestedOutputs();
+        if (requestedOutputs != null) {
+            for (OutputDefinitionType output : requestedOutputs) {
+                String outputIdentifier = output.getIdentifier().getValue();
+                if (!resultInfo.containsKey(outputIdentifier)) {
+                    String locator = output instanceof DocumentOutputDefinitionType ? 
+                            "ResponseDocument" : "RawDataOutput";
+                    throw new WPSException("Unknow output " + outputIdentifier, ServiceException
+                            .INVALID_PARAMETER_VALUE, locator);                    
+                }
+            }
+        }
+    }
 }
diff --git a/...tension/wps/wps-core/src/main/java/org/geoserver/wps/executor/ExecuteResponseBuilder.java b/...tension/wps/wps-core/src/main/java/org/geoserver/wps/executor/ExecuteResponseBuilder.java
@@ -193,11 +193,11 @@ public ExecuteResponseType build(ProgressListener listener) {
             }
 
             // output definitions, if any was requested explicitly
-            List<DocumentOutputDefinitionType> outputList = helper.getRequestedOutputs();
+            List<OutputDefinitionType> outputList = helper.getRequestedOutputs();
             if (outputList != null) {
                 OutputDefinitionsType outputs = f.createOutputDefinitionsType();
                 response.setOutputDefinitions(outputs);
-                for (DocumentOutputDefinitionType output : outputList) {
+                for (OutputDefinitionType output : outputList) {
                     outputs.getOutput().add(EMFUtils.clone(output, f, true));
                 }
             }

diff --git a/src/extension/wps/wps-core/src/main/java/org/geoserver/wps/executor/WPSExecutionManager.java b/src/extension/wps/wps-core/src/main/java/org/geoserver/wps/executor/WPSExecutionManager.java
@@ -170,6 +170,7 @@ public ExecuteResponseType submit(final ExecuteRequest request, boolean synchron
         ProcessManager processManager = getProcessManager(processName);
         String executionId = resourceManager.getExecutionId(synchronous);
         LazyInputMap inputs = request.getProcessInputs(WPSExecutionManager.this);
+        request.validateOutputs(inputs);
         ExecutionStatus status = new ExecutionStatus(processName, executionId,
                 request.isAsynchronous());
         status.setRequest(request.getRequest());

diff --git a/src/extension/wps/wps-core/src/test/java/org/geoserver/wps/ExecuteTest.java b/src/extension/wps/wps-core/src/test/java/org/geoserver/wps/ExecuteTest.java
@@ -5,33 +5,11 @@
  */
 package org.geoserver.wps;
 
-import static org.custommonkey.xmlunit.XMLAssert.assertXpathEvaluatesTo;
+import com.vividsolutions.jts.geom.Geometry;
-import static org.custommonkey.xmlunit.XMLAssert.assertXpathExists;
+import com.vividsolutions.jts.geom.Point;
-import static org.geoserver.data.test.MockData.PRIMITIVEGEOFEATURE;
+import com.vividsolutions.jts.geom.Polygon;
-import static org.hamcrest.CoreMatchers.instanceOf;
+import com.vividsolutions.jts.io.WKTReader;
-import static org.junit.Assert.assertArrayEquals;
+import net.opengis.ows11.BoundingBoxType;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.URL;
-import java.net.URLEncoder;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipInputStream;
-
-import javax.xml.namespace.QName;
-
 import org.apache.commons.codec.binary.Base64;
 import org.custommonkey.xmlunit.XMLUnit;
 import org.custommonkey.xmlunit.XpathEngine;
@@ -41,6 +19,7 @@
 import org.geoserver.data.test.SystemTestData.LayerProperty;
 import org.geoserver.ows.util.KvpUtils;
 import org.geoserver.platform.GeoServerExtensions;
+import org.geoserver.platform.ServiceException;
 import org.geoserver.platform.resource.Resource;
 import org.geoserver.test.RemoteOWSTestSupport;
 import org.geoserver.wps.executor.ExecutionStatus;
@@ -59,21 +38,41 @@
 import org.geotools.process.ProcessException;
 import org.geotools.referencing.CRS;
 import org.geotools.referencing.crs.DefaultGeographicCRS;
-import org.geotools.xml.PreventLocalEntityResolver;
 import org.geotools.xml.Parser;
+import org.geotools.xml.PreventLocalEntityResolver;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.opengis.feature.simple.SimpleFeatureType;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.w3c.dom.Document;
 
-import com.vividsolutions.jts.geom.Geometry;
+import javax.xml.namespace.QName;
-import com.vividsolutions.jts.geom.Point;
+import java.io.ByteArrayInputStream;
-import com.vividsolutions.jts.geom.Polygon;
+import java.io.IOException;
-import com.vividsolutions.jts.io.WKTReader;
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLEncoder;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
 
-import net.opengis.ows11.BoundingBoxType;
+import static org.custommonkey.xmlunit.XMLAssert.assertXpathEvaluatesTo;
+import static org.custommonkey.xmlunit.XMLAssert.assertXpathExists;
+import static org.geoserver.data.test.MockData.PRIMITIVEGEOFEATURE;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 public class ExecuteTest extends WPSTestSupport {
 
@@ -1800,7 +1799,86 @@ public void testMultiOutputProcess() throws Exception {
                 "/wps:ExecuteResponse/wps:ProcessOutputs/wps:Output[ows:Identifier='result']/wps:Data/wps:LiteralData",
                 d);
     }
-
+
+    /**
+     * Tests a process execution an invalid output identifier fails immediately with an appropriate error message
+     */
+    @Test
+    public void testWrongOutputIdentifierRawOutput() throws Exception {
+        String responseFormContents = 
+                "    <wps:RawDataOutput>\n" +
+                "      <ows:Identifier>fooBar</ows:Identifier>\n" +
+                "    </wps:RawDataOutput>\n";
+        String request = buildGetBoundsRequest(responseFormContents);
+
+        Document dom = postAsDOM(root(), request);
+
+        String message = checkOws11Exception(dom, ServiceException.INVALID_PARAMETER_VALUE, "RawDataOutput");
+        assertThat(message, containsString("fooBar"));
+    }
+
+    /**
+     * Tests a process execution an invalid output identifier fails immediately with an appropriate error message
+     */
+    @Test
+    public void testWrongOutputIdentifierDocumentOutputAsynch() throws Exception {
+        String responseFormContents =
+                "<wps:ResponseDocument storeExecuteResponse='true' status='true'>"
+                        + "<wps:Output>"
+                        + "<ows:Identifier>fooBar</ows:Identifier>"
+                        + "</wps:Output>"
+                        + "</wps:ResponseDocument>";
+        String request = buildGetBoundsRequest(responseFormContents);
+
+        Document dom = postAsDOM(root(), request);
+
+        String message = checkOws11Exception(dom, ServiceException.INVALID_PARAMETER_VALUE, "ResponseDocument");
+        assertThat(message, containsString("fooBar"));
+    }
+
+    /**
+     * Tests a process execution an invalid output identifier fails immediately with an appropriate error message
+     */
+    @Test
+    public void testWrongOutputIdentifierDocumentOutputSynch() throws Exception {
+        String responseFormContents =
+                "<wps:ResponseDocument>"
+                        + "<wps:Output>"
+                        + "<ows:Identifier>fooBar</ows:Identifier>"
+                        + "</wps:Output>"
+                        + "</wps:ResponseDocument>";
+        String request = buildGetBoundsRequest(responseFormContents);
+
+        Document dom = postAsDOM(root(), request);
+
+        String message = checkOws11Exception(dom, ServiceException.INVALID_PARAMETER_VALUE, "ResponseDocument");
+        assertThat(message, containsString("fooBar"));
+    }
+
+    public String buildGetBoundsRequest(String responseFormContents) {
+        return "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<wps:Execute version=\"1.0.0\" service=\"WPS\" xmlns:xsi=\"http://www" +
+                ".w3.org/2001/XMLSchema-instance\" xmlns=\"http://www.opengis.net/wps/1.0.0\" xmlns:wfs=\"http://www" +
+                ".opengis.net/wfs\" xmlns:wps=\"http://www.opengis.net/wps/1.0.0\" xmlns:ows=\"http://www.opengis" +
+                ".net/ows/1.1\" xmlns:gml=\"http://www.opengis.net/gml\" xmlns:ogc=\"http://www.opengis.net/ogc\" " +
+                "xmlns:wcs=\"http://www.opengis.net/wcs/1.1.1\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" " +
+                "xsi:schemaLocation=\"http://www.opengis.net/wps/1.0.0 http://schemas.opengis.net/wps/1.0.0/wpsAll" +
+                ".xsd\">\n" +
+                "  <ows:Identifier>gs:Bounds</ows:Identifier>\n" +
+                "  <wps:DataInputs>\n" +
+                "    <wps:Input>\n" +
+                "      <ows:Identifier>features</ows:Identifier>\n" +
+                "      <wps:Reference mimeType=\"text/xml; subtype=wfs-collection/1.0\" " +
+                "xlink:href=\"http://geoserver/wfs?service=WFS&amp;request=GetFeature&amp;typename=cite:Streams\" " +
+                "method=\"GET\"/>\n" +
+                "    </wps:Input>\n" +
+                "  </wps:DataInputs>\n" +
+                "  <wps:ResponseForm>\n" +
+                responseFormContents +
+                "  </wps:ResponseForm>\n" +
+                "</wps:Execute>";
+    }
+
 
     private void assertProgress(String statusLocation, String progress) throws Exception {
         Document dom = getAsDOM(statusLocation);

diff --git a/src/extension/wps/wps-core/src/test/java/org/geoserver/wps/gs/StoreCoverageTest.java b/src/extension/wps/wps-core/src/test/java/org/geoserver/wps/gs/StoreCoverageTest.java
@@ -142,7 +142,7 @@ public void testStoreWCS10() throws Exception {
                 + "</wps:DataInputs>"
                 + "<wps:ResponseForm>"
                 + "  <wps:RawDataOutput>"
-                + "    <ows:Identifier>result</ows:Identifier>"
+                + "    <ows:Identifier>coverageLocation</ows:Identifier>"
                 + "  </wps:RawDataOutput>" + "</wps:ResponseForm>" + "</wps:Execute>";
 
         MockHttpServletResponse response = postAsServletResponse(root(), xml);