From 9c4c30dfd00ba0d4e498d4370411debf955de7f5 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Fri, 4 Jul 2025 20:04:00 +0300 Subject: [PATCH] Test that failed/empty changeset uploads keep type changes in sync --- .../api/changesets/uploads_controller_test.rb | 84 ++++++++++++++++--- 1 file changed, 71 insertions(+), 13 deletions(-) diff --git a/test/controllers/api/changesets/uploads_controller_test.rb b/test/controllers/api/changesets/uploads_controller_test.rb index 5247e72c6..350fc95f9 100644 --- a/test/controllers/api/changesets/uploads_controller_test.rb +++ b/test/controllers/api/changesets/uploads_controller_test.rb @@ -28,8 +28,8 @@ module Api assert_response :unauthorized - changeset.reload - assert_equal 0, changeset.num_changes + assert_no_changes_in changeset + node.reload assert_equal 1, node.version assert_equal 0, node.latitude @@ -55,8 +55,8 @@ module Api assert_response :forbidden - changeset.reload - assert_equal 0, changeset.num_changes + assert_no_changes_in changeset + node.reload assert_equal 1, node.version assert_equal 0, node.latitude @@ -82,8 +82,8 @@ module Api assert_response :forbidden - changeset.reload - assert_equal 0, changeset.num_changes + assert_no_changes_in changeset + node.reload assert_equal 1, node.version assert_equal 0, node.latitude @@ -144,6 +144,8 @@ module Api assert_response :bad_request assert_equal "Unknown action ping, choices are create, modify, delete", @response.body + + assert_no_changes_in changeset end ## @@ -161,6 +163,8 @@ module Api assert_response :success end + + assert_no_changes_in changeset end ## @@ -418,6 +422,8 @@ module Api assert_response :bad_request end + + assert_no_changes_in changeset end def test_upload_create_nodes_with_invalid_placeholder_reuse_in_one_action_block @@ -439,6 +445,8 @@ module Api assert_response :bad_request end + + assert_no_changes_in changeset end def test_upload_create_nodes_with_invalid_placeholder_reuse_in_two_action_blocks @@ -462,6 +470,8 @@ module Api assert_response :bad_request end + + assert_no_changes_in changeset end def test_upload_create_way_referring_node_placeholder_defined_later @@ -488,6 +498,8 @@ module Api end end assert_equal "Placeholder node not found for reference -1 in way -1", @response.body + + assert_no_changes_in changeset end def test_upload_create_way_referring_undefined_node_placeholder @@ -511,6 +523,8 @@ module Api assert_response :bad_request end assert_equal "Placeholder node not found for reference -1 in way -1", @response.body + + assert_no_changes_in changeset end def test_upload_create_existing_way_referring_undefined_node_placeholder @@ -534,6 +548,8 @@ module Api assert_response :bad_request assert_equal "Placeholder node not found for reference -1 in way #{way.id}", @response.body + assert_no_changes_in changeset + way.reload assert_equal 1, way.version end @@ -559,6 +575,8 @@ module Api assert_response :bad_request end assert_equal "Placeholder Node not found for reference -1 in relation -1.", @response.body + + assert_no_changes_in changeset end def test_upload_create_existing_relation_referring_undefined_way_placeholder @@ -582,6 +600,8 @@ module Api assert_response :bad_request assert_equal "Placeholder Way not found for reference -1 in relation #{relation.id}.", @response.body + assert_no_changes_in changeset + relation.reload assert_equal 1, relation.version end @@ -617,6 +637,8 @@ module Api assert_response :bad_request assert_equal "Placeholder Relation not found for reference -4 in relation -2.", @response.body + + assert_no_changes_in changeset end # ------------------------------------- @@ -894,6 +916,8 @@ module Api assert_response :conflict + assert_no_changes_in changeset + node.reload assert_equal 1, node.version assert_equal 0, node.latitude @@ -920,6 +944,8 @@ module Api assert_response :bad_request + assert_no_changes_in changeset + node.reload assert_equal 1, node.version assert_equal 0, node.latitude @@ -953,6 +979,9 @@ module Api assert_response :conflict + assert_no_changes_in changeset1 + assert_no_changes_in changeset2 + assert_nodes_are_equal(node1, Node.find(node1.id)) assert_nodes_are_equal(node2, Node.find(node2.id)) end @@ -1227,6 +1256,8 @@ module Api assert_response :precondition_failed assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body + assert_no_changes_in changeset + assert Node.find(used_node.id).visible assert Way.find(used_way.id).visible assert Relation.find(relation.id).visible @@ -1283,6 +1314,8 @@ module Api end end + assert_no_changes_in changeset + assert Node.find(used_node.id).visible assert Way.find(used_way.id).visible assert Relation.find(used_relation.id).visible @@ -1311,8 +1344,9 @@ module Api post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :precondition_failed - assert_equal "Precondition failed: Node #{node.id} is still used by ways #{way.id}.", @response.body + + assert_no_changes_in changeset end def test_upload_delete_unknown_node_placeholder @@ -1450,6 +1484,8 @@ module Api assert_response :gone end + + assert_no_changes_in changeset end def test_upload_create_and_duplicate_delete_if_unused @@ -1606,9 +1642,11 @@ module Api relation = create(:relation) # create a changeset that puts us near the initial rate limit + num_changes = Settings.initial_changes_per_hour - 2 changeset = create(:changeset, :user => user, :created_at => Time.now.utc - 5.minutes, - :num_changes => Settings.initial_changes_per_hour - 2) + :num_changes => num_changes, + :num_created_nodes => num_changes) diff = <<~CHANGESET @@ -1636,6 +1674,10 @@ module Api post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :too_many_requests, "upload did not hit rate limit" + + changeset.reload + assert_equal num_changes, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? end def test_upload_maximum_rate_limit @@ -1649,13 +1691,15 @@ module Api :created_at => Time.now.utc - 28.days) # create changeset to put us near the maximum rate limit - total_changes = Settings.max_changes_per_hour - 2 - while total_changes.positive? - changes = [total_changes, Changeset::MAX_ELEMENTS].min + remaining_num_changes = Settings.max_changes_per_hour - 2 + num_changes = 0 + while remaining_num_changes.positive? + num_changes = [remaining_num_changes, Changeset::MAX_ELEMENTS].min changeset = create(:changeset, :user => user, :created_at => Time.now.utc - 5.minutes, - :num_changes => changes) - total_changes -= changes + :num_changes => num_changes, + :num_created_nodes => num_changes) + remaining_num_changes -= num_changes end diff = <<~CHANGESET @@ -1684,6 +1728,10 @@ module Api post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :too_many_requests, "upload did not hit rate limit" + + changeset.reload + assert_equal num_changes, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? end def test_upload_initial_size_limit @@ -1710,6 +1758,8 @@ module Api post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :payload_too_large, "upload did not hit size limit" + + assert_no_changes_in changeset end def test_upload_size_limit_after_one_week @@ -1737,6 +1787,8 @@ module Api post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :payload_too_large, "upload did not hit size limit" + + assert_no_changes_in changeset end private @@ -1749,8 +1801,14 @@ module Api post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found + + assert_no_changes_in changeset + end + + def assert_no_changes_in(changeset) changeset.reload assert_equal 0, changeset.num_changes + assert_predicate changeset, :num_type_changes_in_sync? end end end -- 2.39.5