Merge 6a418d2 into 5bb3aa8

lib/unleash/constraint.rb

@@ -18,15 +18,20 @@ class Constraint
       DATE_BEFORE: ->(context_v, constraint_v){ on_valid_date(constraint_v, context_v){ |x, y| (x > y) } },
       SEMVER_EQ: ->(context_v, constraint_v){ on_valid_version(constraint_v, context_v){ |x, y| (x == y) } },
       SEMVER_GT: ->(context_v, constraint_v){ on_valid_version(constraint_v, context_v){ |x, y| (x < y) } },
-      SEMVER_LT: ->(context_v, constraint_v){ on_valid_version(constraint_v, context_v){ |x, y| (x > y) } }
+      SEMVER_LT: ->(context_v, constraint_v){ on_valid_version(constraint_v, context_v){ |x, y| (x > y) } },
+      FALLBACK_VALIDATOR: ->(_context_v, _constraint_v){ false }
     }.freeze
 
     LIST_OPERATORS = [:IN, :NOT_IN, :STR_STARTS_WITH, :STR_ENDS_WITH, :STR_CONTAINS].freeze
 
     def initialize(context_name, operator, value = [], inverted: false, case_insensitive: false)
       raise ArgumentError, "context_name is not a String" unless context_name.is_a?(String)
-      raise ArgumentError, "operator does not hold a valid value:" + OPERATORS.keys unless OPERATORS.include? operator.to_sym
 
+      unless OPERATORS.include? operator.to_sym
+        Unleash.logger.warn "Operator #{operator} is not a supported operator, \
+          falling back to FALLBACK_VALIDATOR which skips this constraint"
+        operator = "FALLBACK_VALIDATOR"
+      end
       self.validate_constraint_value_type(operator.to_sym, value)
 
       self.context_name = context_name
@@ -44,6 +49,10 @@ def matches_context?(context)
     rescue KeyError
       Unleash.logger.warn "Attemped to resolve a context key during constraint resolution: #{self.context_name} but it wasn't \
       found on the context"
+
+      # when the operator is NOT_IN and there is no data, return true. In all other cases the operator doesn't match.
+      return true if operator == :NOT_IN
+
       false
     end
 
@@ -79,8 +88,8 @@ def self.on_valid_version(val1, val2)
 
     # This should be a private method but for some reason this fails on Ruby 2.5
     def validate_constraint_value_type(operator, value)
-      raise ArgumentError, "context_name is not an Array" if LIST_OPERATORS.include?(operator) && value.is_a?(String)
-      raise ArgumentError, "context_name is not a String" if !LIST_OPERATORS.include?(operator) && value.is_a?(Array)
+      Unleash.logger.warn "value is a String, operator is expecting an Array" if LIST_OPERATORS.include?(operator) && value.is_a?(String)
+      Unleash.logger.warn "value is an Array, operator is expecting a String" if !LIST_OPERATORS.include?(operator) && value.is_a?(Array)
     end
 
     private

spec/unleash/constraint_spec.rb

@@ -20,12 +20,6 @@
 
       constraint = Unleash::Constraint.new('env', 'IN', ['dev', 'pre'])
       expect(constraint.matches_context?(context)).to be true
-
-      constraint = Unleash::Constraint.new('env', 'NOT_IN', ['dev', 'pre'])
-      expect(constraint.matches_context?(context)).to be false
-
-      constraint = Unleash::Constraint.new('env', 'NOT_IN', ['pre', 'prod'])
-      expect(constraint.matches_context?(context)).to be true
     end
 
     it 'matches based on property NOT_IN value' do
@@ -48,6 +42,16 @@
       expect(constraint.matches_context?(context)).to be true
     end
 
+    it 'matches based on a value NOT_IN in a not existing context field' do
+      context_params = {
+        properties: {
+        }
+      }
+      context = Unleash::Context.new(context_params)
+      constraint = Unleash::Constraint.new('env', 'NOT_IN', ['anything'])
+      expect(constraint.matches_context?(context)).to be true
+    end
+
     it 'matches based on user_id IN/NOT_IN user_id' do
       context_params = {
         user_id: '123',
@@ -402,23 +406,38 @@
       expect(constraint.matches_context?(context)).to be false
     end
 
-    it 'rejects constraint construction for invalid value types for operator' do
+    it 'gracefully handles invalid constraint operators' do
+      context_params = {
+        user_id: '123',
+        session_id: 'verylongsesssionid',
+        remote_address: '127.0.0.1',
+        properties: {
+          env: 'development'
+        }
+      }
+      context = Unleash::Context.new(context_params)
+      constraint = Unleash::Constraint.new('env', 'NOT_A_VALID_OPERATOR', 'dev', inverted: true)
+      expect(constraint.matches_context?(context)).to be true
+
+      constraint = Unleash::Constraint.new('env', 'NOT_A_VALID_OPERATOR', ['dev'], inverted: true)
+      expect(constraint.matches_context?(context)).to be true
+    end
+
+    it 'warns about constraint construction for invalid value types for operator' do
       array_constraints = ['STR_CONTAINS', 'STR_ENDS_WITH', 'STR_STARTS_WITH', 'IN', 'NOT_IN']
 
       array_constraints.each do |operator_name|
         Unleash::Constraint.new('env', operator_name, [])
-        expect do
-          Unleash::Constraint.new('env', operator_name, '')
-        end.to raise_error
+        expect(Unleash.logger).to receive(:warn).with("value is a String, operator is expecting an Array")
+        Unleash::Constraint.new('env', operator_name, '')
       end
 
       string_constraints = ['NUM_EQ', 'NUM_GT', 'NUM_GTE', 'NUM_LT', 'NUM_LTE',
                             'DATE_AFTER', 'DATE_BEFORE', 'SEMVER_EQ', 'SEMVER_GT', 'SEMVER_LT']
       string_constraints.each do |operator_name|
         Unleash::Constraint.new('env', operator_name, '')
-        expect do
-          Unleash::Constraint.new('env', operator_name, [])
-        end.to raise_error
+        expect(Unleash.logger).to receive(:warn).with("value is an Array, operator is expecting a String")
+        Unleash::Constraint.new('env', operator_name, [])
       end
     end
   end