diff options
author | Stan Hu <stanhu@gmail.com> | 2018-01-27 09:16:56 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-01-31 08:46:09 +0300 |
commit | 46a6edc7314ce8acab5d8ce04799bd3557bc26bc (patch) | |
tree | 9b2d004d22a09ca51b4420feb87ca16d372da703 /lib | |
parent | 120c79020ddd3097ae64149c75864353276aaa5f (diff) |
Remove N+1 queries with /projects/:project_id/{access_requests,members} API endpoints
We can simplify the code quite a bit and improve performance by using
grape-entity merge fields:
https://github.com/ruby-grape/grape-entity/tree/v0.6.0#merge-fields
Relates to #42030
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/access_requests.rb | 6 | ||||
-rw-r--r-- | lib/api/entities.rb | 21 | ||||
-rw-r--r-- | lib/api/members.rb | 13 | ||||
-rw-r--r-- | lib/api/v3/members.rb | 15 |
4 files changed, 25 insertions, 30 deletions
diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 374b611f55e..60ae5e6b9a2 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -24,7 +24,7 @@ module API access_requesters = AccessRequestsFinder.new(source).execute!(current_user) access_requesters = paginate(access_requesters.includes(:user)) - present access_requesters.map(&:user), with: Entities::AccessRequester, source: source + present access_requesters, with: Entities::AccessRequester end desc "Requests access for the authenticated user to a #{source_type}." do @@ -36,7 +36,7 @@ module API access_requester = source.request_access(current_user) if access_requester.persisted? - present access_requester.user, with: Entities::AccessRequester, access_requester: access_requester + present access_requester, with: Entities::AccessRequester else render_validation_error!(access_requester) end @@ -56,7 +56,7 @@ module API member = ::Members::ApproveAccessRequestService.new(source, current_user, declared_params).execute status :created - present member.user, with: Entities::Member, member: member + present member, with: Entities::Member end desc 'Denies an access request for the given user.' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cb222697f32..e13463ec66b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -205,22 +205,15 @@ module API expose :build_artifacts_size, as: :job_artifacts_size end - class Member < UserBasic - expose :access_level do |user, options| - member = options[:member] || options[:source].members.find_by(user_id: user.id) - member.access_level - end - expose :expires_at do |user, options| - member = options[:member] || options[:source].members.find_by(user_id: user.id) - member.expires_at - end + class Member < Grape::Entity + expose :user, merge: true, using: UserBasic + expose :access_level + expose :expires_at end - class AccessRequester < UserBasic - expose :requested_at do |user, options| - access_requester = options[:access_requester] || options[:source].requesters.find_by(user_id: user.id) - access_requester.requested_at - end + class AccessRequester < Grape::Entity + expose :user, merge: true, using: UserBasic + expose :requested_at end class Group < Grape::Entity diff --git a/lib/api/members.rb b/lib/api/members.rb index 130c6d6da71..bc1de37284a 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -21,10 +21,11 @@ module API get ":id/members" do source = find_source(source_type, params[:id]) - users = source.users - users = users.merge(User.search(params[:query])) if params[:query].present? + members = source.members.where.not(user_id: nil).includes(:user) + members = members.joins(:user).merge(User.search(params[:query])) if params[:query].present? + members = paginate(members) - present paginate(users), with: Entities::Member, source: source + present members, with: Entities::Member end desc 'Gets a member of a group or project.' do @@ -39,7 +40,7 @@ module API members = source.members member = members.find_by!(user_id: params[:user_id]) - present member.user, with: Entities::Member, member: member + present member, with: Entities::Member end desc 'Adds a member to a group or project.' do @@ -62,7 +63,7 @@ module API if !member not_allowed! # This currently can only be reached in EE elsif member.persisted? && member.valid? - present member.user, with: Entities::Member, member: member + present member, with: Entities::Member else render_validation_error!(member) end @@ -83,7 +84,7 @@ module API member = source.members.find_by!(user_id: params.delete(:user_id)) if member.update_attributes(declared_params(include_missing: false)) - present member.user, with: Entities::Member, member: member + present member, with: Entities::Member else render_validation_error!(member) end diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb index 46145cac7a5..d7bde8ceb89 100644 --- a/lib/api/v3/members.rb +++ b/lib/api/v3/members.rb @@ -22,10 +22,11 @@ module API get ":id/members" do source = find_source(source_type, params[:id]) - users = source.users - users = users.merge(User.search(params[:query])) if params[:query].present? + members = source.members.where.not(user_id: nil).includes(:user) + members = members.joins(:user).merge(User.search(params[:query])) if params[:query].present? + members = paginate(members) - present paginate(users), with: ::API::Entities::Member, source: source + present members, with: ::API::Entities::Member end desc 'Gets a member of a group or project.' do @@ -40,7 +41,7 @@ module API members = source.members member = members.find_by!(user_id: params[:user_id]) - present member.user, with: ::API::Entities::Member, member: member + present member, with: ::API::Entities::Member end desc 'Adds a member to a group or project.' do @@ -69,7 +70,7 @@ module API end if member.persisted? && member.valid? - present member.user, with: ::API::Entities::Member, member: member + present member, with: ::API::Entities::Member else # This is to ensure back-compatibility but 400 behavior should be used # for all validation errors in 9.0! @@ -93,7 +94,7 @@ module API member = source.members.find_by!(user_id: params.delete(:user_id)) if member.update_attributes(declared_params(include_missing: false)) - present member.user, with: ::API::Entities::Member, member: member + present member, with: ::API::Entities::Member else # This is to ensure back-compatibility but 400 behavior should be used # for all validation errors in 9.0! @@ -125,7 +126,7 @@ module API else ::Members::DestroyService.new(source, current_user, declared_params).execute - present member.user, with: ::API::Entities::Member, member: member + present member, with: ::API::Entities::Member end end end |