Repository: ViacomInc/data-point
Pull request #407 (2019-08-29 15:33:41)

Refactor cache api
<!--
Thanks for your interest in the project. We appreciate bugs filed and PRs submitted!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!

BREAKING CHANGES:

If your PR includes a breaking change, please submit it with a codemod under
data-point-codemods that will help users upgrade their codebase.

Breaking changes without a codemod will not be accepted unless a codemod is not
viable or does not apply to the specific situation.
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**:

Changes cache api: 
- `get(acc)` to `get(uid, acc)`
- `set(acc)` to `set(uid, acc)`

<!-- Why are these changes necessary? -->
**Why**:

this change moves away from attaching the uid to the accumulator which would cause confusion on the relationship of the `accumulator` and the `uid` property 

<!-- Have you done all of these things?  -->
**Checklist**:
<!-- add "N/A" to the end of each line that's irrelevant to your changes -->
<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->
- [ ] Has Breaking changes
- [ ] Documentation
- [X] Tests
- [X] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [ ] Added username to **all-contributors** list

<!-- feel free to add additional comments -->

codecov[bot]: # [Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=h1) Report
> Merging [#407](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=desc) into [canary](https://codecov.io/gh/ViacomInc/data-point/commit/2fd412e423059b80e1560f0aa980b4ee36b52dd1?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/ViacomInc/data-point/pull/407/graphs/tree.svg?width=650&token=LYxItnrx4i&height=150&src=pr)](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           canary   #407   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          24     24           
  Lines         388    388           
  Branches       57     57           
=====================================
  Hits          388    388
```


| [Impacted Files](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/data-point/src/ReducerEntity.js](https://codecov.io/gh/ViacomInc/data-point/pull/407/diff?src=pr&el=tree#diff-cGFja2FnZXMvZGF0YS1wb2ludC9zcmMvUmVkdWNlckVudGl0eS5qcw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=footer). Last update [2fd412e...04fa346](https://codecov.io/gh/ViacomInc/data-point/pull/407?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/playground-entity.js b/docs/playground-entity.js
index 399fd7a..dc1c8e6 100644
--- a/docs/playground-entity.js
+++ b/docs/playground-entity.js
@@ -43,12 +43,12 @@ async function main() {
 
   const store = new Map();
 
-  datapoint.cache.get = acc => {
-    return store.get(acc.uid);
+  datapoint.cache.get = (uid, acc) => {
+    return store.get(uid);
   };
 
-  datapoint.cache.set = acc => {
-    return store.set(acc.uid, acc.value);
+  datapoint.cache.set = (uid, acc) => {
+    return store.set(uid, acc.value);
   };
 
   const input = [];
diff --git a/docs/playground.js b/docs/playground.js
index 141ac54..bda67dd 100644
--- a/docs/playground.js
+++ b/docs/playground.js
@@ -44,12 +44,12 @@ async function main() {
 
   const store = new Map();
 
-  datapoint.cache.get = acc => {
-    return store.get(acc.uid);
+  datapoint.cache.get = (uid, acc) => {
+    return store.get(uid);
   };
 
-  datapoint.cache.set = acc => {
-    return store.set(acc.uid, acc.value);
+  datapoint.cache.set = (uid, acc) => {
+    return store.set(uid, acc.value);
   };
 
   const input = [
diff --git a/packages/data-point/src/ReducerEntity.js b/packages/data-point/src/ReducerEntity.js
index 28f2581..5fca7aa 100644
--- a/packages/data-point/src/ReducerEntity.js
+++ b/packages/data-point/src/ReducerEntity.js
@@ -32,7 +32,7 @@ class ReducerEntity extends Reducer {
   async resolveEntityValue(accumulator, resolveReducer) {
     let acc = accumulator;
 
-    acc.uid = this.uid ? this.uid(acc) : undefined;
+    const generatedUId = this.uid ? this.uid(acc) : undefined;
 
     if (this.inputType) {
       await resolveReducer(acc, this.inputType);
@@ -41,7 +41,7 @@ class ReducerEntity extends Reducer {
     const cache = accumulator.cache;
 
     if (typeof cache.get === "function") {
-      const cacheResult = await cache.get(acc);
+      const cacheResult = await cache.get(generatedUId, acc);
       if (cacheResult !== undefined) {
         return cacheResult;
       }
@@ -68,7 +68,7 @@ class ReducerEntity extends Reducer {
     }
 
     if (typeof cache.set === "function") {
-      await cache.set(acc);
+      await cache.set(generatedUId, acc);
     }
 
     return acc.value;


Test patch:
diff --git a/packages/data-point/src/ReducerEntity.test.js b/packages/data-point/src/ReducerEntity.test.js
index 082736a..4de8b2a 100644
--- a/packages/data-point/src/ReducerEntity.test.js
+++ b/packages/data-point/src/ReducerEntity.test.js
@@ -248,20 +248,6 @@ describe("ReducerEntity", () => {
       expect(result).toEqual("initialValue");
     });
 
-    it("should assign acc.uid", async () => {
-      const entity = new ReducerEntity("type", {
-        uid: () => "uid",
-        name: "customEntity"
-      });
-
-      const acc = new Accumulator({
-        value: "initialValue"
-      });
-
-      await entity.resolveEntityValue(acc, mockResolveReducer);
-      expect(acc.uid).toEqual("uid");
-    });
-
     it("should execute inputType but not mutate result", async () => {
       const inputType = jest.fn(() => "inputType");
       const entity = new ReducerEntity("type", {
@@ -280,8 +266,10 @@ describe("ReducerEntity", () => {
 
     describe("cache", () => {
       it("should skip cache if cache.get is not a function", async () => {
+        const mockBefore = jest.fn(value => value);
         const entity = new ReducerEntity("type", {
-          name: "customEntity"
+          name: "customEntity",
+          before: mockBefore
         });
 
         const cache = {
@@ -294,16 +282,37 @@ describe("ReducerEntity", () => {
         });
 
         const result = await entity.resolveEntityValue(acc, mockResolveReducer);
+        expect(mockBefore).toBeCalled();
         expect(result).toEqual("initialValue");
       });
 
-      it("should use cache result if cache.get returns !== undefined", async () => {
+      it("should call this.uid when assigned", async () => {
+        const mockUid = jest.fn(() => "uid");
         const entity = new ReducerEntity("type", {
+          uid: mockUid,
           name: "customEntity"
         });
 
+        const acc = new Accumulator({
+          value: "initialValue"
+        });
+
+        await entity.resolveEntityValue(acc, mockResolveReducer);
+        expect(mockUid).toBeCalledWith(acc);
+      });
+
+      it("should use cache result if cache.get returns !== undefined", async () => {
+        const mockUid = jest.fn(() => "uid");
+        const mockBefore = jest.fn(value => value);
+
+        const entity = new ReducerEntity("type", {
+          uid: mockUid,
+          name: "customEntity",
+          before: mockBefore
+        });
+
         const cache = {
-          get: () => "cachedResult"
+          get: jest.fn(() => "cachedResult")
         };
 
         const acc = new Accumulator({
@@ -312,12 +321,16 @@ describe("ReducerEntity", () => {
         });
 
         const result = await entity.resolveEntityValue(acc, mockResolveReducer);
+        expect(cache.get).toBeCalledWith("uid", acc);
+        expect(mockBefore).not.toBeCalled();
         expect(result).toEqual("cachedResult");
       });
 
       it("should ignore cache result if cache.get returns undefined", async () => {
+        const mockBefore = jest.fn(value => value);
         const entity = new ReducerEntity("type", {
-          name: "customEntity"
+          name: "customEntity",
+          before: mockBefore
         });
 
         const cache = {
@@ -330,6 +343,7 @@ describe("ReducerEntity", () => {
         });
 
         const result = await entity.resolveEntityValue(acc, mockResolveReducer);
+        expect(mockBefore).toBeCalled();
         expect(result).toEqual("initialValue");
       });