Change for a more strict geojson handling.

lib/rgeo-geojson.rb

@@ -1,3 +1,6 @@
 # frozen_string_literal: true
 
-require "rgeo/geo_json"
+# Helper for bundler's `require: true` option.
+# See {file:lib/rgeo/geo_json/interface.rb} for documentation entry point.
+
+require_relative "rgeo/geo_json"

lib/rgeo/geo_json.rb

@@ -1,8 +1,16 @@
 # frozen_string_literal: true
 
-require "rgeo"
-require "rgeo/geo_json/version"
-require "rgeo/geo_json/entities"
-require "rgeo/geo_json/coder"
-require "rgeo/geo_json/interface"
 require "multi_json"
+require "rgeo"
+
+module RGeo
+  module GeoJSON
+    class Error < RGeo::Error::RGeoError
+    end
+  end
+end
+
+require_relative "geo_json/version"
+require_relative "geo_json/entities"
+require_relative "geo_json/coder"
+require_relative "geo_json/interface"

lib/rgeo/geo_json/coder.rb

@@ -6,8 +6,10 @@ module GeoJSON
     # the RGeo::Feature::Factory and the RGeo::GeoJSON::EntityFactory to
     # be used) so that you can encode and decode without specifying those
     # settings every time.
-
     class Coder
+      class Error < RGeo::GeoJSON::Error
+      end
+
       # Create a new coder settings object. The geo factory is passed as
       # a required argument.
       #
@@ -23,11 +25,37 @@ class Coder
       #   RGeo::GeoJSON::Feature or RGeo::GeoJSON::FeatureCollection.
       #   See RGeo::GeoJSON::EntityFactory for more information.
       def initialize(opts = {})
-        @geo_factory = opts[:geo_factory] || RGeo::Cartesian.preferred_factory
-        @entity_factory = opts[:entity_factory] || EntityFactory.instance
+        @geo_factory = opts.fetch(
+          :geo_factory,
+          RGeo::Cartesian.preferred_factory(uses_lenient_assertions: true)
+        )
+        @entity_factory = opts.fetch(:entity_factory, EntityFactory.instance)
+        if @geo_factory.property(:has_m_coordinate)
+          # If a GeoJSON has more than 2 elements, the first one should be
+          # longitude and the second one latitude. M is not part of GeoJSON
+          # specifications and only kept here for backward compatibilities.
+          #
+          # Quote from https://tools.ietf.org/html/rfc7946#section-3.1.1:
+          #
+          # > A position is an array of numbers.  There MUST be two or more
+          # > elements.  The first two elements are longitude and latitude, or
+          # > easting and northing, precisely in that order and using decimal
+          # > numbers.  Altitude or elevation MAY be included as an optional third
+          # > element.
+          # >
+          # > Implementations SHOULD NOT extend positions beyond three elements
+          # > because the semantics of extra elements are unspecified and
+          # > ambiguous.  Historically, some implementations have used a fourth
+          # > element to carry a linear referencing measure (sometimes denoted as
+          # > "M") or a numerical timestamp, but in most situations a parser will
+          # > not be able to properly interpret these values.  The interpretation
+          # > and meaning of additional elements is beyond the scope of this
+          # > specification, and additional elements MAY be ignored by parsers.
+          raise Error, "GeoJSON format cannot handle m coordinate."
+        end
+
         @num_coordinates = 2
         @num_coordinates += 1 if @geo_factory.property(:has_z_coordinate)
-        @num_coordinates += 1 if @geo_factory.property(:has_m_coordinate)
       end
 
       # Encode the given object as GeoJSON. The object may be one of the
@@ -41,17 +69,16 @@ def initialize(opts = {})
       # appropriate JSON library installed.
       #
       # Returns nil if nil is passed in as the object.
-
       def encode(object)
+        return nil if object.nil?
+
         if @entity_factory.is_feature_collection?(object)
           {
             "type" => "FeatureCollection",
             "features" => @entity_factory.map_feature_collection(object) { |f| encode_feature(f) },
           }
         elsif @entity_factory.is_feature?(object)
           encode_feature(object)
