From d9db9a5c58f414b24ef90a2aeedf11056ee85c7e Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 4 Nov 2022 17:08:32 -0400 Subject: [PATCH 01/21] WIP Initial SegmentManager commit --- OptimizelySDK.sln.DotSettings | 3 +- OptimizelySDK/Odp/Constants.cs | 10 +++ OptimizelySDK/Odp/Enums.cs | 7 +- OptimizelySDK/Odp/IOdpConfig.cs | 59 ++++++++++++++++ OptimizelySDK/Odp/IOdpSegmentManager.cs | 11 +++ OptimizelySDK/Odp/LruCache.cs | 9 +-- OptimizelySDK/Odp/OdpConfig.cs | 84 +++++++++++++++++++++++ OptimizelySDK/Odp/OdpSegmentManager.cs | 89 +++++++++++++++++++++++++ OptimizelySDK/OptimizelySDK.csproj | 4 ++ 9 files changed, 267 insertions(+), 9 deletions(-) create mode 100644 OptimizelySDK/Odp/IOdpConfig.cs create mode 100644 OptimizelySDK/Odp/IOdpSegmentManager.cs create mode 100644 OptimizelySDK/Odp/OdpConfig.cs create mode 100644 OptimizelySDK/Odp/OdpSegmentManager.cs diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index d2bd6370..7ed7b3dc 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -48,4 +48,5 @@ <s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean> <s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean> <s:Boolean x:Key="/Default/UserDictionary/Words/=ODP_0027s/@EntryIndexedValue">True</s:Boolean> - <s:Boolean x:Key="/Default/UserDictionary/Words/=vuid/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary> + <s:Boolean x:Key="/Default/UserDictionary/Words/=vuid/@EntryIndexedValue">True</s:Boolean> + <s:Boolean x:Key="/Default/UserDictionary/Words/=VUID/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary> diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index 5ce7d6d3..214bffa0 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -42,5 +42,15 @@ public static class Constants /// Default message when numeric HTTP status code is not available /// </summary> public const string NETWORK_ERROR_REASON = "network error"; + + /// <summary> + /// Default maximum number of elements to cache + /// </summary> + public const int DEFAULT_MAX_CACHE_SIZE = 10000; + + /// <summary> + /// Default number of minutes to cache + /// </summary> + public const int DEFAULT_CACHE_MINUTES = 10; } } diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index 0209b7a8..f8f6590e 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -19,8 +19,13 @@ namespace OptimizelySDK.Odp public enum OdpUserKeyType { // ReSharper disable InconsistentNaming - // ODP expects these names; .ToString() used + // ODP expects these names in UPPERCASE; .ToString() used VUID = 0, FS_USER_ID = 1, } + public enum OdpSegmentOption + { + IgnoreCache = 0, + ResetCache = 1, + } } diff --git a/OptimizelySDK/Odp/IOdpConfig.cs b/OptimizelySDK/Odp/IOdpConfig.cs new file mode 100644 index 00000000..55780915 --- /dev/null +++ b/OptimizelySDK/Odp/IOdpConfig.cs @@ -0,0 +1,59 @@ +/* + * Copyright 2022, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public interface IOdpConfig + { + /// <summary> + /// Public API key for the ODP account from which the audience segments will be fetched (optional). + /// </summary> + string ApiKey { get; } + + /// <summary> + /// Host of ODP audience segments API. + /// </summary> + string ApiHost { get; } + + /// <summary> + /// All ODP segments used in the current datafile (associated with apiHost/apiKey). + /// </summary> + List<string> SegmentsToCheck { get; } + + /// <summary> + /// Update the ODP configuration details + /// </summary> + /// <param name="apiKey">Public API key for the ODP account</param> + /// <param name="apiHost">Host of ODP audience segments API</param> + /// <param name="segmentsToCheck">Audience segments</param> + /// <returns>true if configuration was updated successfully otherwise false</returns> + bool Update(string apiKey, string apiHost, List<string> segmentsToCheck); + + /// <summary> + /// Determines if ODP configuration has the minimum amount of information + /// </summary> + /// <returns>true if ODP configuration can be used otherwise false</returns> + bool IsReady(); + + /// <summary> + /// Determines if ODP configuration contains segments + /// </summary> + /// <returns></returns> + bool HasSegments(); + } +} diff --git a/OptimizelySDK/Odp/IOdpSegmentManager.cs b/OptimizelySDK/Odp/IOdpSegmentManager.cs new file mode 100644 index 00000000..66992b84 --- /dev/null +++ b/OptimizelySDK/Odp/IOdpSegmentManager.cs @@ -0,0 +1,11 @@ +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public class IOdpSegmentManager + { + public List<string> GetQualifiedSegments(string fsUserId, + List<OdpSegmentOption> options = null + ); + } +} diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index c7b04d86..0e231395 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -25,11 +25,6 @@ namespace OptimizelySDK.Odp public class LruCache<T> : ICache<T> where T : class { - /// <summary> - /// Default maximum number of elements to store - /// </summary> - private const int DEFAULT_MAX_SIZE = 10000; - /// <summary> /// The maximum number of elements that should be stored /// </summary> @@ -66,7 +61,7 @@ public class LruCache<T> : ICache<T> /// <param name="maxSize">Maximum number of elements to allow in the cache</param> /// <param name="itemTimeout">Timeout or time to live for each item</param> /// <param name="logger">Implementation used for recording LRU events or errors</param> - public LruCache(int maxSize = DEFAULT_MAX_SIZE, TimeSpan? itemTimeout = default, + public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = default, ILogger logger = null ) { @@ -76,7 +71,7 @@ public LruCache(int maxSize = DEFAULT_MAX_SIZE, TimeSpan? itemTimeout = default, _logger = logger ?? new DefaultLogger(); - _timeout = itemTimeout ?? TimeSpan.FromMinutes(10); + _timeout = itemTimeout ?? TimeSpan.FromMinutes(Constants.DEFAULT_CACHE_MINUTES); if (_timeout < TimeSpan.Zero) { _logger.Log(LogLevel.WARN, diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs new file mode 100644 index 00000000..fd9d31d0 --- /dev/null +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -0,0 +1,84 @@ +/* + * Copyright 2022, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public class OdpConfig : IOdpConfig + { + /// <summary> + /// Public API key for the ODP account from which the audience segments will be fetched (optional). + /// </summary> + public string ApiKey { get; private set; } + + /// <summary> + /// Host of ODP audience segments API. + /// </summary> + public string ApiHost { get; private set; } + + /// <summary> + /// All ODP segments used in the current datafile (associated with apiHost/apiKey). + /// </summary> + public List<string> SegmentsToCheck { get; private set; } + + public OdpConfig(string apiKey, string apiHost, List<string> segmentsToCheck) + { + ApiKey = apiKey; + ApiHost = apiHost; + SegmentsToCheck = segmentsToCheck ?? new List<string>(0); + } + + /// <summary> + /// Update the ODP configuration details + /// </summary> + /// <param name="apiKey">Public API key for the ODP account</param> + /// <param name="apiHost">Host of ODP audience segments API</param> + /// <param name="segmentsToCheck">Audience segments</param> + /// <returns>true if configuration was updated successfully otherwise false</returns> + public bool Update(string apiKey, string apiHost, List<string> segmentsToCheck) + { + if (ApiKey == apiKey && ApiHost == apiHost && SegmentsToCheck == segmentsToCheck) + { + return false; + } + + ApiKey = apiKey; + ApiHost = apiHost; + SegmentsToCheck = segmentsToCheck; + + return true; + } + + /// <summary> + /// Determines if ODP configuration has the minimum amount of information + /// </summary> + /// <returns>true if ODP configuration can be used otherwise false</returns> + public bool IsReady() + { + return !string.IsNullOrWhiteSpace(ApiKey) && !string.IsNullOrWhiteSpace(ApiHost); + } + + /// <summary> + /// Determines if ODP configuration contains segments + /// </summary> + /// <returns></returns> + public bool HasSegments() + { + return SegmentsToCheck?.Count > 0; + } + } +} diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs new file mode 100644 index 00000000..63a1f5f9 --- /dev/null +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -0,0 +1,89 @@ +using OptimizelySDK.Logger; +using System; +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public class OdpSegmentManager + { + private readonly ILogger _logger; + private readonly IOdpSegmentApiManager _apiManager; + private readonly IOdpConfig _odpConfig; + + private readonly Cache<List<string>> _segmentsCache; + + public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, + int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = default, + ILogger logger = null + ) + { + _apiManager = apiManager; + _odpConfig = odpConfig; + _logger = logger ?? new DefaultLogger(); + + var timeout = itemTimeout ?? TimeSpan.FromMinutes(Constants.DEFAULT_CACHE_MINUTES); + if (timeout < TimeSpan.Zero) + { + _logger.Log(LogLevel.WARN, + "Negative item timeout provided. Items will not expire in cache."); + timeout = TimeSpan.Zero; + } + + _segmentsCache = new LruCache<List<string>>(cacheSize, timeout, logger); + } + + public List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> options = null) + { + if (!_odpConfig.IsReady()) + { + _logger.Log(LogLevel.ERROR, "Audience segments fetch failed (ODP is not enabled)"); + return new List<string>(); + } + + if (!_odpConfig.HasSegments()) + { + _logger.Log(LogLevel.DEBUG, + "No Segments are used in the project, Not Fetching segments. Returning empty list"); + return new List<string>(); + } + + List<string> qualifiedSegments; + var cacheKey = GetCacheKey(OdpUserKeyType.FS_USER_ID.ToString().ToLower(), fsUserId); + + if (options?.Contains(OdpSegmentOption.ResetCache)) + { + _segmentsCache.reset(); + } + else if (!options.Contains(OdpSegmentOption.IgnoreCache)) + { + qualifiedSegments = _segmentsCache.lookup(cacheKey); + if (qualifiedSegments != null) + { + _logger.Log(LogLevel.DEBUG,"ODP Cache Hit. Returning segments from Cache."); + return qualifiedSegments; + } + } + + _logger.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server."); + + var parser = ResponseJsonParserFactory.getParser(); + var qualifiedSegmentsResponse = _apiManager.FetchSegments( + _odpConfig.ApiKey, + _odpConfig.ApiHost + Constants.ODP_GRAPHQL_API_ENDPOINT_PATH, + OdpUserKeyType.FS_USER_ID, fsUserId, _odpConfig.SegmentsToCheck); + qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); + + if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IgnoreCache)) + { + _segmentsCache.save(cacheKey, qualifiedSegments); + } + + return qualifiedSegments; + } + + private static string GetCacheKey(string userKey, string userValue) + { + return userKey + "-$-" + userValue; + } + } +} diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index d3ecc043..8008ac04 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -100,12 +100,16 @@ <Compile Include="Odp\Entity\OdpEvent.cs" /> <Compile Include="Odp\Entity\Response.cs" /> <Compile Include="Odp\Enums.cs" /> + <Compile Include="Odp\IOdpConfig.cs" /> + <Compile Include="Odp\IOdpSegmentManager.cs" /> + <Compile Include="Odp\OdpConfig.cs" /> <Compile Include="Odp\OdpSegmentApiManager.cs" /> <Compile Include="Odp\IOdpSegmentApiManager.cs" /> <Compile Include="Odp\ICache.cs" /> <Compile Include="Odp\IOdpEventApiManager.cs" /> <Compile Include="Odp\LruCache.cs" /> <Compile Include="Odp\OdpEventApiManager.cs" /> + <Compile Include="Odp\OdpSegmentManager.cs" /> <Compile Include="OptimizelyDecisions\DecisionMessage.cs" /> <Compile Include="OptimizelyDecisions\DecisionReasons.cs" /> <Compile Include="OptimizelyDecisions\OptimizelyDecideOption.cs" /> From e5fcd9a5c100210a109e84b4f64420e02a8cdb7b Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 4 Nov 2022 17:10:54 -0400 Subject: [PATCH 02/21] WIP Initial commit fixes --- OptimizelySDK/Odp/IOdpSegmentManager.cs | 6 ++---- OptimizelySDK/Odp/OdpSegmentManager.cs | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/OptimizelySDK/Odp/IOdpSegmentManager.cs b/OptimizelySDK/Odp/IOdpSegmentManager.cs index 66992b84..6456dc75 100644 --- a/OptimizelySDK/Odp/IOdpSegmentManager.cs +++ b/OptimizelySDK/Odp/IOdpSegmentManager.cs @@ -2,10 +2,8 @@ namespace OptimizelySDK.Odp { - public class IOdpSegmentManager + public interface IOdpSegmentManager { - public List<string> GetQualifiedSegments(string fsUserId, - List<OdpSegmentOption> options = null - ); + List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> options = null); } } diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 63a1f5f9..e23fa8b1 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -50,7 +50,7 @@ public List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> List<string> qualifiedSegments; var cacheKey = GetCacheKey(OdpUserKeyType.FS_USER_ID.ToString().ToLower(), fsUserId); - if (options?.Contains(OdpSegmentOption.ResetCache)) + if (options.Contains(OdpSegmentOption.ResetCache)) { _segmentsCache.reset(); } @@ -66,11 +66,12 @@ public List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> _logger.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server."); - var parser = ResponseJsonParserFactory.getParser(); var qualifiedSegmentsResponse = _apiManager.FetchSegments( _odpConfig.ApiKey, _odpConfig.ApiHost + Constants.ODP_GRAPHQL_API_ENDPOINT_PATH, OdpUserKeyType.FS_USER_ID, fsUserId, _odpConfig.SegmentsToCheck); + + var parser = ResponseJsonParserFactory.getParser(); qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IgnoreCache)) From 61d3e0f9222afe4f1dc667e5c3d5ff33a2101571 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 4 Nov 2022 17:10:54 -0400 Subject: [PATCH 03/21] WIP Initial commit fixes --- OptimizelySDK/Odp/OdpSegmentManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index e23fa8b1..6ae29ca4 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -4,7 +4,7 @@ namespace OptimizelySDK.Odp { - public class OdpSegmentManager + public class OdpSegmentManager: IOdpSegmentManager { private readonly ILogger _logger; private readonly IOdpSegmentApiManager _apiManager; From c4db70a72536f29ff81ebfc6b187b67379dfe72b Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Tue, 8 Nov 2022 15:09:41 -0500 Subject: [PATCH 04/21] Finish OdpSegmentManager & interface --- OptimizelySDK/Odp/IOdpSegmentManager.cs | 30 ++++++++- OptimizelySDK/Odp/OdpSegmentManager.cs | 88 +++++++++++++++++++------ 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/OptimizelySDK/Odp/IOdpSegmentManager.cs b/OptimizelySDK/Odp/IOdpSegmentManager.cs index 6456dc75..0a316e24 100644 --- a/OptimizelySDK/Odp/IOdpSegmentManager.cs +++ b/OptimizelySDK/Odp/IOdpSegmentManager.cs @@ -1,9 +1,35 @@ -using System.Collections.Generic; +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; namespace OptimizelySDK.Odp { + /// <summary> + /// Interface to schedule connections to ODP for audience segmentation and caches the results. + /// </summary> public interface IOdpSegmentManager { - List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> options = null); + /// <summary> + /// Attempts to fetch and return a list of a user's qualified segments from the local segments cache. + /// If no cached data exists for the target user, this fetches and caches data from the ODP server instead. + /// </summary> + /// <param name="fsUserId">The FS User ID identifying the user</param> + /// <param name="options">An array of OptimizelySegmentOption used to ignore and/or reset the cache.</param> + /// <returns>Qualified segments for the user from the cache or the ODP server if the cache is empty.</returns> + List<string> FetchQualifiedSegments(string fsUserId, List<OdpSegmentOption> options = null); } } diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 6ae29ca4..22a5a646 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -1,19 +1,54 @@ -using OptimizelySDK.Logger; +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using OptimizelySDK.Logger; using System; using System.Collections.Generic; +using System.Linq; namespace OptimizelySDK.Odp { - public class OdpSegmentManager: IOdpSegmentManager + /// <summary> + /// Concrete implementation that schedules connections to ODP for audience segmentation + /// and caches the results. + /// </summary> + public class OdpSegmentManager : IOdpSegmentManager { + /// <summary> + /// Logger used to record messages that occur within the ODP client + /// </summary> private readonly ILogger _logger; + + /// <summary> + /// ODP segment API manager to communicate with ODP + /// </summary> private readonly IOdpSegmentApiManager _apiManager; + + /// <summary> + /// ODP configuration containing the connection parameters + /// </summary> private readonly IOdpConfig _odpConfig; - private readonly Cache<List<string>> _segmentsCache; + /// <summary> + /// Cached segments + /// </summary> + private readonly LruCache<List<string>> _segmentsCache; public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, - int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = default, + int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = null, ILogger logger = null ) { @@ -32,7 +67,16 @@ public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, _segmentsCache = new LruCache<List<string>>(cacheSize, timeout, logger); } - public List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> options = null) + /// <summary> + /// Attempts to fetch and return a list of a user's qualified segments from the local segments cache. + /// If no cached data exists for the target user, this fetches and caches data from the ODP server instead. + /// </summary> + /// <param name="fsUserId">The FS User ID identifying the user</param> + /// <param name="options">An array of OptimizelySegmentOption used to ignore and/or reset the cache.</param> + /// <returns>Qualified segments for the user from the cache or the ODP server if the cache is empty.</returns> + public List<string> FetchQualifiedSegments(string fsUserId, + List<OdpSegmentOption> options = null + ) { if (!_odpConfig.IsReady()) { @@ -46,45 +90,51 @@ public List<string> GetQualifiedSegments(string fsUserId, List<OdpSegmentOption> "No Segments are used in the project, Not Fetching segments. Returning empty list"); return new List<string>(); } - + + options = options ?? new List<OdpSegmentOption>(); + List<string> qualifiedSegments; var cacheKey = GetCacheKey(OdpUserKeyType.FS_USER_ID.ToString().ToLower(), fsUserId); if (options.Contains(OdpSegmentOption.ResetCache)) { - _segmentsCache.reset(); + _segmentsCache.Reset(); } else if (!options.Contains(OdpSegmentOption.IgnoreCache)) { - qualifiedSegments = _segmentsCache.lookup(cacheKey); + qualifiedSegments = _segmentsCache.Lookup(cacheKey); if (qualifiedSegments != null) { - _logger.Log(LogLevel.DEBUG,"ODP Cache Hit. Returning segments from Cache."); + _logger.Log(LogLevel.DEBUG, "ODP Cache Hit. Returning segments from Cache."); return qualifiedSegments; } } _logger.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server."); - var qualifiedSegmentsResponse = _apiManager.FetchSegments( - _odpConfig.ApiKey, - _odpConfig.ApiHost + Constants.ODP_GRAPHQL_API_ENDPOINT_PATH, - OdpUserKeyType.FS_USER_ID, fsUserId, _odpConfig.SegmentsToCheck); - - var parser = ResponseJsonParserFactory.getParser(); - qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); + qualifiedSegments = _apiManager.FetchSegments( + _odpConfig.ApiKey, + _odpConfig.ApiHost + Constants.ODP_GRAPHQL_API_ENDPOINT_PATH, + OdpUserKeyType.FS_USER_ID, fsUserId, _odpConfig.SegmentsToCheck) + .ToList(); - if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IgnoreCache)) + if (!options.Contains(OdpSegmentOption.IgnoreCache)) { - _segmentsCache.save(cacheKey, qualifiedSegments); + _segmentsCache.Save(cacheKey, qualifiedSegments); } return qualifiedSegments; } + /// <summary> + /// Creates a key used to identify which user fetchQualifiedSegments should lookup and save to in the segments cache + /// </summary> + /// <param name="userKey">Always 'fs_user_id' (parameter for consistency with other SDKs)</param> + /// <param name="userValue">Arbitrary string representing the full stack user ID</param> + /// <returns>Concatenates inputs and returns the string "{userKey}-$-{userValue}"</returns> private static string GetCacheKey(string userKey, string userValue) { - return userKey + "-$-" + userValue; + return $"{userKey}-$-{userValue}"; } } } From b79861ebc6434290ed0dde73cdacbf6cc9d7bf4b Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Tue, 8 Nov 2022 16:07:03 -0500 Subject: [PATCH 05/21] WIP unit tests starts --- .../OdpTests/OdpSegmentManagerTest.cs | 113 ++++++++++++++++++ .../OptimizelySDK.Tests.csproj | 1 + OptimizelySDK/Odp/OdpSegmentManager.cs | 4 +- 3 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs new file mode 100644 index 00000000..1676e3fe --- /dev/null +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -0,0 +1,113 @@ +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using Moq; +using NUnit.Framework; +using OptimizelySDK.AudienceConditions; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Logger; +using OptimizelySDK.Odp; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; + +namespace OptimizelySDK.Tests.OdpTests +{ + [TestFixture] + public class OdpSegmentManagerTest + { + private const string API_KEY = "S0m3Ap1KEy4U"; + private const string API_HOST = "https://odp-host.example.com"; + private const string FS_USER_ID = "some_valid_user_id"; + + private readonly List<string> _segmentsToCheck = new List<string> + { + "segment1", + "segment2", + }; + + private OdpConfig _odpConfig; + private Mock<IOdpSegmentApiManager> _mockApiManager; + private Mock<ILogger> _mockLogger; + private Mock<ICache<List<string>>> _mockCache; + + [SetUp] + public void Setup() + { + _odpConfig = new OdpConfig(API_KEY, API_HOST, _segmentsToCheck); + + _mockApiManager = new Mock<IOdpSegmentApiManager>(); + + _mockLogger = new Mock<ILogger>(); + _mockLogger.Setup(i => i.Log(It.IsAny<LogLevel>(), It.IsAny<string>())); + + _mockCache = new Mock<ICache<List<string>>>(); + } + + [Test] + public void ShouldFetchSegmentsOnCacheMiss() + { + var keyCollector = new List<string>(); + _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))) + .Returns(default(List<string>)); + _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())) + .Returns(_segmentsToCheck.ToArray()); + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + var segments = manager.FetchQualifiedSegments(FS_USER_ID); + + var cacheKey = keyCollector.FirstOrDefault(); + Assert.AreEqual($"fs_user_id-$-{FS_USER_ID}", cacheKey); + _mockApiManager.Verify(a => a.FetchSegments(API_KEY, API_HOST, + OdpUserKeyType.FS_USER_ID, FS_USER_ID, _odpConfig.SegmentsToCheck), Times.Once); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server.")); + Assert.AreEqual(_segmentsToCheck, segments); + // verify(mockApiManager, times(1)) + // .fetchQualifiedSegments(odpConfig.getApiKey(), + // odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", + // Arrays.asList("segment1", "segment2")); + // verify(mockCache, times(1)) + // .save("vuid-$-testId", Arrays.asList("segment1", "segment2")); + // verify(mockCache, times(0)).reset(); + // + // + // assertEquals(Arrays.asList("segment1", "segment2"), segments); + } + + [Test] + public void ShouldFetchSegmentsSuccessOnCacheHit() { } + + [Test] + public void ShouldHandleFetchSegmentsWithError() { } + + [Test] + public void ShouldIgnoreCache() { } + + [Test] + public void ShouldResetCache() { } + + [Test] + public void ShouldMakeValidCacheKey() { } + + private void setCache() { } + + private void peekCache() { } + } +} diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index 3271653f..cd06e76e 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -84,6 +84,7 @@ <Compile Include="OdpTests\HttpClientTestUtil.cs" /> <Compile Include="OdpTests\LruCacheTest.cs" /> <Compile Include="OdpTests\OdpEventApiManagerTest.cs" /> + <Compile Include="OdpTests\OdpSegmentManagerTest.cs" /> <Compile Include="OptimizelyConfigTests\OptimizelyConfigTest.cs" /> <Compile Include="OptimizelyDecisions\OptimizelyDecisionTest.cs" /> <Compile Include="OptimizelyJSONTest.cs" /> diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 22a5a646..df58b8c1 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -49,7 +49,7 @@ public class OdpSegmentManager : IOdpSegmentManager public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = null, - ILogger logger = null + ILogger logger = null, ICache<List<string>> cache = null ) { _apiManager = apiManager; @@ -64,7 +64,7 @@ public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, timeout = TimeSpan.Zero; } - _segmentsCache = new LruCache<List<string>>(cacheSize, timeout, logger); + _segmentsCache = cache as LruCache<List<string>> ?? new LruCache<List<string>>(cacheSize, timeout, logger); } /// <summary> From bc60e1f74266aef62b77c4cce154e9e1b6157cbe Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Wed, 9 Nov 2022 09:57:03 -0500 Subject: [PATCH 06/21] Unit tests & Segment Manager edits to satisfy the unit tests. --- .../OdpTests/OdpSegmentManagerTest.cs | 134 ++++++++++++++---- OptimizelySDK/Odp/OdpSegmentManager.cs | 11 +- 2 files changed, 115 insertions(+), 30 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index 1676e3fe..b781020a 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -34,7 +34,9 @@ public class OdpSegmentManagerTest private const string API_HOST = "https://odp-host.example.com"; private const string FS_USER_ID = "some_valid_user_id"; - private readonly List<string> _segmentsToCheck = new List<string> + private static readonly string expectedCacheKey = $"fs_user_id-$-{FS_USER_ID}"; + + private static readonly List<string> segmentsToCheck = new List<string> { "segment1", "segment2", @@ -48,7 +50,7 @@ public class OdpSegmentManagerTest [SetUp] public void Setup() { - _odpConfig = new OdpConfig(API_KEY, API_HOST, _segmentsToCheck); + _odpConfig = new OdpConfig(API_KEY, API_HOST, segmentsToCheck); _mockApiManager = new Mock<IOdpSegmentApiManager>(); @@ -66,48 +68,130 @@ public void ShouldFetchSegmentsOnCacheMiss() .Returns(default(List<string>)); _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())) - .Returns(_segmentsToCheck.ToArray()); + .Returns(segmentsToCheck.ToArray()); var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); var cacheKey = keyCollector.FirstOrDefault(); - Assert.AreEqual($"fs_user_id-$-{FS_USER_ID}", cacheKey); - _mockApiManager.Verify(a => a.FetchSegments(API_KEY, API_HOST, - OdpUserKeyType.FS_USER_ID, FS_USER_ID, _odpConfig.SegmentsToCheck), Times.Once); + Assert.AreEqual(expectedCacheKey, cacheKey); + _mockCache.Verify(c => c.Reset(), Times.Never); + _mockCache.Verify(c => c.Lookup(cacheKey), Times.Once); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server.")); - Assert.AreEqual(_segmentsToCheck, segments); - // verify(mockApiManager, times(1)) - // .fetchQualifiedSegments(odpConfig.getApiKey(), - // odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", - // Arrays.asList("segment1", "segment2")); - // verify(mockCache, times(1)) - // .save("vuid-$-testId", Arrays.asList("segment1", "segment2")); - // verify(mockCache, times(0)).reset(); - // - // - // assertEquals(Arrays.asList("segment1", "segment2"), segments); + l.Log(LogLevel.DEBUG, "ODP Cache Miss. Making a call to ODP Server."), Times.Once); + _mockApiManager.Verify( + a => a.FetchSegments( + API_KEY, + API_HOST, + OdpUserKeyType.FS_USER_ID, + FS_USER_ID, + _odpConfig.SegmentsToCheck), Times.Once); + _mockCache.Verify(c => c.Save(cacheKey, It.IsAny<List<string>>()), Times.Once); + Assert.AreEqual(segmentsToCheck, segments); } [Test] - public void ShouldFetchSegmentsSuccessOnCacheHit() { } + public void ShouldFetchSegmentsSuccessOnCacheHit() + { + var keyCollector = new List<string>(); + _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))) + .Returns(segmentsToCheck); + _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())); + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + var segments = manager.FetchQualifiedSegments(FS_USER_ID); + + var cacheKey = keyCollector.FirstOrDefault(); + Assert.AreEqual(expectedCacheKey, cacheKey); + _mockCache.Verify(c => c.Reset(), Times.Never); + _mockCache.Verify(c => c.Lookup(cacheKey), Times.Once); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP Cache Hit. Returning segments from Cache."), Times.Once); + _mockApiManager.Verify( + a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), + Times.Never); + _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Never); + Assert.AreEqual(segmentsToCheck, segments); + } [Test] - public void ShouldHandleFetchSegmentsWithError() { } + public void ShouldHandleFetchSegmentsWithError() + { + // OdpSegmentApiManager.FetchSegments() return null on any error + _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())) + .Returns(null as string[]); + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + var segments = manager.FetchQualifiedSegments(FS_USER_ID); + + _mockCache.Verify(c => c.Reset(), Times.Never); + _mockCache.Verify(c => c.Lookup(expectedCacheKey), Times.Once); + _mockApiManager.Verify( + a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), + Times.Once); + _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Once); + Assert.IsNull(segments); + } [Test] - public void ShouldIgnoreCache() { } + public void ShouldIgnoreCache() + { + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + manager.FetchQualifiedSegments(FS_USER_ID, new List<OdpSegmentOption> + { + OdpSegmentOption.IgnoreCache, + }); + + _mockCache.Verify(c => c.Reset(), Times.Never); + _mockCache.Verify(c => c.Lookup(It.IsAny<string>()), Times.Never); + _mockApiManager.Verify( + a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), + Times.Once); + _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Never); + } [Test] - public void ShouldResetCache() { } + public void ShouldResetCache() + { + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + manager.FetchQualifiedSegments(FS_USER_ID, new List<OdpSegmentOption> + { + OdpSegmentOption.ResetCache, + }); + + _mockCache.Verify(c => c.Reset(), Times.Once); + _mockCache.Verify(c => c.Lookup(It.IsAny<string>()), Times.Never); + _mockApiManager.Verify( + a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), + Times.Once); + _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Once); + } [Test] - public void ShouldMakeValidCacheKey() { } + public void ShouldMakeValidCacheKey() + { + var keyCollector = new List<string>(); + _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))); + var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); - private void setCache() { } + manager.FetchQualifiedSegments(FS_USER_ID); - private void peekCache() { } + var cacheKey = keyCollector.FirstOrDefault(); + Assert.AreEqual(expectedCacheKey, cacheKey); + } } } diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index df58b8c1..bee31b70 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -45,7 +45,7 @@ public class OdpSegmentManager : IOdpSegmentManager /// <summary> /// Cached segments /// </summary> - private readonly LruCache<List<string>> _segmentsCache; + private readonly ICache<List<string>> _segmentsCache; public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = null, @@ -64,7 +64,7 @@ public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, timeout = TimeSpan.Zero; } - _segmentsCache = cache as LruCache<List<string>> ?? new LruCache<List<string>>(cacheSize, timeout, logger); + _segmentsCache = cache ?? new LruCache<List<string>>(cacheSize, timeout, logger); } /// <summary> @@ -114,9 +114,10 @@ public List<string> FetchQualifiedSegments(string fsUserId, qualifiedSegments = _apiManager.FetchSegments( _odpConfig.ApiKey, - _odpConfig.ApiHost + Constants.ODP_GRAPHQL_API_ENDPOINT_PATH, - OdpUserKeyType.FS_USER_ID, fsUserId, _odpConfig.SegmentsToCheck) - .ToList(); + _odpConfig.ApiHost, + OdpUserKeyType.FS_USER_ID, + fsUserId, + _odpConfig.SegmentsToCheck)?.ToList(); if (!options.Contains(OdpSegmentOption.IgnoreCache)) { From 4f62cd7e1d0aa4349a7bc8b0cba288cc36dc907e Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 15:38:19 -0500 Subject: [PATCH 07/21] Fix merge issues; Add unit test --- .../OdpTests/OdpSegmentManagerTest.cs | 16 ++++++++++++++++ OptimizelySDK/Odp/Constants.cs | 3 +++ OptimizelySDK/Odp/OdpSegmentManager.cs | 9 +++++---- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index b781020a..89ac1d41 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -140,6 +140,22 @@ public void ShouldHandleFetchSegmentsWithError() Assert.IsNull(segments); } + [Test] + public void ShouldLogAndReturnEmptySegmentsListWhenOdpConfigNotReady() + { + var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, new List<string>(0)); + mockOdpConfig.Setup(o => o.IsReady()).Returns(false); + var manager = new OdpSegmentManager(mockOdpConfig.Object, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + var segments = manager.FetchQualifiedSegments(FS_USER_ID); + + Assert.IsTrue(segments.Count == 0); + _mockLogger.Verify( + l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), + Times.Once); + } + [Test] public void ShouldIgnoreCache() { diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index f997d065..e2cfb6ad 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -14,6 +14,9 @@ * limitations under the License. */ +using System; +// ReSharper disable InconsistentNaming + namespace OptimizelySDK.Odp { public static class Constants diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index bee31b70..a887b860 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -40,14 +40,14 @@ public class OdpSegmentManager : IOdpSegmentManager /// <summary> /// ODP configuration containing the connection parameters /// </summary> - private readonly IOdpConfig _odpConfig; + private readonly OdpConfig _odpConfig; /// <summary> /// Cached segments /// </summary> private readonly ICache<List<string>> _segmentsCache; - public OdpSegmentManager(IOdpConfig odpConfig, IOdpSegmentApiManager apiManager, + public OdpSegmentManager(OdpConfig odpConfig, IOdpSegmentApiManager apiManager, int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = null, ILogger logger = null, ICache<List<string>> cache = null ) @@ -80,7 +80,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, { if (!_odpConfig.IsReady()) { - _logger.Log(LogLevel.ERROR, "Audience segments fetch failed (ODP is not enabled)"); + _logger.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE); return new List<string>(); } @@ -117,7 +117,8 @@ public List<string> FetchQualifiedSegments(string fsUserId, _odpConfig.ApiHost, OdpUserKeyType.FS_USER_ID, fsUserId, - _odpConfig.SegmentsToCheck)?.ToList(); + _odpConfig.SegmentsToCheck)?. + ToList(); if (!options.Contains(OdpSegmentOption.IgnoreCache)) { From a614e9f74621eedc98fd7dacda10d42086e573b8 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 15:54:04 -0500 Subject: [PATCH 08/21] Lint fixes --- OptimizelySDK/Odp/Constants.cs | 5 +++-- OptimizelySDK/Odp/LruCache.cs | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index e2cfb6ad..e44c47a5 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -15,6 +15,7 @@ */ using System; + // ReSharper disable InconsistentNaming namespace OptimizelySDK.Odp @@ -100,12 +101,12 @@ public static class Constants /// Default amount of time to wait for ODP response /// </summary> public static readonly TimeSpan DEFAULT_TIMEOUT_INTERVAL = TimeSpan.FromSeconds(10); - + /// <summary> /// Default maximum number of elements to cache /// </summary> public const int DEFAULT_MAX_CACHE_SIZE = 10000; - + /// <summary> /// Default number of minutes to cache /// </summary> diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 0e231395..64044cad 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -61,7 +61,8 @@ public class LruCache<T> : ICache<T> /// <param name="maxSize">Maximum number of elements to allow in the cache</param> /// <param name="itemTimeout">Timeout or time to live for each item</param> /// <param name="logger">Implementation used for recording LRU events or errors</param> - public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = default, + public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, + TimeSpan? itemTimeout = default, ILogger logger = null ) { From a42ed2153b9661a2f9406957c4680c6300772fdd Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 16:01:41 -0500 Subject: [PATCH 09/21] Lint fixes? --- OptimizelySDK/Odp/LruCache.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 64044cad..7badff8e 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -114,9 +114,10 @@ public void Save(string key, T value) { var leastRecentlyUsedItem = _list.Last; - var leastRecentlyUsedItemKey = (from cacheItem in _cache - where cacheItem.Value == leastRecentlyUsedItem.Value - select cacheItem.Key).FirstOrDefault(); + var leastRecentlyUsedItemKey = + (from cacheItem in _cache + where cacheItem.Value == leastRecentlyUsedItem.Value + select cacheItem.Key).FirstOrDefault(); if (leastRecentlyUsedItemKey != null) { From 7efe971f426c8e9b291799da93f26729fc8d9f5d Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 16:08:46 -0500 Subject: [PATCH 10/21] Lint fixes?? --- OptimizelySDK/Odp/LruCache.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 7badff8e..1505b69b 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -116,8 +116,8 @@ public void Save(string key, T value) var leastRecentlyUsedItemKey = (from cacheItem in _cache - where cacheItem.Value == leastRecentlyUsedItem.Value - select cacheItem.Key).FirstOrDefault(); + where cacheItem.Value == leastRecentlyUsedItem.Value + select cacheItem.Key).FirstOrDefault(); if (leastRecentlyUsedItemKey != null) { @@ -225,8 +225,8 @@ public string[] _readCurrentCacheKeys() _logger.Log(LogLevel.WARN, "_readCurrentCacheKeys used for non-testing purpose"); return (from listItem in _list - join cacheItem in _cache on listItem equals cacheItem.Value - select cacheItem.Key).ToArray(); + join cacheItem in _cache on listItem equals cacheItem.Value + select cacheItem.Key).ToArray(); } } } From 8b4f00218bfac4323bbd13cce11d4c9c045d6542 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 16:35:09 -0500 Subject: [PATCH 11/21] Lint fixes??? via change to Linq method chain --- OptimizelySDK/Odp/LruCache.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 1505b69b..dcc639ac 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -115,9 +115,10 @@ public void Save(string key, T value) var leastRecentlyUsedItem = _list.Last; var leastRecentlyUsedItemKey = - (from cacheItem in _cache - where cacheItem.Value == leastRecentlyUsedItem.Value - select cacheItem.Key).FirstOrDefault(); + _cache.Where( + cacheItem => cacheItem.Value == leastRecentlyUsedItem.Value). + Select(cacheItem => cacheItem.Key). + FirstOrDefault(); if (leastRecentlyUsedItemKey != null) { @@ -224,9 +225,14 @@ public string[] _readCurrentCacheKeys() { _logger.Log(LogLevel.WARN, "_readCurrentCacheKeys used for non-testing purpose"); - return (from listItem in _list - join cacheItem in _cache on listItem equals cacheItem.Value - select cacheItem.Key).ToArray(); + lock (_mutex) + { + return _list.Join(_cache, + listItem => listItem, + cacheItem => cacheItem.Value, + (listItem, cacheItem) => cacheItem.Key). + ToArray(); + } } } } From 3d27572b6b0fafd3162dadc75604ba746a3eec11 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 16:46:59 -0500 Subject: [PATCH 12/21] Remove re-added IOdpConfig.cs --- OptimizelySDK/Odp/IOdpConfig.cs | 59 ------------------------------ OptimizelySDK/OptimizelySDK.csproj | 1 - 2 files changed, 60 deletions(-) delete mode 100644 OptimizelySDK/Odp/IOdpConfig.cs diff --git a/OptimizelySDK/Odp/IOdpConfig.cs b/OptimizelySDK/Odp/IOdpConfig.cs deleted file mode 100644 index 55780915..00000000 --- a/OptimizelySDK/Odp/IOdpConfig.cs +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2022, Optimizely - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -using System.Collections.Generic; - -namespace OptimizelySDK.Odp -{ - public interface IOdpConfig - { - /// <summary> - /// Public API key for the ODP account from which the audience segments will be fetched (optional). - /// </summary> - string ApiKey { get; } - - /// <summary> - /// Host of ODP audience segments API. - /// </summary> - string ApiHost { get; } - - /// <summary> - /// All ODP segments used in the current datafile (associated with apiHost/apiKey). - /// </summary> - List<string> SegmentsToCheck { get; } - - /// <summary> - /// Update the ODP configuration details - /// </summary> - /// <param name="apiKey">Public API key for the ODP account</param> - /// <param name="apiHost">Host of ODP audience segments API</param> - /// <param name="segmentsToCheck">Audience segments</param> - /// <returns>true if configuration was updated successfully otherwise false</returns> - bool Update(string apiKey, string apiHost, List<string> segmentsToCheck); - - /// <summary> - /// Determines if ODP configuration has the minimum amount of information - /// </summary> - /// <returns>true if ODP configuration can be used otherwise false</returns> - bool IsReady(); - - /// <summary> - /// Determines if ODP configuration contains segments - /// </summary> - /// <returns></returns> - bool HasSegments(); - } -} diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 9780f145..c67269d2 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -101,7 +101,6 @@ <Compile Include="Odp\Entity\OdpEvent.cs" /> <Compile Include="Odp\Entity\Response.cs" /> <Compile Include="Odp\Enums.cs" /> - <Compile Include="Odp\IOdpConfig.cs" /> <Compile Include="Odp\IOdpSegmentManager.cs" /> <Compile Include="Odp\OdpConfig.cs" /> <Compile Include="Odp\OdpSegmentApiManager.cs" /> From 1c5a914cda2ece30a0a2317b2a398c2213124bee Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Fri, 18 Nov 2022 16:51:06 -0500 Subject: [PATCH 13/21] Add internal doc --- OptimizelySDK/Odp/Enums.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index f8f6590e..7aeb2753 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -16,13 +16,20 @@ namespace OptimizelySDK.Odp { + /// <summary> + /// Type of ODP key used for fetching segments & sending events + /// </summary> public enum OdpUserKeyType { // ReSharper disable InconsistentNaming // ODP expects these names in UPPERCASE; .ToString() used - VUID = 0, + VUID = 0, // kept for SDK consistency and awareness FS_USER_ID = 1, } + + /// <summary> + /// Options used during segment cache handling + /// </summary> public enum OdpSegmentOption { IgnoreCache = 0, From 9d61e43b6caaaa0522363c9500ac3e095238d1be Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Tue, 22 Nov 2022 08:54:20 -0500 Subject: [PATCH 14/21] PR code review revisions --- OptimizelySDK/Odp/LruCache.cs | 5 ++++- OptimizelySDK/Odp/OdpSegmentManager.cs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index dcc639ac..563a5019 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -225,14 +225,17 @@ public string[] _readCurrentCacheKeys() { _logger.Log(LogLevel.WARN, "_readCurrentCacheKeys used for non-testing purpose"); + string[] cacheKeys; lock (_mutex) { - return _list.Join(_cache, + cacheKeys = _list.Join(_cache, listItem => listItem, cacheItem => cacheItem.Value, (listItem, cacheItem) => cacheItem.Key). ToArray(); } + + return cacheKeys; } } } diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index a887b860..2314dd26 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -81,7 +81,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, if (!_odpConfig.IsReady()) { _logger.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE); - return new List<string>(); + return null; } if (!_odpConfig.HasSegments()) From 40f2fdfab71421fecf3d21058747f80ae596a16b Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Tue, 22 Nov 2022 09:28:27 -0500 Subject: [PATCH 15/21] Update unit test --- OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index 89ac1d41..15ed9d87 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -141,7 +141,7 @@ public void ShouldHandleFetchSegmentsWithError() } [Test] - public void ShouldLogAndReturnEmptySegmentsListWhenOdpConfigNotReady() + public void ShouldLogAndReturnNullWhenWhenOdpConfigNotReady() { var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, new List<string>(0)); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); @@ -150,7 +150,7 @@ public void ShouldLogAndReturnEmptySegmentsListWhenOdpConfigNotReady() var segments = manager.FetchQualifiedSegments(FS_USER_ID); - Assert.IsTrue(segments.Count == 0); + Assert.IsNull(segments); _mockLogger.Verify( l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), Times.Once); From 3f91f81f550962225ba6b3d57086c109ed41d6ac Mon Sep 17 00:00:00 2001 From: Mike Chu <104384559+mikechu-optimizely@users.noreply.github.com> Date: Tue, 22 Nov 2022 13:11:59 -0500 Subject: [PATCH 16/21] Update OptimizelySDK/Odp/OdpSegmentManager.cs Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com> --- OptimizelySDK/Odp/OdpSegmentManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 2314dd26..dd25b896 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -100,7 +100,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, { _segmentsCache.Reset(); } - else if (!options.Contains(OdpSegmentOption.IgnoreCache)) + if (!options.Contains(OdpSegmentOption.IgnoreCache)) { qualifiedSegments = _segmentsCache.Lookup(cacheKey); if (qualifiedSegments != null) From be49cb1a8517246eab3524dff678c0821d00a014 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Tue, 22 Nov 2022 14:17:17 -0500 Subject: [PATCH 17/21] Pull request code revisions --- .../OdpTests/OdpSegmentManagerTest.cs | 37 +++++++++++++------ OptimizelySDK/Odp/Constants.cs | 4 +- OptimizelySDK/Odp/LruCache.cs | 4 +- OptimizelySDK/Odp/OdpSegmentManager.cs | 18 +++++---- 4 files changed, 41 insertions(+), 22 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index 15ed9d87..c8abe650 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -64,11 +64,11 @@ public void Setup() public void ShouldFetchSegmentsOnCacheMiss() { var keyCollector = new List<string>(); - _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))) - .Returns(default(List<string>)); + _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))). + Returns(default(List<string>)); _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), - It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())) - .Returns(segmentsToCheck.ToArray()); + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())). + Returns(segmentsToCheck.ToArray()); var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); @@ -95,8 +95,7 @@ public void ShouldFetchSegmentsOnCacheMiss() public void ShouldFetchSegmentsSuccessOnCacheHit() { var keyCollector = new List<string>(); - _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))) - .Returns(segmentsToCheck); + _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))).Returns(segmentsToCheck); _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())); var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, @@ -123,8 +122,8 @@ public void ShouldHandleFetchSegmentsWithError() { // OdpSegmentApiManager.FetchSegments() return null on any error _mockApiManager.Setup(a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), - It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())) - .Returns(null as string[]); + It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>())). + Returns(null as string[]); var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); @@ -136,12 +135,28 @@ public void ShouldHandleFetchSegmentsWithError() a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), Times.Once); - _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Once); + _mockCache.Verify(c => c.Save(expectedCacheKey, It.IsAny<List<string>>()), Times.Never); Assert.IsNull(segments); } [Test] - public void ShouldLogAndReturnNullWhenWhenOdpConfigNotReady() + public void ShouldLogAndReturnAnEmptySetWhenNoSegmentsToCheck() + { + var odpConfig = new OdpConfig(API_KEY, API_HOST, new List<string>(0)); + var manager = new OdpSegmentManager(odpConfig, _mockApiManager.Object, + Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + + var segments = manager.FetchQualifiedSegments(FS_USER_ID); + + Assert.IsTrue(segments.Count == 0); + _mockLogger.Verify( + l => l.Log(LogLevel.DEBUG, + "No Segments are used in the project, Not Fetching segments. Returning empty list."), + Times.Once); + } + + [Test] + public void ShouldLogAndReturnNullWhenOdpConfigNotReady() { var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, new List<string>(0)); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); @@ -188,7 +203,7 @@ public void ShouldResetCache() }); _mockCache.Verify(c => c.Reset(), Times.Once); - _mockCache.Verify(c => c.Lookup(It.IsAny<string>()), Times.Never); + _mockCache.Verify(c => c.Lookup(It.IsAny<string>()), Times.Once); _mockApiManager.Verify( a => a.FetchSegments(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<OdpUserKeyType>(), It.IsAny<string>(), It.IsAny<List<string>>()), diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index e44c47a5..9d928699 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -108,8 +108,8 @@ public static class Constants public const int DEFAULT_MAX_CACHE_SIZE = 10000; /// <summary> - /// Default number of minutes to cache + /// Default number of seconds to cache /// </summary> - public const int DEFAULT_CACHE_MINUTES = 10; + public const int DEFAULT_CACHE_SECONDS = 600; } } diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 563a5019..cb26e45e 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -62,7 +62,7 @@ public class LruCache<T> : ICache<T> /// <param name="itemTimeout">Timeout or time to live for each item</param> /// <param name="logger">Implementation used for recording LRU events or errors</param> public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, - TimeSpan? itemTimeout = default, + TimeSpan? itemTimeout = null, ILogger logger = null ) { @@ -72,7 +72,7 @@ public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, _logger = logger ?? new DefaultLogger(); - _timeout = itemTimeout ?? TimeSpan.FromMinutes(Constants.DEFAULT_CACHE_MINUTES); + _timeout = itemTimeout ?? TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); if (_timeout < TimeSpan.Zero) { _logger.Log(LogLevel.WARN, diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index dd25b896..e6bead9d 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -48,7 +48,7 @@ public class OdpSegmentManager : IOdpSegmentManager private readonly ICache<List<string>> _segmentsCache; public OdpSegmentManager(OdpConfig odpConfig, IOdpSegmentApiManager apiManager, - int cacheSize = Constants.DEFAULT_MAX_CACHE_SIZE, TimeSpan? itemTimeout = null, + int? cacheSize = null, TimeSpan? itemTimeout = null, ILogger logger = null, ICache<List<string>> cache = null ) { @@ -56,15 +56,18 @@ public OdpSegmentManager(OdpConfig odpConfig, IOdpSegmentApiManager apiManager, _odpConfig = odpConfig; _logger = logger ?? new DefaultLogger(); - var timeout = itemTimeout ?? TimeSpan.FromMinutes(Constants.DEFAULT_CACHE_MINUTES); - if (timeout < TimeSpan.Zero) + itemTimeout = itemTimeout ?? TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); + if (itemTimeout < TimeSpan.Zero) { _logger.Log(LogLevel.WARN, "Negative item timeout provided. Items will not expire in cache."); - timeout = TimeSpan.Zero; + itemTimeout = TimeSpan.Zero; } - _segmentsCache = cache ?? new LruCache<List<string>>(cacheSize, timeout, logger); + cacheSize = cacheSize ?? Constants.DEFAULT_MAX_CACHE_SIZE; + + _segmentsCache = + cache ?? new LruCache<List<string>>(cacheSize.Value, itemTimeout, logger); } /// <summary> @@ -87,7 +90,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, if (!_odpConfig.HasSegments()) { _logger.Log(LogLevel.DEBUG, - "No Segments are used in the project, Not Fetching segments. Returning empty list"); + "No Segments are used in the project, Not Fetching segments. Returning empty list."); return new List<string>(); } @@ -100,6 +103,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, { _segmentsCache.Reset(); } + if (!options.Contains(OdpSegmentOption.IgnoreCache)) { qualifiedSegments = _segmentsCache.Lookup(cacheKey); @@ -120,7 +124,7 @@ public List<string> FetchQualifiedSegments(string fsUserId, _odpConfig.SegmentsToCheck)?. ToList(); - if (!options.Contains(OdpSegmentOption.IgnoreCache)) + if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IgnoreCache)) { _segmentsCache.Save(cacheKey, qualifiedSegments); } From 892cd3fa7b3f486f4500c56b9f8b5e9c7fd1fbb6 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Wed, 23 Nov 2022 08:51:07 -0500 Subject: [PATCH 18/21] Remove time complexity looping/Linq --- OptimizelySDK/Odp/LruCache.cs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index cb26e45e..9a841c32 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -112,23 +112,15 @@ public void Save(string key, T value) { if (_cache.Count >= _maxSize) { - var leastRecentlyUsedItem = _list.Last; + var leastRecentlyUsedItem = _list.Last.Value; - var leastRecentlyUsedItemKey = - _cache.Where( - cacheItem => cacheItem.Value == leastRecentlyUsedItem.Value). - Select(cacheItem => cacheItem.Key). - FirstOrDefault(); - - if (leastRecentlyUsedItemKey != null) - { - _cache.Remove(leastRecentlyUsedItemKey); - } + var leastRecentlyUsedItemKey = leastRecentlyUsedItem.Key; + _cache.Remove(leastRecentlyUsedItemKey); _list.Remove(leastRecentlyUsedItem); } - var item = new ItemWrapper(value); + var item = new ItemWrapper(key, value); _list.AddFirst(item); _cache.Add(key, item); } @@ -196,6 +188,11 @@ public void Reset() /// </summary> public class ItemWrapper { + /// <summary> + /// Key of the item + /// </summary> + public readonly string Key; + /// <summary> /// Value of the item /// </summary> @@ -209,9 +206,11 @@ public class ItemWrapper /// <summary> /// Initialize the wrapper /// </summary> + /// <param name="key">Key of the item to be stored</param> /// <param name="value">Item to be stored</param> - public ItemWrapper(T value) + public ItemWrapper(string key, T value) { + Key = key; Value = value; CreationTimestamp = DateTime.Now.MillisecondsSince1970(); } From 77d56dd993882ac218dc3b9928b83a4bbab3d0a0 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Wed, 23 Nov 2022 09:34:22 -0500 Subject: [PATCH 19/21] Small refactor --- OptimizelySDK/Odp/LruCache.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 9a841c32..3c598605 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -61,14 +61,14 @@ public class LruCache<T> : ICache<T> /// <param name="maxSize">Maximum number of elements to allow in the cache</param> /// <param name="itemTimeout">Timeout or time to live for each item</param> /// <param name="logger">Implementation used for recording LRU events or errors</param> - public LruCache(int maxSize = Constants.DEFAULT_MAX_CACHE_SIZE, + public LruCache(int? maxSize = null, TimeSpan? itemTimeout = null, ILogger logger = null ) { _mutex = new object(); - _maxSize = Math.Max(0, maxSize); + _maxSize = Math.Max(0, maxSize ?? Constants.DEFAULT_MAX_CACHE_SIZE); _logger = logger ?? new DefaultLogger(); @@ -114,9 +114,7 @@ public void Save(string key, T value) { var leastRecentlyUsedItem = _list.Last.Value; - var leastRecentlyUsedItemKey = leastRecentlyUsedItem.Key; - _cache.Remove(leastRecentlyUsedItemKey); - + _cache.Remove(leastRecentlyUsedItem.Key); _list.Remove(leastRecentlyUsedItem); } From eedf60333f38858f9057d93cf7329d0f31799e66 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Wed, 23 Nov 2022 13:30:45 -0500 Subject: [PATCH 20/21] Use OrderedDictionary --- OptimizelySDK/Odp/LruCache.cs | 58 ++++++++++------------------------- 1 file changed, 16 insertions(+), 42 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 3c598605..40aaae9c 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -48,12 +48,7 @@ public class LruCache<T> : ICache<T> /// <summary> /// Indexed data held in the cache /// </summary> - private readonly Dictionary<string, ItemWrapper> _cache; - - /// <summary> - /// Ordered list of objects being held in the cache - /// </summary> - private readonly LinkedList<ItemWrapper> _list; + private readonly OrderedDictionary _cache; /// <summary> /// A Least Recently Used in-memory cache @@ -80,9 +75,7 @@ public LruCache(int? maxSize = null, _timeout = TimeSpan.Zero; } - _cache = new Dictionary<string, ItemWrapper>(_maxSize); - - _list = new LinkedList<ItemWrapper>(); + _cache = new OrderedDictionary(_maxSize); } /// <summary> @@ -101,26 +94,20 @@ public void Save(string key, T value) lock (_mutex) { - if (_cache.ContainsKey(key)) + if (_cache.Contains(key)) { var item = _cache[key]; - _list.Remove(item); - _list.AddFirst(item); - _cache[key] = item; + _cache.Remove(key); + _cache.Insert(0, key, item); } else { if (_cache.Count >= _maxSize) { - var leastRecentlyUsedItem = _list.Last.Value; - - _cache.Remove(leastRecentlyUsedItem.Key); - _list.Remove(leastRecentlyUsedItem); + _cache.RemoveAt(_cache.Count - 1); } - var item = new ItemWrapper(key, value); - _list.AddFirst(item); - _cache.Add(key, item); + _cache.Insert(0, key, new ItemWrapper(value)); } } } @@ -141,28 +128,27 @@ public T Lookup(string key) lock (_mutex) { - if (!_cache.ContainsKey(key)) + if (!_cache.Contains(key)) { return default; } - ItemWrapper item = _cache[key]; - var currentTimestamp = DateTime.Now.MillisecondsSince1970(); + var item = _cache[key] as ItemWrapper; var itemReturn = default(T); - if (_timeout == TimeSpan.Zero || - (currentTimestamp - item.CreationTimestamp < _timeout.TotalMilliseconds)) + if (item != null && (_timeout == TimeSpan.Zero || + currentTimestamp - item.CreationTimestamp < + _timeout.TotalMilliseconds)) { - _list.Remove(item); - _list.AddFirst(item); + _cache.Remove(key); + _cache.Insert(0, key, item); itemReturn = item.Value; } else { _cache.Remove(key); - _list.Remove(item); } return itemReturn; @@ -177,7 +163,6 @@ public void Reset() lock (_mutex) { _cache.Clear(); - _list.Clear(); } } @@ -186,11 +171,6 @@ public void Reset() /// </summary> public class ItemWrapper { - /// <summary> - /// Key of the item - /// </summary> - public readonly string Key; - /// <summary> /// Value of the item /// </summary> @@ -204,11 +184,9 @@ public class ItemWrapper /// <summary> /// Initialize the wrapper /// </summary> - /// <param name="key">Key of the item to be stored</param> /// <param name="value">Item to be stored</param> - public ItemWrapper(string key, T value) + public ItemWrapper(T value) { - Key = key; Value = value; CreationTimestamp = DateTime.Now.MillisecondsSince1970(); } @@ -225,11 +203,7 @@ public string[] _readCurrentCacheKeys() string[] cacheKeys; lock (_mutex) { - cacheKeys = _list.Join(_cache, - listItem => listItem, - cacheItem => cacheItem.Value, - (listItem, cacheItem) => cacheItem.Key). - ToArray(); + cacheKeys = _cache.Keys.Cast<string>().ToArray(); } return cacheKeys; From ec813543472a6a292c9c2e0dcf425e20116b34c7 Mon Sep 17 00:00:00 2001 From: Mike Chu <michael.chu@optimizely.com> Date: Wed, 23 Nov 2022 13:30:45 -0500 Subject: [PATCH 21/21] Use OrderedDictionary --- OptimizelySDK/Odp/LruCache.cs | 60 ++++++++++------------------------- 1 file changed, 17 insertions(+), 43 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 3c598605..a475c117 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -17,7 +17,7 @@ using OptimizelySDK.Logger; using OptimizelySDK.Utils; using System; -using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; namespace OptimizelySDK.Odp @@ -48,12 +48,7 @@ public class LruCache<T> : ICache<T> /// <summary> /// Indexed data held in the cache /// </summary> - private readonly Dictionary<string, ItemWrapper> _cache; - - /// <summary> - /// Ordered list of objects being held in the cache - /// </summary> - private readonly LinkedList<ItemWrapper> _list; + private readonly OrderedDictionary _cache; /// <summary> /// A Least Recently Used in-memory cache @@ -80,9 +75,7 @@ public LruCache(int? maxSize = null, _timeout = TimeSpan.Zero; } - _cache = new Dictionary<string, ItemWrapper>(_maxSize); - - _list = new LinkedList<ItemWrapper>(); + _cache = new OrderedDictionary(_maxSize); } /// <summary> @@ -101,26 +94,20 @@ public void Save(string key, T value) lock (_mutex) { - if (_cache.ContainsKey(key)) + if (_cache.Contains(key)) { var item = _cache[key]; - _list.Remove(item); - _list.AddFirst(item); - _cache[key] = item; + _cache.Remove(key); + _cache.Insert(0, key, item); } else { if (_cache.Count >= _maxSize) { - var leastRecentlyUsedItem = _list.Last.Value; - - _cache.Remove(leastRecentlyUsedItem.Key); - _list.Remove(leastRecentlyUsedItem); + _cache.RemoveAt(_cache.Count - 1); } - var item = new ItemWrapper(key, value); - _list.AddFirst(item); - _cache.Add(key, item); + _cache.Insert(0, key, new ItemWrapper(value)); } } } @@ -141,28 +128,27 @@ public T Lookup(string key) lock (_mutex) { - if (!_cache.ContainsKey(key)) + if (!_cache.Contains(key)) { return default; } - ItemWrapper item = _cache[key]; - var currentTimestamp = DateTime.Now.MillisecondsSince1970(); + var item = _cache[key] as ItemWrapper; var itemReturn = default(T); - if (_timeout == TimeSpan.Zero || - (currentTimestamp - item.CreationTimestamp < _timeout.TotalMilliseconds)) + if (item != null && (_timeout == TimeSpan.Zero || + currentTimestamp - item.CreationTimestamp < + _timeout.TotalMilliseconds)) { - _list.Remove(item); - _list.AddFirst(item); + _cache.Remove(key); + _cache.Insert(0, key, item); itemReturn = item.Value; } else { _cache.Remove(key); - _list.Remove(item); } return itemReturn; @@ -177,7 +163,6 @@ public void Reset() lock (_mutex) { _cache.Clear(); - _list.Clear(); } } @@ -186,11 +171,6 @@ public void Reset() /// </summary> public class ItemWrapper { - /// <summary> - /// Key of the item - /// </summary> - public readonly string Key; - /// <summary> /// Value of the item /// </summary> @@ -204,11 +184,9 @@ public class ItemWrapper /// <summary> /// Initialize the wrapper /// </summary> - /// <param name="key">Key of the item to be stored</param> /// <param name="value">Item to be stored</param> - public ItemWrapper(string key, T value) + public ItemWrapper(T value) { - Key = key; Value = value; CreationTimestamp = DateTime.Now.MillisecondsSince1970(); } @@ -225,11 +203,7 @@ public string[] _readCurrentCacheKeys() string[] cacheKeys; lock (_mutex) { - cacheKeys = _list.Join(_cache, - listItem => listItem, - cacheItem => cacheItem.Value, - (listItem, cacheItem) => cacheItem.Key). - ToArray(); + cacheKeys = _cache.Keys.Cast<string>().ToArray(); } return cacheKeys;