Skip to content

Commit 96a253d

Browse files
[OASIS-8370] Handle null, empty, or whitespaces for RolloutId (#304)
* Handle null, empty, or whitespaces for rolloutId Includes unit test covering existing logic * Add e2e test using retrieved config from datafile * Satisfy .NET 3.5 lack of string.IsNullOrWhiteSpace() Grrr we have to put up with this until 2029 unless we check w/ our clients to see if we can remove it. * Copyright updates I need to get those pre-commit hooks running * Code review corrections.
1 parent 689804a commit 96a253d

File tree

4 files changed

+77
-3
lines changed

4 files changed

+77
-3
lines changed

OptimizelySDK.Tests/EmptyRolloutRule.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@
2525
],
2626
"id": "18244658520",
2727
"key": "empty_rollout"
28+
},
29+
{
30+
"experimentIds": [],
31+
"rolloutId": "",
32+
"variables": [
33+
{
34+
"defaultValue": "",
35+
"type": "string",
36+
"id": "2832355113",
37+
"key": "var_2"
38+
}
39+
],
40+
"id": "24246538512",
41+
"key": "empty_rollout_id"
2842
}
2943
],
3044
"experiments": [],

OptimizelySDK.Tests/ProjectConfigTest.cs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017-2021, Optimizely
2+
* Copyright 2017-2022, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -822,5 +822,51 @@ public void TestIsFeatureExperimentReturnsTrueForExperimentThatBelongsToAFeature
822822

823823
Assert.True(typedConfig.IsFeatureExperiment(experiment.Id));
824824
}
825+
826+
[Test]
827+
public void TestRolloutWithEmptyStringRolloutIdFromConfigFile()
828+
{
829+
var projectConfig = DatafileProjectConfig.Create(TestData.EmptyRolloutDatafile, null, null);
830+
Assert.IsNotNull(projectConfig);
831+
var featureFlag = projectConfig.FeatureKeyMap["empty_rollout_id"];
832+
833+
var rollout = projectConfig.GetRolloutFromId(featureFlag.RolloutId);
834+
835+
Assert.IsNull(rollout.Experiments);
836+
Assert.IsNull(rollout.Id);
837+
}
838+
839+
[Test]
840+
public void TestRolloutWithEmptyStringRolloutId()
841+
{
842+
var rolloutId = string.Empty;
843+
844+
var rollout = Config.GetRolloutFromId(rolloutId);
845+
846+
Assert.IsNull(rollout.Experiments);
847+
Assert.IsNull(rollout.Id);
848+
}
849+
850+
[Test]
851+
public void TestRolloutWithConsistingOfASingleSpaceRolloutId()
852+
{
853+
var rolloutId = " "; // single space
854+
855+
var rollout = Config.GetRolloutFromId(rolloutId);
856+
857+
Assert.IsNull(rollout.Experiments);
858+
Assert.IsNull(rollout.Id);
859+
}
860+
861+
[Test]
862+
public void TestRolloutWithConsistingOfANullRolloutId()
863+
{
864+
string nullRolloutId = null;
865+
866+
var rollout = Config.GetRolloutFromId(nullRolloutId);
867+
868+
Assert.IsNull(rollout.Experiments);
869+
Assert.IsNull(rollout.Id);
870+
}
825871
}
826872
}

OptimizelySDK.sln.DotSettings

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/CASE_BLOCK_BRACES/@EntryValue">END_OF_LINE</s:String>
1414
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/EMPTY_BLOCK_STYLE/@EntryValue">TOGETHER_SAME_LINE</s:String>
1515
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/KEEP_EXISTING_EXPR_MEMBER_ARRANGEMENT/@EntryValue">False</s:Boolean>
16+
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/KEEP_EXISTING_INITIALIZER_ARRANGEMENT/@EntryValue">False</s:Boolean>
1617
<s:Int64 x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/MAX_INITIALIZER_ELEMENTS_ON_LINE/@EntryValue">0</s:Int64>
1718
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/OTHER_BRACES/@EntryValue">END_OF_LINE</s:String>
1819
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/PLACE_ACCESSOR_ATTRIBUTE_ON_SAME_LINE_EX/@EntryValue">NEVER</s:String>
@@ -40,4 +41,8 @@
4041
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=EnumMember/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb"&gt;&lt;ExtraRule Prefix="" Suffix="" Style="AaBb" /&gt;&lt;/Policy&gt;</s:String>
4142
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=Interfaces/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /&gt;</s:String>
4243
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=LocalConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /&gt;</s:String>
43-
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String></wpf:ResourceDictionary>
44+
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
45+
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
46+
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
47+
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean>
48+
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

OptimizelySDK/Config/DatafileProjectConfig.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019-2021, Optimizely
2+
* Copyright 2019-2022, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -636,6 +636,15 @@ public Variation GetFlagVariationByKey(string flagKey, string variationKey)
636636
/// <returns>Rollout Entity corresponding to the rollout ID or a dummy entity if ID is invalid</returns>
637637
public Rollout GetRolloutFromId(string rolloutId)
638638
{
639+
#if NET35 || NET40
640+
if (string.IsNullOrEmpty(rolloutId) || string.IsNullOrEmpty(rolloutId.Trim()))
641+
#else
642+
if (string.IsNullOrWhiteSpace(rolloutId))
643+
#endif
644+
{
645+
return new Rollout();
646+
}
647+
639648
if (_RolloutIdMap.ContainsKey(rolloutId))
640649
return _RolloutIdMap[rolloutId];
641650

0 commit comments

Comments
 (0)