[entity][entity-cache-adapter-redis] move transformer responsibility …

…to cache adapter

packages/entity-cache-adapter-redis/src/RedisCacheAdapter.ts

@@ -4,6 +4,9 @@ import {
   CacheLoadResult,
   FieldTransformerMap,
   mapKeys,
+  transformCacheObjectToFields,
+  CacheStatus,
+  transformFieldsToCacheObject,
 } from '@expo/entity';
 import invariant from 'invariant';
 import { Redis } from 'ioredis';
@@ -49,6 +52,12 @@ export interface RedisCacheAdapterContext {
 }
 
 export default class RedisCacheAdapter<TFields> extends EntityCacheAdapter<TFields> {
+  /**
+   * Transformer definitions for field types. Used to modify values as they are read from or written to
+   * the cache. Override in concrete subclasses to change transformation behavior.
+   * If a field type is not present in the map, then fields of that type will not be transformed.
+   */
+  private readonly redisFieldTransformer: FieldTransformerMap = redisTransformerMap;
   private readonly genericRedisCacher: GenericRedisCacher;
   constructor(
     entityConfiguration: EntityConfiguration<TFields>,
@@ -63,20 +72,31 @@ export default class RedisCacheAdapter<TFields> extends EntityCacheAdapter<TFiel
     });
   }
 
