diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-10 14:13:30 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-10 14:13:30 +0000 |
commit | 747a167a2dce3175102e4aff78504c72178cc5a5 (patch) | |
tree | 489bde9547aecffc9289b83a5eaa015f1fd0ca60 | |
parent | f29fd65cdde1d769fc89f0cc57ea989765b5068f (diff) | |
parent | be98ee2586466b8f9f4e96e4b54490c412f6b43b (diff) | |
download | gitlab-ce-747a167a2dce3175102e4aff78504c72178cc5a5.tar.gz |
Merge branch '18027-cache-project-external_issue_tracker' into 'master'
Cache the presence of an issue_tracker at project level
See merge request !4466
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/project.rb | 18 | ||||
-rw-r--r-- | app/models/service.rb | 10 | ||||
-rw-r--r-- | db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb | 10 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/lib/banzai/pipeline/wiki_pipeline_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/project_services/bamboo_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_services/teamcity_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 63 | ||||
-rw-r--r-- | spec/models/service_spec.rb | 33 |
10 files changed, 137 insertions, 7 deletions
diff --git a/CHANGELOG b/CHANGELOG index 1c64739f701..0c712b445a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -62,6 +62,7 @@ v 8.9.0 (unreleased) - Markdown editor now correctly resets the input value on edit cancellation !4175 - Toggling a task list item in a issue/mr description does not creates a Todo for mentions - Improved UX of date pickers on issue & milestone forms + - Cache on the database if a project has an active external issue tracker. v 8.8.5 (unreleased) - Ensure branch cleanup regardless of whether the GitHub import process succeeds diff --git a/app/models/project.rb b/app/models/project.rb index f47ef8a81de..472e79cfcf8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -523,9 +523,21 @@ class Project < ActiveRecord::Base end def external_issue_tracker - return @external_issue_tracker if defined?(@external_issue_tracker) - @external_issue_tracker ||= - services.issue_trackers.active.without_defaults.first + if has_external_issue_tracker.nil? # To populate existing projects + cache_has_external_issue_tracker + end + + if has_external_issue_tracker? + return @external_issue_tracker if defined?(@external_issue_tracker) + + @external_issue_tracker = services.external_issue_trackers.first + else + nil + end + end + + def cache_has_external_issue_tracker + update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) end def can_have_issues_tracker_id? diff --git a/app/models/service.rb b/app/models/service.rb index de3fd24584a..bf352397509 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -16,6 +16,7 @@ class Service < ActiveRecord::Base after_initialize :initialize_properties after_commit :reset_updated_properties + after_commit :cache_project_has_external_issue_tracker belongs_to :project has_one :service_hook @@ -34,6 +35,7 @@ class Service < ActiveRecord::Base scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } + scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } default_value_for :category, 'common' @@ -192,4 +194,12 @@ class Service < ActiveRecord::Base service.project_id = project_id service if service.save end + + private + + def cache_project_has_external_issue_tracker + if project && !project.destroyed? + project.cache_has_external_issue_tracker + end + end end diff --git a/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb b/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb new file mode 100644 index 00000000000..be295f0181d --- /dev/null +++ b/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb @@ -0,0 +1,10 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddHasExternalIssueTrackerToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column(:projects, :has_external_issue_tracker, :boolean) + end +end diff --git a/db/schema.rb b/db/schema.rb index 09ff52ccc8e..aac327797e7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -780,6 +780,7 @@ ActiveRecord::Schema.define(version: 20160608155312) do t.datetime "last_repository_check_at" t.boolean "container_registry_enabled" t.boolean "only_allow_merge_if_build_succeeds", default: false, null: false + t.boolean "has_external_issue_tracker" 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/lib/banzai/pipeline/wiki_pipeline_spec.rb b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb index ea4ab2c852e..72bc6a0b704 100644 --- a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb @@ -52,8 +52,8 @@ describe Banzai::Pipeline::WikiPipeline do end describe "Links" do - let(:namespace) { build_stubbed(:namespace, name: "wiki_link_ns") } - let(:project) { build_stubbed(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } + let(:namespace) { create(:namespace, name: "wiki_link_ns") } + let(:project) { create(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } let(:project_wiki) { ProjectWiki.new(project, double(:user)) } let(:page) { build(:wiki_page, wiki: project_wiki, page: OpenStruct.new(url_path: 'nested/twice/start-page')) } diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index e771f35811e..ec81f05fc7a 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -194,7 +194,7 @@ describe BambooService, models: true do def service(bamboo_url: 'http://gitlab.com') described_class.create( - project: build_stubbed(:empty_project), + project: create(:empty_project), properties: { bamboo_url: bamboo_url, username: 'mic', diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index ad24b895170..24a708ca849 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -182,7 +182,7 @@ describe TeamcityService, models: true do def service(teamcity_url: 'http://gitlab.com') described_class.create( - project: build_stubbed(:empty_project), + project: create(:empty_project), properties: { teamcity_url: teamcity_url, username: 'mic', diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 553556ed326..b6f36d3481e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -258,6 +258,69 @@ describe Project, models: true do end end + describe :external_issue_tracker do + let(:project) { create(:project) } + let(:ext_project) { create(:redmine_project) } + + context 'on existing projects with no value for has_external_issue_tracker' do + before(:each) do + project.update_column(:has_external_issue_tracker, nil) + ext_project.update_column(:has_external_issue_tracker, nil) + end + + it 'updates the has_external_issue_tracker boolean' do + expect do + project.external_issue_tracker + end.to change { project.reload.has_external_issue_tracker }.to(false) + + expect do + ext_project.external_issue_tracker + end.to change { ext_project.reload.has_external_issue_tracker }.to(true) + end + end + + it 'returns nil and does not query services when there is no external issue tracker' do + project.build_missing_services + project.reload + + expect(project).not_to receive(:services) + + expect(project.external_issue_tracker).to eq(nil) + end + + it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do + ext_project.reload # Factory returns a project with changed attributes + ext_project.build_missing_services + ext_project.reload + + expect(ext_project).to receive(:services).once.and_call_original + + 2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) } + end + end + + describe :cache_has_external_issue_tracker do + let(:project) { create(:project) } + + it 'stores true if there is any external_issue_tracker' do + services = double(:service, external_issue_trackers: [RedmineService.new]) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_issue_tracker + end.to change { project.has_external_issue_tracker}.to(true) + end + + it 'stores false if there is no external_issue_tracker' do + services = double(:service, external_issue_trackers: []) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_issue_tracker + end.to change { project.has_external_issue_tracker}.to(false) + end + end + describe :can_have_issues_tracker_id? do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 8592e112c50..2f000dbc01a 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -204,4 +204,37 @@ describe Service, models: true do expect(service.bamboo_url_was).to be_nil end end + + describe "callbacks" do + let(:project) { create(:project) } + let!(:service) do + RedmineService.new( + project: project, + active: true, + properties: { + project_url: 'http://redmine/projects/project_name_in_redmine', + issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id", + new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new' + } + ) + end + + describe "on create" do + it "updates the has_external_issue_tracker boolean" do + expect do + service.save! + end.to change { service.project.has_external_issue_tracker }.from(nil).to(true) + end + end + + describe "on update" do + it "updates the has_external_issue_tracker boolean" do + service.save! + + expect do + service.update_attributes(active: false) + end.to change { service.project.has_external_issue_tracker }.from(true).to(false) + end + end + end end |