Impression seenAt

.github/workflows/codeql-analysis.yml

@@ -0,0 +1,52 @@
+name: "Code scanning - action"
+
+on:
+  push:
+  pull_request:
+  schedule:
+    - cron: '0 21 * * 3'
+
+jobs:
+  CodeQL-Build:
+
+    # CodeQL runs on ubuntu-latest and windows-latest
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+      with:
+        # We must fetch at least the immediate parents so that if this is
+        # a pull request then we can checkout the head.
+        fetch-depth: 2
+
+    # If this run was triggered by a pull request event, then checkout
+    # the head of the pull request instead of the merge commit.
+    - run: git checkout HEAD^2
+      if: ${{ github.event_name == 'pull_request' }}
+
+    # Initializes the CodeQL tools for scanning.
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v1
+      # Override language selection by uncommenting this and choosing your languages
+      # with:
+      #   languages: go, javascript, csharp, python, cpp, java
+
+    # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
+    # If this step fails, then you should remove it and run the build manually (see below)
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v1
+
+    # ℹ️ Command-line programs to run using the OS shell.
+    # 📚 https://git.io/JvXDl
+
+    # ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
+    #    and modify them (or add more) to build your code if your project
+    #    uses a compiled language
+
+    #- run: |
+    #   make bootstrap
+    #   make release
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v1

client/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>io.split.client</groupId>
         <artifactId>java-client-parent</artifactId>
-        <version>3.3.3</version>
+        <version>3.3.4-rc1</version>
     </parent>
     <artifactId>java-client</artifactId>
     <packaging>jar</packaging>

client/src/main/java/io/split/client/dtos/KeyImpression.java

