fix tests and implemetation

lib/experiment/local/assignment/assignment_service.rb

@@ -7,7 +7,7 @@ def initialize(amplitude, assignment_filter)
     end
 
     def track(assignment)
-      @amplitude.track(to_event(assignment)) if @assignment_filter.should_track(assignment)
+      @amplitude.track(AssignmentService.to_event(assignment)) if @assignment_filter.should_track(assignment)
     end
 
     def self.to_event(assignment)
@@ -46,4 +46,3 @@ def self.to_event(assignment)
     end
   end
 end
-# AssignmentService

lib/experiment/local/client.rb

@@ -38,17 +38,8 @@ def initialize(api_key, config = nil)
     #
     # @return [Hash[String, Variant]] The evaluated variants
     def evaluate(user, flag_keys = [])
-      flags = @flags_mutex.synchronize do
-        @flags
-      end
-      return {} if flags.nil?
-
-      user_str = user.to_json
-
-      @logger.debug("[Experiment] Evaluate: User: #{user_str} - Rules: #{flags}") if @config.debug
-      result = evaluation(flags, user_str)
-      @logger.debug("[Experiment] evaluate - result: #{result}") if @config.debug
-      parse_results(result, flag_keys, user)
+      variants = evaluate_v2(user, flag_keys)
+      AmplitudeExperiment.filter_default_variants(variants)
     end
 
     # Locally evaluates flag variants for a user.
@@ -65,7 +56,7 @@ def evaluate_v2(user, flag_keys = [])
       end
       return {} if flags.nil?
 
-      sorted_flags = AmplitudeExperiment.topological_sort(flags, flag_keys)
+      sorted_flags = AmplitudeExperiment.topological_sort(flags, flag_keys.to_set)
       flags_json = sorted_flags.to_json
 
       enriched_user = AmplitudeExperiment.user_to_evaluation_context(user)
@@ -75,6 +66,7 @@ def evaluate_v2(user, flag_keys = [])
       result = evaluation(flags_json, user_str)
       @logger.debug("[Experiment] evaluate - result: #{result}") if @config.debug
       variants = AmplitudeExperiment.evaluation_variants_json_to_variants(result)
+      @assignment_service&.track(Assignment.new(user, variants))
       variants
     end
 
@@ -96,23 +88,6 @@ def stop
 
     private
 
-    def parse_results(result, flag_keys, user)
-      variants = {}
-      assignments = {}
-      result.each do |key, value|
-        included = flag_keys.empty? || flag_keys.include?(key)
-        if !value['isDefaultVariant'] && included
-          variant_key = value['variant']['key']
-          variant_payload = value['variant']['payload']
-          variants.store(key, Variant.new(variant_key, variant_payload))
-        end
-
-        assignments[key] = value if included || value['type'] == FLAG_TYPE_MUTUAL_EXCLUSION_GROUP || value['type'] == FLAG_TYPE_HOLDOUT_GROUP
-      end
-      @assignment_service&.track(Assignment.new(user, assignments))
-      variants
-    end
-
     def run
       @is_running = true
       begin

lib/experiment/remote/client.rb

@@ -30,7 +30,7 @@ def initialize(api_key, config = nil)
     # @param [User] user
     # @return [Hash] Variants Hash
     def fetch(user)
-      filter_default_variants(fetch_internal(user))
+      AmplitudeExperiment.filter_default_variants(fetch_internal(user))
     rescue StandardError => e
       @logger.error("[Experiment] Failed to fetch variants: #{e.message}")
       {}
@@ -144,30 +144,11 @@ def do_fetch(user, timeout_millis)
       raise FetchError.new(response.code.to_i, "Fetch error response: status=#{response.code} #{response.message}") if response.code != '200'
 
       json = JSON.parse(response.body)
-      variants = parse_json_variants(json)
+      variants = AmplitudeExperiment.evaluation_variants_json_to_variants(json)
       @logger.debug("[Experiment] Fetched variants: #{variants}")
       variants
     end
 
-    # Parse JSON response hash
-    #
-    # @param [Hash] json
-    # @return [Hash] Hash with String => Variant
-    def parse_json_variants(json)
-      variants = {}
-      json.each do |key, value|
-        variant_value = ''
-        if value.key?('value')
-          variant_value = value.fetch('value')
-        elsif value.key?('key')
-          # value was previously under the "key" field
-          variant_value = value.fetch('key')
-        end
-        variants.store(key, Variant.new(variant_value, value.fetch('payload', nil), value.fetch('key', nil), value.fetch('metadata', nil)))
-      end
-      variants
-    end
-
     # @param [User] user
     # @return [User, Hash] user with library context
     def add_context(user)
