Commit c591b430 authored by Tyler Amos's avatar Tyler Amos

Consolidate namespace storage app setting check

Includes the automatic_purchased_storage_allocation? app setting check
in Namespace#additional_repo_storage_by_namespace_enabled?. This
allows for more consolidation of the app setting code and ensure
consistency across the app where the namespace storage feature is
gated.  The code can be simplified, especially in tests.
parent 2c4cdee7
...@@ -47,8 +47,7 @@ module EE ...@@ -47,8 +47,7 @@ module EE
end end
def can_purchase_storage_for_namespace?(namespace) def can_purchase_storage_for_namespace?(namespace)
::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? && ::Feature.enabled?(:buy_storage_link) &&
::Feature.enabled?(:buy_storage_link) &&
namespace.additional_repo_storage_by_namespace_enabled? namespace.additional_repo_storage_by_namespace_enabled?
end end
......
...@@ -373,7 +373,9 @@ module EE ...@@ -373,7 +373,9 @@ module EE
end end
def additional_repo_storage_by_namespace_enabled? def additional_repo_storage_by_namespace_enabled?
!::Feature.enabled?(:namespace_storage_limit, self) && ::Feature.enabled?(:additional_repo_storage_by_namespace, self) !::Feature.enabled?(:namespace_storage_limit, self) &&
::Feature.enabled?(:additional_repo_storage_by_namespace, self) &&
::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
end end
def root_storage_size def root_storage_size
......
...@@ -32,10 +32,7 @@ module EE ...@@ -32,10 +32,7 @@ module EE
end end
def enforce_limit? def enforce_limit?
return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? root_namespace.additional_repo_storage_by_namespace_enabled?
return false unless root_namespace.additional_repo_storage_by_namespace_enabled?
true
end end
private private
......
...@@ -26,8 +26,6 @@ module EE ...@@ -26,8 +26,6 @@ module EE
private private
def additional_repo_storage_available? def additional_repo_storage_available?
return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
!!namespace&.additional_repo_storage_by_namespace_enabled? !!namespace&.additional_repo_storage_by_namespace_enabled?
end end
......
...@@ -9,13 +9,16 @@ RSpec.describe Groups::UsageQuotasController do ...@@ -9,13 +9,16 @@ RSpec.describe Groups::UsageQuotasController do
before do before do
sign_in(user) sign_in(user)
group.add_owner(user) group.add_owner(user)
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:additional_repo_storage_by_namespace_enabled?)
.and_return(additional_repo_storage_by_namespace_enabled)
end
end end
describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do
context 'when both flags are true' do context 'when additional_repo_storage_by_namespace_enabled is false' do
before do let(:additional_repo_storage_by_namespace_enabled) { false }
stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: true)
end
it 'is disabled' do it 'is disabled' do
get :index, params: { group_id: group } get :index, params: { group_id: group }
...@@ -24,10 +27,8 @@ RSpec.describe Groups::UsageQuotasController do ...@@ -24,10 +27,8 @@ RSpec.describe Groups::UsageQuotasController do
end end
end end
context 'when `namespace_storage_limit` flag is false' do context 'when additional_repo_storage_by_namespace_enabled is true' do
before do let(:additional_repo_storage_by_namespace_enabled) { true }
stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: false)
end
it 'is enabled' do it 'is enabled' do
get :index, params: { group_id: group } get :index, params: { group_id: group }
...@@ -35,17 +36,5 @@ RSpec.describe Groups::UsageQuotasController do ...@@ -35,17 +36,5 @@ RSpec.describe Groups::UsageQuotasController do
expect(Gon.features).to include('additionalRepoStorageByNamespace' => true) expect(Gon.features).to include('additionalRepoStorageByNamespace' => true)
end end
end end
context 'when both flags are false' do
before do
stub_feature_flags(additional_repo_storage_by_namespace: false, namespace_storage_limit: false)
end
it 'is disabled' do
get :index, params: { group_id: group }
expect(Gon.features).to include('additionalRepoStorageByNamespace' => false)
end
end
end end
end end
...@@ -18,10 +18,15 @@ RSpec.describe Profiles::UsageQuotasController do ...@@ -18,10 +18,15 @@ RSpec.describe Profiles::UsageQuotasController do
end end
describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do
context 'when both flags are true' do before do
before do allow_next_found_instance_of(Namespace) do |namespace|
stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: true) allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
.and_return(additional_repo_storage_by_namespace_enabled)
end end
end
context 'when additional_repo_storage_by_namespace_enabled is false' do
let(:additional_repo_storage_by_namespace_enabled) { false }
it 'is disabled' do it 'is disabled' do
get :index get :index
...@@ -30,10 +35,8 @@ RSpec.describe Profiles::UsageQuotasController do ...@@ -30,10 +35,8 @@ RSpec.describe Profiles::UsageQuotasController do
end end
end end
context 'when `namespace_storage_limit` flag is false' do context 'when additional_repo_storage_by_namespace_enabled is true' do
before do let(:additional_repo_storage_by_namespace_enabled) { true }
stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: false)
end
it 'is enabled' do it 'is enabled' do
get :index get :index
...@@ -41,17 +44,5 @@ RSpec.describe Profiles::UsageQuotasController do ...@@ -41,17 +44,5 @@ RSpec.describe Profiles::UsageQuotasController do
expect(Gon.features).to include('additionalRepoStorageByNamespace' => true) expect(Gon.features).to include('additionalRepoStorageByNamespace' => true)
end end
end end
context 'when both flags are false' do
before do
stub_feature_flags(additional_repo_storage_by_namespace: false, namespace_storage_limit: false)
end
it 'is disabled' do
get :index
expect(Gon.features).to include('additionalRepoStorageByNamespace' => false)
end
end
end end
end end
...@@ -112,17 +112,15 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do ...@@ -112,17 +112,15 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do
} }
end end
where(:namespace_storage_limit_enabled, :additional_repo_storage_by_namespace_enabled, :service_class_name) do where(:additional_repo_storage_by_namespace_enabled, :service_class_name) do
true | false | Namespaces::CheckStorageSizeService false | Namespaces::CheckStorageSizeService
true | true | Namespaces::CheckStorageSizeService true | Namespaces::CheckExcessStorageSizeService
false | true | Namespaces::CheckExcessStorageSizeService
false | false | Namespaces::CheckStorageSizeService
end end
with_them do with_them do
before do before do
stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled) allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
stub_feature_flags(additional_repo_storage_by_namespace: additional_repo_storage_by_namespace_enabled) .and_return(additional_repo_storage_by_namespace_enabled)
allow(helper).to receive(:current_user).and_return(admin) allow(helper).to receive(:current_user).and_return(admin)
allow_next_instance_of(service_class_name, namespace, admin) do |service| allow_next_instance_of(service_class_name, namespace, admin) do |service|
...@@ -203,28 +201,18 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do ...@@ -203,28 +201,18 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do
let_it_be(:namespace) { build(:namespace) } let_it_be(:namespace) { build(:namespace) }
where( where(:buy_storage_link_enabled, :additional_repo_storage_by_namespace_enabled, :result) do
auto_storage_allocation_enabled: [true, false], false | false | false
buy_storage_link_enabled: [true, false], false | true | false
namespace_storage_limit_enabled: [true, false], true | false | false
additional_storage_enabled: [true, false] true | true | true
) end
with_them do with_them do
let(:result) do
auto_storage_allocation_enabled &&
buy_storage_link_enabled &&
!namespace_storage_limit_enabled &&
additional_storage_enabled
end
before do before do
stub_application_setting(automatic_purchased_storage_allocation: auto_storage_allocation_enabled) stub_feature_flags(buy_storage_link: buy_storage_link_enabled)
stub_feature_flags( allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
namespace_storage_limit: namespace_storage_limit_enabled, .and_return(additional_repo_storage_by_namespace_enabled)
additional_repo_storage_by_namespace: additional_storage_enabled,
buy_storage_link: buy_storage_link_enabled
)
end end
it { is_expected.to eq(result) } it { is_expected.to eq(result) }
......
...@@ -16,40 +16,26 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do ...@@ -16,40 +16,26 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
subject { model.above_size_limit? } subject { model.above_size_limit? }
before do before do
allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled } allow(model).to receive(:enforce_limit?) { enforce_limit }
end end
context 'when limit enforcement is off' do context 'when limit enforcement is off' do
let(:storage_allocation_enabled) { false } let(:enforce_limit) { false }
it { is_expected.to eq(false) } it { is_expected.to eq(false) }
end end
context 'when limit enforcement is on' do context 'when limit enforcement is on' do
let(:storage_allocation_enabled) { true } let(:enforce_limit) { true }
context 'with feature flag :namespace_storage_limit disabled' do context 'when below limit' do
before do it { is_expected.to eq(false) }
stub_feature_flags(namespace_storage_limit: false)
end
context 'when below limit' do
it { is_expected.to eq(false) }
end
context 'when above limit' do
let(:total_repository_size_excess) { 101.megabytes }
it { is_expected.to eq(true) }
end
end end
context 'with feature flag :additional_repo_storage_by_namespace disabled' do context 'when above limit' do
before do let(:total_repository_size_excess) { 101.megabytes }
stub_feature_flags(additional_repo_storage_by_namespace: false)
end
it { is_expected.to eq(false) } it { is_expected.to eq(true) }
end end
end end
end end
...@@ -99,36 +85,21 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do ...@@ -99,36 +85,21 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
describe '#enforce_limit?' do describe '#enforce_limit?' do
subject { model.enforce_limit? } subject { model.enforce_limit? }
let(:storage_allocation_enabled) { true }
before do before do
allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled } allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
.and_return(additional_repo_storage_by_namespace_enabled)
end end
context 'with application setting is disabled' do context 'when additional_repo_storage_by_namespace_enabled is false' do
let(:storage_allocation_enabled) { false } let(:additional_repo_storage_by_namespace_enabled) { false }
it { is_expected.to eq(false) } it { is_expected.to eq(false) }
end end
context 'with feature flags (:namespace_storage_limit & :additional_repo_storage_by_namespace) enabled' do
it { is_expected.to eq(false) }
end
context 'with feature flag :namespace_storage_limit disabled' do context 'with feature flag :namespace_storage_limit disabled' do
before do let(:additional_repo_storage_by_namespace_enabled) { true }
stub_feature_flags(namespace_storage_limit: false)
end
it { is_expected.to eq(true) } it { is_expected.to eq(true) }
end end
context 'with feature flag :additional_repo_storage_by_namespace disabled' do
before do
stub_feature_flags(additional_repo_storage_by_namespace: false)
end
it { is_expected.to eq(false) }
end
end end
end end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Namespace do RSpec.describe Namespace do
using RSpec::Parameterized::TableSyntax
include EE::GeoHelpers include EE::GeoHelpers
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
...@@ -1770,12 +1772,48 @@ RSpec.describe Namespace do ...@@ -1770,12 +1772,48 @@ RSpec.describe Namespace do
end end
end end
describe '#additional_repo_storage_by_namespace_enabled?' do
let_it_be(:namespace) { build(:namespace) }
subject { namespace.additional_repo_storage_by_namespace_enabled? }
where(:namespace_storage_limit, :additional_repo_storage_by_namespace, :automatic_purchased_storage_allocation, :result) do
false | false | false | false
false | false | true | false
false | true | false | false
true | false | false | false
false | true | true | true
true | true | false | false
true | false | true | false
true | true | true | false
end
with_them do
before do
stub_feature_flags(
namespace_storage_limit: namespace_storage_limit,
additional_repo_storage_by_namespace: additional_repo_storage_by_namespace
)
stub_application_setting(automatic_purchased_storage_allocation: automatic_purchased_storage_allocation)
end
it { is_expected.to eq(result) }
end
end
describe '#root_storage_size' do describe '#root_storage_size' do
let_it_be(:namespace) { build(:namespace) } let_it_be(:namespace) { build(:namespace) }
subject { namespace.root_storage_size } subject { namespace.root_storage_size }
context 'with feature flag :namespace_storage_limit enabled' do before do
allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
.and_return(additional_repo_storage_by_namespace_enabled)
end
context 'when additional_repo_storage_by_namespace_enabled is false' do
let(:additional_repo_storage_by_namespace_enabled) { false }
it 'initializes a new instance of EE::Namespace::RootStorageSize' do it 'initializes a new instance of EE::Namespace::RootStorageSize' do
expect(EE::Namespace::RootStorageSize).to receive(:new).with(namespace) expect(EE::Namespace::RootStorageSize).to receive(:new).with(namespace)
...@@ -1783,10 +1821,8 @@ RSpec.describe Namespace do ...@@ -1783,10 +1821,8 @@ RSpec.describe Namespace do
end end
end end
context 'with feature flag :namespace_storage_limit disabled' do context 'when additional_repo_storage_by_namespace_enabled is true' do
before do let(:additional_repo_storage_by_namespace_enabled) { true }
stub_feature_flags(namespace_storage_limit: false)
end
it 'initializes a new instance of EE::Namespace::RootExcessStorageSize' do it 'initializes a new instance of EE::Namespace::RootExcessStorageSize' do
expect(EE::Namespace::RootExcessStorageSize).to receive(:new).with(namespace) expect(EE::Namespace::RootExcessStorageSize).to receive(:new).with(namespace)
......
...@@ -108,17 +108,15 @@ RSpec.describe PostReceiveService, :geo do ...@@ -108,17 +108,15 @@ RSpec.describe PostReceiveService, :geo do
let(:check_storage_size_response) { ServiceResponse.success } let(:check_storage_size_response) { ServiceResponse.success }
where(:namespace_storage_limit_enabled, :additional_repo_storage_by_namespace_enabled, :service_class_name) do where(:additional_repo_storage_by_namespace_enabled, :service_class_name) do
true | false | Namespaces::CheckStorageSizeService true | Namespaces::CheckExcessStorageSizeService
true | true | Namespaces::CheckStorageSizeService false | Namespaces::CheckStorageSizeService
false | true | Namespaces::CheckExcessStorageSizeService
false | false | Namespaces::CheckStorageSizeService
end end
with_them do with_them do
before do before do
stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled) allow(project.namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
stub_feature_flags(additional_repo_storage_by_namespace: additional_repo_storage_by_namespace_enabled) .and_return(additional_repo_storage_by_namespace_enabled)
allow_next_instance_of(service_class_name, project.namespace, user) do |service| allow_next_instance_of(service_class_name, project.namespace, user) do |service|
expect(service).to receive(:execute).and_return(check_storage_size_response) expect(service).to receive(:execute).and_return(check_storage_size_response)
......
...@@ -8,43 +8,24 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do ...@@ -8,43 +8,24 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
let(:service) { described_class.new(namespace, user) } let(:service) { described_class.new(namespace, user) }
let(:total_repository_size_excess) { 150.megabytes } let(:total_repository_size_excess) { 150.megabytes }
let(:additional_purchased_storage_size) { 100 } let(:additional_purchased_storage_size) { 100 }
let(:namespace_storage_limit_enabled) { false } let(:additional_repo_storage_by_namespace_enabled) { true }
let(:storage_allocation_enabled) { true }
let(:actual_size_limit) { 10.gigabytes } let(:actual_size_limit) { 10.gigabytes }
let(:locked_project_count) { 1 } let(:locked_project_count) { 1 }
subject(:response) { service.execute } subject(:response) { service.execute }
before do before do
stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled) allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?).and_return(additional_repo_storage_by_namespace_enabled)
allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
allow(namespace).to receive(:root_ancestor).and_return(namespace) allow(namespace).to receive(:root_ancestor).and_return(namespace)
allow(namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess) allow(namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess)
allow(namespace).to receive(:actual_size_limit).and_return(actual_size_limit) allow(namespace).to receive(:actual_size_limit).and_return(actual_size_limit)
allow(namespace).to receive(:repository_size_excess_project_count).and_return(locked_project_count) allow(namespace).to receive(:repository_size_excess_project_count).and_return(locked_project_count)
end end
context 'without limit enforcement' do context 'when additional_repo_storage_by_namespace_enabled is false' do
context 'with application setting disabled' do let(:additional_repo_storage_by_namespace_enabled) { false }
let(:storage_allocation_enabled) { false }
it { is_expected.to be_success }
end
context 'with feature flag :additional_repo_storage_by_namespace disabled' do
before do
stub_feature_flags(additional_repo_storage_by_namespace: false)
end
it { is_expected.to be_success }
end
context 'with feature flag :namespace_storage_limit enabled' do it { is_expected.to be_success }
let(:namespace_storage_limit_enabled) { true }
it { is_expected.to be_success }
end
end end
context 'when additional_purchased_storage_size is set to 0' do context 'when additional_purchased_storage_size is set to 0' do
...@@ -77,14 +58,7 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do ...@@ -77,14 +58,7 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
end end
context 'when not admin of the namespace' do context 'when not admin of the namespace' do
let(:other_namespace) { build(:namespace, additional_purchased_storage_size: additional_purchased_storage_size) } let(:user) { build(:user) }
subject(:response) { described_class.new(other_namespace, user).execute }
before do
allow(other_namespace).to receive(:root_ancestor).and_return(other_namespace)
allow(other_namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess)
end
it 'errors and has no payload' do it 'errors and has no payload' do
expect(response).to be_error expect(response).to be_error
......
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