Skip to content

Commit cecc9f3

Browse files
sanzmaurosanzmauro
authored
Merge pull request #528 from splitio/unsupported-matcher
[Cherry pick] Unsupported matcher
2 parents 6804f26 + 0113765 commit cecc9f3

4 files changed

Lines changed: 139 additions & 29 deletions

File tree

‎lib/splitclient-rb/cache/repositories/splits_repository.rb‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,32 @@ module Cache
55
module Repositories
66
class SplitsRepository < Repository
77
attr_reader :adapter
8-
8+
DEFAULT_CONDITIONS_TEMPLATE = [{
9+
conditionType: "ROLLOUT",
10+
matcherGroup: {
11+
combiner: "AND",
12+
matchers: [
13+
{
14+
keySelector: nil,
15+
matcherType: "ALL_KEYS",
16+
negate: false,
17+
userDefinedSegmentMatcherData: nil,
18+
whitelistMatcherData: nil,
19+
unaryNumericMatcherData: nil,
20+
betweenMatcherData: nil,
21+
dependencyMatcherData: nil,
22+
booleanMatcherData: nil,
23+
stringMatcherData: nil
24+
}]
25+
},
26+
partitions: [
27+
{
28+
treatment: "control",
29+
size: 100
30+
}
31+
],
32+
label: "unsupported matcher type"
33+
}]
934
def initialize(config, flag_sets_repository, flag_set_filter)
1035
super(config)
1136
@tt_cache = {}
@@ -155,6 +180,10 @@ def add_feature_flag(split)
155180
remove_from_flag_sets(existing_split)
156181
end
157182

183+
if check_undefined_matcher(split)
184+
@config.logger.warn("Feature Flag #{split[:name]} has undefined matcher, setting conditions to default template.")
185+
split[:conditions] = SplitsRepository::DEFAULT_CONDITIONS_TEMPLATE
186+
end
158187
if !split[:sets].nil?
159188
for flag_set in split[:sets]
160189
if !@flag_sets.flag_set_exist?(flag_set)
@@ -170,6 +199,18 @@ def add_feature_flag(split)
170199
@adapter.set_string(namespace_key(".split.#{split[:name]}"), split.to_json)
171200
end
172201

202+
def check_undefined_matcher(split)
203+
for condition in split[:conditions]
204+
for matcher in condition[:matcherGroup][:matchers]
205+
if !SplitIoClient::Condition.instance_methods(false).map(&:to_s).include?("matcher_#{matcher[:matcherType].downcase}")
206+
@config.logger.error("Detected undefined matcher #{matcher[:matcherType].downcase} in feature flag #{split[:name]}")
207+
return true
208+
end
209+
end
210+
end
211+
return false
212+
end
213+
173214
def remove_feature_flag(split)
174215
decrease_tt_name_count(split[:trafficTypeName])
175216
remove_from_flag_sets(split)

‎spec/cache/repositories/splits_repository_spec.rb‎

Lines changed: 86 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
before do
1919
# in memory setup
20-
repository.update([{name: 'foo', trafficTypeName: 'tt_name_1'},
21-
{name: 'bar', trafficTypeName: 'tt_name_2'},
22-
{name: 'baz', trafficTypeName: 'tt_name_1'}], [], -1)
20+
repository.update([{name: 'foo', trafficTypeName: 'tt_name_1', conditions: []},
21+
{name: 'bar', trafficTypeName: 'tt_name_2', conditions: []},
22+
{name: 'baz', trafficTypeName: 'tt_name_1', conditions: []}], [], -1)
2323

