X-Git-Url: https://git.openstreetmap.org/rails.git/blobdiff_plain/be0e33862fedbf02e52a9e126cac31dfb5775b43..64b3e289ac14533c6fb2b1223b6417ad0446878c:/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 75896b202..29b3d5dac 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -17,10 +17,6 @@ module Api { :path => "/api/0.6/changeset/1/download", :method => :get }, { :controller => "api/changesets", :action => "download", :id => "1" } ) - assert_routing( - { :path => "/api/0.6/changeset/1/expand_bbox", :method => :post }, - { :controller => "api/changesets", :action => "expand_bbox", :id => "1" } - ) assert_routing( { :path => "/api/0.6/changeset/1", :method => :get }, { :controller => "api/changesets", :action => "show", :id => "1" } @@ -132,31 +128,40 @@ module Api # check that the changeset can be shown and returns the correct # document structure. def test_show - changeset_id = create(:changeset).id + changeset = create(:changeset) - get :show, :params => { :id => changeset_id } + get :show, :params => { :id => changeset.id } assert_response :success, "cannot get first changeset" - assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 - assert_select "osm>changeset[id='#{changeset_id}']", 1 + assert_select "osm[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 + assert_select "osm>changeset[id='#{changeset.id}']", 1 + assert_select "osm>changeset>@open", "true" + assert_select "osm>changeset>@created_at", changeset.created_at.xmlschema + assert_select "osm>changeset>@closed_at", 0 assert_select "osm>changeset>discussion", 0 - get :show, :params => { :id => changeset_id, :include_discussion => true } + get :show, :params => { :id => changeset.id, :include_discussion => true } assert_response :success, "cannot get first changeset with comments" - assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 - assert_select "osm>changeset[id='#{changeset_id}']", 1 + assert_select "osm[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 + assert_select "osm>changeset[id='#{changeset.id}']", 1 + assert_select "osm>changeset>@open", "true" + assert_select "osm>changeset>@created_at", changeset.created_at.xmlschema + assert_select "osm>changeset>@closed_at", 0 assert_select "osm>changeset>discussion", 1 assert_select "osm>changeset>discussion>comment", 0 - changeset_id = create(:changeset, :closed).id - create_list(:changeset_comment, 3, :changeset_id => changeset_id) + changeset = create(:changeset, :closed) + create_list(:changeset_comment, 3, :changeset_id => changeset.id) - get :show, :params => { :id => changeset_id, :include_discussion => true } + get :show, :params => { :id => changeset.id, :include_discussion => true } assert_response :success, "cannot get closed changeset with comments" - assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 - assert_select "osm>changeset[id='#{changeset_id}']", 1 + assert_select "osm[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 + assert_select "osm>changeset[id='#{changeset.id}']", 1 + assert_select "osm>changeset>@open", "false" + assert_select "osm>changeset>@created_at", changeset.created_at.xmlschema + assert_select "osm>changeset>@closed_at", changeset.closed_at.xmlschema assert_select "osm>changeset>discussion", 1 assert_select "osm>changeset>discussion>comment", 3 end @@ -165,12 +170,10 @@ 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| - begin - get :show, :params => { :id => id } - assert_response :not_found, "should get a not found" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + get :show, :params => { :id => id } + assert_response :not_found, "should get a not found" + rescue ActionController::UrlGenerationError => e + assert_match(/No route matches/, e.to_s) end end @@ -239,23 +242,19 @@ module Api # First try to do it with no auth cs_ids.each do |id| - begin - put :close, :params => { :id => id } - assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + put :close, :params => { :id => 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) end # Now try with auth basic_authorization create(:user).email, "test" cs_ids.each do |id| - begin - put :close, :params => { :id => id } - assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) - end + put :close, :params => { :id => id } + 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) end end @@ -283,7 +282,7 @@ module Api # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -299,7 +298,7 @@ module Api -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -312,7 +311,7 @@ CHANGESET # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -328,7 +327,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -341,7 +340,7 @@ CHANGESET # simple diff to change a node, way and relation by removing # their tags - diff = < @@ -357,7 +356,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -382,7 +381,7 @@ CHANGESET basic_authorization user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -401,7 +400,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -409,7 +408,7 @@ CHANGESET "can't upload a simple valid creation to changeset: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 + assert_select "diffResult[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 assert_select "diffResult>node", 1 assert_select "diffResult>way", 1 assert_select "diffResult>relation", 1 @@ -455,10 +454,10 @@ CHANGESET diff.root = XML::Node.new "osmChange" delete = XML::Node.new "delete" diff.root << delete - delete << super_relation.to_xml_node - delete << used_relation.to_xml_node - delete << used_way.to_xml_node - delete << used_node.to_xml_node + 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| @@ -478,10 +477,10 @@ CHANGESET assert_select "diffResult>relation", 2 # check that everything was deleted - assert_equal false, Node.find(used_node.id).visible - assert_equal false, Way.find(used_way.id).visible - assert_equal false, Relation.find(super_relation.id).visible - assert_equal false, Relation.find(used_relation.id).visible + 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 ## @@ -503,7 +502,7 @@ CHANGESET assert_select "diffResult>node", 1 # check that everything was deleted - assert_equal false, Node.find(node.id).visible + assert_not Node.find(node.id).visible end def test_repeated_changeset_create @@ -530,7 +529,7 @@ CHANGESET changeset_id = @response.body.to_i # upload some widely-spaced nodes, spiralling positive and negative - diff = < @@ -553,7 +552,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it, which used to cause an error like "PGError: ERROR: # integer out of range" (bug #2152). but shouldn't any more. @@ -587,9 +586,9 @@ CHANGESET diff.root = XML::Node.new "osmChange" delete = XML::Node.new "delete" diff.root << delete - delete << other_relation.to_xml_node - delete << used_way.to_xml_node - delete << used_node.to_xml_node + 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| @@ -605,10 +604,10 @@ CHANGESET 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_equal true, Node.find(used_node.id).visible - assert_equal true, Way.find(used_way.id).visible - assert_equal true, Relation.find(relation.id).visible - assert_equal true, Relation.find(other_relation.id).visible + 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 ## @@ -630,9 +629,9 @@ CHANGESET delete = XML::Node.new "delete" diff.root << delete delete["if-unused"] = "" - delete << used_relation.to_xml_node - delete << used_way.to_xml_node - delete << used_node.to_xml_node + 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| @@ -647,7 +646,7 @@ CHANGESET "can't do a conditional delete of in use objects: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 + assert_select "diffResult[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 assert_select "diffResult>node", 1 assert_select "diffResult>way", 1 assert_select "diffResult>relation", 1 @@ -671,9 +670,9 @@ CHANGESET assert_equal used_relation.version, doc.find("//diffResult/relation").first["new_version"].to_i # check that nothing was, in fact, deleted - assert_equal true, Node.find(used_node.id).visible - assert_equal true, Way.find(used_way.id).visible - assert_equal true, Relation.find(used_relation.id).visible + assert Node.find(used_node.id).visible + assert Way.find(used_way.id).visible + assert Relation.find(used_relation.id).visible end ## @@ -684,7 +683,7 @@ CHANGESET basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -692,7 +691,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -714,7 +713,7 @@ CHANGESET basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -734,7 +733,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -742,7 +741,7 @@ CHANGESET "can't upload a complex diff to changeset: #{@response.body}" # check the returned payload - assert_select "diffResult[version='#{API_VERSION}'][generator='#{GENERATOR}']", 1 + 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 @@ -773,7 +772,7 @@ CHANGESET basic_authorization changeset.user.email, "test" # simple diff to create a node way and relation using placeholders - diff = < @@ -795,7 +794,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -818,7 +817,7 @@ CHANGESET # change the location of a node multiple times, each time referencing # the last version. doesn't this depend on version numbers being # sequential? - diff = < @@ -831,7 +830,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -853,14 +852,14 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -875,13 +874,13 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -896,13 +895,13 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < -CHANGESET + CHANGESET post :upload, :params => { :id => changeset.id }, :body => diff assert_response :bad_request, "Shouldn't be able to upload a diff with the action ping" assert_equal @response.body, "Unknown action ping, choices are create, modify, delete" @@ -921,7 +920,7 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < @@ -933,7 +932,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -956,7 +955,7 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < @@ -970,7 +969,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -990,7 +989,7 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < @@ -998,7 +997,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -1015,7 +1014,7 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < @@ -1029,7 +1028,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -1038,7 +1037,7 @@ CHANGESET assert_equal "Placeholder node not found for reference -4 in way -1", @response.body # the same again, but this time use an existing way - diff = < @@ -1052,7 +1051,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -1070,7 +1069,7 @@ CHANGESET basic_authorization changeset.user.email, "test" - diff = < @@ -1084,7 +1083,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -1093,7 +1092,7 @@ CHANGESET assert_equal "Placeholder Node not found for reference -4 in relation -1.", @response.body # the same again, but this time use an existing relation - diff = < @@ -1107,7 +1106,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset.id }, :body => diff @@ -1134,7 +1133,7 @@ CHANGESET diff = XML::Document.new diff.root = XML::Node.new "osmChange" modify = XML::Node.new "modify" - xml_old_node = old_node.to_xml_node + xml_old_node = xml_node_for_node(old_node) xml_old_node["lat"] = 2.0.to_s xml_old_node["lon"] = 2.0.to_s xml_old_node["changeset"] = changeset_id.to_s @@ -1172,7 +1171,7 @@ CHANGESET diff = XML::Document.new diff.root = XML::Node.new "osmChange" modify = XML::Node.new "modify" - xml_old_way = old_way.to_xml_node + 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 xml_old_way << nd_ref @@ -1225,7 +1224,7 @@ CHANGESET diff.root = XML::Node.new "osmChange" delete = XML::Node.new "delete" diff.root << delete - delete << node.to_xml_node + delete << xml_node_for_node(node) # upload it error_format "xml" @@ -1234,7 +1233,7 @@ CHANGESET "failed to return error in XML format" # check the returned payload - assert_select "osmError[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1 + assert_select "osmError[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1 assert_select "osmError>status", 1 assert_select "osmError>message", 1 end @@ -1267,7 +1266,7 @@ CHANGESET changeset_id = @response.body.to_i # add a diff to it - diff = < @@ -1280,7 +1279,7 @@ CHANGESET -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -1311,7 +1310,7 @@ CHANGESET assert_response :success changeset_id = @response.body.to_i - diff = < @@ -1338,7 +1337,7 @@ CHANGESET -OSMFILE + OSMFILE # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -1373,7 +1372,7 @@ OSMFILE changeset_id = @response.body.to_i # add a diff to it - diff = < @@ -1393,7 +1392,7 @@ OSMFILE -CHANGESET + CHANGESET # upload it post :upload, :params => { :id => changeset_id }, :body => diff @@ -1428,7 +1427,7 @@ CHANGESET assert_template nil # print @response.body # FIXME: needs more assert_select tests - assert_select "osmChange[version='#{API_VERSION}'][generator='#{GENERATOR}']" do + assert_select "osmChange[version='#{Settings.api_version}'][generator='#{Settings.generator}']" do assert_select "create", :count => 5 assert_select "create>node[id='#{node.id}'][visible='#{node.visible?}'][version='#{node.version}']" do assert_select "tag[k='#{tag.k}'][v='#{tag.v}']" @@ -1484,7 +1483,7 @@ CHANGESET # add (delete) a way to it, which contains a point at (3,3) with_controller(WaysController.new) do - xml = update_changeset(way.to_xml, changeset_id) + xml = update_changeset(xml_for_way(way), changeset_id) put :delete, :params => { :id => way.id }, :body => xml.to_s assert_response :success, "Couldn't delete a way." end @@ -1498,57 +1497,6 @@ CHANGESET assert_select "osm>changeset[max_lat='3.0000000']", 1 end - ## - # test that the changeset :include method works as it should - def test_changeset_include - basic_authorization create(:user).display_name, "test" - - # create a new changeset - put :create, :body => "" - assert_response :success, "Creating of changeset failed." - changeset_id = @response.body.to_i - - # NOTE: the include method doesn't over-expand, like inserting - # a real method does. this is because we expect the client to - # know what it is doing! - check_after_include(changeset_id, 1, 1, [1, 1, 1, 1]) - check_after_include(changeset_id, 3, 3, [1, 1, 3, 3]) - check_after_include(changeset_id, 4, 2, [1, 1, 4, 3]) - check_after_include(changeset_id, 2, 2, [1, 1, 4, 3]) - check_after_include(changeset_id, -1, -1, [-1, -1, 4, 3]) - check_after_include(changeset_id, -2, 5, [-2, -1, 4, 5]) - end - - ## - # test that a not found, wrong method with the expand bbox works as expected - def test_changeset_expand_bbox_error - basic_authorization create(:user).display_name, "test" - - # create a new changeset - xml = "" - put :create, :body => xml - assert_response :success, "Creating of changeset failed." - changeset_id = @response.body.to_i - - lon = 58.2 - lat = -0.45 - - # Try and put - xml = "" - put :expand_bbox, :params => { :id => changeset_id }, :body => xml - assert_response :method_not_allowed, "shouldn't be able to put a bbox expand" - - # Try to get the update - xml = "" - get :expand_bbox, :params => { :id => changeset_id }, :body => xml - assert_response :method_not_allowed, "shouldn't be able to get a bbox expand" - - # Try to use a hopefully missing changeset - xml = "" - post :expand_bbox, :params => { :id => changeset_id + 13245 }, :body => xml - assert_response :not_found, "shouldn't be able to do a bbox expand on a nonexistant changeset" - end - ## # test the query functionality of changesets def test_query @@ -1558,8 +1506,8 @@ CHANGESET user = create(:user) changeset = create(:changeset, :user => user) closed_changeset = create(:changeset, :closed, :user => user, :created_at => Time.utc(2008, 1, 1, 0, 0, 0), :closed_at => Time.utc(2008, 1, 2, 0, 0, 0)) - changeset2 = create(:changeset, :min_lat => 5 * GeoRecord::SCALE, :min_lon => 5 * GeoRecord::SCALE, :max_lat => 15 * GeoRecord::SCALE, :max_lon => 15 * GeoRecord::SCALE) - changeset3 = create(:changeset, :min_lat => 4.5 * GeoRecord::SCALE, :min_lon => 4.5 * GeoRecord::SCALE, :max_lat => 5 * GeoRecord::SCALE, :max_lon => 5 * GeoRecord::SCALE) + changeset2 = create(:changeset, :min_lat => (5 * GeoRecord::SCALE).round, :min_lon => (5 * GeoRecord::SCALE).round, :max_lat => (15 * GeoRecord::SCALE).round, :max_lon => (15 * GeoRecord::SCALE).round) + changeset3 = create(:changeset, :min_lat => (4.5 * GeoRecord::SCALE).round, :min_lon => (4.5 * GeoRecord::SCALE).round, :max_lat => (5 * GeoRecord::SCALE).round, :max_lon => (5 * GeoRecord::SCALE).round) get :query, :params => { :bbox => "-10,-10, 10, 10" } assert_response :success, "can't get changesets in bbox" @@ -1674,7 +1622,7 @@ CHANGESET changeset = create(:changeset, :user => user) ## First try with a non-public user - new_changeset = private_changeset.to_xml + new_changeset = create_changeset_xml(:user => private_user) new_tag = XML::Node.new "tag" new_tag["k"] = "tagtesting" new_tag["v"] = "valuetesting" @@ -1695,8 +1643,7 @@ CHANGESET assert_require_public_data "user with their data non-public, shouldn't be able to edit their changeset" ## Now try with the public user - create(:changeset_tag, :changeset => changeset) - new_changeset = changeset.to_xml + new_changeset = create_changeset_xml(:id => 1) new_tag = XML::Node.new "tag" new_tag["k"] = "tagtesting" new_tag["v"] = "valuetesting" @@ -1718,7 +1665,7 @@ CHANGESET assert_response :success assert_select "osm>changeset[id='#{changeset.id}']", 1 - assert_select "osm>changeset>tag", 2 + assert_select "osm>changeset>tag", 1 assert_select "osm>changeset>tag[k='tagtesting'][v='valuetesting']", 1 end @@ -1729,7 +1676,7 @@ CHANGESET basic_authorization create(:user).email, "test" changeset = create(:changeset) - new_changeset = changeset.to_xml + new_changeset = create_changeset_xml(:user => changeset.user, :id => changeset.id) new_tag = XML::Node.new "tag" new_tag["k"] = "testing" new_tag["v"] = "testing" @@ -1927,26 +1874,6 @@ CHANGESET end end - ## - # call the include method and assert properties of the bbox - def check_after_include(changeset_id, lon, lat, bbox) - xml = "" - post :expand_bbox, :params => { :id => changeset_id }, :body => xml - assert_response :success, "Setting include of changeset failed: #{@response.body}" - - # check exactly one changeset - assert_select "osm>changeset", 1 - assert_select "osm>changeset[id='#{changeset_id}']", 1 - - # check the bbox - doc = XML::Parser.string(@response.body).parse - changeset = doc.find("//osm/changeset").first - assert_equal bbox[0], changeset["min_lon"].to_f, "min lon" - assert_equal bbox[1], changeset["min_lat"].to_f, "min lat" - assert_equal bbox[2], changeset["max_lon"].to_f, "max lon" - assert_equal bbox[3], changeset["max_lat"].to_f, "max lat" - end - ## # update the changeset_id of a way element def update_changeset(xml, changeset_id) @@ -1959,5 +1886,20 @@ CHANGESET xml.find("//osm/way").first[name] = value.to_s xml end + + ## + # build XML for changesets + def create_changeset_xml(user: nil, id: nil) + root = XML::Document.new + root.root = XML::Node.new "osm" + cs = XML::Node.new "changeset" + if user + cs["user"] = user.display_name + cs["uid"] = user.id.to_s + end + cs["id"] = id.to_s if id + root.root << cs + root + end end end