Skip to content

Commit d761e16

Browse files
committed
Fix lint issues
1 parent 5e26ae6 commit d761e16

File tree

5 files changed

+57
-61
lines changed

5 files changed

+57
-61
lines changed

lib/optimizely/config/datafile_project_config.rb

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,7 @@ def initialize(datafile, logger, error_handler)
198198
flag_id = feature_flag['id']
199199
applicable_holdouts = []
200200

201-
if @included_holdouts[flag_id]
202-
applicable_holdouts.concat(@included_holdouts[flag_id])
203-
end
201+
applicable_holdouts.concat(@included_holdouts[flag_id]) if @included_holdouts[flag_id]
204202

205203
@global_holdouts.each_value do |holdout|
206204
excluded_flag_ids = holdout['excludedFlags'] || []
@@ -215,20 +213,20 @@ def initialize(datafile, logger, error_handler)
215213
@holdouts.each do |holdout|
216214
holdout_key = holdout['key']
217215
holdout_id = holdout['id']
218-
216+
219217
@variation_key_map[holdout_key] = {}
220218
@variation_id_map[holdout_key] = {}
221219
@variation_id_map_by_experiment_id[holdout_id] = {}
222220
@variation_key_map_by_experiment_id[holdout_id] = {}
223221

224222
variations = holdout['variations']
225-
if variations && !variations.empty?
226-
variations.each do |variation|
227-
@variation_key_map[holdout_key][variation['key']] = variation
228-
@variation_id_map[holdout_key][variation['id']] = variation
229-
@variation_key_map_by_experiment_id[holdout_id][variation['key']] = variation
230-
@variation_id_map_by_experiment_id[holdout_id][variation['id']] = variation
231-
end
223+
next unless variations && !variations.empty?
224+
225+
variations.each do |variation|
226+
@variation_key_map[holdout_key][variation['key']] = variation
227+
@variation_id_map[holdout_key][variation['id']] = variation
228+
@variation_key_map_by_experiment_id[holdout_id][variation['key']] = variation
229+
@variation_id_map_by_experiment_id[holdout_id][variation['id']] = variation
232230
end
233231
end
234232
end

lib/optimizely/decision_service.rb

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,18 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
192192
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
193193
reasons.push(*holdout_decision.reasons)
194194

195-
if holdout_decision.decision
196-
message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'."
197-
@logger.log(Logger::INFO, message)
198-
reasons.push(message)
199-
return DecisionResult.new(holdout_decision.decision, false, reasons)
200-
end
195+
next unless holdout_decision.decision
196+
message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'."
197+
@logger.log(Logger::INFO, message)
198+
reasons.push(message)
199+
return DecisionResult.new(holdout_decision.decision, false, reasons)
201200
end
202201

203202
# Check if the feature flag has an experiment and the user is bucketed into that experiment
204203
experiment_decision = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options)
205204
reasons.push(*experiment_decision.reasons)
206205

207-
if experiment_decision.decision
208-
return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons)
209-
end
206+
return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons) if experiment_decision.decision
210207

211208
# Check if the feature flag has a rollout and the user is bucketed into that rollout
212209
rollout_decision = get_variation_for_feature_rollout(project_config, feature_flag, user_context)
@@ -216,13 +213,13 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
216213
message = "The user '#{user_id}' is bucketed into a rollout for feature flag '#{feature_flag['key']}'."
217214
@logger.log(Logger::INFO, message)
218215
reasons.push(message)
219-
return DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons)
216+
DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons)
220217
else
221218
message = "The user '#{user_id}' is not bucketed into a rollout for feature flag '#{feature_flag['key']}'."
222219
@logger.log(Logger::INFO, message)
223220
reasons.push(message)
224221
default_decision = Decision.new(nil, nil, DECISION_SOURCES['ROLLOUT'], nil)
225-
return DecisionResult.new(nil, false, reasons)
222+
DecisionResult.new(nil, false, reasons)
226223
end
227224
end
228225

@@ -262,12 +259,12 @@ def get_variation_for_holdout(holdout, user_context, project_config)
262259
decide_reasons.push(message)
263260

264261
holdout_decision = Decision.new(holdout, variation, DECISION_SOURCES['HOLDOUT'], nil)
265-
return DecisionResult.new(holdout_decision, false, decide_reasons)
262+
DecisionResult.new(holdout_decision, false, decide_reasons)
266263
else
267264
message = "The user '#{user_id}' is not bucketed into holdout '#{holdout['key']}'."
268265
@logger.log(Logger::DEBUG, message)
269266
decide_reasons.push(message)
270-
return DecisionResult.new(nil, false, decide_reasons)
267+
DecisionResult.new(nil, false, decide_reasons)
271268
end
272269
end
273270

spec/bucketing_holdout_spec.rb

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# frozen_string_literal: true
2+
13
#
24
# Copyright 2025 Optimizely and contributors
35
#
@@ -46,7 +48,7 @@
4648
# Set bucket value to be within first variation's traffic allocation (0-5000 range)
4749
test_bucketer.set_bucket_values([2500])
4850