2424
# redis setup
2525
repository.instance_variable_get(:@adapter).set_string(
@@ -31,13 +31,13 @@
3131
end
3232

3333
after do
34-
repository.update([], [{name: 'foo', trafficTypeName: 'tt_name_1'},
35-
{name: 'bar', trafficTypeName: 'tt_name_2'},
36-
{name: 'bar', trafficTypeName: 'tt_name_2'},
37-
{name: 'qux', trafficTypeName: 'tt_name_3'},
38-
{name: 'quux', trafficTypeName: 'tt_name_4'},
39-
{name: 'corge', trafficTypeName: 'tt_name_5'},
40-
{name: 'corge', trafficTypeName: 'tt_name_6'}], -1)
34+
repository.update([], [{name: 'foo', trafficTypeName: 'tt_name_1', conditions: []},
35+
{name: 'bar', trafficTypeName: 'tt_name_2', conditions: []},
36+
{name: 'bar', trafficTypeName: 'tt_name_2', conditions: []},
37+
{name: 'qux', trafficTypeName: 'tt_name_3', conditions: []},
38+
{name: 'quux', trafficTypeName: 'tt_name_4', conditions: []},
39+
{name: 'corge', trafficTypeName: 'tt_name_5', conditions: []},
40+
{name: 'corge', trafficTypeName: 'tt_name_6', conditions: []}], -1)
4141
end
4242

4343
it 'returns splits names' do
@@ -52,7 +52,7 @@
5252
expect(repository.traffic_type_exists('tt_name_1')).to be true
5353
expect(repository.traffic_type_exists('tt_name_2')).to be true
5454

55-
split = { name: 'qux', trafficTypeName: 'tt_name_3' }
55+
split = { name: 'qux', trafficTypeName: 'tt_name_3', conditions: [] }
5656

5757
repository.update([split], [], -1)
5858
repository.update([], [split], -1)
@@ -61,7 +61,7 @@
6161
end
6262

6363
it 'does not increment traffic type count when adding same split twice' do
64-
split = { name: 'quux', trafficTypeName: 'tt_name_4' }
64+
split = { name: 'quux', trafficTypeName: 'tt_name_4', conditions: [] }
6565

6666
repository.update([split, split], [], -1)
6767
repository.update([], [split], -1)
@@ -70,7 +70,7 @@
7070
end
7171

7272
it 'updates traffic type count accordingly when split changes traffic type' do
73-
split = { name: 'corge', trafficTypeName: 'tt_name_5' }
73+
split = { name: 'corge', trafficTypeName: 'tt_name_5', conditions: [] }
7474

7575
repository.update([split], [], -1)
7676
repository.instance_variable_get(:@adapter).set_string(
@@ -79,7 +79,7 @@
7979

8080
expect(repository.traffic_type_exists('tt_name_5')).to be true
8181

82-
split = { name: 'corge', trafficTypeName: 'tt_name_6' }
82+
split = { name: 'corge', trafficTypeName: 'tt_name_6', conditions: [] }
8383

8484
repository.update([split], [], -1)
8585

@@ -97,11 +97,80 @@
9797

9898
it 'returns splits data' do
9999
expect(repository.splits).to eq(
100-
'foo' => { name: 'foo', trafficTypeName: 'tt_name_1' },
101-
'bar' => { name: 'bar', trafficTypeName: 'tt_name_2' },
102-
'baz' => { name: 'baz', trafficTypeName: 'tt_name_1' }
100+
'foo' => { name: 'foo', trafficTypeName: 'tt_name_1', conditions: [] },
101+
'bar' => { name: 'bar', trafficTypeName: 'tt_name_2', conditions: [] },
102+
'baz' => { name: 'baz', trafficTypeName: 'tt_name_1', conditions: [] }
103103
)
104104
end
105+
106+
it 'remove undefined matcher with template condition' do
107+
split = { name: 'corge', trafficTypeName: 'tt_name_5', conditions: [
108+
{
109+
partitions: [
110+
{treatment: 'on', size: 50},
111+
{treatment: 'off', size: 50}
112+
],
113+
contitionType: 'WHITELIST',
114+
label: 'some_label',
115+
matcherGroup: {
116+
matchers: [
117+
{
118+
matcherType: 'UNDEFINED',
119+
whitelistMatcherData: {
120+
whitelist: ['k1', 'k2', 'k3']
121+
},
122+
negate: false,
123+
}
124+
],
125+
combiner: 'AND'
126+
}
127+
}]
128+
}
129+
repository.update([split], [], -1)
130+
expect(repository.get_split('corge')[:conditions]).to eq SplitIoClient::Cache::Repositories::SplitsRepository::DEFAULT_CONDITIONS_TEMPLATE
131+
132+
# test with multiple conditions
133+
split2 = {
134+
name: 'corge2',
135+
trafficTypeName: 'tt_name_5',
136+
conditions: [
137+
{
138+
partitions: [
139+
{treatment: 'on', size: 50},
140+
{treatment: 'off', size: 50}
141+
],
142+
contitionType: 'WHITELIST',
143+
label: 'some_label',
144+
matcherGroup: {
145+
matchers: [
146+
{
147+
matcherType: 'UNDEFINED',
148+
whitelistMatcherData: {
149+
whitelist: ['k1', 'k2', 'k3']
150+
},
151+
negate: false,
152+
}
153+
],
154+
combiner: 'AND'
155+
}
156+
},
157+
{
158+
partitions: [
159+
{treatment: 'on', size: 25},
160+
{treatment: 'off', size: 75}
161+
],
162+
contitionType: 'WHITELIST',
163+
label: 'some_other_label',
164+
matcherGroup: {
165+
matchers: [{matcherType: 'ALL_KEYS', negate: false}],
166+
combiner: 'AND'
167+
}
168+
}]
169+
}
170+
171+
repository.update([split2], [], -1)
172+
expect(repository.get_split('corge2')[:conditions]).to eq SplitIoClient::Cache::Repositories::SplitsRepository::DEFAULT_CONDITIONS_TEMPLATE
173+
end
105174
end
106175

107176
describe 'with Memory Adapter' do

‎spec/repository_helper.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@
1313
flag_sets_repository,
1414
flag_set_filter)
1515

16-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', :sets => []}], -1, config)
16+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', conditions: [], :sets => []}], -1, config)
1717
expect(feature_flag_repository.get_split('split1').nil?).to eq(true)
1818

19-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', :sets => ['set_3']}], -1, config)
19+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', conditions: [], :sets => ['set_3']}], -1, config)
2020
expect(feature_flag_repository.get_split('split1').nil?).to eq(true)
2121

22-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', :sets => ['set_1']}], -1, config)
22+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', conditions: [], :sets => ['set_1']}], -1, config)
2323
expect(feature_flag_repository.get_split('split1').nil?).to eq(false)
2424

25-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ARCHIVED', :sets => ['set_1']}], -1, config)
25+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ARCHIVED', conditions: [], :sets => ['set_1']}], -1, config)
2626
expect(feature_flag_repository.get_split('split1').nil?).to eq(true)
2727
end
2828

@@ -35,16 +35,16 @@
3535
flag_sets_repository,
3636
flag_set_filter)
3737

