Merge remote-tracking branch 'upstream/pull/2434'

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 6d62ece5172f0f5ad9eaccb63dc2fc8c798bcd4a..217fe9713c8745f0c666e261389175e883425834 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -34,7 +34,7 @@ class ApiAbility
         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

diff --git a/app/abilities/api_capability.rb b/app/abilities/api_capability.rb
index 7d8a13364c8fec33c091ebd32516a297b51c6050..64861f1d679cad8b962618909e86c31cfc9d2742 100644 (file)
--- a/app/abilities/api_capability.rb
+++ b/app/abilities/api_capability.rb
@@ -10,8 +10,8 @@ class ApiCapability
       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)

diff --git a/app/controllers/api/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb
index 39e0dff300d83078e3fffcb54b5b4d04d29a74cf..83e28bc4e0047ef4e3a9a880a08406813f654693 100644 (file)
--- a/app/controllers/api/user_preferences_controller.rb
+++ b/app/controllers/api/user_preferences_controller.rb
@@ -9,31 +9,22 @@ module Api
 
     ##
     # return all the preferences as an XML document
-    def read
-      doc = OSM::API.new.get_xml_doc
+    def index
+      @user_preferences = current_user.preferences
 
-      prefs = current_user.preferences
-
-      el1 = XML::Node.new "preferences"
-
-      prefs.each do |pref|
-        el1 << pref.to_xml_node
-      end
-
-      doc.root << el1
-      render :xml => doc.to_s
+      render :formats => [:xml]
     end
 
     ##
     # return the value for a single preference
-    def read_one
+    def show
       pref = UserPreference.find([current_user.id, params[:preference_key]])
 
       render :plain => pref.v.to_s
     end
 
     # update the entire set of preferences
-    def update
+    def update_all
       old_preferences = current_user.preferences.each_with_object({}) do |preference, preferences|
         preferences[preference.k] = preference
       end
@@ -63,7 +54,7 @@ module Api
 
     ##
     # update the value of a single preference
-    def update_one
+    def update
       begin
         pref = UserPreference.find([current_user.id, params[:preference_key]])
       rescue ActiveRecord::RecordNotFound
@@ -80,7 +71,7 @@ module Api
 
     ##
     # delete a single preference
-    def delete_one
+    def destroy
       UserPreference.find([current_user.id, params[:preference_key]]).delete
 
       render :plain => ""

diff --git a/app/models/relation.rb b/app/models/relation.rb
index dc2b71ac4193f292ea6febf9e9df8176cc606647..a73d63183b44ee2b31dd7348f9d4e44e5e956c28 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -121,31 +121,6 @@ class Relation < ActiveRecord::Base
     relation
   end
 
-  def to_xml
-    doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node
-    doc
-  end
-
-  def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
-    el = XML::Node.new "relation"
-    el["id"] = id.to_s
-
-    add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
-
-    relation_members.each do |member|
-      member_el = XML::Node.new "member"
-      member_el["type"] = member.member_type.downcase
-      member_el["ref"] = member.member_id.to_s
-      member_el["role"] = member.member_role
-      el << member_el
-    end
-
-    add_tags_to_xml_node(el, relation_tags)
-
-    el
-  end
-
   # FIXME: is this really needed?
   def members
     @members ||= relation_members.map do |member|

diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb
index 583ced3c56d19ed15de8dd2a4ff6922b37312de8..37bab38b0ccb7c6f5ff481e0efc234a01aa18d2d 100644 (file)
--- a/app/models/user_preference.rb
+++ b/app/models/user_preference.rb
@@ -18,13 +18,4 @@ class UserPreference < ActiveRecord::Base
 
   validates :user, :presence => true, :associated => true
   validates :k, :v, :length => 1..255, :characters => true
-
-  # Turn this Node in to an XML Node without the <osm> wrapper.
-  def to_xml_node
-    el1 = XML::Node.new "preference"
-    el1["k"] = k
-    el1["v"] = v
-
-    el1
-  end
 end

diff --git a/app/models/way.rb b/app/models/way.rb
index b3466213c34de289b325718bcbdbed8c37a4a744..3284c47f37312aed1215b64298e108ba86720c6b 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -106,44 +106,6 @@ class Way < ActiveRecord::Base
     way
   end
 
