Change for a more strict geojson handling.

- Do not handle m coordinate.
- Raise for unknown geojson `type`.
- Add various tests.

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,34 @@ 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)
+        @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 +66,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,35 +135,20 @@ 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::Polygon,
+             RGeo::Feature::MultiPoint,
+             RGeo::Feature::MultiLineString,
+             RGeo::Feature::MultiPolygon
           {
-            "type" => "LineString",
-            "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
-          }
-        when RGeo::Feature::MultiPolygon
-          {
-            "type" => "MultiPolygon",
+            "type" => object.geometry_type.type_name,
             "coordinates" => object.coordinates
           }
         when RGeo::Feature::GeometryCollection
@@ -178,7 +187,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.

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,7 +1,6 @@
 # frozen_string_literal: true
 
-require "minitest/autorun"
-require "rgeo/geo_json"
+require_relative "test_helper"
 
 class BasicTest < Minitest::Test # :nodoc:
   def setup
@@ -22,7 +21,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 +59,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 +69,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

test/coder/encode_geometry_test.rb

@@ -0,0 +1,147 @@
+# frozen_string_literal: true
+
+require_relative "../test_helper"
+
+class CoderTest < Minitest::Test # :nodoc:
+  include TestHelper
+  def setup
+    @geo_factory = RGeo::Cartesian.simple_factory(srid: 4326)
+    @entity_factory = RGeo::GeoJSON::EntityFactory.instance
+    @coder = RGeo::GeoJSON::Coder.new(
+      geo_factory: @geo_factory,
+      entity_factory: @entity_factory
+    )
+  end
+
+  def test_encode_geometry_point
+    point = rand_point
+    assert_equal(
+      { "type" => "Point", "coordinates" => [point.x, point.y] },
+      @coder.send(:encode_geometry, point)
+    )
+  end
+
+  def test_encode_geometry_linestring
+    point1 = rand_point
+    point2 = rand_point
+    point3 = rand_point
+    line = @geo_factory.line_string([point1, point2, point3])
+    assert_equal(
+      { "type" => "LineString", "coordinates" => [
+        [point1.x, point1.y], [point2.x, point2.y], [point3.x, point3.y]
+      ] },
+      @coder.send(:encode_geometry, line)
+    )
+  end
+
+  def test_encode_geometry_polygon
+    point1 = rand_point
+    point2 = rand_point
+    point3 = rand_point
+    exterior = @geo_factory.linear_ring([point1, point2, point3, point1])
+    polygon = @geo_factory.polygon(exterior)
+    assert_equal(
+      { "type" => "Polygon", "coordinates" => [[
+        [point1.x, point1.y],
+        [point2.x, point2.y],
+        [point3.x, point3.y],
+        [point1.x, point1.y]
+      ]] },
+      @coder.send(:encode_geometry, polygon)
+    )
+  end
+
+  def test_encode_geometry_polygon_with_one_hole
+    point1 = @geo_factory.point(0.1, 0.2)
+    point2 = @geo_factory.point(0.1, 9.2)
+    point3 = @geo_factory.point(10.1, 9.2)
+    point4 = @geo_factory.point(10.1, 0.2)
+    point5 = @geo_factory.point(4.1, 4.2)
+    point6 = @geo_factory.point(5.1, 6.2)
+    point7 = @geo_factory.point(6.1, 4.2)
+    exterior = @geo_factory.linear_ring([point1, point2, point3, point4, point1])
+    interior = @geo_factory.linear_ring([point5, point6, point7, point5])
+    polygon = @geo_factory.polygon(exterior, [interior])
+    assert_equal(
+      { "type" => "Polygon", "coordinates" => [
+        [
+          [point1.x, point1.y],
+          [point2.x, point2.y],
+          [point3.x, point3.y],
+          [point4.x, point4.y],
+          [point1.x, point1.y]
+        ],
+        [
+          [point5.x, point5.y],
+          [point6.x, point6.y],
+          [point7.x, point7.y],
+          [point5.x, point5.y]
+        ]
+      ] },
+      @coder.send(:encode_geometry, polygon)
+    )
+  end
+
+  def test_encode_geometry_multipoint
+    point1 = rand_point
+    point2 = rand_point
+    multipoint = @geo_factory.multi_point([point1, point2])
+    assert_equal(
+      { "type" => "MultiPoint", "coordinates" => [
+        [point1.x, point1.y], [point2.x, point2.y]
+      ] },
+      @coder.send(:encode_geometry, multipoint)
+    )
+  end
+
+  def test_encode_geometry_multilinestring
+    linestring1 = rand_linestring(2)
+    linestring2 = rand_linestring(3)
+    multilinestring = @geo_factory.multi_line_string([linestring1, linestring2])
+    assert_equal(
+      { "type" => "MultiLineString", "coordinates" => [
+        linestring1.coordinates,
+        linestring2.coordinates
+      ] },
+      @coder.send(:encode_geometry, multilinestring)
+    )
+  end
+
+  def test_encode_geometry_multipolygon
+    point1 = @geo_factory.point(0, 0)
+    point2 = @geo_factory.point(0, 10)
+    point3 = @geo_factory.point(10, 10)
+    point4 = @geo_factory.point(10, 0)
+    point5 = @geo_factory.point(4, 4)
+    point6 = @geo_factory.point(5, 6)
+    point7 = @geo_factory.point(6, 4)
+    point8 = @geo_factory.point(0, -10)
+    point9 = @geo_factory.point(-10, 0)
+    exterior1 = @geo_factory.linear_ring([point1, point8, point9, point1])
+    exterior2 = @geo_factory.linear_ring([point1, point2, point3, point4, point1])
+    interior2 = @geo_factory.linear_ring([point5, point6, point7, point5])
+    exterior3 = @geo_factory.linear_ring([point1, point2, point3, point1])
+    poly1 = @geo_factory.polygon(exterior1)
+    poly2 = @geo_factory.polygon(exterior2, [interior2])
+    poly3 = @geo_factory.polygon(exterior3)
+    multypolygon = @geo_factory.multi_polygon([poly1, poly2, poly3])
+    assert_equal(
+      { "type" => "MultiPolygon", "coordinates" => [
+        [ # poly1
+          exterior1.coordinates
+        ],
+        [ # poly2
+          exterior2.coordinates,
+          interior2.coordinates
+        ],
+        [ # poly3
+          exterior3.coordinates
+        ]
+      ] },
+      @coder.send(:encode_geometry, multypolygon)
+    )
+  end
+
+  def test_encode_geometry_geometrycollection
+  end
+end