Update Constraint#matches_constraint? #157
Conversation
Pull Request Test Coverage Report for Build 6563818978
💛 - Coveralls |
| v.map!(&:upcase) if self.case_insensitive | ||
| context_value.upcase! if self.case_insensitive | ||
| context_value = context_value.upcase if self.case_insensitive | ||
|
|
||
| OPERATORS[self.operator].call(context_value, v) |
There was a problem hiding this comment.
would have been a prettier solution:
| v.map!(&:upcase) if self.case_insensitive | |
| context_value.upcase! if self.case_insensitive | |
| context_value = context_value.upcase if self.case_insensitive | |
| OPERATORS[self.operator].call(context_value, v) | |
| OPERATORS[self.operator].call( | |
| context_value.then{|c| self.case_insensitive ? c.upcase : c} , | |
| v.then{|v| self.case_insensitive ? v.map(&:upcase) : v} | |
| ) |
There was a problem hiding this comment.
the same issue will reappear if the values in v are frozen.
Additionally would have been nice with a unit test that triggers the bug previously, and wouldn't trigger now, so to prevent possible regressions.
There was a problem hiding this comment.
note that using ! methods in general is to be avoided, and specially on user provided data is dangerous, as you shouldn't be altering data provided / referenced by the user. (if he is to use again later on, it will be changed by this method in the SDK).
There was a problem hiding this comment.
the same issue will reappear if the values in v are frozen.
Unless you're doing something super weird, the values in v should only come from internal code. If that starts to fail then we have bigger problems
About the changes
unleash-client-ruby/lib/unleash/constraint.rb
Line 110 in e935495
# frozen-string-literal: trueis used, so calling#upcase!throwsFrozenErrorCloses #156