From 6556df2a8586670bead95e1b064b712ea0989899 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 5 May 2020 10:26:13 +0200 Subject: [PATCH] Fix a crash in case of invalid geometry --- app/models/champs/carte_champ.rb | 4 +- app/models/dossier.rb | 4 +- app/models/geo_area.rb | 2 + spec/factories/geo_area.rb | 23 +++++++++ spec/models/geo_area_spec.rb | 88 ++++++++++++++++++++------------ 5 files changed, 83 insertions(+), 38 deletions(-) diff --git a/app/models/champs/carte_champ.rb b/app/models/champs/carte_champ.rb index 6bdc54f9e..3aa35bac9 100644 --- a/app/models/champs/carte_champ.rb +++ b/app/models/champs/carte_champ.rb @@ -58,8 +58,8 @@ class Champs::CarteChamp < Champ bounding_box = RGeo::Cartesian::BoundingBox.new(factory) if geo_areas.present? - geo_areas.each do |area| - bounding_box.add(area.rgeo_geometry) + geo_areas.map(&:rgeo_geometry).compact.each do |geometry| + bounding_box.add(geometry) end elsif dossier.present? point = dossier.geo_position diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 5aeb5c359..a2cc20fcd 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -723,8 +723,8 @@ class Dossier < ApplicationRecord factory = RGeo::Geographic.simple_mercator_factory bounding_box = RGeo::Cartesian::BoundingBox.new(factory) - geo_areas.each do |area| - bounding_box.add(area.rgeo_geometry) + geo_areas.map(&:rgeo_geometry).compact.each do |geometry| + bounding_box.add(geometry) end [bounding_box.max_point, bounding_box.min_point].compact.flat_map(&:coordinates) diff --git a/app/models/geo_area.rb b/app/models/geo_area.rb index 604a37c3e..fe0ca9f2f 100644 --- a/app/models/geo_area.rb +++ b/app/models/geo_area.rb @@ -49,6 +49,8 @@ class GeoArea < ApplicationRecord def rgeo_geometry RGeo::GeoJSON.decode(geometry.to_json, geo_factory: RGeo::Geographic.simple_mercator_factory) + rescue RGeo::Error::InvalidGeometry + nil end def self.from_feature_collection(feature_collection) diff --git a/spec/factories/geo_area.rb b/spec/factories/geo_area.rb index 406844ef3..e46aa7bea 100644 --- a/spec/factories/geo_area.rb +++ b/spec/factories/geo_area.rb @@ -35,5 +35,28 @@ FactoryBot.define do } end end + + trait :line_string do + geometry do + { + "type": "LineString", + "coordinates": [ + [2.4282521009445195, 46.53841410755813], + [2.42824137210846, 46.53847314771794], + [2.428284287452698, 46.53847314771794], + [2.4284291267395024, 46.538491597754714] + ] + } + end + end + + trait :point do + geometry do + { + "type": "Point", + "coordinates": [2.428439855575562, 46.538476837725796] + } + end + end end end diff --git a/spec/models/geo_area_spec.rb b/spec/models/geo_area_spec.rb index 08ede4acd..15c394df8 100644 --- a/spec/models/geo_area_spec.rb +++ b/spec/models/geo_area_spec.rb @@ -1,50 +1,70 @@ RSpec.describe GeoArea, type: :model do describe '#area' do - let(:geo_area) do - create(:geo_area, geometry: { - "type": "Polygon", - "coordinates": [ - [ - [2.428439855575562, 46.538476837725796], - [2.4284291267395024, 46.53842148758162], - [2.4282521009445195, 46.53841410755813], - [2.42824137210846, 46.53847314771794], - [2.428284287452698, 46.53847314771794], - [2.428364753723145, 46.538487907747864], - [2.4284291267395024, 46.538491597754714], - [2.428439855575562, 46.538476837725796] - ] - ] - }) - end + let(:geo_area) { create(:geo_area, :polygon) } it { expect(geo_area.area).to eq(219.0) } end describe '#length' do - let(:geo_area) do - create(:geo_area, geometry: { - "type": "LineString", - "coordinates": [ - [2.4282521009445195, 46.53841410755813], - [2.42824137210846, 46.53847314771794], - [2.428284287452698, 46.53847314771794], - [2.4284291267395024, 46.538491597754714] - ] - }) - end + let(:geo_area) { create(:geo_area, :line_string) } it { expect(geo_area.length).to eq(30.8) } end describe '#location' do - let(:geo_area) do - create(:geo_area, geometry: { - "type": "Point", - "coordinates": [2.428439855575562, 46.538476837725796] - }) - end + let(:geo_area) { create(:geo_area, :point) } it { expect(geo_area.location).to eq("2°25'42\"N 46°32'19\"E") } end + + describe '#rgeo_geometry' do + let(:geo_area) { create(:geo_area, geometry: geometry) } + + context 'invalid' do + let(:geometry) do + { + "type" => "MultiPolygon", + "coordinates" => [ + [ + [ + [5.894422531127931, 48.22810341752755], + [5.893049240112306, 48.22427237832278], + [5.892534255981446, 48.22593062452037], + [5.892791748046875, 48.2260449843468], + [5.894422531127931, 48.229933066408215], + [5.894422531127931, 48.22810341752755] + ] + ], + [ + [ + [5.8950233459472665, 48.229933066408215], + [5.893478393554688, 48.228961073585126], + [5.892791748046875, 48.228903896961775], + [5.892705917358398, 48.230390468407535], + [5.8950233459472665, 48.229933066408215] + ] + ], + [ + [ + [5.893220901489259, 48.229246955743626], + [5.893392562866212, 48.22884672027457], + [5.892705917358398, 48.22878954352343], + [5.892019271850587, 48.22856083588024], + [5.892019271850587, 48.2277031731152], + [5.890989303588868, 48.22787470681807], + [5.889959335327149, 48.22787470681807], + [5.890560150146485, 48.22838930447709], + [5.890645980834962, 48.22878954352343], + [5.890989303588868, 48.229018250144584], + [5.892362594604493, 48.22930413198368], + [5.893220901489259, 48.229246955743626] + ] + ] + ] + } + end + + it { expect(geo_area.rgeo_geometry).to be_nil } + end + end end