Issue 509: Important fix on ValueGeometry

    Make ValueGeometry#getDimensionCount more reliable.
    Add unit test to check for illegal ValueGeometry comparison
    Add unit test for conversion of Geometry object into Object
    Add optional export to MANIFEST.MF for JTS Geometry classes
    Validate that geometry values can be represented in WKB.

h2/src/docsrc/html/changelog.html

@@ -81,6 +81,12 @@ Change Log
     extremely large statements.
 </li><li>Fix bug with ALLOW_LITERALS=NONE, where the periodic analyze table on insert would throw an exception.
 </li><li>Issue 510: Make org.h2.bnf public for consumption by external projects, patch by Nicolas Fortin
+</li><li>Issue 509:	Important fix on ValueGeometry, patch by Nicolas Fortin (with some tweaking)
+    Make ValueGeometry#getDimensionCount more reliable.
+    Add unit test to check for illegal ValueGeometry comparison
+    Add unit test for conversion of Geometry object into Object
+    Add optional export to MANIFEST.MF for JTS Geometry classes
+    Validate that geometry values can be represented in WKB.
 </li></ul>
 
 <h2>Version 1.3.173 (2013-07-28)</h2>

h2/src/main/META-INF/MANIFEST.MF

@@ -30,6 +30,8 @@ Import-Package: javax.management,
  org.apache.lucene.search;version="[3.0.0,3.1.0)";resolution:=optional,
  org.apache.lucene.store;version="[3.0.0,3.1.0)";resolution:=optional,
  org.apache.lucene.util;version="[3.0.0,3.1.0)";resolution:=optional,
+ com.vividsolutions.jts.geom;version="1.13";resolution:=optional,
+ com.vividsolutions.jts.io;version="1.13";resolution:=optional,
  org.h2;version="[${version},1.4.0)",
  org.h2.api;version="[${version},1.4.0)",
  org.h2.fulltext;version="[${version},1.4.0)",

h2/src/main/org/h2/value/ValueGeometry.java

@@ -8,9 +8,12 @@ package org.h2.value;
 
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
+import java.util.Arrays;
+
+import com.vividsolutions.jts.geom.CoordinateSequence;
+import com.vividsolutions.jts.geom.CoordinateSequenceFilter;
 import org.h2.message.DbException;
 import org.h2.util.StringUtils;
-import com.vividsolutions.jts.geom.Coordinate;
 import com.vividsolutions.jts.geom.Envelope;
 import com.vividsolutions.jts.geom.Geometry;
 import com.vividsolutions.jts.geom.GeometryFactory;
@@ -49,6 +52,9 @@ public class ValueGeometry extends Value {
     }
 
     private static ValueGeometry get(Geometry g) {
+        // not all WKT values can be represented in WKB, but since we persist it in WKB format,
+        // it has to be valid in WKB
+        toWKB(g);
         return (ValueGeometry) Value.cache(new ValueGeometry(g));
     }
 
