Fix GeometryFactory.create to deep-copy polygonal geometry (#1158)

Author: dr-jts

modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryEditor.java

@@ -165,18 +165,21 @@ private Geometry editInternal(Geometry geometry, GeometryEditorOperation operati
 
   private Polygon editPolygon(Polygon polygon,
                               GeometryEditorOperation operation) {
+
     Polygon newPolygon = (Polygon) operation.edit(polygon, factory);
-    // create one if needed
+    // create empty polygon if needed (will be removed subsequently if a multi component)
     if (newPolygon == null)
       newPolygon = factory.createPolygon();
     if (newPolygon.isEmpty()) {
-      //RemoveSelectedPlugIn relies on this behaviour. [Jon Aquino]
+      //-- ensure empty polygons are copied
+      if (newPolygon == polygon) {
+        newPolygon = factory.createPolygon();
+      }
       return newPolygon;
     }
 
     LinearRing shell = (LinearRing) edit(newPolygon.getExteriorRing(), operation);
     if (shell == null || shell.isEmpty()) {
-      //RemoveSelectedPlugIn relies on this behaviour. [Jon Aquino]
       return factory.createPolygon();
     }
 

modules/core/src/test/java/org/locationtech/jts/geom/GeometryFactoryTest.java

@@ -17,16 +17,16 @@
 import org.locationtech.jts.io.ParseException;
 import org.locationtech.jts.io.WKTReader;
 
-import junit.framework.TestCase;
 import junit.textui.TestRunner;
+import test.jts.GeometryTestCase;
 
 
 /**
  * Tests for {@link GeometryFactory}.
  *
  * @version 1.13
  */
-public class GeometryFactoryTest extends TestCase {
+public class GeometryFactoryTest extends GeometryTestCase {
 
   PrecisionModel precisionModel = new PrecisionModel();
   GeometryFactory geometryFactory = new GeometryFactory(precisionModel, 0);
@@ -38,18 +38,27 @@ public static void main(String args[]) {
 
   public GeometryFactoryTest(String name) { super(name); }
 
-  public void testCreateGeometry() throws ParseException
+  public void testCreateGeometry()
   {
-    checkCreateGeometryExact("POINT EMPTY");
     checkCreateGeometryExact("POINT ( 10 20 )");
-    checkCreateGeometryExact("LINESTRING EMPTY");
+    checkCreateGeometryExact("MULTIPOINT ( (10 20), (30 40) )");
     checkCreateGeometryExact("LINESTRING(0 0, 10 10)");
     checkCreateGeometryExact("MULTILINESTRING ((50 100, 100 200), (100 100, 150 200))");
     checkCreateGeometryExact("POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200))");
     checkCreateGeometryExact("MULTIPOLYGON (((100 200, 200 200, 200 100, 100 100, 100 200)), ((300 200, 400 200, 400 100, 300 100, 300 200)))");
     checkCreateGeometryExact("GEOMETRYCOLLECTION (POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200)), LINESTRING (250 100, 350 200), POINT (350 150))");
   }
 
+  public void testCreateGeometryEmpty() {
+    checkCreateGeometryExact("POINT EMPTY");
+    checkCreateGeometryExact("LINESTRING EMPTY");
+    checkCreateGeometryExact("POLYGON EMPTY");
+    checkCreateGeometryExact("MULTIPOINT EMPTY");
+    checkCreateGeometryExact("MULTILINESTRING EMPTY");
+    checkCreateGeometryExact("MULTIPOLYGON EMPTY");
+    checkCreateGeometryExact("GEOMETRYCOLLECTION EMPTY");
+  }
+  
   public void testCreateEmpty() {
     checkEmpty( geometryFactory.createEmpty(0), Point.class);
     checkEmpty( geometryFactory.createEmpty(1), LineString.class);
@@ -70,7 +79,7 @@ private void checkEmpty(Geometry geom, Class clz) {
     assertTrue( geom.getClass() == clz );
   }
 
-  public void testDeepCopy() throws ParseException
+  public void testDeepCopy() 
   {
     Point g = (Point) read("POINT ( 10 10) ");
     Geometry g2 = geometryFactory.createGeometry(g);
@@ -100,7 +109,7 @@ public void testMultiPointCS()
    * 
    * @throws ParseException
    */
-  public void testCopyGeometryWithNonDefaultDimension() throws ParseException
+  public void testCopyGeometryWithNonDefaultDimension() 
   {
     GeometryFactory gf = new GeometryFactory(CoordinateArraySequenceFactory.instance());
     CoordinateSequence mpSeq = gf.getCoordinateSequenceFactory().create(1, 2);
@@ -116,15 +125,13 @@ public void testCopyGeometryWithNonDefaultDimension() throws ParseException
 
   }
 
-  private void checkCreateGeometryExact(String wkt) throws ParseException
+  private void checkCreateGeometryExact(String wkt) 
   {
     Geometry g = read(wkt);
     Geometry g2 = geometryFactory.createGeometry(g);
     assertTrue(g.equalsExact(g2));
+    // check a copy has been made
+    assertTrue(g != g2);
   }
 
-  private Geometry read(String wkt) throws ParseException
-  {
-    return reader.read(wkt);
-  }
 }

modules/core/src/test/java/org/locationtech/jts/geom/util/GeometryEditorTest.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2021 Martin Davis.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * and Eclipse Distribution License v. 1.0 which accompanies this distribution.
+ * The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v20.html
+ * and the Eclipse Distribution License is available at
+ *
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ */
+package org.locationtech.jts.geom.util;
+
+import org.locationtech.jts.geom.CoordinateSequence;
+import org.locationtech.jts.geom.CoordinateSequenceFactory;
+import org.locationtech.jts.geom.Geometry;
+
+import junit.textui.TestRunner;
+import test.jts.GeometryTestCase;
+
+public class GeometryEditorTest extends GeometryTestCase {
+
+  public static void main(String args[]) {
+    TestRunner.run(GeometryFixerTest.class);
+  }
+
+  public GeometryEditorTest(String name) {
+    super(name);
+  }
+
+  public void testCopy() {
+    checkCopy("POINT ( 10 20 )");
+    checkCopy("MULTIPOINT ( (10 20), (30 40) )");
+    checkCopy("LINESTRING(0 0, 10 10)");
+    checkCopy("MULTILINESTRING ((50 100, 100 200), (100 100, 150 200))");
+    checkCopy("POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200))");
+    checkCopy(
+        "MULTIPOLYGON (((100 200, 200 200, 200 100, 100 100, 100 200)), ((300 200, 400 200, 400 100, 300 100, 300 200)))");
+    checkCopy(
+        "GEOMETRYCOLLECTION (POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200)), LINESTRING (250 100, 350 200), POINT (350 150))");
+  }
+
+  public void testCopyEmpty() {
+    checkCopy("POINT EMPTY");
+    checkCopy("LINESTRING EMPTY");
+    checkCopy("POLYGON EMPTY");
+    checkCopy("MULTIPOINT EMPTY");
+    checkCopy("MULTILINESTRING EMPTY");
+    checkCopy("MULTIPOLYGON EMPTY");
+    checkCopy("GEOMETRYCOLLECTION EMPTY");
+  }
+
+  //========================================================
+  
+  private void checkCopy(String wkt) {
+    Geometry g = read(wkt);
+    Geometry g2 = copy(g);
+    assertTrue(g.equalsExact(g2));
+    // check a copy has been made
+    assertTrue(g != g2);
+  }
+
+  private static Geometry copy(Geometry g) {
+    GeometryEditor editor = new GeometryEditor(g.getFactory());
+    return editor.edit(g, new CoordSeqCloneOp(g.getFactory().getCoordinateSequenceFactory()));
+  }
+
+  private static class CoordSeqCloneOp extends GeometryEditor.CoordinateSequenceOperation {
+    CoordinateSequenceFactory coordinateSequenceFactory;
+
+    public CoordSeqCloneOp(CoordinateSequenceFactory coordinateSequenceFactory) {
+      this.coordinateSequenceFactory = coordinateSequenceFactory;
+    }
+
+    public CoordinateSequence edit(CoordinateSequence coordSeq, Geometry geometry) {
+      return coordinateSequenceFactory.create(coordSeq);
+    }
+  }
+}