From 13e74543f9d0f91b7b3ef14279fd78d83f442750 Mon Sep 17 00:00:00 2001 From: Eugene Howe Date: Sun, 17 Jul 2016 10:32:11 -0400 Subject: [PATCH] speed up ExternalWikiService#get_project_wiki_path * This method previously iterated over all services in a project. Now it will directly query the ExternalWikiService for the project and filter by active state. * The presence of an external wiki is also cached * When an external wiki is added or removed, the cached value is updated --- CHANGELOG | 1 + app/helpers/external_wiki_helper.rb | 5 +-- app/models/project.rb | 16 ++++++++ app/models/service.rb | 8 ++++ ...53603_add_has_external_wiki_to_projects.rb | 7 ++++ db/schema.rb | 3 +- spec/models/project_spec.rb | 41 +++++++++++++++++++ 7 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20160718153603_add_has_external_wiki_to_projects.rb diff --git a/CHANGELOG b/CHANGELOG index 428d41178d4..d305d996cac 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.11.0 (unreleased) v 8.10.0 (unreleased) - Fix profile activity heatmap to show correct day name (eanplatter) + - Speed up ExternalWikiHelper#get_project_wiki_path - Expose {should,force}_remove_source_branch (Ben Boeckel) - Disable PostgreSQL statement timeout during migrations - Fix projects dropdown loading performance with a simplified api cal. !5113 (tiagonbotelho) diff --git a/app/helpers/external_wiki_helper.rb b/app/helpers/external_wiki_helper.rb index 1f3401f2906..defd87d6bbe 100644 --- a/app/helpers/external_wiki_helper.rb +++ b/app/helpers/external_wiki_helper.rb @@ -1,8 +1,7 @@ module ExternalWikiHelper def get_project_wiki_path(project) - external_wiki_service = project.services. - find { |service| service.to_param == 'external_wiki' } - if external_wiki_service.present? && external_wiki_service.active? + external_wiki_service = project.external_wiki + if external_wiki_service external_wiki_service.properties['external_wiki_url'] else namespace_project_wiki_path(project.namespace, project, :home) diff --git a/app/models/project.rb b/app/models/project.rb index a805f5d97bc..d08b737e813 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -650,6 +650,22 @@ class Project < ActiveRecord::Base update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) end + def external_wiki + if has_external_wiki.nil? + cache_has_external_wiki # Populate + end + + if has_external_wiki + @external_wiki ||= services.external_wikis.first + else + nil + end + end + + def cache_has_external_wiki + update_column(:has_external_wiki, services.external_wikis.any?) + end + def build_missing_services services_templates = Service.where(template: true) diff --git a/app/models/service.rb b/app/models/service.rb index 5432f8c7ab4..4821096379c 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -17,6 +17,7 @@ class Service < ActiveRecord::Base after_commit :reset_updated_properties after_commit :cache_project_has_external_issue_tracker + after_commit :cache_project_has_external_wiki belongs_to :project, inverse_of: :services has_one :service_hook @@ -25,6 +26,7 @@ class Service < ActiveRecord::Base scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } scope :issue_trackers, -> { where(category: 'issue_tracker') } + scope :external_wikis, -> { where(type: 'ExternalWikiService') } scope :active, -> { where(active: true) } scope :without_defaults, -> { where(default: false) } @@ -212,4 +214,10 @@ class Service < ActiveRecord::Base project.cache_has_external_issue_tracker end end + + def cache_project_has_external_wiki + if project && !project.destroyed? + project.cache_has_external_wiki + end + end end diff --git a/db/migrate/20160718153603_add_has_external_wiki_to_projects.rb b/db/migrate/20160718153603_add_has_external_wiki_to_projects.rb new file mode 100644 index 00000000000..55a3e954292 --- /dev/null +++ b/db/migrate/20160718153603_add_has_external_wiki_to_projects.rb @@ -0,0 +1,7 @@ +class AddHasExternalWikiToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column :projects, :has_external_wiki, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 8882377f9f4..ebf31ded369 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160716115710) do +ActiveRecord::Schema.define(version: 20160718153603) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -842,6 +842,7 @@ ActiveRecord::Schema.define(version: 20160716115710) do t.boolean "only_allow_merge_if_build_succeeds", default: false, null: false t.boolean "has_external_issue_tracker" t.string "repository_storage", default: "default", null: false + t.boolean "has_external_wiki" end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9dc34276f18..e3e7319beb2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -458,6 +458,47 @@ describe Project, models: true do end end + describe "#cache_has_external_wiki" do + let(:project) { create(:project) } + + it "stores true if there is an external wiki" do + services = double(:service, external_wikis: [ExternalWikiService.new]) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_wiki + end.to change { project.has_external_wiki }.to(true) + end + + it "stores false if there is no external wiki" do + services = double(:service, external_wikis: []) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_wiki + end.to change { project.has_external_wiki }.to(false) + end + + it "changes to true if an external wiki service is created later" do + expect do + project.cache_has_external_wiki + end.to change { project.has_external_wiki }.to(false) + + expect do + create(:service, type: "ExternalWikiService", project: project) + end.to change { project.has_external_wiki }.to(true) + end + + it "changes to false if an external wiki service is destroyed later" do + service = create(:service, type: "ExternalWikiService", project: project) + expect(project.has_external_wiki).to be_truthy + + expect do + service.destroy + end.to change { project.has_external_wiki }.to(false) + end + end + describe '#open_branches' do let(:project) { create(:project) }