Commit 72f5dc0a authored by Giorgenes Gelatti's avatar Giorgenes Gelatti

Fix package_file storage computation

When package_files are created without
an explicit size, the size is updated
on an after_save hook by copying over
the size from the file object.

This is called after the statistics
hook therefore the delta is not
added to the statistics.

This fixes that issue and updates
the statistic spec to be more robust.
parent c76fbee3
...@@ -37,11 +37,12 @@ class Packages::PackageFile < ApplicationRecord ...@@ -37,11 +37,12 @@ class Packages::PackageFile < ApplicationRecord
update_project_statistics project_statistics_name: :packages_size update_project_statistics project_statistics_name: :packages_size
before_save :update_size_from_file
def update_file_metadata def update_file_metadata
# The file.object_store is set during `uploader.store!` # The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated # which happens after object is inserted/updated
self.update_column(:file_store, file.object_store) self.update_column(:file_store, file.object_store)
self.update_column(:size, file.size) unless file.size == self.size
end end
def download_path def download_path
...@@ -51,6 +52,12 @@ class Packages::PackageFile < ApplicationRecord ...@@ -51,6 +52,12 @@ class Packages::PackageFile < ApplicationRecord
def local? def local?
file_store == ::Packages::PackageFileUploader::Store::LOCAL file_store == ::Packages::PackageFileUploader::Store::LOCAL
end end
private
def update_size_from_file
self.size ||= file.size
end
end end
Packages::PackageFile.prepend_if_ee('EE::Packages::PackageFileGeo') Packages::PackageFile.prepend_if_ee('EE::Packages::PackageFileGeo')
---
title: Fix Pypi and Nuget Storage Statistics
merge_request: 37386
author:
type: fixed
...@@ -32,11 +32,17 @@ RSpec.describe Packages::PackageFile, type: :model do ...@@ -32,11 +32,17 @@ RSpec.describe Packages::PackageFile, type: :model do
end end
end end
it_behaves_like 'UpdateProjectStatistics' do context 'updating project statistics' do
subject { build(:package_file, :jar, size: 42) } context 'when the package file has an explicit size' do
it_behaves_like 'UpdateProjectStatistics' do
subject { build(:package_file, :jar, size: 42) }
end
end
before do context 'when the package file does not have a size' do
allow_any_instance_of(Packages::PackageFileUploader).to receive(:size).and_return(42) it_behaves_like 'UpdateProjectStatistics' do
subject { build(:package_file, size: nil) }
end
end end
end end
......
...@@ -17,11 +17,14 @@ RSpec.shared_examples 'UpdateProjectStatistics' do ...@@ -17,11 +17,14 @@ RSpec.shared_examples 'UpdateProjectStatistics' do
context 'when creating' do context 'when creating' do
it 'updates the project statistics' do it 'updates the project statistics' do
delta = read_attribute delta0 = reload_stat
expect { subject.save! } subject.save!
.to change { reload_stat }
.by(delta) delta1 = reload_stat
expect(delta1).to eq(delta0 + read_attribute)
expect(delta1).to be > delta0
end end
it 'schedules a namespace statistics worker' do it 'schedules a namespace statistics worker' do
...@@ -80,15 +83,14 @@ RSpec.shared_examples 'UpdateProjectStatistics' do ...@@ -80,15 +83,14 @@ RSpec.shared_examples 'UpdateProjectStatistics' do
end end
it 'updates the project statistics' do it 'updates the project statistics' do
delta = -read_attribute delta0 = reload_stat
expect(ProjectStatistics) subject.destroy!
.to receive(:increment_statistic)
.and_call_original
expect { subject.destroy! } delta1 = reload_stat
.to change { reload_stat }
.by(delta) expect(delta1).to eq(delta0 - read_attribute)
expect(delta1).to be < delta0
end end
it 'schedules a namespace statistics worker' do it 'schedules a namespace statistics worker' do
......
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