diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index f743fca423a..aaa326afea5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -15,7 +15,6 @@ module Ci include Gitlab::Utils::StrongMemoize include Deployable include HasRef - include UpdateProjectStatistics BuildArchivedError = Class.new(StandardError) diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 0dbeab30498..f80e98e5bca 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -59,7 +59,7 @@ module Ci validate :valid_file_format?, unless: :trace?, on: :create 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? diff --git a/app/models/concerns/update_project_statistics.rb b/app/models/concerns/update_project_statistics.rb index 67e1f0ec930..1f881249322 100644 --- a/app/models/concerns/update_project_statistics.rb +++ b/app/models/concerns/update_project_statistics.rb @@ -5,43 +5,47 @@ # 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. # -# How to use -# - Invoke `update_project_statistics stat: :a_project_statistics_column, attribute: :an_attr_to_track` in a model class body. +# Example: # -# Expectation -# - `attribute` must be an ActiveRecord attribute +# module Ci +# 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 +# module UpdateProjectStatistics extend ActiveSupport::Concern 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 - # +attribute+:: an attribute of the current model, default to +:size+ - def update_project_statistics(stat:, attribute: :size) - @statistic_name = stat - @statistic_attribute = attribute + # - project_statistics_name: A column of `ProjectStatistics` to update + # - statistic_attribute: An attribute of the current model, default to `size` + # + def update_project_statistics(project_statistics_name:, statistic_attribute: :size) + @project_statistics_name = project_statistics_name + @statistic_attribute = statistic_attribute after_save(:update_project_statistics_after_save, if: :update_project_statistics_attribute_changed?) after_destroy(:update_project_statistics_after_destroy, unless: :project_destroyed?) end + private :update_project_statistics end included do 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 attr = self.class.statistic_attribute delta = read_attribute(attr).to_i - attribute_before_last_save(attr).to_i @@ -49,12 +53,20 @@ module UpdateProjectStatistics update_project_statistics(delta) end + def update_project_statistics_attribute_changed? + saved_change_to_attribute?(self.class.statistic_attribute) + end + def update_project_statistics_after_destroy update_project_statistics(-read_attribute(self.class.statistic_attribute).to_i) end + def project_destroyed? + project.pending_delete? + end + 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 diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index e6d682c24d9..1ba66565e03 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -5,10 +5,6 @@ require 'spec_helper' describe Ci::JobArtifact do let(:artifact) { create(:ci_job_artifact, :archive) } - it_behaves_like 'UpdateProjectStatistics' do - subject { build(:ci_job_artifact, :archive, size: 106365) } - end - describe "Associations" do it { is_expected.to belong_to(:project) } 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 'UpdateProjectStatistics' do + subject { build(:ci_job_artifact, :archive, size: 106365) } + end + describe '.with_reports' do let!(:artifact) { create(:ci_job_artifact, :archive) } diff --git a/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb b/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb index 7a04e940ee5..1b09c3dd636 100644 --- a/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb +++ b/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb @@ -3,16 +3,16 @@ require 'spec_helper' shared_examples_for 'UpdateProjectStatistics' do - let(:project) { subject.project } - let(:stat) { described_class.statistic_name } - let(:attribute) { described_class.statistic_attribute } + let(:project) { subject.project } + let(:project_statistics_name) { described_class.project_statistics_name } + let(:statistic_attribute) { described_class.statistic_attribute } def reload_stat - project.statistics.reload.send(stat).to_i + project.statistics.reload.send(project_statistics_name).to_i end def read_attribute - subject.read_attribute(attribute).to_i + subject.read_attribute(statistic_attribute).to_i end it { is_expected.to be_new_record } @@ -39,7 +39,8 @@ shared_examples_for 'UpdateProjectStatistics' do .to receive(:increment_statistic) .and_call_original - subject.write_attribute(attribute, read_attribute + delta) + subject.write_attribute(statistic_attribute, read_attribute + delta) + expect { subject.save! } .to change { reload_stat } .by(delta)