-  # Find a way given it's ID, and in a single SQL call also grab its nodes and tags
-  def to_xml
-    doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node
-    doc
-  end
-
-  def to_xml_node(visible_nodes = nil, changeset_cache = {}, user_display_name_cache = {})
-    el = XML::Node.new "way"
-    el["id"] = id.to_s
-
-    add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
-
-    # make sure nodes are output in sequence_id order
-    ordered_nodes = []
-    way_nodes.each do |nd|
-      if visible_nodes
-        # if there is a list of visible nodes then use that to weed out deleted nodes
-        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if visible_nodes[nd.node_id]
-      else
-        # otherwise, manually go to the db to check things
-        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node&.visible?
-      end
-    end
-
-    ordered_nodes.each do |nd_id|
-      next unless nd_id && nd_id != "0"
-
-      node_el = XML::Node.new "nd"
-      node_el["ref"] = nd_id
-      el << node_el
-    end
-
-    add_tags_to_xml_node(el, way_tags)
-
-    el
-  end
-
   def nds
     @nds ||= way_nodes.collect(&:node_id)
   end

diff --git a/app/views/api/user_preferences/_user_preference.xml.builder b/app/views/api/user_preferences/_user_preference.xml.builder
new file mode 100644 (file)
index 0000000..ae830be
--- /dev/null
+++ b/app/views/api/user_preferences/_user_preference.xml.builder
@@ -0,0 +1,6 @@
+attrs = {
+  "k" => user_preference.k,
+  "v" => user_preference.v
+}
+
+xml.preference(attrs)

diff --git a/app/views/api/user_preferences/index.xml.builder b/app/views/api/user_preferences/index.xml.builder
new file mode 100644 (file)
index 0000000..0a852b2
--- /dev/null
+++ b/app/views/api/user_preferences/index.xml.builder
@@ -0,0 +1,7 @@
+xml.instruct!
+
+xml.osm(OSM::API.new.xml_root_attributes) do |osm|
+  osm.preferences do |preferences|
+    preferences << (render(@user_preferences) || "")
+  end
+end

diff --git a/config/routes.rb b/config/routes.rb
index 58c05da171e3c2628179d9ab543d7b3d42553306..d936072d78e7cee4b6d2e12841671c8c14bff41a 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -67,11 +67,11 @@ OpenStreetMap::Application.routes.draw do
     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+/

diff --git a/db/migrate/20191120140058_remove_nearby_from_users.rb b/db/migrate/20191120140058_remove_nearby_from_users.rb
new file mode 100644 (file)
index 0000000..7d9383d
--- /dev/null
+++ b/db/migrate/20191120140058_remove_nearby_from_users.rb
@@ -0,0 +1,6 @@
+class RemoveNearbyFromUsers < ActiveRecord::Migration[5.2]
+  def change
+    # We've already ignored this column in the model, so it is safe to remove
+    safety_assured { remove_column :users, :nearby, :integer, :default => 50 }
+  end
+end