@@ -181,18 +162,5 @@ def should_retry_fetch?(err)
 
       true
     end
-
-    def filter_default_variants(variants)
-      variants.each do |key, value|
-        default = value&.metadata&.fetch('default', nil)
-        deployed = value&.metadata&.fetch('deployed', nil)
-        default = false if default.nil?
-        deployed = true if deployed.nil?
-        variants.delete(key) if default || !deployed
-      end
-      variants
-    end
-
-    private :filter_default_variants
   end
 end

lib/experiment/util/topological_sort.rb

@@ -17,7 +17,7 @@ def self.parent_traversal(flag_key, available, path)
     flag = available[flag_key]
     return nil if flag.nil?
 
-    dependencies = flag[:dependencies]
+    dependencies = flag["dependencies"]
     if dependencies.nil? || dependencies.empty?
       available.delete(flag_key)
       return [flag]

lib/experiment/util/variant.rb

@@ -18,5 +18,16 @@ def self.evaluation_variant_json_to_variant(variant_json)
       metadata: variant_json['metadata']
     )
   end
+
+  def self.filter_default_variants(variants)
+    variants.each do |key, value|
+      default = value&.metadata&.fetch('default', nil)
+      deployed = value&.metadata&.fetch('deployed', nil)
+      default = false if default.nil?
+      deployed = true if deployed.nil?
+      variants.delete(key) if default || !deployed
+    end
+    variants
+  end
 end
 

spec/experiment/local/assignment/assignment_spec.rb

