[GEOS-9179] Encode nested features as full GeoJSON features in comple…

…x GetFeature output

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/AbstractAppSchemaTestSupport.java

@@ -6,9 +6,12 @@
 
 package org.geoserver.test;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.awt.Color;
 import java.awt.image.BufferedImage;
@@ -22,13 +25,17 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TimeZone;
 import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
+import net.sf.json.JSON;
+import net.sf.json.JSONArray;
+import net.sf.json.JSONObject;
 import org.custommonkey.xmlunit.SimpleNamespaceContext;
 import org.custommonkey.xmlunit.XMLAssert;
 import org.custommonkey.xmlunit.XMLUnit;
@@ -639,4 +646,35 @@ protected String readResource(String resourcePath) {
             throw new RuntimeException(String.format("Error reading resource '%s'.", resourcePath));
         }
     }
+
+    /** Drills into nested JSON objects (won't traverse arrays though) */
+    protected JSONObject getNestedObject(JSONObject root, String... keys) {
+        JSONObject curr = root;
+        for (String key : keys) {
+            if (!curr.has(key)) {
+                fail("Could not find property " + key + " in " + curr);
+            }
+            curr = curr.getJSONObject(key);
+        }
+        return curr;
+    }
+
+    /**
+     * Helper method that just extracts \ looks for a station in the provided GeoJSON response based
+     * on its ID.
+     */
+    protected JSONObject getFeaturePropertiesById(JSON geoJson, String id) {
+        assertThat(geoJson, instanceOf(JSONObject.class));
+        JSONObject json = (JSONObject) geoJson;
+        JSONArray features = json.getJSONArray("features");
+        for (int i = 0; i < features.size(); i++) {
+            JSONObject feature = features.getJSONObject(i);
+            if (Objects.equals(id, feature.get("id"))) {
+                // we found the feature we are looking for
+                return feature.getJSONObject("properties");
+            }
+        }
+        // feature matching the provided ID not found
+        return null;
+    }
 }

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/FeatureChainingWfsTest.java

@@ -9,6 +9,7 @@
 import static org.geoserver.test.AbstractAppSchemaMockData.GSML_SCHEMA_LOCATION_URL;
 import static org.geoserver.test.AbstractAppSchemaMockData.GSML_URI;
 import static org.geoserver.test.FeatureChainingMockData.EX_URI;
+import static org.hamcrest.CoreMatchers.anyOf;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
@@ -29,6 +30,7 @@
 import java.util.List;
 import java.util.Set;
 import net.sf.json.JSON;
+import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
 import org.geoserver.catalog.FeatureTypeInfo;
 import org.geoserver.util.IOUtils;
@@ -488,6 +490,30 @@ public void testGetFeatureGML() {
         assertEquals("ows:ExceptionReport", doc.getDocumentElement().getNodeName());
     }
 
+    @Test
+    public void testGetFeatureJSON() throws Exception {
+        JSON json =
+                getAsJSON(
+                        "wfs?request=GetFeature&version=1.1"
+                                + ".0&typename=gsml:GeologicUnit&outputFormat=application/json&featureId=gu.25678");
+        print(json);
+        JSONObject properties = getFeaturePropertiesById(json, "gu.25678");
+        assertNotNull(properties);
+        // make sure these are not encoded as GeoJSON features even if they are GeoTools Feature
+        // objects
+        JSONArray colors = properties.getJSONArray("exposureColor");
+        assertNotNull(colors);
+        JSONObject color = colors.getJSONObject(0);
+        // no top level feature elements
+        assertFalse(color.has("type"));
+        assertFalse(color.has("geometry"));
+        assertFalse(color.has("properties"));
+        // but value and codespace right in instead
+        color = color.getJSONObject("CGI_TermValue").getJSONObject("value");
+        assertThat(color.getString("value"), anyOf(is("Blue"), is("Yellow")));
+        assertThat(color.getString("@codeSpace"), is("some:uri"));
+    }
+
     @Test
     public void testGetFeatureValid() {
         String path = "wfs?request=GetFeature&version=1.1.0&typename=gsml:MappedFeature";

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/GeoJsonOutputFormatWfsTest.java

@@ -5,14 +5,11 @@
 package org.geoserver.test;
 
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
 
-import java.util.Objects;
 import net.sf.json.JSON;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
@@ -68,6 +65,10 @@ public void addContent() {
                     "Borehole",
                     "Gsml32Borehole.xml",
                     "Gsml32Borehole.properties");
+            // tricky, the above registered GSML in a different URI and here we override
+            // works, but be on the lookout for issues when modifying the test
+            putNamespace(GSML_PREFIX, GSML_URI);
+            addFeatureType(GSML_PREFIX, "Borehole", "Borehole.xml", "Borehole.properties");
         }
     }
 
