Add /api/0.6/users to fetch multiple users

Fixes #1921

.rubocop_todo.yml

@@ -54,7 +54,7 @@ Metrics/AbcSize:
 # Offense count: 41
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 257
+  Max: 258
 
 # Offense count: 11
 # Configuration parameters: CountBlocks.

app/controllers/user_controller.rb

@@ -1,21 +1,21 @@
 class UserController < ApplicationController
   layout "site", :except => [:api_details]
 
-  skip_before_action :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files, :auth_success]
+  skip_before_action :verify_authenticity_token, :only => [:api_read, :api_users, :api_details, :api_gpx_files, :auth_success]
   before_action :disable_terms_redirect, :only => [:terms, :save, :logout, :api_details]
   before_action :authorize, :only => [:api_details, :api_gpx_files]
-  before_action :authorize_web, :except => [:api_read, :api_details, :api_gpx_files]
+  before_action :authorize_web, :except => [:api_read, :api_users, :api_details, :api_gpx_files]
-  before_action :set_locale, :except => [:api_read, :api_details, :api_gpx_files]
+  before_action :set_locale, :except => [:api_read, :api_users, :api_details, :api_gpx_files]
   before_action :require_user, :only => [:account, :go_public, :make_friend, :remove_friend]
   before_action :require_self, :only => [:account]
-  before_action :check_database_readable, :except => [:login, :api_read, :api_details, :api_gpx_files]
+  before_action :check_database_readable, :except => [:login, :api_read, :api_users, :api_details, :api_gpx_files]
   before_action :check_database_writable, :only => [:new, :account, :confirm, :confirm_email, :lost_password, :reset_password, :go_public, :make_friend, :remove_friend]
-  before_action :check_api_readable, :only => [:api_read, :api_details, :api_gpx_files]
+  before_action :check_api_readable, :only => [:api_read, :api_users, :api_details, :api_gpx_files]
   before_action :require_allow_read_prefs, :only => [:api_details]
   before_action :require_allow_read_gpx, :only => [:api_gpx_files]
   before_action :require_cookies, :only => [:new, :login, :confirm]
   before_action :require_administrator, :only => [:set_status, :delete, :list]
-  around_action :api_call_handle_error, :only => [:api_read, :api_details, :api_gpx_files]
+  around_action :api_call_handle_error, :only => [:api_read, :api_users, :api_details, :api_gpx_files]
   before_action :lookup_user_by_id, :only => [:api_read]
   before_action :lookup_user_by_name, :only => [:set_status, :delete]
   before_action :allow_thirdparty_images, :only => [:view, :account]
@@ -389,6 +389,18 @@ class UserController < ApplicationController
     render :action => :api_read, :content_type => "text/xml"
   end
 
+  def api_users
+    raise OSM::APIBadUserInput, "The parameter users is required, and must be of the form users=id[,id[,id...]]" unless params["users"]
+
+    ids = params["users"].split(",").collect(&:to_i)
+
+    raise OSM::APIBadUserInput, "No users were given to search for" if ids.empty?
+
+    @users = User.visible.find(ids)
+
+    render :action => :api_users, :content_type => "text/xml"
+  end
+
   def api_gpx_files
     doc = OSM::API.new.get_xml_doc
     current_user.traces.reload.each do |trace|

app/views/user/_api_user.builder

@@ -0,0 +1,44 @@
+xml.tag! "user", :id => api_user.id,
+                 :display_name => api_user.display_name,
+                 :account_created => api_user.creation_time.xmlschema do
+  xml.tag! "description", api_user.description if api_user.description
+  if current_user && current_user == api_user
+    xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present?,
+                                  :pd => api_user.consider_pd
+  else
+    xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present?
+  end
+  xml.tag! "img", :href => user_image_url(api_user) if api_user.image.file? || api_user.image_use_gravatar
+  xml.tag! "roles" do
+    api_user.roles.each do |role|
+      xml.tag! role.role
+    end
+  end
+  xml.tag! "changesets", :count => api_user.changesets.size
+  xml.tag! "traces", :count => api_user.traces.size
+  xml.tag! "blocks" do
+    xml.tag! "received", :count => api_user.blocks.size,
+                         :active => api_user.blocks.active.size
+    if api_user.moderator?
+      xml.tag! "issued", :count => api_user.blocks_created.size,
+                         :active => api_user.blocks_created.active.size
+    end
+  end
+  if current_user && current_user == api_user
+    if api_user.home_lat && api_user.home_lon
+      xml.tag! "home", :lat => api_user.home_lat,
+                       :lon => api_user.home_lon,
+                       :zoom => api_user.home_zoom
+    end
+    if api_user.languages
+      xml.tag! "languages" do
+        api_user.languages.split(",") { |lang| xml.tag! "lang", lang }
+      end
+    end
+    xml.tag! "messages" do
+      xml.tag! "received", :count => api_user.messages.size,
+                           :unread => api_user.new_messages.size
+      xml.tag! "sent", :count => api_user.sent_messages.size
+    end
+  end
+end

app/views/user/api_read.builder

@@ -1,47 +1,4 @@
 xml.instruct! :xml, :version => "1.0"
