Merge 9e43d83 into 5bb3aa8

lib/unleash/constraint.rb

@@ -18,16 +18,21 @@ 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
 
-      self.validate_constraint_value_type(operator.to_sym, value)
+      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.log_inconsistent_constraint_configuration(operator.to_sym, value)
 
       self.context_name = context_name
       self.operator = operator.to_sym
@@ -37,8 +42,6 @@ def initialize(context_name, operator, value = [], inverted: false, case_insensi
     end
 
     def matches_context?(context)
-      Unleash.logger.debug "Unleash::Constraint matches_context? value: #{self.value} context.get_by_name(#{self.context_name})" \
-        " #{context.get_by_name(self.context_name)} "
       match = matches_constraint?(context)
       self.inverted ? !match : match
     rescue KeyError
@@ -78,9 +81,9 @@ def self.on_valid_version(val1, val2)
     end
 
     # 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)
+    def log_inconsistent_constraint_configuration(operator, value)
+      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
@@ -91,6 +94,12 @@ def matches_constraint?(context)
         false
       end
 
+      # when the operator is NOT_IN and there is no data, return true. In all other cases the operator doesn't match.
+      return self.operator == :NOT_IN unless context.has_property(self.context_name)
+
+      Unleash.logger.debug "Unleash::Constraint matches_context? value: #{self.value} context.get_by_name(#{self.context_name})" \
+      " #{context.get_by_name(self.context_name)} "
+
       v = self.value.dup
       context_value = context.get_by_name(self.context_name)
 

lib/unleash/context.rb

@@ -33,6 +33,14 @@ def get_by_name(name)
       end
     end
 
+    def has_property(name)
+      normalized_name = underscore(name)
+      if ATTRS.include? normalized_name.to_sym
+        return self.instance_variable_defined? "@#{normalized_name}"
+      end
+      return self.properties.include?(normalized_name.to_sym) || self.properties.include?(name.to_sym)
+    end
+
     private
 
     # Method to fetch values from hash for two types of keys: string in camelCase and symbol in snake_case

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

spec/unleash/context_spec.rb

@@ -134,6 +134,25 @@
     expect(context.get_by_name(:CountryCode)).to eq('UK')
   end
 
+  it "checks if property is found" do
+    params = {
+      'user_id' => '123',
+      'sessionId' => 'verylongsesssionid',
+      'properties' => {
+        'fancy' => 'polarbear',
+        'country_code' => 'UK'
+      }
+    }
+    context = Unleash::Context.new(params)
+    expect(context.has_property(:user_id)).to be true
+    expect(context.has_property(:user_name)).to be false
+    expect(context.has_property(:session_id)).to be true
+    expect(context.has_property(:fancy)).to be true
+    expect(context.has_property(:country_code)).to be true
+    expect(context.has_property(:countryCode)).to be true
+    expect(context.has_property(:CountryCode)).to be true
+  end
+
   it "creates default date for current time if not populated" do
     params = {}
     context = Unleash::Context.new(params)