@@ -0,0 +1,164 @@
+module AmplitudeExperiment
+  describe AssignmentService do
+    let(:instance) { double('Amplitude') }
+    let(:user) { User.new(user_id: 'user', device_id: 'device') }
+
+    def build_variant(key, value, metadata = {})
+      Variant.new(
+        key: key,
+        value: value,
+        metadata: metadata
+      )
+    end
+
+    it 'assignment to event as expected' do
+      variants = {
+        'basic' => build_variant('control', 'control', 'segmentName' => 'All Other Users', 'flagType' => 'experiment', 'flagVersion' => 10, 'default' => false),
+        'different_value' => build_variant('on', 'control', 'segmentName' => 'All Other Users', 'flagType' => 'experiment', 'flagVersion' => 10, 'default' => false),
+        'default' => build_variant('off', nil, 'segmentName' => 'All Other Users', 'flagType' => 'experiment', 'flagVersion' => 10, 'default' => true),
+        'mutex' => build_variant('slot-1', 'slot-1', 'segmentName' => 'All Other Users', 'flagType' => 'mutual-exclusion-group', 'flagVersion' => 10, 'default' => false),
+        'holdout' => build_variant('holdout', 'holdout', 'segmentName' => 'All Other Users', 'flagType' => 'holdout-group', 'flagVersion' => 10, 'default' => false),
+        'partial_metadata' => build_variant('on', 'on', 'segmentName' => 'All Other Users', 'flagType' => 'release'),
+        'empty_metadata' => build_variant('on', 'on'),
+        'empty_variant' => build_variant(nil, nil)
+      }
+      assignment = Assignment.new(user, variants)
+      event = AssignmentService.to_event(assignment)
+
+      expect(event.user_id).to eq(user.user_id)
+      expect(event.device_id).to eq(user.device_id)
+      expect(event.event_type).to eq('[Experiment] Assignment')
+
+      event_properties = event.event_properties
+      expect(event_properties['basic.variant']).to eq('control')
+      expect(event_properties['basic.details']).to eq('v10 rule:All Other Users')
+      expect(event_properties['different_value.variant']).to eq('on')
+      expect(event_properties['different_value.details']).to eq('v10 rule:All Other Users')
+      expect(event_properties['default.variant']).to eq('off')
+      expect(event_properties['default.details']).to eq('v10 rule:All Other Users')
+      expect(event_properties['mutex.variant']).to eq('slot-1')
+      expect(event_properties['mutex.details']).to eq('v10 rule:All Other Users')
+      expect(event_properties['holdout.variant']).to eq('holdout')
+      expect(event_properties['holdout.details']).to eq('v10 rule:All Other Users')
+      expect(event_properties['partial_metadata.variant']).to eq('on')
+      expect(event_properties['empty_metadata.variant']).to eq('on')
+
+      user_properties = event.user_properties
+      set_properties = user_properties['$set']
+      expect(set_properties['[Experiment] basic']).to eq('control')
+      expect(set_properties['[Experiment] different_value']).to eq('on')
+      expect(set_properties['[Experiment] holdout']).to eq('holdout')
+      expect(set_properties['[Experiment] partial_metadata']).to eq('on')
+      expect(set_properties['[Experiment] empty_metadata']).to eq('on')
+      unset_properties = user_properties['$unset']
+      expect(unset_properties['[Experiment] default']).to eq('-')
+
+      canonicalization = 'user device basic control default off different_value on empty_metadata on holdout holdout mutex slot-1 partial_metadata on '
+      expected = "user device #{AmplitudeExperiment.hash_code(canonicalization)} #{assignment.timestamp / DAY_MILLIS}"
+      expect(event.insert_id).to eq(expected)
+    end
+
+    it 'calls track on the Amplitude instance' do
+      service = AssignmentService.new(instance, AssignmentFilter.new(2))
+      user = User.new(user_id: 'user', device_id: 'device')
+      results = { 'flag-key-1' => Variant.new(key: 'on') }
+      allow(instance).to receive(:track)
+      service.track(Assignment.new(user, results))
+      expect(instance).to have_received(:track)
+    end
+  end
+
+  describe AssignmentFilter do
+    let(:filter) { AssignmentFilter.new(100) }
+    let(:user) { User.new(user_id: 'user', device_id: 'device') }
+    let(:variant_on) { Variant.new(key: 'on', value: 'on') }
+    let(:variant_control) { Variant.new(key: 'control', value: 'control') }
+    let(:results) { { 'flag-key-1' => variant_on, 'flag-key-2' => variant_control } }
+
+    it 'filters single assignment' do
+      assignment = Assignment.new(user, results)
+      expect(filter.should_track(assignment)).to eq(true)
+    end
+
+    it 'filters duplicate assignment' do
+      assignment1 = Assignment.new(user, results)
+      assignment2 = Assignment.new(user, results)
+      filter.should_track(assignment1)
+      expect(filter.should_track(assignment2)).to eq(false)
+    end
+
+    it 'filters same user different results' do
+      results1 = results
+      results2 = {
+        'flag-key-1' => variant_control,
+        'flag-key-2' => variant_on
+      }
+      assignment1 = Assignment.new(user, results1)
+      assignment2 = Assignment.new(user, results2)
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(true)
+    end
+
+    it 'filters same result different user' do
+      user1 = User.new(user_id: 'user1')
+      user2 = User.new(user_id: 'different-user')
+      assignment1 = Assignment.new(user1, results)
+      assignment2 = Assignment.new(user2, results)
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(true)
+    end
+
+    it 'filters empty result' do
+      user1 = User.new(user_id: 'user')
+      user2 = User.new(user_id: 'different-user')
+      assignment1 = Assignment.new(user1, {})
+      assignment2 = Assignment.new(user1, {})
+      assignment3 = Assignment.new(user2, {})
+      expect(filter.should_track(assignment1)).to eq(false)
+      expect(filter.should_track(assignment2)).to eq(false)
+      expect(filter.should_track(assignment3)).to eq(false)
+    end
+
+    it 'filters duplicate assignments with different result ordering' do
+      results1 = results
+      results2 = {
+        'flag-key-2' => variant_control,
+        'flag-key-1' => variant_on
+      }
+      assignment1 = Assignment.new(user, results1)
+      assignment2 = Assignment.new(user, results2)
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(false)
+    end
+
+    it 'handles LRU replacement' do
+      filter = AssignmentFilter.new(2)
+      user1 = User.new(user_id: 'user1')
+      user2 = User.new(user_id: 'user2')
+      user3 = User.new(user_id: 'user3')
+      assignment1 = Assignment.new(user1, results)
+      assignment2 = Assignment.new(user2, results)
+      assignment3 = Assignment.new(user3, results)
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(true)
+      expect(filter.should_track(assignment3)).to eq(true)
+      expect(filter.should_track(assignment1)).to eq(true)
+    end
+
+    it 'handles LRU TTL-based expiration' do
+      filter = AssignmentFilter.new(2, 1000)
+      user1 = User.new(user_id: 'user1')
+      user2 = User.new(user_id: 'user2')
+      assignment1 = Assignment.new(user1, results)
+      assignment2 = Assignment.new(user2, results)
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment1)).to eq(false)
+      sleep 1.05
+      expect(filter.should_track(assignment1)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(true)
+      expect(filter.should_track(assignment2)).to eq(false)
+      sleep 0.95
+      expect(filter.should_track(assignment2)).to eq(false)
+    end
+  end
+end