X-Git-Url: https://git.openstreetmap.org./rails.git/blobdiff_plain/4fc1455d51a681e961a3ca568c95723518106b24..30f975c609bfd05a654ad9facaf13135ca0691cd:/test/controllers/api/changesets_controller_test.rb diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index be1033ea6..1d7afa035 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -396,7 +396,7 @@ module Api # check that a changeset that doesn't exist returns an appropriate message def test_show_not_found [0, -32, 233455644, "afg", "213"].each do |id| - get changeset_show_path(:id => id) + get changeset_show_path(id) assert_response :not_found, "should get a not found" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -424,7 +424,7 @@ module Api auth_header = basic_authorization_header user.email, "test" cs_id = changeset.id - put changeset_close_path(:id => cs_id), :headers => auth_header + put changeset_close_path(cs_id), :headers => auth_header assert_response :success # test that it really is closed now @@ -470,7 +470,7 @@ module Api # First try to do it with no auth cs_ids.each do |id| - put changeset_close_path(:id => id) + put changeset_close_path(id) assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -479,7 +479,7 @@ module Api # Now try with auth auth_header = basic_authorization_header create(:user).email, "test" cs_ids.each do |id| - put changeset_close_path(:id => id), :headers => auth_header + put changeset_close_path(id), :headers => auth_header assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -636,26 +636,27 @@ module Api "can't upload a simple valid creation to changeset: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 - assert_select "diffResult>node", 1 - assert_select "diffResult>way", 1 - assert_select "diffResult>relation", 1 - - # inspect the response to find out what the new element IDs are - doc = XML::Parser.string(@response.body).parse - new_node_id = doc.find("//diffResult/node").first["new_id"].to_i - new_way_id = doc.find("//diffResult/way").first["new_id"].to_i - new_rel_id = doc.find("//diffResult/relation").first["new_id"].to_i - - # check the old IDs are all present and negative one - assert_equal(-1, doc.find("//diffResult/node").first["old_id"].to_i) - assert_equal(-1, doc.find("//diffResult/way").first["old_id"].to_i) - assert_equal(-1, doc.find("//diffResult/relation").first["old_id"].to_i) - - # check the versions are present and equal one - assert_equal 1, doc.find("//diffResult/node").first["new_version"].to_i - assert_equal 1, doc.find("//diffResult/way").first["new_version"].to_i - assert_equal 1, doc.find("//diffResult/relation").first["new_version"].to_i + new_node_id, new_way_id, new_rel_id = nil + assert_dom "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 do + # inspect the response to find out what the new element IDs are + # check the old IDs are all present and negative one + # check the versions are present and equal one + assert_dom "> node", 1 do |(node_el)| + new_node_id = node_el["new_id"].to_i + assert_dom "> @old_id", "-1" + assert_dom "> @new_version", "1" + end + assert_dom "> way", 1 do |(way_el)| + new_way_id = way_el["new_id"].to_i + assert_dom "> @old_id", "-1" + assert_dom "> @new_version", "1" + end + assert_dom "> relation", 1 do |(rel_el)| + new_rel_id = rel_el["new_id"].to_i + assert_dom "> @old_id", "-1" + assert_dom "> @new_version", "1" + end + end # check that the changes made it into the database assert_equal 2, Node.find(new_node_id).tags.size, "new node should have two tags" @@ -749,7 +750,11 @@ module Api end def test_upload_large_changeset - auth_header = basic_authorization_header create(:user).email, "test" + user = create(:user) + auth_header = basic_authorization_header user.email, "test" + + # create an old changeset to ensure we have the maximum rate limit + create(:changeset, :user => user, :created_at => Time.now.utc - 28.days) # create a changeset put changeset_create_path, :params => "", :headers => auth_header @@ -784,7 +789,7 @@ module Api # upload it, which used to cause an error like "PGError: ERROR: # integer out of range" (bug #2152). but shouldn't any more. - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a spatially-large diff to changeset: #{@response.body}" @@ -874,28 +879,26 @@ module Api "can't do a conditional delete of in use objects: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 - assert_select "diffResult>node", 1 - assert_select "diffResult>way", 1 - assert_select "diffResult>relation", 1 - - # parse the response - doc = XML::Parser.string(@response.body).parse - - # check the old IDs are all present and what we expect - assert_equal used_node.id, doc.find("//diffResult/node").first["old_id"].to_i - assert_equal used_way.id, doc.find("//diffResult/way").first["old_id"].to_i - assert_equal used_relation.id, doc.find("//diffResult/relation").first["old_id"].to_i - - # check the new IDs are all present and unchanged - assert_equal used_node.id, doc.find("//diffResult/node").first["new_id"].to_i - assert_equal used_way.id, doc.find("//diffResult/way").first["new_id"].to_i - assert_equal used_relation.id, doc.find("//diffResult/relation").first["new_id"].to_i - - # check the new versions are all present and unchanged - assert_equal used_node.version, doc.find("//diffResult/node").first["new_version"].to_i - assert_equal used_way.version, doc.find("//diffResult/way").first["new_version"].to_i - assert_equal used_relation.version, doc.find("//diffResult/relation").first["new_version"].to_i + 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 @@ -969,14 +972,14 @@ module Api "can't upload a complex diff to changeset: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 - assert_select "diffResult>node", 1 - assert_select "diffResult>way", 1 - assert_select "diffResult>relation", 1 - - # inspect the response to find out what the new element IDs are - doc = XML::Parser.string(@response.body).parse - new_node_id = doc.find("//diffResult/node").first["new_id"].to_i + new_node_id = nil + assert_dom "diffResult[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1 do + assert_dom "> node", 1 do |(node_el)| + new_node_id = node_el["new_id"].to_i + end + assert_dom "> way", 1 + assert_dom "> relation", 1 + end # check that the changes made it into the database assert_equal 2, Node.find(new_node_id).tags.size, "new node should have two tags" @@ -1048,14 +1051,14 @@ module Api diff = <<~CHANGESET - - - - - - - - + + + + + + + + CHANGESET @@ -1329,9 +1332,9 @@ module Api diff = <<~CHANGESET - - - + + + @@ -1352,9 +1355,9 @@ module Api diff = <<~CHANGESET - - - + + + @@ -1384,9 +1387,9 @@ module Api diff = <<~CHANGESET - - - + + + @@ -1407,9 +1410,9 @@ module Api diff = <<~CHANGESET - - - + + + @@ -1453,7 +1456,7 @@ module Api diff.root << modify # upload it - post changeset_upload_path(:id => changeset_id), :params => diff.to_s, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" @@ -1478,30 +1481,30 @@ module Api changeset_id = @response.body.to_i old_way = create(:way) - create(:way_node, :way => old_way, :node => create(:node, :lat => 1, :lon => 1)) + create(:way_node, :way => old_way, :node => create(:node, :lat => 0.1, :lon => 0.1)) diff = XML::Document.new diff.root = XML::Node.new "osmChange" modify = XML::Node.new "modify" xml_old_way = xml_node_for_way(old_way) nd_ref = XML::Node.new "nd" - nd_ref["ref"] = create(:node, :lat => 3, :lon => 3).id.to_s + nd_ref["ref"] = create(:node, :lat => 0.3, :lon => 0.3).id.to_s xml_old_way << nd_ref xml_old_way["changeset"] = changeset_id.to_s modify << xml_old_way diff.root << modify # upload it - post changeset_upload_path(:id => changeset_id), :params => diff.to_s, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" # check the bbox changeset = Changeset.find(changeset_id) - assert_equal 1 * GeoRecord::SCALE, changeset.min_lon, "min_lon should be 1 degree" - assert_equal 3 * GeoRecord::SCALE, changeset.max_lon, "max_lon should be 3 degrees" - assert_equal 1 * GeoRecord::SCALE, changeset.min_lat, "min_lat should be 1 degree" - assert_equal 3 * GeoRecord::SCALE, changeset.max_lat, "max_lat should be 3 degrees" + assert_equal 0.1 * GeoRecord::SCALE, changeset.min_lon, "min_lon should be 0.1 degree" + assert_equal 0.3 * GeoRecord::SCALE, changeset.max_lon, "max_lon should be 0.3 degrees" + assert_equal 0.1 * GeoRecord::SCALE, changeset.min_lat, "min_lat should be 0.1 degree" + assert_equal 0.3 * GeoRecord::SCALE, changeset.max_lat, "max_lat should be 0.3 degrees" end ## @@ -1798,6 +1801,71 @@ module Api assert_response :too_many_requests, "upload did not hit rate limit" end + ## + # test initial size limit + def test_upload_initial_size_limit + # create a user + user = create(:user) + + # create a changeset that puts us near the initial size limit + changeset = create(:changeset, :user => user, + :min_lat => (-0.5 * GeoRecord::SCALE).round, :min_lon => (0.5 * GeoRecord::SCALE).round, + :max_lat => (0.5 * GeoRecord::SCALE).round, :max_lon => (2.5 * GeoRecord::SCALE).round) + + # create authentication header + auth_header = basic_authorization_header user.email, "test" + + # simple diff to create a node + diff = <<~CHANGESET + + + + + + + + + CHANGESET + + # upload it + post changeset_upload_path(changeset), :params => diff, :headers => auth_header + assert_response :payload_too_large, "upload did not hit size limit" + end + + ## + # test size limit after one week + def test_upload_week_size_limit + # create a user + user = create(:user) + + # create a changeset to establish our initial edit time + create(:changeset, :user => user, :created_at => Time.now.utc - 7.days) + + # create a changeset that puts us near the initial size limit + changeset = create(:changeset, :user => user, + :min_lat => (-0.5 * GeoRecord::SCALE).round, :min_lon => (0.5 * GeoRecord::SCALE).round, + :max_lat => (0.5 * GeoRecord::SCALE).round, :max_lon => (2.5 * GeoRecord::SCALE).round) + + # create authentication header + auth_header = basic_authorization_header user.email, "test" + + # simple diff to create a node way and relation using placeholders + diff = <<~CHANGESET + + + + + + + + + CHANGESET + + # upload it + post changeset_upload_path(changeset), :params => diff, :headers => auth_header + assert_response :payload_too_large, "upload did not hit size limit" + end + ## # when we make some simple changes we get the same changes back from the # diff download. @@ -1829,24 +1897,24 @@ module Api diff = <<~CHANGESET - - - - - - - - + + + + + + + + CHANGESET # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload multiple versions of an element in a diff: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -1900,11 +1968,11 @@ module Api OSMFILE # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a diff from JOSM: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -1935,15 +2003,15 @@ module Api diff = <<~CHANGESET - + - - - + + + - + @@ -1955,11 +2023,11 @@ module Api CHANGESET # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload multiple versions of an element in a diff: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -2034,7 +2102,7 @@ module Api # FIXME: This should really be moded to a integration test due to the with_controller def test_changeset_bbox way = create(:way) - create(:way_node, :way => way, :node => create(:node, :lat => 3, :lon => 3)) + create(:way_node, :way => way, :node => create(:node, :lat => 0.3, :lon => 0.3)) auth_header = basic_authorization_header create(:user).email, "test" @@ -2046,33 +2114,33 @@ module Api # add a single node to it with_controller(NodesController.new) do - xml = "" + xml = "" put node_create_path, :params => xml, :headers => auth_header assert_response :success, "Couldn't create node." end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset." - assert_select "osm>changeset[min_lon='1.0000000']", 1 - assert_select "osm>changeset[max_lon='1.0000000']", 1 - assert_select "osm>changeset[min_lat='2.0000000']", 1 - assert_select "osm>changeset[max_lat='2.0000000']", 1 + assert_select "osm>changeset[min_lon='0.1000000']", 1 + assert_select "osm>changeset[max_lon='0.1000000']", 1 + assert_select "osm>changeset[min_lat='0.2000000']", 1 + assert_select "osm>changeset[max_lat='0.2000000']", 1 # add another node to it with_controller(NodesController.new) do - xml = "" + xml = "" put node_create_path, :params => xml, :headers => auth_header assert_response :success, "Couldn't create second node." end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset for the second time." - assert_select "osm>changeset[min_lon='1.0000000']", 1 - assert_select "osm>changeset[max_lon='2.0000000']", 1 - assert_select "osm>changeset[min_lat='1.0000000']", 1 - assert_select "osm>changeset[max_lat='2.0000000']", 1 + assert_select "osm>changeset[min_lon='0.1000000']", 1 + assert_select "osm>changeset[max_lon='0.2000000']", 1 + assert_select "osm>changeset[min_lat='0.1000000']", 1 + assert_select "osm>changeset[max_lat='0.2000000']", 1 # add (delete) a way to it, which contains a point at (3,3) with_controller(WaysController.new) do @@ -2082,12 +2150,12 @@ module Api end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset for the third time." - assert_select "osm>changeset[min_lon='1.0000000']", 1 - assert_select "osm>changeset[max_lon='3.0000000']", 1 - assert_select "osm>changeset[min_lat='1.0000000']", 1 - assert_select "osm>changeset[max_lat='3.0000000']", 1 + assert_select "osm>changeset[min_lon='0.1000000']", 1 + assert_select "osm>changeset[max_lon='0.3000000']", 1 + assert_select "osm>changeset[min_lat='0.1000000']", 1 + assert_select "osm>changeset[max_lat='0.3000000']", 1 end ## @@ -2104,11 +2172,11 @@ module Api get changesets_path(:bbox => "-10,-10, 10, 10") assert_response :success, "can't get changesets in bbox" - assert_changesets [changeset2, changeset3] + assert_changesets_in_order [changeset3, changeset2] get changesets_path(:bbox => "4.5,4.5,4.6,4.6") assert_response :success, "can't get changesets in bbox" - assert_changesets [changeset3] + assert_changesets_in_order [changeset3] # not found when looking for changesets of non-existing users get changesets_path(:user => User.maximum(:id) + 1) @@ -2128,11 +2196,11 @@ module Api auth_header = basic_authorization_header private_user.email, "test" get changesets_path(:user => private_user.id), :headers => auth_header assert_response :success, "can't get changesets by user ID" - assert_changesets [private_user_changeset, private_user_closed_changeset] + assert_changesets_in_order [private_user_changeset, private_user_closed_changeset] get changesets_path(:display_name => private_user.display_name), :headers => auth_header assert_response :success, "can't get changesets by user name" - assert_changesets [private_user_changeset, private_user_closed_changeset] + assert_changesets_in_order [private_user_changeset, private_user_closed_changeset] # test json endpoint get changesets_path(:display_name => private_user.display_name), :headers => auth_header, :params => { :format => "json" } @@ -2152,39 +2220,39 @@ module Api get changesets_path(:user => private_user.id, :open => true), :headers => auth_header assert_response :success, "can't get changesets by user and open" - assert_changesets [private_user_changeset] + assert_changesets_in_order [private_user_changeset] get changesets_path(:time => "2007-12-31"), :headers => auth_header assert_response :success, "can't get changesets by time-since" - assert_changesets [private_user_changeset, private_user_closed_changeset, changeset, closed_changeset, changeset2, changeset3] + assert_changesets_in_order [changeset3, changeset2, changeset, private_user_changeset, private_user_closed_changeset, closed_changeset] get changesets_path(:time => "2008-01-01T12:34Z"), :headers => auth_header assert_response :success, "can't get changesets by time-since with hour" - assert_changesets [private_user_changeset, private_user_closed_changeset, changeset, closed_changeset, changeset2, changeset3] + assert_changesets_in_order [changeset3, changeset2, changeset, private_user_changeset, private_user_closed_changeset, closed_changeset] get changesets_path(:time => "2007-12-31T23:59Z,2008-01-02T00:01Z"), :headers => auth_header assert_response :success, "can't get changesets by time-range" - assert_changesets [closed_changeset] + assert_changesets_in_order [closed_changeset] get changesets_path(:open => "true"), :headers => auth_header assert_response :success, "can't get changesets by open-ness" - assert_changesets [private_user_changeset, changeset, changeset2, changeset3] + assert_changesets_in_order [changeset3, changeset2, changeset, private_user_changeset] get changesets_path(:closed => "true"), :headers => auth_header assert_response :success, "can't get changesets by closed-ness" - assert_changesets [private_user_closed_changeset, closed_changeset] + assert_changesets_in_order [private_user_closed_changeset, closed_changeset] get changesets_path(:closed => "true", :user => private_user.id), :headers => auth_header assert_response :success, "can't get changesets by closed-ness and user" - assert_changesets [private_user_closed_changeset] + assert_changesets_in_order [private_user_closed_changeset] get changesets_path(:closed => "true", :user => user.id), :headers => auth_header assert_response :success, "can't get changesets by closed-ness and user" - assert_changesets [closed_changeset] + assert_changesets_in_order [closed_changeset] get changesets_path(:changesets => "#{private_user_changeset.id},#{changeset.id},#{closed_changeset.id}"), :headers => auth_header assert_response :success, "can't get changesets by id (as comma-separated string)" - assert_changesets [private_user_changeset, changeset, closed_changeset] + assert_changesets_in_order [changeset, private_user_changeset, closed_changeset] get changesets_path(:changesets => ""), :headers => auth_header assert_response :bad_request, "should be a bad request since changesets is empty" @@ -2523,7 +2591,7 @@ module Api # bad changeset id assert_no_difference "changeset.subscribers.count" do - post api_changeset_subscribe_path(:id => 999111), :headers => auth_header + post api_changeset_subscribe_path(999111), :headers => auth_header end assert_response :not_found @@ -2573,7 +2641,7 @@ module Api # bad changeset id assert_no_difference "changeset.subscribers.count" do - post api_changeset_unsubscribe_path(:id => 999111), :headers => auth_header + post api_changeset_unsubscribe_path(999111), :headers => auth_header end assert_response :not_found @@ -2614,15 +2682,6 @@ module Api end end - ## - # check that certain changesets exist in the output - def assert_changesets(changesets) - assert_select "osm>changeset", changesets.size - changesets.each do |changeset| - assert_select "osm>changeset[id='#{changeset.id}']", 1 - end - end - ## # check that certain changesets exist in the output in the specified order def assert_changesets_in_order(changesets)