feat: Read entries out of AbstractAssignmentCache (#89)

* feat: Read entries out of `AbstractAssignmentCache`

* make these protected

* comments

* new Set-based assignment cache

* fix imports

* update tests to reflect new logic

* extract key to string method

* export symbols

* update comments

* go back to map implementation

* fix tests

* expose map store

* fix assignment cache key/value handling

* export this fn

* fix nits

Author: felipecsl

src/cache/abstract-assignment-cache.spec.ts

@@ -0,0 +1,27 @@
+import {
+  assignmentCacheKeyToString,
+  assignmentCacheValueToString,
+  NonExpiringInMemoryAssignmentCache,
+} from './abstract-assignment-cache';
+
+describe('NonExpiringInMemoryAssignmentCache', () => {
+  it('read and write entries', () => {
+    const cache = new NonExpiringInMemoryAssignmentCache();
+    const key1 = { subjectKey: 'a', flagKey: 'b', allocationKey: 'c', variationKey: 'd' };
+    const key2 = { subjectKey: '1', flagKey: '2', allocationKey: '3', variationKey: '4' };
+    cache.set(key1);
+    expect(cache.has(key1)).toBeTruthy();
+    expect(cache.has(key2)).toBeFalsy();
+    cache.set(key2);
+    expect(cache.has(key2)).toBeTruthy();
+    // this makes an assumption about the internal implementation of the cache, which is not ideal
+    // but it's the only way to test the cache without exposing the internal state
+    expect(Array.from(cache.entries())).toEqual([
+      [assignmentCacheKeyToString(key1), assignmentCacheValueToString(key1)],
+      [assignmentCacheKeyToString(key2), assignmentCacheValueToString(key2)],
+    ]);
+
+    expect(cache.has({ ...key1, allocationKey: 'c1' })).toBeFalsy();
+    expect(cache.has({ ...key2, variationKey: 'd1' })).toBeFalsy();
+  });
+});

src/cache/assignment-cache.ts renamed to src/cache/abstract-assignment-cache.ts

@@ -2,22 +2,42 @@ import { getMD5Hash } from '../obfuscation';
 
 import { LRUCache } from './lru-cache';
 
+export type AssignmentCacheValue = {
+  allocationKey: string;
+  variationKey: string;
+};
+
 export type AssignmentCacheKey = {
   subjectKey: string;
   flagKey: string;
-  allocationKey: string;
-  variationKey: string;
 };
 
+export type AssignmentCacheEntry = AssignmentCacheKey & AssignmentCacheValue;
+
+/** Converts an {@link AssignmentCacheKey} to a string. */
+export function assignmentCacheKeyToString({ subjectKey, flagKey }: AssignmentCacheKey): string {
+  return getMD5Hash([subjectKey, flagKey].join(';'));
+}
+
+export function assignmentCacheValueToString({
+  allocationKey,
+  variationKey,
+}: AssignmentCacheValue): string {
+  return getMD5Hash([allocationKey, variationKey].join(';'));
+}
+
 export interface AsyncMap<K, V> {
   get(key: K): Promise<V | undefined>;
+
   set(key: K, value: V): Promise<void>;
+
   has(key: K): Promise<boolean>;
 }
 
 export interface AssignmentCache {
-  set(key: AssignmentCacheKey): void;
-  has(key: AssignmentCacheKey): boolean;
+  set(key: AssignmentCacheEntry): void;
+
+  has(key: AssignmentCacheEntry): boolean;
 }
 
 export abstract class AbstractAssignmentCache<T extends Map<string, string>>
@@ -26,30 +46,29 @@ export abstract class AbstractAssignmentCache<T extends Map<string, string>>
   // key -> variation value hash
   protected constructor(protected readonly delegate: T) {}
 
-  has(key: AssignmentCacheKey): boolean {
-    const isPresent = this.delegate.has(this.toCacheKeyString(key));
-    if (!isPresent) {
-      // no cache key present
-      return false;
-    }
-
-    // the subject has been assigned to a different variation
-    // than was previously logged.
-    // in this case we need to log the assignment again.
-    const cachedValue = this.get(key);
-    return cachedValue === getMD5Hash(key.variationKey);
+  /** Returns whether the provided {@link AssignmentCacheEntry} is present in the cache. */
+  has(entry: AssignmentCacheEntry): boolean {
+    return this.get(entry) === assignmentCacheValueToString(entry);
   }
 
   private get(key: AssignmentCacheKey): string | undefined {
-    return this.delegate.get(this.toCacheKeyString(key));
+    return this.delegate.get(assignmentCacheKeyToString(key));
   }
 
-  set(key: AssignmentCacheKey): void {
-    this.delegate.set(this.toCacheKeyString(key), getMD5Hash(key.variationKey));
+  /**
+   * Stores the provided {@link AssignmentCacheEntry} in the cache. If the key already exists, it
+   * will be overwritten.
+   */
+  set(entry: AssignmentCacheEntry): void {
+    this.delegate.set(assignmentCacheKeyToString(entry), assignmentCacheValueToString(entry));
   }
 
-  private toCacheKeyString({ subjectKey, flagKey, allocationKey }: AssignmentCacheKey): string {
-    return [`subject:${subjectKey}`, `flag:${flagKey}`, `allocation:${allocationKey}`].join(';');
+  /**
+   * Returns an array with all {@link AssignmentCacheEntry} entries in the cache as an array of
+   * {@link string}s.
+   */
+  entries(): IterableIterator<[string, string]> {
+    return this.delegate.entries();
   }
 }
 
@@ -62,8 +81,8 @@ export abstract class AbstractAssignmentCache<T extends Map<string, string>>
 export class NonExpiringInMemoryAssignmentCache extends AbstractAssignmentCache<
   Map<string, string>
 > {
-  constructor() {
-    super(new Map<string, string>());
+  constructor(store = new Map<string, string>()) {
+    super(store);
   }
 }
 

src/cache/lru-cache.spec.ts

@@ -13,7 +13,7 @@ describe('LRUCache', () => {
   });
 
   it('should return undefined for missing values', () => {
-    expect(cache.get('missing')).toBeUndefined();
+    expect(cache.get('missing')).toBeFalsy();
   });
 
   it('should overwrite existing values', () => {
@@ -26,7 +26,7 @@ describe('LRUCache', () => {
     cache.set('a', 'apple');
     cache.set('b', 'banana');
     cache.set('c', 'cherry');
-    expect(cache.get('a')).toBeUndefined();
+    expect(cache.get('a')).toBeFalsy();
     expect(cache.get('b')).toBe('banana');
     expect(cache.get('c')).toBe('cherry');
   });
@@ -37,7 +37,7 @@ describe('LRUCache', () => {
     cache.get('a'); // Access 'a' to make it recently used
     cache.set('c', 'cherry');
     expect(cache.get('a')).toBe('apple');
-    expect(cache.get('b')).toBeUndefined();
+    expect(cache.get('b')).toBeFalsy();
     expect(cache.get('c')).toBe('cherry');
   });
 
@@ -50,15 +50,15 @@ describe('LRUCache', () => {
   it('should handle the cache capacity of zero', () => {
     const zeroCache = new LRUCache(0);
     zeroCache.set('a', 'apple');
-    expect(zeroCache.get('a')).toBeUndefined();
+    expect(zeroCache.get('a')).toBeFalsy();
   });
 
   it('should handle the cache capacity of one', () => {
     const oneCache = new LRUCache(1);
     oneCache.set('a', 'apple');
     expect(oneCache.get('a')).toBe('apple');
     oneCache.set('b', 'banana');
-    expect(oneCache.get('a')).toBeUndefined();
+    expect(oneCache.get('a')).toBeFalsy();
     expect(oneCache.get('b')).toBe('banana');
   });
 });

src/cache/lru-cache.ts

@@ -1,10 +1,15 @@
 /**
- * LRUCache is a cache that stores a maximum number of items.
+ * LRUCache is a simple implementation of a Least Recently Used (LRU) cache.
  *
- * Items are removed from the cache when the cache is full.
+ * Old items are evicted when the cache reaches its capacity.
  *
- * The cache is implemented as a Map, which is a built-in JavaScript object.
- * The Map object holds key-value pairs and remembers the order of key-value pairs as they were inserted.
+ * The cache is implemented as a Map, which maintains insertion order:
+ * ```
+ * Iteration happens in insertion order, which corresponds to the order in which each key-value pair
+ * was first inserted into the map by the set() method (that is, there wasn't a key with the same
+ * value already in the map when set() was called).
+ * ```
+ * Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
  */
 export class LRUCache implements Map<string, string> {
   private readonly cache = new Map<string, string>();

src/client/eppo-client.ts

@@ -5,7 +5,7 @@ import {
   AssignmentCache,
   LRUInMemoryAssignmentCache,
   NonExpiringInMemoryAssignmentCache,
-} from '../cache/assignment-cache';
+} from '../cache/abstract-assignment-cache';
 import { IConfigurationStore } from '../configuration-store/configuration-store';
 import {
   BASE_URL as DEFAULT_BASE_URL,
@@ -540,17 +540,17 @@ export default class EppoClient implements IEppoClient {
     }
 
     // assignment logger may be null while waiting for initialization
-    if (this.assignmentLogger == null) {
+    if (!this.assignmentLogger) {
       this.queuedEvents.length < MAX_EVENT_QUEUE_SIZE && this.queuedEvents.push(event);
       return;
     }
     try {
       this.assignmentLogger.logAssignment(event);
       this.assignmentCache?.set({
-        flagKey: flagKey,
-        subjectKey: result.subjectKey,
-        allocationKey: result.allocationKey ?? '__eppo_no_allocation',
-        variationKey: result.variation?.key ?? '__eppo_no_variation',
+        flagKey,
+        subjectKey,
+        allocationKey: allocationKey ?? '__eppo_no_allocation',
+        variationKey: variation?.key ?? '__eppo_no_variation',
       });
     } catch (error) {
       logger.error(`[Eppo SDK] Error logging assignment event: ${error.message}`);

src/configuration-store/hybrid.store.ts

@@ -4,8 +4,8 @@ import { IAsyncStore, IConfigurationStore, ISyncStore } from './configuration-st
 
 export class HybridConfigurationStore<T> implements IConfigurationStore<T> {
   constructor(
-    private readonly servingStore: ISyncStore<T>,
-    private readonly persistentStore: IAsyncStore<T> | null,
+    protected readonly servingStore: ISyncStore<T>,
+    protected readonly persistentStore: IAsyncStore<T> | null,
   ) {}
 
   /**

src/index.ts

@@ -7,7 +7,12 @@ import {
   NonExpiringInMemoryAssignmentCache,
   LRUInMemoryAssignmentCache,
   AsyncMap,
-} from './cache/assignment-cache';
+  AssignmentCacheKey,
+  AssignmentCacheValue,
+  AssignmentCacheEntry,
+  assignmentCacheKeyToString,
+  assignmentCacheValueToString,
+} from './cache/abstract-assignment-cache';
 import EppoClient, { FlagConfigurationRequestParameters, IEppoClient } from './client/eppo-client';
 import {
   IConfigurationStore,
@@ -45,10 +50,15 @@ export {
   MemoryOnlyConfigurationStore,
 
   // Assignment cache
+  AssignmentCacheKey,
+  AssignmentCacheValue,
+  AssignmentCacheEntry,
   AssignmentCache,
   AsyncMap,
   NonExpiringInMemoryAssignmentCache,
   LRUInMemoryAssignmentCache,
+  assignmentCacheKeyToString,
+  assignmentCacheValueToString,
 
   // Interfaces
   FlagConfigurationRequestParameters,