From dcca3ab0e9fad052e884e5df715ba2391020f0cc Mon Sep 17 00:00:00 2001 From: Shaun McDonald Date: Wed, 29 Apr 2009 17:29:58 +0000 Subject: [PATCH] More changeset test improvements. Another fixture was required for one of the tests --- test/fixtures/changeset_tags.yml | 5 + test/fixtures/users.yml | 14 + test/functional/changeset_controller_test.rb | 271 ++++++++++++++----- test/test_helper.rb | 6 + test/unit/changeset_tag_test.rb | 2 +- 5 files changed, 233 insertions(+), 65 deletions(-) diff --git a/test/fixtures/changeset_tags.yml b/test/fixtures/changeset_tags.yml index 34d2bf4e6..2292f0dfc 100644 --- a/test/fixtures/changeset_tags.yml +++ b/test/fixtures/changeset_tags.yml @@ -2,3 +2,8 @@ changeset_1_tag_1: id: 1 k: created_by v: test suite yml + +changeset_2_tag_1: + id: 2 + k: created_by + v: test suite yml 2 diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 46f8885d9..ac0029a62 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -37,3 +37,17 @@ inactive_user: home_lat: 123.4 home_lon: 12.34 home_zoom: 15 + +second_public_user: + id: 4 + email: public@OpenStreetMap.org + active: true + pass_crypt: <%= Digest::MD5.hexdigest('test') %> + creation_time: "2008-05-01 01:23:45" + display_name: pulibc_test2 + data_public: true + description: some test description + home_lat: 89 + home_lon: 87 + home_zoom: 12 + diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb index b50e1b54a..018e5af23 100644 --- a/test/functional/changeset_controller_test.rb +++ b/test/functional/changeset_controller_test.rb @@ -9,8 +9,16 @@ class ChangesetControllerTest < ActionController::TestCase # ----------------------- def test_create - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:normal_user).email, "test" + # Create the first user's changeset + content "" + + "" + + "" + put :create + assert_require_public_data + + basic_authorization users(:public_user).email, "test" # Create the first user's changeset content "" + "" + @@ -34,22 +42,45 @@ class ChangesetControllerTest < ActionController::TestCase end def test_create_invalid - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:normal_user).email, "test" + content "" + put :create + assert_require_public_data + + ## Try the public user + basic_authorization users(:public_user).email, "test" content "" put :create assert_response :bad_request, "creating a invalid changeset should fail" end def test_create_invalid_no_content - basic_authorization "test@openstreetmap.org", "test" + ## First check with no auth + put :create + assert_response :unauthorized, "shouldn't be able to create a changeset with no auth" + + ## Now try to with the non-public user + basic_authorization users(:normal_user).email, "test" + put :create + assert_require_public_data + + ## Try the inactive user + basic_authorization users(:inactive_user).email, "test" + put :create + assert_inactive_user + + ## Now try to use the public user + basic_authorization users(:public_user).email, "test" put :create assert_response :bad_request, "creating a changeset with no content should fail" end def test_create_wrong_method - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" get :create assert_response :method_not_allowed + post :create + assert_response :method_not_allowed end ## @@ -82,7 +113,7 @@ class ChangesetControllerTest < ActionController::TestCase ## # test that a different user can't close another user's changeset def test_close_invalid - basic_authorization "test@example.com", "test" + basic_authorization user(:public_user).email, "test" put :close, :id => changesets(:normal_user_first_change).id assert_response :conflict @@ -91,22 +122,24 @@ class ChangesetControllerTest < ActionController::TestCase ## # upload something simple, but valid and check that it can - # be read back ok. + # be read back ok + # Also try without auth and another user. def test_upload_simple_valid - basic_authorization "test@openstreetmap.org", "test" + ## Try with no auth + changeset_id = changesets(:public_user_first_change).id # simple diff to change a node, way and relation by removing # their tags diff = < - - + + - + @@ -117,7 +150,71 @@ EOF # upload it content diff - post :upload, :id => 1 + post :upload, :id => changeset_id + assert_response :unauthorized, + "shouldnn't be able to upload a simple valid diff to changeset: #{@response.body}" + + + + ## Now try with a private user + basic_authorization users(:normal_user).email, "test" + changeset_id = changesets(:normal_user_first_change).id + + # simple diff to change a node, way and relation by removing + # their tags + diff = < + + + + + + + + + + + + + + +EOF + + # upload it + content diff + post :upload, :id => changeset_id + assert_response :forbidden, + "can't upload a simple valid diff to changeset: #{@response.body}" + + + + ## Now try with the public user + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id + + # simple diff to change a node, way and relation by removing + # their tags + diff = < + + + + + + + + + + + + + + +EOF + + # upload it + content diff + post :upload, :id => changeset_id assert_response :success, "can't upload a simple valid diff to changeset: #{@response.body}" @@ -486,15 +583,16 @@ EOF # upload a valid changeset which has a mixture of whitespace # to check a bug reported by ivansanchez (#1565). def test_upload_whitespace_valid - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id diff = < - - + - @@ -504,7 +602,7 @@ EOF # upload it content diff - post :upload, :id => 1 + post :upload, :id => changeset_id assert_response :success, "can't upload a valid diff with whitespace variations to changeset: #{@response.body}" @@ -521,27 +619,28 @@ EOF # upload a valid changeset which has a mixture of whitespace # to check a bug reported by ivansanchez. def test_upload_reuse_placeholder_valid - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id diff = < - + - + - + EOF # upload it content diff - post :upload, :id => 1 + post :upload, :id => changeset_id assert_response :success, "can't upload a valid diff with re-used placeholders to changeset: #{@response.body}" @@ -554,21 +653,22 @@ EOF # test what happens if a diff upload re-uses placeholder IDs in an # illegal way. def test_upload_placeholder_invalid - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id diff = < - - - + + + EOF # upload it content diff - post :upload, :id => 1 + post :upload, :id => changeset_id assert_response :bad_request, "shouldn't be able to re-use placeholder IDs" end @@ -577,15 +677,16 @@ EOF # test that uploading a way referencing invalid placeholders gives a # proper error, not a 500. def test_upload_placeholder_invalid_way - basic_authorization "test@example.com", "test" + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id diff = < - - - - + + + + @@ -597,7 +698,7 @@ EOF # upload it content diff - post :upload, :id => 2 + post :upload, :id => changeset_id assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder node not found for reference -4 in way -1", @response.body @@ -606,10 +707,10 @@ EOF diff = < - - - - + + + + @@ -621,7 +722,7 @@ EOF # upload it content diff - post :upload, :id => 2 + post :upload, :id => changeset_id assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder node not found for reference -4 in way 1", @response.body @@ -631,15 +732,16 @@ EOF # test that uploading a relation referencing invalid placeholders gives a # proper error, not a 500. def test_upload_placeholder_invalid_relation - basic_authorization "test@example.com", "test" + basic_authorization users(:public_user).email, "test" + changeset_id = changesets(:public_user_first_change).id diff = < - - - - + + + + @@ -651,7 +753,7 @@ EOF # upload it content diff - post :upload, :id => 2 + post :upload, :id => changeset_id assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder Node not found for reference -4 in relation -1.", @response.body @@ -660,10 +762,10 @@ EOF diff = < - - - - + + + + @@ -675,7 +777,7 @@ EOF # upload it content diff - post :upload, :id => 2 + post :upload, :id => changeset_id assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder Way not found for reference -1 in relation 1.", @response.body @@ -685,7 +787,7 @@ EOF # test what happens if a diff is uploaded containing only a node # move. def test_upload_node_move - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" content "" + "" + @@ -723,7 +825,7 @@ EOF ## # test what happens if a diff is uploaded adding a node to a way. def test_upload_way_extend - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" content "" + "" + @@ -762,7 +864,7 @@ EOF ## # test for more issues in #1568 def test_upload_empty_invalid - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" [ "", "", @@ -771,7 +873,7 @@ EOF ].each do |diff| # upload it content diff - post :upload, :id => 1 + post :upload, :id => changesets(:public_user_first_change).id assert_response(:success, "should be able to upload " + "empty changeset: " + diff) end @@ -781,8 +883,21 @@ EOF # when we make some simple changes we get the same changes back from the # diff download. def test_diff_download_simple + ## First try with the normal user, which should get a forbidden basic_authorization(users(:normal_user).email, "test") + # create a temporary changeset + content "" + + "" + + "" + put :create + assert_response :forbidden + + + + ## Now try with the public user + basic_authorization(users(:public_user).email, "test") + # create a temporary changeset content "" + "" + @@ -827,7 +942,7 @@ EOF # # NOTE: the error turned out to be something else completely! def test_josm_upload - basic_authorization(users(:normal_user).email, "test") + basic_authorization(users(:public_user).email, "test") # create a temporary changeset content "" + @@ -837,7 +952,7 @@ EOF assert_response :success changeset_id = @response.body.to_i - diff = < @@ -864,7 +979,7 @@ EOF -OSM +OSMFILE # upload it content diff @@ -886,7 +1001,7 @@ OSM # when we make some complex changes we get the same changes back from the # diff download. def test_diff_download_complex - basic_authorization(users(:normal_user).email, "test") + basic_authorization(users(:public_user).email, "test") # create a temporary changeset content "" + @@ -940,8 +1055,9 @@ EOF ## # check that the bounding box of a changeset gets updated correctly + ## FIXME: This should really be moded to a integration test due to the with_controller def test_changeset_bbox - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" # create a new changeset content "" @@ -1000,7 +1116,7 @@ EOF ## # test that the changeset :include method works as it should def test_changeset_include - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).display_name, "test" # create a new changeset content "" @@ -1038,11 +1154,11 @@ EOF basic_authorization "test@openstreetmap.org", "test" get :query, :user => users(:normal_user).id assert_response :success, "can't get changesets by user" - assert_changesets [1,3,4,6] + assert_changesets [1,3,6] get :query, :user => users(:normal_user).id, :open => true assert_response :success, "can't get changesets by user and open" - assert_changesets [1,4] + assert_changesets [1] get :query, :time => '2007-12-31' assert_response :success, "can't get changesets by time-since" @@ -1096,6 +1212,7 @@ EOF ## # check updating tags on a changeset def test_changeset_update + ## First try with the non-public user changeset = changesets(:normal_user_first_change) new_changeset = changeset.to_xml new_tag = XML::Node.new "tag" @@ -1109,12 +1226,37 @@ EOF assert_response :unauthorized # try with the wrong authorization - basic_authorization "test@example.com", "test" + basic_authorization users(:public_user).email, "test" + put :update, :id => changeset.id + assert_response :conflict + + # now this should get an unauthorized + basic_authorization users(:normal_user).email, "test" + put :update, :id => changeset.id + assert_require_public_data "user with their data non-public, shouldn't be able to edit their changeset" + + + ## Now try with the public user + changeset = changesets(:public_user_first_change) + new_changeset = changeset.to_xml + new_tag = XML::Node.new "tag" + new_tag['k'] = "tagtesting" + new_tag['v'] = "valuetesting" + new_changeset.find("//osm/changeset").first << new_tag + content new_changeset + + # try without any authorization + @request.env["HTTP_AUTHORIZATION"] = nil + put :update, :id => changeset.id + assert_response :unauthorized + + # try with the wrong authorization + basic_authorization users(:second_public_user).email, "test" put :update, :id => changeset.id assert_response :conflict # now this should work... - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" put :update, :id => changeset.id assert_response :success @@ -1127,7 +1269,7 @@ EOF # check that a user different from the one who opened the changeset # can't modify it. def test_changeset_update_invalid - basic_authorization "test@example.com", "test" + basic_authorization users(:public_user).email, "test" changeset = changesets(:normal_user_first_change) new_changeset = changeset.to_xml @@ -1143,8 +1285,9 @@ EOF ## # check that a changeset can contain a certain max number of changes. + ## FIXME should be changed to an integration test due to the with_controller def test_changeset_limits - basic_authorization "test@openstreetmap.org", "test" + basic_authorization users(:public_user).email, "test" # open a new changeset content "" diff --git a/test/test_helper.rb b/test/test_helper.rb index b32dd0a79..5dd319e02 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -130,5 +130,11 @@ class Test::Unit::TestCase assert_equal @response.headers['Error'], "You must make your edits public to upload new data", "Wrong error message" end + # Not sure this is the best response we could give + def assert_inactive_user(msg = "an inactive user shouldn't be able to access the API") + assert_response :unauthorized, msg + #assert_equal @response.headers['Error'], "" + end + # Add more helper methods to be used by all tests here... end diff --git a/test/unit/changeset_tag_test.rb b/test/unit/changeset_tag_test.rb index c89f426cc..40f9c5ac3 100644 --- a/test/unit/changeset_tag_test.rb +++ b/test/unit/changeset_tag_test.rb @@ -4,7 +4,7 @@ class ChangesetTagTest < Test::Unit::TestCase api_fixtures def test_changeset_tag_count - assert_equal 1, ChangesetTag.count + assert_equal 2, ChangesetTag.count end def test_length_key_valid -- 2.43.2