@@ -122,25 +123,6 @@ private void checkStation1Exists(JSON geoJson) {
                 containsString("http://www.stations.org/ms."));
     }
 
-    /**
-     * Helper method that just extracts \ looks for a station in the provided GeoJSON response based
-     * on its ID.
-     */
-    private JSONObject getFeaturePropertiesById(JSON geoJson, String id) {
-        assertThat(geoJson, instanceOf(JSONObject.class));
-        JSONObject json = (JSONObject) geoJson;
-        JSONArray features = json.getJSONArray("features");
-        for (int i = 0; i < features.size(); i++) {
-            JSONObject feature = features.getJSONObject(i);
-            if (Objects.equals(id, feature.get("id"))) {
-                // we found the feature we are looking for
-                return feature.getJSONObject("properties");
-            }
-        }
-        // feature matching the provided ID not found
-        return null;
-    }
-
     @Test
     public void testSimpleContentTimeEncoding() throws Exception {
         String path = "wfs?request=GetFeature&typename=gsmlbh:Borehole&outputFormat=json";
@@ -155,6 +137,7 @@ public void testSimpleContentTimeEncoding() throws Exception {
                         "SamplingFeatureComplex",
                         "relatedSamplingFeature",
                         "SF_Specimen",
+                        "properties",
                         "samplingTime",
                         "TimeInstant");
         // property file uses a java.util.Date, but the database uses a java.sql.Date, hence
@@ -178,8 +161,7 @@ public void testOneDimensionalEncoding() throws Exception {
                         "SamplingFeatureComplex",
                         "relatedSamplingFeature",
                         "SF_Specimen",
-                        "samplingLocation",
-                        "value");
+                        "geometry");
         JSONArray coordinates = samplingLocation.getJSONArray("coordinates");
         assertThat(coordinates.size(), is(2));
         JSONArray c1 = coordinates.getJSONArray(0);
@@ -190,15 +172,20 @@ public void testOneDimensionalEncoding() throws Exception {
         assertEquals(66.5, c2.getDouble(0), 0.1);
     }
 
-    /** Drills into nested JSON objects (won't traverse arrays though) */
-    private JSONObject getNestedObject(JSONObject root, String... keys) {
-        JSONObject curr = root;
-        for (String key : keys) {
-            if (!curr.has(key)) {
-                fail("Could not find property " + key + " in " + curr);
-            }
-            curr = curr.getJSONObject(key);
-        }
-        return curr;
+    @Test
+    public void testNestedFeatureEncoding() throws Exception {
+        String path = "wfs?request=GetFeature&typename=gsml:Borehole&outputFormat=json";
+        JSON json = getAsJSON(path);
+        print(json);
+        JSONObject properties = getFeaturePropertiesById(json, "BOREHOLE.WTB5");
+        assertThat(properties, is(notNullValue()));
+        JSONObject collar = getNestedObject(properties, "collarLocation", "BoreholeCollar");
+        assertEquals("BOREHOLE.COLLAR.WTB5", collar.getString("id"));
+        assertEquals("Feature", collar.getString("type"));
+        JSONObject collarGeometry = collar.getJSONObject("geometry");
+        JSONArray coordinates = collarGeometry.getJSONArray("coordinates");
+        assertThat(coordinates.size(), is(2));
+        assertEquals(-28.4139, coordinates.getDouble(0), 0.1);
+        assertEquals(121.142, coordinates.getDouble(1), 0.1);
     }
 }

src/wfs/src/main/java/org/geoserver/wfs/json/ComplexGeoJsonWriter.java

@@ -12,10 +12,17 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.stream.StreamSupport;
+import org.geotools.data.DataStoreFinder;
 import org.geotools.feature.FeatureCollection;
 import org.geotools.feature.FeatureIterator;
 import org.geotools.feature.NameImpl;
 import org.geotools.referencing.CRS;
+import org.geotools.util.logging.Logging;
 import org.locationtech.jts.geom.Geometry;
 import org.opengis.feature.Attribute;
 import org.opengis.feature.ComplexAttribute;
