From 467cbccaf7afa4f0b22a0664c564356e2d5c8916 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 3 Oct 2022 17:24:31 -0400 Subject: [PATCH 01/14] WIP REST API Manager --- .../OdpTests/GraphQLManagerTest.cs | 4 +- OptimizelySDK.sln.DotSettings | 3 +- OptimizelySDK/Odp/Client/OdpClient.cs | 136 ---------- OptimizelySDK/Odp/Entity/OdpEvent.cs | 38 +++ .../Odp/Entity/QuerySegmentsParameters.cs | 234 ------------------ OptimizelySDK/Odp/Enums.cs | 9 + OptimizelySDK/Odp/GraphQLManager.cs | 191 ++++++++++++-- OptimizelySDK/Odp/IGraphQLManager.cs | 2 +- .../IOdpClient.cs => IRestApiManager.cs} | 16 +- OptimizelySDK/Odp/RestApiManager.cs | 134 ++++++++++ OptimizelySDK/OptimizelySDK.csproj | 7 +- 11 files changed, 359 insertions(+), 415 deletions(-) delete mode 100644 OptimizelySDK/Odp/Client/OdpClient.cs create mode 100644 OptimizelySDK/Odp/Entity/OdpEvent.cs delete mode 100644 OptimizelySDK/Odp/Entity/QuerySegmentsParameters.cs create mode 100644 OptimizelySDK/Odp/Enums.cs rename OptimizelySDK/Odp/{Client/IOdpClient.cs => IRestApiManager.cs} (55%) create mode 100644 OptimizelySDK/Odp/RestApiManager.cs diff --git a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs index f1024c51..d07ead36 100644 --- a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs @@ -287,7 +287,7 @@ public void ShouldHandleUnrecognizedJsonResponse() [Test] public void ShouldHandle400HttpCode() { - var odpClient = new OdpClient(_mockErrorHandler.Object, _mockLogger.Object, + var odpClient = new OdpClient(_mockLogger.Object, GetHttpClientThatReturnsStatus(HttpStatusCode.BadRequest)); var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, odpClient); @@ -307,7 +307,7 @@ public void ShouldHandle400HttpCode() [Test] public void ShouldHandle500HttpCode() { - var odpClient = new OdpClient(_mockErrorHandler.Object, _mockLogger.Object, + var odpClient = new OdpClient(_mockLogger.Object, GetHttpClientThatReturnsStatus(HttpStatusCode.InternalServerError)); var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, odpClient); diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index 7cb4f57e..d900f70e 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -45,4 +45,5 @@ True True True - True \ No newline at end of file + True + True \ No newline at end of file diff --git a/OptimizelySDK/Odp/Client/OdpClient.cs b/OptimizelySDK/Odp/Client/OdpClient.cs deleted file mode 100644 index f8b4a5a7..00000000 --- a/OptimizelySDK/Odp/Client/OdpClient.cs +++ /dev/null @@ -1,136 +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 OptimizelySDK.ErrorHandler; -using OptimizelySDK.Logger; -using OptimizelySDK.Odp.Entity; -using System; -using System.Net.Http; -using System.Text; -using System.Threading.Tasks; - -namespace OptimizelySDK.Odp.Client -{ - /// - /// Http implementation for sending requests and handling responses to Optimizely Data Platform - /// - public class OdpClient : IOdpClient - { - /// - /// Error handler used to record errors - /// - private readonly IErrorHandler _errorHandler; - - /// - /// Logger used to record messages that occur within the ODP client - /// - private readonly ILogger _logger; - - /// - /// Http client used for handling requests and responses over HTTP - /// - private readonly HttpClient _client; - - /// - /// An implementation for sending requests and handling responses to Optimizely Data Platform (ODP) - /// - /// Handler to record exceptions - /// Collect and record events/errors for this ODP client - /// Client implementation to send/receive requests over HTTP - public OdpClient(IErrorHandler errorHandler = null, ILogger logger = null, - HttpClient client = null - ) - { - _errorHandler = errorHandler ?? new NoOpErrorHandler(); - _logger = logger ?? new NoOpLogger(); - _client = client ?? new HttpClient(); - } - - /// - /// Synchronous handler for querying the ODP GraphQL endpoint - /// - /// Parameters inputs to send to ODP - /// JSON response from ODP - public string QuerySegments(QuerySegmentsParameters parameters) - { - HttpResponseMessage response; - try - { - response = QuerySegmentsAsync(parameters).GetAwaiter().GetResult(); - } - catch (Exception ex) - { - _errorHandler.HandleError(ex); - _logger.Log(LogLevel.ERROR, "Audience segments fetch failed (network error)"); - return default; - } - - var responseStatusCode = (int)response.StatusCode; - if (responseStatusCode >= 400 && responseStatusCode < 600) - { - _logger.Log(LogLevel.ERROR, - $"Audience segments fetch failed ({responseStatusCode})"); - return default; - } - - return response.Content.ReadAsStringAsync().Result; - } - - /// - /// Asynchronous handler for querying the ODP GraphQL endpoint - /// - /// Parameters inputs to send to ODP - /// JSON response from ODP - private async Task QuerySegmentsAsync( - QuerySegmentsParameters parameters - ) - { - var request = BuildRequestMessage(parameters.ToGraphQLJson(), parameters); - - var response = await _client.SendAsync(request); - - return response; - } - - /// - /// Produces the request GraphQL query payload - /// - /// JSON GraphQL query - /// Configuration used to connect to ODP - /// Formed HTTP request message ready to be transmitted - private static HttpRequestMessage BuildRequestMessage(string jsonQuery, - QuerySegmentsParameters parameters - ) - { - const string API_HEADER_KEY = "x-api-key"; - const string CONTENT_TYPE = "application/json"; - var request = new HttpRequestMessage - { - RequestUri = new Uri(parameters.ApiHost), - Method = HttpMethod.Post, - Headers = - { - { - API_HEADER_KEY, parameters.ApiKey - }, - }, - Content = new StringContent(jsonQuery, Encoding.UTF8, CONTENT_TYPE), - }; - - return request; - } - } -} diff --git a/OptimizelySDK/Odp/Entity/OdpEvent.cs b/OptimizelySDK/Odp/Entity/OdpEvent.cs new file mode 100644 index 00000000..0370cb85 --- /dev/null +++ b/OptimizelySDK/Odp/Entity/OdpEvent.cs @@ -0,0 +1,38 @@ +/* + * 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.Entity +{ + public class OdpEvent + { + public string Type; + + public string Action; + + public Dictionary Identifiers; + + public Dictionary Data; + + public OdpEvent(string type, string action, Dictionary identifiers, Dictionary data) { + Type = type; + Action = action; + Identifiers = identifiers ?? new Dictionary(); + Data = data ?? new Dictionary(); + } + } +} diff --git a/OptimizelySDK/Odp/Entity/QuerySegmentsParameters.cs b/OptimizelySDK/Odp/Entity/QuerySegmentsParameters.cs deleted file mode 100644 index 32e89c1c..00000000 --- a/OptimizelySDK/Odp/Entity/QuerySegmentsParameters.cs +++ /dev/null @@ -1,234 +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 Newtonsoft.Json; -using OptimizelySDK.Logger; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; - -namespace OptimizelySDK.Odp.Entity -{ - /// - /// Handles parameters used in querying ODP segments - /// - public class QuerySegmentsParameters - { - public class Builder - { - /// - /// Builder API key - /// - private string ApiKey { get; set; } - - /// - /// Builder ODP endpoint - /// - private string ApiHost { get; set; } - - /// - /// Builder user type key - /// - private string UserKey { get; set; } - - /// - /// Builder user key value - /// - private string UserValue { get; set; } - - /// - /// Builder audience segments - /// - private List SegmentsToCheck { get; set; } - - /// - /// Builder logger to report problems during build - /// - private ILogger Logger { get; } - - public Builder(ILogger logger) - { - Logger = logger; - } - - /// - /// Sets the API key for accessing ODP - /// - /// Optimizely Data Platform API key - /// Current state of builder - public Builder WithApiKey(string apiKey) - { - ApiKey = apiKey; - return this; - } - - /// - /// Set the API endpoint for ODP - /// - /// Fully-qualified URL to ODP endpoint - /// Current state of builder - public Builder WithApiHost(string apiHost) - { - ApiHost = apiHost; - return this; - } - - /// - /// Sets the user key on which to query ODP - /// - /// 'vuid' or 'fs_user_id' - /// Current state of builder - public Builder WithUserKey(string userKey) - { - UserKey = userKey; - return this; - } - - /// - /// Set the user key's value - /// - /// Value for user key - /// Current state of builder - public Builder WithUserValue(string userValue) - { - UserValue = userValue; - return this; - } - - /// - /// Sets the segments to check - /// - /// List of audience segments to check - /// Current state of builder - public Builder WithSegmentsToCheck(List segmentsToCheck) - { - SegmentsToCheck = segmentsToCheck; - return this; - } - - /// - /// Validates and constructs the QuerySegmentsParameters object based on provided spec - /// - /// QuerySegmentsParameters object - public QuerySegmentsParameters Build() - { - const string INVALID_MISSING_BUILDER_INPUT_MESSAGE = - "QuerySegmentsParameters Builder was provided an invalid"; - - if (string.IsNullOrWhiteSpace(ApiKey)) - { - Logger.Log(LogLevel.ERROR, - $"{INVALID_MISSING_BUILDER_INPUT_MESSAGE} API Key"); - return default; - } - - if (string.IsNullOrWhiteSpace(ApiHost) || - !Uri.TryCreate(ApiHost, UriKind.Absolute, out Uri _)) - { - Logger.Log(LogLevel.ERROR, - $"{INVALID_MISSING_BUILDER_INPUT_MESSAGE} API Host"); - return default; - } - - if (string.IsNullOrWhiteSpace(UserKey) || !Enum.TryParse(UserKey, out UserKeyType _)) - { - Logger.Log(LogLevel.ERROR, - $"{INVALID_MISSING_BUILDER_INPUT_MESSAGE} User Key"); - return default; - } - - if (string.IsNullOrWhiteSpace(UserValue)) - { - Logger.Log(LogLevel.ERROR, - $"{INVALID_MISSING_BUILDER_INPUT_MESSAGE} User Value"); - return default; - } - - if (SegmentsToCheck.Any(string.IsNullOrWhiteSpace)) - { - Logger.Log(LogLevel.ERROR, - $"Segments To Check contained a null or empty segment"); - return default; - } - - return new QuerySegmentsParameters - { - ApiKey = ApiKey, - ApiHost = ApiHost, - UserKey = UserKey, - UserValue = UserValue, - SegmentsToCheck = SegmentsToCheck, - }; - } - - /// - /// Enumeration used during validation of User Key string - /// - private enum UserKeyType - { - vuid = 0, fs_user_id = 1 - } - } - - /// - /// Optimizely Data Platform API key - /// - public string ApiKey { get; private set; } - - /// - /// Fully-qualified URL to ODP endpoint - /// - public string ApiHost { get; private set; } - - /// - /// 'vuid' or 'fs_user_id' (client device id or fullstack id) - /// - private string UserKey { get; set; } - - /// - /// Value for the user key - /// - private string UserValue { get; set; } - - /// - /// Audience segments to check for inclusion in the experiment - /// - private List SegmentsToCheck { get; set; } - - private QuerySegmentsParameters() { } - - /// - /// Converts the current QuerySegmentsParameters into a GraphQL JSON string - /// - /// GraphQL JSON payload - public string ToGraphQLJson() - { - var userValueWithEscapedQuotes = $"\\\"{UserValue}\\\""; - var segmentsArrayJson = - JsonConvert.SerializeObject(SegmentsToCheck).Replace("\"", "\\\""); - - var json = new StringBuilder(); - json.Append("{\"query\" : \"query {customer"); - json.Append($"({UserKey} : {userValueWithEscapedQuotes}) "); - json.Append("{audiences"); - json.Append($"(subset: {segmentsArrayJson})"); - json.Append("{edges {node {name state}}}}}\"}"); - - return json.ToString(); - } - } -} diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs new file mode 100644 index 00000000..ae1251f1 --- /dev/null +++ b/OptimizelySDK/Odp/Enums.cs @@ -0,0 +1,9 @@ +namespace OptimizelySDK.Odp +{ + public enum OdpUserKeyType + { + // ODP expects these names; .ToString() used + VUID = 0, + FS_USER_KEY = 1, + } +} diff --git a/OptimizelySDK/Odp/GraphQLManager.cs b/OptimizelySDK/Odp/GraphQLManager.cs index a246c054..49c41d4c 100644 --- a/OptimizelySDK/Odp/GraphQLManager.cs +++ b/OptimizelySDK/Odp/GraphQLManager.cs @@ -18,10 +18,14 @@ using OptimizelySDK.AudienceConditions; using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; -using OptimizelySDK.Odp.Client; using OptimizelySDK.Odp.Entity; +using System; +using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Net.Http; +using System.Text; +using System.Threading.Tasks; namespace OptimizelySDK.Odp { @@ -30,62 +34,99 @@ namespace OptimizelySDK.Odp /// public class GraphQLManager : IGraphQLManager { + /// + /// Standard message for audience querying fetch errors + /// + private const string AUDIENCE_FETCH_FAILURE_MESSAGE = "Audience segments fetch failed"; + + /// + /// Error handler used to record errors + /// + private readonly IErrorHandler _errorHandler; + + /// + /// Logger used to record messages that occur within the ODP client + /// private readonly ILogger _logger; - private readonly IOdpClient _odpClient; + + /// + /// Http client used for handling requests and responses over HTTP + /// + private readonly HttpClient _httpClient; /// /// Retrieves the audience segments from the Optimizely Data Platform (ODP) /// - /// Handler to record exceptions /// Collect and record events/errors for this GraphQL implementation - /// Client to use to send queries to ODP - public GraphQLManager(IErrorHandler errorHandler = null, ILogger logger = null, IOdpClient client = null) + /// Handler to record exceptions + /// HttpClient to use to send queries to ODP + public GraphQLManager(ILogger logger = null, IErrorHandler errorHandler = null, + HttpClient httpClient = null + ) { _logger = logger ?? new NoOpLogger(); - _odpClient = client ?? new OdpClient(errorHandler ?? new NoOpErrorHandler(), _logger); + _errorHandler = errorHandler ?? new NoOpErrorHandler(); + _httpClient = httpClient ?? new HttpClient(); } /// /// Retrieves the audience segments from ODP /// /// ODP public key - /// Fully-qualified URL of ODP + /// Host of ODP endpoint /// 'vuid' or 'fs_user_id key' /// Associated value to query for the user key /// Audience segments to check for experiment inclusion /// Array of audience segments - public string[] FetchSegments(string apiKey, string apiHost, string userKey, + public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType userKey, string userValue, List segmentsToCheck ) { - var emptySegments = new string[0]; + if (string.IsNullOrWhiteSpace(apiKey) || string.IsNullOrWhiteSpace(apiHost)) + { + _logger.Log(LogLevel.ERROR, + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (Parameters apiKey or apiHost invalid)"); + return null; + } + + if (segmentsToCheck.Count == 0) + { + return new string[0]; + } - var parameters = new QuerySegmentsParameters.Builder(_logger).WithApiKey(apiKey). - WithApiHost(apiHost).WithUserKey(userKey).WithUserValue(userValue). - WithSegmentsToCheck(segmentsToCheck).Build(); + var endpoint = $"{apiHost}/v3/graphql"; + var query = ToGraphQLJson(userKey.ToString(), userValue, segmentsToCheck); - var segmentsResponseJson = _odpClient.QuerySegments(parameters); + var segmentsResponseJson = QuerySegments(apiKey, endpoint, query); if (CanBeJsonParsed(segmentsResponseJson)) { - _logger.Log(LogLevel.WARN, $"Audience segments fetch failed"); - return emptySegments; + _logger.Log(LogLevel.ERROR, + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (network error)"); + return null; } var parsedSegments = ParseSegmentsResponseJson(segmentsResponseJson); + if (parsedSegments is null) + { + _logger.Log(LogLevel.ERROR, + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"); + return null; + } + if (parsedSegments.HasErrors) { var errors = string.Join(";", parsedSegments.Errors.Select(e => e.ToString())); - _logger.Log(LogLevel.WARN, $"Audience segments fetch failed ({errors})"); + _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({errors})"); - return emptySegments; + return null; } - if (parsedSegments?.Data?.Customer?.Audiences?.Edges is null) + if (parsedSegments.Data?.Customer?.Audiences?.Edges is null) { - _logger.Log(LogLevel.ERROR, "Audience segments fetch failed (decode error)"); + _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"); - return emptySegments; + return null; } return parsedSegments.Data.Customer.Audiences.Edges. @@ -94,15 +135,101 @@ public string[] FetchSegments(string apiKey, string apiHost, string userKey, } /// - /// Parses JSON response + /// Converts the current QuerySegmentsParameters into a GraphQL query string /// - /// JSON response from ODP - /// Strongly-typed ODP Response object - public static Response ParseSegmentsResponseJson(string jsonResponse) + /// GraphQL payload + private static string ToGraphQLJson(string userKey, string userValue, + IEnumerable segmentsToCheck + ) + { + var userValueWithEscapedQuotes = $"\\\"{userValue}\\\""; + var segmentsArrayJson = + JsonConvert.SerializeObject(segmentsToCheck).Replace("\"", "\\\""); + + var json = new StringBuilder(); + json.Append("{\"query\" : \"query {customer"); + json.Append($"({userKey} : {userValueWithEscapedQuotes}) "); + json.Append("{audiences"); + json.Append($"(subset: {segmentsArrayJson})"); + json.Append("{edges {node {name state}}}}}\"}"); + + return json.ToString(); + } + + /// + /// Synchronous handler for querying the ODP GraphQL endpoint + /// + /// JSON response from ODP + private string QuerySegments(string apiKey, string endpoint, + string query + ) + { + HttpResponseMessage response; + try + { + response = QuerySegmentsAsync(apiKey, endpoint, query).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + _errorHandler.HandleError(ex); + _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (network error)"); + return default; + } + + var responseStatusCode = int.Parse(response.StatusCode.ToString()); + if (responseStatusCode >= 400 && responseStatusCode < 600) + { + _logger.Log(LogLevel.ERROR, + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({responseStatusCode})"); + return default; + } + + return response.Content.ReadAsStringAsync().Result; + } + + /// + /// Asynchronous handler for querying the ODP GraphQL endpoint + /// + /// ODP API Key + /// Fully-qualified ODP GraphQL endpoint URL + /// GraphQL query + /// JSON response from ODP + private async Task QuerySegmentsAsync(string apiKey, string endpoint, + string query + ) + { + var request = BuildRequestMessage(apiKey, endpoint, query); + + var response = await _httpClient.SendAsync(request); + + return response; + } + + /// + /// Produces the request GraphQL query payload + /// + /// ODP API Key + /// Fully-qualified ODP GraphQL endpoint URL + /// GraphQL query + /// Formed HTTP request message ready to be transmitted + private static HttpRequestMessage BuildRequestMessage(string apiKey, string endpoint, + string query + ) { - return CanBeJsonParsed(jsonResponse) ? - default : - JsonConvert.DeserializeObject(jsonResponse); + var request = new HttpRequestMessage + { + RequestUri = new Uri(endpoint), + Method = HttpMethod.Post, + Headers = + { + { + "x-api-key", apiKey + }, + }, + Content = new StringContent(query, Encoding.UTF8, "application/json"), + }; + + return request; } /// @@ -114,5 +241,15 @@ private static bool CanBeJsonParsed(string jsonToValidate) { return string.IsNullOrWhiteSpace(jsonToValidate); } + + /// + /// Parses JSON response + /// + /// JSON response from ODP + /// Strongly-typed ODP Response object + public static Response ParseSegmentsResponseJson(string jsonResponse) + { + return JsonConvert.DeserializeObject(jsonResponse); + } } } diff --git a/OptimizelySDK/Odp/IGraphQLManager.cs b/OptimizelySDK/Odp/IGraphQLManager.cs index e29c6a40..99f3ba02 100644 --- a/OptimizelySDK/Odp/IGraphQLManager.cs +++ b/OptimizelySDK/Odp/IGraphQLManager.cs @@ -31,7 +31,7 @@ public interface IGraphQLManager /// Array of audience segments string[] FetchSegments(string apiKey, string apiHost, - string userKey, + OdpUserKeyType userKey, string userValue, List segmentsToCheck ); diff --git a/OptimizelySDK/Odp/Client/IOdpClient.cs b/OptimizelySDK/Odp/IRestApiManager.cs similarity index 55% rename from OptimizelySDK/Odp/Client/IOdpClient.cs rename to OptimizelySDK/Odp/IRestApiManager.cs index 17c99d38..e3a32788 100644 --- a/OptimizelySDK/Odp/Client/IOdpClient.cs +++ b/OptimizelySDK/Odp/IRestApiManager.cs @@ -15,19 +15,13 @@ */ using OptimizelySDK.Odp.Entity; +using System.Collections.Generic; +using System.Threading.Tasks; -namespace OptimizelySDK.Odp.Client +namespace OptimizelySDK.Odp { - /// - /// An implementation for sending requests and handling responses to Optimizely Data Platform - /// - public interface IOdpClient + public interface IRestApiManager { - /// - /// Synchronous handler for querying the ODP GraphQL endpoint - /// - /// Parameters inputs to send to ODP - /// JSON response from ODP - string QuerySegments(QuerySegmentsParameters parameters); + bool SendEvents(string apiKey, string apiHost, List events); } } diff --git a/OptimizelySDK/Odp/RestApiManager.cs b/OptimizelySDK/Odp/RestApiManager.cs new file mode 100644 index 00000000..20caad52 --- /dev/null +++ b/OptimizelySDK/Odp/RestApiManager.cs @@ -0,0 +1,134 @@ +/* + * 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 Newtonsoft.Json; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Logger; +using OptimizelySDK.Odp.Entity; +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Text; +using System.Threading.Tasks; + +namespace OptimizelySDK.Odp +{ + public class RestApiManager : IRestApiManager + { + private const string EVENT_SENDING_FAILURE_MESSAGE = "ODP event send failed"; + + /// + /// Error handler used to record errors + /// + private readonly IErrorHandler _errorHandler; + + /// + /// Logger used to record messages that occur within the ODP client + /// + private readonly ILogger _logger; + + /// + /// Http client used for handling requests and responses over HTTP + /// + private readonly HttpClient _httpClient; + + public RestApiManager(ILogger logger = null, IErrorHandler errorHandler = null, + HttpClient httpClient = null + ) + { + _logger = logger ?? new NoOpLogger(); + _errorHandler = errorHandler ?? new NoOpErrorHandler(); + _httpClient = httpClient ?? new HttpClient(); + } + + public bool SendEvents(string apiKey, string apiHost, List events) + { + if (string.IsNullOrWhiteSpace(apiKey) || string.IsNullOrWhiteSpace(apiHost)) + { + _logger.Log(LogLevel.ERROR, + $"{EVENT_SENDING_FAILURE_MESSAGE} (Parameters apiKey or apiHost invalid)"); + return false; + } + + if (events.Count == 0) + { + _logger.Log(LogLevel.ERROR, $"{EVENT_SENDING_FAILURE_MESSAGE} (no events)"); + return false; + } + + var endpoint = $"{apiHost}/v3/events"; + var data = JsonConvert.SerializeObject(events); + var shouldRetry = false; + + HttpResponseMessage response = default; + try + { + response = SendEventsAsync(apiKey, endpoint, data).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + _errorHandler.HandleError(ex); + _logger.Log(LogLevel.ERROR, $"{EVENT_SENDING_FAILURE_MESSAGE} (network error)"); + shouldRetry = true; + } + + var responseStatusCode = int.Parse(response?.StatusCode.ToString() ?? "0"); + if (responseStatusCode >= 400) + { + _logger.Log(LogLevel.ERROR, + $"{EVENT_SENDING_FAILURE_MESSAGE} (${responseStatusCode})"); + } + + if (responseStatusCode >= 500) + { + shouldRetry = true; + } + + return shouldRetry; + } + + private async Task SendEventsAsync(string apiKey, string endpoint, + string data + ) + { + var request = BuildRequestMessage(apiKey, endpoint, data); + + var response = await _httpClient.SendAsync(request); + + return response; + } + + private static HttpRequestMessage BuildRequestMessage(string apiKey, string endpoint, + string data + ) + { + var request = new HttpRequestMessage + { + RequestUri = new Uri(endpoint), + Method = HttpMethod.Post, + Headers = + { + { + "x-api-key", apiKey + }, + }, + Content = new StringContent(data, Encoding.UTF8, "application/json"), + }; + + return request; + } + } +} diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 102ae901..3924df64 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -88,8 +88,6 @@ - - @@ -98,12 +96,15 @@ - + + + + From 15b939806da09060b5c951ed7b023a5afb8f14b8 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 4 Oct 2022 14:14:43 -0400 Subject: [PATCH 02/14] Add docs --- OptimizelySDK/Odp/GraphQLManager.cs | 4 ++-- OptimizelySDK/Odp/RestApiManager.cs | 30 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Odp/GraphQLManager.cs b/OptimizelySDK/Odp/GraphQLManager.cs index 49c41d4c..af1b1b07 100644 --- a/OptimizelySDK/Odp/GraphQLManager.cs +++ b/OptimizelySDK/Odp/GraphQLManager.cs @@ -55,9 +55,9 @@ public class GraphQLManager : IGraphQLManager private readonly HttpClient _httpClient; /// - /// Retrieves the audience segments from the Optimizely Data Platform (ODP) + /// Manager for communicating with the Optimizely Data Platform (ODP) GraphQL endpoint /// - /// Collect and record events/errors for this GraphQL implementation + /// Collect and record events to log /// Handler to record exceptions /// HttpClient to use to send queries to ODP public GraphQLManager(ILogger logger = null, IErrorHandler errorHandler = null, diff --git a/OptimizelySDK/Odp/RestApiManager.cs b/OptimizelySDK/Odp/RestApiManager.cs index 20caad52..2ed0a0f0 100644 --- a/OptimizelySDK/Odp/RestApiManager.cs +++ b/OptimizelySDK/Odp/RestApiManager.cs @@ -26,6 +26,9 @@ namespace OptimizelySDK.Odp { + /// + /// Manager for communicating with ODP's REST API endpoint + /// public class RestApiManager : IRestApiManager { private const string EVENT_SENDING_FAILURE_MESSAGE = "ODP event send failed"; @@ -45,6 +48,12 @@ public class RestApiManager : IRestApiManager /// private readonly HttpClient _httpClient; + /// + /// Manager for communicating with ODP's REST API endpoint + /// + /// Collect and record events to log + /// Handler to record exceptions + /// HttpClient to use to send ODP events public RestApiManager(ILogger logger = null, IErrorHandler errorHandler = null, HttpClient httpClient = null ) @@ -54,6 +63,13 @@ public RestApiManager(ILogger logger = null, IErrorHandler errorHandler = null, _httpClient = httpClient ?? new HttpClient(); } + /// + /// Send events to ODP's RESTful API + /// + /// ODP public key + /// Host of ODP endpoint + /// ODP events to send + /// Retry is true - if network or server error (5xx), otherwise false public bool SendEvents(string apiKey, string apiHost, List events) { if (string.IsNullOrWhiteSpace(apiKey) || string.IsNullOrWhiteSpace(apiHost)) @@ -100,6 +116,13 @@ public bool SendEvents(string apiKey, string apiHost, List events) return shouldRetry; } + /// + /// Async call to send the event data to ODP + /// + /// ODP public key + /// Fully-qualified ODP REST API endpoint + /// JSON string version of ODP event data + /// HTTP response endpoint private async Task SendEventsAsync(string apiKey, string endpoint, string data ) @@ -111,6 +134,13 @@ string data return response; } + /// + /// Build an HTTP request message + /// + /// ODP public key + /// Fully-qualified ODP REST API endpoint + /// JSON string version of ODP event data + /// Ready request message to send private static HttpRequestMessage BuildRequestMessage(string apiKey, string endpoint, string data ) From 54a61de70c773b79750c727d8c535273ea4c433c Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 4 Oct 2022 16:07:52 -0400 Subject: [PATCH 03/14] Fix GraphQL tests --- .../OdpTests/GraphQLManagerTest.cs | 128 +++++++----------- OptimizelySDK/Odp/GraphQLManager.cs | 3 +- 2 files changed, 52 insertions(+), 79 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs index d07ead36..4798d627 100644 --- a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs @@ -21,8 +21,6 @@ using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; using OptimizelySDK.Odp; -using OptimizelySDK.Odp.Client; -using OptimizelySDK.Odp.Entity; using System.Collections.Generic; using System.Net; using System.Net.Http; @@ -36,7 +34,6 @@ public class GraphQLManagerTest { private const string VALID_ODP_PUBLIC_KEY = "not-real-odp-public-key"; private const string ODP_GRAPHQL_URL = "https://example.com/endpoint"; - private const string FS_USER_ID = "fs_user_id"; private readonly List _segmentsToCheck = new List { @@ -47,7 +44,6 @@ public class GraphQLManagerTest private Mock _mockErrorHandler; private Mock _mockLogger; - private Mock _mockOdpClient; [SetUp] public void Setup() @@ -55,8 +51,6 @@ public void Setup() _mockErrorHandler = new Mock(); _mockLogger = new Mock(); _mockLogger.Setup(i => i.Log(It.IsAny(), It.IsAny())); - - _mockOdpClient = new Mock(); } [Test] @@ -144,16 +138,14 @@ public void ShouldFetchValidQualifiedSegments() "{\"edges\":[{\"node\":{\"name\":\"has_email\"," + "\"state\":\"qualified\"}},{\"node\":{\"name\":" + "\"has_email_opted_in\",\"state\":\"qualified\"}}]}}}}"; - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); + var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var manager = + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - FS_USER_ID, + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -168,16 +160,14 @@ public void ShouldHandleEmptyQualifiedSegments() { const string RESPONSE_DATA = "{\"data\":{\"customer\":{\"audiences\":" + "{\"edges\":[ ]}}}}"; - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); + var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var manager = + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - FS_USER_ID, + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -194,67 +184,38 @@ public void ShouldHandleErrorWithInvalidIdentifier() "\"locations\":[{\"line\":1,\"column\":8}],\"path\":[\"customer\"]," + "\"extensions\":{\"classification\":\"DataFetchingException\"}}]," + "\"data\":{\"customer\":null}}"; - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); + var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var manager = + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - FS_USER_ID, + OdpUserKeyType.FS_USER_KEY, "invalid-user", _segmentsToCheck); - Assert.IsTrue(segments.Length == 0); - _mockLogger.Verify(l => l.Log(LogLevel.WARN, It.IsAny()), + Assert.IsNull(segments); + _mockLogger.Verify(l => l.Log(LogLevel.ERROR, It.IsAny()), Times.Once); } - [Test] - public void ShouldHandleOtherException() - { - const string RESPONSE_DATA = "{\"errors\":[{\"message\":\"Validation error of type " + - "UnknownArgument: Unknown field argument not_real_userKey @ " + - "'customer'\",\"locations\":[{\"line\":1,\"column\":17}]," + - "\"extensions\":{\"classification\":\"ValidationError\"}}]}"; - - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); - - var segments = manager.FetchSegments( - VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, - "not_real_userKey", - "tester-101", - _segmentsToCheck); - - Assert.IsTrue(segments.Length == 0); - _mockLogger.Verify(l => l.Log(LogLevel.WARN, It.IsAny()), Times.Once); - } - [Test] public void ShouldHandleBadResponse() { const string RESPONSE_DATA = "{\"data\":{ }}"; - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); + var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var manager = + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - "not_real_userKey", + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); - Assert.IsTrue(segments.Length == 0); + Assert.IsNull(segments); _mockLogger.Verify( l => l.Log(LogLevel.ERROR, "Audience segments fetch failed (decode error)"), Times.Once); @@ -265,20 +226,18 @@ public void ShouldHandleUnrecognizedJsonResponse() { const string RESPONSE_DATA = "{\"unExpectedObject\":{ \"withSome\": \"value\", \"thatIsNotParseable\": \"true\" }}"; - _mockOdpClient.Setup( - c => c.QuerySegments(It.IsAny())). - Returns(RESPONSE_DATA); - var manager = new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, - _mockOdpClient.Object); + var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var manager = + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - "not_real_userKey", + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); - Assert.IsTrue(segments.Length == 0); + Assert.IsNull(segments); _mockLogger.Verify( l => l.Log(LogLevel.ERROR, "Audience segments fetch failed (decode error)"), Times.Once); @@ -287,19 +246,18 @@ public void ShouldHandleUnrecognizedJsonResponse() [Test] public void ShouldHandle400HttpCode() { - var odpClient = new OdpClient(_mockLogger.Object, - GetHttpClientThatReturnsStatus(HttpStatusCode.BadRequest)); + var httpClient = MakeHttpClient(HttpStatusCode.BadRequest); var manager = - new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, odpClient); + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - FS_USER_ID, + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); - Assert.IsTrue(segments.Length == 0); + Assert.IsNull(segments); _mockLogger.Verify(l => l.Log(LogLevel.ERROR, "Audience segments fetch failed (400)"), Times.Once); } @@ -307,31 +265,45 @@ public void ShouldHandle400HttpCode() [Test] public void ShouldHandle500HttpCode() { - var odpClient = new OdpClient(_mockLogger.Object, - GetHttpClientThatReturnsStatus(HttpStatusCode.InternalServerError)); + var httpClient = MakeHttpClient(HttpStatusCode.InternalServerError); var manager = - new GraphQLManager(_mockErrorHandler.Object, _mockLogger.Object, odpClient); - + new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_URL, - FS_USER_ID, + OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); - Assert.IsTrue(segments.Length == 0); + Assert.IsNull(segments); _mockLogger.Verify(l => l.Log(LogLevel.ERROR, "Audience segments fetch failed (500)"), Times.Once); } - private static HttpClient GetHttpClientThatReturnsStatus(HttpStatusCode statusCode) + /// + /// Create an HttpClient instance that returns an expected response + /// + /// Http Status Code to return + /// Message body content to return + /// HttpClient instance that will return desired HttpResponseMessage + private static HttpClient MakeHttpClient(HttpStatusCode statusCode, + string content = default + ) { + var response = new HttpResponseMessage(statusCode); + + if (!string.IsNullOrWhiteSpace(content)) + { + response.Content = new StringContent(content); + } + var mockedHandler = new Mock(); mockedHandler.Protected().Setup>( "SendAsync", ItExpr.IsAny(), ItExpr.IsAny()). - ReturnsAsync(() => new HttpResponseMessage(statusCode)); + ReturnsAsync(response); return new HttpClient(mockedHandler.Object); } } diff --git a/OptimizelySDK/Odp/GraphQLManager.cs b/OptimizelySDK/Odp/GraphQLManager.cs index af1b1b07..891f800b 100644 --- a/OptimizelySDK/Odp/GraphQLManager.cs +++ b/OptimizelySDK/Odp/GraphQLManager.cs @@ -25,6 +25,7 @@ using System.Linq; using System.Net.Http; using System.Text; +using System.Threading; using System.Threading.Tasks; namespace OptimizelySDK.Odp @@ -176,7 +177,7 @@ string query return default; } - var responseStatusCode = int.Parse(response.StatusCode.ToString()); + var responseStatusCode = (int)response.StatusCode; if (responseStatusCode >= 400 && responseStatusCode < 600) { _logger.Log(LogLevel.ERROR, From adaf5bc9e9e85bc4467a9071cd1c5276b50bd28e Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 4 Oct 2022 17:17:36 -0400 Subject: [PATCH 04/14] WIP Fixing unit tests... --- .../OdpTests/GraphQLManagerTest.cs | 56 +++------ .../OdpTests/HttpClientTestUtil.cs | 38 ++++++ .../OdpTests/RestApiManagerTest.cs | 109 ++++++++++++++++++ .../OptimizelySDK.Tests.csproj | 2 + 4 files changed, 164 insertions(+), 41 deletions(-) create mode 100644 OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs create mode 100644 OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs diff --git a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs index 4798d627..d3b27f8b 100644 --- a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs @@ -33,7 +33,7 @@ namespace OptimizelySDK.Tests.OdpTests public class GraphQLManagerTest { private const string VALID_ODP_PUBLIC_KEY = "not-real-odp-public-key"; - private const string ODP_GRAPHQL_URL = "https://example.com/endpoint"; + private const string ODP_GRAPHQL_HOST = "https://graph.example.com"; private readonly List _segmentsToCheck = new List { @@ -138,13 +138,13 @@ public void ShouldFetchValidQualifiedSegments() "{\"edges\":[{\"node\":{\"name\":\"has_email\"," + "\"state\":\"qualified\"}},{\"node\":{\"name\":" + "\"has_email_opted_in\",\"state\":\"qualified\"}}]}}}}"; - var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -160,13 +160,13 @@ public void ShouldHandleEmptyQualifiedSegments() { const string RESPONSE_DATA = "{\"data\":{\"customer\":{\"audiences\":" + "{\"edges\":[ ]}}}}"; - var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -184,13 +184,13 @@ public void ShouldHandleErrorWithInvalidIdentifier() "\"locations\":[{\"line\":1,\"column\":8}],\"path\":[\"customer\"]," + "\"extensions\":{\"classification\":\"DataFetchingException\"}}]," + "\"data\":{\"customer\":null}}"; - var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "invalid-user", _segmentsToCheck); @@ -204,13 +204,13 @@ public void ShouldHandleErrorWithInvalidIdentifier() public void ShouldHandleBadResponse() { const string RESPONSE_DATA = "{\"data\":{ }}"; - var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -226,13 +226,13 @@ public void ShouldHandleUnrecognizedJsonResponse() { const string RESPONSE_DATA = "{\"unExpectedObject\":{ \"withSome\": \"value\", \"thatIsNotParseable\": \"true\" }}"; - var httpClient = MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -246,13 +246,13 @@ public void ShouldHandleUnrecognizedJsonResponse() [Test] public void ShouldHandle400HttpCode() { - var httpClient = MakeHttpClient(HttpStatusCode.BadRequest); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.BadRequest); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -265,13 +265,13 @@ public void ShouldHandle400HttpCode() [Test] public void ShouldHandle500HttpCode() { - var httpClient = MakeHttpClient(HttpStatusCode.InternalServerError); + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.InternalServerError); var manager = new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, - ODP_GRAPHQL_URL, + ODP_GRAPHQL_HOST, OdpUserKeyType.FS_USER_KEY, "tester-101", _segmentsToCheck); @@ -280,31 +280,5 @@ public void ShouldHandle500HttpCode() _mockLogger.Verify(l => l.Log(LogLevel.ERROR, "Audience segments fetch failed (500)"), Times.Once); } - - /// - /// Create an HttpClient instance that returns an expected response - /// - /// Http Status Code to return - /// Message body content to return - /// HttpClient instance that will return desired HttpResponseMessage - private static HttpClient MakeHttpClient(HttpStatusCode statusCode, - string content = default - ) - { - var response = new HttpResponseMessage(statusCode); - - if (!string.IsNullOrWhiteSpace(content)) - { - response.Content = new StringContent(content); - } - - var mockedHandler = new Mock(); - mockedHandler.Protected().Setup>( - "SendAsync", - ItExpr.IsAny(), - ItExpr.IsAny()). - ReturnsAsync(response); - return new HttpClient(mockedHandler.Object); - } } } diff --git a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs new file mode 100644 index 00000000..d9daf8d3 --- /dev/null +++ b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs @@ -0,0 +1,38 @@ +using Moq; +using Moq.Protected; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; + +namespace OptimizelySDK.Tests.OdpTests +{ + public static class HttpClientTestUtil + { + /// + /// Create an HttpClient instance that returns an expected response + /// + /// Http Status Code to return + /// Message body content to return + /// HttpClient instance that will return desired HttpResponseMessage + public static HttpClient MakeHttpClient(HttpStatusCode statusCode, + string content = default + ) + { + var response = new HttpResponseMessage(statusCode); + + if (!string.IsNullOrWhiteSpace(content)) + { + response.Content = new StringContent(content); + } + + var mockedHandler = new Mock(); + mockedHandler.Protected().Setup>( + "SendAsync", + ItExpr.IsAny(), + ItExpr.IsAny()). + ReturnsAsync(response); + return new HttpClient(mockedHandler.Object); + } + } +} diff --git a/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs new file mode 100644 index 00000000..6f7ce297 --- /dev/null +++ b/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs @@ -0,0 +1,109 @@ +/* + * 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 Moq.Protected; +using NUnit.Framework; +using OptimizelySDK.AudienceConditions; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Logger; +using OptimizelySDK.Odp; +using OptimizelySDK.Odp.Entity; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; + +namespace OptimizelySDK.Tests.OdpTests +{ + [TestFixture] + public class RestApiManagerTest + { + private const string VALID_ODP_PUBLIC_KEY = "a-valid-odp-public-key"; + private const string ODP_REST_API_HOST = "https://api.example.com"; + + private List _odpEvents = new List(); + + private Mock _mockErrorHandler; + private Mock _mockLogger; + + [SetUp] + public void Setup() + { + _mockErrorHandler = new Mock(); + _mockLogger = new Mock(); + _mockLogger.Setup(i => i.Log(It.IsAny(), It.IsAny())); + + _odpEvents.Add(new OdpEvent("t1", "a1", + new Dictionary + { + { + "id-key-1", "id-value-1" + }, + }, new Dictionary + { + { + "key11", "value-1" + }, + { + "key12", true + }, + { + "key12", 3.5 + }, + { + "key14", null + }, + } + )); + _odpEvents.Add(new OdpEvent("t2", "a2", new Dictionary + { + { + "id-key-2", "id-value-2" + }, + }, new Dictionary + { + { + "key2", "value-2" + }, + } + )); + } + + [Test] + public void ShouldSendEventsSuccessfullyAnyNotSuggestRetry() + { + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK); + var manger = + new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + + var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, + _odpEvents); + + Assert.IsFalse(shouldRetry); + } + + [Test] + public void ShouldSuggestRetryFor400HttpResponse() { } + + [Test] + public void ShouldSuggestRetryFor500HttpResponse() { } + + [Test] + public void ShouldSuggestRetryForNetworkTimeout() { } + } +} diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index d39fc44e..94e90b86 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -81,7 +81,9 @@ + + From 7fb27beef6bb30c2e9040784a8afc669d07cccfd Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 10 Oct 2022 14:21:26 -0400 Subject: [PATCH 05/14] Completed unit tests --- .../OdpTests/HttpClientTestUtil.cs | 16 ++++++ .../OdpTests/RestApiManagerTest.cs | 53 ++++++++++++++----- OptimizelySDK/Odp/RestApiManager.cs | 2 +- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs index d9daf8d3..dbf12a44 100644 --- a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs +++ b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs @@ -1,5 +1,6 @@ using Moq; using Moq.Protected; +using System; using System.Net; using System.Net.Http; using System.Threading; @@ -34,5 +35,20 @@ public static HttpClient MakeHttpClient(HttpStatusCode statusCode, ReturnsAsync(response); return new HttpClient(mockedHandler.Object); } + + /// + /// Create an HttpClient instance that will timeout for SendAsync calls + /// + /// HttpClient instance that throw TimeoutException + public static HttpClient MakeHttpClientWithTimeout() + { + var mockedHandler = new Mock(); + mockedHandler.Protected().Setup>( + "SendAsync", + ItExpr.IsAny(), + ItExpr.IsAny()). + Throws(); + return new HttpClient(mockedHandler.Object); + } } } diff --git a/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs index 6f7ce297..1d172b67 100644 --- a/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs @@ -15,18 +15,13 @@ */ using Moq; -using Moq.Protected; using NUnit.Framework; -using OptimizelySDK.AudienceConditions; using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; using OptimizelySDK.Odp; using OptimizelySDK.Odp.Entity; using System.Collections.Generic; using System.Net; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; namespace OptimizelySDK.Tests.OdpTests { @@ -36,12 +31,12 @@ public class RestApiManagerTest private const string VALID_ODP_PUBLIC_KEY = "a-valid-odp-public-key"; private const string ODP_REST_API_HOST = "https://api.example.com"; - private List _odpEvents = new List(); + private readonly List _odpEvents = new List(); private Mock _mockErrorHandler; private Mock _mockLogger; - [SetUp] + [TestFixtureSetUp] public void Setup() { _mockErrorHandler = new Mock(); @@ -54,7 +49,8 @@ public void Setup() { "id-key-1", "id-value-1" }, - }, new Dictionary + }, + new Dictionary { { "key11", "value-1" @@ -63,14 +59,15 @@ public void Setup() "key12", true }, { - "key12", 3.5 + "key13", 3.5 }, { "key14", null }, } )); - _odpEvents.Add(new OdpEvent("t2", "a2", new Dictionary + _odpEvents.Add(new OdpEvent("t2", "a2", + new Dictionary { { "id-key-2", "id-value-2" @@ -85,7 +82,7 @@ public void Setup() } [Test] - public void ShouldSendEventsSuccessfullyAnyNotSuggestRetry() + public void ShouldSendEventsSuccessfullyAndNotSuggestRetry() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK); var manger = @@ -98,12 +95,40 @@ public void ShouldSendEventsSuccessfullyAnyNotSuggestRetry() } [Test] - public void ShouldSuggestRetryFor400HttpResponse() { } + public void ShouldNotSuggestRetryFor400HttpResponse() + { + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.BadRequest); + var manger = + new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + + var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, + _odpEvents); + + Assert.IsFalse(shouldRetry); + } [Test] - public void ShouldSuggestRetryFor500HttpResponse() { } + public void ShouldSuggestRetryFor500HttpResponse() + { + var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.InternalServerError); + var manger = + new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + + var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, + _odpEvents); + + Assert.IsTrue(shouldRetry); + } [Test] - public void ShouldSuggestRetryForNetworkTimeout() { } + public void ShouldSuggestRetryForNetworkTimeout() { + var httpClient = HttpClientTestUtil.MakeHttpClientWithTimeout(); + var manger = + new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + + var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, + _odpEvents); + + Assert.IsTrue(shouldRetry);} } } diff --git a/OptimizelySDK/Odp/RestApiManager.cs b/OptimizelySDK/Odp/RestApiManager.cs index 2ed0a0f0..929f788d 100644 --- a/OptimizelySDK/Odp/RestApiManager.cs +++ b/OptimizelySDK/Odp/RestApiManager.cs @@ -101,7 +101,7 @@ public bool SendEvents(string apiKey, string apiHost, List events) shouldRetry = true; } - var responseStatusCode = int.Parse(response?.StatusCode.ToString() ?? "0"); + var responseStatusCode = response == null ? 0 : (int)response.StatusCode; if (responseStatusCode >= 400) { _logger.Log(LogLevel.ERROR, From f8bce3a5b4574be311d278008db8e47556b89de7 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 13 Oct 2022 09:28:08 -0400 Subject: [PATCH 06/14] Clean up usings; add missing Copyright notices --- .../OdpTests/GraphQLManagerTest.cs | 4 ---- .../OdpTests/HttpClientTestUtil.cs | 18 +++++++++++++++++- OptimizelySDK.Tests/OdpTests/LruCacheTest.cs | 1 - OptimizelySDK/Odp/Enums.cs | 18 +++++++++++++++++- OptimizelySDK/Odp/GraphQLManager.cs | 1 - OptimizelySDK/Odp/IRestApiManager.cs | 1 - 6 files changed, 34 insertions(+), 9 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs index d3b27f8b..b75e8854 100644 --- a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs @@ -15,7 +15,6 @@ */ using Moq; -using Moq.Protected; using NUnit.Framework; using OptimizelySDK.AudienceConditions; using OptimizelySDK.ErrorHandler; @@ -23,9 +22,6 @@ using OptimizelySDK.Odp; using System.Collections.Generic; using System.Net; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; namespace OptimizelySDK.Tests.OdpTests { diff --git a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs index dbf12a44..c530a798 100644 --- a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs +++ b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs @@ -1,4 +1,20 @@ -using Moq; +/* + * 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 Moq.Protected; using System; using System.Net; diff --git a/OptimizelySDK.Tests/OdpTests/LruCacheTest.cs b/OptimizelySDK.Tests/OdpTests/LruCacheTest.cs index a1443956..dd5183ab 100644 --- a/OptimizelySDK.Tests/OdpTests/LruCacheTest.cs +++ b/OptimizelySDK.Tests/OdpTests/LruCacheTest.cs @@ -18,7 +18,6 @@ using OptimizelySDK.Odp; using System; using System.Collections.Generic; -using System.Linq; using System.Threading; namespace OptimizelySDK.Tests.OdpTests diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index ae1251f1..30eecc63 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -1,4 +1,20 @@ -namespace OptimizelySDK.Odp +/* + * 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. + */ + +namespace OptimizelySDK.Odp { public enum OdpUserKeyType { diff --git a/OptimizelySDK/Odp/GraphQLManager.cs b/OptimizelySDK/Odp/GraphQLManager.cs index 891f800b..0e60989a 100644 --- a/OptimizelySDK/Odp/GraphQLManager.cs +++ b/OptimizelySDK/Odp/GraphQLManager.cs @@ -25,7 +25,6 @@ using System.Linq; using System.Net.Http; using System.Text; -using System.Threading; using System.Threading.Tasks; namespace OptimizelySDK.Odp diff --git a/OptimizelySDK/Odp/IRestApiManager.cs b/OptimizelySDK/Odp/IRestApiManager.cs index e3a32788..295b3bdd 100644 --- a/OptimizelySDK/Odp/IRestApiManager.cs +++ b/OptimizelySDK/Odp/IRestApiManager.cs @@ -16,7 +16,6 @@ using OptimizelySDK.Odp.Entity; using System.Collections.Generic; -using System.Threading.Tasks; namespace OptimizelySDK.Odp { From 0c4885e7eaf32a25ec437866663b21feb5da7580 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 13 Oct 2022 09:35:23 -0400 Subject: [PATCH 07/14] Add 'QL' to DotSettings; Disable Enum InconsistentNaming --- OptimizelySDK.sln.DotSettings | 3 ++- OptimizelySDK/Odp/Enums.cs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index d900f70e..8f730231 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -38,6 +38,7 @@ True True True + QL <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb"><ExtraRule Prefix="" Suffix="" Style="AaBb" /></Policy> <Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> @@ -46,4 +47,4 @@ True True True - True \ No newline at end of file + True diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index 30eecc63..b08deb9c 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -18,6 +18,7 @@ namespace OptimizelySDK.Odp { public enum OdpUserKeyType { + // ReSharper disable InconsistentNaming // ODP expects these names; .ToString() used VUID = 0, FS_USER_KEY = 1, From c98ee82a033e6c39483c2a88a552cbdf5729fba1 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 17 Oct 2022 10:05:59 -0400 Subject: [PATCH 08/14] Code review changes --- .../OdpTests/HttpClientTestUtil.cs | 3 + ...nagerTest.cs => OdpEventApiManagerTest.cs} | 10 +-- ...gerTest.cs => OdpSegmentApiManagerTest.cs} | 20 +++--- .../OptimizelySDK.Tests.csproj | 4 +- OptimizelySDK/Odp/Entity/OdpEvent.cs | 37 ++++++++-- ...stApiManager.cs => IOdpEventApiManager.cs} | 2 +- ...hQLManager.cs => IOdpSegmentApiManager.cs} | 2 +- ...estApiManager.cs => OdpEventApiManager.cs} | 14 ++-- ...phQLManager.cs => OdpSegmentApiManager.cs} | 72 ++++++++++++------- OptimizelySDK/OptimizelySDK.csproj | 8 +-- 10 files changed, 110 insertions(+), 62 deletions(-) rename OptimizelySDK.Tests/OdpTests/{RestApiManagerTest.cs => OdpEventApiManagerTest.cs} (90%) rename OptimizelySDK.Tests/OdpTests/{GraphQLManagerTest.cs => OdpSegmentApiManagerTest.cs} (91%) rename OptimizelySDK/Odp/{IRestApiManager.cs => IOdpEventApiManager.cs} (95%) rename OptimizelySDK/Odp/{IGraphQLManager.cs => IOdpSegmentApiManager.cs} (97%) rename OptimizelySDK/Odp/{RestApiManager.cs => OdpEventApiManager.cs} (92%) rename OptimizelySDK/Odp/{GraphQLManager.cs => OdpSegmentApiManager.cs} (76%) diff --git a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs index c530a798..e0b3b372 100644 --- a/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs +++ b/OptimizelySDK.Tests/OdpTests/HttpClientTestUtil.cs @@ -24,6 +24,9 @@ namespace OptimizelySDK.Tests.OdpTests { + /// + /// Shared utility methods used for stubbing HttpClient instances + /// public static class HttpClientTestUtil { /// diff --git a/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpEventApiManagerTest.cs similarity index 90% rename from OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs rename to OptimizelySDK.Tests/OdpTests/OdpEventApiManagerTest.cs index 1d172b67..2a987625 100644 --- a/OptimizelySDK.Tests/OdpTests/RestApiManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventApiManagerTest.cs @@ -26,7 +26,7 @@ namespace OptimizelySDK.Tests.OdpTests { [TestFixture] - public class RestApiManagerTest + public class OdpEventApiManagerTest { private const string VALID_ODP_PUBLIC_KEY = "a-valid-odp-public-key"; private const string ODP_REST_API_HOST = "https://api.example.com"; @@ -86,7 +86,7 @@ public void ShouldSendEventsSuccessfullyAndNotSuggestRetry() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK); var manger = - new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpEventApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, _odpEvents); @@ -99,7 +99,7 @@ public void ShouldNotSuggestRetryFor400HttpResponse() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.BadRequest); var manger = - new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpEventApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, _odpEvents); @@ -112,7 +112,7 @@ public void ShouldSuggestRetryFor500HttpResponse() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.InternalServerError); var manger = - new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpEventApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, _odpEvents); @@ -124,7 +124,7 @@ public void ShouldSuggestRetryFor500HttpResponse() public void ShouldSuggestRetryForNetworkTimeout() { var httpClient = HttpClientTestUtil.MakeHttpClientWithTimeout(); var manger = - new RestApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpEventApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var shouldRetry = manger.SendEvents(VALID_ODP_PUBLIC_KEY, ODP_REST_API_HOST, _odpEvents); diff --git a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs similarity index 91% rename from OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs rename to OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs index b75e8854..5e82e979 100644 --- a/OptimizelySDK.Tests/OdpTests/GraphQLManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs @@ -26,7 +26,7 @@ namespace OptimizelySDK.Tests.OdpTests { [TestFixture] - public class GraphQLManagerTest + public class OdpSegmentApiManagerTest { private const string VALID_ODP_PUBLIC_KEY = "not-real-odp-public-key"; private const string ODP_GRAPHQL_HOST = "https://graph.example.com"; @@ -76,7 +76,7 @@ public void ShouldParseSuccessfulResponse() } }"; - var response = GraphQLManager.ParseSegmentsResponseJson(RESPONSE_JSON); + var response = OdpSegmentApiManager.ParseSegmentsResponseJson(RESPONSE_JSON); Assert.IsNull(response.Errors); Assert.IsNotNull(response.Data); @@ -119,7 +119,7 @@ public void ShouldParseErrorResponse() } }"; - var response = GraphQLManager.ParseSegmentsResponseJson(RESPONSE_JSON); + var response = OdpSegmentApiManager.ParseSegmentsResponseJson(RESPONSE_JSON); Assert.IsNull(response.Data.Customer); Assert.IsNotNull(response.Errors); @@ -136,7 +136,7 @@ public void ShouldFetchValidQualifiedSegments() "\"has_email_opted_in\",\"state\":\"qualified\"}}]}}}}"; var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -158,7 +158,7 @@ public void ShouldHandleEmptyQualifiedSegments() "{\"edges\":[ ]}}}}"; var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -182,7 +182,7 @@ public void ShouldHandleErrorWithInvalidIdentifier() "\"data\":{\"customer\":null}}"; var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -202,7 +202,7 @@ public void ShouldHandleBadResponse() const string RESPONSE_DATA = "{\"data\":{ }}"; var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -224,7 +224,7 @@ public void ShouldHandleUnrecognizedJsonResponse() "{\"unExpectedObject\":{ \"withSome\": \"value\", \"thatIsNotParseable\": \"true\" }}"; var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.OK, RESPONSE_DATA); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -244,7 +244,7 @@ public void ShouldHandle400HttpCode() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.BadRequest); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, @@ -263,7 +263,7 @@ public void ShouldHandle500HttpCode() { var httpClient = HttpClientTestUtil.MakeHttpClient(HttpStatusCode.InternalServerError); var manager = - new GraphQLManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); + new OdpSegmentApiManager(_mockLogger.Object, _mockErrorHandler.Object, httpClient); var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index 94e90b86..3271653f 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -80,10 +80,10 @@ - + - + diff --git a/OptimizelySDK/Odp/Entity/OdpEvent.cs b/OptimizelySDK/Odp/Entity/OdpEvent.cs index 0370cb85..5255d761 100644 --- a/OptimizelySDK/Odp/Entity/OdpEvent.cs +++ b/OptimizelySDK/Odp/Entity/OdpEvent.cs @@ -18,21 +18,46 @@ namespace OptimizelySDK.Odp.Entity { + /// + /// Event data object targeted to requirements of the Optimizely Data Platform + /// public class OdpEvent { - public string Type; + /// + /// Type of event (typically `fullstack` from SDK events) + /// + public string Type { get; } - public string Action; + /// + /// Subcategory of the event type + /// + public string Action { get; } - public Dictionary Identifiers; + /// + /// Key-value map of user identifiers + /// + public Dictionary Identifiers { get; } - public Dictionary Data; + /// + /// Event data in a key-value pair format + /// + public Dictionary Data { get; } - public OdpEvent(string type, string action, Dictionary identifiers, Dictionary data) { + /// + /// Event to be sent and stored in the Optimizely Data Platform + /// + /// Type of event (typically `fullstack` from SDK events) + /// Subcategory of the event type + /// Key-value map of user identifiers + /// Event data in a key-value pair format + public OdpEvent(string type, string action, Dictionary identifiers, + Dictionary data + ) + { Type = type; Action = action; Identifiers = identifiers ?? new Dictionary(); - Data = data ?? new Dictionary(); + Data = data ?? new Dictionary(); } } } diff --git a/OptimizelySDK/Odp/IRestApiManager.cs b/OptimizelySDK/Odp/IOdpEventApiManager.cs similarity index 95% rename from OptimizelySDK/Odp/IRestApiManager.cs rename to OptimizelySDK/Odp/IOdpEventApiManager.cs index 295b3bdd..7787681c 100644 --- a/OptimizelySDK/Odp/IRestApiManager.cs +++ b/OptimizelySDK/Odp/IOdpEventApiManager.cs @@ -19,7 +19,7 @@ namespace OptimizelySDK.Odp { - public interface IRestApiManager + public interface IOdpEventApiManager { bool SendEvents(string apiKey, string apiHost, List events); } diff --git a/OptimizelySDK/Odp/IGraphQLManager.cs b/OptimizelySDK/Odp/IOdpSegmentApiManager.cs similarity index 97% rename from OptimizelySDK/Odp/IGraphQLManager.cs rename to OptimizelySDK/Odp/IOdpSegmentApiManager.cs index 99f3ba02..e1cbf23b 100644 --- a/OptimizelySDK/Odp/IGraphQLManager.cs +++ b/OptimizelySDK/Odp/IOdpSegmentApiManager.cs @@ -18,7 +18,7 @@ namespace OptimizelySDK.Odp { - public interface IGraphQLManager + public interface IOdpSegmentApiManager { /// /// Retrieves the audience segments from ODP diff --git a/OptimizelySDK/Odp/RestApiManager.cs b/OptimizelySDK/Odp/OdpEventApiManager.cs similarity index 92% rename from OptimizelySDK/Odp/RestApiManager.cs rename to OptimizelySDK/Odp/OdpEventApiManager.cs index 929f788d..772db1a2 100644 --- a/OptimizelySDK/Odp/RestApiManager.cs +++ b/OptimizelySDK/Odp/OdpEventApiManager.cs @@ -29,7 +29,7 @@ namespace OptimizelySDK.Odp /// /// Manager for communicating with ODP's REST API endpoint /// - public class RestApiManager : IRestApiManager + public class OdpEventApiManager : IOdpEventApiManager { private const string EVENT_SENDING_FAILURE_MESSAGE = "ODP event send failed"; @@ -54,7 +54,7 @@ public class RestApiManager : IRestApiManager /// Collect and record events to log /// Handler to record exceptions /// HttpClient to use to send ODP events - public RestApiManager(ILogger logger = null, IErrorHandler errorHandler = null, + public OdpEventApiManager(ILogger logger = null, IErrorHandler errorHandler = null, HttpClient httpClient = null ) { @@ -127,21 +127,19 @@ private async Task SendEventsAsync(string apiKey, string en string data ) { - var request = BuildRequestMessage(apiKey, endpoint, data); + var request = BuildOdpEventMessage(apiKey, endpoint, data); - var response = await _httpClient.SendAsync(request); - - return response; + return await _httpClient.SendAsync(request); } /// - /// Build an HTTP request message + /// Build an HTTP request message to send the ODP events /// /// ODP public key /// Fully-qualified ODP REST API endpoint /// JSON string version of ODP event data /// Ready request message to send - private static HttpRequestMessage BuildRequestMessage(string apiKey, string endpoint, + private static HttpRequestMessage BuildOdpEventMessage(string apiKey, string endpoint, string data ) { diff --git a/OptimizelySDK/Odp/GraphQLManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs similarity index 76% rename from OptimizelySDK/Odp/GraphQLManager.cs rename to OptimizelySDK/Odp/OdpSegmentApiManager.cs index 0e60989a..b0421c66 100644 --- a/OptimizelySDK/Odp/GraphQLManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -32,13 +32,18 @@ namespace OptimizelySDK.Odp /// /// Manager for communicating with the Optimizely Data Platform GraphQL endpoint /// - public class GraphQLManager : IGraphQLManager + public class OdpSegmentApiManager : IOdpSegmentApiManager { /// /// Standard message for audience querying fetch errors /// private const string AUDIENCE_FETCH_FAILURE_MESSAGE = "Audience segments fetch failed"; + /// + /// Specific key for designating the ODP API public key + /// + private const string HEADER_API_KEY = "x-api-key"; + /// /// Error handler used to record errors /// @@ -60,7 +65,7 @@ public class GraphQLManager : IGraphQLManager /// Collect and record events to log /// Handler to record exceptions /// HttpClient to use to send queries to ODP - public GraphQLManager(ILogger logger = null, IErrorHandler errorHandler = null, + public OdpSegmentApiManager(ILogger logger = null, IErrorHandler errorHandler = null, HttpClient httpClient = null ) { @@ -95,7 +100,8 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user } var endpoint = $"{apiHost}/v3/graphql"; - var query = ToGraphQLJson(userKey.ToString(), userValue, segmentsToCheck); + var query = + BuildGetSegmentsGraphQLQuery(userKey.ToString().ToLower(), userValue, segmentsToCheck); var segmentsResponseJson = QuerySegments(apiKey, endpoint, query); if (CanBeJsonParsed(segmentsResponseJson)) @@ -108,8 +114,8 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user var parsedSegments = ParseSegmentsResponseJson(segmentsResponseJson); if (parsedSegments is null) { - _logger.Log(LogLevel.ERROR, - $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"); + var message = $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"; + _logger.Log(LogLevel.ERROR, message); return null; } @@ -122,7 +128,7 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user return null; } - if (parsedSegments.Data?.Customer?.Audiences?.Edges is null) + if (parsedSegments.Data?.Customer?.Audiences?.Edges == null) { _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"); @@ -135,30 +141,49 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user } /// - /// Converts the current QuerySegmentsParameters into a GraphQL query string + /// Build GraphQL query for getting segments /// - /// GraphQL payload - private static string ToGraphQLJson(string userKey, string userValue, + /// 'vuid' or 'fs_user_id key' + /// Associated value to query for the user key + /// Audience segments to check for experiment inclusion + /// GraphQL string payload + private static string BuildGetSegmentsGraphQLQuery(string userKey, string userValue, IEnumerable segmentsToCheck ) { + // make `example-user-4213` into `\"example-user-4213\"` var userValueWithEscapedQuotes = $"\\\"{userValue}\\\""; + + // serialize collection of `"has_cart_items", "has_seen_promo"` into + // JSON array with quotation marks `[\"has_cart_items\", \"has_seen_promo\"] var segmentsArrayJson = JsonConvert.SerializeObject(segmentsToCheck).Replace("\"", "\\\""); - var json = new StringBuilder(); - json.Append("{\"query\" : \"query {customer"); - json.Append($"({userKey} : {userValueWithEscapedQuotes}) "); - json.Append("{audiences"); - json.Append($"(subset: {segmentsArrayJson})"); - json.Append("{edges {node {name state}}}}}\"}"); - - return json.ToString(); + // Under C# 11 we can use $$""" ... """ for multiline string interpolation and + // surround code with double {{ }} + return + "{\"query\" : " + + " \"query {" + + $" customer({userKey} : {userValueWithEscapedQuotes}) {{" + + $" audiences(subset: {segmentsArrayJson}) {{" + + " edges {" + + " node {" + + " name" + + " state" + + " }" + + " }" + + " }" + + " }" + + " \"}" + + "}"; } /// - /// Synchronous handler for querying the ODP GraphQL endpoint + /// Synchronous handler for querying the ODP GraphQL endpoint /// + /// ODP public API key + /// Fully-qualified ODP GraphQL Endpoint + /// GraphQL query string to send /// JSON response from ODP private string QuerySegments(string apiKey, string endpoint, string query @@ -176,11 +201,10 @@ string query return default; } - var responseStatusCode = (int)response.StatusCode; - if (responseStatusCode >= 400 && responseStatusCode < 600) + if (!response.IsSuccessStatusCode) { _logger.Log(LogLevel.ERROR, - $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({responseStatusCode})"); + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({(int)response.StatusCode})"); return default; } @@ -200,9 +224,7 @@ string query { var request = BuildRequestMessage(apiKey, endpoint, query); - var response = await _httpClient.SendAsync(request); - - return response; + return await _httpClient.SendAsync(request); } /// @@ -223,7 +245,7 @@ string query Headers = { { - "x-api-key", apiKey + HEADER_API_KEY, apiKey }, }, Content = new StringContent(query, Encoding.UTF8, "application/json"), diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 3924df64..819f6292 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -99,12 +99,12 @@ - - + + - + - + From 7bfb3bff4354b23a45bd9d676db7702e827b9545 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 18 Oct 2022 11:31:39 -0400 Subject: [PATCH 09/14] Code review changes --- .../OdpTests/OdpSegmentApiManagerTest.cs | 14 +++--- OptimizelySDK.sln.DotSettings | 1 + OptimizelySDK/Odp/Constants.cs | 10 ++++ OptimizelySDK/Odp/Enums.cs | 2 +- OptimizelySDK/Odp/OdpEventApiManager.cs | 8 +++- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 48 +++++++------------ OptimizelySDK/OptimizelySDK.csproj | 1 + 7 files changed, 44 insertions(+), 40 deletions(-) create mode 100644 OptimizelySDK/Odp/Constants.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs index 5e82e979..ebdf3a68 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs @@ -141,7 +141,7 @@ public void ShouldFetchValidQualifiedSegments() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); @@ -163,7 +163,7 @@ public void ShouldHandleEmptyQualifiedSegments() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); @@ -187,7 +187,7 @@ public void ShouldHandleErrorWithInvalidIdentifier() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "invalid-user", _segmentsToCheck); @@ -207,7 +207,7 @@ public void ShouldHandleBadResponse() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); @@ -229,7 +229,7 @@ public void ShouldHandleUnrecognizedJsonResponse() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); @@ -249,7 +249,7 @@ public void ShouldHandle400HttpCode() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); @@ -268,7 +268,7 @@ public void ShouldHandle500HttpCode() var segments = manager.FetchSegments( VALID_ODP_PUBLIC_KEY, ODP_GRAPHQL_HOST, - OdpUserKeyType.FS_USER_KEY, + OdpUserKeyType.FS_USER_ID, "tester-101", _segmentsToCheck); diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index 8f730231..d2bd6370 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -47,4 +47,5 @@ True True True + True True diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs new file mode 100644 index 00000000..0f82e491 --- /dev/null +++ b/OptimizelySDK/Odp/Constants.cs @@ -0,0 +1,10 @@ +namespace OptimizelySDK.Odp +{ + public class Constants + { + /// + /// Specific key for designating the ODP API public key + /// + public const string HEADER_API_KEY = "x-api-key"; + } +} diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index b08deb9c..0209b7a8 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -21,6 +21,6 @@ public enum OdpUserKeyType // ReSharper disable InconsistentNaming // ODP expects these names; .ToString() used VUID = 0, - FS_USER_KEY = 1, + FS_USER_ID = 1, } } diff --git a/OptimizelySDK/Odp/OdpEventApiManager.cs b/OptimizelySDK/Odp/OdpEventApiManager.cs index 772db1a2..86082793 100644 --- a/OptimizelySDK/Odp/OdpEventApiManager.cs +++ b/OptimizelySDK/Odp/OdpEventApiManager.cs @@ -20,6 +20,7 @@ using OptimizelySDK.Odp.Entity; using System; using System.Collections.Generic; +using System.Net; using System.Net.Http; using System.Text; using System.Threading.Tasks; @@ -31,6 +32,9 @@ namespace OptimizelySDK.Odp /// public class OdpEventApiManager : IOdpEventApiManager { + /// + /// Standard message for ODP event sending errors + /// private const string EVENT_SENDING_FAILURE_MESSAGE = "ODP event send failed"; /// @@ -129,6 +133,8 @@ string data { var request = BuildOdpEventMessage(apiKey, endpoint, data); + ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; + return await _httpClient.SendAsync(request); } @@ -150,7 +156,7 @@ string data Headers = { { - "x-api-key", apiKey + Constants.HEADER_API_KEY, apiKey }, }, Content = new StringContent(data, Encoding.UTF8, "application/json"), diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index b0421c66..fe9d1b15 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -23,6 +23,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Net; using System.Net.Http; using System.Text; using System.Threading.Tasks; @@ -39,10 +40,6 @@ public class OdpSegmentApiManager : IOdpSegmentApiManager /// private const string AUDIENCE_FETCH_FAILURE_MESSAGE = "Audience segments fetch failed"; - /// - /// Specific key for designating the ODP API public key - /// - private const string HEADER_API_KEY = "x-api-key"; /// /// Error handler used to record errors @@ -101,7 +98,8 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user var endpoint = $"{apiHost}/v3/graphql"; var query = - BuildGetSegmentsGraphQLQuery(userKey.ToString().ToLower(), userValue, segmentsToCheck); + BuildGetSegmentsGraphQLQuery(userKey.ToString().ToLower(), userValue, + segmentsToCheck); var segmentsResponseJson = QuerySegments(apiKey, endpoint, query); if (CanBeJsonParsed(segmentsResponseJson)) @@ -151,33 +149,19 @@ private static string BuildGetSegmentsGraphQLQuery(string userKey, string userVa IEnumerable segmentsToCheck ) { - // make `example-user-4213` into `\"example-user-4213\"` - var userValueWithEscapedQuotes = $"\\\"{userValue}\\\""; - - // serialize collection of `"has_cart_items", "has_seen_promo"` into - // JSON array with quotation marks `[\"has_cart_items\", \"has_seen_promo\"] - var segmentsArrayJson = - JsonConvert.SerializeObject(segmentsToCheck).Replace("\"", "\\\""); - - // Under C# 11 we can use $$""" ... """ for multiline string interpolation and - // surround code with double {{ }} - return - "{\"query\" : " + - " \"query {" + - $" customer({userKey} : {userValueWithEscapedQuotes}) {{" + - $" audiences(subset: {segmentsArrayJson}) {{" + - " edges {" + - " node {" + - " name" + - " state" + - " }" + - " }" + - " }" + - " }" + - " \"}" + - "}"; + var query = + "query($userId: String, $audiences: [String]) {customer(|userKey|: $userId){audiences(subset: $audiences) {edges {node {name state}}}}}". + Replace("|userKey|", userKey); + + var variables = + "\"variables\": { \"userId\": \"|userValue|\", \"audiences\": |audiences| }". + Replace("|userValue|", userValue). + Replace("|audiences|", JsonConvert.SerializeObject(segmentsToCheck)); + + return $"{{ \"query\": \"{query}\", {variables} }}"; } + /// /// Synchronous handler for querying the ODP GraphQL endpoint /// @@ -224,6 +208,8 @@ string query { var request = BuildRequestMessage(apiKey, endpoint, query); + ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; + return await _httpClient.SendAsync(request); } @@ -245,7 +231,7 @@ string query Headers = { { - HEADER_API_KEY, apiKey + Constants.HEADER_API_KEY, apiKey }, }, Content = new StringContent(query, Encoding.UTF8, "application/json"), diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 819f6292..d3ecc043 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -88,6 +88,7 @@ + From 6269a4cc216f39038d3ad0805594868d6ae899a8 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 18 Oct 2022 11:43:51 -0400 Subject: [PATCH 10/14] Copyright notice & static --- OptimizelySDK/Odp/Constants.cs | 20 ++++++++++++++++++-- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 1 - 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index 0f82e491..ee204e4e 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -1,6 +1,22 @@ -namespace OptimizelySDK.Odp +/* + * 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. + */ + +namespace OptimizelySDK.Odp { - public class Constants + public static class Constants { /// /// Specific key for designating the ODP API public key diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index fe9d1b15..d3a7e3cc 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -40,7 +40,6 @@ public class OdpSegmentApiManager : IOdpSegmentApiManager /// private const string AUDIENCE_FETCH_FAILURE_MESSAGE = "Audience segments fetch failed"; - /// /// Error handler used to record errors /// From 6ce4d491f7681e5beef2296be0f352732c4ea254 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 19 Oct 2022 13:09:43 -0400 Subject: [PATCH 11/14] Bitwise OR TLS12 into SystemDefault instead of hard-coding to 1.2 --- OptimizelySDK/Odp/OdpEventApiManager.cs | 2 +- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventApiManager.cs b/OptimizelySDK/Odp/OdpEventApiManager.cs index 86082793..556d1b60 100644 --- a/OptimizelySDK/Odp/OdpEventApiManager.cs +++ b/OptimizelySDK/Odp/OdpEventApiManager.cs @@ -133,7 +133,7 @@ string data { var request = BuildOdpEventMessage(apiKey, endpoint, data); - ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; + ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12; return await _httpClient.SendAsync(request); } diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index d3a7e3cc..7564fa6c 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -207,7 +207,7 @@ string query { var request = BuildRequestMessage(apiKey, endpoint, query); - ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; + ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12; return await _httpClient.SendAsync(request); } From 8f23e031c5b766ea8ba678d55114be3cf9c3a494 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 19 Oct 2022 13:32:28 -0400 Subject: [PATCH 12/14] Remove ServicePointManager.SecurityProtocol enum declarations --- OptimizelySDK/Odp/OdpEventApiManager.cs | 2 -- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventApiManager.cs b/OptimizelySDK/Odp/OdpEventApiManager.cs index 556d1b60..35ee441f 100644 --- a/OptimizelySDK/Odp/OdpEventApiManager.cs +++ b/OptimizelySDK/Odp/OdpEventApiManager.cs @@ -133,8 +133,6 @@ string data { var request = BuildOdpEventMessage(apiKey, endpoint, data); - ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12; - return await _httpClient.SendAsync(request); } diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index 7564fa6c..280d5b23 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -207,8 +207,6 @@ string query { var request = BuildRequestMessage(apiKey, endpoint, query); - ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12; - return await _httpClient.SendAsync(request); } From ca90ccb038db64f717d24b1e1e8a4c576d596ef2 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 20 Oct 2022 16:54:54 -0400 Subject: [PATCH 13/14] Code review changes --- .../OdpTests/OdpSegmentApiManagerTest.cs | 4 +- OptimizelySDK/Odp/Constants.cs | 20 ++++ OptimizelySDK/Odp/OdpEventApiManager.cs | 31 +++--- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 97 +++++++++++-------- 4 files changed, 94 insertions(+), 58 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs index ebdf3a68..f0aa9c16 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs @@ -76,7 +76,7 @@ public void ShouldParseSuccessfulResponse() } }"; - var response = OdpSegmentApiManager.ParseSegmentsResponseJson(RESPONSE_JSON); + var response = OdpSegmentApiManager.DeserializeSegmentsFromJson(RESPONSE_JSON); Assert.IsNull(response.Errors); Assert.IsNotNull(response.Data); @@ -119,7 +119,7 @@ public void ShouldParseErrorResponse() } }"; - var response = OdpSegmentApiManager.ParseSegmentsResponseJson(RESPONSE_JSON); + var response = OdpSegmentApiManager.DeserializeSegmentsFromJson(RESPONSE_JSON); Assert.IsNull(response.Data.Customer); Assert.IsNotNull(response.Errors); diff --git a/OptimizelySDK/Odp/Constants.cs b/OptimizelySDK/Odp/Constants.cs index ee204e4e..5ce7d6d3 100644 --- a/OptimizelySDK/Odp/Constants.cs +++ b/OptimizelySDK/Odp/Constants.cs @@ -22,5 +22,25 @@ public static class Constants /// Specific key for designating the ODP API public key /// public const string HEADER_API_KEY = "x-api-key"; + + /// + /// Media type for json requests + /// + public const string APPLICATION_JSON_MEDIA_TYPE = "application/json"; + + /// + /// Path to ODP REST events API + /// + public const string ODP_EVENTS_API_ENDPOINT_PATH = "/v3/events"; + + /// + /// Path to ODP GraphQL API + /// + public const string ODP_GRAPHQL_API_ENDPOINT_PATH = "/v3/graphql"; + + /// + /// Default message when numeric HTTP status code is not available + /// + public const string NETWORK_ERROR_REASON = "network error"; } } diff --git a/OptimizelySDK/Odp/OdpEventApiManager.cs b/OptimizelySDK/Odp/OdpEventApiManager.cs index 35ee441f..25b62ec7 100644 --- a/OptimizelySDK/Odp/OdpEventApiManager.cs +++ b/OptimizelySDK/Odp/OdpEventApiManager.cs @@ -89,7 +89,7 @@ public bool SendEvents(string apiKey, string apiHost, List events) return false; } - var endpoint = $"{apiHost}/v3/events"; + var endpoint = $"{apiHost}{Constants.ODP_EVENTS_API_ENDPOINT_PATH}"; var data = JsonConvert.SerializeObject(events); var shouldRetry = false; @@ -97,23 +97,25 @@ public bool SendEvents(string apiKey, string apiHost, List events) try { response = SendEventsAsync(apiKey, endpoint, data).GetAwaiter().GetResult(); + response.EnsureSuccessStatusCode(); } - catch (Exception ex) + catch (HttpRequestException ex) { _errorHandler.HandleError(ex); - _logger.Log(LogLevel.ERROR, $"{EVENT_SENDING_FAILURE_MESSAGE} (network error)"); - shouldRetry = true; - } - var responseStatusCode = response == null ? 0 : (int)response.StatusCode; - if (responseStatusCode >= 400) - { + var statusCode = response == null ? 0 : (int)response.StatusCode; + _logger.Log(LogLevel.ERROR, - $"{EVENT_SENDING_FAILURE_MESSAGE} (${responseStatusCode})"); - } + $"{EVENT_SENDING_FAILURE_MESSAGE} ({(statusCode == 0 ? Constants.NETWORK_ERROR_REASON : statusCode.ToString())})"); - if (responseStatusCode >= 500) + shouldRetry = statusCode >= 500; + } + catch (Exception ex) { + _errorHandler.HandleError(ex); + + _logger.Log(LogLevel.ERROR, $"{EVENT_SENDING_FAILURE_MESSAGE} ({Constants.NETWORK_ERROR_REASON})"); + shouldRetry = true; } @@ -127,13 +129,13 @@ public bool SendEvents(string apiKey, string apiHost, List events) /// Fully-qualified ODP REST API endpoint /// JSON string version of ODP event data /// HTTP response endpoint - private async Task SendEventsAsync(string apiKey, string endpoint, + private Task SendEventsAsync(string apiKey, string endpoint, string data ) { var request = BuildOdpEventMessage(apiKey, endpoint, data); - return await _httpClient.SendAsync(request); + return _httpClient.SendAsync(request); } /// @@ -157,7 +159,8 @@ string data Constants.HEADER_API_KEY, apiKey }, }, - Content = new StringContent(data, Encoding.UTF8, "application/json"), + Content = new StringContent(data, Encoding.UTF8, + Constants.APPLICATION_JSON_MEDIA_TYPE), }; return request; diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index 280d5b23..9a8dcf7f 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -23,7 +23,6 @@ using System.Collections; using System.Collections.Generic; using System.Linq; -using System.Net; using System.Net.Http; using System.Text; using System.Threading.Tasks; @@ -75,7 +74,7 @@ public OdpSegmentApiManager(ILogger logger = null, IErrorHandler errorHandler = /// /// ODP public key /// Host of ODP endpoint - /// 'vuid' or 'fs_user_id key' + /// Either `vuid` or `fs_user_id key` /// Associated value to query for the user key /// Audience segments to check for experiment inclusion /// Array of audience segments @@ -95,21 +94,21 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user return new string[0]; } - var endpoint = $"{apiHost}/v3/graphql"; + var endpoint = $"{apiHost}{Constants.ODP_GRAPHQL_API_ENDPOINT_PATH}"; var query = BuildGetSegmentsGraphQLQuery(userKey.ToString().ToLower(), userValue, segmentsToCheck); var segmentsResponseJson = QuerySegments(apiKey, endpoint, query); - if (CanBeJsonParsed(segmentsResponseJson)) + if (string.IsNullOrWhiteSpace(segmentsResponseJson)) { _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (network error)"); return null; } - var parsedSegments = ParseSegmentsResponseJson(segmentsResponseJson); - if (parsedSegments is null) + var parsedSegments = DeserializeSegmentsFromJson(segmentsResponseJson); + if (parsedSegments == null) { var message = $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"; _logger.Log(LogLevel.ERROR, message); @@ -132,15 +131,16 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user return null; } - return parsedSegments.Data.Customer.Audiences.Edges. - Where(e => e.Node.State == BaseCondition.QUALIFIED). - Select(e => e.Node.Name).ToArray(); + return parsedSegments.Data.Customer.Audiences.Edges + .Where(e => e.Node.State == BaseCondition.QUALIFIED) + .Select(e => e.Node.Name) + .ToArray(); } /// /// Build GraphQL query for getting segments /// - /// 'vuid' or 'fs_user_id key' + /// Either `vuid` or `fs_user_id` key /// Associated value to query for the user key /// Audience segments to check for experiment inclusion /// GraphQL string payload @@ -148,19 +148,34 @@ private static string BuildGetSegmentsGraphQLQuery(string userKey, string userVa IEnumerable segmentsToCheck ) { - var query = - "query($userId: String, $audiences: [String]) {customer(|userKey|: $userId){audiences(subset: $audiences) {edges {node {name state}}}}}". - Replace("|userKey|", userKey); - - var variables = - "\"variables\": { \"userId\": \"|userValue|\", \"audiences\": |audiences| }". - Replace("|userValue|", userValue). - Replace("|audiences|", JsonConvert.SerializeObject(segmentsToCheck)); - - return $"{{ \"query\": \"{query}\", {variables} }}"; + return + @"{ + ""query"": ""{ + query($userId: String, $audiences: [String]) { + { + customer({userKey}: $userId) { + audiences(subset: $audiences) { + edges { + node { + name + state + } + } + } + } + } + } + }"", + ""variables"" : { + ""userId"": ""{userValue}"", + ""audiences"": {audiences} + } + }" + .Replace("{userKey}", userKey) + .Replace("{userValue}", userValue) + .Replace("{audiences}", JsonConvert.SerializeObject(segmentsToCheck)); } - /// /// Synchronous handler for querying the ODP GraphQL endpoint /// @@ -168,26 +183,33 @@ IEnumerable segmentsToCheck /// Fully-qualified ODP GraphQL Endpoint /// GraphQL query string to send /// JSON response from ODP - private string QuerySegments(string apiKey, string endpoint, - string query - ) + private string QuerySegments(string apiKey, string endpoint, string query) { - HttpResponseMessage response; + HttpResponseMessage response = null; try { response = QuerySegmentsAsync(apiKey, endpoint, query).GetAwaiter().GetResult(); + response.EnsureSuccessStatusCode(); } - catch (Exception ex) + catch (HttpRequestException ex) { _errorHandler.HandleError(ex); - _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (network error)"); + + var statusCode = response == null ? 0 : (int)response.StatusCode; + + + _logger.Log(LogLevel.ERROR, + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({(statusCode == 0 ? Constants.NETWORK_ERROR_REASON : statusCode.ToString())})"); + return default; } - - if (!response.IsSuccessStatusCode) + catch (Exception ex) { + _errorHandler.HandleError(ex); + _logger.Log(LogLevel.ERROR, - $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({(int)response.StatusCode})"); + $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({Constants.NETWORK_ERROR_REASON})"); + return default; } @@ -231,28 +253,19 @@ string query Constants.HEADER_API_KEY, apiKey }, }, - Content = new StringContent(query, Encoding.UTF8, "application/json"), + Content = new StringContent(query, Encoding.UTF8, + Constants.APPLICATION_JSON_MEDIA_TYPE), }; return request; } - /// - /// Ensure a string has content that can be parsed from JSON to an object - /// - /// Value containing possible JSON - /// True if content could be interpreted as JSON else False - private static bool CanBeJsonParsed(string jsonToValidate) - { - return string.IsNullOrWhiteSpace(jsonToValidate); - } - /// /// Parses JSON response /// /// JSON response from ODP /// Strongly-typed ODP Response object - public static Response ParseSegmentsResponseJson(string jsonResponse) + public static Response DeserializeSegmentsFromJson(string jsonResponse) { return JsonConvert.DeserializeObject(jsonResponse); } From ea7c2ce6eddf51422dc1658c847611c3301c9374 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 25 Oct 2022 11:21:13 -0400 Subject: [PATCH 14/14] More Pull Request code changes --- .../OdpTests/OdpSegmentApiManagerTest.cs | 16 ++++++++++++++ OptimizelySDK/Odp/OdpSegmentApiManager.cs | 21 ++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs index f0aa9c16..9a08f27b 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentApiManagerTest.cs @@ -91,6 +91,22 @@ public void ShouldParseSuccessfulResponse() Assert.AreEqual(node.Name, "has_email_opted_in"); Assert.AreNotEqual(node.State, BaseCondition.QUALIFIED); } + + [Test] + public void ShouldHandleAttemptToDeserializeInvalidJsonResponse() + { + const string VALID_ARRAY_JSON = "[\"item-1\", \"item-2\", \"item-3\"]"; + const string KEY_WITHOUT_VALUE = "{ \"keyWithoutValue\": }"; + const string VALUE_WITHOUT_KEY = "{ : \"valueWithoutKey\" }"; + const string STRING_ONLY = "\"just some text\""; + const string MISSING_BRACE = "{ \"goodKeyWith\": \"goodValueButMissingBraceHere\" "; + + Assert.IsNull(OdpSegmentApiManager.DeserializeSegmentsFromJson(VALID_ARRAY_JSON)); + Assert.IsNull(OdpSegmentApiManager.DeserializeSegmentsFromJson(KEY_WITHOUT_VALUE)); + Assert.IsNull(OdpSegmentApiManager.DeserializeSegmentsFromJson(VALUE_WITHOUT_KEY)); + Assert.IsNull(OdpSegmentApiManager.DeserializeSegmentsFromJson(STRING_ONLY)); + Assert.IsNull(OdpSegmentApiManager.DeserializeSegmentsFromJson(MISSING_BRACE)); + } [Test] public void ShouldParseErrorResponse() diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index 9a8dcf7f..36f2c20b 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -107,31 +107,31 @@ public string[] FetchSegments(string apiKey, string apiHost, OdpUserKeyType user return null; } - var parsedSegments = DeserializeSegmentsFromJson(segmentsResponseJson); - if (parsedSegments == null) + var segments = DeserializeSegmentsFromJson(segmentsResponseJson); + if (segments == null) { var message = $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"; _logger.Log(LogLevel.ERROR, message); return null; } - if (parsedSegments.HasErrors) + if (segments.HasErrors) { - var errors = string.Join(";", parsedSegments.Errors.Select(e => e.ToString())); + var errors = string.Join(";", segments.Errors.Select(e => e.ToString())); _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({errors})"); return null; } - if (parsedSegments.Data?.Customer?.Audiences?.Edges == null) + if (segments.Data?.Customer?.Audiences?.Edges == null) { _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} (decode error)"); return null; } - return parsedSegments.Data.Customer.Audiences.Edges + return segments.Data.Customer.Audiences.Edges .Where(e => e.Node.State == BaseCondition.QUALIFIED) .Select(e => e.Node.Name) .ToArray(); @@ -267,7 +267,14 @@ string query /// Strongly-typed ODP Response object public static Response DeserializeSegmentsFromJson(string jsonResponse) { - return JsonConvert.DeserializeObject(jsonResponse); + try + { + return JsonConvert.DeserializeObject(jsonResponse); + } + catch (Exception ex) + { + return default; + } } } }