-        elsif object.nil?
-          nil
         else
           encode_geometry(object)
         end
@@ -111,36 +138,32 @@ def encode_feature(object)
       end
 
       def encode_geometry(object)
+        return nil if object.nil?
+        if object.factory.property(:has_m_coordinate)
+          raise Error, "GeoJSON format cannot handle m coordinate."
+        end
+
         case object
-        when RGeo::Feature::Point
-          {
-            "type" => "Point",
-            "coordinates" => object.coordinates
-          }
-        when RGeo::Feature::LineString
+        when RGeo::Feature::Point,
+             RGeo::Feature::LineString,
+             RGeo::Feature::MultiPoint,
+             RGeo::Feature::MultiLineString
           {
-            "type" => "LineString",
+            "type" => object.geometry_type.type_name,
             "coordinates" => object.coordinates
           }
         when RGeo::Feature::Polygon
           {
             "type" => "Polygon",
-            "coordinates" => object.coordinates
-          }
-        when RGeo::Feature::MultiPoint
-          {
-            "type" => "MultiPoint",
-            "coordinates" => object.coordinates
-          }
-        when RGeo::Feature::MultiLineString
-          {
-            "type" => "MultiLineString",
-            "coordinates" => object.coordinates
+            "coordinates" => right_hand_ruled_coordinates(object)
           }
         when RGeo::Feature::MultiPolygon
