]> git.openstreetmap.org Git - rails.git/commitdiff
Merge pull request #2427 from mmd-osm/patch/remove_expand_bbox
authorAndy Allan <git@gravitystorm.co.uk>
Wed, 4 Dec 2019 11:48:34 +0000 (12:48 +0100)
committerGitHub <noreply@github.com>
Wed, 4 Dec 2019 11:48:34 +0000 (12:48 +0100)
Remove expand_bbox endpoint

1  2 
app/abilities/api_ability.rb
app/abilities/api_capability.rb
config/routes.rb
test/controllers/api/changesets_controller_test.rb

index 217fe9713c8745f0c666e261389175e883425834,7aef15f114e017b7748cd2fa0185f918c209f4c4..62cd2b17ebbd41ad9408ebcfc5730134991b3506
@@@ -34,10 -34,10 +34,10 @@@ class ApiAbilit
          can [:new, :create], Report
          can [:create, :show, :update, :destroy, :data], Trace
          can [:details, :gpx_files], User
 -        can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
 +        can [:index, :show, :update, :update_all, :destroy], UserPreference
  
          if user.terms_agreed?
-           can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset
+           can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset
            can :create, ChangesetComment
            can [:create, :update, :delete], Node
            can [:create, :update, :delete], Way
index 64861f1d679cad8b962618909e86c31cfc9d2742,a4ca252045aebae3bba2e559e71e4a2bbe3963d9..beb4d39bf3fa62ba20641899a89220cb91594b71
@@@ -10,11 -10,11 +10,11 @@@ class ApiCapabilit
        can [:create, :update, :destroy], Trace if capability?(token, :allow_write_gpx)
        can [:details], User if capability?(token, :allow_read_prefs)
        can [:gpx_files], User if capability?(token, :allow_read_gpx)
 -      can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs)
 -      can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs)
 +      can [:index, :show], UserPreference if capability?(token, :allow_read_prefs)
 +      can [:update, :update_all, :destroy], UserPreference if capability?(token, :allow_write_prefs)
  
        if token&.user&.terms_agreed?
-         can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset if capability?(token, :allow_write_api)
+         can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset if capability?(token, :allow_write_api)
          can :create, ChangesetComment if capability?(token, :allow_write_api)
          can [:create, :update, :delete], Node if capability?(token, :allow_write_api)
          can [:create, :update, :delete], Way if capability?(token, :allow_write_api)
diff --combined config/routes.rb
index d61da1ea93872cd20429b60728fa00a91bb5e86c,d8d91f47ebcabdbf4e7589bb4e579c5b5f0b58d5..992197814ee15b2dad1c7a8bb07e9f14216f4ac7
@@@ -12,7 -12,6 +12,6 @@@ OpenStreetMap::Application.routes.draw 
      put "changeset/create" => "api/changesets#create"
      post "changeset/:id/upload" => "api/changesets#upload", :id => /\d+/
      get "changeset/:id/download" => "api/changesets#download", :as => :changeset_download, :id => /\d+/
-     post "changeset/:id/expand_bbox" => "api/changesets#expand_bbox", :id => /\d+/
      get "changeset/:id" => "api/changesets#show", :as => :changeset_show, :id => /\d+/
      post "changeset/:id/subscribe" => "api/changesets#subscribe", :as => :changeset_subscribe, :id => /\d+/
      post "changeset/:id/unsubscribe" => "api/changesets#unsubscribe", :as => :changeset_unsubscribe, :id => /\d+/
      get "user/gpx_files" => "api/users#gpx_files"
      get "users" => "api/users#index", :as => :api_users
  
 -    get "user/preferences" => "api/user_preferences#read"
 -    get "user/preferences/:preference_key" => "api/user_preferences#read_one"
 -    put "user/preferences" => "api/user_preferences#update"
 -    put "user/preferences/:preference_key" => "api/user_preferences#update_one"
 -    delete "user/preferences/:preference_key" => "api/user_preferences#delete_one"
 +    get "user/preferences" => "api/user_preferences#index"
 +    get "user/preferences/:preference_key" => "api/user_preferences#show"
 +    put "user/preferences" => "api/user_preferences#update_all"
 +    put "user/preferences/:preference_key" => "api/user_preferences#update"
 +    delete "user/preferences/:preference_key" => "api/user_preferences#destroy"
  
      post "gpx/create" => "api/traces#create"
      get "gpx/:id" => "api/traces#show", :id => /\d+/
    get "/trace/create", :to => redirect(:path => "/traces/new")
    get "/trace/:id/data" => "traces#data", :id => /\d+/, :as => "trace_data"
    get "/trace/:id/edit", :to => redirect(:path => "/traces/%{id}/edit")
 -  post "/trace/:id/delete" => "traces#delete", :id => /\d+/
  
    # diary pages
    resources :diary_entries, :path => "diary", :only => [:new, :create, :index] do
index a4583a9289e4c95ec90fc476afc163c94bfed890,e8dda0ba16407f1dedc234eedd2488d848a342cc..be6b84b47aa76e52b3fcbd3c0ff327d628797d14
@@@ -17,10 -17,6 +17,6 @@@ module Ap
          { :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" }
@@@ -458,10 -454,10 +454,10 @@@ CHANGESE
        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|
@@@ -590,9 -586,9 +586,9 @@@ CHANGESE
        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|
        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|
@@@ -1137,7 -1133,7 +1133,7 @@@ CHANGESE
        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
        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
        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"
@@@ -1487,7 -1483,7 +1483,7 @@@ CHANGESE
  
        # 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
        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 => "<osm><changeset/></osm>"
-       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 = "<osm><changeset/></osm>"
-       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 = "<osm><node lon='#{lon}' lat='#{lat}'/></osm>"
-       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 = "<osm><node lon='#{lon}' lat='#{lat}'/></osm>"
-       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 = "<osm><node lon='#{lon}' lat='#{lat}'/></osm>"
-       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
        end
      end
  
-     ##
-     # call the include method and assert properties of the bbox
-     def check_after_include(changeset_id, lon, lat, bbox)
-       xml = "<osm><node lon='#{lon}' lat='#{lat}'/></osm>"
-       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)