diff options
author | Valery Sizov <valery@gitlab.com> | 2018-09-25 16:29:17 +0300 |
---|---|---|
committer | Valery Sizov <valery@gitlab.com> | 2018-09-25 17:49:39 +0300 |
commit | 15d011d64d579616dd9df3e3e5082ed5b99bcdf8 (patch) | |
tree | cb89f3316d037e19ddc5e1b39fdf37955313e103 | |
parent | cb0f024cd4c4aa51c7824e4e1a6c4d0f307afad1 (diff) | |
download | gitlab-ce-15d011d64d579616dd9df3e3e5082ed5b99bcdf8.tar.gz |
Geo: sync disabled wikis. Stage 2follow-up-geo-sync-disabled-wikis
We started syncing all the wiki regardless of the fact it's disabled or
not. We couldn't do that in one stage because of needing of smoth update
and deprecating things. This is the second stage that finally removes
unused columns in the geo_node_status table.
-rw-r--r-- | app/models/project.rb | 7 | ||||
-rw-r--r-- | app/models/project_feature.rb | 26 | ||||
-rw-r--r-- | app/models/site_statistic.rb | 2 | ||||
-rw-r--r-- | db/post_migrate/20180917172041_remove_wikis_count_from_site_statistics.rb | 6 | ||||
-rw-r--r-- | db/schema.rb | 3 | ||||
-rw-r--r-- | lib/tasks/gitlab/site_statistics.rake | 8 | ||||
-rw-r--r-- | spec/factories/site_statistics.rb | 1 | ||||
-rw-r--r-- | spec/models/project_feature_spec.rb | 42 | ||||
-rw-r--r-- | spec/models/site_statistic_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/groups/destroy_service_spec.rb | 8 | ||||
-rw-r--r-- | spec/tasks/gitlab/site_statistics_rake_spec.rb | 5 |
11 files changed, 11 insertions, 99 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 2b2dff3a718..1156a400a21 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -86,7 +86,7 @@ class Project < ActiveRecord::Base after_create :create_project_feature, unless: :project_feature after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) } - before_destroy :untrack_site_statistics + before_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) } after_create :create_ci_cd_settings, unless: :ci_cd_settings, @@ -2110,11 +2110,6 @@ class Project < ActiveRecord::Base Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path) end - def untrack_site_statistics - SiteStatistic.untrack(STATISTICS_ATTRIBUTE) - self.project_feature.untrack_statistics_for_deletion! - end - # rubocop: disable CodeReuse/ServiceClass def execute_rename_repository_hooks!(full_path_before) # When we import a project overwriting the original project, there diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index d74cb2506ba..4a0324e8b5c 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -21,7 +21,6 @@ class ProjectFeature < ActiveRecord::Base ENABLED = 20 FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze - STATISTICS_ATTRIBUTE = 'wikis_count'.freeze class << self def access_level_attribute(feature) @@ -55,9 +54,6 @@ class ProjectFeature < ActiveRecord::Base default_value_for :wiki_access_level, value: ENABLED, allows_nil: false default_value_for :repository_access_level, value: ENABLED, allows_nil: false - after_create ->(model) { SiteStatistic.track(STATISTICS_ATTRIBUTE) if model.wiki_enabled? } - after_update :update_site_statistics - def feature_available?(feature, user) get_permission(user, access_level(feature)) end @@ -82,30 +78,8 @@ class ProjectFeature < ActiveRecord::Base issues_access_level > DISABLED end - # This is a workaround for the removal hooks not been triggered when removing a Project. - # - # ProjectFeature is removed using database cascade index rule. - # This method is called by Project model when deletion starts. - def untrack_statistics_for_deletion! - return unless wiki_enabled? - - SiteStatistic.untrack(STATISTICS_ATTRIBUTE) - end - private - def update_site_statistics - return unless wiki_access_level_changed? - - if self.wiki_access_level_was == DISABLED - # possible new states are PRIVATE / ENABLED, both should be tracked - SiteStatistic.track(STATISTICS_ATTRIBUTE) - elsif self.wiki_access_level == DISABLED - # old state was either PRIVATE / ENABLED, only untrack if new state is DISABLED - SiteStatistic.untrack(STATISTICS_ATTRIBUTE) - end - end - # Validates builds and merge requests access level # which cannot be higher than repository access level def repository_children_level diff --git a/app/models/site_statistic.rb b/app/models/site_statistic.rb index 48324570f0b..3a7912ed53a 100644 --- a/app/models/site_statistic.rb +++ b/app/models/site_statistic.rb @@ -4,7 +4,7 @@ class SiteStatistic < ActiveRecord::Base # prevents the creation of multiple rows default_value_for :id, 1 - COUNTER_ATTRIBUTES = %w(repositories_count wikis_count).freeze + COUNTER_ATTRIBUTES = %w(repositories_count).freeze REQUIRED_SCHEMA_VERSION = 20180629153018 # Tracks specific attribute diff --git a/db/post_migrate/20180917172041_remove_wikis_count_from_site_statistics.rb b/db/post_migrate/20180917172041_remove_wikis_count_from_site_statistics.rb new file mode 100644 index 00000000000..0a39abe3bdf --- /dev/null +++ b/db/post_migrate/20180917172041_remove_wikis_count_from_site_statistics.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +class RemoveWikisCountFromSiteStatistics < ActiveRecord::Migration + def change + remove_column :site_statistics, :wikis_count, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 16e3c44513b..f92d8005dfb 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: 20180914201132) do +ActiveRecord::Schema.define(version: 20180917172041) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1908,7 +1908,6 @@ ActiveRecord::Schema.define(version: 20180914201132) do create_table "site_statistics", force: :cascade do |t| t.integer "repositories_count", default: 0, null: false - t.integer "wikis_count", default: 0, null: false end create_table "snippets", force: :cascade do |t| diff --git a/lib/tasks/gitlab/site_statistics.rake b/lib/tasks/gitlab/site_statistics.rake index 7d24ec72a9d..d97f11b2ed5 100644 --- a/lib/tasks/gitlab/site_statistics.rake +++ b/lib/tasks/gitlab/site_statistics.rake @@ -10,14 +10,6 @@ namespace :gitlab do SiteStatistic.update_all('repositories_count = (SELECT COUNT(*) FROM projects)') end puts 'OK!'.color(:green) - - print '* Wikis... ' - SiteStatistic.transaction do - # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967 - ActiveRecord::Base.connection.execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? - SiteStatistic.update_all('wikis_count = (SELECT COUNT(*) FROM project_features WHERE wiki_access_level != 0)') - end - puts 'OK!'.color(:green) puts end end diff --git a/spec/factories/site_statistics.rb b/spec/factories/site_statistics.rb index dd8c795515a..2533d0eecc2 100644 --- a/spec/factories/site_statistics.rb +++ b/spec/factories/site_statistics.rb @@ -2,6 +2,5 @@ FactoryBot.define do factory :site_statistics, class: 'SiteStatistic' do id 1 repositories_count 999 - wikis_count 555 end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 10617edec0f..cd7f77024da 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -119,46 +119,4 @@ describe ProjectFeature do end end end - - context 'Site Statistics' do - set(:project_with_wiki) { create(:project, :wiki_enabled) } - set(:project_without_wiki) { create(:project, :wiki_disabled) } - - context 'when creating a project' do - it 'tracks wiki availability when wikis are enabled by default' do - expect { create(:project) }.to change { SiteStatistic.fetch.wikis_count }.by(1) - end - - it 'does not track wiki availability when wikis are disabled by default' do - expect { create(:project, :wiki_disabled) }.not_to change { SiteStatistic.fetch.wikis_count } - end - end - - context 'when updating a project_feature' do - it 'untracks wiki availability when disabling wiki access' do - expect { project_with_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) } - .to change { SiteStatistic.fetch.wikis_count }.by(-1) - end - - it 'tracks again wiki availability when re-enabling wiki access as public' do - expect { project_without_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::ENABLED) } - .to change { SiteStatistic.fetch.wikis_count }.by(1) - end - - it 'tracks again wiki availability when re-enabling wiki access as private' do - expect { project_without_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::PRIVATE) } - .to change { SiteStatistic.fetch.wikis_count }.by(1) - end - end - - context 'when removing a project' do - it 'untracks wiki availability when removing a project with previous wiki access' do - expect { project_with_wiki.destroy }.to change { SiteStatistic.fetch.wikis_count }.by(-1) - end - - it 'does not untrack wiki availability when removing a project without wiki access' do - expect { project_without_wiki.destroy }.not_to change { SiteStatistic.fetch.wikis_count } - end - end - end end diff --git a/spec/models/site_statistic_spec.rb b/spec/models/site_statistic_spec.rb index 9b056fbf332..0e739900065 100644 --- a/spec/models/site_statistic_spec.rb +++ b/spec/models/site_statistic_spec.rb @@ -25,7 +25,6 @@ describe SiteStatistic do it 'increases the attribute counter' do expect { described_class.track('repositories_count') }.to change { statistics.reload.repositories_count }.by(1) - expect { described_class.track('wikis_count') }.to change { statistics.reload.wikis_count }.by(1) end it 'doesnt increase the attribute counter when an exception happens during transaction' do @@ -56,7 +55,6 @@ describe SiteStatistic do it 'decreases the attribute counter' do expect { described_class.untrack('repositories_count') }.to change { statistics.reload.repositories_count }.by(-1) - expect { described_class.untrack('wikis_count') }.to change { statistics.reload.wikis_count }.by(-1) end it 'doesnt decrease the attribute counter when an exception happens during transaction' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 97a88b5d697..d80d0f5a8a8 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -35,14 +35,6 @@ describe Groups::DestroyService do it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) } end - context 'site statistics' do - it 'doesnt trigger project deletion hooks twice' do - expect_any_instance_of(Project).to receive(:untrack_site_statistics).once - - destroy_group(group, user, async) - end - end - context 'mattermost team' do let!(:chat_team) { create(:chat_team, namespace: group) } diff --git a/spec/tasks/gitlab/site_statistics_rake_spec.rb b/spec/tasks/gitlab/site_statistics_rake_spec.rb index 20f0df65e63..c43ce25a540 100644 --- a/spec/tasks/gitlab/site_statistics_rake_spec.rb +++ b/spec/tasks/gitlab/site_statistics_rake_spec.rb @@ -6,7 +6,7 @@ describe 'rake gitlab:refresh_site_statistics' do Rake.application.rake_require 'tasks/gitlab/site_statistics' create(:project) - SiteStatistic.fetch.update(repositories_count: 0, wikis_count: 0) + SiteStatistic.fetch.update(repositories_count: 0) end let(:task) { 'gitlab:refresh_site_statistics' } @@ -15,10 +15,9 @@ describe 'rake gitlab:refresh_site_statistics' do run_rake_task(task) expect(SiteStatistic.fetch.repositories_count).to eq(1) - expect(SiteStatistic.fetch.wikis_count).to eq(1) end it 'displays message listing counters' do - expect { run_rake_task(task) }.to output(/Updating Site Statistics counters:.* Repositories\.\.\. OK!.* Wikis\.\.\. OK!/m).to_stdout + expect { run_rake_task(task) }.to output(/Updating Site Statistics counters:.* Repositories\.\.\. OK!/m).to_stdout end end |