Merge pull request #3440 from mmd-osm/relationmemberlimit

Introduce relation member limit

app/models/relation.rb

@@ -206,6 +206,8 @@ class Relation < ApplicationRecord
   end
 
   def preconditions_ok?(good_members = [])
+    raise OSM::APITooManyRelationMembersError.new(id, members.length, Settings.max_number_of_relation_members) if members.length > Settings.max_number_of_relation_members
+
     # These are hastables that store an id in the index of all
     # the nodes/way/relations that have already been added.
     # If the member is valid and visible then we add it to the

app/views/api/capabilities/show.builder

@@ -6,6 +6,7 @@ xml.osm(OSM::API.new.xml_root_attributes) do |osm|
     api.note_area(:maximum => Settings.max_note_request_area)
     api.tracepoints(:per_page => Settings.tracepoints_per_page)
     api.waynodes(:maximum => Settings.max_number_of_way_nodes)
+    api.relationmembers(:maximum => Settings.max_number_of_relation_members)
     api.changesets(:maximum_elements => Changeset::MAX_ELEMENTS)
     api.timeout(:seconds => Settings.api_timeout)
     api.status(:database => @database_status,

config/initializers/config.rb

@@ -74,6 +74,7 @@ Config.setup do |config|
     required(:max_note_request_area).filled(:number?)
     required(:tracepoints_per_page).filled(:int?)
     required(:max_number_of_way_nodes).filled(:int?)
+    required(:max_number_of_relation_members).filled(:int?)
     required(:api_timeout).filled(:int?)
     required(:imagery_blacklist).maybe(:array?)
     required(:status).filled(:str?, :included_in? => ALLOWED_STATUS)

config/settings.yml

@@ -31,6 +31,8 @@ tracepoints_per_page: 5000
 max_number_of_nodes: 50000
 # Maximum number of nodes that can be in a way (checked on save)
 max_number_of_way_nodes: 2000
+# Maximum number of members that can be in a relation (checked on save)
+max_number_of_relation_members: 32000
 # The maximum area you're allowed to request notes from, in square degrees
 max_note_request_area: 25
 # Zoom level to use for postcode results from the geocoder

lib/osm.rb

@@ -237,6 +237,24 @@ module OSM
     end
   end
 
+  # Raised when a relation has more than the configured number of relation members.
+  # This prevents relations from being too complex and difficult to work with
+  class APITooManyRelationMembersError < APIError
+    def initialize(id, provided, max)
+      super "You tried to add #{provided} members to relation #{id}, however only #{max} are allowed"
+
+      @id = id
+      @provided = provided
+      @max = max
+    end
+
+    attr_reader :id, :provided, :max
+
+    def status
+      :bad_request
+    end
+  end
+
   ##
   # raised when user input couldn't be parsed
   class APIBadUserInput < APIError

test/controllers/api/capabilities_controller_test.rb

@@ -25,6 +25,7 @@ module Api
           assert_select "note_area[maximum='#{Settings.max_note_request_area}']", :count => 1
           assert_select "tracepoints[per_page='#{Settings.tracepoints_per_page}']", :count => 1
           assert_select "changesets[maximum_elements='#{Changeset::MAX_ELEMENTS}']", :count => 1
+          assert_select "relationmembers[maximum='#{Settings.max_number_of_relation_members}']", :count => 1
           assert_select "status[database='online']", :count => 1
           assert_select "status[api='online']", :count => 1
           assert_select "status[gpx='online']", :count => 1

test/models/relation_test.rb

@@ -230,4 +230,27 @@ class RelationTest < ActiveSupport::TestCase
     assert_equal 39, changeset.min_lat
     assert_equal 116, changeset.max_lat
   end
+
+  # Check that the preconditions fail when you are over the defined limit of
+  # the maximum number of members in a relation.
+  def test_max_members_per_relation_limit
+    # Speed up unit test by using a small relation member limit
+    default_limit = Settings.max_number_of_relation_members
+    Settings.max_number_of_relation_members = 20
+
+    user = create(:user)
+    changeset = create(:changeset, :user => user)
+    relation = create(:relation, :changeset => changeset)
+    node = create(:node, :longitude => 116, :latitude => 39)
+    # Create relation which exceeds the relation member limit by one
+    0.upto(Settings.max_number_of_relation_members) do |i|
+      create(:relation_member, :relation => relation, :member_type => "Node", :member_id => node.id, :sequence_id => i)
+    end
+
+    assert_raise OSM::APITooManyRelationMembersError do
+      relation.create_with_history user
+    end
+
+    Settings.max_number_of_relation_members = default_limit
+  end
 end