feat: match array properties element-wise for non-set operators

lib/experiment/evaluation/evaluation.rb

@@ -66,24 +66,25 @@ def evaluate_conditions(target, conditions)
 
       def match_condition(target, condition)
         prop_value = Evaluation.select(target, condition.selector)
-        # Special matching for null properties and set type prop values and operators
-        if !prop_value
-          match_null(condition.op, condition.values)
-        elsif set_operator?(condition.op)
-          prop_value_string_list = coerce_string_array(prop_value)
+        return match_null(condition.op, condition.values) unless prop_value
+
+        prop_value_string_list = coerce_string_array(prop_value)
+        if set_operator?(condition.op)
           return false unless prop_value_string_list
 
           match_set(prop_value_string_list, condition.op, condition.values)
+        elsif prop_value_string_list
+          match_strings_non_set(prop_value_string_list, condition.op, condition.values)
         else
           prop_value_string = coerce_string(prop_value)
-          if prop_value_string
-            match_string(prop_value_string, condition.op, condition.values)
-          else
-            false
-          end
+          prop_value_string ? match_string(prop_value_string, condition.op, condition.values) : false
         end
       end
 
+      def match_strings_non_set(prop_values, op, filter_values)
+        prop_values.any? { |v| match_string(v, op, filter_values) }
+      end
+
       def get_hash(key)
         Murmur3.hash32x86(key)
       end
@@ -263,23 +264,19 @@ def coerce_string(value)
       end
 
       def coerce_string_array(value)
-        if value.is_a?(Array)
-          value.map { |e| coerce_string(e) }.compact
-        else
-          string_value = value.to_s
-          begin
-            parsed_value = JSON.parse(string_value)
-            if parsed_value.is_a?(Array)
-              parsed_value.map { |e| coerce_string(e) }.compact
-            else
-              s = coerce_string(string_value)
-              s ? [s] : nil
-            end
-          rescue JSON::ParserError
-            s = coerce_string(string_value)
-            s ? [s] : nil
-          end
+        return value.map { |e| coerce_string(e) }.compact if value.is_a?(Array)
+
+        string_value = value.to_s
+        return nil unless string_value.start_with?('[')
+
+        begin
+          parsed_value = JSON.parse(string_value)
+        rescue JSON::ParserError
+          return nil
         end
+        return nil unless parsed_value.is_a?(Array)
+
+        parsed_value.map { |e| coerce_string(e) }.compact
       end
 
       def set_operator?(op)

spec/experiment/evaluation/evaluation_intgration_spec.rb

@@ -4,8 +4,12 @@
 require 'json'
 
 module AmplitudeExperiment
+  # Cached once to work around a JRuby 10.x issue where repeated URI() parsing
+  # under sustained string allocation spuriously raises "URI must be ascii only".
+  FLAGS_URI = URI('https://api.lab.amplitude.com/sdk/v2/flags?eval_mode=remote').freeze
+
   describe Evaluation::Engine do
-    let(:deployment_key) { 'server-NgJxxvg8OGwwBsWVXqyxQbdiflbhvugy' }
+    let(:deployment_key) { 'server-VVhLULXCxxY0xqmszXouXxiEzoeJWmSh' }
     let(:engine) { Evaluation::Engine.new }
     let(:flags) { get_flags(deployment_key) }
 
@@ -421,6 +425,48 @@ module AmplitudeExperiment
         expect(result.key).to eq('on')
       end
 