@@ -32,6 +39,35 @@
 /** GeoJSON writer capable of handling complex features. */
 class ComplexGeoJsonWriter {
 
+    static final Logger LOGGER = Logging.getLogger(ComplexGeoJsonWriter.class);
+
+    private static Class NON_FEATURE_TYPE_PROXY;
+
+    static {
+        try {
+            NON_FEATURE_TYPE_PROXY =
+                    Class.forName("org.geotools.data.complex.config.NonFeatureTypeProxy");
+        } catch (ClassNotFoundException e) {
+            // might be ok if the app-schema datastore is not around
+            if (StreamSupport.stream(
+                            Spliterators.spliteratorUnknownSize(
+                                    DataStoreFinder.getAllDataStores(), Spliterator.ORDERED),
+                            false)
+                    .anyMatch(
+                            f ->
+                                    f != null
+                                            && f.getClass()
+                                                    .getSimpleName()
+                                                    .equals("AppSchemaDataAccessFactory"))) {
+                LOGGER.log(
+                        Level.FINE,
+                        "Could not find NonFeatureTypeProxy yet App-schema is around, probably the class changed name, package or does not exist anymore",
+                        e);
+            }
+            NON_FEATURE_TYPE_PROXY = null;
+        }
+    }
+
     private final GeoJSONBuilder jsonWriter;
 
     private boolean geometryFound = false;
@@ -207,10 +243,15 @@ private void encodeChainedFeatures(String attributeName, List<Feature> chainedFe
         jsonWriter.key(attributeName);
         jsonWriter.array();
         for (Feature feature : chainedFeatures) {
-            // encode each chained feature
-            jsonWriter.object();
-            encodeProperties(null, feature.getType(), feature.getProperties());
-            jsonWriter.endObject();
+            // if it's GeoJSON compatible, encode as a full blown GeoJSON feature (must have a
+            // default geometry)
+            if (feature.getType().getGeometryDescriptor() != null) {
+                encodeFeature(feature);
+            } else {
+                jsonWriter.object();
+                encodeProperties(null, feature.getType(), feature.getProperties());
+                jsonWriter.endObject();
+            }
         }
         // end the JSON chained features array
         jsonWriter.endArray();
@@ -431,21 +472,37 @@ private Object getSimpleContent(ComplexAttribute property) {
     /** Encode a complex attribute as a JSON object. */
     private void encodeComplexAttribute(
             ComplexAttribute attribute, Map<NameImpl, String> attributes) {
-        // get the attribute name and start a JSON object
         String name = attribute.getName().getLocalPart();
-        jsonWriter.key(name);
-        jsonWriter.object();
-        // let's see if we have actually some properties to encode
-        if (attribute.getProperties() != null && !attribute.getProperties().isEmpty()) {
-            // encode the object properties, since this is not a top feature or a
-            // chained feature we don't need to explicitly handle the geometry attribute
-            encodeProperties(null, attribute.getType(), attribute.getProperties());
-        }
-        if (attributes != null && !attributes.isEmpty()) {
-            // encode the attributes list
-            encodeAttributes(attributes);
+        if (isFullFeature(attribute)) {
+            jsonWriter.key(name);
+            encodeFeature((Feature) attribute);
+        } else {
+            // get the attribute name and start a JSON object
+
+            jsonWriter.key(name);
+            jsonWriter.object();
+            // let's see if we have actually some properties to encode
+            if (attribute.getProperties() != null && !attribute.getProperties().isEmpty()) {
+                // encode the object properties, since this is not a top feature or a
+                // chained feature we don't need to explicitly handle the geometry attribute
+                encodeProperties(null, attribute.getType(), attribute.getProperties());
+            }
+            if (attributes != null && !attributes.isEmpty()) {
+                // encode the attributes list
+                encodeAttributes(attributes);
+            }
+            jsonWriter.endObject();
         }
-        jsonWriter.endObject();
+    }
+
+    /**
+     * Checks if an attribute is an actual feature, skipping the NonFeatureTypeProxy case app-schema
+     * is using for technical reasons
+     */
+    private boolean isFullFeature(ComplexAttribute attribute) {
+        return attribute instanceof Feature
+                && (NON_FEATURE_TYPE_PROXY == null
+                        || !NON_FEATURE_TYPE_PROXY.isInstance(attribute.getType()));
     }
 
     /**