[FSSDK-9533] Log error & reject on track event with null, empty or whitespace event key #362
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -35,6 +35,7 @@ | |
| using OptimizelySDK.OptlyConfig; | ||
| using OptimizelySDK.OptimizelyDecisions; | ||
| using System.Linq; | ||
| using System.Text.RegularExpressions; | ||
| | ||
| #if USE_ODP | ||
| using OptimizelySDK.Odp; | ||
| | @@ -87,7 +88,7 @@ public static String SDK_VERSION | |
| { | ||
| get | ||
| { | ||
| // Example output: "2.1.0". Should be kept in sync with NuGet package version. | ||
| #if NET35 || NET40 | ||
| var assembly = Assembly.GetExecutingAssembly(); | ||
| #else | ||
| | @@ -338,54 +339,60 @@ private bool ValidateInputs(string datafile, bool skipJsonValidation) | |
| /// <summary> | ||
| /// Sends conversion event to Optimizely. | ||
| /// </summary> | ||
| /// <param name="eventKey">Event key representing the event (must not be null, empty, or whitespace)</param> | ||
| /// <param name="userId">ID for user</param> | ||
| /// <param name="userAttributes">Attributes of the user</param> | ||
| /// <param name="eventTags">eventTags array Hash representing metadata associated with the event.</param> | ||
| public void Track(string eventKey, string userId, UserAttributes userAttributes = null, | ||
| EventTags eventTags = null | ||
| ) | ||
| { | ||
| Comment on lines -349 to -351 Contributor There was a problem hiding this comment. can we remove this config ready check? Contributor Author There was a problem hiding this comment. If we're going to call | ||
| if (eventKey == null || Regex.IsMatch(eventKey, @"^\s*$")) | ||
| Contributor Author There was a problem hiding this comment. I added an earlier eventKey check since it's cheaper before digging into the ProjectConfig to see if the eventKey is present in the datafile since it's required per the docs. | ||
| { | ||
| Logger.Log(LogLevel.ERROR, | ||
| "Event key cannot be null, empty, or whitespace string. Failing 'Track'."); | ||
| return; | ||
| } | ||
| | ||
| var inputValues = new Dictionary<string, string> | ||
| { | ||
| { USER_ID, userId }, | ||
| { EVENT_KEY, eventKey }, | ||
| }; | ||
| | ||
| if (!ValidateStringInputs(inputValues)) | ||
| { | ||
| return; | ||
| } | ||
| | ||
| Comment on lines -365 to -367 Contributor There was a problem hiding this comment. I think these 2 lines will filter out null or empty eventKey cases. Contributor Author There was a problem hiding this comment. I think I see what you're talking about: The eventKey must be in the datafile before it can be tracked anyway, regardless of whether it's not null, empty, or whitespace. | ||
| var config = ProjectConfigManager?.GetConfig(); | ||
| if (config == null) | ||
| { | ||
| Logger.Log(LogLevel.ERROR, "Datafile has invalid format. Failing 'Track'."); | ||
| return; | ||
| } | ||
| | ||
| var eventToTrack = config.GetEvent(eventKey); | ||
| if (eventToTrack.Key == null) | ||
| { | ||
| Logger.Log(LogLevel.INFO, $"Not tracking user {userId} for event {eventKey}."); | ||
| return; | ||
| } | ||
| | ||
| eventTags = eventTags?.FilterNullValues(Logger); | ||
| | ||
| var userEvent = UserEventFactory.CreateConversionEvent(config, eventKey, userId, | ||
| userAttributes, eventTags); | ||
| | ||
| EventProcessor.Process(userEvent); | ||
| | ||
| Logger.Log(LogLevel.INFO, $"Tracking event {eventKey} for user {userId}."); | ||
| | ||
| if (NotificationCenter.GetNotificationCount(NotificationCenter.NotificationType.Track) > | ||
| 0) | ||
| { | ||
| var conversionEvent = EventFactory.CreateLogEvent(userEvent, Logger); | ||
| | ||
| NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Track, | ||
| eventKey, userId, | ||
| userAttributes, eventTags, conversionEvent); | ||
| | @@ -1347,7 +1354,8 @@ List<OdpSegmentOption> segmentOptions | |
| | ||
| if (config == null) | ||
| { | ||
| Logger.Log(LogLevel.ERROR, | ||
| "Datafile has invalid format. Failing 'FetchQualifiedSegments'."); | ||
| return null; | ||
| } | ||
| | ||
| | @@ -1378,7 +1386,8 @@ internal void IdentifyUser(string userId) | |
| /// <param name="identifiers">Dictionary for identifiers. The caller must provide at least one key-value pair.</param> | ||
| /// <param name="type">Type of event (defaults to `fullstack`)</param> | ||
| /// <param name="data">Optional event data in a key-value pair format</param> | ||
| public void SendOdpEvent(string action, Dictionary<string, string> identifiers, string type | ||
| = Constants.ODP_EVENT_TYPE, | ||
| Dictionary<string, object> data = null | ||
| ) | ||
| { | ||
| | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this should read: "Event key representing the event in the datafile which must not be null, empty, or whitespace"