diff options
| -rw-r--r-- | app/models/group.rb | 8 | ||||
| -rw-r--r-- | app/models/project.rb | 6 | ||||
| -rw-r--r-- | app/models/user.rb | 6 | ||||
| -rw-r--r-- | app/services/projects/batch_count_service.rb | 21 | ||||
| -rw-r--r-- | app/services/projects/batch_forks_count_service.rb | 8 | ||||
| -rw-r--r-- | app/services/projects/batch_open_issues_count_service.rb | 7 | ||||
| -rw-r--r-- | app/services/projects/count_service.rb | 2 | ||||
| -rw-r--r-- | app/services/projects/forks_count_service.rb | 6 | ||||
| -rw-r--r-- | app/services/projects/open_issues_count_service.rb | 10 | ||||
| -rw-r--r-- | lib/api/entities.rb | 75 | ||||
| -rw-r--r-- | lib/api/projects.rb | 10 | ||||
| -rw-r--r-- | lib/api/projects_relation_builder.rb | 10 |
12 files changed, 96 insertions, 73 deletions
diff --git a/app/models/group.rb b/app/models/group.rb index 76262acf50c..27287f079a4 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -289,6 +289,14 @@ class Group < Namespace "#{parent.full_path}/#{path_was}" end + def group_member(user) + if group_members.loaded? + group_members.find { |gm| gm.user_id == user.id } + else + group_members.find_by(user_id: user) + end + end + private def update_two_factor_requirement diff --git a/app/models/project.rb b/app/models/project.rb index eaf4f555d3b..6f57763b05b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1114,7 +1114,11 @@ class Project < ActiveRecord::Base end def project_member(user) - project_members.find_by(user_id: user) + if project_members.loaded? + project_members.find { |member| member.user_id == user.id } + else + project_members.find_by(user_id: user) + end end def default_branch diff --git a/app/models/user.rb b/app/models/user.rb index 14941fd7f98..ee12f541b0f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -998,7 +998,11 @@ class User < ActiveRecord::Base end def notification_settings_for(source) - notification_settings.find_or_initialize_by(source: source) + if notification_settings.loaded? + notification_settings.find { |notification| notification.source == source } + else + notification_settings.find_or_initialize_by(source: source) + end end # Lazy load global notification setting diff --git a/app/services/projects/batch_count_service.rb b/app/services/projects/batch_count_service.rb index b8a030e7bc5..a79fdb62d69 100644 --- a/app/services/projects/batch_count_service.rb +++ b/app/services/projects/batch_count_service.rb @@ -4,28 +4,19 @@ module Projects @projects = projects end - def count - @projects.map do |project| - [project.id, current_count_service(project).count] - end.to_h - end - def refresh_cache @projects.each do |project| - unless current_count_service(project).count_stored? - current_count_service(project).refresh_cache { global_count[project.id].to_i } + service = count_service.new(project) + unless service.count_stored? + service.refresh_cache { global_count[project.id].to_i } end end end - def current_count_service(project) - if defined? @service - @service.project = project - else - @service = count_service.new(project) - end + def project_ids + return @projects if @projects.is_a?(ActiveRecord::Relation) - @service + @projects.map(&:id) end def global_count(project) diff --git a/app/services/projects/batch_forks_count_service.rb b/app/services/projects/batch_forks_count_service.rb index 51fad1c12a4..b9ed0b3cf4f 100644 --- a/app/services/projects/batch_forks_count_service.rb +++ b/app/services/projects/batch_forks_count_service.rb @@ -2,9 +2,11 @@ module Projects # Service class for getting and caching the number of forks of several projects class BatchForksCountService < Projects::BatchCountService def global_count - @global_count ||= ForkedProjectLink.where(forked_from_project: @projects.map(&:id)) - .group(:forked_from_project_id) - .count + @global_count ||= begin + count_service.query(project_ids) + .group(:forked_from_project_id) + .count + end end def count_service diff --git a/app/services/projects/batch_open_issues_count_service.rb b/app/services/projects/batch_open_issues_count_service.rb index a0d9120487c..5087684863b 100644 --- a/app/services/projects/batch_open_issues_count_service.rb +++ b/app/services/projects/batch_open_issues_count_service.rb @@ -2,10 +2,9 @@ module Projects # Service class for getting and caching the number of forks of several projects class BatchOpenIssuesCountService < Projects::BatchCountService def global_count - @global_count ||= Issue.opened.public_only - .where(project: @projects.map(&:id)) - .group(:project_id) - .count + @global_count ||= begin + count_service.query(project_ids).group(:project_id).count + end end def count_service diff --git a/app/services/projects/count_service.rb b/app/services/projects/count_service.rb index a3ec52232dd..7e575b2d6f3 100644 --- a/app/services/projects/count_service.rb +++ b/app/services/projects/count_service.rb @@ -7,8 +7,6 @@ module Projects # all caches. VERSION = 1 - attr_accessor :project - def initialize(project) @project = project end diff --git a/app/services/projects/forks_count_service.rb b/app/services/projects/forks_count_service.rb index d9bdf3a8ad7..d67ae78b268 100644 --- a/app/services/projects/forks_count_service.rb +++ b/app/services/projects/forks_count_service.rb @@ -2,11 +2,15 @@ module Projects # Service class for getting and caching the number of forks of a project. class ForksCountService < Projects::CountService def relation_for_count - @project.forks + self.class.query(@project.id) end def cache_key_name 'forks_count' end + + def self.query(project_ids) + ForkNetworkMember.where(forked_from_project: project_ids) + end end end diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index 25de97325e2..ab1c477936a 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -3,13 +3,17 @@ module Projects # project. class OpenIssuesCountService < Projects::CountService def relation_for_count - # We don't include confidential issues in this number since this would - # expose the number of confidential issues to non project members. - @project.issues.opened.public_only + self.class.query(@project.id) end def cache_key_name 'open_issues_count' end + + def self.query(project_ids) + # We don't include confidential issues in this number since this would + # expose the number of confidential issues to non project members. + Issue.opened.public_only.where(project: project_ids) + end end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8f41b930c0a..dada353a314 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -96,7 +96,7 @@ module API expose :star_count, :forks_count expose :created_at, :last_activity_at - def self.preload_relation(projects_relation) + def self.preload_relation(projects_relation, options = {}) projects_relation.preload(:project_feature, :route) .preload(namespace: [:route, :owner], tags: :taggings) @@ -134,18 +134,6 @@ module API expose :members do |project| expose_url(api_v4_projects_members_path(id: project.id)) end - - def self.preload_relation(projects_relation) - super(projects_relation).preload(:group) - .preload(project_group_links: :group, - fork_network: :root_project, - forked_project_link: :forked_from_project, - forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) - end - - def self.forks_counting_projects(projects_relation) - projects_relation + projects_relation.map(&:forked_from_project).compact - end end expose :archived?, as: :archived @@ -165,7 +153,9 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::NamespaceBasic' - expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } + expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } do |project, options| + project.fork_network_member.forked_from_project + end expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } @@ -182,6 +172,20 @@ module API expose :printing_merge_request_link_enabled expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics + + def self.preload_relation(projects_relation, options = {}) + relation = super(projects_relation).preload(:group) + .preload(project_group_links: :group, + fork_network: :root_project, + fork_network_member: [forked_from_project: [:route, namespace: :route, tags: :taggings]]) + + # Remove this preload once forked_project_links and forked_from_project models have been removed + relation.preload(forked_project_link: :forked_from_project) + end + + def self.forks_counting_projects(projects_relation) + projects_relation + projects_relation.map(&:fork_network_member).compact.map(&:forked_from_project).compact + end end class ProjectStatistics < Grape::Entity @@ -653,16 +657,10 @@ module API class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| - notification = member_notification_setting(member) - ::NotificationSetting.levels[notification.level] if notification - end - - private - - def member_notification_setting(member) - member.user.notification_settings.select do |notification| - notification.source_id == member.source_id && notification.source_type == member.source_type - end.last + # binding.pry if member.id == 5 + if member.notification_setting + ::NotificationSetting.levels[member.notification_setting.level] + end end end @@ -705,25 +703,36 @@ module API expose :permissions do expose :project_access, using: Entities::ProjectAccess do |project, options| if options.key?(:project_members) - (options[:project_members] || []).select { |member| member.source_id == project.id }.last - elsif project.project_members.any? - # This is not the bet option to search in a CollectionProxy, but if - # we use find_by we will perform another query, even if the association - # is loaded - project.project_members.select { |project_member| project_member.user_id == options[:current_user].id }.last + (options[:project_members] || []).find { |member| member.source_id == project.id } + else + project.project_member(options[:current_user]) end end expose :group_access, using: Entities::GroupAccess do |project, options| if project.group if options.key?(:group_members) - (options[:group_members] || []).select { |member| member.source_id == project.namespace_id }.last - elsif project.group.group_members.any? - project.group.group_members.select { |group_member| group_member.user_id == options[:current_user].id }.last + (options[:group_members] || []).find { |member| member.source_id == project.namespace_id } + else + project.group.group_member(options[:current_user]) end end end end + + def self.preload_relation(projects_relation, options = {}) + relation = super(projects_relation, options) + + unless options.key?(:group_members) + relation = relation.preload(group: [group_members: [:source, user: [notification_settings: :source]]]) + end + + unless options.key?(:project_members) + relation = relation.preload(project_members: [:source, user: [notification_settings: :source]]) + end + + relation + end end class LabelBasic < Grape::Entity diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 12506203429..9e92ff1417e 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -82,20 +82,20 @@ module API projects = paginate(projects) if current_user - project_members = current_user.project_members - group_members = current_user.group_members + project_members = current_user.project_members.preload(:source, user: [notification_settings: :source]) + group_members = current_user.group_members.preload(:source, user: [notification_settings: :source]) end options = options.reverse_merge( with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, statistics: params[:statistics], - project_members: project_members, - group_members: group_members, + project_members: nil, + group_members: nil, current_user: current_user ) options[:with] = Entities::BasicProjectDetails if params[:simple] - present options[:with].prepare_relation(projects), options + present options[:with].prepare_relation(projects, options), options end end diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb index 57df6157225..2e042d51c05 100644 --- a/lib/api/projects_relation_builder.rb +++ b/lib/api/projects_relation_builder.rb @@ -3,13 +3,13 @@ module API extend ActiveSupport::Concern module ClassMethods - def prepare_relation(relation) - relation = preload_relation(relation) - execute_batch_counting(relation) - relation + def prepare_relation(projects_relation, options = {}) + projects_relation = preload_relation(projects_relation, options) + execute_batch_counting(projects_relation) + projects_relation end - def preload_relation(relation) + def preload_relation(projects_relation, options = {}) raise NotImplementedError, 'self.preload_relation method must be defined and return a relation' end |