-  public getFieldTransformerMap(): FieldTransformerMap {
-    return redisTransformerMap;
-  }
-
   public async loadManyAsync<N extends keyof TFields>(
     fieldName: N,
     fieldValues: readonly NonNullable<TFields[N]>[]
   ): Promise<ReadonlyMap<NonNullable<TFields[N]>, CacheLoadResult>> {
     const redisCacheKeyToFieldValueMapping = new Map(
       fieldValues.map((fieldValue) => [this.makeCacheKey(fieldName, fieldValue), fieldValue])
     );
-    const cacheResults = await this.genericRedisCacher.loadManyAsync(
+    const rawCacheResults = await this.genericRedisCacher.loadManyAsync(
       Array.from(redisCacheKeyToFieldValueMapping.keys())
     );
+    const cacheResults = new Map<string, CacheLoadResult>();
+    for (const [redisCacheKey, rawCacheResult] of rawCacheResults) {
+      if (rawCacheResult.status === CacheStatus.HIT) {
+        cacheResults.set(redisCacheKey, {
+          status: CacheStatus.HIT,
+          item: transformCacheObjectToFields(
+            this.entityConfiguration,
+            this.redisFieldTransformer,
+            rawCacheResult.item
+          ),
+        });
+      } else {
+        cacheResults.set(redisCacheKey, rawCacheResult);
+      }
+    }
 
     return mapKeys(cacheResults, (redisCacheKey) => {
       const fieldValue = redisCacheKeyToFieldValueMapping.get(redisCacheKey);
@@ -93,8 +113,19 @@ export default class RedisCacheAdapter<TFields> extends EntityCacheAdapter<TFiel
     fieldName: N,
     objectMap: ReadonlyMap<NonNullable<TFields[N]>, object>
   ): Promise<void> {
+    const cacheObjectMap = new Map<NonNullable<TFields[N]>, object>();
+    for (const [fieldValue, object] of objectMap) {
+      cacheObjectMap.set(
+        fieldValue,
+        transformFieldsToCacheObject(
+          this.entityConfiguration,
+          this.redisFieldTransformer,
+          object as Readonly<TFields>
+        )
+      );
+    }
     await this.genericRedisCacher.cacheManyAsync(
-      mapKeys(objectMap, (fieldValue) => this.makeCacheKey(fieldName, fieldValue))
+      mapKeys(cacheObjectMap, (fieldValue) => this.makeCacheKey(fieldName, fieldValue))
     );
   }
 

packages/entity-cache-adapter-redis/src/__tests__/RedisCacheAdapter-test.ts

@@ -1,18 +1,20 @@
-import { CacheStatus, UUIDField, EntityConfiguration } from '@expo/entity';
+import { CacheStatus, UUIDField, EntityConfiguration, DateField } from '@expo/entity';
 import { Redis, Pipeline } from 'ioredis';
 import { mock, when, instance, anything, verify } from 'ts-mockito';
 
 import RedisCacheAdapter from '../RedisCacheAdapter';
 
 type BlahFields = {
   id: string;
+  date: Date | null;
 };
 
 const entityConfiguration = new EntityConfiguration<BlahFields>({
   idField: 'id',
   tableName: 'blah',
   schema: {
     id: new UUIDField({ columnName: 'id', cache: true }),
+    date: new DateField({ columnName: 'date' }),
   },
   databaseAdapterFlavor: 'postgres',
   cacheAdapterFlavor: 'redis',
@@ -62,6 +64,40 @@ describe(RedisCacheAdapter, () => {
       const results = await cacheAdapter.loadManyAsync('id', []);
       expect(results).toEqual(new Map());
     });
+
+    it('transforms a cache object to an Entity object', async () => {
+      const redisResults = new Map();
+
+      const mockRedisClient = mock<Redis>();
+
+      // need to have one anything() for each element of spread, in this case 1
+      when(mockRedisClient.mget(anything())).thenCall(async (...keys) =>
+        keys.map((k) => redisResults.get(k) ?? null)
+      );
+
+      const cacheAdapter = new RedisCacheAdapter(entityConfiguration, {
+        redisClient: instance(mockRedisClient),
+        makeKeyFn: (...parts) => parts.join(':'),
+        cacheKeyVersion: 1,
+        cacheKeyPrefix: 'hello-',
+        ttlSecondsPositive: 1,
+        ttlSecondsNegative: 2,
+      });
+
+      const date = new Date();
+      redisResults.set(
+        cacheAdapter['makeCacheKey']('id', 'wat'),
+        JSON.stringify({ id: 'wat', date: date.toISOString() })
+      );
+
+      const results = await cacheAdapter.loadManyAsync('id', ['wat']);
+
+      expect(results.get('wat')).toMatchObject({
+        status: CacheStatus.HIT,
+        item: { id: 'wat', date },
+      });
+      expect(results.size).toBe(1);
+    });
   });
 
   describe('cacheManyAsync', () => {
@@ -98,6 +134,41 @@ describe(RedisCacheAdapter, () => {
         ttl: 1,
       });
     });
+
+    it('transforms an Entity object to a cache object', async () => {
+      const redisResults = new Map();
+
+      const mockPipeline = mock<Pipeline>();
+      when(mockPipeline.set(anything(), anything(), anything(), anything())).thenCall(
+        (key, value, code, ttl) => {
+          redisResults.set(key, { value, code, ttl });
+          return pipeline;
+        }
+      );
+      when(mockPipeline.exec()).thenResolve({} as any);
+      const pipeline = instance(mockPipeline);
+
+      const mockRedisClient = mock<Redis>();
+      when(mockRedisClient.multi()).thenReturn(pipeline);
+
+      const cacheAdapter = new RedisCacheAdapter(entityConfiguration, {
+        redisClient: instance(mockRedisClient),
+        makeKeyFn: (...parts) => parts.join(':'),
+        cacheKeyVersion: 1,
+        cacheKeyPrefix: 'hello-',
+        ttlSecondsPositive: 1,
+        ttlSecondsNegative: 2,
+      });
+      const date = new Date();
+      await cacheAdapter.cacheManyAsync('id', new Map([['wat', { id: 'wat', date }]]));
+
+      const cacheKey = cacheAdapter['makeCacheKey']('id', 'wat');
+      expect(redisResults.get(cacheKey)).toMatchObject({
+        value: JSON.stringify({ id: 'wat', date: date.toISOString() }),
+        code: 'EX',
+        ttl: 1,
+      });
+    });
   });
 
   describe('cacheDBMissesAsync', () => {

packages/entity/src/EntityCacheAdapter.ts

@@ -1,5 +1,4 @@
 import EntityConfiguration from './EntityConfiguration';
-import { FieldTransformerMap } from './internal/EntityFieldTransformationUtils';
 import { CacheLoadResult } from './internal/ReadThroughEntityCache';
 
 /**
@@ -9,13 +8,6 @@ import { CacheLoadResult } from './internal/ReadThroughEntityCache';
 export default abstract class EntityCacheAdapter<TFields> {
   constructor(protected readonly entityConfiguration: EntityConfiguration<TFields>) {}
 
-  /**
-   * Transformer definitions for field types. Used to modify values as they are read from or written to
-   * the cache. Override in concrete subclasses to change transformation behavior.
-   * If a field type is not present in the map, then fields of that type will not be transformed.
-   */
-  public abstract getFieldTransformerMap(): FieldTransformerMap;
-
   /**
    * Load many objects from cache.
    * @param fieldName - object field being queried

packages/entity/src/internal/ReadThroughEntityCache.ts

@@ -3,11 +3,6 @@ import invariant from 'invariant';
 import EntityCacheAdapter from '../EntityCacheAdapter';
 import EntityConfiguration from '../EntityConfiguration';
 import { filterMap } from '../utils/collections/maps';
-import {
-  FieldTransformerMap,
-  transformCacheObjectToFields,
-  transformFieldsToCacheObject,
-} from './EntityFieldTransformationUtils';
 
 export enum CacheStatus {
   HIT,
@@ -32,14 +27,10 @@ export type CacheLoadResult =
  * {@link EntityCacheAdapter} within the {@link EntityDataManager}.
  */
 export default class ReadThroughEntityCache<TFields> {
-  private readonly fieldTransformerMap: FieldTransformerMap;
-
   constructor(
     private readonly entityConfiguration: EntityConfiguration<TFields>,
     private readonly entityCacheAdapter: EntityCacheAdapter<TFields>
-  ) {
-    this.fieldTransformerMap = entityCacheAdapter.getFieldTransformerMap();
-  }
+  ) {}
 
   private isFieldCacheable<N extends keyof TFields>(fieldName: N): boolean {
     return this.entityConfiguration.cacheableKeys.has(fieldName);
@@ -91,13 +82,7 @@ export default class ReadThroughEntityCache<TFields> {
     const results: Map<NonNullable<TFields[N]>, readonly Readonly<TFields>[]> = new Map();
     cacheLoadResults.forEach((cacheLoadResult, fieldValue) => {
       if (cacheLoadResult.status === CacheStatus.HIT) {
-        results.set(fieldValue, [
-          transformCacheObjectToFields(
-            this.entityConfiguration,
-            this.fieldTransformerMap,
-            cacheLoadResult.item
-          ),
-        ]);
+        results.set(fieldValue, [cacheLoadResult.item as Readonly<TFields>]);
       }
     });
 
@@ -123,14 +108,7 @@ export default class ReadThroughEntityCache<TFields> {
         }
         const uniqueObject = objects[0];
         if (uniqueObject) {
-          objectsToCache.set(
-            fieldValue,
-            transformFieldsToCacheObject(
-              this.entityConfiguration,
-              this.fieldTransformerMap,
-              uniqueObject
-            )
-          );
+          objectsToCache.set(fieldValue, uniqueObject);
           results.set(fieldValue, [uniqueObject]);
         }
       }

packages/entity/src/internal/__tests__/ReadThroughEntityCache-test.ts

@@ -40,7 +40,6 @@ describe(ReadThroughEntityCache, () => {
   describe('readManyThroughAsync', () => {
     it('fetches from DB upon cache miss and caches the result', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(true), cacheAdapter);
       const fetcher = createIdFetcher(['wat', 'who']);
@@ -77,7 +76,6 @@ describe(ReadThroughEntityCache, () => {
 
     it('does not fetch from the DB or cache results when all cache fetches are hits', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(true), cacheAdapter);
       const fetcher = createIdFetcher(['wat', 'who']);
@@ -114,7 +112,6 @@ describe(ReadThroughEntityCache, () => {
 
     it('negatively caches db misses', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(true), cacheAdapter);
 
@@ -135,7 +132,6 @@ describe(ReadThroughEntityCache, () => {
 
     it('does not return or fetch negatively cached results from DB', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(true), cacheAdapter);
       const fetcher = createIdFetcher([]);
@@ -153,7 +149,6 @@ describe(ReadThroughEntityCache, () => {
 
     it('does a mix and match of hit, miss, and negative', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(true), cacheAdapter);
       const fetcher = createIdFetcher(['wat', 'who', 'why']);
@@ -189,7 +184,6 @@ describe(ReadThroughEntityCache, () => {
 
     it('does not call into cache for field that is not cacheable', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
-      when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(new Map());
       const cacheAdapter = instance(cacheAdapterMock);
       const entityCache = new ReadThroughEntityCache(makeEntityConfiguration(false), cacheAdapter);
       const fetcher = createIdFetcher(['wat']);
@@ -198,7 +192,7 @@ describe(ReadThroughEntityCache, () => {
       expect(result).toEqual(new Map([['wat', [{ id: 'wat' }]]]));
     });
 
-    it('transforms fields for cache storage', async () => {
+    /*     it('transforms fields for cache storage', async () => {
       const cacheAdapterMock = mock<EntityCacheAdapter<BlahFields>>();
       when(cacheAdapterMock.getFieldTransformerMap()).thenReturn(
         new Map([
@@ -234,7 +228,7 @@ describe(ReadThroughEntityCache, () => {
           ['who', [{ id: 'who' }]],
         ])
       );
-    });
+    }); */
   });
 
   describe('invalidateManyAsync', () => {