Cleans up UpdateProjectStatistics concern
- Renames attributes from stat to project_statistiscs_name and attribute to statistic_attribute - Reordes methods on UpdateProjectStatistics concern - Removes unused module from Ci::Build
This commit is contained in:
parent
07630b3bdf
commit
4bf3f54607
|
@ -15,7 +15,6 @@ module Ci
|
||||||
include Gitlab::Utils::StrongMemoize
|
include Gitlab::Utils::StrongMemoize
|
||||||
include Deployable
|
include Deployable
|
||||||
include HasRef
|
include HasRef
|
||||||
include UpdateProjectStatistics
|
|
||||||
|
|
||||||
BuildArchivedError = Class.new(StandardError)
|
BuildArchivedError = Class.new(StandardError)
|
||||||
|
|
||||||
|
|
|
@ -59,7 +59,7 @@ module Ci
|
||||||
validate :valid_file_format?, unless: :trace?, on: :create
|
validate :valid_file_format?, unless: :trace?, on: :create
|
||||||
before_save :set_size, if: :file_changed?
|
before_save :set_size, if: :file_changed?
|
||||||
|
|
||||||
update_project_statistics stat: :build_artifacts_size
|
update_project_statistics project_statistics_name: :build_artifacts_size
|
||||||
|
|
||||||
after_save :update_file_store, if: :saved_change_to_file?
|
after_save :update_file_store, if: :saved_change_to_file?
|
||||||
|
|
||||||
|
|
|
@ -5,43 +5,47 @@
|
||||||
# It deals with `ProjectStatistics.increment_statistic` making sure not to update statistics on a cascade delete from the
|
# It deals with `ProjectStatistics.increment_statistic` making sure not to update statistics on a cascade delete from the
|
||||||
# project, and keeping track of value deltas on each save. It updates the DB only when a change is needed.
|
# project, and keeping track of value deltas on each save. It updates the DB only when a change is needed.
|
||||||
#
|
#
|
||||||
# How to use
|
# Example:
|
||||||
# - Invoke `update_project_statistics stat: :a_project_statistics_column, attribute: :an_attr_to_track` in a model class body.
|
|
||||||
#
|
#
|
||||||
# Expectation
|
# module Ci
|
||||||
# - `attribute` must be an ActiveRecord attribute
|
# class JobArtifact < ApplicationRecord
|
||||||
|
# include UpdateProjectStatistics
|
||||||
|
#
|
||||||
|
# update_project_statistics project_statistics_name: :build_artifacts_size
|
||||||
|
# end
|
||||||
|
# end
|
||||||
|
#
|
||||||
|
# Expectation:
|
||||||
|
#
|
||||||
|
# - `statistic_attribute` must be an ActiveRecord attribute
|
||||||
# - The model must implement `project` and `project_id`. i.e. direct Project relationship or delegation
|
# - The model must implement `project` and `project_id`. i.e. direct Project relationship or delegation
|
||||||
|
#
|
||||||
module UpdateProjectStatistics
|
module UpdateProjectStatistics
|
||||||
extend ActiveSupport::Concern
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
class_methods do
|
class_methods do
|
||||||
attr_reader :statistic_name, :statistic_attribute
|
attr_reader :project_statistics_name, :statistic_attribute
|
||||||
|
|
||||||
# Configure the model to update +stat+ on ProjectStatistics when +attribute+ changes
|
# Configure the model to update `project_statistics_name` on ProjectStatistics,
|
||||||
|
# when `statistic_attribute` changes
|
||||||
#
|
#
|
||||||
# +stat+:: a column of ProjectStatistics to update
|
# - project_statistics_name: A column of `ProjectStatistics` to update
|
||||||
# +attribute+:: an attribute of the current model, default to +:size+
|
# - statistic_attribute: An attribute of the current model, default to `size`
|
||||||
def update_project_statistics(stat:, attribute: :size)
|
#
|
||||||
@statistic_name = stat
|
def update_project_statistics(project_statistics_name:, statistic_attribute: :size)
|
||||||
@statistic_attribute = attribute
|
@project_statistics_name = project_statistics_name
|
||||||
|
@statistic_attribute = statistic_attribute
|
||||||
|
|
||||||
after_save(:update_project_statistics_after_save, if: :update_project_statistics_attribute_changed?)
|
after_save(:update_project_statistics_after_save, if: :update_project_statistics_attribute_changed?)
|
||||||
after_destroy(:update_project_statistics_after_destroy, unless: :project_destroyed?)
|
after_destroy(:update_project_statistics_after_destroy, unless: :project_destroyed?)
|
||||||
end
|
end
|
||||||
|
|
||||||
private :update_project_statistics
|
private :update_project_statistics
|
||||||
end
|
end
|
||||||
|
|
||||||
included do
|
included do
|
||||||
private
|
private
|
||||||
|
|
||||||
def project_destroyed?
|
|
||||||
project.pending_delete?
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_project_statistics_attribute_changed?
|
|
||||||
saved_change_to_attribute?(self.class.statistic_attribute)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_project_statistics_after_save
|
def update_project_statistics_after_save
|
||||||
attr = self.class.statistic_attribute
|
attr = self.class.statistic_attribute
|
||||||
delta = read_attribute(attr).to_i - attribute_before_last_save(attr).to_i
|
delta = read_attribute(attr).to_i - attribute_before_last_save(attr).to_i
|
||||||
|
@ -49,12 +53,20 @@ module UpdateProjectStatistics
|
||||||
update_project_statistics(delta)
|
update_project_statistics(delta)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def update_project_statistics_attribute_changed?
|
||||||
|
saved_change_to_attribute?(self.class.statistic_attribute)
|
||||||
|
end
|
||||||
|
|
||||||
def update_project_statistics_after_destroy
|
def update_project_statistics_after_destroy
|
||||||
update_project_statistics(-read_attribute(self.class.statistic_attribute).to_i)
|
update_project_statistics(-read_attribute(self.class.statistic_attribute).to_i)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def project_destroyed?
|
||||||
|
project.pending_delete?
|
||||||
|
end
|
||||||
|
|
||||||
def update_project_statistics(delta)
|
def update_project_statistics(delta)
|
||||||
ProjectStatistics.increment_statistic(project_id, self.class.statistic_name, delta)
|
ProjectStatistics.increment_statistic(project_id, self.class.project_statistics_name, delta)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -5,10 +5,6 @@ require 'spec_helper'
|
||||||
describe Ci::JobArtifact do
|
describe Ci::JobArtifact do
|
||||||
let(:artifact) { create(:ci_job_artifact, :archive) }
|
let(:artifact) { create(:ci_job_artifact, :archive) }
|
||||||
|
|
||||||
it_behaves_like 'UpdateProjectStatistics' do
|
|
||||||
subject { build(:ci_job_artifact, :archive, size: 106365) }
|
|
||||||
end
|
|
||||||
|
|
||||||
describe "Associations" do
|
describe "Associations" do
|
||||||
it { is_expected.to belong_to(:project) }
|
it { is_expected.to belong_to(:project) }
|
||||||
it { is_expected.to belong_to(:job) }
|
it { is_expected.to belong_to(:job) }
|
||||||
|
@ -23,6 +19,10 @@ describe Ci::JobArtifact do
|
||||||
|
|
||||||
it_behaves_like 'having unique enum values'
|
it_behaves_like 'having unique enum values'
|
||||||
|
|
||||||
|
it_behaves_like 'UpdateProjectStatistics' do
|
||||||
|
subject { build(:ci_job_artifact, :archive, size: 106365) }
|
||||||
|
end
|
||||||
|
|
||||||
describe '.with_reports' do
|
describe '.with_reports' do
|
||||||
let!(:artifact) { create(:ci_job_artifact, :archive) }
|
let!(:artifact) { create(:ci_job_artifact, :archive) }
|
||||||
|
|
||||||
|
|
|
@ -3,16 +3,16 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
shared_examples_for 'UpdateProjectStatistics' do
|
shared_examples_for 'UpdateProjectStatistics' do
|
||||||
let(:project) { subject.project }
|
let(:project) { subject.project }
|
||||||
let(:stat) { described_class.statistic_name }
|
let(:project_statistics_name) { described_class.project_statistics_name }
|
||||||
let(:attribute) { described_class.statistic_attribute }
|
let(:statistic_attribute) { described_class.statistic_attribute }
|
||||||
|
|
||||||
def reload_stat
|
def reload_stat
|
||||||
project.statistics.reload.send(stat).to_i
|
project.statistics.reload.send(project_statistics_name).to_i
|
||||||
end
|
end
|
||||||
|
|
||||||
def read_attribute
|
def read_attribute
|
||||||
subject.read_attribute(attribute).to_i
|
subject.read_attribute(statistic_attribute).to_i
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.to be_new_record }
|
it { is_expected.to be_new_record }
|
||||||
|
@ -39,7 +39,8 @@ shared_examples_for 'UpdateProjectStatistics' do
|
||||||
.to receive(:increment_statistic)
|
.to receive(:increment_statistic)
|
||||||
.and_call_original
|
.and_call_original
|
||||||
|
|
||||||
subject.write_attribute(attribute, read_attribute + delta)
|
subject.write_attribute(statistic_attribute, read_attribute + delta)
|
||||||
|
|
||||||
expect { subject.save! }
|
expect { subject.save! }
|
||||||
.to change { reload_stat }
|
.to change { reload_stat }
|
||||||
.by(delta)
|
.by(delta)
|
||||||
|
|
Loading…
Reference in New Issue