X-Git-Url: https://git.openstreetmap.org./rails.git/blobdiff_plain/baf10cd39289cd7e94a819305e46f43e85a136c6..6e5240e1987a429e29d230e811af9176772a8228:/test/controllers/relation_controller_test.rb?ds=sidebyside diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index c8ca07a38..fc8b8bbe5 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -1,5 +1,5 @@ -require 'test_helper' -require 'relation_controller' +require "test_helper" +require "relation_controller" class RelationControllerTest < ActionController::TestCase api_fixtures @@ -68,21 +68,65 @@ class RelationControllerTest < ActionController::TestCase # check that all relations containing a particular node, and no extra # relations, are returned from the relations_for_node call. def test_relations_for_node + node = create(:node) + # should include relations with that node as a member + relation_with_node = create(:relation_member, :member => node).relation + # should ignore relations without that node as a member + _relation_without_node = create(:relation_member).relation + # should ignore relations with the node involved indirectly, via a way + way = create(:way_node, :node => node).way + _relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations with the node involved indirectly, via a relation + second_relation = create(:relation_member, :member => node).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => node, :relation => relation_with_node, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => node, :relation => deleted_relation) + check_relations_for_element(:relations_for_node, "node", - current_nodes(:node_used_by_relationship).id, - [:visible_relation, :used_relation]) + node.id, + [relation_with_node, second_relation]) end def test_relations_for_way + way = create(:way) + # should include relations with that way as a member + relation_with_way = create(:relation_member, :member => way).relation + # should ignore relations without that way as a member + _relation_without_way = create(:relation_member).relation + # should ignore relations with the way involved indirectly, via a relation + second_relation = create(:relation_member, :member => way).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => way, :relation => relation_with_way, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => way, :relation => deleted_relation) + check_relations_for_element(:relations_for_way, "way", - current_ways(:used_way).id, - [:visible_relation]) + way.id, + [relation_with_way, second_relation]) end def test_relations_for_relation + relation = create(:relation) + # should include relations with that relation as a member + relation_with_relation = create(:relation_member, :member => relation).relation + # should ignore any relation without that relation as a member + _relation_without_relation = create(:relation_member).relation + # should ignore relations with the relation involved indirectly, via a relation + second_relation = create(:relation_member, :member => relation).relation + _super_relation = create(:relation_member, :member => second_relation).relation + # should combine multiple relation_member references into just one relation entry + create(:relation_member, :member => relation, :relation => relation_with_relation, :sequence_id => 2) + # should not include deleted relations + deleted_relation = create(:relation, :deleted) + create(:relation_member, :member => relation, :relation => deleted_relation) check_relations_for_element(:relations_for_relation, "relation", - current_relations(:used_relation).id, - [:visible_relation]) + relation.id, + [relation_with_relation, second_relation]) end def check_relations_for_element(method, type, id, expected_relations) @@ -96,21 +140,25 @@ class RelationControllerTest < ActionController::TestCase # we should have only the expected number of relations assert_select "osm>relation", expected_relations.size - # and each of them should contain the node we originally searched for - expected_relations.each do |r| - relation_id = current_relations(r).id - assert_select "osm>relation[id='#{relation_id}']>member[type='#{type}'][ref='#{id}']", 1 + # and each of them should contain the element we originally searched for + expected_relations.each do |relation| + # The relation should appear once, but the element could appear multiple times + assert_select "osm>relation[id='#{relation.id}']", 1 + assert_select "osm>relation[id='#{relation.id}']>member[type='#{type}'][ref='#{id}']" end end def test_full # check the "full" mode + get :full, :id => 999999 + assert_response :not_found + + get :full, :id => current_relations(:invisible_relation).id + assert_response :gone + get :full, :id => current_relations(:visible_relation).id assert_response :success - # FIXME check whether this contains the stuff we want! - if $VERBOSE - print @response.body - end + # FIXME: check whether this contains the stuff we want! end ## @@ -145,13 +193,17 @@ class RelationControllerTest < ActionController::TestCase # ------------------------------------- def test_create - basic_authorization users(:normal_user).email, "test" + private_user = create(:user, :data_public => false) + private_changeset = create(:changeset, :user => private_user) + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) + way = create(:way_with_nodes, :nodes_count => 2) - # put the relation in a dummy fixture changset - changeset_id = changesets(:normal_user_first_change).id + basic_authorization private_user.email, "test" # create an relation without members - content "" + content "" put :create # hope for forbidden, due to user assert_response :forbidden, @@ -160,10 +212,9 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member # This time try with a role attribute in the relation - nid = current_nodes(:used_node_1).id - content "" + - "" + - "" + content "" + + "" + + "" put :create # hope for forbidden due to user assert_response :forbidden, @@ -172,9 +223,8 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - nid = current_nodes(:used_node_1).id - content "" + - "" + "" + content "" + + "" + "" put :create # hope for forbidden due to user assert_response :forbidden, @@ -182,25 +232,20 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a way and a node as members - nid = current_nodes(:used_node_1).id - wid = current_ways(:used_way).id - content "" + - "" + - "" + - "" + content "" + + "" + + "" + + "" put :create # hope for forbidden, due to user assert_response :forbidden, "relation upload did not return success status" ## Now try with the public user - basic_authorization users(:public_user).email, "test" - - # put the relation in a dummy fixture changset - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create an relation without members - content "" + content "" put :create # hope for success assert_response :success, @@ -215,9 +260,9 @@ class RelationControllerTest < ActionController::TestCase "saved relation contains members but should not" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -228,10 +273,9 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member # This time try with a role attribute in the relation - nid = current_nodes(:used_node_1).id - content "" + - "" + - "" + content "" + + "" + + "" put :create # hope for success assert_response :success, @@ -246,9 +290,9 @@ class RelationControllerTest < ActionController::TestCase "saved relation does not contain exactly one member" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -260,9 +304,8 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a node as member, this time test that we don't # need a role attribute to be included - nid = current_nodes(:used_node_1).id - content "" + - "" + "" + content "" + + "" + "" put :create # hope for success assert_response :success, @@ -277,9 +320,9 @@ class RelationControllerTest < ActionController::TestCase "saved relation does not contain exactly one member" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -290,12 +333,10 @@ class RelationControllerTest < ActionController::TestCase ### # create an relation with a way and a node as members - nid = current_nodes(:used_node_1).id - wid = current_ways(:used_way).id - content "" + - "" + - "" + - "" + content "" + + "" + + "" + + "" put :create # hope for success assert_response :success, @@ -310,9 +351,9 @@ class RelationControllerTest < ActionController::TestCase "saved relation does not have exactly two members" assert_equal checkrelation.tags.length, 1, "saved relation does not contain exactly one tag" - assert_equal changeset_id, checkrelation.changeset.id, + assert_equal changeset.id, checkrelation.changeset.id, "saved relation does not belong in the changeset it was assigned to" - assert_equal users(:public_user).id, checkrelation.changeset.user_id, + assert_equal user.id, checkrelation.changeset.user_id, "saved relation does not belong to user that created it" assert_equal true, checkrelation.visible, "saved relation is not visible" @@ -334,12 +375,13 @@ class RelationControllerTest < ActionController::TestCase def test_update_relation_tags basic_authorization "test@example.com", "test" rel_id = current_relations(:multi_tag_relation).id + create_list(:relation_tag, 4, :relation => current_relations(:multi_tag_relation)) cs_id = changesets(:public_user_first_change).id with_relation(rel_id) do |rel| # alter one of the tags tag = rel.find("//osm/relation/tag").first - tag['v'] = 'some changed value' + tag["v"] = "some changed value" update_changeset(rel, cs_id) # check that the downloaded tags are the same as the uploaded tags... @@ -361,15 +403,18 @@ class RelationControllerTest < ActionController::TestCase # and the API gives sensible results. this is to test a case that # gregory marler noticed and posted to josm-dev. def test_update_relation_tags_via_upload - basic_authorization users(:public_user).email, "test" - rel_id = current_relations(:multi_tag_relation).id - cs_id = changesets(:public_user_first_change).id + user = create(:user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + create_list(:relation_tag, 4, :relation => relation) - with_relation(rel_id) do |rel| + basic_authorization user.email, "test" + + with_relation(relation.id) do |rel| # alter one of the tags tag = rel.find("//osm/relation/tag").first - tag['v'] = 'some changed value' - update_changeset(rel, cs_id) + tag["v"] = "some changed value" + update_changeset(rel, changeset.id) # check that the downloaded tags are the same as the uploaded tags... new_version = with_update_diff(rel) do |new_rel| @@ -377,10 +422,25 @@ class RelationControllerTest < ActionController::TestCase end # check the original one in the current_* table again - with_relation(rel_id) { |r| assert_tags_equal rel, r } + with_relation(relation.id) { |r| assert_tags_equal rel, r } # now check the version in the history - with_relation(rel_id, new_version) { |r| assert_tags_equal rel, r } + with_relation(relation.id, new_version) { |r| assert_tags_equal rel, r } + end + end + + def test_update_wrong_id + user = create(:user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + other_relation = create(:relation) + + basic_authorization user.email, "test" + with_relation(relation.id) do |rel| + update_changeset(rel, changeset.id) + content rel + put :update, :id => other_relation.id + assert_response :bad_request end end @@ -389,15 +449,15 @@ class RelationControllerTest < ActionController::TestCase # ------------------------------------- def test_create_invalid - basic_authorization users(:public_user).email, "test" + user = create(:user) + changeset = create(:changeset, :user => user) - # put the relation in a dummy fixture changset - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create a relation with non-existing node as member - content "" + - "" + - "" + content "" + + "" + + "" put :create # expect failure assert_response :precondition_failed, @@ -409,15 +469,16 @@ class RelationControllerTest < ActionController::TestCase # Test creating a relation, with some invalid XML # ------------------------------------- def test_create_invalid_xml - basic_authorization users(:public_user).email, "test" + user = create(:user) + changeset = create(:changeset, :user => user) + node = create(:node) - # put the relation in a dummy fixture changeset that works - changeset_id = changesets(:public_user_first_change).id + basic_authorization user.email, "test" # create some xml that should return an error - content "" + - "" + - "" + content "" + + "" + + "" put :create # expect failure assert_response :bad_request @@ -430,52 +491,58 @@ class RelationControllerTest < ActionController::TestCase # ------------------------------------- def test_delete + private_user = create(:user, :data_public => false) + private_user_closed_changeset = create(:changeset, :closed, :user => private_user) + user = create(:user) + closed_changeset = create(:changeset, :closed, :user => user) + changeset = create(:changeset, :user => user) + relation = create(:relation) + used_relation = create(:relation) + super_relation = create(:relation_member, :member => used_relation).relation + deleted_relation = create(:relation, :deleted) + multi_tag_relation = create(:relation) + create_list(:relation_tag, 4, :relation => multi_tag_relation) + ## First try to delete relation without auth - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :unauthorized ## Then try with the private user, to make sure that you get a forbidden - basic_authorization(users(:normal_user).email, "test") + basic_authorization(private_user.email, "test") # this shouldn't work, as we should need the payload... - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :forbidden # try to delete without specifying a changeset - content "" - delete :delete, :id => current_relations(:visible_relation).id + content "" + delete :delete, :id => relation.id assert_response :forbidden # try to delete with an invalid (closed) changeset - content update_changeset(current_relations(:visible_relation).to_xml, - changesets(:normal_user_closed_change).id) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, + private_user_closed_changeset.id) + delete :delete, :id => relation.id assert_response :forbidden # try to delete with an invalid (non-existent) changeset - content update_changeset(current_relations(:visible_relation).to_xml, 0) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, 0) + delete :delete, :id => relation.id assert_response :forbidden # this won't work because the relation is in-use by another relation - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content(used_relation.to_xml) + delete :delete, :id => used_relation.id assert_response :forbidden # this should work when we provide the appropriate payload... - content(relations(:visible_relation).to_xml) - delete :delete, :id => current_relations(:visible_relation).id + content(relation.to_xml) + delete :delete, :id => relation.id assert_response :forbidden # this won't work since the relation is already deleted - content(relations(:invisible_relation).to_xml) - delete :delete, :id => current_relations(:invisible_relation).id - assert_response :forbidden - - # this works now because the relation which was using this one - # has been deleted. - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content(deleted_relation.to_xml) + delete :delete, :id => deleted_relation.id assert_response :forbidden # this won't work since the relation never existed @@ -483,72 +550,71 @@ class RelationControllerTest < ActionController::TestCase assert_response :forbidden ## now set auth for the public user - basic_authorization(users(:public_user).email, "test") + basic_authorization(user.email, "test") # this shouldn't work, as we should need the payload... - delete :delete, :id => current_relations(:visible_relation).id + delete :delete, :id => relation.id assert_response :bad_request # try to delete without specifying a changeset - content "" - delete :delete, :id => current_relations(:visible_relation).id + content "" + delete :delete, :id => relation.id assert_response :bad_request assert_match(/Changeset id is missing/, @response.body) # try to delete with an invalid (closed) changeset - content update_changeset(current_relations(:visible_relation).to_xml, - changesets(:normal_user_closed_change).id) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, + closed_changeset.id) + delete :delete, :id => relation.id assert_response :conflict # try to delete with an invalid (non-existent) changeset - content update_changeset(current_relations(:visible_relation).to_xml, 0) - delete :delete, :id => current_relations(:visible_relation).id + content update_changeset(relation.to_xml, 0) + delete :delete, :id => relation.id assert_response :conflict # this won't work because the relation is in a changeset owned by someone else - content(relations(:used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id + content update_changeset(relation.to_xml, create(:changeset).id) + delete :delete, :id => relation.id assert_response :conflict, "shouldn't be able to delete a relation in a changeset owned by someone else (#{@response.body})" # this won't work because the relation in the payload is different to that passed - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:used_relation).id - assert_not_equal relations(:public_used_relation).id, current_relations(:used_relation).id + content update_changeset(relation.to_xml, changeset.id) + delete :delete, :id => create(:relation).id assert_response :bad_request, "shouldn't be able to delete a relation when payload is different to the url" # this won't work because the relation is in-use by another relation - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:public_used_relation).id + content update_changeset(used_relation.to_xml, changeset.id) + delete :delete, :id => used_relation.id assert_response :precondition_failed, "shouldn't be able to delete a relation used in a relation (#{@response.body})" - assert_equal "Precondition failed: The relation 5 is used in relation 6.", @response.body + assert_equal "Precondition failed: The relation #{used_relation.id} is used in relation #{super_relation.id}.", @response.body # this should work when we provide the appropriate payload... - content(relations(:multi_tag_relation).to_xml) - delete :delete, :id => current_relations(:multi_tag_relation).id + content update_changeset(multi_tag_relation.to_xml, changeset.id) + delete :delete, :id => multi_tag_relation.id assert_response :success # valid delete should return the new version number, which should # be greater than the old version number - assert @response.body.to_i > current_relations(:visible_relation).version, + assert @response.body.to_i > multi_tag_relation.version, "delete request should return a new version number for relation" # this won't work since the relation is already deleted - content(relations(:invisible_relation).to_xml) - delete :delete, :id => current_relations(:invisible_relation).id + content update_changeset(deleted_relation.to_xml, changeset.id) + delete :delete, :id => deleted_relation.id assert_response :gone # Public visible relation needs to be deleted - content(relations(:public_visible_relation).to_xml) - delete :delete, :id => current_relations(:public_visible_relation).id + content update_changeset(super_relation.to_xml, changeset.id) + delete :delete, :id => super_relation.id assert_response :success # this works now because the relation which was using this one # has been deleted. - content(relations(:public_used_relation).to_xml) - delete :delete, :id => current_relations(:public_used_relation).id + content update_changeset(used_relation.to_xml, changeset.id) + delete :delete, :id => used_relation.id assert_response :success, "should be able to delete a relation used in an old relation (#{@response.body})" @@ -568,8 +634,8 @@ class RelationControllerTest < ActionController::TestCase relation_xml = current_relations(:visible_relation).to_xml relation_element = relation_xml.find("//osm/relation").first new_tag = XML::Node.new("tag") - new_tag['k'] = "some_new_tag" - new_tag['v'] = "some_new_value" + new_tag["k"] = "some_new_tag" + new_tag["v"] = "some_new_value" relation_element << new_tag # update changeset ID to point to new changeset @@ -591,16 +657,15 @@ class RelationControllerTest < ActionController::TestCase [current_nodes(:used_node_1), current_nodes(:used_node_2), current_ways(:used_way), - current_ways(:way_with_versions) - ].each_with_index do |element, _version| + current_ways(:way_with_versions)].each_with_index do |element, _version| bbox = element.bbox.to_unscaled check_changeset_modify(bbox) do |changeset_id| relation_xml = Relation.find(relation_id).to_xml relation_element = relation_xml.find("//osm/relation").first new_member = XML::Node.new("member") - new_member['ref'] = element.id.to_s - new_member['type'] = element.class.to_s.downcase - new_member['role'] = "some_role" + new_member["ref"] = element.id.to_s + new_member["type"] = element.class.to_s.downcase + new_member["role"] = "some_role" relation_element << new_member # update changeset ID to point to new changeset @@ -669,13 +734,13 @@ OSM # insert a member at the front new_member = XML::Node.new "member" - new_member['ref'] = 5.to_s - new_member['type'] = 'node' - new_member['role'] = 'new first' + new_member["ref"] = 5.to_s + new_member["type"] = "node" + new_member["role"] = "new first" doc.find("//osm/relation").first.child.prev = new_member # update the version, should be 1? - doc.find("//osm/relation").first['id'] = relation_id.to_s - doc.find("//osm/relation").first['version'] = 1.to_s + doc.find("//osm/relation").first["id"] = relation_id.to_s + doc.find("//osm/relation").first["version"] = 1.to_s # upload the next version of the relation content doc @@ -802,11 +867,11 @@ OSM new_doc = XML::Parser.string(xml).parse doc_members = doc.find("//osm/relation/member").collect do |m| - [m['ref'].to_i, m['type'].to_sym, m['role']] + [m["ref"].to_i, m["type"].to_sym, m["role"]] end new_members = new_doc.find("//osm/relation/member").collect do |m| - [m['ref'].to_i, m['type'].to_sym, m['role']] + [m["ref"].to_i, m["type"].to_sym, m["role"]] end doc_members.zip(new_members).each do |d, n| @@ -900,14 +965,14 @@ OSM # the parsed XML doc is retured. def with_update_diff(rel) rel_id = rel.find("//osm/relation").first["id"].to_i - cs_id = rel.find("//osm/relation").first['changeset'].to_i + cs_id = rel.find("//osm/relation").first["changeset"].to_i version = nil with_controller(ChangesetController.new) do doc = OSM::API.new.get_xml_doc - change = XML::Node.new 'osmChange' + change = XML::Node.new "osmChange" doc.root = change - modify = XML::Node.new 'modify' + modify = XML::Node.new "modify" change << modify modify << doc.import(rel.find("//osm/relation").first) @@ -930,9 +995,8 @@ OSM ## # returns a k->v hash of tags from an xml doc def get_tags_as_hash(a) - a.find("//osm/relation/tag").sort_by { |v| v['k'] }.inject({}) do |h, v| - h[v['k']] = v['v'] - h + a.find("//osm/relation/tag").sort_by { |v| v["k"] }.each_with_object({}) do |v, h| + h[v["k"]] = v["v"] end end @@ -948,14 +1012,14 @@ OSM a_tags.each do |k, v| assert_equal v, b_tags[k], "Tags which were not altered should be the same. " + - "#{a_tags.inspect} != #{b_tags.inspect}" + "#{a_tags.inspect} != #{b_tags.inspect}" end end ## # update the changeset_id of a node element def update_changeset(xml, changeset_id) - xml_attr_rewrite(xml, 'changeset', changeset_id) + xml_attr_rewrite(xml, "changeset", changeset_id) end ##