Add impression observer & Add previous time to impression

lib/splitclient-rb.rb

@@ -13,6 +13,7 @@
 require 'splitclient-rb/cache/fetchers/segment_fetcher'
 require 'splitclient-rb/cache/fetchers/split_fetcher'
 require 'splitclient-rb/cache/hashers/impression_hasher'
+require 'splitclient-rb/cache/observers/impression_observer'
 require 'splitclient-rb/cache/repositories/repository'
 require 'splitclient-rb/cache/repositories/segments_repository'
 require 'splitclient-rb/cache/repositories/splits_repository'

lib/splitclient-rb/cache/observers/impression_observer.rb

@@ -0,0 +1,22 @@
+module SplitIoClient
+  module Observers
+    class ImpressionObserver
+      LAST_SEEN_CACHE_SIZE = 500000
+
+      def initialize
+        @cache = LruRedux::TTL::ThreadSafeCache.new(LAST_SEEN_CACHE_SIZE)
+        @impression_hasher = Hashers::ImpressionHasher.new
+      end
+
+      def test_and_set(impression)
+        return if impression.nil?
+
+        hash = @impression_hasher.process(impression)
+        previous = @cache[hash]
+        @cache[hash] = impression[:m]
+
+        previous.nil? ? nil : [previous, impression[:m]].min
+      end
+    end
+  end
+end

lib/splitclient-rb/cache/senders/impressions_formatter.rb

@@ -45,7 +45,8 @@ def current_impressions(feature_impressions)
               time: impression[:i][:m],
               bucketingKey: impression[:i][:b],
               label: impression[:i][:r],
-              changeNumber: impression[:i][:c]
+              changeNumber: impression[:i][:c],
+              previousTime: impression[:i][:pt]
             }
           end
         end
@@ -73,7 +74,8 @@ def impression_hash(impression)
           "#{impression[:i][:k]}:" \
           "#{impression[:i][:b]}:" \
           "#{impression[:i][:c]}:" \
-          "#{impression[:i][:t]}"
+          "#{impression[:i][:t]}:" \
+          "#{impression[:i][:pt]}"
         end
       end
     end

lib/splitclient-rb/engine/common/impressions_manager.rb

@@ -8,13 +8,14 @@ def initialize(config, impressions_repository)
           @config = config
           @impressions_repository = impressions_repository
           @impression_router = SplitIoClient::ImpressionRouter.new(@config)
-          # @impression_observer = impression_observer
+          @impression_observer = SplitIoClient::Observers::ImpressionObserver.new
         end
 
         # added param time for test
         def build_impression(matching_key, bucketing_key, split_name, treatment, params = {})
           impression_data = impression_data(matching_key, bucketing_key, split_name, treatment, params[:time])
-          # impression_data[:pt] = @impression_observer.test_and_set(impression)
+
+          impression_data[:pt] = @impression_observer.test_and_set(impression_data) if should_add_pt
 
           { m: metadata, i: impression_data, attributes: params[:attributes] }
         rescue StandardError => error
@@ -55,6 +56,10 @@ def metadata
         def applied_rule(label)
           @config.labels_enabled ? label : nil
         end
+
+        def should_add_pt
+          @config.impressions_adapter.class.to_s != 'SplitIoClient::Cache::Adapters::RedisAdapter'
+        end
       end
     end
   end

spec/cache/observers/impression_observer_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SplitIoClient::Observers::ImpressionObserver do
+  subject { SplitIoClient::Observers::ImpressionObserver }
+  let(:log) { StringIO.new }
+  let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log)) }
+  let(:ip) { config.machine_ip }
+  let(:machine_name) { config.machine_name }
+  let(:version) { "#{config.language}-#{config.version}" }
+  let(:impression_data1) do
+    {
+      k: 'matching_key',
+      b: 'bucketing_key',
+      f: 'split_name',
+      t: 'treatment',
+      r: 'label',
+      c: 1_533_177_602_748,
+      m: 1_478_113_516_002,
+      pt: nil
+    }
+  end
+
+  let(:impression_data2) do
+    {
+      k: 'matching_key_2',
+      b: 'bucketing_key',
+      f: 'split_name',
+      t: 'treatment',
+      r: 'label',
+      c: 1_533_177_602_748,
+      m: 1_478_113_516_022,
+      pt: nil
+    }
+  end
+
+  context 'test_and_set' do
+    before do
+      @impression_observer = subject.new
+    end
+
+    it 'first time should be nil and after that always return previous time' do
+      result1 = @impression_observer.test_and_set(impression_data1)
+      expect(result1).to be_nil
+
+      # should return previous time
+      impression_data1[:m] = 1_478_113_516_500
+      result1 = @impression_observer.test_and_set(impression_data1)
+      expect(result1).to eq(1_478_113_516_002)
+
+      # should return new impression.time
+      result1 = @impression_observer.test_and_set(impression_data1)
+      expect(result1).to eq(1_478_113_516_500)
+
+      # when impression.time < impression.pt should return the min.
+      impression_data1[:m] = 1_478_113_516_001
+      result1 = @impression_observer.test_and_set(impression_data1)
+      expect(result1).to eq(1_478_113_516_001)
+
+      # should return nil because is another impression
+      result2 = @impression_observer.test_and_set(impression_data2)
+      expect(result2).to be_nil
+
+      # should return previous time
+      result2 = @impression_observer.test_and_set(impression_data2)
+      expect(result2).to eq(1_478_113_516_022)
+    end
+
+    it 'return nil because impression is nil' do
+      result = @impression_observer.test_and_set(nil)
+      expect(result).to be_nil
+    end
+  end
+end

