From 3ce4de1295ecec082313740a3cdf25c2831164f7 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 19 Sep 2012 18:59:49 +0100 Subject: [PATCH] Add a /api/0.6/user/NNNN call to the API --- app/controllers/user_controller.rb | 31 +++++++++++++++++++------ app/views/user/api_details.builder | 26 --------------------- app/views/user/api_read.builder | 28 ++++++++++++++++++++++ config/routes.rb | 4 +++- db/structure.sql | 28 +++++++++++----------- test/functional/user_controller_test.rb | 28 +++++++++++++++++++++- 6 files changed, 96 insertions(+), 49 deletions(-) delete mode 100644 app/views/user/api_details.builder create mode 100644 app/views/user/api_read.builder diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index bfb0ef83b..9e1585243 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -1,20 +1,22 @@ class UserController < ApplicationController layout :choose_layout - skip_before_filter :verify_authenticity_token, :only => [:api_details, :api_gpx_files] + skip_before_filter :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files] before_filter :disable_terms_redirect, :only => [:terms, :save, :logout, :api_details] before_filter :authorize, :only => [:api_details, :api_gpx_files] - before_filter :authorize_web, :except => [:api_details, :api_gpx_files] - before_filter :set_locale, :except => [:api_details, :api_gpx_files] + before_filter :authorize_web, :except => [:api_read, :api_details, :api_gpx_files] + before_filter :set_locale, :except => [:api_read, :api_details, :api_gpx_files] before_filter :require_user, :only => [:account, :go_public, :make_friend, :remove_friend] - before_filter :check_database_readable, :except => [:login, :api_details, :api_gpx_files] + before_filter :check_database_readable, :except => [:login, :api_read, :api_details, :api_gpx_files] before_filter :check_database_writable, :only => [:new, :account, :confirm, :confirm_email, :lost_password, :reset_password, :go_public, :make_friend, :remove_friend] - before_filter :check_api_readable, :only => [:api_details, :api_gpx_files] + before_filter :check_api_readable, :only => [:api_read, :api_details, :api_gpx_files] before_filter :require_allow_read_prefs, :only => [:api_details] before_filter :require_allow_read_gpx, :only => [:api_gpx_files] before_filter :require_cookies, :only => [:login, :confirm] before_filter :require_administrator, :only => [:set_status, :delete, :list] - before_filter :lookup_this_user, :only => [:set_status, :delete] + around_filter :api_call_handle_error, :only => [:api_read, :api_details, :api_gpx_files] + before_filter :lookup_user_by_id, :only => [:api_read] + before_filter :lookup_user_by_name, :only => [:set_status, :delete] cache_sweeper :user_sweeper, :only => [:account, :set_status, :delete] @@ -373,6 +375,15 @@ class UserController < ApplicationController end end + def api_read + render :nothing => true, :status => :gone unless @this_user.visible? + end + + def api_details + @this_user = @user + render :action => :api_read + end + def api_gpx_files doc = OSM::API.new.get_xml_doc @user.traces.each do |trace| @@ -714,7 +725,13 @@ private ## # ensure that there is a "this_user" instance variable - def lookup_this_user + def lookup_user_by_id + @this_user = User.find(params[:id]) + end + + ## + # ensure that there is a "this_user" instance variable + def lookup_user_by_name @this_user = User.find_by_display_name(params[:display_name]) rescue ActiveRecord::RecordNotFound redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name] unless @this_user diff --git a/app/views/user/api_details.builder b/app/views/user/api_details.builder deleted file mode 100644 index 6e77bfed8..000000000 --- a/app/views/user/api_details.builder +++ /dev/null @@ -1,26 +0,0 @@ -xml.instruct! :xml, :version => "1.0" -xml.osm("version" => API_VERSION, "generator" => GENERATOR) do - xml.tag! "user", :id => @user.id, - :display_name => @user.display_name, - :account_created => @user.creation_time.xmlschema do - if @user.description - xml.tag! "description", @user.description - end - xml.tag! "contributor-terms", - :agreed => !!@user.terms_agreed, - :pd => !!@user.consider_pd - if @user.home_lat and @user.home_lon - xml.tag! "home", :lat => @user.home_lat, - :lon => @user.home_lon, - :zoom => @user.home_zoom - end - if @user.image.file? - xml.tag! "img", :href => "http://#{SERVER_URL}#{@user.image.url}" - end - if @user.languages - xml.tag! "languages" do - @user.languages.split(",") { |lang| xml.tag! "lang", lang } - end - end - end -end diff --git a/app/views/user/api_read.builder b/app/views/user/api_read.builder new file mode 100644 index 000000000..145607446 --- /dev/null +++ b/app/views/user/api_read.builder @@ -0,0 +1,28 @@ +xml.instruct! :xml, :version => "1.0" +xml.osm("version" => API_VERSION, "generator" => GENERATOR) do + xml.tag! "user", :id => @this_user.id, + :display_name => @this_user.display_name, + :account_created => @this_user.creation_time.xmlschema do + if @this_user.description + xml.tag! "description", @this_user.description + end + xml.tag! "contributor-terms", + :agreed => !!@this_user.terms_agreed, + :pd => !!@this_user.consider_pd + if @this_user.image.file? + xml.tag! "img", :href => "http://#{SERVER_URL}#{@this_user.image.url}" + end + if @user && @user == @this_user + if @this_user.home_lat and @this_user.home_lon + xml.tag! "home", :lat => @this_user.home_lat, + :lon => @this_user.home_lon, + :zoom => @this_user.home_zoom + end + if @this_user.languages + xml.tag! "languages" do + @this_user.languages.split(",") { |lang| xml.tag! "lang", lang } + end + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index b84d27243..9725c0d12 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -57,13 +57,15 @@ OpenStreetMap::Application.routes.draw do match 'api/0.6/relations/search' => 'search#search_relations', :via => :get match 'api/0.6/nodes/search' => 'search#search_nodes', :via => :get + match 'api/0.6/user/:id' => 'user#api_read', :via => :get, :id => /\d+/ match 'api/0.6/user/details' => 'user#api_details', :via => :get + match 'api/0.6/user/gpx_files' => 'user#api_gpx_files', :via => :get + match 'api/0.6/user/preferences' => 'user_preference#read', :via => :get match 'api/0.6/user/preferences/:preference_key' => 'user_preference#read_one', :via => :get match 'api/0.6/user/preferences' => 'user_preference#update', :via => :put match 'api/0.6/user/preferences/:preference_key' => 'user_preference#update_one', :via => :put match 'api/0.6/user/preferences/:preference_key' => 'user_preference#delete_one', :via => :delete - match 'api/0.6/user/gpx_files' => 'user#api_gpx_files', :via => :get match 'api/0.6/gpx/create' => 'trace#api_create', :via => :post match 'api/0.6/gpx/:id' => 'trace#api_read', :via => :get, :id => /\d+/ diff --git a/db/structure.sql b/db/structure.sql index 5f71a9fb4..a049e5ebd 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -101,7 +101,7 @@ CREATE TYPE user_status_enum AS ENUM ( CREATE FUNCTION maptile_for_point(bigint, bigint, integer) RETURNS integer LANGUAGE c STRICT - AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'maptile_for_point'; + AS '/srv/www/userapi.osm.compton.nu/db/functions/libpgosm.so', 'maptile_for_point'; -- @@ -110,7 +110,7 @@ CREATE FUNCTION maptile_for_point(bigint, bigint, integer) RETURNS integer CREATE FUNCTION tile_for_point(integer, integer) RETURNS bigint LANGUAGE c STRICT - AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'tile_for_point'; + AS '/srv/www/userapi.osm.compton.nu/db/functions/libpgosm.so', 'tile_for_point'; -- @@ -119,7 +119,7 @@ CREATE FUNCTION tile_for_point(integer, integer) RETURNS bigint CREATE FUNCTION xid_to_int4(xid) RETURNS integer LANGUAGE c IMMUTABLE STRICT - AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'xid_to_int4'; + AS '/srv/www/userapi.osm.compton.nu/db/functions/libpgosm.so', 'xid_to_int4'; SET default_tablespace = ''; @@ -218,8 +218,8 @@ CREATE TABLE client_applications ( key character varying(50), secret character varying(50), user_id integer, - created_at timestamp without time zone, - updated_at timestamp without time zone, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL, allow_read_prefs boolean DEFAULT false NOT NULL, allow_write_prefs boolean DEFAULT false NOT NULL, allow_write_diary boolean DEFAULT false NOT NULL, @@ -708,8 +708,8 @@ CREATE TABLE oauth_nonces ( id integer NOT NULL, nonce character varying(255), "timestamp" integer, - created_at timestamp without time zone, - updated_at timestamp without time zone + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL ); @@ -745,8 +745,8 @@ CREATE TABLE oauth_tokens ( secret character varying(50), authorized_at timestamp without time zone, invalidated_at timestamp without time zone, - created_at timestamp without time zone, - updated_at timestamp without time zone, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL, allow_read_prefs boolean DEFAULT false NOT NULL, allow_write_prefs boolean DEFAULT false NOT NULL, allow_write_diary boolean DEFAULT false NOT NULL, @@ -874,8 +874,8 @@ CREATE TABLE user_blocks ( ends_at timestamp without time zone NOT NULL, needs_view boolean DEFAULT false NOT NULL, revoker_id bigint, - created_at timestamp without time zone, - updated_at timestamp without time zone, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL, reason_format format_enum DEFAULT 'html'::format_enum NOT NULL ); @@ -917,8 +917,8 @@ CREATE TABLE user_preferences ( CREATE TABLE user_roles ( id integer NOT NULL, user_id bigint NOT NULL, - created_at timestamp without time zone, - updated_at timestamp without time zone, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL, role user_role_enum NOT NULL, granter_id bigint NOT NULL ); @@ -1000,9 +1000,9 @@ CREATE TABLE users ( status user_status_enum DEFAULT 'pending'::user_status_enum NOT NULL, terms_agreed timestamp without time zone, consider_pd boolean DEFAULT false NOT NULL, + openid_url character varying(255), preferred_editor character varying(255), terms_seen boolean DEFAULT false NOT NULL, - openid_url character varying(255), description_format format_enum DEFAULT 'html'::format_enum NOT NULL, image_fingerprint character varying(255), changesets_count integer DEFAULT 0 NOT NULL, diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb index f756b0514..afff1b2a8 100644 --- a/test/functional/user_controller_test.rb +++ b/test/functional/user_controller_test.rb @@ -6,6 +6,10 @@ class UserControllerTest < ActionController::TestCase ## # test all routes which lead to this controller def test_routes + assert_routing( + { :path => "/api/0.6/user/1", :method => :get }, + { :controller => "user", :action => "api_read", :id => "1" } + ) assert_routing( { :path => "/api/0.6/user/details", :method => :get }, { :controller => "user", :action => "api_details" } @@ -520,7 +524,29 @@ class UserControllerTest < ActionController::TestCase assert_select "a[href=/blocks/new/test]", 1 end end - + + def test_user_api_read + # check that a visible user is returned properly + get :api_read, :id => users(:normal_user).id + assert_response :success + + # check that we aren't revealing private information + assert_select "home", false + assert_select "languages", false + + # check that a suspended user is not returned + get :api_read, :id => users(:suspended_user).id + assert_response :gone + + # check that a deleted user is not returned + get :api_read, :id => users(:deleted_user).id + assert_response :gone + + # check that a non-existent user is not returned + get :api_read, :id => 0 + assert_response :not_found + end + def test_user_api_details get :api_details assert_response :unauthorized -- 2.39.5