From 11dd64c11194dbb0a4c13578e651b64b3476efe5 Mon Sep 17 00:00:00 2001
From: Tim Schaub <tim.schaub@gmail.com>
Date: Sun, 16 Sep 2012 23:06:44 -0600
Subject: [PATCH] Consistent projection handling (closes #26)

---
 .../org/geoscript/js/feature/Feature.java     | 54 +++++++++--------
 .../java/org/geoscript/js/feature/Field.java  |  4 +-
 .../java/org/geoscript/js/feature/Schema.java | 11 +++-
 .../java/org/geoscript/js/geom/Geometry.java  |  9 ++-
 .../org/geoscript/js/proj/Projection.java     |  2 +-
 .../tests/geoscript/feature/test_feature.js   | 60 ++++++++++++++++++-
 .../js/tests/geoscript/geom/test_point.js     | 32 ++++++++++
 .../js/tests/geoscript/layer/common.js        | 23 +++++++
 8 files changed, 163 insertions(+), 32 deletions(-)

diff --git a/src/main/java/org/geoscript/js/feature/Feature.java b/src/main/java/org/geoscript/js/feature/Feature.java
index 86dcd547..bb8c5584 100644
--- a/src/main/java/org/geoscript/js/feature/Feature.java
+++ b/src/main/java/org/geoscript/js/feature/Feature.java
@@ -3,7 +3,6 @@
 import org.geoscript.js.GeoObject;
 import org.geoscript.js.geom.Bounds;
 import org.geoscript.js.geom.Geometry;
-import org.geoscript.js.geom.GeometryWrapper;
 import org.geoscript.js.proj.Projection;
 import org.geotools.feature.simple.SimpleFeatureBuilder;
 import org.mozilla.javascript.Context;
@@ -23,6 +22,7 @@
 import org.opengis.feature.simple.SimpleFeature;
 import org.opengis.feature.simple.SimpleFeatureType;
 import org.opengis.feature.type.AttributeDescriptor;
+import org.opengis.feature.type.GeometryDescriptor;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 
 public class Feature extends GeoObject implements Wrapper {
@@ -35,11 +35,6 @@ public class Feature extends GeoObject implements Wrapper {
      */
     private SimpleFeature feature;
 
-    /**
-     * Optional coordinate reference system for the feature.
-     */
-    CoordinateReferenceSystem crs;
-    
     /**
      * Layer from which this feature was accessed.  Used to persist 
      * modifications.
@@ -158,36 +153,29 @@ public String getGeometryName() {
     
     @JSGetter
     public Geometry getGeometry() {
-        Geometry jsGeom = null;
-        com.vividsolutions.jts.geom.Geometry geometry = 
-            (com.vividsolutions.jts.geom.Geometry) feature.getDefaultGeometry();
-        if (geometry != null) {
-            jsGeom = (Geometry) GeometryWrapper.wrap(getParentScope(), geometry);
-            jsGeom.setProjection(getProjection());
-        }
-        return jsGeom;
+        String name = getGeometryName();
+        return (Geometry) get(name);
     }
     
     @JSSetter
     public void setGeometry(Geometry geometry) {
         String name = getGeometryName();
+        if (name == null) {
+            throw ScriptRuntime.constructError("Error", "Feature schema has no geometry field");
+        }
         set(name, geometry);
     }
     
-    @JSSetter
-    public void setProjection(Projection projection) {
-        CoordinateReferenceSystem crs = null;
-        if (projection != null) {
-            crs = projection.unwrap();
-        }
-        this.crs = crs;
-    }
-
     @JSGetter
     public Projection getProjection() {
         Projection projection = null;
-        if (crs != null) {
-            projection = new Projection(getParentScope(), crs);
+        SimpleFeatureType featureType = feature.getFeatureType();
+        GeometryDescriptor descriptor = featureType.getGeometryDescriptor();
+        if (descriptor != null) {
+            CoordinateReferenceSystem crs = descriptor.getCoordinateReferenceSystem();
+            if (crs != null) {
+                projection = new Projection(getParentScope(), crs);
+            }
         }
         return projection;
     }
@@ -219,7 +207,18 @@ public Feature set(String name, Object value) {
             if (!(value instanceof Geometry)) {
                 throw ScriptRuntime.constructError("Error", "Attempted to set geometry property to a non-geometry object: " + Context.toString(value));
             }
-            setProjection(((Geometry) value).getProjection());
+            Projection featureProj = getProjection();
+            Geometry geometry = (Geometry) value;
+            Projection geomProj = geometry.getProjection();
+            if (featureProj != null) {
+                if (geomProj != null) {
+                    if (!featureProj.equals(geometry.getProjection())) {
+                        value = geometry.transform(featureProj);
+                    }
+                }
+            } else if (geomProj != null) {
+                throw ScriptRuntime.constructError("Error", "Cannot add a geometry with a projection to a feature without a projection");
+            }
         }
         feature.setAttribute(name, jsToJava(value));
         if (layer != null) {
@@ -244,6 +243,9 @@ public Object get(String name) {
             value = Context.getUndefinedValue();
         } else {
             value = javaToJS(feature.getAttribute(name), getParentScope());
+            if (value instanceof Geometry) {
+                ((Geometry) value).setProjection(getProjection());
+            }
         }
         return value;
     }
diff --git a/src/main/java/org/geoscript/js/feature/Field.java b/src/main/java/org/geoscript/js/feature/Field.java
index 219cab1f..97633ed2 100644
--- a/src/main/java/org/geoscript/js/feature/Field.java
+++ b/src/main/java/org/geoscript/js/feature/Field.java
@@ -192,7 +192,9 @@ public Projection getProjection() {
         Projection projection = null;
         if (descriptor instanceof GeometryDescriptor) {
             CoordinateReferenceSystem crs = ((GeometryDescriptor) descriptor).getCoordinateReferenceSystem();
-            projection = new Projection(getParentScope(), crs);
+            if (crs != null) {
+                projection = new Projection(getParentScope(), crs);
+            }
         }
         return projection;
     }
diff --git a/src/main/java/org/geoscript/js/feature/Schema.java b/src/main/java/org/geoscript/js/feature/Schema.java
index 9eba26ed..1c39c3d5 100644
--- a/src/main/java/org/geoscript/js/feature/Schema.java
+++ b/src/main/java/org/geoscript/js/feature/Schema.java
@@ -3,6 +3,8 @@
 import java.util.List;
 
 import org.geoscript.js.GeoObject;
+import org.geoscript.js.geom.Geometry;
+import org.geoscript.js.proj.Projection;
 import org.geotools.feature.NameImpl;
 import org.geotools.feature.simple.SimpleFeatureTypeBuilder;
 import org.mozilla.javascript.Context;
@@ -217,7 +219,8 @@ public static Schema fromValues(Scriptable scope, NativeObject values) {
         Scriptable fields = cx.newArray(scope, names.length);
         for (int i=0; i<names.length; ++i) {
             String name = (String) names[i];
-            Object value = jsToJava(values.get(name));
+            Object jsValue = values.get(name, values);
+            Object value = jsToJava(jsValue);
             String typeName = Field.getTypeName(value);
             if (typeName == null) {
                 throw ScriptRuntime.constructError("Error", "Unable to determine type for field: " + name);
@@ -225,6 +228,12 @@ public static Schema fromValues(Scriptable scope, NativeObject values) {
             Scriptable fieldConfig = cx.newObject(scope);
             fieldConfig.put("name", fieldConfig, name);
             fieldConfig.put("type", fieldConfig, typeName);
+            if (jsValue instanceof Geometry) {
+                Projection projection = ((Geometry) jsValue).getProjection();
+                if (projection != null) {
+                    fieldConfig.put("projection", fieldConfig, projection.getId());
+                }
+            }
             Field field = new Field(scope, (NativeObject) fieldConfig);
             fields.put(i, fields, field);
         }
diff --git a/src/main/java/org/geoscript/js/geom/Geometry.java b/src/main/java/org/geoscript/js/geom/Geometry.java
index b9b01489..f56879a3 100644
--- a/src/main/java/org/geoscript/js/geom/Geometry.java
+++ b/src/main/java/org/geoscript/js/geom/Geometry.java
@@ -216,7 +216,9 @@ public Geometry transform(Object projObj) {
         } catch (TransformException e) {
             throw new RuntimeException("Failed to transform.", e);
         }
-        return (Geometry) GeometryWrapper.wrap(getParentScope(), transGeom);
+        Geometry transformed = (Geometry) GeometryWrapper.wrap(getParentScope(), transGeom);
+        transformed.projection = toProj;
+        return transformed;
     }
     
     @JSFunction 
@@ -264,7 +266,10 @@ public void setProjection(Object projObj) {
             } else if (projObj instanceof String) {
                 projection = new Projection(getParentScope(), (String) projObj);
             } else {
-                throw new RuntimeException("Set projection with Projection object or string identifier.");
+                throw ScriptRuntime.constructError("Error", "Set projection with Projection object or string identifier.");
+            }
+            if (this.projection != null && !projection.equals(this.projection)) {
+                throw ScriptRuntime.constructError("Error", "Geometry projection already set.  Use the transform method to transform coordinates.");
             }
         }
         this.projection = projection;
diff --git a/src/main/java/org/geoscript/js/proj/Projection.java b/src/main/java/org/geoscript/js/proj/Projection.java
index 647f0474..3f48b250 100644
--- a/src/main/java/org/geoscript/js/proj/Projection.java
+++ b/src/main/java/org/geoscript/js/proj/Projection.java
@@ -76,7 +76,7 @@ public Projection(String id) {
             try {
                 crs = CRS.parseWKT(id);
             } catch (Exception e2) {
-                throw new RuntimeException("Trouble creating projection", e2);
+                throw ScriptRuntime.constructError("Error", "Trouble creating projection: " + id);
             }
         }
     }
diff --git a/src/test/resources/org/geoscript/js/tests/geoscript/feature/test_feature.js b/src/test/resources/org/geoscript/js/tests/geoscript/feature/test_feature.js
index 4a199033..ee88d215 100644
--- a/src/test/resources/org/geoscript/js/tests/geoscript/feature/test_feature.js
+++ b/src/test/resources/org/geoscript/js/tests/geoscript/feature/test_feature.js
@@ -1,6 +1,7 @@
 var ASSERT = require("assert");
 var GEOM = require("geoscript/geom");
 var FEATURE = require("geoscript/feature");
+var Projection = require("geoscript/proj").Projection;
 
 exports["test: constructor"] = function() {
     
@@ -83,9 +84,9 @@ exports["test: set"] = function() {
     f.set("location", point);
     ASSERT.ok(point.equals(f.get("location")), "correct new location value using get");
     ASSERT.ok(point.equals(f.geometry), "correct new location value using geometry");
+    ASSERT.strictEqual(f.projection, null, "null projection");
     
     point = new GEOM.Point([3, 4]);
-    point.projection = "EPSG:4326";
     f.geometry = point;
     ASSERT.ok(point.equals(f.geometry), "geometry correctly set");
     
@@ -105,6 +106,63 @@ exports["test: set"] = function() {
     
 };
 
+exports["test: projection handling"] = function() {
+    var feature;
+    var point0 = new GEOM.Point([1, 2]);
+    var point1 = point0.clone();
+    point1.projection = "epsg:4326";
+    
+    // feature without a projection
+    feature = new FEATURE.Feature({
+        properties: {geom: point0}
+    });
+    ASSERT.strictEqual(feature.projection, null, "null feature projection");
+    ASSERT.strictEqual(feature.geometry.projection, null, "null geometry projection");
+    ASSERT.strictEqual(feature.schema.geometry.projection, null, "null schema geometry projection");
+    
+    ASSERT.throws(function() {
+        f.geometry = point1;
+    }, Error, "cant add geometry with projection to feature without");
+    
+    // feature with a projection
+    feature = new FEATURE.Feature({
+        properties: {geom: point1}
+    });
+    var projection = feature.projection;
+    ASSERT.ok(projection instanceof Projection, "feature projection");
+    ASSERT.strictEqual(projection.id, "EPSG:4326");
+    ASSERT.ok(projection.equals(feature.geometry.projection), "correct geom projection");
+    
+    var schema = new FEATURE.Schema({
+        name: "test-schema",
+        fields: [{
+            name: "geom", type: "Point", projection: "EPSG:3857"
+        }]
+    });
+    
+    feature = new FEATURE.Feature({
+        schema: schema
+    });
+    
+    projection = feature.projection;
+    ASSERT.ok(projection instanceof Projection, "feature projection");
+    ASSERT.strictEqual(projection.id, "EPSG:3857");
+    
+    feature.geometry = point0;
+    projection = feature.geometry.projection;
+    ASSERT.ok(projection instanceof Projection, "geometry projection");
+    ASSERT.strictEqual(projection.id, "EPSG:3857");
+    
+    feature.geometry = point1;
+    var geometry = feature.geometry;
+    projection = geometry.projection;
+    ASSERT.ok(projection instanceof Projection, "geometry projection");
+    ASSERT.strictEqual(projection.id, "EPSG:3857");
+    
+    ASSERT.ok(geometry.equals(point1.transform("EPSG:3857")));
+    
+};
+
 exports["test: bounds"] = function() {
 
     var schema = new FEATURE.Schema({fields: [
diff --git a/src/test/resources/org/geoscript/js/tests/geoscript/geom/test_point.js b/src/test/resources/org/geoscript/js/tests/geoscript/geom/test_point.js
index fdaf50a3..2876d30e 100644
--- a/src/test/resources/org/geoscript/js/tests/geoscript/geom/test_point.js
+++ b/src/test/resources/org/geoscript/js/tests/geoscript/geom/test_point.js
@@ -1,5 +1,6 @@
 var ASSERT = require("assert");
 var GEOM = require("geoscript/geom");
+var Projection = require("geoscript/proj").Projection;
 
 exports["test: constructor"] = function() {
     
@@ -124,6 +125,37 @@ exports["test: clone"] = function() {
     
 }
 
+exports["test: projection"] = function() {
+    
+    var projection;
+
+    var p = new GEOM.Point([-110, 45]);
+    projection = p.projection;
+    ASSERT.strictEqual(projection, null, "null projection by default");
+    
+    p.projection = "EPSG:4326";
+    projection = p.projection;
+    ASSERT.ok(projection instanceof Projection, "assigned projection");
+    ASSERT.strictEqual(projection.id, "EPSG:4326");
+    
+    ASSERT.throws(function() {
+        p.projection = "EPSG:900913";
+    }, Error, "cannot reset geometry projection");
+};
+
+exports["test: transform"] = function() {
+    var p = new GEOM.Point([-110, 45]);
+    p.projection = "EPSG:4326";
+    
+    var p2 = p.transform("EPSG:3857");
+    ASSERT.ok(p2 instanceof GEOM.Point);
+    var projection = p2.projection;
+    ASSERT.ok(projection instanceof Projection, "assigned projection");
+    ASSERT.strictEqual(p2.x.toFixed(3), "-12245143.987");
+    ASSERT.strictEqual(p2.y.toFixed(3), "5621521.486");
+};
+
+
 if (require.main == module.id) {
     system.exit(require("test").run(exports));
 }
diff --git a/src/test/resources/org/geoscript/js/tests/geoscript/layer/common.js b/src/test/resources/org/geoscript/js/tests/geoscript/layer/common.js
index afceb44d..98dac895 100644
--- a/src/test/resources/org/geoscript/js/tests/geoscript/layer/common.js
+++ b/src/test/resources/org/geoscript/js/tests/geoscript/layer/common.js
@@ -199,3 +199,26 @@ exports["test: remove"] = function(getLayer) {
         layer.workspace.close();
     };
 };
+
+exports["test: projection"] = function(getLayer) {
+    return function() {
+        var projection;
+        var layer = getLayer();
+        
+        projection = layer.projection;
+        ASSERT.ok(projection instanceof PROJ.Projection, "layer projection");
+        ASSERT.strictEqual(projection.id, "EPSG:4326");
+        
+        var feature = layer.get("STATE_ABBR = 'MT'");
+        ASSERT.ok(feature instanceof Feature, "got feature");
+        projection = feature.projection;
+        ASSERT.ok(projection instanceof PROJ.Projection, "feature projection");
+        ASSERT.strictEqual(projection.id, "EPSG:4326");
+        
+        var geometry = feature.geometry;
+        ASSERT.ok(geometry instanceof GEOM.Geometry, "got geometry");
+        projection = geometry.projection;
+        ASSERT.ok(projection instanceof PROJ.Projection, "geometry projection");
+        ASSERT.strictEqual(projection.id, "EPSG:4326");
+    }
+}