fix: Removed filtering of unknown attributes and assignment of empty map to null attributes and null event tags. #221
Conversation
Pull Request Test Coverage Report for Build 695
💛 - Coveralls |
There was a problem hiding this comment.
Why didn’t you just keep filters attributes method possibly rename to validate attributes and have it return a empty attribute or a copy of the attribute list passed in. A null in notification listeners attributes is definitely a breaking change. But, is it necessary?
It seems like making a copy at least could be valuable.
Just thought maybe we could/should discuss.
| (entry.getValue() instanceof Double) || | ||
| (entry.getValue() instanceof Boolean))) { | ||
| continue; | ||
| if (attributes != null) { |
There was a problem hiding this comment.
Rather than a large enclosed if != how about if == null continue; I think it reads better.
There was a problem hiding this comment.
Tom there is no for loop here and if attributes is not null then we have to build attribute list (for loop is inside if) from attributes and later on, we are also building attribute for bot filtering.
|
@thomaszurkan-optimizely We can discuss offline tomorrow, this is the part of audience match type, what is sent to attributes from APIs must be passed same to notification listener. @mikeng13 Tom is right, notification listener has nonnull attributes property which will not let the user to pass NULL, but if we by pass annotation validation, then it will cause problem. |
in UserAttribute.evaluate if attributes are null than assigns it into empty map. to avoid null pointer exception
…nt tag if passed null
|
build |
2 similar comments
|
build |
|
build |
# Conflicts: # core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java # core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java
…ck' into mnoman/MakeCopyOfAttributes
Note: first we have to merge #219 PR