-
Notifications
You must be signed in to change notification settings - Fork 31
(feat): Feature Management getFeatureVariable Listener #270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
e6f6dd2
Added Listener in getFeatureVariable
mnoman09 11e1d01
fixed findBugs Issue
mnoman09 708c6fe
changed OnDecision enum to Decision
mnoman09 c6bf1d8
Sending converted value to from string to object in listener decision…
mnoman09 4aa8c44
sending empty map in case when null is passed in attributes listener
mnoman09 017b0d2
did naming changes
mnoman09 3c20d8e
Merge branch 'master' into mnoman/getFeatVarListener
mfahadahmed ff5b878
Nit fixes and checking if listener is getting called
mnoman09 4fa7a37
Merge branch 'mnoman/getFeatVarListener' of github.com:optimizely/jav…
mnoman09 3dde5da
Updated getFeatureVariableValueForType to return casted object so tha…
mnoman09 158bee4
Refact: Added builder pattern in decision listener (#275)
mnoman09 9486ab1
Merge branch 'master' into mnoman/getFeatVarListener
mnoman09 52cd72a
Fixed Find bugs errors
mnoman09 21f24a2
Merge branch 'master' into mnoman/getFeatVarListener
aliabbasrizvi d728c85
fixed testcase
NomanShoaib c31522d
Merge branch 'master' into mnoman/getFeatVarListener
mnoman09 fc19aff
Merge branch 'master' into mnoman/getFeatVarListener
mnoman09 c34c3e7
Conflicts with master resolved
mnoman09 8a8b6eb
Nit fixes and remove extra check
mnoman09 21c8eb7
Feat: Updated getFeatVariable notification listener implementation a…
mnoman09 2b797fb
Updated naming convension of DecisionNotification keys
mnoman09 b070cc7
Merge branch 'master' into mnoman/getFeatVarListener
aliabbasrizvi 2eee571
merged master and resolved conflicts
mnoman09 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -465,17 +465,13 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, | |
| return null; | ||
| } | ||
|
|
||
| String variableValue = getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.BOOLEAN | ||
| ); | ||
| if (variableValue != null) { | ||
| return Boolean.parseBoolean(variableValue); | ||
| } | ||
| return null; | ||
| return getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.BOOLEAN | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -514,20 +510,19 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, | |
| return null; | ||
| } | ||
|
|
||
| String variableValue = getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.DOUBLE | ||
| ); | ||
| if (variableValue != null) { | ||
| try { | ||
| return Double.parseDouble(variableValue); | ||
| } catch (NumberFormatException exception) { | ||
| logger.error("NumberFormatException while trying to parse \"" + variableValue + | ||
| "\" as Double. " + exception); | ||
| } | ||
| Double variableValue = null; | ||
| try { | ||
| variableValue = getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.DOUBLE | ||
| ); | ||
| return variableValue; | ||
| } catch (Exception exception) { | ||
| logger.error("NumberFormatException while trying to parse \"" + variableValue + | ||
| "\" as Double. " + exception); | ||
| } | ||
| return null; | ||
| } | ||
|
|
@@ -567,21 +562,19 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, | |
| logger.error("Optimizely instance is not valid, failing getFeatureVariableInteger call."); | ||
| return null; | ||
| } | ||
|
|
||
| String variableValue = getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.INTEGER | ||
| ); | ||
| if (variableValue != null) { | ||
| try { | ||
| return Integer.parseInt(variableValue); | ||
| } catch (NumberFormatException exception) { | ||
| logger.error("NumberFormatException while trying to parse \"" + variableValue + | ||
| "\" as Integer. " + exception.toString()); | ||
| try { | ||
| Integer variableValue = getFeatureVariableValueForType( | ||
| featureKey, | ||
| variableKey, | ||
| userId, | ||
| attributes, | ||
| FeatureVariable.VariableType.INTEGER | ||
| ); | ||
| if (variableValue != null) { | ||
| return variableValue; | ||
| } | ||
| } catch (Exception exception) { | ||
| logger.error("NumberFormatException while trying to parse value as Integer. " + exception.toString()); | ||
| } | ||
| return null; | ||
| } | ||
|
|
@@ -631,11 +624,11 @@ public String getFeatureVariableString(@Nonnull String featureKey, | |
| } | ||
|
|
||
| @VisibleForTesting | ||
| String getFeatureVariableValueForType(@Nonnull String featureKey, | ||
| @Nonnull String variableKey, | ||
| @Nonnull String userId, | ||
| @Nonnull Map<String, ?> attributes, | ||
| @Nonnull FeatureVariable.VariableType variableType) { | ||
| <T extends Object> T getFeatureVariableValueForType(@Nonnull String featureKey, | ||
| @Nonnull String variableKey, | ||
| @Nonnull String userId, | ||
| @Nonnull Map<String, ?> attributes, | ||
| @Nonnull FeatureVariable.VariableType variableType) { | ||
| if (featureKey == null) { | ||
| logger.warn("The featureKey parameter must be nonnull."); | ||
| return null; | ||
|
|
@@ -668,6 +661,7 @@ String getFeatureVariableValueForType(@Nonnull String featureKey, | |
| String variableValue = variable.getDefaultValue(); | ||
| Map<String, ?> copiedAttributes = copyAttributes(attributes); | ||
| FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes); | ||
| Boolean featureEnabled = false; | ||
| if (featureDecision.variation != null) { | ||
| if (featureDecision.variation.getFeatureEnabled()) { | ||
| FeatureVariableUsageInstance featureVariableUsageInstance = | ||
|
|
@@ -683,14 +677,61 @@ String getFeatureVariableValueForType(@Nonnull String featureKey, | |
| featureKey, featureDecision.variation.getKey(), variableValue, variableKey | ||
| ); | ||
| } | ||
| featureEnabled = featureDecision.variation.getFeatureEnabled(); | ||
| } else { | ||
| logger.info("User \"{}\" was not bucketed into any variation for feature flag \"{}\". " + | ||
| "The default value \"{}\" for \"{}\" is being returned.", | ||
| userId, featureKey, variableValue, variableKey | ||
| ); | ||
| } | ||
|
|
||
| return variableValue; | ||
| Object convertedValue = convertStringToType(variableValue, variableType); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This conversion is now happening twice per api call. Once here and once in the originating method. |
||
|
|
||
| DecisionNotification decisionNotification = DecisionNotification.newFeatureVariableBuilder() | ||
| .withUserId(userId) | ||
| .withAttributes(copiedAttributes) | ||
| .withFeatureKey(featureKey) | ||
| .withFeatureEnabled(featureEnabled) | ||
| .withVariableKey(variableKey) | ||
| .withVariableType(variableType) | ||
| .withVariableValue(convertedValue) | ||
| .withFeatureDecision(featureDecision) | ||
| .build(); | ||
|
|
||
|
|
||
| notificationCenter.sendNotifications(decisionNotification); | ||
|
|
||
| return (T) convertedValue; | ||
| } | ||
|
|
||
| // Helper method which takes type and variable value and convert it to object to use in Listener DecisionInfo object variable value | ||
| @VisibleForTesting | ||
| Object convertStringToType(String variableValue, FeatureVariable.VariableType type) { | ||
| if (variableValue != null) { | ||
| switch (type) { | ||
| case DOUBLE: | ||
| try { | ||
| return Double.parseDouble(variableValue); | ||
| } catch (NumberFormatException exception) { | ||
| logger.error("NumberFormatException while trying to parse \"" + variableValue + | ||
| "\" as Double. " + exception); | ||
| } | ||
| break; | ||
| case STRING: | ||
| return variableValue; | ||
| case BOOLEAN: | ||
| return Boolean.parseBoolean(variableValue); | ||
| case INTEGER: | ||
| try { | ||
| return Integer.parseInt(variableValue); | ||
| } catch (NumberFormatException exception) { | ||
| logger.error("NumberFormatException while trying to parse \"" + variableValue + | ||
| "\" as Integer. " + exception.toString()); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| return 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
Oops, something went wrong.
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.
@mnoman09 this and a other log statements in this PR append the exception as a string on log message. the logger should be passed the exception object as a last argument