Add packages_size to ProjectStatistics

This new field will allow to keep track of the storage used by the
packages features, it provides also aggregation at namespace level.

12 files changed, 96 insertions, 30 deletions

diff --git a/app/helpers/storage_helper.rb b/app/helpers/storage_helper.rb
index be8761db562..15041bd5805 100644
--- a/app/helpers/storage_helper.rb
+++ b/app/helpers/storage_helper.rb
@@ -6,4 +6,14 @@ module StorageHelper
 
     number_to_human_size(size_in_bytes, delimiter: ',', precision: precision, significant: false)
   end
+
+  def storage_counters_details(statistics)
+    counters = {
+      counter_repositories: storage_counter(statistics.repository_size),
+      counter_build_artifacts: storage_counter(statistics.build_artifacts_size),
+      counter_lfs_objects: storage_counter(statistics.lfs_objects_size)
+    }
+
+    _("%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS") % counters
+  end
 end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 168f6bedd63..7393ef4b05c 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -77,7 +77,8 @@ class Namespace < ApplicationRecord
         'COALESCE(SUM(ps.storage_size), 0) AS storage_size',
         'COALESCE(SUM(ps.repository_size), 0) AS repository_size',
         'COALESCE(SUM(ps.lfs_objects_size), 0) AS lfs_objects_size',
-        'COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size'
+        'COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size',
+        'COALESCE(SUM(ps.packages_size), 0) AS packages_size'
       )
   end
 
diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb
index c020e72908c..6fe8cb40d25 100644
--- a/app/models/project_statistics.rb
+++ b/app/models/project_statistics.rb
@@ -7,7 +7,7 @@ class ProjectStatistics < ApplicationRecord
   before_save :update_storage_size
 
   COLUMNS_TO_REFRESH = [:repository_size, :lfs_objects_size, :commit_count].freeze
-  INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size] }.freeze
+  INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size], packages_size: %i[storage_size] }.freeze
 
   def total_repository_size
     repository_size + lfs_objects_size
@@ -36,8 +36,13 @@ class ProjectStatistics < ApplicationRecord
     self.lfs_objects_size = project.lfs_objects.sum(:size)
   end
 
+  # older migrations fail due to non-existent attribute without this
+  def packages_size
+    has_attribute?(:packages_size) ? super.to_i : 0
+  end
+
   def update_storage_size
-    self.storage_size = repository_size + lfs_objects_size + build_artifacts_size
+    self.storage_size = repository_size + lfs_objects_size + build_artifacts_size + packages_size
   end
 
   # Since this incremental update method does not call update_storage_size above,
diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml
index 00d255846f9..f524d35d79e 100644
--- a/app/views/admin/groups/show.html.haml
+++ b/app/views/admin/groups/show.html.haml
@@ -44,12 +44,10 @@
 
         %li
           %span.light= _('Storage:')
-          - counter_storage = storage_counter(@group.storage_size)
-          - counter_repositories = storage_counter(@group.repository_size)
-          - counter_build_artifacts = storage_counter(@group.build_artifacts_size)
-          - counter_lfs_objects = storage_counter(@group.lfs_objects_size)
-          %strong
-            = _("%{counter_storage} (%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS)") % { counter_storage: counter_storage, counter_repositories: counter_repositories, counter_build_artifacts: counter_build_artifacts, counter_lfs_objects: counter_lfs_objects }
+          %strong= storage_counter(@group.storage_size)
+          (
+          = storage_counters_details(@group)
+          )
 
         %li
           %span.light= _('Group Git LFS status:')
diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml
index 03cce4745aa..bc34af88928 100644
--- a/app/views/admin/projects/show.html.haml
+++ b/app/views/admin/projects/show.html.haml
@@ -73,15 +73,10 @@
               = @project.repository.relative_path
 
           %li
-            %span.light Storage used:
+            %span.light= _('Storage:')
             %strong= storage_counter(@project.statistics.storage_size)
             (
-            = storage_counter(@project.statistics.repository_size)
-            repository,
-            = storage_counter(@project.statistics.build_artifacts_size)
-            build artifacts,
-            = storage_counter(@project.statistics.lfs_objects_size)
-            LFS
+            = storage_counters_details(@project.statistics)
             )
 
           %li
diff --git a/changelogs/unreleased/ac-package-storage-stats.yml b/changelogs/unreleased/ac-package-storage-stats.yml
new file mode 100644
index 00000000000..fedffb41597
--- /dev/null
+++ b/changelogs/unreleased/ac-package-storage-stats.yml
@@ -0,0 +1,5 @@
+---
+title: Add packages_size to ProjectStatistics
+merge_request: 27373
+author:
+type: added
diff --git a/db/migrate/20190415095825_add_packages_size_to_project_statistics.rb b/db/migrate/20190415095825_add_packages_size_to_project_statistics.rb
new file mode 100644
index 00000000000..99625981563
--- /dev/null
+++ b/db/migrate/20190415095825_add_packages_size_to_project_statistics.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddPackagesSizeToProjectStatistics < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :project_statistics, :packages_size, :bigint
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2e77bbb51e5..11230d4e691 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1715,6 +1715,7 @@ ActiveRecord::Schema.define(version: 20190426180107) do
     t.bigint "repository_size", default: 0, null: false
     t.bigint "lfs_objects_size", default: 0, null: false
     t.bigint "build_artifacts_size", default: 0, null: false
