Read about our upcoming Code of Conduct on this issue

Commit 1c69d919 authored by Gabriel Mazetto's avatar Gabriel Mazetto
Browse files

Added SiteStatistics as counter cache for Projects and Wikis

parent aedd78a07b58
...@@ -31,6 +31,7 @@ class Project < ActiveRecord::Base ...@@ -31,6 +31,7 @@ class Project < ActiveRecord::Base
BoardLimitExceeded = Class.new(StandardError) BoardLimitExceeded = Class.new(StandardError)
STATISTICS_ATTRIBUTE = 'repositories_count'.freeze
NUMBER_OF_PERMITTED_BOARDS = 1 NUMBER_OF_PERMITTED_BOARDS = 1
UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze
# Hashed Storage versions handle rolling out new storage to project and dependents models: # Hashed Storage versions handle rolling out new storage to project and dependents models:
...@@ -79,6 +80,10 @@ class Project < ActiveRecord::Base ...@@ -79,6 +80,10 @@ class Project < ActiveRecord::Base
after_create :create_project_feature, unless: :project_feature after_create :create_project_feature, unless: :project_feature
after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) }
before_destroy ->(project) { project.project_feature.untrack_statistics_for_deletion! }
after_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) }
after_create :create_ci_cd_settings, after_create :create_ci_cd_settings,
unless: :ci_cd_settings, unless: :ci_cd_settings,
if: proc { ProjectCiCdSetting.available? } if: proc { ProjectCiCdSetting.available? }
......
...@@ -19,6 +19,7 @@ class ProjectFeature < ActiveRecord::Base ...@@ -19,6 +19,7 @@ class ProjectFeature < ActiveRecord::Base
ENABLED = 20 ENABLED = 20
FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze
STATISTICS_ATTRIBUTE = 'wikis_count'.freeze
class << self class << self
def access_level_attribute(feature) def access_level_attribute(feature)
...@@ -52,6 +53,9 @@ def quoted_access_level_column(feature) ...@@ -52,6 +53,9 @@ def quoted_access_level_column(feature)
default_value_for :wiki_access_level, value: ENABLED, allows_nil: false default_value_for :wiki_access_level, value: ENABLED, allows_nil: false
default_value_for :repository_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) def feature_available?(feature, user)
get_permission(user, access_level(feature)) get_permission(user, access_level(feature))
end end
...@@ -76,8 +80,30 @@ def issues_enabled? ...@@ -76,8 +80,30 @@ def issues_enabled?
issues_access_level > DISABLED issues_access_level > DISABLED
end 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 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 # Validates builds and merge requests access level
# which cannot be higher than repository access level # which cannot be higher than repository access level
def repository_children_level def repository_children_level
......
class SiteStatistic < ActiveRecord::Base
# prevents the creation of multiple rows
default_value_for :id, 1
COUNTER_ATTRIBUTES = %w(repositories_count wikis_count).freeze
# https://dev.mysql.com/doc/refman/5.5/en/error-messages-server.html
MYSQL_NO_SUCH_TABLE = 1146
def self.track(raw_attribute)
with_statistics_available(raw_attribute) do |attribute|
SiteStatistic.update_all(["#{attribute} = #{attribute}+1"])
end
end
def self.untrack(raw_attribute)
with_statistics_available(raw_attribute) do |attribute|
SiteStatistic.update_all(["#{attribute} = #{attribute}-1 WHERE #{attribute} > 0"])
end
end
def self.with_statistics_available(raw_attribute)
unless raw_attribute.in?(COUNTER_ATTRIBUTES)
raise ArgumentError, "Invalid attribute: '#{raw_attribute}' to '#{caller_locations(1, 1)[0].label}' method. " \
"Valid attributes are: #{COUNTER_ATTRIBUTES.join(', ')}"
end
# we have quite a lot of specs testing migrations, we need this and the rescue to not break them
SiteStatistic.transaction(requires_new: true) do
SiteStatistic.first_or_create if Rails.env.test? # make sure SiteStatistic exists during tests
attribute = self.connection.quote_column_name(raw_attribute)
yield(attribute)
end
rescue ActiveRecord::StatementInvalid => ex
# we want to ignore this so we don't break the migration specs
unless ex.original_exception.is_a?(PG::UndefinedTable) ||
(ex.original_exception.is_a?(Mysql2::Error) && ex.original_exception.error_number == MYSQL_NO_SUCH_TABLE)
raise ex
end
end
def self.fetch
SiteStatistic.first_or_create!
end
end
class CreateSiteStatistics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
create_table :site_statistics do |t|
t.integer :repositories_count, default: 0, null: false
t.integer :wikis_count, default: 0, null: false
end
execute('INSERT INTO site_statistics (id) VALUES(1)')
end
def down
drop_table :site_statistics
end
end
class PopulateSiteStatistics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
transaction do
execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967
execute("UPDATE site_statistics SET repositories_count = (SELECT COUNT(*) FROM projects)")
end
transaction do
execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967
execute("UPDATE site_statistics SET wikis_count = (SELECT COUNT(*) FROM project_features WHERE wiki_access_level != 0)")
end
end
def down
# No downside in keeping the counter up-to-date
end
end
...@@ -1841,6 +1841,11 @@ ...@@ -1841,6 +1841,11 @@
add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
add_index "services", ["template"], name: "index_services_on_template", using: :btree add_index "services", ["template"], name: "index_services_on_template", using: :btree
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| create_table "snippets", force: :cascade do |t|
t.string "title" t.string "title"
t.text "content" t.text "content"
......
...@@ -34,7 +34,7 @@ ...@@ -34,7 +34,7 @@
builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min
merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min
project.project_feature.update_columns( project.project_feature.update(
wiki_access_level: evaluator.wiki_access_level, wiki_access_level: evaluator.wiki_access_level,
builds_access_level: builds_access_level, builds_access_level: builds_access_level,
snippets_access_level: evaluator.snippets_access_level, snippets_access_level: evaluator.snippets_access_level,
......
FactoryBot.define do
factory :site_statistics, class: 'SiteStatistic' do
id 1
repositories_count 999
wikis_count 555
end
end
...@@ -119,4 +119,46 @@ ...@@ -119,4 +119,46 @@
end end
end 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 end
...@@ -102,6 +102,22 @@ ...@@ -102,6 +102,22 @@
end end
end end
context 'Site Statistics' do
context 'when creating a new project' do
it 'tracks project in SiteStatistic' do
expect { create(:project) }.to change { SiteStatistic.fetch.repositories_count }.by(1)
end
end
context 'when deleting a project' do
it 'untracks project in SiteStatistic' do
project = create(:project)
expect { project.destroy }.to change { SiteStatistic.fetch.repositories_count }.by(-1)
end
end
end
context 'updating cd_cd_settings' do context 'updating cd_cd_settings' do
it 'does not raise an error' do it 'does not raise an error' do
project = create(:project) project = create(:project)
......
require 'spec_helper'
describe SiteStatistic do
describe '.fetch' do
context 'existing record' do
it 'returns existing SiteStatistic model' do
statistics = create(:site_statistics)
expect(described_class.fetch).to be_a(described_class)
expect(described_class.fetch).to eq(statistics)
end
end
context 'non existing record' do
it 'creates a new SiteStatistic model' do
expect(described_class.first).to be_nil
expect(described_class.fetch).to be_a(described_class)
end
end
end
describe '.track' do
context 'with allowed attributes' do
let(:statistics) { create(:site_statistics) }
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
expect do
begin
described_class.transaction do
described_class.track('repositories_count')
raise StandardError
end
rescue StandardError
# no-op
end
end.not_to change { statistics.reload.repositories_count }
end
end
context 'with not allowed attributes' do
it 'returns error' do
expect { described_class.track('something_else') }.to raise_error(ArgumentError).with_message(/Invalid attribute: \'something_else\' to \'track\' method/)
end
end
end
describe '.untrack' do
context 'with allowed attributes' do
let(:statistics) { create(:site_statistics) }
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
expect do
begin
described_class.transaction do
described_class.track('repositories_count')
raise StandardError
end
rescue StandardError
# no-op
end
end.not_to change { described_class.fetch.repositories_count }
end
end
context 'with not allowed attributes' do
it 'returns error' do
expect { described_class.untrack('something_else') }.to raise_error(ArgumentError).with_message(/Invalid attribute: \'something_else\' to \'untrack\' method/)
end
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment