diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-08-23 17:21:41 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-08-23 17:21:41 +0300 |
commit | 75d1283e59b296616ed1592de9856fc3d049f1ef (patch) | |
tree | 603d4ad986da877d0c32e9abcb95b3df0ef51203 /app | |
parent | 9ec60b5030608b7fbb9bd1a2e9a4d95d9b350f99 (diff) | |
parent | 6ec53f5d488fcdc6ef2b076c37a46525b5176224 (diff) |
Merge branch 'cache-issue-and-mr-counts' into 'master'
Cache the number of open issues and merge requests
Closes #36622
See merge request !13639
Diffstat (limited to 'app')
-rw-r--r-- | app/models/issue.rb | 7 | ||||
-rw-r--r-- | app/models/merge_request.rb | 5 | ||||
-rw-r--r-- | app/models/project.rb | 6 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 4 | ||||
-rw-r--r-- | app/services/issues/create_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/create_service.rb | 2 | ||||
-rw-r--r-- | app/services/projects/count_service.rb | 43 | ||||
-rw-r--r-- | app/services/projects/forks_count_service.rb | 28 | ||||
-rw-r--r-- | app/services/projects/open_issues_count_service.rb | 15 | ||||
-rw-r--r-- | app/services/projects/open_merge_requests_count_service.rb | 13 | ||||
-rw-r--r-- | app/views/layouts/nav/_new_project_sidebar.html.haml | 6 | ||||
-rw-r--r-- | app/views/layouts/nav/_project.html.haml | 6 |
12 files changed, 109 insertions, 28 deletions
diff --git a/app/models/issue.rb b/app/models/issue.rb index 043da9967a1..b9aa937d2f9 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -50,7 +50,10 @@ class Issue < ActiveRecord::Base scope :preload_associations, -> { preload(:labels, project: :namespace) } + scope :public_only, -> { where(confidential: false) } + after_save :expire_etag_cache + after_commit :update_project_counter_caches, on: :destroy attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true @@ -266,6 +269,10 @@ class Issue < ActiveRecord::Base end end + def update_project_counter_caches + Projects::OpenIssuesCountService.new(project).refresh_cache + end + private # Returns `true` if the given User can read the current Issue. diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f028d2395c1..dde43e94cb5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -31,6 +31,7 @@ class MergeRequest < ActiveRecord::Base after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed + after_commit :update_project_counter_caches, on: :destroy # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests @@ -936,6 +937,10 @@ class MergeRequest < ActiveRecord::Base true end + def update_project_counter_caches + Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache + end + private def write_ref diff --git a/app/models/project.rb b/app/models/project.rb index 37f4dd08355..3118a480f7b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1167,7 +1167,11 @@ class Project < ActiveRecord::Base end def open_issues_count - issues.opened.count + Projects::OpenIssuesCountService.new(self).count + end + + def open_merge_requests_count + Projects::OpenMergeRequestsCountService.new(self).count end def visibility_level_allowed_as_fork?(level = self.visibility_level) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 4a4f2b91182..1486db046b5 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -192,6 +192,8 @@ class IssuableBaseService < BaseService def after_create(issuable) # To be overridden by subclasses + + issuable.update_project_counter_caches end def before_update(issuable) @@ -200,6 +202,8 @@ class IssuableBaseService < BaseService def after_update(issuable) # To be overridden by subclasses + + issuable.update_project_counter_caches end def update(issuable) diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 234fcbede03..0307634c0b6 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -27,6 +27,8 @@ module Issues todo_service.new_issue(issuable, current_user) user_agent_detail_service.create resolve_discussions_with_issue(issuable) + + super end def resolve_discussions_with_issue(issue) diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 194413bf321..3d53fe0646b 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -28,6 +28,8 @@ module MergeRequests todo_service.new_merge_request(issuable, current_user) issuable.cache_merge_request_closes_issues!(current_user) update_merge_requests_head_pipeline(issuable) + + super end private diff --git a/app/services/projects/count_service.rb b/app/services/projects/count_service.rb new file mode 100644 index 00000000000..5e633c37bf8 --- /dev/null +++ b/app/services/projects/count_service.rb @@ -0,0 +1,43 @@ +module Projects + # Base class for the various service classes that count project data (e.g. + # issues or forks). + class CountService + def initialize(project) + @project = project + end + + def relation_for_count + raise( + NotImplementedError, + '"relation_for_count" must be implemented and return an ActiveRecord::Relation' + ) + end + + def count + Rails.cache.fetch(cache_key) { uncached_count } + end + + def refresh_cache + Rails.cache.write(cache_key, uncached_count) + end + + def uncached_count + relation_for_count.count + end + + def delete_cache + Rails.cache.delete(cache_key) + end + + def cache_key_name + raise( + NotImplementedError, + '"cache_key_name" must be implemented and return a String' + ) + end + + def cache_key + ['projects', @project.id, cache_key_name] + end + end +end diff --git a/app/services/projects/forks_count_service.rb b/app/services/projects/forks_count_service.rb index e2e2b1da91d..3a0fa84b868 100644 --- a/app/services/projects/forks_count_service.rb +++ b/app/services/projects/forks_count_service.rb @@ -1,30 +1,12 @@ module Projects # Service class for getting and caching the number of forks of a project. - class ForksCountService - def initialize(project) - @project = project + class ForksCountService < CountService + def relation_for_count + @project.forks end - def count - Rails.cache.fetch(cache_key) { uncached_count } - end - - def refresh_cache - Rails.cache.write(cache_key, uncached_count) - end - - def delete_cache - Rails.cache.delete(cache_key) - end - - private - - def uncached_count - @project.forks.count - end - - def cache_key - ['projects', @project.id, 'forks_count'] + def cache_key_name + 'forks_count' end end end diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb new file mode 100644 index 00000000000..3c0d186a73c --- /dev/null +++ b/app/services/projects/open_issues_count_service.rb @@ -0,0 +1,15 @@ +module Projects + # Service class for counting and caching the number of open issues of a + # project. + class OpenIssuesCountService < 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 + end + + def cache_key_name + 'open_issues_count' + end + end +end diff --git a/app/services/projects/open_merge_requests_count_service.rb b/app/services/projects/open_merge_requests_count_service.rb new file mode 100644 index 00000000000..2a90f78b90d --- /dev/null +++ b/app/services/projects/open_merge_requests_count_service.rb @@ -0,0 +1,13 @@ +module Projects + # Service class for counting and caching the number of open merge requests of + # a project. + class OpenMergeRequestsCountService < CountService + def relation_for_count + @project.merge_requests.opened + end + + def cache_key_name + 'open_merge_requests_count' + end + end +end diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 0ef81375c3a..81c84756ff8 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -86,7 +86,8 @@ %span.nav-item-name Issues - if @project.issues_enabled? - %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count) + %span.badge.count.issue_counter + = number_with_delimiter(@project.open_issues_count) %ul.sidebar-sub-level-items = nav_link(controller: :issues) do @@ -116,7 +117,8 @@ = custom_icon('mr_bold') %span.nav-item-name Merge Requests - %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count) + %span.badge.count.merge_counter.js-merge-counter + = number_with_delimiter(@project.open_merge_requests_count) - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts]) do diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 924cd2e9681..b88465848e3 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -28,7 +28,8 @@ %span Issues - if @project.issues_enabled? - %span.badge.count.issue_counter= number_with_delimiter(issuables_count_for_state(:issues, :opened, finder: IssuesFinder.new(current_user, project_id: @project.id))) + %span.badge.count.issue_counter + = number_with_delimiter(@project.open_issues_count) - if project_nav_tab? :merge_requests - controllers = [:merge_requests, 'projects/merge_requests/conflicts'] @@ -37,7 +38,8 @@ = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do %span Merge Requests - %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(issuables_count_for_state(:merge_requests, :opened, finder: MergeRequestsFinder.new(current_user, project_id: @project.id))) + %span.badge.count.merge_counter.js-merge-counter + = number_with_delimiter(@project.open_merge_requests_count) - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :environments, :artifacts]) do |