Move the user preferences controller into the api namespace
authorAndy Allan <git@gravitystorm.co.uk>
Sun, 24 Feb 2019 11:47:26 +0000 (12:47 +0100)
committerAndy Allan <git@gravitystorm.co.uk>
Sun, 24 Feb 2019 11:47:26 +0000 (12:47 +0100)
.rubocop_todo.yml
app/controllers/api/user_preferences_controller.rb [new file with mode: 0644]
app/controllers/user_preferences_controller.rb [deleted file]
config/routes.rb
test/controllers/api/user_preferences_controller_test.rb [new file with mode: 0644]
test/controllers/user_preferences_controller_test.rb [deleted file]

index 35107e9..609e02b 100644 (file)
@@ -15,7 +15,7 @@ Lint/AssignmentInCondition:
     - 'app/controllers/notes_controller.rb'
     - 'app/controllers/traces_controller.rb'
     - 'app/controllers/users_controller.rb'
-    - 'app/controllers/user_preferences_controller.rb'
+    - 'app/controllers/api/user_preferences_controller.rb'
     - 'app/helpers/application_helper.rb'
     - 'app/helpers/browse_helper.rb'
     - 'app/helpers/browse_tags_helper.rb'
diff --git a/app/controllers/api/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb
new file mode 100644 (file)
index 0000000..82f6c6a
--- /dev/null
@@ -0,0 +1,90 @@
+# Update and read user preferences, which are arbitrayr key/val pairs
+module Api
+  class UserPreferencesController < ApplicationController
+    skip_before_action :verify_authenticity_token
+    before_action :authorize
+
+    authorize_resource
+
+    around_action :api_call_handle_error
+
+    ##
+    # return all the preferences as an XML document
+    def read
+      doc = OSM::API.new.get_xml_doc
+
+      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
+    end
+
+    ##
+    # return the value for a single preference
+    def read_one
+      pref = UserPreference.find([current_user.id, params[:preference_key]])
+
+      render :plain => pref.v.to_s
+    end
+
+    # update the entire set of preferences
+    def update
+      old_preferences = current_user.preferences.each_with_object({}) do |preference, preferences|
+        preferences[preference.k] = preference
+      end
+
+      new_preferences = {}
+
+      doc = XML::Parser.string(request.raw_post, :options => XML::Parser::Options::NOERROR).parse
+
+      doc.find("//preferences/preference").each do |pt|
+        if preference = old_preferences.delete(pt["k"])
+          preference.v = pt["v"]
+        elsif new_preferences.include?(pt["k"])
+          raise OSM::APIDuplicatePreferenceError, pt["k"]
+        else
+          preference = current_user.preferences.build(:k => pt["k"], :v => pt["v"])
+        end
+
+        new_preferences[preference.k] = preference
+      end
+
+      old_preferences.each_value(&:delete)
+
+      new_preferences.each_value(&:save!)
+
+      render :plain => ""
+    end
+
+    ##
+    # update the value of a single preference
+    def update_one
+      begin
+        pref = UserPreference.find([current_user.id, params[:preference_key]])
+      rescue ActiveRecord::RecordNotFound
+        pref = UserPreference.new
+        pref.user = current_user
+        pref.k = params[:preference_key]
+      end
+
+      pref.v = request.raw_post.chomp
+      pref.save!
+
+      render :plain => ""
+    end
+
+    ##
+    # delete a single preference
+    def delete_one
+      UserPreference.find([current_user.id, params[:preference_key]]).delete
+
+      render :plain => ""
+    end
+  end
+end
diff --git a/app/controllers/user_preferences_controller.rb b/app/controllers/user_preferences_controller.rb
deleted file mode 100644 (file)
index 915c847..0000000
+++ /dev/null
@@ -1,88 +0,0 @@
-# Update and read user preferences, which are arbitrayr key/val pairs
-class UserPreferencesController < ApplicationController
-  skip_before_action :verify_authenticity_token
-  before_action :authorize
-
-  authorize_resource
-
-  around_action :api_call_handle_error
-
-  ##
-  # return all the preferences as an XML document
-  def read
-    doc = OSM::API.new.get_xml_doc
-
-    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
-  end
-
-  ##
-  # return the value for a single preference
-  def read_one
-    pref = UserPreference.find([current_user.id, params[:preference_key]])
-
-    render :plain => pref.v.to_s
-  end
-
-  # update the entire set of preferences
-  def update
-    old_preferences = current_user.preferences.each_with_object({}) do |preference, preferences|
-      preferences[preference.k] = preference
-    end
-
-    new_preferences = {}
-
-    doc = XML::Parser.string(request.raw_post, :options => XML::Parser::Options::NOERROR).parse
-
-    doc.find("//preferences/preference").each do |pt|
-      if preference = old_preferences.delete(pt["k"])
-        preference.v = pt["v"]
-      elsif new_preferences.include?(pt["k"])
-        raise OSM::APIDuplicatePreferenceError, pt["k"]
-      else
-        preference = current_user.preferences.build(:k => pt["k"], :v => pt["v"])
-      end
-
-      new_preferences[preference.k] = preference
-    end
-
-    old_preferences.each_value(&:delete)
-
-    new_preferences.each_value(&:save!)
-
-    render :plain => ""
-  end
-
-  ##
-  # update the value of a single preference
-  def update_one
-    begin
-      pref = UserPreference.find([current_user.id, params[:preference_key]])
-    rescue ActiveRecord::RecordNotFound
-      pref = UserPreference.new
-      pref.user = current_user
-      pref.k = params[:preference_key]
-    end
-
-    pref.v = request.raw_post.chomp
-    pref.save!
-
-    render :plain => ""
-  end
-
-  ##
-  # delete a single preference
-  def delete_one
-    UserPreference.find([current_user.id, params[:preference_key]]).delete
-
-    render :plain => ""
-  end
-end
index a124901..5815861 100644 (file)
@@ -71,11 +71,11 @@ OpenStreetMap::Application.routes.draw do
     get "user/gpx_files" => "users#api_gpx_files"
     get "users" => "users#api_users", :as => :api_users
 