+    t.bigint "packages_size"
     t.index ["namespace_id"], name: "index_project_statistics_on_namespace_id", using: :btree
     t.index ["project_id"], name: "index_project_statistics_on_project_id", unique: true, using: :btree
   end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index cae94fb2af4..d1b960a4b26 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -110,7 +110,7 @@ msgstr ""
 msgid "%{commit_author_link} authored %{commit_timeago}"
 msgstr ""
 
-msgid "%{counter_storage} (%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS)"
+msgid "%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS"
 msgstr ""
 
 msgid "%{count} more"
diff --git a/spec/helpers/storage_helper_spec.rb b/spec/helpers/storage_helper_spec.rb
index 03df9deafa1..50c74a7c2f9 100644
--- a/spec/helpers/storage_helper_spec.rb
+++ b/spec/helpers/storage_helper_spec.rb
@@ -18,4 +18,28 @@ describe StorageHelper do
       expect(helper.storage_counter(100_000_000_000_000_000_000_000)).to eq("86,736.2 EB")
     end
   end
+
+  describe "#storage_counters_details" do
+    let(:namespace) { create :namespace }
+    let(:project) do
+      create(:project,
+             namespace: namespace,
+             statistics: build(:project_statistics,
+                               repository_size:      10.kilobytes,
+                               lfs_objects_size:     20.gigabytes,
+                               build_artifacts_size: 30.megabytes))
+    end
+
+    let(:message) { '10 KB repositories, 30 MB build artifacts, 20 GB LFS' }
+
+    it 'works on ProjectStatistics' do
+      expect(helper.storage_counters_details(project.statistics)).to eq(message)
+    end
+
+    it 'works on Namespace.with_statistics' do
+      namespace_stats = Namespace.with_statistics.find(project.namespace.id)
+
+      expect(helper.storage_counters_details(namespace_stats)).to eq(message)
+    end
+  end
 end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index dd5edca5059..bfde367c47f 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -146,20 +146,20 @@ describe Namespace do
       create(:project,
              namespace: namespace,
              statistics: build(:project_statistics,
-                               storage_size:         606,
                                repository_size:      101,
                                lfs_objects_size:     202,
-                               build_artifacts_size: 303))
+                               build_artifacts_size: 303,
+                               packages_size:        404))
     end
 
     let(:project2) do
       create(:project,
              namespace: namespace,
              statistics: build(:project_statistics,
-                               storage_size:         60,
                                repository_size:      10,
                                lfs_objects_size:     20,
-                               build_artifacts_size: 30))
+                               build_artifacts_size: 30,
+                               packages_size:        40))
     end
 
     it "sums all project storage counters in the namespace" do
@@ -167,10 +167,11 @@ describe Namespace do
       project2
       statistics = described_class.with_statistics.find(namespace.id)
 
-      expect(statistics.storage_size).to eq 666
+      expect(statistics.storage_size).to eq 1110
       expect(statistics.repository_size).to eq 111
       expect(statistics.lfs_objects_size).to eq 222
       expect(statistics.build_artifacts_size).to eq 333
+      expect(statistics.packages_size).to eq 444
     end
 
     it "correctly handles namespaces without projects" do
@@ -180,6 +181,7 @@ describe Namespace do
       expect(statistics.repository_size).to eq 0
       expect(statistics.lfs_objects_size).to eq 0
       expect(statistics.build_artifacts_size).to eq 0
+      expect(statistics.packages_size).to eq 0
     end
   end
 
diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb
index c670b6aac56..738398a06f9 100644
--- a/spec/models/project_statistics_spec.rb
+++ b/spec/models/project_statistics_spec.rb
@@ -124,16 +124,30 @@ describe ProjectStatistics do
   end
 
   describe '.increment_statistic' do
-    it 'increases the statistic by that amount' do
-      expect { described_class.increment_statistic(project.id, :build_artifacts_size, 13) }
-        .to change { statistics.reload.build_artifacts_size }
-        .by(13)
+    shared_examples 'a statistic that increases storage_size' do
+      it 'increases the statistic by that amount' do
+        expect { described_class.increment_statistic(project.id, stat, 13) }
+          .to change { statistics.reload.send(stat) || 0 }
+          .by(13)
+      end
+
+      it 'increases also storage size by that amount' do
+        expect { described_class.increment_statistic(project.id, stat, 20) }
+         .to change { statistics.reload.storage_size }
+         .by(20)
+      end
     end
 
-    it 'increases also storage size by that amount' do
-      expect { described_class.increment_statistic(project.id, :build_artifacts_size, 20) }
-        .to change { statistics.reload.storage_size }
-        .by(20)
+    context 'when adjusting :build_artifacts_size' do
+      let(:stat) { :build_artifacts_size }
+
+      it_behaves_like 'a statistic that increases storage_size'
+    end
+
+    context 'when adjusting :packages_size' do
+      let(:stat) { :packages_size }
+
+      it_behaves_like 'a statistic that increases storage_size'
     end
 
     context 'when the amount is 0' do