-xml.osm("version" => API_VERSION, "generator" => GENERATOR) do
+xml.osm("version" => API_VERSION, "generator" => GENERATOR) do |osm|
-  xml.tag! "user", :id => @user.id,
+  osm << render(:partial => "api_user", :object => @user)
-                   :display_name => @user.display_name,
-                   :account_created => @user.creation_time.xmlschema do
-    xml.tag! "description", @user.description if @user.description
-    if current_user && current_user == @user
-      xml.tag! "contributor-terms", :agreed => @user.terms_agreed.present?,
-                                    :pd => @user.consider_pd
-    else
-      xml.tag! "contributor-terms", :agreed => @user.terms_agreed.present?
-    end
-    xml.tag! "img", :href => user_image_url(@user) if @user.image.file? || @user.image_use_gravatar
-    xml.tag! "roles" do
-      @user.roles.each do |role|
-        xml.tag! role.role
-      end
-    end
-    xml.tag! "changesets", :count => @user.changesets.size
-    xml.tag! "traces", :count => @user.traces.size
-    xml.tag! "blocks" do
-      xml.tag! "received", :count => @user.blocks.size,
-                           :active => @user.blocks.active.size
-      if @user.moderator?
-        xml.tag! "issued", :count => @user.blocks_created.size,
-                           :active => @user.blocks_created.active.size
-      end
-    end
-    if current_user && current_user == @user
-      if @user.home_lat && @user.home_lon
-        xml.tag! "home", :lat => @user.home_lat,
-                         :lon => @user.home_lon,
-                         :zoom => @user.home_zoom
-      end
-      if @user.languages
-        xml.tag! "languages" do
-          @user.languages.split(",") { |lang| xml.tag! "lang", lang }
-        end
-      end
-      xml.tag! "messages" do
-        xml.tag! "received", :count => @user.messages.size,
-                             :unread => @user.new_messages.size
-        xml.tag! "sent", :count => @user.sent_messages.size
-      end
-    end
-  end
 end

app/views/user/api_users.builder

@@ -0,0 +1,4 @@
+xml.instruct! :xml, :version => "1.0"
+xml.osm("version" => API_VERSION, "generator" => GENERATOR) do |osm|
+  osm << render(:partial => "api_user", :collection => @users)
+end

config/routes.rb

@@ -67,6 +67,7 @@ OpenStreetMap::Application.routes.draw do
     get "user/:id" => "user#api_read", :id => /\d+/
     get "user/details" => "user#api_details"
     get "user/gpx_files" => "user#api_gpx_files"
+    get "users" => "user#api_users", :as => :api_users
 
     get "user/preferences" => "user_preferences#read"
     get "user/preferences/:preference_key" => "user_preferences#read_one"

test/controllers/user_controller_test.rb

@@ -20,6 +20,10 @@ class UserControllerTest < ActionController::TestCase
       { :path => "/api/0.6/user/gpx_files", :method => :get },
       { :controller => "user", :action => "api_gpx_files" }
     )
+    assert_routing(
+      { :path => "/api/0.6/users", :method => :get },
+      { :controller => "user", :action => "api_users" }
+    )
 
     assert_routing(
       { :path => "/login", :method => :get },
@@ -1146,6 +1150,48 @@ class UserControllerTest < ActionController::TestCase
     end
   end
 
+  def test_api_users
+    user1 = create(:user, :description => "test1", :terms_agreed => Date.yesterday)
+    user2 = create(:user, :description => "test2", :terms_agreed => Date.yesterday)
+    user3 = create(:user, :description => "test3", :terms_agreed => Date.yesterday)
+
+    get :api_users, :params => { :users => user1.id }
+    assert_response :success
+    assert_equal "text/xml", response.content_type
+    assert_select "user", :count => 1 do
+      assert_select "user[id='#{user1.id}']", :count => 1
+      assert_select "user[id='#{user2.id}']", :count => 0
+      assert_select "user[id='#{user3.id}']", :count => 0
+    end
+
+    get :api_users, :params => { :users => user2.id }
+    assert_response :success
+    assert_equal "text/xml", response.content_type
+    assert_select "user", :count => 1 do
+      assert_select "user[id='#{user1.id}']", :count => 0
+      assert_select "user[id='#{user2.id}']", :count => 1
+      assert_select "user[id='#{user3.id}']", :count => 0
+    end
+
+    get :api_users, :params => { :users => "#{user1.id},#{user3.id}" }
+    assert_response :success
+    assert_equal "text/xml", response.content_type
+    assert_select "user", :count => 2 do
+      assert_select "user[id='#{user1.id}']", :count => 1
+      assert_select "user[id='#{user2.id}']", :count => 0
+      assert_select "user[id='#{user3.id}']", :count => 1
+    end
+
+    get :api_users, :params => { :users => create(:user, :suspended).id }
+    assert_response :not_found
+
+    get :api_users, :params => { :users => create(:user, :deleted).id }
+    assert_response :not_found
+
+    get :api_users, :params => { :users => 0 }
+    assert_response :not_found
+  end
+
   def test_api_gpx_files
     user = create(:user)
     trace1 = create(:trace, :user => user) do |trace|