diff --git a/db/structure.sql b/db/structure.sql
index 6370eb95a0904a8b63b978b7d9abb1f5972822e6..78e58d4be5ba0b3394b42232cf3d62ebdb2d5ece 100644 (file)
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -1394,7 +1394,6 @@ CREATE TABLE public.users (
     home_lat double precision,
     home_lon double precision,
     home_zoom smallint DEFAULT 3,
-    nearby integer DEFAULT 50,
     pass_salt character varying,
     email_valid boolean DEFAULT false NOT NULL,
     new_email character varying,
@@ -3137,6 +3136,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20190623093642'),
 ('20190702193519'),
 ('20190716173946'),
+('20191120140058'),
 ('21'),
 ('22'),
 ('23'),

diff --git a/test/abilities/api_capability_test.rb b/test/abilities/api_capability_test.rb
index 8d0e682f62b4a3ac91b5b875e43c8d311f82c483..8a98f29e0e6274c6c871e57bbc9b697a1aa6666a 100644 (file)
--- a/test/abilities/api_capability_test.rb
+++ b/test/abilities/api_capability_test.rb
@@ -100,33 +100,33 @@ class UserApiCapabilityTest < ApiCapabilityTest
   test "user preferences" do
     # a user with no tokens
     capability = ApiCapability.new nil
-    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
+    [:index, :show, :update_all, :update, :destroy].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
     # A user with empty tokens
     capability = ApiCapability.new tokens
 
-    [:read, :read_one, :update, :update_one, :delete_one].each do |act|
+    [:index, :show, :update_all, :update, :destroy].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
     capability = ApiCapability.new tokens(:allow_read_prefs)
 
-    [:update, :update_one, :delete_one].each do |act|
+    [:update_all, :update, :destroy].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
-    [:read, :read_one].each do |act|
+    [:index, :show].each do |act|
       assert capability.can? act, UserPreference
     end
 
     capability = ApiCapability.new tokens(:allow_write_prefs)
-    [:read, :read_one].each do |act|
+    [:index, :show].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
-    [:update, :update_one, :delete_one].each do |act|
+    [:update_all, :update, :destroy].each do |act|
       assert capability.can? act, UserPreference
     end
   end

diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb
index c3c9c5058ea963f53f373f74ad2fac1757019163..a4583a9289e4c95ec90fc476afc163c94bfed890 100644 (file)
--- a/test/controllers/api/changesets_controller_test.rb
+++ b/test/controllers/api/changesets_controller_test.rb
@@ -458,9 +458,9 @@ 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 << 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
@@ -590,8 +590,8 @@ 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 << 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
@@ -633,8 +633,8 @@ 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 << 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
@@ -1175,7 +1175,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
@@ -1487,7 +1487,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

diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 4d2969026aa3621dea4936c2abf674425880553d..1b54e4863c52fe6024bd6a0eb1079012b40e1c24 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -525,28 +525,28 @@ module Api
       assert_response :forbidden
 
       # try to delete with an invalid (closed) changeset
-      xml = update_changeset(relation.to_xml,
+      xml = update_changeset(xml_for_relation(relation),
                              private_user_closed_changeset.id)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       assert_response :forbidden
 
       # try to delete with an invalid (non-existent) changeset
-      xml = update_changeset(relation.to_xml, 0)
+      xml = update_changeset(xml_for_relation(relation), 0)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       assert_response :forbidden
 
       # this won't work because the relation is in-use by another relation
-      xml = used_relation.to_xml
+      xml = xml_for_relation(used_relation)
       delete :delete, :params => { :id => used_relation.id }, :body => xml.to_s
       assert_response :forbidden
 
       # this should work when we provide the appropriate payload...
-      xml = relation.to_xml
+      xml = xml_for_relation(relation)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       assert_response :forbidden
 
       # this won't work since the relation is already deleted
-      xml = deleted_relation.to_xml
+      xml = xml_for_relation(deleted_relation)
       delete :delete, :params => { :id => deleted_relation.id }, :body => xml.to_s
       assert_response :forbidden
 
@@ -568,36 +568,36 @@ module Api
       assert_match(/Changeset id is missing/, @response.body)
 
       # try to delete with an invalid (closed) changeset
-      xml = update_changeset(relation.to_xml,
+      xml = update_changeset(xml_for_relation(relation),
                              closed_changeset.id)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       assert_response :conflict
 
       # try to delete with an invalid (non-existent) changeset
-      xml = update_changeset(relation.to_xml, 0)
+      xml = update_changeset(xml_for_relation(relation), 0)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       assert_response :conflict
 
       # this won't work because the relation is in a changeset owned by someone else
-      xml = update_changeset(relation.to_xml, create(:changeset).id)
+      xml = update_changeset(xml_for_relation(relation), create(:changeset).id)
       delete :delete, :params => { :id => relation.id }, :body => xml.to_s
       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
-      xml = update_changeset(relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(relation), changeset.id)
       delete :delete, :params => { :id => create(:relation).id }, :body => xml.to_s
       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
-      xml = update_changeset(used_relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(used_relation), changeset.id)
       delete :delete, :params => { :id => used_relation.id }, :body => xml.to_s
       assert_response :precondition_failed,
                       "shouldn't be able to delete a relation used in a relation (#{@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...
-      xml = update_changeset(multi_tag_relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(multi_tag_relation), changeset.id)
       delete :delete, :params => { :id => multi_tag_relation.id }, :body => xml.to_s
       assert_response :success
 
@@ -607,18 +607,18 @@ module Api
              "delete request should return a new version number for relation"
 
       # this won't work since the relation is already deleted
-      xml = update_changeset(deleted_relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(deleted_relation), changeset.id)
       delete :delete, :params => { :id => deleted_relation.id }, :body => xml.to_s
       assert_response :gone
 
       # Public visible relation needs to be deleted
-      xml = update_changeset(super_relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(super_relation), changeset.id)
       delete :delete, :params => { :id => super_relation.id }, :body => xml.to_s
       assert_response :success
 
       # this works now because the relation which was using this one
       # has been deleted.
-      xml = update_changeset(used_relation.to_xml, changeset.id)
+      xml = update_changeset(xml_for_relation(used_relation), changeset.id)
       delete :delete, :params => { :id => used_relation.id }, :body => xml.to_s
       assert_response :success,
                       "should be able to delete a relation used in an old relation (#{@response.body})"
@@ -643,7 +643,7 @@ module Api
       # indirectly via the way), so the bbox should be [3,3,5,5].
       check_changeset_modify(BoundingBox.new(3, 3, 5, 5)) do |changeset_id|
         # add a tag to an existing relation
-        relation_xml = relation.to_xml
+        relation_xml = xml_for_relation(relation)
         relation_element = relation_xml.find("//osm/relation").first
         new_tag = XML::Node.new("tag")
         new_tag["k"] = "some_new_tag"
@@ -675,7 +675,7 @@ module Api
       [node1, node2, way1, way2].each do |element|
         bbox = element.bbox.to_unscaled
         check_changeset_modify(bbox) do |changeset_id|
-          relation_xml = Relation.find(relation.id).to_xml
+          relation_xml = xml_for_relation(Relation.find(relation.id))
           relation_element = relation_xml.find("//osm/relation").first
           new_member = XML::Node.new("member")
           new_member["ref"] = element.id.to_s
@@ -710,7 +710,7 @@ module Api
 
       check_changeset_modify(BoundingBox.new(5, 5, 5, 5)) do |changeset_id|
         # remove node 5 (5,5) from an existing relation
-        relation_xml = relation.to_xml
+        relation_xml = xml_for_relation(relation)
         relation_xml
           .find("//osm/relation/member[@type='node'][@ref='#{node2.id}']")
           .first.remove!
@@ -879,7 +879,7 @@ OSM
       create(:relation_member, :relation => relation, :member => node2)
 
       check_changeset_modify(BoundingBox.new(3, 3, 5, 5)) do |changeset_id|
-        relation_xml = relation.to_xml
+        relation_xml = xml_for_relation(relation)
         relation_xml
           .find("//osm/relation/member")
           .each(&:remove!)

diff --git a/test/controllers/api/user_preferences_controller_test.rb b/test/controllers/api/user_preferences_controller_test.rb
index 99dad959732343dfff6e467b4e5059bd9d68806f..ac377848c9332d5afe66df800219059c8041db8e 100644 (file)
--- a/test/controllers/api/user_preferences_controller_test.rb
+++ b/test/controllers/api/user_preferences_controller_test.rb
@@ -7,38 +7,38 @@ module Api
     def test_routes
       assert_routing(
         { :path => "/api/0.6/user/preferences", :method => :get },
-        { :controller => "api/user_preferences", :action => "read" }
+        { :controller => "api/user_preferences", :action => "index" }
       )
       assert_routing(
         { :path => "/api/0.6/user/preferences", :method => :put },
-        { :controller => "api/user_preferences", :action => "update" }
+        { :controller => "api/user_preferences", :action => "update_all" }
       )
       assert_routing(
         { :path => "/api/0.6/user/preferences/key", :method => :get },
-        { :controller => "api/user_preferences", :action => "read_one", :preference_key => "key" }
+        { :controller => "api/user_preferences", :action => "show", :preference_key => "key" }
       )
       assert_routing(
         { :path => "/api/0.6/user/preferences/key", :method => :put },
-        { :controller => "api/user_preferences", :action => "update_one", :preference_key => "key" }
+        { :controller => "api/user_preferences", :action => "update", :preference_key => "key" }
       )
       assert_routing(
         { :path => "/api/0.6/user/preferences/key", :method => :delete },
-        { :controller => "api/user_preferences", :action => "delete_one", :preference_key => "key" }
+        { :controller => "api/user_preferences", :action => "destroy", :preference_key => "key" }
       )
     end
 
     ##
-    # test read action
-    def test_read
+    # test showing all preferences
+    def test_index
       # first try without auth
-      get :read
+      get :index
       assert_response :unauthorized, "should be authenticated"
 
       # authenticate as a user with no preferences
       basic_authorization create(:user).email, "test"
 
       # try the read again
-      get :read
+      get :index
       assert_select "osm" do
         assert_select "preferences", :count => 1 do
           assert_select "preference", :count => 0
@@ -52,7 +52,7 @@ module Api
       basic_authorization user.email, "test"
 
       # try the read again
-      get :read
+      get :index
       assert_response :success
       assert_equal "application/xml", @response.content_type
       assert_select "osm" do
@@ -65,39 +65,39 @@ module Api
     end
 
     ##
-    # test read_one action
-    def test_read_one
+    # test showing one preference
+    def test_show
       user = create(:user)
       create(:user_preference, :user => user, :k => "key", :v => "value")
 
       # try a read without auth
-      get :read_one, :params => { :preference_key => "key" }
+      get :show, :params => { :preference_key => "key" }
       assert_response :unauthorized, "should be authenticated"
 
       # authenticate as a user with preferences
       basic_authorization user.email, "test"
 
       # try the read again
-      get :read_one, :params => { :preference_key => "key" }
+      get :show, :params => { :preference_key => "key" }
       assert_response :success
       assert_equal "text/plain", @response.content_type
       assert_equal "value", @response.body
 
       # try the read again for a non-existent key
-      get :read_one, :params => { :preference_key => "unknown_key" }
+      get :show, :params => { :preference_key => "unknown_key" }
       assert_response :not_found
     end
 
     ##
-    # test update action
-    def test_update
+    # test bulk update action
+    def test_update_all
       user = create(:user)
       create(:user_preference, :user => user, :k => "key", :v => "value")
       create(:user_preference, :user => user, :k => "some_key", :v => "some_value")
 
       # try a put without auth
       assert_no_difference "UserPreference.count" do
-        put :update, :body => "<osm><preferences><preference k='key' v='new_value'/><preference k='new_key' v='value'/></preferences></osm>"
+        put :update_all, :body => "<osm><preferences><preference k='key' v='new_value'/><preference k='new_key' v='value'/></preferences></osm>"
       end
       assert_response :unauthorized, "should be authenticated"
       assert_equal "value", UserPreference.find([user.id, "key"]).v
@@ -111,7 +111,7 @@ module Api
 
       # try the put again
       assert_no_difference "UserPreference.count" do
-        put :update, :body => "<osm><preferences><preference k='key' v='new_value'/><preference k='new_key' v='value'/></preferences></osm>"
+        put :update_all, :body => "<osm><preferences><preference k='key' v='new_value'/><preference k='new_key' v='value'/></preferences></osm>"
       end
       assert_response :success
       assert_equal "text/plain", @response.content_type
@@ -124,7 +124,7 @@ module Api
 
       # try a put with duplicate keys
       assert_no_difference "UserPreference.count" do
-        put :update, :body => "<osm><preferences><preference k='key' v='value'/><preference k='key' v='newer_value'/></preferences></osm>"
+        put :update_all, :body => "<osm><preferences><preference k='key' v='value'/><preference k='key' v='newer_value'/></preferences></osm>"
       end
       assert_response :bad_request
       assert_equal "text/plain", @response.content_type
@@ -133,20 +133,20 @@ module Api
 
       # try a put with invalid content
       assert_no_difference "UserPreference.count" do
-        put :update, :body => "nonsense"
+        put :update_all, :body => "nonsense"
       end
       assert_response :bad_request
     end
 
     ##
-    # test update_one action
-    def test_update_one
+    # test update action
+    def test_update
       user = create(:user)
       create(:user_preference, :user => user)
 
       # try a put without auth
       assert_no_difference "UserPreference.count" do
-        put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value"
+        put :update, :params => { :preference_key => "new_key" }, :body => "new_value"
       end
       assert_response :unauthorized, "should be authenticated"
       assert_raises ActiveRecord::RecordNotFound do
@@ -158,7 +158,7 @@ module Api
 
       # try adding a new preference
       assert_difference "UserPreference.count", 1 do
-        put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value"
+        put :update, :params => { :preference_key => "new_key" }, :body => "new_value"
       end
       assert_response :success
       assert_equal "text/plain", @response.content_type
@@ -167,7 +167,7 @@ module Api
 
       # try changing the value of a preference
       assert_no_difference "UserPreference.count" do
-        put :update_one, :params => { :preference_key => "new_key" }, :body => "newer_value"
+        put :update, :params => { :preference_key => "new_key" }, :body => "newer_value"
       end
       assert_response :success
       assert_equal "text/plain", @response.content_type
@@ -176,14 +176,14 @@ module Api
     end
 
     ##
-    # test delete_one action
-    def test_delete_one
+    # test destroy action
+    def test_destroy
       user = create(:user)
       create(:user_preference, :user => user, :k => "key", :v => "value")
 
       # try a delete without auth
       assert_no_difference "UserPreference.count" do
-        delete :delete_one, :params => { :preference_key => "key" }
+        delete :destroy, :params => { :preference_key => "key" }
       end
       assert_response :unauthorized, "should be authenticated"
       assert_equal "value", UserPreference.find([user.id, "key"]).v
@@ -193,7 +193,7 @@ module Api
 
       # try the delete again
       assert_difference "UserPreference.count", -1 do
-        get :delete_one, :params => { :preference_key => "key" }
+        get :destroy, :params => { :preference_key => "key" }
       end
       assert_response :success
       assert_equal "text/plain", @response.content_type
@@ -204,7 +204,7 @@ module Api
 
       # try the delete again for the same key
       assert_no_difference "UserPreference.count" do
-        get :delete_one, :params => { :preference_key => "key" }
+        get :destroy, :params => { :preference_key => "key" }
       end
       assert_response :not_found
       assert_raises ActiveRecord::RecordNotFound do
@@ -214,7 +214,7 @@ module Api
 
     # Ensure that a valid access token with correct capabilities can be used to
     # read preferences
-    def test_read_one_using_token
+    def test_show_using_token
       user = create(:user)
       token = create(:access_token, :user => user, :allow_read_prefs => true)
       create(:user_preference, :user => user, :k => "key", :v => "value")
@@ -224,14 +224,14 @@ module Api
       @request.env["oauth.strategies"] = [:token]
       @request.env["oauth.token"] = token
 
-      get :read_one, :params => { :preference_key => "key" }
+      get :show, :params => { :preference_key => "key" }
       assert_response :success
     end
 
     # Ensure that a valid access token with incorrect capabilities can't be used
     # to read preferences even, though the owner of that token could read them
     # by other methods.
-    def test_read_one_using_token_fail
+    def test_show_using_token_fail
       user = create(:user)
       token = create(:access_token, :user => user, :allow_read_prefs => false)
       create(:user_preference, :user => user, :k => "key", :v => "value")
@@ -239,7 +239,7 @@ module Api
       @request.env["oauth.strategies"] = [:token]
       @request.env["oauth.token"] = token
 
-      get :read_one, :params => { :preference_key => "key" }
+      get :show, :params => { :preference_key => "key" }
       assert_response :forbidden
     end
   end

diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb
index c390e8bc29d32e61290fdf03ed6b8756c9076fe2..433b43f27cbea7d63e10306f03573ab54dfe31cf 100644 (file)
--- a/test/controllers/api/ways_controller_test.rb
+++ b/test/controllers/api/ways_controller_test.rb
@@ -290,17 +290,17 @@ module Api
       assert_response :forbidden
 
       # try to delete with an invalid (closed) changeset
-      xml = update_changeset(private_way.to_xml, private_closed_changeset.id)
+      xml = update_changeset(xml_for_way(private_way), private_closed_changeset.id)
       delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
       assert_response :forbidden
 
       # try to delete with an invalid (non-existent) changeset
-      xml = update_changeset(private_way.to_xml, 0)
+      xml = update_changeset(xml_for_way(private_way), 0)
       delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
       assert_response :forbidden
 
       # Now try with a valid changeset
-      xml = private_way.to_xml
+      xml = xml_for_way(private_way)
       delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
       assert_response :forbidden
 
@@ -311,12 +311,12 @@ module Api
       #   "delete request should return a new version number for way"
 
       # this won't work since the way is already deleted
-      xml = private_deleted_way.to_xml
+      xml = xml_for_way(private_deleted_way)
       delete :delete, :params => { :id => private_deleted_way.id }, :body => xml.to_s
       assert_response :forbidden
 
       # this shouldn't work as the way is used in a relation
-      xml = private_used_way.to_xml
+      xml = xml_for_way(private_used_way)
       delete :delete, :params => { :id => private_used_way.id }, :body => xml.to_s
       assert_response :forbidden,
                       "shouldn't be able to delete a way used in a relation (#{@response.body}), when done by a private user"
@@ -339,17 +339,17 @@ module Api
       assert_response :bad_request
 
       # try to delete with an invalid (closed) changeset
-      xml = update_changeset(way.to_xml, closed_changeset.id)
+      xml = update_changeset(xml_for_way(way), closed_changeset.id)
       delete :delete, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict
 
       # try to delete with an invalid (non-existent) changeset
-      xml = update_changeset(way.to_xml, 0)
+      xml = update_changeset(xml_for_way(way), 0)
       delete :delete, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict
 
       # Now try with a valid changeset
-      xml = way.to_xml
+      xml = xml_for_way(way)
       delete :delete, :params => { :id => way.id }, :body => xml.to_s
       assert_response :success
 
@@ -360,12 +360,12 @@ module Api
              "delete request should return a new version number for way"
 
       # this won't work since the way is already deleted
-      xml = deleted_way.to_xml
+      xml = xml_for_way(deleted_way)
       delete :delete, :params => { :id => deleted_way.id }, :body => xml.to_s
       assert_response :gone
 
       # this shouldn't work as the way is used in a relation
-      xml = used_way.to_xml
+      xml = xml_for_way(used_way)
       delete :delete, :params => { :id => used_way.id }, :body => xml.to_s
       assert_response :precondition_failed,
                       "shouldn't be able to delete a way used in a relation (#{@response.body})"
@@ -390,7 +390,7 @@ module Api
 
       ## First test with no user credentials
       # try and update a way without authorisation
-      xml = way.to_xml
+      xml = xml_for_way(way)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :unauthorized
 
@@ -402,33 +402,33 @@ module Api
       ## trying to break changesets
 
       # try and update in someone else's changeset
-      xml = update_changeset(private_way.to_xml,
+      xml = update_changeset(xml_for_way(private_way),
                              create(:changeset).id)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data "update with other user's changeset should be forbidden when date isn't public"
 
       # try and update in a closed changeset
-      xml = update_changeset(private_way.to_xml,
+      xml = update_changeset(xml_for_way(private_way),
                              create(:changeset, :closed, :user => private_user).id)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
 
       # try and update in a non-existant changeset
-      xml = update_changeset(private_way.to_xml, 0)
+      xml = update_changeset(xml_for_way(private_way), 0)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data("update with changeset=0 should be forbidden, when data isn't public")
 
       ## try and submit invalid updates
-      xml = xml_replace_node(private_way.to_xml, node.id, 9999)
+      xml = xml_replace_node(xml_for_way(private_way), node.id, 9999)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data "way with non-existent node should be forbidden, when data isn't public"
 
-      xml = xml_replace_node(private_way.to_xml, node.id, create(:node, :deleted).id)
+      xml = xml_replace_node(xml_for_way(private_way), node.id, create(:node, :deleted).id)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data "way with deleted node should be forbidden, when data isn't public"
 
       ## finally, produce a good request which will still not work
-      xml = private_way.to_xml
+      xml = xml_for_way(private_way)
       put :update, :params => { :id => private_way.id }, :body => xml.to_s
       assert_require_public_data "should have failed with a forbidden when data isn't public"
 
@@ -440,28 +440,28 @@ module Api
       ## trying to break changesets
 
       # try and update in someone else's changeset
-      xml = update_changeset(way.to_xml,
+      xml = update_changeset(xml_for_way(way),
                              create(:changeset).id)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict, "update with other user's changeset should be rejected"
 
       # try and update in a closed changeset
-      xml = update_changeset(way.to_xml,
+      xml = update_changeset(xml_for_way(way),
                              create(:changeset, :closed, :user => user).id)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict, "update with closed changeset should be rejected"
 
       # try and update in a non-existant changeset
-      xml = update_changeset(way.to_xml, 0)
+      xml = update_changeset(xml_for_way(way), 0)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict, "update with changeset=0 should be rejected"
 
       ## try and submit invalid updates
-      xml = xml_replace_node(way.to_xml, node.id, 9999)
+      xml = xml_replace_node(xml_for_way(way), node.id, 9999)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :precondition_failed, "way with non-existent node should be rejected"
 
-      xml = xml_replace_node(way.to_xml, node.id, create(:node, :deleted).id)
+      xml = xml_replace_node(xml_for_way(way), node.id, create(:node, :deleted).id)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :precondition_failed, "way with deleted node should be rejected"
 
@@ -469,26 +469,26 @@ module Api
       current_way_version = way.version
 
       # try and submit a version behind
-      xml = xml_attr_rewrite(way.to_xml,
+      xml = xml_attr_rewrite(xml_for_way(way),
                              "version", current_way_version - 1)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict, "should have failed on old version number"
 
       # try and submit a version ahead
-      xml = xml_attr_rewrite(way.to_xml,
+      xml = xml_attr_rewrite(xml_for_way(way),
                              "version", current_way_version + 1)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict, "should have failed on skipped version number"
 
       # try and submit total crap in the version field
-      xml = xml_attr_rewrite(way.to_xml,
+      xml = xml_attr_rewrite(xml_for_way(way),
                              "version", "p1r4t3s!")
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :conflict,
                       "should not be able to put 'p1r4at3s!' in the version field"
 
       ## try an update with the wrong ID
-      xml = create(:way).to_xml
+      xml = xml_for_way(create(:way))
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :bad_request,
                       "should not be able to update a way with a different ID from the XML"
@@ -500,7 +500,7 @@ module Api
                       "should not be able to update a way with non-OSM XML doc."
 
       ## finally, produce a good request which should work
-      xml = way.to_xml
+      xml = xml_for_way(way)
       put :update, :params => { :id => way.id }, :body => xml.to_s
       assert_response :success, "a valid update request failed"
     end
@@ -527,7 +527,7 @@ module Api
       tag_xml["v"] = "yes"
 
       # add the tag into the existing xml
-      way_xml = private_way.to_xml
+      way_xml = xml_for_way(private_way)
       way_xml.find("//osm/way").first << tag_xml
 
       # try and upload it
@@ -545,7 +545,7 @@ module Api
       tag_xml["v"] = "yes"
 
       # add the tag into the existing xml
-      way_xml = way.to_xml
+      way_xml = xml_for_way(way)
       way_xml.find("//osm/way").first << tag_xml
 
       # try and upload it
@@ -575,7 +575,7 @@ module Api
       tag_xml["v"] = private_existing_tag.v
 
       # add the tag into the existing xml
-      way_xml = private_way.to_xml
+      way_xml = xml_for_way(private_way)
       way_xml.find("//osm/way").first << tag_xml
 
       # try and upload it
@@ -593,7 +593,7 @@ module Api
       tag_xml["v"] = existing_tag.v
 
       # add the tag into the existing xml
-      way_xml = way.to_xml
+      way_xml = xml_for_way(way)
       way_xml.find("//osm/way").first << tag_xml
 
       # try and upload it
@@ -621,7 +621,7 @@ module Api
       tag_xml["v"] = "foobar"
 
       # add the tag into the existing xml
-      way_xml = private_way.to_xml
+      way_xml = xml_for_way(private_way)
 
       # add two copies of the tag
       way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml
@@ -641,7 +641,7 @@ module Api
       tag_xml["v"] = "foobar"
 
       # add the tag into the existing xml
-      way_xml = way.to_xml
+      way_xml = xml_for_way(way)
 
       # add two copies of the tag
       way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml

diff --git a/test/test_helper.rb b/test/test_helper.rb
index e4db27d3132fd9233e33806e5b51ea0774bdc4ae..05385b4f9c8678b85b207e352fe047e52bb9956c 100644 (file)
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -193,6 +193,62 @@ module ActiveSupport
       el
     end
 
+    def xml_for_way(way)
+      doc = OSM::API.new.get_xml_doc
+      doc.root << xml_node_for_way(way)
+      doc
+    end
+
+    def xml_node_for_way(way)
+      el = XML::Node.new "way"
+      el["id"] = way.id.to_s
+
+      OMHelper.add_metadata_to_xml_node(el, way, {}, {})
+
+      # make sure nodes are output in sequence_id order
+      ordered_nodes = []
+      way.way_nodes.each do |nd|
+        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node&.visible?
+      end
+
+      ordered_nodes.each do |nd_id|
+        next unless nd_id && nd_id != "0"
+
+        node_el = XML::Node.new "nd"
+        node_el["ref"] = nd_id
+        el << node_el
+      end
+
+      OMHelper.add_tags_to_xml_node(el, way.way_tags)
+
+      el
+    end
+
+    def xml_for_relation(relation)
+      doc = OSM::API.new.get_xml_doc
+      doc.root << xml_node_for_relation(relation)
+      doc
+    end
+
+    def xml_node_for_relation(relation)
+      el = XML::Node.new "relation"
+      el["id"] = relation.id.to_s
+
+      OMHelper.add_metadata_to_xml_node(el, relation, {}, {})
+
+      relation.relation_members.each do |member|
+        member_el = XML::Node.new "member"
+        member_el["type"] = member.member_type.downcase
+        member_el["ref"] = member.member_id.to_s
+        member_el["role"] = member.member_role
+        el << member_el
+      end
+
+      OMHelper.add_tags_to_xml_node(el, relation.relation_tags)
+
+      el
+    end
+
     class OMHelper
       extend ObjectMetadata
     end