@@ -9,6 +9,7 @@ public class KeyImpression {
     public String label;
     public long time;
     public Long changeNumber; // can be null if there is no changeNumber
+    public Long pt;
 
     @Override
     public boolean equals(Object o) {

client/src/main/java/io/split/client/impressions/ImpressionHasher.java

@@ -0,0 +1,30 @@
+package io.split.client.impressions;
+
+import io.split.client.dtos.KeyImpression;
+import io.split.client.utils.MurmurHash3;
+
+public class ImpressionHasher {
+
+    private static final String HASHABLE_FORMAT = "%s:%s:%s:%s:%d";
+    private static final String UNKNOWN = "UNKNOWN";
+
+    private static String unknownIfNull(String s) {
+        return (s == null) ? UNKNOWN : s;
+    }
+
+    private static Long zeroIfNull(Long l) {
+        return (l == null) ? 0 : l;
+    }
+
+    public static Long process(KeyImpression impression) {
+        if (null == impression) {
+            return null;
+        }
+        return MurmurHash3.hash128x64(String.format(HASHABLE_FORMAT,
+                unknownIfNull(impression.keyName),
+                unknownIfNull(impression.feature),
+                unknownIfNull(impression.treatment),
+                unknownIfNull(impression.label),
+                zeroIfNull(impression.changeNumber)).getBytes())[0];
+    }
+}

client/src/main/java/io/split/client/impressions/ImpressionObserver.java

@@ -0,0 +1,38 @@
+package io.split.client.impressions;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import io.split.client.dtos.KeyImpression;
+import org.apache.http.annotation.NotThreadSafe;
+
+/*
+According to guava's docs (https://guava.dev/releases/18.0/api/docs/com/google/common/annotations/Beta.html),
+the @Beta decorator only means that the api is not frozen, and has nothing to do with behaviour stability, but
+rather to a non-frozen API which may introduce breaking changes at any time in future versions.
+Since the library is shaded and should not be exposed to users of the SDK, it's safe to use it here.
+ */
+
+@SuppressWarnings("UnstableApiUsage")
+@NotThreadSafe
+public class ImpressionObserver {
+
+    private final Cache<Long, Long> _cache;
+
+    public ImpressionObserver(long size) {
+        _cache = CacheBuilder.newBuilder()
+                .maximumSize(size)
+                .concurrencyLevel(4)  // Just setting the default value explicitly
+                .build();
+    }
+
+    public Long testAndSet(KeyImpression impression) {
+        if (null == impression) {
+            return null;
+        }
+
+        Long hash = ImpressionHasher.process(impression);
+        Long previous = _cache.getIfPresent(hash);
+        _cache.put(hash, impression.time);
+        return previous;
+    }
+}

client/src/main/java/io/split/client/impressions/ImpressionsManager.java

@@ -28,12 +28,14 @@
 public class ImpressionsManager implements ImpressionListener, Runnable {
 
     private static final Logger _log = LoggerFactory.getLogger(ImpressionsManager.class);
+    private static final long LAST_SEEN_CACHE_SIZE = 500000; // cache up to 500k impression hashes
 
     private final SplitClientConfig _config;
     private final CloseableHttpClient _client;
     private final BlockingQueue<KeyImpression> _queue;
     private final ScheduledExecutorService _scheduler;
     private final ImpressionsSender _impressionsSender;
+    private final ImpressionObserver _impressionObserver;
 
     public static ImpressionsManager instance(CloseableHttpClient client,
                                               SplitClientConfig config) throws URISyntaxException {
@@ -51,6 +53,7 @@ private ImpressionsManager(CloseableHttpClient client, SplitClientConfig config,
         _config = config;
         _client = client;
         _queue = new ArrayBlockingQueue<KeyImpression>(config.impressionsQueueSize());
+        _impressionObserver = new ImpressionObserver(LAST_SEEN_CACHE_SIZE);
 
         ThreadFactory threadFactory = new ThreadFactoryBuilder()
                 .setDaemon(true)
@@ -129,6 +132,7 @@ private void sendImpressions() {
                 impressionsForTest = new ArrayList<>();
                 tests.put(ki.feature, impressionsForTest);
             }
+            ki.pt = _impressionObserver.testAndSet(ki);
             impressionsForTest.add(ki);
         }
 

client/src/main/java/io/split/client/utils/MurmurHash3.java

@@ -161,5 +161,142 @@ public static long murmurhash3_x86_32(CharSequence data, int offset, int len, in
         return h1 & 0xFFFFFFFFL;
     }
 
+    // The following set of methods and constants are borrowed from:
+    // `This method is borrowed from `org.apache.commons.codec.digest.MurmurHash3`
 
+    // Constants for 128-bit variant
+    private static final long C1 = 0x87c37b91114253d5L;
+    private static final long C2 = 0x4cf5ad432745937fL;
+    private static final int R1 = 31;
+    private static final int R2 = 27;
+    private static final int R3 = 33;
+    private static final int M = 5;
+    private static final int N1 = 0x52dce729;
+    private static final int N2 = 0x38495ab5;
+
+    /**
+     * Gets the little-endian long from 8 bytes starting at the specified index.
+     *
+     * @param data The data
+     * @param index The index
+     * @return The little-endian long
+     */
+    private static long getLittleEndianLong(final byte[] data, final int index) {
+        return (((long) data[index    ] & 0xff)      ) |
+                (((long) data[index + 1] & 0xff) <<  8) |
+                (((long) data[index + 2] & 0xff) << 16) |
+                (((long) data[index + 3] & 0xff) << 24) |
+                (((long) data[index + 4] & 0xff) << 32) |
+                (((long) data[index + 5] & 0xff) << 40) |
+                (((long) data[index + 6] & 0xff) << 48) |
+                (((long) data[index + 7] & 0xff) << 56);
+    }
+
+    public static long[] hash128x64(final byte[] data) {
+        return hash128x64(data, 0, data.length, 0);
+    }
+
+    /**
+     * Generates 128-bit hash from the byte array with the given offset, length and seed.
+     *
+     * <p>This is an implementation of the 128-bit hash function {@code MurmurHash3_x64_128}
+     * from from Austin Applyby's original MurmurHash3 {@code c++} code in SMHasher.</p>
+     *
+     * @param data The input byte array
+     * @param offset The first element of array
+     * @param length The length of array
+     * @param seed The initial seed value
+     * @return The 128-bit hash (2 longs)
+     */
+    public static long[] hash128x64(final byte[] data, final int offset, final int length, final long seed) {
+        long h1 = seed;
+        long h2 = seed;
+        final int nblocks = length >> 4;
+
+        // body
+        for (int i = 0; i < nblocks; i++) {
+            final int index = offset + (i << 4);
+            long k1 = getLittleEndianLong(data, index);
+            long k2 = getLittleEndianLong(data, index + 8);
+
+            // mix functions for k1
+            k1 *= C1;
+            k1 = Long.rotateLeft(k1, R1);
+            k1 *= C2;
+            h1 ^= k1;
+            h1 = Long.rotateLeft(h1, R2);
+            h1 += h2;
+            h1 = h1 * M + N1;
+
+            // mix functions for k2
+            k2 *= C2;
+            k2 = Long.rotateLeft(k2, R3);
+            k2 *= C1;
+            h2 ^= k2;
+            h2 = Long.rotateLeft(h2, R1);
+            h2 += h1;
+            h2 = h2 * M + N2;
+        }
+
+        // tail
+        long k1 = 0;
+        long k2 = 0;
+        final int index = offset + (nblocks << 4);
+        switch (offset + length - index) {
+            case 15:
+                k2 ^= ((long) data[index + 14] & 0xff) << 48;
+            case 14:
+                k2 ^= ((long) data[index + 13] & 0xff) << 40;
+            case 13:
+                k2 ^= ((long) data[index + 12] & 0xff) << 32;
+            case 12:
+                k2 ^= ((long) data[index + 11] & 0xff) << 24;
+            case 11:
+                k2 ^= ((long) data[index + 10] & 0xff) << 16;
+            case 10:
+                k2 ^= ((long) data[index + 9] & 0xff) << 8;
+            case 9:
+                k2 ^= data[index + 8] & 0xff;
+                k2 *= C2;
+                k2 = Long.rotateLeft(k2, R3);
+                k2 *= C1;
+                h2 ^= k2;
+
+            case 8:
+                k1 ^= ((long) data[index + 7] & 0xff) << 56;
+            case 7:
+                k1 ^= ((long) data[index + 6] & 0xff) << 48;
+            case 6:
+                k1 ^= ((long) data[index + 5] & 0xff) << 40;
+            case 5:
+                k1 ^= ((long) data[index + 4] & 0xff) << 32;
+            case 4:
+                k1 ^= ((long) data[index + 3] & 0xff) << 24;
+            case 3:
+                k1 ^= ((long) data[index + 2] & 0xff) << 16;
+            case 2:
+                k1 ^= ((long) data[index + 1] & 0xff) << 8;
+            case 1:
+                k1 ^= data[index] & 0xff;
+                k1 *= C1;
+                k1 = Long.rotateLeft(k1, R1);
+                k1 *= C2;
+                h1 ^= k1;
+        }
+
+        // finalization
+        h1 ^= length;
+        h2 ^= length;
+
+        h1 += h2;
+        h2 += h1;
+
+        h1 = fmix64(h1);
+        h2 = fmix64(h2);
+
+        h1 += h2;
+        h2 += h1;
+
+        return new long[] { h1, h2 };
+    }
 }

client/src/test/java/io/split/client/impressions/ImpressionHasherTest.java

@@ -0,0 +1,73 @@
+package io.split.client.impressions;
+
+import io.split.client.dtos.KeyImpression;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+
+public class ImpressionHasherTest {
+
+    @Test
+    public void works() {
+        KeyImpression imp1 = new KeyImpression();
+        imp1.feature = "someFeature";
+        imp1.keyName = "someKey";
+        imp1.changeNumber = 123L;
+        imp1.label = "someLabel";
+        imp1.treatment = "someTreatment";
+
+        // Different feature
+        KeyImpression imp2 = new KeyImpression();
+        imp2.feature = "someOtherFeature";
+        imp2.keyName = "someKey";
+        imp2.changeNumber = 123L;
+        imp2.label = "someLabel";
+        assertThat(ImpressionHasher.process(imp1), not(equalTo(ImpressionHasher.process(imp2))));
+
+        // different key
+        imp2.feature = imp1.feature;
+        imp2.keyName = "someOtherKey";
+        assertThat(ImpressionHasher.process(imp1), not(equalTo(ImpressionHasher.process(imp2))));
+
+        // different changeNumber
+        imp2.keyName = imp1.keyName;
+        imp2.changeNumber = 456L;
+        assertThat(ImpressionHasher.process(imp1), not(equalTo(ImpressionHasher.process(imp2))));
+
+        // different label
+        imp2.changeNumber = imp1.changeNumber;
+        imp2.label = "someOtherLabel";
+        assertThat(ImpressionHasher.process(imp1), not(equalTo(ImpressionHasher.process(imp2))));
+
+        // different treatment
+        imp2.label = imp1.label;
+        imp2.treatment = "someOtherTreatment";
+        assertThat(ImpressionHasher.process(imp1), not(equalTo(ImpressionHasher.process(imp2))));
+    }
+
+    @Test
+    public void doesNotCrash() {
+        KeyImpression imp1 = new KeyImpression();
+        imp1.feature = null;
+        imp1.keyName = "someKey";
+        imp1.changeNumber = 123L;
+        imp1.label = "someLabel";
+        assertNotNull(ImpressionHasher.process(imp1));
+
+        imp1.keyName = null;
+        assertNotNull(ImpressionHasher.process(imp1));
+
+        imp1.changeNumber = null;
+        assertNotNull(ImpressionHasher.process(imp1));
+
+        imp1.label = null;
+        assertNotNull(ImpressionHasher.process(imp1));
+
+        imp1.treatment = null;
+        assertNotNull(ImpressionHasher.process(imp1));
+
+        assertNull(ImpressionHasher.process(null));
+    }
+}