+          coordinates = Array.new(object.num_geometries) do |i|
+            right_hand_ruled_coordinates(object.geometry_n(i))
+          end
           {
             "type" => "MultiPolygon",
-            "coordinates" => object.coordinates
+            "coordinates" => coordinates
           }
         when RGeo::Feature::GeometryCollection
           {
@@ -152,6 +175,28 @@ def encode_geometry(object)
         end
       end
 
+      def right_hand_ruled_coordinates(polygon)
+        # Exterior should be ccw.
+        exterior = if clockwise?(polygon.exterior_ring)
+                     polygon.exterior_ring.coordinates.reverse
+                   else
+                     polygon.exterior_ring.coordinates
+                   end
+
+        interiors = polygon.interior_rings.map do |ring|
+          # Interiors should be cw.
+          next ring.coordinates if clockwise?(ring)
+
+          ring.coordinates.reverse
+        end
+
+        [exterior, *interiors]
+      end
+
+      def clockwise?(ring)
+        RGeo::Cartesian::Analysis.ring_direction(ring) == -1
+      end
+
       def decode_feature(input)
         geometry = input["geometry"]
         if geometry
@@ -178,7 +223,7 @@ def decode_geometry(input)
         when "MultiPolygon"
           decode_multi_polygon_coords(input["coordinates"])
         else
-          nil
+          raise Error, "'#{input['type']}' type is not part of GeoJSON spec."
         end
       end
 

lib/rgeo/geo_json/entities.rb

@@ -11,7 +11,6 @@ module GeoJSON
     # implementation need not subclass or even duck-type this class.
     # the entity factory mediates all interaction between the GeoJSON
     # engine and features.
-
     class Feature
       # Create a feature wrapping the given geometry, with the given ID
       # and properties.
@@ -95,7 +94,6 @@ def keys
     # FeatureCollection implementation need not subclass or even
     # duck-type this class. The entity factory mediates all interaction
     # between the GeoJSON engine and feature collections.
-
     class FeatureCollection
       include Enumerable
 

lib/rgeo/geo_json/interface.rb

@@ -1,6 +1,30 @@
 # frozen_string_literal: true
 
 module RGeo
+  # `RGeo::GeoJSON` is a part of `RGeo` designed to decode GeoJSON into
+  # `RGeo::Feature::Geometry`, or encode `RGeo::Feature::Geometry` objects as
+  #  GeoJSON.
+  #
+  # This implementation tries to stick to GeoJSON specifications, and may raise
+  # when trying to decode and invalid GeoJSON string. It may also raise if one
+  # tries to encode a feature that cannot be handled per GeoJSON spec.
+  #
+  # @example Basic usage
+  #   require 'rgeo/geo_json'
+  #
+  #   str1 = '{"type":"Point","coordinates":[1,2]}'
+  #   geom = RGeo::GeoJSON.decode(str1)
+  #   geom.as_text              # => "POINT (1.0 2.0)"
+  #
+  #   str2 = '{"type":"Feature","geometry":{"type":"Point","coordinates":[2.5,4.0]},"properties":{"color":"red"}}'
+  #   feature = RGeo::GeoJSON.decode(str2)
+  #   feature['color']          # => 'red'
+  #   feature.geometry.as_text  # => "POINT (2.5 4.0)"
+  #
+  #   hash = RGeo::GeoJSON.encode(feature)
+  #   hash.to_json == str2      # => true
+  #
+  # @see https://tools.ietf.org/html/rfc7946
   module GeoJSON
     class << self
       # High-level convenience routine for encoding an object as GeoJSON.
@@ -13,7 +37,6 @@ class << self
       # RGeo::GeoJSON::EntityFactory for more information. By default,
       # encode supports objects of type RGeo::GeoJSON::Feature and
       # RGeo::GeoJSON::FeatureCollection.
-
       def encode(object, opts = {})
         Coder.new(opts).encode(object)
       end

test/basic_test.rb

@@ -1,11 +1,12 @@
 # frozen_string_literal: true
 
-require "minitest/autorun"
-require "rgeo/geo_json"
+require_relative "test_helper"
 
 class BasicTest < Minitest::Test # :nodoc:
+  include TestHelper
+
   def setup
-    @geo_factory = RGeo::Cartesian.simple_factory(srid: 4326)
+    @geo_factory = RGeo::GeoJSON.coder.instance_variable_get(:@geo_factory)
     @geo_factory_z = RGeo::Cartesian.simple_factory(srid: 4326, has_z_coordinate: true)
     @geo_factory_m = RGeo::Cartesian.simple_factory(srid: 4326, has_m_coordinate: true)
     @geo_factory_zm = RGeo::Cartesian.simple_factory(srid: 4326, has_z_coordinate: true, has_m_coordinate: true)
@@ -22,7 +23,7 @@ def test_nil
   end
 
   def test_decode_simple_point
-    json = %({"type":"Point","coordinates":[1,2]})
+    json = '{"type":"Point","coordinates":[1,2]}'
     point = RGeo::GeoJSON.decode(json)
     assert_equal "POINT (1.0 2.0)", point.as_text
   end
@@ -60,8 +61,8 @@ def test_point_m
       "type" => "Point",
       "coordinates" => [10.0, 20.0, -1.0],
     }
-    assert_equal(json, RGeo::GeoJSON.encode(object))
-    assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_m).eql?(object))
+    assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.encode(object) }
+    assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_m) }
   end
 
   def test_point_zm
@@ -70,8 +71,8 @@ def test_point_zm
       "type" => "Point",
       "coordinates" => [10.0, 20.0, -1.0, -2.0],
     }
-    assert_equal(json, RGeo::GeoJSON.encode(object))
-    assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_zm).eql?(object))
+    assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.encode(object) }
+    assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_zm) }
   end
 
   def test_line_string
@@ -94,11 +95,30 @@ def test_polygon
     assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object))
   end
 
+  def test_not_simple_polygon
+    coordinates = [[0, 0], [2, 2], [2, 0], [0, 2], [0, 0]]
+    object = @geo_factory.polygon(line_string(coordinates))
+    json = {
+      "type" => "Polygon",
+      "coordinates" => [coordinates]
+    }
+    assert_equal(json, RGeo::GeoJSON.encode(object))
+    assert(
+      RGeo::GeoJSON.decode(json).eql?(object),
+      "It should decodes with the uses_lenient_assertions param"
+    )
+  end
+
   def test_polygon_complex