@@ -59,7 +65,11 @@ public class ValueGeometry extends Value {
      * @return the value
      */
     public static ValueGeometry get(String s) {
-        return (ValueGeometry) Value.cache(new ValueGeometry(fromWKT(s)));
+        Geometry g = fromWKT(s);
+        // not all WKT values can be represented in WKB, but since we persist it in WKB format,
+        // it has to be valid in WKB
+        toWKB(g);
+        return (ValueGeometry) Value.cache(new ValueGeometry(g));
     }
 
     /**
@@ -184,7 +194,9 @@ public class ValueGeometry extends Value {
 
     @Override
     public boolean equals(Object other) {
-        return other instanceof ValueGeometry && geometry.equals(((ValueGeometry) other).geometry);
+      	// The JTS library only does half-way support for 3D coords, so
+      	// their equals method only checks the first two coords.
+        return other instanceof ValueGeometry && Arrays.equals(toWKB(), ((ValueGeometry) other).toWKB());
     }
 
     /**
@@ -202,23 +214,45 @@ public class ValueGeometry extends Value {
      * @return the well-known-binary
      */
     public byte[] toWKB() {
-        int dimensionCount = getDimensionCount();
+        return toWKB(geometry);
+    }
+    
+    private static byte[] toWKB(Geometry geometry) {
+        int dimensionCount = getDimensionCount(geometry);
         boolean includeSRID = geometry.getSRID() != 0;
         WKBWriter writer = new WKBWriter(dimensionCount, includeSRID);
         return writer.write(geometry);
     }
 
-    private int getDimensionCount() {
-        Coordinate[] coordinates = geometry.getCoordinates();
-        if (coordinates == null) {
-            return 2;
+    private static int getDimensionCount(Geometry geometry) {
+        ZVisitor finder = new ZVisitor();
+        geometry.apply(finder);
+        return finder.isFoundZ() ? 3 : 2;
+    }
+
+    private static class ZVisitor implements CoordinateSequenceFilter {
+        boolean foundZ = false;
+
+        public boolean isFoundZ() {
+            return foundZ;
+        }
+
+        @Override
+        public void filter(CoordinateSequence coordinateSequence, int i) {
+            if(!Double.isNaN(coordinateSequence.getOrdinate(i, 2))) {
+                foundZ = true;
             }
-        for (Coordinate coordinate : coordinates) {
-            if (!Double.isNaN(coordinate.z)) {
-                return 3;
         }
+
+        @Override
+        public boolean isDone() {
+            return foundZ;
+        }
+
+        @Override
+        public boolean isGeometryChanged() {
+            return false;
         }
-        return 2;
     }
 
     /**
@@ -249,4 +283,11 @@ public class ValueGeometry extends Value {
         }
     }
 
+    public Value convertTo(int targetType) {
+        if(targetType == Value.JAVA_OBJECT) {
+            return this;
+        } else {
+            return super.convertTo(targetType);
+        }
+    }
 }

h2/src/test/org/h2/test/db/TestSpatial.java

@@ -23,6 +23,7 @@ import com.vividsolutions.jts.geom.GeometryFactory;
 import com.vividsolutions.jts.geom.Polygon;
 import com.vividsolutions.jts.io.ParseException;
 import com.vividsolutions.jts.io.WKTReader;
+import org.h2.value.ValueGeometry;
 
 /**
  * Spatial datatype and index tests.
@@ -59,6 +60,9 @@ public class TestSpatial extends TestBase {
             testJavaAliasTableFunction();
             testMemorySpatialIndex();
             testGeometryDataType();
+            testWKB();
+            testValueConversion();
+            testEquals();
             deleteDb("spatial");
         }
     }
@@ -511,4 +515,64 @@ public class TestSpatial extends TestBase {
         Geometry geometry = geometryFactory.createPoint(new Coordinate(0, 0));
         assertEquals(Value.GEOMETRY, DataType.getTypeFromClass(geometry.getClass()));
     }
+
+    /**
+     * Test serialisation of Z and SRID values.
+     */
+    private void testWKB() {
+        ValueGeometry geom3d = ValueGeometry.get("POLYGON ((67 13 6, 67 18 5, 59 18 4, 59 13 6,  67 13 6))");
+        ValueGeometry copy = ValueGeometry.get(geom3d.getBytes());
+        assertEquals(6, copy.getGeometry().getCoordinates()[0].z);
+        assertEquals(5, copy.getGeometry().getCoordinates()[1].z);
+        assertEquals(4, copy.getGeometry().getCoordinates()[2].z);
+        // Test SRID
+        geom3d.getGeometry().setSRID(27572);
+        copy = ValueGeometry.get(geom3d.getBytes());
+        assertEquals(27572, copy.getGeometry().getSRID());
+    }
+
+    /**
+     * Test conversion of Geometry object into Object
+     */
+    private void testValueConversion() throws SQLException {
+        deleteDb("spatialIndex");
+        Connection conn = getConnection("spatialIndex");
+        Statement stat = conn.createStatement();
+        stat.execute("CREATE ALIAS OBJSTRING FOR \"" +
+                TestSpatial.class.getName() + ".getObjectString\"");
+        ResultSet rs = stat.executeQuery("select OBJSTRING('POINT( 15 25 )'::geometry)");
+        assertTrue(rs.next());
+        assertEquals("POINT (15 25)", rs.getString(1));
+        conn.close();
+        deleteDb("spatialIndex");
+    }
+
+    public static String getObjectString(Object object) {
+        return object.toString();
+    }
+
+    /**
+     * Test equality method on ValueGeometry
+     */
+    public void testEquals() {
+        // 3d equality test
+        ValueGeometry geom3d = ValueGeometry.get("POLYGON ((67 13 6, 67 18 5, 59 18 4, 59 13 6,  67 13 6))");
+        ValueGeometry geom2d = ValueGeometry.get("POLYGON ((67 13, 67 18, 59 18, 59 13,  67 13))");
+        assertFalse(geom3d.equals(geom2d));
+        // SRID equality test
+        GeometryFactory geometryFactory = new GeometryFactory();
+        Geometry geometry = geometryFactory.createPoint(new Coordinate(0, 0));
+        geometry.setSRID(27572);
+        ValueGeometry valueGeometry = ValueGeometry.getFromGeometry(geometry);
+        Geometry geometry2 = geometryFactory.createPoint(new Coordinate(0, 0));
+        geometry2.setSRID(5326);
+        ValueGeometry valueGeometry2 = ValueGeometry.getFromGeometry(geometry2);
+        assertFalse(valueGeometry.equals(valueGeometry2));
+        // Check illegal geometry (no WKB representation)
+        try {
+            ValueGeometry.get("POINT EMPTY");
+            fail("expected this to throw IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+        }
+    }
 }