From 231f0837346118d5f2ddecb2e0637849be3f1b22 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sat, 3 May 2025 06:40:49 +0300 Subject: [PATCH] Move api changeset upload delete tests --- .../api/changesets/uploads_controller_test.rb | 195 +++++++++++++++++ .../api/changesets_controller_test.rb | 204 ------------------ 2 files changed, 195 insertions(+), 204 deletions(-) diff --git a/test/controllers/api/changesets/uploads_controller_test.rb b/test/controllers/api/changesets/uploads_controller_test.rb index f6075d3a7..d7ea0b505 100644 --- a/test/controllers/api/changesets/uploads_controller_test.rb +++ b/test/controllers/api/changesets/uploads_controller_test.rb @@ -489,6 +489,201 @@ module Api assert_equal 0, relation.tags.size, "relation #{relation.id} should now have no tags" assert_equal [["Way", way.id, "some"], ["Node", node.id, "some"], ["Relation", other_relation.id, "some"]], relation.members end + + # ------------------------------------- + # Test deleting elements. + # ------------------------------------- + + ## + # test a complex delete where we delete elements which rely on each other + # in the same transaction. + def test_upload_delete_elements + changeset = create(:changeset) + super_relation = create(:relation) + used_relation = create(:relation) + used_way = create(:way) + used_node = create(:node) + create(:relation_member, :relation => super_relation, :member => used_relation) + create(:relation_member, :relation => super_relation, :member => used_way) + create(:relation_member, :relation => super_relation, :member => used_node) + + diff = XML::Document.new + diff.root = XML::Node.new "osmChange" + delete = XML::Node.new "delete" + diff.root << delete + delete << xml_node_for_relation(super_relation) + delete << xml_node_for_relation(used_relation) + delete << xml_node_for_way(used_way) + delete << xml_node_for_node(used_node) + %w[node way relation].each do |type| + delete.find("//osmChange/delete/#{type}").each do |n| + n["changeset"] = changeset.id.to_s + end + end + + auth_header = bearer_authorization_header changeset.user + + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + + assert_response :success + + assert_dom "diffResult", 1 do + assert_dom "> node", 1 + assert_dom "> way", 1 + assert_dom "> relation", 2 + end + + assert_not Node.find(used_node.id).visible + assert_not Way.find(used_way.id).visible + assert_not Relation.find(super_relation.id).visible + assert_not Relation.find(used_relation.id).visible + end + + ## + # test uploading a delete with no lat/lon, as they are optional in the osmChange spec. + def test_upload_delete_node_without_latlon + node = create(:node) + changeset = create(:changeset) + + diff = "" + + auth_header = bearer_authorization_header changeset.user + + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header + + assert_response :success + + assert_dom "diffResult", 1 do + assert_dom "> node", 1 do + assert_dom "> @old_id", node.id.to_s + assert_dom "> @new_id", 0 + assert_dom "> @new_version", 0 + end + end + + node.reload + assert_not node.visible + end + + ## + # test that deleting stuff in a transaction doesn't bypass the checks + # to ensure that used elements are not deleted. + def test_upload_delete_referenced_elements + changeset = create(:changeset) + relation = create(:relation) + other_relation = create(:relation) + used_way = create(:way) + used_node = create(:node) + create(:relation_member, :relation => relation, :member => used_way) + create(:relation_member, :relation => relation, :member => used_node) + + diff = XML::Document.new + diff.root = XML::Node.new "osmChange" + delete = XML::Node.new "delete" + diff.root << delete + delete << xml_node_for_relation(other_relation) + delete << xml_node_for_way(used_way) + delete << xml_node_for_node(used_node) + %w[node way relation].each do |type| + delete.find("//osmChange/delete/#{type}").each do |n| + n["changeset"] = changeset.id.to_s + end + end + + auth_header = bearer_authorization_header changeset.user + + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + + assert_response :precondition_failed + assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body + + assert Node.find(used_node.id).visible + assert Way.find(used_way.id).visible + assert Relation.find(relation.id).visible + assert Relation.find(other_relation.id).visible + end + + ## + # test that a conditional delete of an in use object works. + def test_upload_delete_if_unused + changeset = create(:changeset) + super_relation = create(:relation) + used_relation = create(:relation) + used_way = create(:way) + used_node = create(:node) + create(:relation_member, :relation => super_relation, :member => used_relation) + create(:relation_member, :relation => super_relation, :member => used_way) + create(:relation_member, :relation => super_relation, :member => used_node) + + diff = XML::Document.new + diff.root = XML::Node.new "osmChange" + delete = XML::Node.new "delete" + diff.root << delete + delete["if-unused"] = "" + delete << xml_node_for_relation(used_relation) + delete << xml_node_for_way(used_way) + delete << xml_node_for_node(used_node) + %w[node way relation].each do |type| + delete.find("//osmChange/delete/#{type}").each do |n| + n["changeset"] = changeset.id.to_s + end + end + + auth_header = bearer_authorization_header changeset.user + + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + + assert_response :success + + assert_dom "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 do + assert_dom "> node", 1 do + assert_dom "> @old_id", used_node.id.to_s + assert_dom "> @new_id", used_node.id.to_s + assert_dom "> @new_version", used_node.version.to_s + end + assert_dom "> way", 1 do + assert_dom "> @old_id", used_way.id.to_s + assert_dom "> @new_id", used_way.id.to_s + assert_dom "> @new_version", used_way.version.to_s + end + assert_dom "> relation", 1 do + assert_dom "> @old_id", used_relation.id.to_s + assert_dom "> @new_id", used_relation.id.to_s + assert_dom "> @new_version", used_relation.version.to_s + end + end + + assert Node.find(used_node.id).visible + assert Way.find(used_way.id).visible + assert Relation.find(used_relation.id).visible + end + + def test_upload_delete_with_multiple_blocks_and_if_unused + changeset = create(:changeset) + node = create(:node) + way = create(:way) + create(:way_node, :way => way, :node => node) + alone_node = create(:node) + + diff = <<~CHANGESET + + + + + + + + + CHANGESET + + auth_header = bearer_authorization_header changeset.user + + 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 + end end end end diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 4dcadbedf..1a8c47c26 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -643,76 +643,6 @@ module Api end end - ## - # test a complex delete where we delete elements which rely on eachother - # in the same transaction. - def test_upload_delete - changeset = create(:changeset) - super_relation = create(:relation) - used_relation = create(:relation) - used_way = create(:way) - used_node = create(:node) - create(:relation_member, :relation => super_relation, :member => used_relation) - create(:relation_member, :relation => super_relation, :member => used_way) - create(:relation_member, :relation => super_relation, :member => used_node) - - auth_header = bearer_authorization_header changeset.user - - diff = XML::Document.new - diff.root = XML::Node.new "osmChange" - delete = XML::Node.new "delete" - diff.root << delete - delete << xml_node_for_relation(super_relation) - delete << xml_node_for_relation(used_relation) - delete << xml_node_for_way(used_way) - delete << xml_node_for_node(used_node) - - # update the changeset to one that this user owns - %w[node way relation].each do |type| - delete.find("//osmChange/delete/#{type}").each do |n| - n["changeset"] = changeset.id.to_s - end - end - - # upload it - post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header - assert_response :success, - "can't upload a deletion diff to changeset: #{@response.body}" - - # check the response is well-formed - assert_select "diffResult>node", 1 - assert_select "diffResult>way", 1 - assert_select "diffResult>relation", 2 - - # check that everything was deleted - assert_not Node.find(used_node.id).visible - assert_not Way.find(used_way.id).visible - assert_not Relation.find(super_relation.id).visible - assert_not Relation.find(used_relation.id).visible - end - - ## - # test uploading a delete with no lat/lon, as they are optional in - # the osmChange spec. - def test_upload_nolatlon_delete - node = create(:node) - changeset = create(:changeset) - - auth_header = bearer_authorization_header changeset.user - diff = "" - - # upload it - post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header - assert_response :success, - "can't upload a deletion diff to changeset: #{@response.body}" - - # check the response is well-formed - assert_select "diffResult>node", 1 - - # check that everything was deleted - assert_not Node.find(node.id).visible - end - def test_repeated_changeset_create 3.times do auth_header = bearer_authorization_header @@ -780,111 +710,6 @@ module Api assert_operator cs.max_lat, :<=, 90 * GeoRecord::SCALE, "Maximum latitude (#{cs.max_lat / GeoRecord::SCALE}) should be <= 90 to be valid." end - ## - # test that deleting stuff in a transaction doesn't bypass the checks - # to ensure that used elements are not deleted. - def test_upload_delete_invalid - changeset = create(:changeset) - relation = create(:relation) - other_relation = create(:relation) - used_way = create(:way) - used_node = create(:node) - create(:relation_member, :relation => relation, :member => used_way) - create(:relation_member, :relation => relation, :member => used_node) - - auth_header = bearer_authorization_header changeset.user - - diff = XML::Document.new - diff.root = XML::Node.new "osmChange" - delete = XML::Node.new "delete" - diff.root << delete - delete << xml_node_for_relation(other_relation) - delete << xml_node_for_way(used_way) - delete << xml_node_for_node(used_node) - - # update the changeset to one that this user owns - %w[node way relation].each do |type| - delete.find("//osmChange/delete/#{type}").each do |n| - n["changeset"] = changeset.id.to_s - end - end - - # upload it - post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header - assert_response :precondition_failed, - "shouldn't be able to upload a invalid deletion diff: #{@response.body}" - assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body - - # check that nothing was, in fact, deleted - assert Node.find(used_node.id).visible - assert Way.find(used_way.id).visible - assert Relation.find(relation.id).visible - assert Relation.find(other_relation.id).visible - end - - ## - # test that a conditional delete of an in use object works. - def test_upload_delete_if_unused - changeset = create(:changeset) - super_relation = create(:relation) - used_relation = create(:relation) - used_way = create(:way) - used_node = create(:node) - create(:relation_member, :relation => super_relation, :member => used_relation) - create(:relation_member, :relation => super_relation, :member => used_way) - create(:relation_member, :relation => super_relation, :member => used_node) - - auth_header = bearer_authorization_header changeset.user - - diff = XML::Document.new - diff.root = XML::Node.new "osmChange" - delete = XML::Node.new "delete" - diff.root << delete - delete["if-unused"] = "" - delete << xml_node_for_relation(used_relation) - delete << xml_node_for_way(used_way) - delete << xml_node_for_node(used_node) - - # update the changeset to one that this user owns - %w[node way relation].each do |type| - delete.find("//osmChange/delete/#{type}").each do |n| - n["changeset"] = changeset.id.to_s - end - end - - # upload it - post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header - assert_response :success, - "can't do a conditional delete of in use objects: #{@response.body}" - - # check the returned payload - assert_dom "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 do - # check the old IDs are all present and what we expect - # check the new IDs are all present and unchanged - # check the new versions are all present and unchanged - assert_dom "> node", 1 do - assert_dom "> @old_id", used_node.id.to_s - assert_dom "> @new_id", used_node.id.to_s - assert_dom "> @new_version", used_node.version.to_s - end - assert_dom "> way", 1 do - assert_dom "> @old_id", used_way.id.to_s - assert_dom "> @new_id", used_way.id.to_s - assert_dom "> @new_version", used_way.version.to_s - end - assert_dom "> relation", 1 do - assert_dom "> @old_id", used_relation.id.to_s - assert_dom "> @new_id", used_relation.id.to_s - assert_dom "> @new_version", used_relation.version.to_s - end - end - - # check that nothing was, in fact, deleted - assert Node.find(used_node.id).visible - assert Way.find(used_way.id).visible - assert Relation.find(used_relation.id).visible - end - ## # upload something which creates new objects and inserts them into # existing containers using placeholders. @@ -1417,35 +1242,6 @@ module Api assert_response :not_found, "Relation should not be deleted" end - def test_upload_multiple_delete_block - changeset = create(:changeset) - - auth_header = bearer_authorization_header changeset.user - - node = create(:node) - way = create(:way) - create(:way_node, :way => way, :node => node) - alone_node = create(:node) - - # modify node - diff = <<~CHANGESET - - - - - - - - - CHANGESET - - # upload it - post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header - assert_response :precondition_failed, - "shouldn't be able to upload a invalid deletion diff: #{@response.body}" - assert_equal "Precondition failed: Node #{node.id} is still used by ways #{way.id}.", @response.body - end - ## # test initial rate limit def test_upload_initial_rate_limit -- 2.39.5