-    object = @geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(0, 0), @geo_factory.point(10, 0), @geo_factory.point(10, 10), @geo_factory.point(0, 10), @geo_factory.point(0, 0)]), [@geo_factory.linear_ring([@geo_factory.point(4, 4), @geo_factory.point(6, 5), @geo_factory.point(4, 6), @geo_factory.point(4, 4)])])
+    exterior = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]]
+    interior = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]]
+    object = @geo_factory.polygon(
+      line_string(exterior),
+      [line_string(interior)]
+    )
     json = {
       "type" => "Polygon",
-      "coordinates" => [[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [6.0, 5.0], [4.0, 6.0], [4.0, 4.0]]],
+      "coordinates" => [[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]]]
     }
     assert_equal(json, RGeo::GeoJSON.encode(object))
     assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object))
@@ -125,10 +145,23 @@ def test_multi_line_string
   end
 
   def test_multi_polygon
-    object = @geo_factory.multi_polygon([@geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(0, 0), @geo_factory.point(10, 0), @geo_factory.point(10, 10), @geo_factory.point(0, 10), @geo_factory.point(0, 0)]), [@geo_factory.linear_ring([@geo_factory.point(4, 4), @geo_factory.point(6, 5), @geo_factory.point(4, 6), @geo_factory.point(4, 4)])]), @geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(-10, -10), @geo_factory.point(-15, -10), @geo_factory.point(-10, -15), @geo_factory.point(-10, -10)]))])
+    exterior1 = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]]
+    interior1 = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]]
+    exterior2 = [[-10.0, -10.0], [-15.0, -10.0], [-10.0, -15.0], [-10.0, -10.0]]
+    polygon1 = @geo_factory.polygon(
+      line_string(exterior1),
+      [line_string(interior1)]
+    )
+    polygon2 = @geo_factory.polygon(
+      line_string(exterior2)
+    )
+    object = @geo_factory.multi_polygon([polygon1, polygon2])
     json = {
       "type" => "MultiPolygon",
-      "coordinates" => [[[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [6.0, 5.0], [4.0, 6.0], [4.0, 4.0]]], [[[-10.0, -10.0], [-15.0, -10.0], [-10.0, -15.0], [-10.0, -10.0]]]]
+      "coordinates" => [
+        [exterior1, interior1],
+        [exterior2]
+      ]
     }
     assert_equal(json, RGeo::GeoJSON.encode(object))
     assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object))
@@ -258,4 +291,39 @@ def test_feature_property
     assert_equal "b", feature.properties["a"]
     assert_equal "b", feature["a"]
   end
+
+  def test_right_hand_rule
+    ccw_exterior = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]]
+    cw_interior = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]]
+
+    json = { "type" => "Polygon", "coordinates" => [
+      ccw_exterior,
+      cw_interior
+    ] }
+
+    bad_interior = @geo_factory.polygon(
+      line_string(ccw_exterior),
+      [line_string(cw_interior.reverse)]
+    )
+    bad_exterior = @geo_factory.polygon(
+      line_string(ccw_exterior.reverse),
+      [line_string(cw_interior)]
+    )
+    bad_both = @geo_factory.polygon(
+      line_string(ccw_exterior.reverse),
+      [line_string(cw_interior.reverse)]
+    )
+    [bad_exterior, bad_interior, bad_both].each do |polygon|
+      assert_equal(json, RGeo::GeoJSON.encode(polygon))
+    end
+
+    multi_polygon = @geo_factory.multi_polygon(
+      [bad_both, bad_exterior, bad_interior]
+    )
+    assert_equal({ "type" => "MultiPolygon", "coordinates" => [
+      [ccw_exterior, cw_interior],
+      [ccw_exterior, cw_interior],
+      [ccw_exterior, cw_interior]
+    ] }, RGeo::GeoJSON.encode(multi_polygon))
+  end
 end