-    get "user/preferences" => "user_preferences#read"
-    get "user/preferences/:preference_key" => "user_preferences#read_one"
-    put "user/preferences" => "user_preferences#update"
-    put "user/preferences/:preference_key" => "user_preferences#update_one"
-    delete "user/preferences/:preference_key" => "user_preferences#delete_one"
+    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"
 
     post "gpx/create" => "traces#api_create"
     get "gpx/:id" => "traces#api_read", :id => /\d+/
diff --git a/test/controllers/api/user_preferences_controller_test.rb b/test/controllers/api/user_preferences_controller_test.rb
new file mode 100644 (file)
index 0000000..99dad95
--- /dev/null
@@ -0,0 +1,246 @@
+require "test_helper"
+
+module Api
+  class UserPreferencesControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/user/preferences", :method => :get },
+        { :controller => "api/user_preferences", :action => "read" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/user/preferences", :method => :put },
+        { :controller => "api/user_preferences", :action => "update" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/user/preferences/key", :method => :get },
+        { :controller => "api/user_preferences", :action => "read_one", :preference_key => "key" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/user/preferences/key", :method => :put },
+        { :controller => "api/user_preferences", :action => "update_one", :preference_key => "key" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/user/preferences/key", :method => :delete },
+        { :controller => "api/user_preferences", :action => "delete_one", :preference_key => "key" }
+      )
+    end
+
+    ##
+    # test read action
+    def test_read
+      # first try without auth
+      get :read
+      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
+      assert_select "osm" do
+        assert_select "preferences", :count => 1 do
+          assert_select "preference", :count => 0
+        end
+      end
+
+      # authenticate as a user with preferences
+      user = create(:user)
+      user_preference = create(:user_preference, :user => user)
+      user_preference2 = create(:user_preference, :user => user)
+      basic_authorization user.email, "test"
+
+      # try the read again
+      get :read
+      assert_response :success
+      assert_equal "application/xml", @response.content_type
+      assert_select "osm" do
+        assert_select "preferences", :count => 1 do
+          assert_select "preference", :count => 2
+          assert_select "preference[k=\"#{user_preference.k}\"][v=\"#{user_preference.v}\"]", :count => 1
+          assert_select "preference[k=\"#{user_preference2.k}\"][v=\"#{user_preference2.v}\"]", :count => 1
+        end
+      end
+    end
+
+    ##
+    # test read_one action
+    def test_read_one
+      user = create(:user)
+      create(:user_preference, :user => user, :k => "key", :v => "value")
+
+      # try a read without auth
+      get :read_one, :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" }
+      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" }
+      assert_response :not_found
+    end
+
+    ##
+    # test update action
+    def test_update
+      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>"
+      end
+      assert_response :unauthorized, "should be authenticated"
+      assert_equal "value", UserPreference.find([user.id, "key"]).v
+      assert_equal "some_value", UserPreference.find([user.id, "some_key"]).v
+      assert_raises ActiveRecord::RecordNotFound do
+        UserPreference.find([user.id, "new_key"])
+      end
+
+      # authenticate as a user with preferences
+      basic_authorization user.email, "test"
+
+      # 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>"
+      end
+      assert_response :success
+      assert_equal "text/plain", @response.content_type
+      assert_equal "", @response.body
+      assert_equal "new_value", UserPreference.find([user.id, "key"]).v
+      assert_equal "value", UserPreference.find([user.id, "new_key"]).v
+      assert_raises ActiveRecord::RecordNotFound do
+        UserPreference.find([user.id, "some_key"])
+      end
+
+      # 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>"
+      end
+      assert_response :bad_request
+      assert_equal "text/plain", @response.content_type
+      assert_equal "Duplicate preferences with key key", @response.body
+      assert_equal "new_value", UserPreference.find([user.id, "key"]).v
+
+      # try a put with invalid content
+      assert_no_difference "UserPreference.count" do
+        put :update, :body => "nonsense"
+      end
+      assert_response :bad_request
+    end
+
+    ##
+    # test update_one action
+    def test_update_one
+      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"
+      end
+      assert_response :unauthorized, "should be authenticated"
+      assert_raises ActiveRecord::RecordNotFound do
+        UserPreference.find([user.id, "new_key"])
+      end
+
+      # authenticate as a user with preferences
+      basic_authorization user.email, "test"
+
+      # try adding a new preference
+      assert_difference "UserPreference.count", 1 do
+        put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value"
+      end
+      assert_response :success
+      assert_equal "text/plain", @response.content_type
+      assert_equal "", @response.body
+      assert_equal "new_value", UserPreference.find([user.id, "new_key"]).v
+
+      # try changing the value of a preference
+      assert_no_difference "UserPreference.count" do
+        put :update_one, :params => { :preference_key => "new_key" }, :body => "newer_value"
+      end
+      assert_response :success
+      assert_equal "text/plain", @response.content_type
+      assert_equal "", @response.body
+      assert_equal "newer_value", UserPreference.find([user.id, "new_key"]).v
+    end
+
+    ##
+    # test delete_one action
+    def test_delete_one
+      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" }
+      end
+      assert_response :unauthorized, "should be authenticated"
+      assert_equal "value", UserPreference.find([user.id, "key"]).v
+
+      # authenticate as a user with preferences
+      basic_authorization user.email, "test"
+
+      # try the delete again
+      assert_difference "UserPreference.count", -1 do
+        get :delete_one, :params => { :preference_key => "key" }
+      end
+      assert_response :success
+      assert_equal "text/plain", @response.content_type
+      assert_equal "", @response.body
+      assert_raises ActiveRecord::RecordNotFound do
+        UserPreference.find([user.id, "key"])
+      end
+
+      # try the delete again for the same key
+      assert_no_difference "UserPreference.count" do
+        get :delete_one, :params => { :preference_key => "key" }
+      end
+      assert_response :not_found
+      assert_raises ActiveRecord::RecordNotFound do
+        UserPreference.find([user.id, "key"])
+      end
+    end
+
+    # Ensure that a valid access token with correct capabilities can be used to
+    # read preferences
+    def test_read_one_using_token
+      user = create(:user)
+      token = create(:access_token, :user => user, :allow_read_prefs => true)
+      create(:user_preference, :user => user, :k => "key", :v => "value")
+
+      # Hack together an oauth request - an alternative would be to sign the request properly
+      @request.env["oauth.version"] = 1
+      @request.env["oauth.strategies"] = [:token]
+      @request.env["oauth.token"] = token
+
+      get :read_one, :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
+      user = create(:user)
+      token = create(:access_token, :user => user, :allow_read_prefs => false)
+      create(:user_preference, :user => user, :k => "key", :v => "value")
+      @request.env["oauth.version"] = 1
+      @request.env["oauth.strategies"] = [:token]
+      @request.env["oauth.token"] = token
+
+      get :read_one, :params => { :preference_key => "key" }
+      assert_response :forbidden
+    end
+  end
+end
diff --git a/test/controllers/user_preferences_controller_test.rb b/test/controllers/user_preferences_controller_test.rb
deleted file mode 100644 (file)
index 99b40d5..0000000
+++ /dev/null
@@ -1,244 +0,0 @@
-require "test_helper"
-
-class UserPreferencesControllerTest < ActionController::TestCase
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/user/preferences", :method => :get },
-      { :controller => "user_preferences", :action => "read" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/user/preferences", :method => :put },
-      { :controller => "user_preferences", :action => "update" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/user/preferences/key", :method => :get },
-      { :controller => "user_preferences", :action => "read_one", :preference_key => "key" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/user/preferences/key", :method => :put },
-      { :controller => "user_preferences", :action => "update_one", :preference_key => "key" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/user/preferences/key", :method => :delete },
-      { :controller => "user_preferences", :action => "delete_one", :preference_key => "key" }
-    )
-  end
-
-  ##
-  # test read action
-  def test_read
-    # first try without auth
-    get :read
-    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
-    assert_select "osm" do
-      assert_select "preferences", :count => 1 do
-        assert_select "preference", :count => 0
-      end
-    end
-
-    # authenticate as a user with preferences
-    user = create(:user)
-    user_preference = create(:user_preference, :user => user)
-    user_preference2 = create(:user_preference, :user => user)
-    basic_authorization user.email, "test"
-
-    # try the read again
-    get :read
-    assert_response :success
-    assert_equal "application/xml", @response.content_type
-    assert_select "osm" do
-      assert_select "preferences", :count => 1 do
-        assert_select "preference", :count => 2
-        assert_select "preference[k=\"#{user_preference.k}\"][v=\"#{user_preference.v}\"]", :count => 1
-        assert_select "preference[k=\"#{user_preference2.k}\"][v=\"#{user_preference2.v}\"]", :count => 1
-      end
-    end
-  end
-
-  ##
-  # test read_one action
-  def test_read_one
-    user = create(:user)
-    create(:user_preference, :user => user, :k => "key", :v => "value")
-
-    # try a read without auth
-    get :read_one, :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" }
-    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" }
-    assert_response :not_found
-  end
-
-  ##
-  # test update action
-  def test_update
-    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>"
-    end
-    assert_response :unauthorized, "should be authenticated"
-    assert_equal "value", UserPreference.find([user.id, "key"]).v
-    assert_equal "some_value", UserPreference.find([user.id, "some_key"]).v
-    assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find([user.id, "new_key"])
-    end
-
-    # authenticate as a user with preferences
-    basic_authorization user.email, "test"
-
-    # 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>"
-    end
-    assert_response :success
-    assert_equal "text/plain", @response.content_type
-    assert_equal "", @response.body
-    assert_equal "new_value", UserPreference.find([user.id, "key"]).v
-    assert_equal "value", UserPreference.find([user.id, "new_key"]).v
-    assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find([user.id, "some_key"])
-    end
-
-    # 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>"
-    end
-    assert_response :bad_request
-    assert_equal "text/plain", @response.content_type
-    assert_equal "Duplicate preferences with key key", @response.body
-    assert_equal "new_value", UserPreference.find([user.id, "key"]).v
-
-    # try a put with invalid content
-    assert_no_difference "UserPreference.count" do
-      put :update, :body => "nonsense"
-    end
-    assert_response :bad_request
-  end
-
-  ##
-  # test update_one action
-  def test_update_one
-    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"
-    end
-    assert_response :unauthorized, "should be authenticated"
-    assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find([user.id, "new_key"])
-    end
-
-    # authenticate as a user with preferences
-    basic_authorization user.email, "test"
-
-    # try adding a new preference
-    assert_difference "UserPreference.count", 1 do
-      put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value"
-    end
-    assert_response :success
-    assert_equal "text/plain", @response.content_type
-    assert_equal "", @response.body
-    assert_equal "new_value", UserPreference.find([user.id, "new_key"]).v
-
-    # try changing the value of a preference
-    assert_no_difference "UserPreference.count" do
-      put :update_one, :params => { :preference_key => "new_key" }, :body => "newer_value"
-    end
-    assert_response :success
-    assert_equal "text/plain", @response.content_type
-    assert_equal "", @response.body
-    assert_equal "newer_value", UserPreference.find([user.id, "new_key"]).v
-  end
-
-  ##
-  # test delete_one action
-  def test_delete_one
-    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" }
-    end
-    assert_response :unauthorized, "should be authenticated"
-    assert_equal "value", UserPreference.find([user.id, "key"]).v
-
-    # authenticate as a user with preferences
-    basic_authorization user.email, "test"
-
-    # try the delete again
-    assert_difference "UserPreference.count", -1 do
-      get :delete_one, :params => { :preference_key => "key" }
-    end
-    assert_response :success
-    assert_equal "text/plain", @response.content_type
-    assert_equal "", @response.body
-    assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find([user.id, "key"])
-    end
-
-    # try the delete again for the same key
-    assert_no_difference "UserPreference.count" do
-      get :delete_one, :params => { :preference_key => "key" }
-    end
-    assert_response :not_found
-    assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find([user.id, "key"])
-    end
-  end
-
-  # Ensure that a valid access token with correct capabilities can be used to
-  # read preferences
-  def test_read_one_using_token
-    user = create(:user)
-    token = create(:access_token, :user => user, :allow_read_prefs => true)
-    create(:user_preference, :user => user, :k => "key", :v => "value")
-
-    # Hack together an oauth request - an alternative would be to sign the request properly
-    @request.env["oauth.version"] = 1
-    @request.env["oauth.strategies"] = [:token]
-    @request.env["oauth.token"] = token
-
-    get :read_one, :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
-    user = create(:user)
-    token = create(:access_token, :user => user, :allow_read_prefs => false)
-    create(:user_preference, :user => user, :k => "key", :v => "value")
-    @request.env["oauth.version"] = 1
-    @request.env["oauth.strategies"] = [:token]
-    @request.env["oauth.token"] = token
-
-    get :read_one, :params => { :preference_key => "key" }
-    assert_response :forbidden
-  end
-end