From ff008749d3e8dee00d378b0371c117b8a825a027 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Thu, 28 Aug 2025 20:38:59 +0300 Subject: [PATCH] Lock changeset in api create element actions --- app/controllers/api/nodes_controller.rb | 6 ++-- app/controllers/api/relations_controller.rb | 6 ++-- app/controllers/api/ways_controller.rb | 6 ++-- test/controllers/api/nodes_controller_test.rb | 21 ++++++++++++++ .../api/relations_controller_test.rb | 28 +++++++++++++++++++ test/controllers/api/ways_controller_test.rb | 28 +++++++++++++++++++ 6 files changed, 89 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index 893f935bc..a585f5c88 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -49,8 +49,10 @@ module Api def create node = Node.from_xml(request.raw_post, :create => true) - # Assume that Node.from_xml has thrown an exception if there is an error parsing the xml - node.create_with_history current_user + Changeset.transaction do + node.changeset&.lock! + node.create_with_history current_user + end render :plain => node.id.to_s end diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index f9e113479..37c5cfc5e 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -97,8 +97,10 @@ module Api def create relation = Relation.from_xml(request.raw_post, :create => true) - # Assume that Relation.from_xml has thrown an exception if there is an error parsing the xml - relation.create_with_history current_user + Changeset.transaction do + relation.changeset&.lock! + relation.create_with_history current_user + end render :plain => relation.id.to_s end diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index 5de3bd9b5..05035d17c 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -54,8 +54,10 @@ module Api def create way = Way.from_xml(request.raw_post, :create => true) - # Assume that Way.from_xml has thrown an exception if there is an error parsing the xml - way.create_with_history current_user + Changeset.transaction do + way.changeset&.lock! + way.create_with_history current_user + end render :plain => way.id.to_s end diff --git a/test/controllers/api/nodes_controller_test.rb b/test/controllers/api/nodes_controller_test.rb index 511585e41..3e91d8cee 100644 --- a/test/controllers/api/nodes_controller_test.rb +++ b/test/controllers/api/nodes_controller_test.rb @@ -265,6 +265,27 @@ module Api end end + def test_create_race_condition + user = create(:user) + changeset = create(:changeset, :user => user) + auth_header = bearer_authorization_header user + path = api_nodes_path + concurrency_level = 16 + + threads = Array.new(concurrency_level) do + Thread.new do + osm = "" + post path, :params => osm, :headers => auth_header + end + end + threads.each(&:join) + + changeset.reload + assert_equal concurrency_level, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? + assert_equal concurrency_level, changeset.num_created_nodes + end + def test_show_not_found get api_node_path(0) assert_response :not_found diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index acbc216ba..8aa29d887 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -497,6 +497,34 @@ module Api assert_response :success end + def test_create_race_condition + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) + auth_header = bearer_authorization_header user + path = api_relations_path + concurrency_level = 16 + + threads = Array.new(concurrency_level) do + Thread.new do + osm = <<~OSM + + + + + + OSM + post path, :params => osm, :headers => auth_header + end + end + threads.each(&:join) + + changeset.reload + assert_equal concurrency_level, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? + assert_equal concurrency_level, changeset.num_created_relations + end + # ------------------------------------ # Test updating relations # ------------------------------------ diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index 905e4d0b8..015f14ab9 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -435,6 +435,34 @@ module Api end end + def test_create_race_condition + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) + auth_header = bearer_authorization_header user + path = api_ways_path + concurrency_level = 16 + + threads = Array.new(concurrency_level) do + Thread.new do + osm = <<~OSM + + + + + + OSM + post path, :params => osm, :headers => auth_header + end + end + threads.each(&:join) + + changeset.reload + assert_equal concurrency_level, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? + assert_equal concurrency_level, changeset.num_created_ways + end + # ------------------------------------- # Test deleting ways. # ------------------------------------- -- 2.39.5