38-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', :sets => []}], -1, config)
38+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', conditions: [], :sets => []}], -1, config)
3939
expect(feature_flag_repository.get_split('split1').nil?).to eq(false)
4040

41-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split2', :status => 'ACTIVE', :sets => ['set_3']}], -1, config)
41+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split2', :status => 'ACTIVE', conditions: [], :sets => ['set_3']}], -1, config)
4242
expect(feature_flag_repository.get_split('split2').nil?).to eq(false)
4343

44-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split3', :status => 'ACTIVE', :sets => ['set_1']}], -1, config)
44+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split3', :status => 'ACTIVE', conditions: [], :sets => ['set_1']}], -1, config)
4545
expect(feature_flag_repository.get_split('split1').nil?).to eq(false)
4646

47-
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ARCHIVED', :sets => ['set_1']}], -1, config)
47+
SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ARCHIVED', conditions: [], :sets => ['set_1']}], -1, config)
4848
expect(feature_flag_repository.get_split('split1').nil?).to eq(true)
4949
end
5050
end

‎spec/telemetry/synchronizer_spec.rb‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@
6565
end
6666

6767
it 'with data' do
68-
splits_repository.update([{name: 'foo', trafficTypeName: 'tt_name_1'},
69-
{name: 'bar', trafficTypeName: 'tt_name_2'},
70-
{name: 'baz', trafficTypeName: 'tt_name_1'}], [], -1)
68+
splits_repository.update([{name: 'foo', trafficTypeName: 'tt_name_1', conditions: []},
69+
{name: 'bar', trafficTypeName: 'tt_name_2', conditions: []},
70+
{name: 'baz', trafficTypeName: 'tt_name_1', conditions: []}], [], -1)
7171

7272
segments_repository.add_to_segment(name: 'foo-1', added: [1, 2, 3], removed: [])
7373
segments_repository.add_to_segment(name: 'foo-2', added: [1, 2, 3, 4], removed: [])

0 commit comments

Comments
 (0)