Add setFixStructure() to WKB/WKT readers (#639)

Adds reader options for WKB and WKT in C++ and C API to request that readers close unclosed rings while processing polygonal input. Closes #587

capi/geos_c.cpp

@@ -1220,6 +1220,11 @@ extern "C" {
         GEOSWKTReader_destroy_r(handle, reader);
     }
 
+    void
+    GEOSWKTReader_setFixStructure(WKTReader* reader, char doFix)
+    {
+        GEOSWKTReader_setFixStructure_r(handle, reader, doFix);
+    }
 
     Geometry*
     GEOSWKTReader_read(WKTReader* reader, const char* wkt)
@@ -1289,6 +1294,11 @@ extern "C" {
         GEOSWKBReader_destroy_r(handle, reader);
     }
 
+    void
+    GEOSWKBReader_setFixStructure(WKBReader* reader, char doFix)
+    {
+        GEOSWKBReader_setFixStructure_r(handle, reader, doFix);
+    }
 
     Geometry*
     GEOSWKBReader_read(WKBReader* reader, const unsigned char* wkb, std::size_t size)

capi/geos_c.h.in

@@ -1744,6 +1744,12 @@ extern GEOSGeometry GEOS_DLL *GEOSWKTReader_read_r(
     GEOSWKTReader* reader,
     const char *wkt);
 
+/** \see GEOSWKTReader_setFixStructure */
+extern void GEOS_DLL GEOSWKTReader_setFixStructure_r(
+    GEOSContextHandle_t handle,
+    GEOSWKTReader *reader,
+    char doFix);
+
 /* ========== WKT Writer ========== */
 
 /** \see GEOSWKTReader_create */
@@ -1801,6 +1807,12 @@ extern void GEOS_DLL GEOSWKBReader_destroy_r(
     GEOSContextHandle_t handle,
     GEOSWKBReader* reader);
 
+/** \see GEOSWKBReader_setFixStructure */
+extern void GEOS_DLL GEOSWKBReader_setFixStructure_r(
+    GEOSContextHandle_t handle,
+    GEOSWKBReader *reader,
+    char doFix);
+
 /** \see GEOSWKBReader_read */
 extern GEOSGeometry GEOS_DLL *GEOSWKBReader_read_r(
     GEOSContextHandle_t handle,
@@ -1815,6 +1827,7 @@ extern GEOSGeometry GEOS_DLL *GEOSWKBReader_readHEX_r(
     const unsigned char *hex,
     size_t size);
 
+
 /* ========== WKB Writer ========== */
 
 /** \see GEOSWKBWriter_create */
@@ -1892,7 +1905,7 @@ extern GEOSGeoJSONReader GEOS_DLL *GEOSGeoJSONReader_create_r(
 extern void GEOS_DLL GEOSGeoJSONReader_destroy_r(GEOSContextHandle_t handle,
     GEOSGeoJSONReader* reader);
 
-/** \see GEOSWKTReader_read */
+/** \see GEOSGeoJSONReader_read */
 extern GEOSGeometry GEOS_DLL *GEOSGeoJSONReader_readGeometry_r(
     GEOSContextHandle_t handle,
     GEOSGeoJSONReader* reader,
@@ -4629,6 +4642,16 @@ extern GEOSGeometry GEOS_DLL *GEOSWKTReader_read(
     GEOSWKTReader* reader,
     const char *wkt);
 
+/**
+* Set the reader to automatically repair structural errors
+* in the input (currently just unclosed rings) while reading.
+* \param reader A WKT reader object, caller retains ownership
+* \param doFix Set to 1 to repair, 0 for no repair (default).
+*/
+extern void GEOS_DLL GEOSWKTReader_setFixStructure(
+    GEOSWKTReader *reader,
+    char doFix);
+
 /* ========= WKT Writer ========= */
 
 /**
@@ -4727,6 +4750,16 @@ extern GEOSWKBReader GEOS_DLL *GEOSWKBReader_create(void);
 extern void GEOS_DLL GEOSWKBReader_destroy(
     GEOSWKBReader* reader);
 
+/**
+* Set the reader to automatically repair structural errors
+* in the input (currently just unclosed rings) while reading.
+* \param reader A WKB reader object, caller retains ownership
+* \param doFix Set to 1 to repair, 0 for no repair (default).
+*/
+extern void GEOS_DLL GEOSWKBReader_setFixStructure(
+    GEOSWKBReader *reader,
+    char doFix);
+
 /**
 * Read a geometry from a well-known binary buffer.
 * \param reader A \ref GEOSWKBReader

capi/geos_ts_c.cpp

@@ -3040,6 +3040,14 @@ extern "C" {
         });
     }
 
+    void
+    GEOSWKTReader_setFixStructure_r(GEOSContextHandle_t extHandle, WKTReader* reader, char doFix)
+    {
+        return execute(extHandle, [&]() {
+            return reader->setFixStructure(doFix);
+        });
+    }
+
     Geometry*
     GEOSWKTReader_read_r(GEOSContextHandle_t extHandle, WKTReader* reader, const char* wkt)
     {
@@ -3139,6 +3147,14 @@ extern "C" {
         });
     }
 
+    void
+    GEOSWKBReader_setFixStructure_r(GEOSContextHandle_t extHandle, WKBReader* reader, char doFix)
+    {
+        return execute(extHandle, [&]() {
+            return reader->setFixStructure(doFix);
+        });
+    }
+
     struct membuf : public std::streambuf {
         membuf(char* s, std::size_t n)
         {

include/geos/geom/CoordinateSequence.h

@@ -184,14 +184,12 @@ class GEOS_DLL CoordinateSequence {
 
 
     /** \brief
-    * Tests whether an array of {@link Coordinate}s forms a ring,
-    * by checking length and closure.
-    * Self-intersection is not checked.
+    * Tests whether an a {@link CoordinateSequence} forms a ring,
+    * by checking length and closure. Self-intersection is not checked.
     *
-    * @param pts an array of Coordinates
     * @return true if the coordinate form a ring.
     */
-    static bool isRing(const CoordinateSequence *pts);
+    bool isRing() const;
 
     /// Reverse Coordinate order in given CoordinateSequence
     static void reverse(CoordinateSequence* cl);

include/geos/io/WKBReader.h

@@ -85,6 +85,8 @@ class GEOS_DLL WKBReader {
     /// Inizialize parser with default GeometryFactory.
     WKBReader();
 
+    void setFixStructure(bool doFixStructure);
+
     /**
      * \brief Reads a Geometry from an istream.
      *
@@ -132,6 +134,7 @@ class GEOS_DLL WKBReader {
     unsigned int inputDimension;
     bool hasZ;
     bool hasM;
+    bool fixStructure;
 
     ByteOrderDataInStream dis;
 

include/geos/io/WKTReader.h

@@ -69,12 +69,14 @@ class GEOS_DLL WKTReader {
     explicit WKTReader(const geom::GeometryFactory& gf)
         : geometryFactory(&gf)
         , precisionModel(gf.getPrecisionModel())
+        , fixStructure(false)
         {};
 
         /** @deprecated in 3.4.0 */
     explicit WKTReader(const geom::GeometryFactory* gf)
         : geometryFactory(gf)
         , precisionModel(gf->getPrecisionModel())
+        , fixStructure(false)
         {};
 
     /**
@@ -84,10 +86,16 @@ class GEOS_DLL WKTReader {
     WKTReader()
         : geometryFactory(geom::GeometryFactory::getDefaultInstance())
         , precisionModel(geometryFactory->getPrecisionModel())
+        , fixStructure(false)
         {};
 
     ~WKTReader() {};
 
+    void
+    setFixStructure(bool doFixStructure) {
+        fixStructure = doFixStructure;
+    }
+
     /// Parse a WKT string returning a Geometry
     template<typename T>
     std::unique_ptr<T> read(const std::string& wkt) const {
@@ -121,6 +129,7 @@ class GEOS_DLL WKTReader {
 private:
     const geom::GeometryFactory* geometryFactory;
     const geom::PrecisionModel* precisionModel;
+    bool fixStructure;
 
     void getPreciseCoordinate(io::StringTokenizer* tokenizer, geom::Coordinate&, std::size_t& dim) const;
 

src/geom/CoordinateSequence.cpp

@@ -157,15 +157,15 @@ CoordinateSequence::increasingDirection(const CoordinateSequence& pts)
     return 1;
 }
 
-/* public static */
+/* public */
 bool
-CoordinateSequence::isRing(const CoordinateSequence *pts)
+CoordinateSequence::isRing() const
 {
-    if (pts->size() < 4) return false;
+    if (size() < 4)
+        return false;
 
-    if (pts->getAt(0) != pts->getAt(pts->size()-1)) {
+    if (getAt(0) != getAt(size()-1))
         return false;
-    }
 
     return true;
 }

src/io/WKBReader.cpp

@@ -31,6 +31,7 @@
 #include <geos/geom/MultiPolygon.h>
 #include <geos/geom/CoordinateSequenceFactory.h>
 #include <geos/geom/CoordinateSequence.h>
+#include <geos/geom/CoordinateArraySequence.h>
 #include <geos/geom/PrecisionModel.h>
 
 #include <iomanip>
@@ -51,12 +52,19 @@ WKBReader::WKBReader(geom::GeometryFactory const& f)
     , inputDimension(2)
     , hasZ(false)
     , hasM(false)
+    , fixStructure(false)
     {}
 
 WKBReader::WKBReader()
     : WKBReader(*(GeometryFactory::getDefaultInstance()))
     {}
 
+void
+WKBReader::setFixStructure(bool doFixStructure)
+{
+    fixStructure = doFixStructure;
+}
+
 std::ostream&
 WKBReader::printHEX(std::istream& is, std::ostream& os)
 {
@@ -366,6 +374,12 @@ WKBReader::readLinearRing()
     std::size_t << "WKB npoints: " << size << std::endl;
 #endif
     auto pts = readCoordinateSequence(size);
+    // Replace unclosed ring with closed
+    if (fixStructure && !pts->isRing()) {
+        std::unique_ptr<CoordinateArraySequence> cas(new CoordinateArraySequence(*pts));
+        cas->closeRing();
+        pts.reset(cas.release());
+    }
     return factory.createLinearRing(std::move(pts));
 }
 

src/io/WKTReader.cpp

@@ -264,6 +264,11 @@ std::unique_ptr<LinearRing>
 WKTReader::readLinearRingText(StringTokenizer* tokenizer) const
 {
     auto&& coords = getCoordinates(tokenizer);
+    if (fixStructure && !coords->isRing()) {
+        std::unique_ptr<CoordinateArraySequence> cas(new CoordinateArraySequence(*coords));
+        cas->closeRing();
+        coords.reset(cas.release());
+    }
     return geometryFactory->createLinearRing(std::move(coords));
 }
 

src/operation/buffer/BufferCurveSetBuilder.cpp

@@ -207,7 +207,7 @@ BufferCurveSetBuilder::addLineString(const LineString* line)
      *
      * Singled-sided buffers currently treat rings as if they are lines.
      */
-    if (CoordinateSequence::isRing(coord.get()) && ! curveBuilder.getBufferParameters().isSingleSided()) {
+    if (coord->isRing() && ! curveBuilder.getBufferParameters().isSingleSided()) {
         addRingBothSides(coord.get(), distance);
     }
     else {

tests/unit/capi/GEOSGeomFromWKBTest.cpp

@@ -27,21 +27,17 @@ struct test_capigeosgeomfromwkb_data : public capitest::utility {
     }
 
     void
-    test_wkb(std::string const& wkbhex, std::string const& wkt)
+    test_wkb(const std::string& wkbhex, const std::string& wkt)
     {
         wkb_hex_decoder::binary_type wkb;
         wkb_hex_decoder::decode(wkbhex, wkb);
 
         geom1_ = GEOSGeomFromWKB_buf(&wkb[0], wkb.size());
         ensure("GEOSGeomFromWKB_buf failed to create geometry", nullptr != geom1_);
 
-        // TODO: Update test to compare with WKT-based geometry
-        (void)wkt;
-        //       ATM, some XYZ and XYZM geometries fail
-        //geom2_ = GEOSWKTReader_read(reader_, wkt.c_str());
-        //ensure ( 0 != geom2_ );
-        //char result = GEOSEquals(geom1_, geom2_);
-        //ensure_equals(result, char(1));
+        geom2_ = GEOSWKTReader_read(reader_, wkt.c_str());
+        ensure ("GEOSWKTReader_read failed to create geometry", nullptr != geom2_ );
+        ensure_geometry_equals(geom1_, geom2_);
     }
 };
 
@@ -109,17 +105,30 @@ void object::test<5>
     test_wkb(ewkb, wkt);
 }
 
-// TODO: Does GEOSGeomFromWKB_buf accept EWKB or WKB only?
-//       The cases below test EWKB input and they are failing.
-//template<>
-//template<>
-//void object::test<6>()
-//{
-//    // SELECT st_geomfromewkt('MULTIPOINT((0 0 1 1), (3 2 2 1))') ;
-//    std::string wkt("MULTIPOINT((0 0 1 1), (3 2 2 1))");
-//    std::string ewkb("01040000C00200000001010000C000000000000000000000000000000000000000000000F03F000000000000F03F01010000C0000000000000084000000000000000400000000000000040000000000000F03F");
-//    test_wkb(ewkb, wkt);
-//}
+// Check force close on unclosed rings
+template<>
+template<>
+void object::test<6>
+()
+{
+    geom1_ = GEOSWKTReader_read(reader_, "POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))");
+    ensure("geom1_ not null", geom1_ != nullptr);
+    GEOSWKTReader_setFixStructure(reader_, 1);
+    geom2_ = GEOSWKTReader_read(reader_, "POLYGON((0 0, 0 1, 1 1, 1 0))");
+    ensure("geom2_ not null", geom2_ != nullptr);
+    ensure_geometry_equals(geom1_, geom2_);
+}
+
+// Supply EWKB input
+template<>
+template<>
+void object::test<7>()
+{
+    test_wkb(
+        "01040000C00200000001010000C000000000000000000000000000000000000000000000F03F000000000000F03F01010000C0000000000000084000000000000000400000000000000040000000000000F03F",
+        "MULTIPOINT((0 0 1 1), (3 2 2 1))"
+    );
+}
 
 } // namespace tut
 

tests/unit/io/WKTReaderTest.cpp

@@ -10,6 +10,7 @@
 #include <geos/geom/GeometryFactory.h>
 #include <geos/geom/Geometry.h>
 #include <geos/geom/LineString.h>
+#include <geos/geom/Polygon.h>
 #include <geos/geom/Point.h>
 #include <geos/geom/CoordinateSequence.h>
 #include <geos/util/GEOSException.h>
@@ -303,5 +304,17 @@ void object::test<12>
     ensure("MULTIPOINT( EMPTY, (1 1))", geom3->getGeometryN(0)->isEmpty());
 }
 
+template<>
+template<>
+void object::test<13>
+()
+{
+    wktreader.setFixStructure(true);
+    auto geom = wktreader.read("POLYGON((0 0, 0 1, 1 1, 1 0))");
+    std::unique_ptr<geos::geom::Polygon> p(static_cast<geos::geom::Polygon*>(geom.release()));
+    ensure("setFixStructure", p->getExteriorRing()->getNumPoints() == 5);
+}
+
+
 
 } // namespace tut