ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

api: some optimizations to users api 

closes #10051

eliminate some n+1 queries in the users api by being consistent about
preloading necessary associations.  also refactor the user api docs so that
they follow the new @object/@returns model

test plan:
- make sure user api docs look good (and user parts of course api docs)
- there should be no noticiable changes to api behavior
- in dev, tail the logs, make some queries with lots of users, and make sure
  they are reasonable.

Change-Id: I4a3b0b94bbce4c62cdbfc83941b79b25773ba904
Reviewed-on: https://gerrit.instructure.com/13022
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Simon Williams 2012-08-20 10:41:17 -06:00
parent 10a357944c
commit db510f9640
6 changed files with 119 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
app/controllers/courses_controller.rb
View File

@ -263,23 +263,13 @@ class CoursesController < ApplicationController
#
# Returns the list of students enrolled in this course.
#
# @response_field id The unique identifier for the student.
# @response_field name The full student name.
# @response_field sis_user_id The SIS id for the user's primary pseudonym.
# DEPRECATED: Please use the {api:CoursesController#users course users} endpoint
# and pass "student" as the enrollment_type.
#
# @example_response
# [ { 'id': 1, 'name': 'first student', 'sis_user_id': null, 'sis_login_id': null },
# { 'id': 2, 'name': 'second student', 'sis_user_id': 'from-sis', 'sis_login_id': 'login-from-sis' } ]
# @returns [User]
def students
# DEPRECATED. #users should replace this in a new version of the API.
get_context
if authorized_action(@context, @current_user, :read_roster)
proxy = @context.students.order_by_sortable_name
if user_json_is_admin?
proxy = proxy.scoped(:include => :pseudonyms)
end
render :json => proxy.map { |u| user_json(u, @current_user, session) }
end
params[:enrollment_type] = "student"
self.users
end
# @API List users
@ -294,32 +284,7 @@ class CoursesController < ApplicationController
# @argument include[] ["locked"] Optionally include whether an enrollment is locked.
# @argument include[] ["avatar_url"] Optionally include avatar_url.
#
# @response_field id The unique identifier for the user.
# @response_field name The full user name.
# @response_field sortable_name The sortable user name.
# @response_field short_name The short user name.
# @response_field sis_user_id The SIS id for the user's primary pseudonym.
#
# @response_field enrollments The user's enrollments in this course.
# See the API documentation for enrollment data returned; however, the user data is not included.
#
# @example_response
# [
# {
# 'id': 1,
# 'name': 'first user',
# 'sis_user_id': null,
# 'sis_login_id': null,
# 'enrollments': [ ... ],
# },
# {
# 'id': 2,
# 'name': 'second user',
# 'sis_user_id': 'from-sis',
# 'sis_login_id': 'login-from-sis',
# 'enrollments': [ ... ],
# }
# ]
# @returns [User]
def users
get_context
if authorized_action(@context, @current_user, :read_roster)
@ -327,13 +292,13 @@ class CoursesController < ApplicationController
users = @context.users_visible_to(@current_user)
users = users.scoped(:order => "users.sortable_name")
users = users.scoped(:conditions => ["enrollments.type = ? ", enrollment_type]) if enrollment_type
if user_json_is_admin?
users = users.scoped(:include => {:pseudonym => :communication_channel})
end
users = Api.paginate(users, self, api_v1_course_users_path)
includes = Array(params[:include])
user_json_preloads(users, includes.include?('email'))
if includes.include?('enrollments')
User.send(:preload_associations, users, :not_ended_enrollments,
# not_ended_enrollments for enrollment_json
# enrollments course for has_grade_permissions?
User.send(:preload_associations, users, { :not_ended_enrollments => :course },
:conditions => ['enrollments.course_id = ?', @context.id])
end
render :json => users.map { |u|
@ -345,34 +310,23 @@ class CoursesController < ApplicationController
# @API List recently logged in students
#
# Returns the list of users in this course, including a 'last_login' field
# which contains a timestamp of the last time that user logged into canvas.
# The querying user must have the 'View usage reports' permission.
# Returns the list of users in this course, ordered by how recently they have
# logged in. The records include the 'last_login' field which contains
# a timestamp of the last time that user logged into canvas. The querying
# user must have the 'View usage reports' permission.
#
# @example_request
# curl -H 'Authorization: Bearer <token>' \
# https://<canvas>/api/v1/courses/<course_id>/recent_users
#
# @example_response
# [
# {
# 'id': 1,
# 'name': 'first user',
# 'sis_user_id': null,
# 'sis_login_id': null,
# 'last_login': <timestamp>,
# },
# ...
# ]
# @returns [User]
def recent_students
get_context
if authorized_action(@context, @current_user, :read_reports)
scope = User.for_course_with_last_login(@context, @context.root_account_id, 'StudentEnrollment')
scope = scope.scoped(:order => 'login_info_exists, last_login DESC')
users = Api.paginate(scope, self, api_v1_course_recent_students_url)
if user_json_is_admin?
User.send(:preload_associations, users, :pseudonyms)
end
user_json_preloads(users)
render :json => users.map { |u| user_json(u, @current_user, session, ['last_login']) }
end
end
@ -382,17 +336,19 @@ class CoursesController < ApplicationController
#
# Accepts the same include[] parameters as the :users: action, and returns a
# single user with the same fields as that action.
#
# @returns User
def user
get_context
if authorized_action(@context, @current_user, :read_roster)
users = @context.users_visible_to(@current_user)
users = users.scoped(:conditions => ['users.id = ?', params[:id]])
if user_json_is_admin?
users = users.scoped(:include => {:pseudonym => :communication_channel})
end
includes = Array(params[:include])
user_json_preloads(users, includes.include?('email'))
if includes.include?('enrollments')
User.send(:preload_associations, users, :not_ended_enrollments,
# not_ended_enrollments for enrollment_json
# enrollments course for has_grade_permissions?
User.send(:preload_associations, users, { :not_ended_enrollments => :course },
:conditions => ['enrollments.course_id = ?', @context.id])
end
user = users.first or raise ActiveRecord::RecordNotFound
@ -508,7 +464,7 @@ class CoursesController < ApplicationController
:ta => enrollment_counts['TaEnrollment'] || 0,
:observer => enrollment_counts['ObserverEnrollment'] || 0,
:designer => enrollment_counts['DesignerEnrollment'] || 0,
:invited => users_scope.count(:distinct => true, :select => 'users.id', :conditions => ["enrollments.workflow_state = 'invited'"])
:invited => users_scope.count(:distinct => true, :select => 'users.id', :conditions => ["enrollments.workflow_state = 'invited' AND enrollments.type != 'StudentViewEnrollment'"])
}
js_env(:COURSE_ID => @context.id,
:USER_COUNTS => @user_counts,

10
app/controllers/enrollments_api_controller.rb
View File

@ -136,12 +136,11 @@ class EnrollmentsApiController < ApplicationController
}
endpoint_scope = (@context.is_a?(Course) ? (@section.present? ? "section" : "course") : "user")
scope_arguments = { :conditions => @conditions,
scope_arguments = {
:conditions => @conditions,
:order => 'enrollments.type ASC, users.sortable_name ASC',
:include => {:user => [], :course => [], :course_section => []} }
if user_json_is_admin?
scope_arguments[:include][:user] = :pseudonyms
end
:include => [:user, :course, :course_section]
}
return unless enrollments = @context.is_a?(Course) ?
course_index_enrollments(scope_arguments) :
@ -152,6 +151,7 @@ class EnrollmentsApiController < ApplicationController
self, send("api_v1_#{endpoint_scope}_enrollments_path"))
includes = [:user] + Array(params[:include])
user_json_preloads(enrollments.map(&:user))
render :json => enrollments.map { |e| enrollment_json(e, @current_user, session, includes) }
end

2
app/controllers/profile_controller.rb
View File

@ -129,7 +129,7 @@ class ProfileController < ApplicationController
def communication_update
params[:root_account] = @domain_root_account
@policies = NotificationPolicy.setup_for(@current_user, params)
NotificationPolicy.setup_for(@current_user, params)
render :json => {}, :status => :ok
end

100
app/controllers/users_controller.rb
View File

@ -23,6 +23,60 @@
# a shortcut for the id of the user accessing the API. For instance,
# `users/:user_id/page_views` can be accessed as `users/self/page_views` to
# access the current user's page views.
#
# @object User
# {
# // The ID of the user.
# "id": 1,
#
# // The name of the user.
# "name": "Sheldon Cooper",
#
# // The name of the user that is should be used for sorting groups of users,
# // such as in the gradebook.
# "sortable_name": "Cooper, Sheldon",
#
# // A short name the user has selected, for use in conversations or other less
# // formal places through the site.
# "short_name": "Shelly",
#
# // The SIS ID associated with the user. This field is only included if the
# // user came from a SIS import
# "sis_user_id": "",
#
# // DEPRECATED: The SIS login ID associated with the user. Please use the
# // sis_user_id or login_id. This field will be removed in a future version of
# // the API.
# "sis_login_id": "",
#
# // The unique login id for the user. This is what the user uses to log in to
# // canvas.
# "login_id": "sheldon@caltech.example.com",
#
# // If avatars are enabled, this field will be included and contain a url to
# // retrieve the user's avatar.
# "avatar_url": "",
#
# // Optional: This field can be requested with certain API calls, and will
# // return a list of the users active enrollments. See the List enrollments
# // API for more details about the format of these records.
# "enrollments": [
# // ...
# ],
#
# // Optional: This field can be requested with certain API calls, and will
# // return the users primary email address.
# "email": "sheldon@caltech.example.com",
#
# // Optional: This field can be requested with certain API calls, and will
# // return the users locale.
# "locale": "tlh",
#
# // Optional: This field is only returned in certain API calls, and will
# // return a timestamp representing the last time the user logged in to
# // canvas.
# "last_login": "2012-05-30T17:45:25Z",
# }
class UsersController < ApplicationController
@ -160,11 +214,7 @@ class UsersController < ApplicationController
# @API List users
# Retrieve the list of users associated with this account.
#
# @example_response
# [
# { "id": 1, "name": "Dwight Schrute", "sortable_name": "Schrute, Dwight", "short_name": "Dwight", "login_id": "dwight@example.com", "sis_user_id": "12345", "sis_login_id": null },
# { "id": 2, "name": "Gob Bluth", "sortable_name": "Bluth, Gob", "short_name": "Gob Bluth", "login_id": "gob@example.com", "sis_user_id": "67890", "sis_login_id": null }
# ]
# @returns [User]
def index
get_context
if authorized_action(@context, @current_user, :read_roster)
@ -180,13 +230,18 @@ class UsersController < ApplicationController
:conditions => ["courses.enrollment_term_id = ?", params[:enrollment_term_id]],
:group => @context.connection.group_by('users.id', 'users.name', 'users.sortable_name')
})
else
elsif !api_request?
@users = @context.fast_all_users
end
@users = api_request? ?
Api.paginate(@users, self, api_v1_account_users_path, :order => :sortable_name) :
@users.paginate(:page => params[:page], :per_page => @per_page, :total_entries => @users.size)
if api_request?
@users = User.of_account(@context).active.order_by_sortable_name
@users = Api.paginate(@users, self, api_v1_account_users_path, :order => :sortable_name)
user_json_preloads(@users)
else
@users = @users.paginate(:page => params[:page], :per_page => @per_page, :total_entries => @users.size)
end
respond_to do |format|
if @users.length == 1 && params[:term]
format.html {
@ -199,7 +254,7 @@ class UsersController < ApplicationController
end
format.html
end
format.json {
format.json {
cancel_cache_buster
expires_in 30.minutes
api_request? ?
@ -211,6 +266,7 @@ class UsersController < ApplicationController
end
end
before_filter :require_password_session, :only => [:masquerade]
def masquerade
@user = User.find(:first, :conditions => {:id => params[:user_id]})
@ -619,6 +675,8 @@ class UsersController < ApplicationController
# @argument pseudonym[password] [Optional] User's password.
# @argument pseudonym[sis_user_id] [Optional] [Integer] SIS ID for the user's account. To set this parameter, the caller must be able to manage SIS permissions.
# @argument pseudonym[send_confirmation] [Optional, 0|1] [Integer] Send user notification of account creation if set to 1.
#
# @returns User
def create
# Look for an incomplete registration with this pseudonym
@pseudonym = @context.pseudonyms.active.custom_find_by_unique_id(params[:pseudonym][:unique_id])
@ -772,16 +830,7 @@ class UsersController < ApplicationController
# -F 'user[avatar][token]=<opaque_token>' \
# -H "Authorization: Bearer <token>"
#
# @example_response
#
# {
# "id":133,
# "login_id":"sheldor@example.com",
# "name":"Sheldon Cooper",
# "short_name":"Shelly",
# "sortable_name":"Cooper, Sheldon",
# "avatar_url":"http://<canvas>/images/users/133-..."
# }
# @returns User
def update
params[:user] ||= {}
@user = api_request? ?
@ -1029,14 +1078,7 @@ class UsersController < ApplicationController
# -H 'Authorization: Bearer <ACCESS_TOKEN>' \
# -X DELETE
#
# @example_response
# {
# "id":133,
# "login_id":"bieber@example.com",
# "name":"Justin Bieber",
# "short_name":"The Biebs",
# "sortable_name":"Bieber, Justin"
# }
# @returns User
def destroy
@user = api_request? ? api_find(User, params[:id]) : User.find(params[:id])
if authorized_action(@user, @current_user, [:manage, :manage_logins])
@ -1301,6 +1343,4 @@ class UsersController < ApplicationController
data.values.sort_by { |e| e[:enrollment].user.sortable_name.downcase }
end
end

11
app/models/user.rb
View File

@ -632,9 +632,16 @@ class User < ActiveRecord::Base
end
def email
Rails.cache.fetch(['user_email', self].cache_key) do
email_channel.path if email_channel
# if you change this cache_key, change it in email_cached? as well
value = Rails.cache.fetch(['user_email', self].cache_key) do
email_channel.try(:path) || :none
end
# this sillyness is because rails equates falsey as not in the cache
value == :none ? nil : value
end
def email_cached?
Rails.cache.exist?(['user_email', self].cache_key)
end
def self.cached_name(id)

13
lib/api/v1/user.rb
View File

@ -26,6 +26,16 @@ module Api::V1::User
:methods => %w(sortable_name short_name)
}
def user_json_preloads(users, preload_email=false)
# pseudonyms for User#sis_pseudoym_for and User#find_pseudonym_for_account
# pseudonyms account for Pseudonym#works_for_account?
User.send(:preload_associations, users, [{ :pseudonyms => :account }]) if user_json_is_admin?
if preload_email && (no_email_users = users.reject(&:email_cached?)).present?
# communication_channesl for User#email if it is not cached
User.send(:preload_associations, no_email_users, :communication_channels)
end
end
def user_json(user, current_user, session, includes = [], context = @context, enrollments = nil)
includes ||= []
api_json(user, current_user, session, API_USER_JSON_OPTS).tap do |json|
@ -135,9 +145,8 @@ module Api::V1::User
protected
def has_grade_permissions?(user, enrollment)
course = enrollment.course
enrollment_user = enrollment.user
(user == enrollment_user && !course.settings[:hide_final_grade]) ||
(user.id == enrollment.user_id && !course.settings[:hide_final_grade]) ||
course.grants_rights?(user, :manage_grades, :view_all_grades).values.any?
end
end