49-
variation, reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
51+
variation, _reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
5052

5153
expect(variation).not_to be_nil
5254
expect(variation['id']).to eq('var_1')
@@ -70,7 +72,7 @@
7072
# Set bucket value outside traffic allocation range
7173
test_bucketer.set_bucket_values([1500])
7274

73-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
75+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
7476

7577
expect(variation).to be_nil
7678

@@ -91,7 +93,7 @@
9193

9294
test_bucketer.set_bucket_values([5000])
9395

94-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
96+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
9597

9698
expect(variation).to be_nil
9799

@@ -112,7 +114,7 @@
112114

113115
test_bucketer.set_bucket_values([5000])
114116

115-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
117+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
116118

117119
expect(variation).to be_nil
118120

@@ -130,7 +132,7 @@
130132

131133
test_bucketer.set_bucket_values([5000])
132134

133-
variation, reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
135+
variation, _reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
134136

135137
expect(variation).to be_nil
136138

@@ -151,7 +153,7 @@
151153

152154
test_bucketer.set_bucket_values([5000])
153155

154-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
156+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
155157

156158
# Should return nil for invalid experiment key
157159
expect(variation).to be_nil
@@ -167,7 +169,7 @@
167169

168170
test_bucketer.set_bucket_values([5000])
169171

170-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
172+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
171173

172174
# Should return nil for null experiment key
173175
expect(variation).to be_nil
@@ -182,7 +184,7 @@
182184

183185
# Test user buckets into first variation
184186
test_bucketer.set_bucket_values([2500])
185-
variation, reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
187+
variation, _reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
186188

187189
expect(variation).not_to be_nil
188190
expect(variation['id']).to eq('var_1')
@@ -200,7 +202,7 @@
200202

201203
# Test user buckets into second variation (bucket value 7500 should be in 5000-10000 range)
202204
test_bucketer.set_bucket_values([7500])
203-
variation, reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
205+
variation, _reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
204206

205207
expect(variation).not_to be_nil
206208
expect(variation['id']).to eq('var_2')
@@ -217,14 +219,14 @@
217219

218220
# Test exact boundary value (should be included)
219221
test_bucketer.set_bucket_values([4999])
220-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
222+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
221223

222224
expect(variation).not_to be_nil
223225
expect(variation['id']).to eq('var_1')
224226

225227
# Test value just outside boundary (should not be included)
226228
test_bucketer.set_bucket_values([5000])
227-
variation, reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
229+
variation, _reasons = test_bucketer.bucket(config, modified_holdout, test_bucketing_id, test_user_id)
228230

229231
expect(variation).to be_nil
230232
end
@@ -235,8 +237,8 @@
235237

236238
# Create a real bucketer (not test bucketer) for consistent hashing
237239
real_bucketer = Optimizely::Bucketer.new(spy_logger)
238-
variation1, reasons1 = real_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
239-
variation2, reasons2 = real_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
240+
variation1, _reasons1 = real_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
241+
variation2, _reasons2 = real_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
240242

241243
# Results should be identical
242244
if variation1
@@ -267,7 +269,7 @@
267269
expect(holdout).not_to be_nil
268270

269271
test_bucketer.set_bucket_values([5000])
270-
variation, reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
272+
variation, _reasons = test_bucketer.bucket(config, holdout, test_bucketing_id, test_user_id)
271273

272274
expect(reasons).not_to be_nil
273275
# Decision reasons should be populated from the bucketing process
@@ -284,9 +286,8 @@ def initialize(logger)
284286
@bucket_index = 0
285287
end
286288

287-
def set_bucket_values(values)
289+
def bucket_values(values)
288290
@bucket_values = values
289-
@bucket_index = 0
290291
end
291292

292293
def generate_bucket_value(bucketing_id)
@@ -296,4 +297,4 @@ def generate_bucket_value(bucketing_id)
296297
@bucket_index = (@bucket_index + 1) % @bucket_values.length
297298
value
298299
end
299-
end
300+
end

spec/config/datafile_project_config_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,7 +1512,7 @@
15121512

15131513
describe '#decide with multiple holdouts' do
15141514
it 'should handle multiple holdouts for different flags' do
1515-
flag_keys = ['test_flag_1', 'test_flag_2', 'test_flag_3', 'test_flag_4']
1515+
flag_keys = %w[test_flag_1 test_flag_2 test_flag_3 test_flag_4]
15161516

15171517
flag_keys.each do |flag_key|
15181518
feature_flag = config_with_holdouts.feature_flag_key_map[flag_key]
@@ -1667,7 +1667,7 @@
16671667
describe 'holdout status messages' do
16681668
it 'should differentiate between running and non-running holdouts' do
16691669
running_holdouts = config_with_holdouts.holdouts.select { |h| h['status'] == 'Running' }
1670-
non_running_holdouts = config_with_holdouts.holdouts.select { |h| h['status'] != 'Running' }
1670+
non_running_holdouts = config_with_holdouts.holdouts.reject { |h| h['status'] == 'Running' }
16711671

