Commit 0ca267f4 authored by Jody Garnett's avatar Jody Garnett
Browse files

Merge pull request #626 from smithkm/geojson-crs

Correct CRS encoding for WFS GeoJSON, Fixes GEOS-5996
parents 8310f479 7357f062
......@@ -9,8 +9,8 @@ previous version. See :ref:`migrating_data_directory` for more details.
This section contains details about upgrading to specific GeoServer versions.
Upgrade to 2.2
--------------
Upgrade to 2.2+
---------------
Security configuration
^^^^^^^^^^^^^^^^^^^^^^
......@@ -84,3 +84,7 @@ The solution to this problem is to reduce the administrative access role to that
Upgrade to 2.6+
---------------
Before 2.6, the GeoJSON produced by the WFS service used a non-standard encoding for the CRS. Setting ``GEOSERVER_GEOJSON_LEGACY_CRS=true`` as a system property, context parameter, or environment variable will enable the old behaviour.
......@@ -84,6 +84,11 @@
<artifactId>easymockclassextension</artifactId>
<version>2.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
......
......@@ -13,6 +13,7 @@ import net.sf.json.JSONException;
import net.sf.json.util.JSONBuilder;
import org.geotools.geometry.jts.coordinatesequence.CoordinateSequences;
import org.geotools.referencing.CRS;
import org.geotools.util.Converters;
import com.vividsolutions.jts.geom.Coordinate;
......@@ -41,6 +42,8 @@ public class GeoJSONBuilder extends JSONBuilder {
private final Logger LOGGER = org.geotools.util.logging.Logging
.getLogger(this.getClass());
private CRS.AxisOrder axisOrder = CRS.AxisOrder.EAST_NORTH;
public GeoJSONBuilder(Writer w) {
super(w);
}
......@@ -150,17 +153,18 @@ public class GeoJSONBuilder extends JSONBuilder {
}
private JSONBuilder writeCoordinate(double x, double y) {
this.array();
this.value(x);
this.value(y);
return this.endArray();
return writeCoordinate(x, y, Double.NaN);
}
private JSONBuilder writeCoordinate(double x, double y, double z) {
this.array();
this.value(x);
this.value(y);
if(axisOrder==CRS.AxisOrder.NORTH_EAST){
this.value(y);
this.value(x);
} else {
this.value(x);
this.value(y);
}
if(!Double.isNaN(z)) {
this.value(z);
}
......@@ -178,10 +182,17 @@ public class GeoJSONBuilder extends JSONBuilder {
protected JSONBuilder writeBoundingBox(Envelope env) {
this.key("bbox");
this.array();
this.value(env.getMinX());
this.value(env.getMinY());
this.value(env.getMaxX());
this.value(env.getMaxY());
if(axisOrder==CRS.AxisOrder.NORTH_EAST) {
this.value(env.getMinY());
this.value(env.getMinX());
this.value(env.getMaxY());
this.value(env.getMaxX());
} else {
this.value(env.getMinX());
this.value(env.getMinY());
this.value(env.getMaxX());
this.value(env.getMaxY());
}
return this.endArray();
}
......@@ -290,4 +301,13 @@ public class GeoJSONBuilder extends JSONBuilder {
super.value(value);
return this;
}
/**
* Set the axis order to assume all input will be provided in. Has no effect on geometries
* that have already been written.
* @param axisOrder
*/
public void setAxisOrder(CRS.AxisOrder axisOrder) {
this.axisOrder = axisOrder;
}
}
......@@ -26,6 +26,7 @@ import org.geoserver.catalog.FeatureTypeInfo;
import org.geoserver.config.GeoServer;
import org.geoserver.ows.Dispatcher;
import org.geoserver.ows.Request;
import org.geoserver.platform.GeoServerExtensions;
import org.geoserver.platform.Operation;
import org.geoserver.platform.ServiceException;
import org.geoserver.wfs.WFSGetFeatureOutputFormat;
......@@ -37,6 +38,10 @@ import org.geotools.data.Query;
import org.geotools.feature.FeatureCollection;
import org.geotools.feature.FeatureIterator;
import org.geotools.geometry.jts.ReferencedEnvelope;
import org.geotools.gml2.SrsSyntax;
import org.geotools.gml2.bindings.GML2EncodingUtils;
import org.geotools.referencing.CRS;
import org.geotools.referencing.CRS.AxisOrder;
import org.geotools.referencing.NamedIdentifier;
import org.opengis.feature.Feature;
import org.opengis.feature.simple.SimpleFeature;
......@@ -45,6 +50,7 @@ import org.opengis.feature.type.AttributeDescriptor;
import org.opengis.feature.type.FeatureType;
import org.opengis.feature.type.GeometryDescriptor;
import org.opengis.filter.Filter;
import org.opengis.referencing.FactoryException;
import org.opengis.referencing.ReferenceIdentifier;
import org.opengis.referencing.crs.CoordinateReferenceSystem;
......@@ -180,6 +186,11 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
}
GeometryDescriptor defaultGeomType = fType.getGeometryDescriptor();
CoordinateReferenceSystem featureCrs =
fType.getGeometryDescriptor().getCoordinateReferenceSystem();
jsonWriter.setAxisOrder(CRS.getAxisOrder(featureCrs));
if (crs == null && defaultGeomType != null)
crs = fType.getGeometryDescriptor().getCoordinateReferenceSystem();
......@@ -257,29 +268,18 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
}
jsonWriter.endArray(); // end features
// Coordinate Referense System, currently only if the namespace is
// EPSG
if (crs != null) {
Set<ReferenceIdentifier> ids = crs.getIdentifiers();
// WKT defined crs might not have identifiers at all
if (ids != null && ids.size() > 0) {
NamedIdentifier namedIdent = (NamedIdentifier) ids.iterator().next();
String csStr = namedIdent.getCodeSpace().toUpperCase();
if (csStr.equals("EPSG")) {
jsonWriter.key("crs");
jsonWriter.object();
jsonWriter.key("type").value(csStr);
jsonWriter.key("properties");
jsonWriter.object();
jsonWriter.key("code");
jsonWriter.value(namedIdent.getCode());
jsonWriter.endObject(); // end properties
jsonWriter.endObject(); // end crs
}
// Coordinate Referense System
try {
if ("true".equals(GeoServerExtensions.getProperty("GEOSERVER_GEOJSON_LEGACY_CRS"))){
// This is wrong, but GeoServer used to do it this way.
writeCrsLegacy(jsonWriter, crs);
} else {
writeCrs(jsonWriter, crs);
}
} catch (FactoryException e) {
throw (IOException) new IOException("Error looking up crs identifier").initCause(e);
}
// Bounding box for featurecollection
if (hasGeom && featureBounding) {
ReferencedEnvelope e = null;
......@@ -294,6 +294,7 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
}
if (e != null) {
jsonWriter.setAxisOrder(CRS.getAxisOrder(e.getCoordinateReferenceSystem()));
jsonWriter.writeBoundingBox(e);
}
}
......@@ -314,6 +315,61 @@ public class GeoJSONGetFeatureResponse extends WFSGetFeatureOutputFormat {
}
}
private void writeCrs(final GeoJSONBuilder jsonWriter,
CoordinateReferenceSystem crs) throws FactoryException {
if (crs != null) {
String identifier = CRS.lookupIdentifier(crs, true);
// If we get a plain EPSG code, generate a URI as the GeoJSON spec says to
// prefer them.
if(identifier.startsWith("EPSG:")) {
String code = GML2EncodingUtils.epsgCode(crs);
if (code != null) {
identifier = SrsSyntax.OGC_URN.getPrefix() + code;
}
}
jsonWriter.key("crs");
jsonWriter.object();
jsonWriter.key("type").value("name");
jsonWriter.key("properties");
jsonWriter.object();
jsonWriter.key("name");
jsonWriter.value(identifier);
jsonWriter.endObject(); // end properties
jsonWriter.endObject(); // end crs
} else {
jsonWriter.key("crs");
jsonWriter.value(null);
}
}
// Doesn't follow spec, but GeoServer used to do this.
private void writeCrsLegacy(final GeoJSONBuilder jsonWriter,
CoordinateReferenceSystem crs) {
// Coordinate Referense System, currently only if the namespace is
// EPSG
if (crs != null) {
Set<ReferenceIdentifier> ids = crs.getIdentifiers();
// WKT defined crs might not have identifiers at all
if (ids != null && ids.size() > 0) {
NamedIdentifier namedIdent = (NamedIdentifier) ids.iterator().next();
String csStr = namedIdent.getCodeSpace().toUpperCase();
if (csStr.equals("EPSG")) {
jsonWriter.key("crs");
jsonWriter.object();
jsonWriter.key("type").value(csStr);
jsonWriter.key("properties");
jsonWriter.object();
jsonWriter.key("code");
jsonWriter.value(namedIdent.getCode());
jsonWriter.endObject(); // end properties
jsonWriter.endObject(); // end crs
}
}
}
}
private String getCallbackFunction() {
Request request = Dispatcher.REQUEST.get();
if (request == null) {
......
......@@ -6,27 +6,38 @@
package org.geoserver.wfs.json;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import java.io.File;
import java.util.Collection;
import java.util.Collections;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.xml.namespace.QName;
import net.sf.json.JSON;
import net.sf.json.JSONArray;
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;
import org.geoserver.catalog.FeatureTypeInfo;
import org.geoserver.catalog.ProjectionPolicy;
import org.geoserver.config.GeoServer;
import org.geoserver.data.test.SystemTestData;
import org.geoserver.data.util.IOUtils;
import org.geoserver.wfs.WFSInfo;
import org.geoserver.wfs.WFSTestSupport;
import org.geotools.referencing.CRS;
import org.hamcrest.Description;
import org.hamcrest.Matchers;
import org.junit.Test;
import org.opengis.referencing.FactoryException;
import org.opengis.referencing.crs.CoordinateReferenceSystem;
import com.mockrunner.mock.web.MockHttpServletResponse;
......@@ -38,14 +49,37 @@ import com.mockrunner.mock.web.MockHttpServletResponse;
public class GeoJSONTest extends WFSTestSupport {
public static QName LINE3D = new QName(SystemTestData.CITE_URI, "Line3D", SystemTestData.CITE_PREFIX);
public static QName POINT_LATLON = new QName(SystemTestData.CITE_URI, "PointLatLon", SystemTestData.CITE_PREFIX);
public static QName POINT_LONLAT = new QName(SystemTestData.CITE_URI, "PointLonLat", SystemTestData.CITE_PREFIX);
@Override
@SuppressWarnings("unchecked")
protected void setUpInternal(SystemTestData data) throws Exception {
super.setUpInternal(data);
File security = new File(getTestData().getDataDirectoryRoot(), "security");
security.mkdir();
File layers = new File(security, "layers.properties");
IOUtils.copy(GeoJSONTest.class.getResourceAsStream("layers_ro.properties"), layers);
data.addVectorLayer (LINE3D, Collections.EMPTY_MAP, getClass(), getCatalog());
// A feature type with Lat-Lon/North-East axis ordering.
data.addVectorLayer (POINT_LATLON, Collections.EMPTY_MAP, getClass(), getCatalog());
CoordinateReferenceSystem crsLatLon = CRS.decode("urn:ogc:def:crs:EPSG::4326");
FeatureTypeInfo pointLatLon = getCatalog().getFeatureTypeByName(POINT_LATLON.getPrefix(), POINT_LATLON.getLocalPart());
pointLatLon.setNativeCRS(crsLatLon);
pointLatLon.setSRS("urn:ogc:def:crs:EPSG::4326");
pointLatLon.setProjectionPolicy(ProjectionPolicy.FORCE_DECLARED);
getCatalog().save(pointLatLon);
// A feature type with Lon-Lat/East-North axis ordering.
data.addVectorLayer (POINT_LONLAT, Collections.EMPTY_MAP, getClass(), getCatalog());
CoordinateReferenceSystem crsLonLat = CRS.decode("EPSG:4326", true);
FeatureTypeInfo pointLonLat = getCatalog().getFeatureTypeByName(POINT_LONLAT.getPrefix(), POINT_LONLAT.getLocalPart());
pointLatLon.setNativeCRS(crsLonLat);
pointLatLon.setSRS("EPSG:4326");
pointLatLon.setProjectionPolicy(ProjectionPolicy.FORCE_DECLARED);
getCatalog().save(pointLonLat);
}
@Test
......@@ -192,6 +226,10 @@ public class GeoJSONTest extends WFSTestSupport {
geomArray = geomArray.getJSONArray(0);
geomArray = geomArray.getJSONArray(0);
assertEquals(geomArray.getString(0), "55.174");
CoordinateReferenceSystem expectedCrs = getCatalog().getLayerByName(getLayerId(SystemTestData.AGGREGATEGEOFEATURE)).getResource().getCRS();
JSONObject aCRS = rootObject.getJSONObject("crs");
assertThat(aCRS.getString("type"), equalTo("name"));
assertThat(aCRS, encodesCRS(expectedCrs));
}
@Test
......@@ -303,7 +341,7 @@ public class GeoJSONTest extends WFSTestSupport {
+ "&outputformat=" + JSONType.json);
// print(collection);
assertEquals(1, collection.getInt("totalFeatures"));
assertEquals("4327", collection.getJSONObject("crs").getJSONObject("properties").getString("code"));
//assertEquals("4327", collection.getJSONObject("crs").getJSONObject("properties").getString("code"));
JSONArray features = collection.getJSONArray("features");
assertEquals(1, features.size());
JSONObject feature = features.getJSONObject(0);
......@@ -318,5 +356,108 @@ public class GeoJSONTest extends WFSTestSupport {
assertEquals(120, c2.getInt(0));
assertEquals(0, c2.getInt(1));
assertEquals(100, c2.getInt(2));
CoordinateReferenceSystem expectedCrs = CRS.decode("EPSG:4327");
JSONObject aCRS = collection.getJSONObject("crs");
assertThat(aCRS, encodesCRS(expectedCrs));
}
// Checks that the result is in EAST_NORTH/LON_LAT order regardless of the source order
protected void doAxisSwapTest(QName layer, CRS.AxisOrder sourceOrder) throws Exception {
// Failure here means the setup for the test is broken and would invalidate the test
assertThat(CRS.getAxisOrder(
getCatalog().getFeatureTypeByName(layer.getPrefix(), layer.getLocalPart()).getCRS()
), is(sourceOrder));
JSONObject collection = (JSONObject) getAsJSON("wfs?request=GetFeature&version=1.0.0&typename=" + getLayerId(layer)
+ "&outputformat=" + JSONType.json);
// print(collection);
assertThat(collection.getInt("totalFeatures"), is(3));
//assertEquals("4327", collection.getJSONObject("crs").getJSONObject("properties").getString("code"));
JSONArray features = collection.getJSONArray("features");
assertThat((Collection<?>)features, Matchers.hasSize(3));
JSONObject feature = features.getJSONObject(0);
JSONObject geometry = feature.getJSONObject("geometry");
assertThat(geometry.getString("type"), is("Point"));
JSONArray coords = geometry.getJSONArray("coordinates");
assertThat((Iterable<?>)coords, contains((Object)120, 0));
JSONArray bbox = collection.getJSONArray("bbox");
assertThat((Iterable<?>)bbox, Matchers.contains((Object)(-170), -30, 120, 45));
CoordinateReferenceSystem expectedCrs = CRS.decode("EPSG:4326");
JSONObject aCRS = collection.getJSONObject("crs");
assertThat(aCRS, encodesCRS(expectedCrs));
}
@Test
public void testGetFeatureAxisSwap() throws Exception {
// Check that a NORTH_EAST source is swapped
doAxisSwapTest(POINT_LATLON, CRS.AxisOrder.NORTH_EAST);
}
@Test
public void testGetFeatureNoAxisSwap() throws Exception {
// Check that an EAST_NORTH source is not swapped
doAxisSwapTest(POINT_LONLAT, CRS.AxisOrder.EAST_NORTH);
}
@Test
public void testGetFeatureCRS() throws Exception {
QName layer = SystemTestData.LINES;
JSONObject collection = (JSONObject) getAsJSON("wfs?request=GetFeature&version=1.0.0&typename=" + getLayerId(layer)
+ "&outputformat=" + JSONType.json);
CoordinateReferenceSystem expectedCrs = getCatalog().getLayerByName(getLayerId(layer)).getResource().getCRS();
JSONObject aCRS = collection.getJSONObject("crs");
assertThat(aCRS, encodesCRS(expectedCrs));
}
private org.hamcrest.Matcher<JSONObject> encodesCRS(final CoordinateReferenceSystem crs) {
return new org.hamcrest.BaseMatcher<JSONObject>(){
@Override
public boolean matches(Object item) {
// Try to decode the CRS with both axis orders and check if either matches against
// the expected CRS. Sorry, this is a horrible hack. KS
CoordinateReferenceSystem decodedDefault = decodeCRS((JSONObject)item, false);
if(CRS.equalsIgnoreMetadata(crs, decodedDefault)) return true;
CoordinateReferenceSystem decodedXY = decodeCRS((JSONObject)item, true);
if(CRS.equalsIgnoreMetadata(crs, decodedXY)) return true;
String identifier = ((JSONObject)item).getJSONObject("properties").getString("name");
Pattern p = Pattern.compile("^urn:ogc:def:crs:EPSG:[^:]*:(\\d+)$");
Matcher m = p.matcher(identifier);
if(m.matches()){
String code = "EPSG:"+m.group(1);
CoordinateReferenceSystem decodedStripped;
try {
decodedStripped = CRS.decode(code, true);
} catch (FactoryException e) {
throw new IllegalStateException(e);
}
if(CRS.equalsIgnoreMetadata(crs, decodedStripped)) return true;
}
return false;
}
@Override
public void describeTo(Description description) {
description.appendText("JSON representation of CRS ");
description.appendValue(crs);
}
};
}
static private CoordinateReferenceSystem decodeCRS(JSONObject json, boolean forceXY) {
if(!json.getString("type").equals("name")) throw new IllegalArgumentException();
String identifier = json.getJSONObject("properties").getString("name");
try {
return CRS.decode(identifier, forceXY);
}catch (FactoryException e) {
throw new IllegalStateException(e);
}
}
}
_=the_geom:Point:srid=4326,Name:String
PointLatLon.0=POINT(0 120)| foo
PointLatLon.1=POINT(45 -170)| bar
PointLatLon.2=POINT(-30 60)| baz
\ No newline at end of file
_=the_geom:Point:srid=4326,Name:String
PointLatLon.0=POINT(120 0)| foo
PointLatLon.1=POINT(-170 45)| bar
PointLatLon.2=POINT(60 -30)| baz
\ No newline at end of file
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