+      it 'tests set is with json array string' do
+        user = user_context(nil, nil, nil, { 'key' => '["1", "2", "3"]' })
+        result = engine.evaluate(user, flags)['test-set-is']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests is with array' do
+        user = user_context(nil, nil, nil, { 'key' => %w[value1 value2] })
+        result = engine.evaluate(user, flags)['test-is-array']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests is with json array string' do
+        user = user_context(nil, nil, nil, { 'key' => '["value1", "value2"]' })
+        result = engine.evaluate(user, flags)['test-is-array']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests is not with array' do
+        user = user_context(nil, nil, nil, { 'key' => %w[value3 value4] })
+        result = engine.evaluate(user, flags)['test-is-not-array']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests contains with array' do
+        user = user_context(nil, nil, nil, { 'key' => %w[has-target-value has value] })
+        result = engine.evaluate(user, flags)['test-contains-array']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests does not contain with array' do
+        user = user_context(nil, nil, nil, { 'key' => %w[has-value has value] })
+        result = engine.evaluate(user, flags)['test-does-not-contain-array']
+        expect(result.key).to eq('on')
+      end
+
+      it 'tests does not contain with json array string' do
+        user = user_context(nil, nil, nil, { 'key' => '["has-value", "has", "value"]' })
+        result = engine.evaluate(user, flags)['test-does-not-contain-array']
+        expect(result.key).to eq('on')
+      end
+
       it 'tests is with booleans' do
         # Test with uppercase TRUE/FALSE
         user = user_context(nil, nil, nil, {
@@ -479,13 +525,10 @@ def group_context(group_type, group_name, group_properties = nil)
     end
 
     def get_flags(deployment_key)
-      server_url = 'https://api.lab.amplitude.com'
-      uri = URI("#{server_url}/sdk/v2/flags?eval_mode=remote")
-
-      request = Net::HTTP::Get.new(uri)
+      request = Net::HTTP::Get.new(FLAGS_URI)
       request['Authorization'] = "Api-Key #{deployment_key}"
 
-      response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) do |http|
+      response = Net::HTTP.start(FLAGS_URI.hostname, FLAGS_URI.port, use_ssl: true) do |http|
         http.request(request)
       end
 

spec/experiment/evaluation/evaluation_spec.rb

@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+module AmplitudeExperiment
+  describe Evaluation::Engine do
+    let(:engine) { Evaluation::Engine.new }
+
+    def flag_with_condition(op, values)
+      Evaluation::Flag.from_hash(
+        'key' => 'test-flag',
+        'variants' => {
+          'on' => { 'key' => 'on', 'value' => 'on' }
+        },
+        'segments' => [
+          {
+            'conditions' => [[
+              {
+                'selector' => %w[context user user_properties test_prop],
+                'op' => op,
+                'values' => values
+              }
+            ]],
+            'variant' => 'on'
+          }
+        ]
+      )
+    end
+
+    def context_with_prop(value)
+      {
+        'user' => {
+          'user_properties' => { 'test_prop' => value }
+        }
+      }
+    end
+
+    def evaluate(prop_value, op, values)
+      flag = flag_with_condition(op, values)
+      context = context_with_prop(prop_value)
+      engine.evaluate(context, [flag])['test-flag']
+    end
+
+    def assert_match(prop_value, op, values)
+      result = evaluate(prop_value, op, values)
+      expect(result).not_to be_nil
+      expect(result.key).to eq('on')
+    end
+
+    def assert_no_match(prop_value, op, values)
+      result = evaluate(prop_value, op, values)
+      expect(result).to be_nil
+    end
+
+    describe 'non-set operator array matching' do
+      it 'matches scalar string IS' do
+        assert_match('hello', Evaluation::Operator::IS, ['hello'])
+      end
+
+      it 'matches scalar string CONTAINS' do
+        assert_match('hello', Evaluation::Operator::CONTAINS, ['ell'])
+      end
+
+      it 'matches scalar string GREATER_THAN' do
+        assert_match('2', Evaluation::Operator::GREATER_THAN, ['1'])
+      end
+
+      it 'does not match scalar string IS when value differs' do
+        assert_no_match('world', Evaluation::Operator::IS, ['hello'])
+      end
+
+      it 'matches non-string scalar GREATER_THAN' do
+        assert_match(42, Evaluation::Operator::GREATER_THAN, ['1'])
+      end
+
+      it 'matches non-string scalar IS (boolean)' do
+        assert_match(true, Evaluation::Operator::IS, ['true'])
+      end
+
+      it 'matches JSON array string with set operator' do
+        assert_match('["a","b"]', Evaluation::Operator::SET_CONTAINS, ['a'])
+      end
+
+      it 'matches JSON array string with non-set operator' do
+        assert_match('["a","b"]', Evaluation::Operator::IS, ['a'])
+      end
+
+      it 'matches collection with set operator' do
+        assert_match(%w[a b], Evaluation::Operator::SET_CONTAINS, ['a'])
+      end
+
+      it 'matches collection with non-set operator' do
+        assert_match(%w[a b], Evaluation::Operator::IS, ['a'])
+      end
+
+      it 'falls through for malformed JSON array and matches as scalar' do
+        assert_match('[broken', Evaluation::Operator::IS, ['[broken'])
+      end
+
+      it 'does not match empty JSON array for set operator' do
+        assert_no_match('[]', Evaluation::Operator::SET_CONTAINS, ['a'])
+      end
+
+      it 'treats leading-whitespace string as scalar for non-set operator' do
+        assert_match(' ["a"]', Evaluation::Operator::IS, [' ["a"]'])
+      end
+
+      it 'treats leading-whitespace string as scalar for set operator (no match)' do
+        assert_no_match(' ["a"]', Evaluation::Operator::SET_CONTAINS, ['a'])
+      end
+    end
+  end
+end