16721672
# Only running holdouts should be in the holdout_id_map
16731673
running_holdouts.each do |holdout|

spec/decision_service_spec.rb

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,7 @@
12161216

12171217
user_context = project_with_holdouts.create_user_context('testUserId', {})
12181218

1219-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1219+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
12201220
[feature_flag],
12211221
user_context,
12221222
config_with_holdouts,
@@ -1228,7 +1228,7 @@
12281228
expect(result.length).to be > 0
12291229

12301230
# Check if any decision is from holdout source
1231-
holdout_decision = result.find do |decision_result|
1231+
_holdout_decision = result.find do |decision_result|
12321232
decision_result.decision&.source == Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']
12331233
end
12341234

@@ -1255,7 +1255,7 @@
12551255

12561256
user_context = project_with_holdouts.create_user_context('testUserId', {})
12571257

1258-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1258+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
12591259
[feature_flag],
12601260
user_context,
12611261
config_with_holdouts,
@@ -1283,7 +1283,7 @@
12831283

12841284
user_context = project_with_holdouts.create_user_context('testUserId', {})
12851285

1286-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1286+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
12871287
[feature_flag],
12881288
user_context,
12891289
config_with_holdouts,
@@ -1312,7 +1312,7 @@
13121312

13131313
user_context = project_with_holdouts.create_user_context('testUserId', user_attributes)
13141314

1315-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1315+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
13161316
[feature_flag],
13171317
user_context,
13181318
config_with_holdouts,
@@ -1334,7 +1334,7 @@
13341334

13351335
user_context = project_with_holdouts.create_user_context('testUserId', {})
13361336

1337-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1337+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
13381338
[feature_flag],
13391339
user_context,
13401340
config_with_holdouts,
@@ -1357,7 +1357,7 @@
13571357
# Empty user ID should still be valid for bucketing
13581358
user_context = project_with_holdouts.create_user_context('', {})
13591359

1360-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1360+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
13611361
[feature_flag],
13621362
user_context,
13631363
config_with_holdouts,
@@ -1384,7 +1384,7 @@
13841384

13851385
user_context = project_with_holdouts.create_user_context('testUserId', {})
13861386

1387-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1387+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
13881388
[feature_flag],
13891389
user_context,
13901390
config_with_holdouts,
@@ -1396,12 +1396,12 @@
13961396
# With real bucketer, we expect proper decision reasons to be generated
13971397
# Find any decision with reasons
13981398
decision_with_reasons = result.find do |decision_result|
1399-
decision_result.reasons && decision_result.reasons.length > 0
1399+
if decision_result.reasons && !decision_result.reasons.empty?
1400+
true
1401+
end
14001402
end
14011403

1402-
if decision_with_reasons
1403-
expect(decision_with_reasons.reasons.length).to be > 0
1404-
end
1404+
expect(decision_with_reasons.reasons).not_to be_empty if decision_with_reasons
14051405
end
14061406
end
14071407
end
@@ -1513,7 +1513,7 @@
15131513
if global_holdouts.length > 0
15141514
user_context = project_with_holdouts.create_user_context('testUserId', {})
15151515

1516-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1516+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
15171517
[feature_flag],
15181518
user_context,
15191519
config_with_holdouts,
@@ -1569,7 +1569,7 @@
15691569

15701570
user_context = project_with_holdouts.create_user_context('testUserId', {})
15711571

1572-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1572+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
15731573
[feature_flag],
15741574
user_context,
15751575
config_with_holdouts,
@@ -1589,7 +1589,7 @@
15891589
user_context = project_with_holdouts.create_user_context('testUserId', {})
15901590

15911591
# The method should handle invalid holdout data without crashing
1592-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1592+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
15931593
[feature_flag],
15941594
user_context,
15951595
config_with_holdouts,
@@ -1651,7 +1651,7 @@
16511651

16521652
user_context = project_with_holdouts.create_user_context('testUserId', user_attributes)
16531653

1654-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1654+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
16551655
[feature_flag],
16561656
user_context,
16571657
config_with_holdouts,
@@ -1669,12 +1669,12 @@
16691669
expect(feature_flag).not_to be_nil
16701670

16711671
# Test with multiple users to see varying bucketing results
1672-
users = ['user1', 'user2', 'user3', 'user4', 'user5']
1672+
users = %w[user1 user2 user3 user4 user5]
16731673
results = []
16741674

16751675
users.each do |user_id|
16761676
user_context = project_with_holdouts.create_user_context(user_id, {})
1677-
result = decision_service_with_holdouts.get_variations_for_feature_list(
1677+
_result = decision_service_with_holdouts.get_variations_for_feature_list(
16781678
[feature_flag],
16791679
user_context,
16801680
config_with_holdouts,

0 commit comments

Comments
 (0)