spec/cache/senders/impressions_formatter_spec.rb

@@ -18,10 +18,11 @@
       Redis.new.flushall
       params = { attributes: {}, time: 1_478_113_516_002 }
       params2 = { attributes: {}, time: 1_478_113_518_285 }
-      impressions = []
-      impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
-      impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
-      impressions_manager.track(impressions)
+
+      @impressions = []
+      @impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
+      @impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
+      impressions_manager.track(@impressions)
     end
 
     it 'formats impressions to be sent' do
@@ -32,7 +33,8 @@
                                              treatment: 'on',
                                              time: 1_478_113_516_002,
                                              bucketingKey: 'foo1', label: 'custom_label1',
-                                             changeNumber: 123_456 }],
+                                             changeNumber: 123_456,
+                                             previousTime: nil }],
                           ip: ip
                         },
                          {
@@ -42,7 +44,8 @@
                                               time: 1_478_113_518_285,
                                               bucketingKey: 'foo2',
                                               label: 'custom_label2',
-                                              changeNumber: 123_499 }],
+                                              changeNumber: 123_499,
+                                              previousTime: nil }],
                            ip: ip
                          }])
     end
@@ -61,7 +64,8 @@
             time: 1_478_113_516_002,
             bucketingKey: 'foo1',
             label: 'custom_label1',
-            changeNumber: 123_456
+            changeNumber: 123_456,
+            previousTime: nil
           }
         ]
       )
@@ -74,39 +78,31 @@
             time: 1_478_113_518_285,
             bucketingKey: 'foo2',
             label: 'custom_label2',
-            changeNumber: 123_499
+            changeNumber: 123_499,
+            previousTime: nil
           },
           {
             keyName: 'matching_key3',
             treatment: 'off',
             time: 1_478_113_518_900,
             bucketingKey: nil,
             label: nil,
-            changeNumber: nil
+            changeNumber: nil,
+            previousTime: nil
           }
         ]
       )
     end
 
     it 'filters out impressions with the same key/treatment' do
-      params = { attributes: {}, time: 1_478_113_516_902 }
-      params2 = { attributes: {}, time: 1_478_113_518_285 }
-      impressions = []
-      impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
-      impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
-      impressions_manager.track(impressions)
+      impressions_manager.track(@impressions)
 
       expect(formatted_impressions.find { |i| i[:testName] == :foo1 }[:keyImpressions].size).to eq(1)
       expect(formatted_impressions.find { |i| i[:testName] == :foo2 }[:keyImpressions].size).to eq(1)
     end
 
     it 'filters out impressions with the same key/treatment legacy' do
-      params = { attributes: {}, time: 1_478_113_516_902 }
-      params2 = { attributes: {}, time: 1_478_113_518_285 }
-      impressions = []
-      impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
-      impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
-      impressions_manager.track(impressions)
+      impressions_manager.track(@impressions)
 
       expect(formatted_impressions.find { |i| i[:testName] == :foo1 }[:keyImpressions].size).to eq(1)
       expect(formatted_impressions.find { |i| i[:testName] == :foo2 }[:keyImpressions].size).to eq(1)

spec/cache/senders/impressions_sender_spec.rb

@@ -50,7 +50,8 @@
                 time: 1_478_113_516_002,
                 bucketingKey: 'foo1',
                 label: 'custom_label1',
-                changeNumber: 123_456
+                changeNumber: 123_456,
+                previousTime: nil
               }
             ],
             ip: config.machine_ip
@@ -64,7 +65,8 @@
                 time: 1_478_113_518_285,
                 bucketingKey: 'foo2',
                 label: 'custom_label2',
-                changeNumber: 123_499
+                changeNumber: 123_499,
+                previousTime: nil
               }
             ],
             ip: config.machine_ip