-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the possibility to fetch all features already released to a given resource #67
Changes from all commits
2a9e18c
cdbf8b5
d83cd8b
25180ad
52c0919
33b3358
3b65b47
56178cb
5984e80
7d6ce1b
67426b3
9187826
7322aee
43479d1
71648b7
679fcc2
2977d70
1eb46df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
module FeatureFlagger | ||
module Storage | ||
module Keys | ||
MINIMUM_VALID_FEATURE_PATH = 2.freeze | ||
|
||
def self.resource_key(prefix, resource_name, resource_id) | ||
"#{prefix}:#{resource_name}:#{resource_id}" | ||
end | ||
|
||
def self.extract_resource_name_from_feature_key(feature_key) | ||
feature_paths = feature_key.split(':') | ||
|
||
raise InvalidResourceNameError if feature_paths.size < MINIMUM_VALID_FEATURE_PATH | ||
|
||
feature_paths.first | ||
end | ||
|
||
class InvalidResourceNameError < StandardError; end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,9 @@ module FeatureFlagger | |
let(:redis) { FakeRedis::Redis.new } | ||
let(:storage) { Storage::Redis.new(redis) } | ||
let(:control) { Control.new(storage) } | ||
let(:key) { 'key' } | ||
let(:key) { 'account:email_marketing:whitelabel' } | ||
let(:resource_id) { 'resource_id' } | ||
let(:resource_name) { 'account' } | ||
|
||
before do | ||
redis.flushdb | ||
|
@@ -19,19 +20,19 @@ module FeatureFlagger | |
it { expect(result).to be_falsey } | ||
|
||
context 'and a feature is release to all' do | ||
before { storage.add(FeatureFlagger::Control::RELEASED_FEATURES, key) } | ||
before { control.release_to_all(key) } | ||
|
||
it { expect(result).to be_truthy } | ||
end | ||
end | ||
|
||
context 'when resource entity id has access to release_key' do | ||
before { storage.add(key, resource_id) } | ||
before { control.release(key, resource_id) } | ||
|
||
it { expect(result).to be_truthy } | ||
|
||
context 'and a feature is release to all' do | ||
before { storage.add(FeatureFlagger::Control::RELEASED_FEATURES, key) } | ||
before { control.release_to_all(key) } | ||
|
||
it { expect(result).to be_truthy } | ||
end | ||
|
@@ -41,13 +42,21 @@ module FeatureFlagger | |
describe '#release' do | ||
it 'adds resource_id to storage' do | ||
control.release(key, resource_id) | ||
expect(storage).to have_value(key, resource_id) | ||
expect(control).to be_released(key, resource_id) | ||
end | ||
end | ||
|
||
describe '#releases' do | ||
it 'return all releases to a given resource' do | ||
control.release(key, resource_id) | ||
resource_name = 'account' | ||
expect(control.releases(resource_name, resource_id)).to match_array(['account:email_marketing:whitelabel']) | ||
end | ||
end | ||
|
||
describe '#release_to_all' do | ||
it 'adds feature_key to storage' do | ||
storage.add(key, 1) | ||
storage.add(key, resource_name, 1) | ||
control.release_to_all(key) | ||
expect(storage).not_to have_value(key, 1) | ||
expect(storage).to have_value(FeatureFlagger::Control::RELEASED_FEATURES, key) | ||
|
@@ -56,21 +65,21 @@ module FeatureFlagger | |
|
||
describe '#unrelease' do | ||
it 'removes resource_id from storage' do | ||
storage.add(key, resource_id) | ||
storage.add(key, resource_name, resource_id) | ||
control.unrelease(key, resource_id) | ||
expect(storage).not_to have_value(key, resource_id) | ||
end | ||
end | ||
|
||
describe '#unrelease_to_all' do | ||
it 'removes feature_key to storage' do | ||
storage.add(FeatureFlagger::Control::RELEASED_FEATURES, key) | ||
storage.add(FeatureFlagger::Control::RELEASED_FEATURES, resource_name, key) | ||
control.unrelease_to_all(key) | ||
expect(storage).not_to have_value(FeatureFlagger::Control::RELEASED_FEATURES, key) | ||
end | ||
|
||
it 'removes added resources' do | ||
storage.add(key, 1) | ||
storage.add(key, resource_name, 1) | ||
control.unrelease_to_all(key) | ||
expect(storage).not_to have_value(key, 1) | ||
expect(storage).not_to have_value(FeatureFlagger::Control::RELEASED_FEATURES, key) | ||
|
@@ -92,34 +101,32 @@ module FeatureFlagger | |
subject { control.released_features_to_all } | ||
|
||
it 'returns all the values to given features' do | ||
control.release(FeatureFlagger::Control::RELEASED_FEATURES, 'feature::name1') | ||
control.release(FeatureFlagger::Control::RELEASED_FEATURES, 'feature::name2') | ||
control.release(FeatureFlagger::Control::RELEASED_FEATURES, 'feature::name15') | ||
expect(subject).to match_array %w[feature::name1 feature::name2 feature::name15] | ||
control.release_to_all('account:feature:name1') | ||
control.release_to_all('account:feature:name2') | ||
control.release_to_all('account:feature:name15') | ||
expect(subject).to match_array %w[account:feature:name1 account:feature:name2 account:feature:name15] | ||
end | ||
end | ||
|
||
describe '#released_to_all?' do | ||
let(:result) { control.released_to_all?(key) } | ||
|
||
context 'when feature was not released to all' do | ||
before { storage.remove(FeatureFlagger::Control::RELEASED_FEATURES, key) } | ||
|
||
it { expect(result).to be_falsey } | ||
end | ||
|
||
context 'when feature was released to all' do | ||
before { storage.add(FeatureFlagger::Control::RELEASED_FEATURES, key) } | ||
before { storage.add_all(FeatureFlagger::Control::RELEASED_FEATURES, key) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're right, tests are better now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you missed this one :) It still using storage, IMHO this spec we should not use the |
||
|
||
it { expect(result).to be_truthy } | ||
end | ||
end | ||
|
||
describe '#search_keys' do | ||
before do | ||
storage.add('namespace:1', 1) | ||
storage.add('namespace:2', 2) | ||
storage.add('exclusive', 3) | ||
storage.add('namespace:1', resource_name, 1) | ||
storage.add('namespace:2', resource_name, 2) | ||
storage.add('exclusive', resource_name, 3) | ||
end | ||
|
||
context 'without matching result' do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