From bfef57f2ac82c9a59a372228f6b2c47545166384 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Mon, 28 Apr 2025 18:16:16 +0300 Subject: [PATCH] Create api changeset upload resource --- app/abilities/api_ability.rb | 2 +- .../api/changesets/uploads_controller.rb | 44 +++++++++ app/controllers/api/changesets_controller.rb | 37 +------- config/routes.rb | 9 +- .../api/changesets_controller_test.rb | 94 +++++++++---------- .../api/relations_controller_test.rb | 2 +- 6 files changed, 102 insertions(+), 86 deletions(-) create mode 100644 app/controllers/api/changesets/uploads_controller.rb diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 14d332ed0..3b621686c 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -34,7 +34,7 @@ class ApiAbility can :read, :active_user_blocks_list if scopes.include?("read_prefs") if user.terms_agreed? - can [:create, :update, :upload], Changeset if scopes.include?("write_map") + can [:create, :update], Changeset if scopes.include?("write_map") can [:create, :destroy], ChangesetSubscription if scopes.include?("write_map") can :create, ChangesetComment if scopes.include?("write_changeset_comments") can [:create, :update, :destroy], [Node, Way, Relation] if scopes.include?("write_map") diff --git a/app/controllers/api/changesets/uploads_controller.rb b/app/controllers/api/changesets/uploads_controller.rb new file mode 100644 index 000000000..ee9406d91 --- /dev/null +++ b/app/controllers/api/changesets/uploads_controller.rb @@ -0,0 +1,44 @@ +module Api + module Changesets + class UploadsController < ApiController + before_action :check_api_writable + before_action :authorize + + authorize_resource :class => Changeset + + before_action :require_public_data + + skip_around_action :api_call_timeout + + # Helper methods for checking consistency + include ConsistencyValidations + + ## + # Upload a diff in a single transaction. + # + # This means that each change within the diff must succeed, i.e: that + # each version number mentioned is still current. Otherwise the entire + # transaction *must* be rolled back. + # + # Furthermore, each element in the diff can only reference the current + # changeset. + # + # Returns: a diffResult document, as described in + # http://wiki.openstreetmap.org/wiki/OSM_Protocol_Version_0.6 + def create + changeset = Changeset.find(params[:changeset_id]) + check_changeset_consistency(changeset, current_user) + + diff_reader = DiffReader.new(request.raw_post, changeset) + Changeset.transaction do + result = diff_reader.commit + # the number of changes in this changeset has already been + # updated and is visible in this transaction so we don't need + # to allow for any more when checking the limit + check_rate_limit(0) + render :xml => result.to_s + end + end + end + end +end diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index 1a60fa99b..cadd4ac33 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -4,16 +4,14 @@ module Api class ChangesetsController < ApiController include QueryMethods - before_action :check_api_writable, :only => [:create, :update, :upload] + before_action :check_api_writable, :only => [:create, :update] before_action :setup_user_auth, :only => [:show] - before_action :authorize, :only => [:create, :update, :upload] + before_action :authorize, :only => [:create, :update] authorize_resource - before_action :require_public_data, :only => [:create, :update, :upload] - before_action :set_request_formats, :except => [:create, :upload] - - skip_around_action :api_call_timeout, :only => [:upload] + before_action :require_public_data, :only => [:create, :update] + before_action :set_request_formats, :except => [:create] # Helper methods for checking consistency include ConsistencyValidations @@ -87,33 +85,6 @@ module Api render :plain => cs.id.to_s end - ## - # Upload a diff in a single transaction. - # - # This means that each change within the diff must succeed, i.e: that - # each version number mentioned is still current. Otherwise the entire - # transaction *must* be rolled back. - # - # Furthermore, each element in the diff can only reference the current - # changeset. - # - # Returns: a diffResult document, as described in - # http://wiki.openstreetmap.org/wiki/OSM_Protocol_Version_0.6 - def upload - changeset = Changeset.find(params[:id]) - check_changeset_consistency(changeset, current_user) - - diff_reader = DiffReader.new(request.raw_post, changeset) - Changeset.transaction do - result = diff_reader.commit - # the number of changes in this changeset has already been - # updated and is visible in this transaction so we don't need - # to allow for any more when checking the limit - check_rate_limit(0) - render :xml => result.to_s - end - end - ## # updates a changeset's tags. none of the changeset's attributes are # user-modifiable, so they will be ignored. diff --git a/config/routes.rb b/config/routes.rb index 3aef845c4..c6d98691e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,15 +16,16 @@ OpenStreetMap::Application.routes.draw do scope "api/0.6", :module => :api do get "capabilities" => "capabilities#show" get "permissions" => "permissions#show" - - post "changeset/:id/upload" => "changesets#upload", :as => :changeset_upload, :id => /\d+/ end namespace :api, :path => "api/0.6" do resources :changesets, :only => [:index, :create] resources :changesets, :path => "changeset", :id => /\d+/, :only => [:show, :update] do - resource :close, :module => :changesets, :only => :update - resource :download, :module => :changesets, :only => :show + scope :module => :changesets do + resource :upload, :only => :create + resource :download, :only => :show + resource :close, :only => :update + end resource :subscription, :controller => :changeset_subscriptions, :only => [:create, :destroy] resources :changeset_comments, :path => "comment", :only => :create end diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 05aa96b15..7338cb138 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -31,7 +31,7 @@ module Api ) assert_routing( { :path => "/api/0.6/changeset/1/upload", :method => :post }, - { :controller => "api/changesets", :action => "upload", :id => "1" } + { :controller => "api/changesets/uploads", :action => "create", :changeset_id => "1" } ) assert_recognizes( @@ -690,7 +690,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff + post api_changeset_upload_path(changeset), :params => diff assert_response :unauthorized, "shouldn't be able to upload a simple valid diff to changeset: #{@response.body}" @@ -719,7 +719,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(private_changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(private_changeset), :params => diff, :headers => auth_header assert_response :forbidden, "can't upload a simple valid diff to changeset: #{@response.body}" @@ -748,7 +748,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a simple valid diff to changeset: #{@response.body}" @@ -792,7 +792,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a simple valid creation to changeset: #{@response.body}" @@ -857,7 +857,7 @@ module Api end # upload it - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :success, "can't upload a deletion diff to changeset: #{@response.body}" @@ -884,7 +884,7 @@ module Api diff = "" # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a deletion diff to changeset: #{@response.body}" @@ -950,7 +950,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(changeset_id), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a spatially-large diff to changeset: #{@response.body}" @@ -992,7 +992,7 @@ module Api end # upload it - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :precondition_failed, "shouldn't be able to upload a invalid deletion diff: #{@response.body}" assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body @@ -1035,7 +1035,7 @@ module Api end # upload it - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :success, "can't do a conditional delete of in use objects: #{@response.body}" @@ -1086,7 +1086,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to upload too long a tag to changeset: #{@response.body}" end @@ -1128,7 +1128,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a complex diff to changeset: #{@response.body}" @@ -1189,7 +1189,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :conflict, "uploading a diff with multiple changesets should have failed" @@ -1225,7 +1225,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload multiple versions of an element in a diff: #{@response.body}" @@ -1254,7 +1254,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :conflict, "shouldn't be able to upload the same element twice in a diff: #{@response.body}" end @@ -1275,7 +1275,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to upload an element without version: #{@response.body}" end @@ -1294,7 +1294,7 @@ module Api CHANGESET - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "Shouldn't be able to upload a diff with the action ping" assert_equal("Unknown action ping, choices are create, modify, delete", @response.body) end @@ -1327,7 +1327,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a valid diff with whitespace variations to changeset: #{@response.body}" @@ -1364,7 +1364,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :success, "can't upload a valid diff with re-used placeholders to changeset: #{@response.body}" @@ -1392,7 +1392,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to re-use placeholder IDs" @@ -1409,7 +1409,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to re-use placeholder IDs" end @@ -1433,7 +1433,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't refer elements behind it" end @@ -1456,7 +1456,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :gone, "transaction should be cancelled by second deletion" @@ -1473,7 +1473,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_select "diffResult>node", 3 assert_select "diffResult>node[old_id='-1']", 3 @@ -1507,7 +1507,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header 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 @@ -1530,7 +1530,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder node not found for reference -4 in way #{way.id}", @response.body @@ -1562,7 +1562,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header 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 @@ -1585,7 +1585,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :bad_request, "shouldn't be able to use invalid placeholder IDs" assert_equal "Placeholder Way not found for reference -1 in relation #{relation.id}.", @response.body @@ -1617,7 +1617,7 @@ module Api diff.root << modify # upload it - post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" @@ -1656,7 +1656,7 @@ module Api diff.root << modify # upload it - post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" @@ -1680,7 +1680,7 @@ module Api "", ""].each do |diff| # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response(:success, "should be able to upload " \ "empty changeset: " + diff) end @@ -1704,7 +1704,7 @@ module Api # upload it error_header = error_format_header "xml" - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header.merge(error_header) + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header.merge(error_header) assert_response :success, "failed to return error in XML format" @@ -1729,7 +1729,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Node should not be found" # modify way @@ -1742,7 +1742,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Way should not be found" # modify relation @@ -1755,7 +1755,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Relation should not be found" # delete node @@ -1768,7 +1768,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Node should not be deleted" # delete way @@ -1781,7 +1781,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Way should not be deleted" # delete relation @@ -1794,7 +1794,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :not_found, "Relation should not be deleted" end @@ -1827,7 +1827,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :bad_request, "shouldn't be able to use reference -4 in relation -2: #{@response.body}" assert_equal "Placeholder Relation not found for reference -4 in relation -2.", @response.body end @@ -1855,7 +1855,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff.to_s, :headers => auth_header assert_response :precondition_failed, "shouldn't be able to upload a invalid deletion diff: #{@response.body}" assert_equal "Precondition failed: Node #{node.id} is still used by ways #{way.id}.", @response.body @@ -1903,7 +1903,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :too_many_requests, "upload did not hit rate limit" end @@ -1958,7 +1958,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :too_many_requests, "upload did not hit rate limit" end @@ -1989,7 +1989,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :payload_too_large, "upload did not hit size limit" end @@ -2021,7 +2021,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset), :params => diff, :headers => auth_header assert_response :payload_too_large, "upload did not hit size limit" end @@ -2069,7 +2069,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header + post api_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}" @@ -2127,7 +2127,7 @@ module Api OSMFILE # upload it - post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header + post api_changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a diff from JOSM: #{@response.body}" @@ -2182,7 +2182,7 @@ module Api CHANGESET # upload it - post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header + post api_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}" diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index a4c8522e8..0d0624518 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -1128,7 +1128,7 @@ module Api change << modify modify << doc.import(rel.find("//osm/relation").first) - post changeset_upload_path(cs_id), :params => doc.to_s, :headers => headers + post api_changeset_upload_path(cs_id), :params => doc.to_s, :headers => headers assert_response :success, "can't upload diff relation: #{@response.body}" version = xml_parse(@response.body).find("//diffResult/relation").first["new_version"].to_i end -- 2.39.5