Repository: ViacomInc/data-point
Pull request #124 (2018-01-05 20:12:58)

Normalize reducer resolve function signatures
<!--
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!
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**: #121

<!-- Why are these changes necessary? -->
**Why**: Adds more consistency and will make it easier to support an API for custom reducer types.

@acatl This commit is the only one that's relevant when squashing this PR:

"refactor(reducer-types): Normalize the reducer resolve functions"

(and it might be better to say "Normalize the reducer resolve function signatures")

<!-- 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" -->
- [ ] Documentation n/a
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
  

coveralls: 
[![Coverage Status](https://coveralls.io/builds/14922854/badge)](https://coveralls.io/builds/14922854)

Coverage remained the same at 100.0% when pulling **03504bf9c7f5a7fc5aa7dbf4adf8949c784640df on raingerber:normalize-reducer-resolve-functions** into **c7edec115e953e01e9247bcd9b5477cf1fda247b on ViacomInc:master**.


Patch:
diff --git a/packages/data-point/lib/reducer-types/reducer-entity/resolve.js b/packages/data-point/lib/reducer-types/reducer-entity/resolve.js
index 2f1f84d..1a96a15 100644
--- a/packages/data-point/lib/reducer-types/reducer-entity/resolve.js
+++ b/packages/data-point/lib/reducer-types/reducer-entity/resolve.js
@@ -8,17 +8,17 @@ const BaseEntity = require('../../entity-types/base-entity/resolve')
  * @param {Object} manager
  * @param {function} resolveReducer
  * @param {Accumulator} accumulator
- * @param {ReducerEntity} reducerEntity
+ * @param {ReducerEntity} reducer
  * @returns {Promise<Accumulator>}
  */
-function resolve (manager, resolveReducer, accumulator, reducerEntity) {
-  const reducerEntityType = reducerEntity.entityType
+function resolve (manager, resolveReducer, accumulator, reducer) {
+  const reducerEntityType = reducer.entityType
   const EntityType = manager.entityTypes.get(reducerEntityType)
   return BaseEntity.resolve(
     manager,
     resolveReducer,
     accumulator,
-    reducerEntity,
+    reducer,
     EntityType.resolve
   )
 }
diff --git a/packages/data-point/lib/reducer-types/reducer-function/resolve.js b/packages/data-point/lib/reducer-types/reducer-function/resolve.js
index 2e2cbba..d744825 100644
--- a/packages/data-point/lib/reducer-types/reducer-function/resolve.js
+++ b/packages/data-point/lib/reducer-types/reducer-function/resolve.js
@@ -4,12 +4,13 @@ const Promise = require('bluebird')
 const utils = require('../../utils')
 
 /**
- * Resolve a ReducerFunction
+ * @param {Object} manager
+ * @param {Function} resolveReducer
  * @param {Accumulator} accumulator
- * @param {ReducerFunction} reducer
+ * @param {ReducerObject} reducer
  * @returns {Promise<Accumulator>}
  */
-function resolve (accumulator, reducer) {
+function resolve (manager, resolveReducer, accumulator, reducer) {
   const callbackFunction = reducer.body
 
   if (callbackFunction.length === 2) {
diff --git a/packages/data-point/lib/reducer-types/reducer-path/resolve.js b/packages/data-point/lib/reducer-types/reducer-path/resolve.js
index 8d7420c..a173772 100644
--- a/packages/data-point/lib/reducer-types/reducer-path/resolve.js
+++ b/packages/data-point/lib/reducer-types/reducer-path/resolve.js
@@ -3,12 +3,14 @@
 const utils = require('../../utils')
 
 /**
- * @param {Accumulator} accumulator - current accumulator
- * @param {ReducerPath} reducerPath
+ * @param {Object} manager
+ * @param {Function} resolveReducer
+ * @param {Accumulator} accumulator
+ * @param {ReducerPath} reducer
  * @returns {Promise<Accumulator>}
  */
-function resolve (accumulator, reducerPath) {
-  const value = reducerPath.body(accumulator)
+function resolve (manager, resolveReducer, accumulator, reducer) {
+  const value = reducer.body(accumulator)
   const acc = utils.set(accumulator, 'value', value)
   return Promise.resolve(acc)
 }
diff --git a/packages/data-point/lib/reducer-types/resolve.js b/packages/data-point/lib/reducer-types/resolve.js
index 3a2679a..042e1c8 100644
--- a/packages/data-point/lib/reducer-types/resolve.js
+++ b/packages/data-point/lib/reducer-types/resolve.js
@@ -1,49 +1,17 @@
-const partial = require('lodash/partial')
-
-const resolveReducerEntity = require('./reducer-entity').resolve
-const resolveReducerFunction = require('./reducer-function').resolve
-const resolveReducerList = require('./reducer-list').resolve
-const resolveReducerObject = require('./reducer-object').resolve
-const resolveReducerPath = require('./reducer-path').resolve
-
-/**
- * @param {Object} manager
- * @param {Function} resolveReducer
- * @param {string} reducerType
- * @returns
- */
-function getReducerFunction (manager, resolveReducer, reducerType) {
-  let resolver
-
-  /* eslint indent: ["error", 2, { "SwitchCase": 1 }] */
-  switch (reducerType) {
-    case 'ReducerPath':
-      resolver = resolveReducerPath
-      break
-    case 'ReducerFunction':
-      resolver = resolveReducerFunction
-      break
-    case 'ReducerObject':
-      // NOTE: recursive call
-      resolver = partial(resolveReducerObject, manager, resolveReducer)
-      break
-    case 'ReducerEntity':
-      // NOTE: recursive call
-      resolver = partial(resolveReducerEntity, manager, resolveReducer)
-      break
-    case 'ReducerList':
-      // NOTE: recursive call
-      resolver = partial(resolveReducerList, manager, resolveReducer)
-      break
-    default:
-      throw new Error(`Reducer type '${reducerType}' was not recognized`)
-  }
-
-  return resolver
+const ReducerEntity = require('./reducer-entity')
+const ReducerFunction = require('./reducer-function')
+const ReducerList = require('./reducer-list')
+const ReducerObject = require('./reducer-object')
+const ReducerPath = require('./reducer-path')
+
+const resolveFunctions = {
+  [ReducerEntity.type]: ReducerEntity.resolve,
+  [ReducerFunction.type]: ReducerFunction.resolve,
+  [ReducerList.type]: ReducerList.resolve,
+  [ReducerObject.type]: ReducerObject.resolve,
+  [ReducerPath.type]: ReducerPath.resolve
 }
 
-module.exports.getReducerFunction = getReducerFunction
-
 /**
  * apply a Reducer to an accumulator
  * @param {Object} manager
@@ -52,13 +20,20 @@ module.exports.getReducerFunction = getReducerFunction
  * @returns {Promise<Accumulator>}
  */
 function resolve (manager, accumulator, reducer) {
+  // this conditional is here because BaseEntity#resolve
+  // does not check that lifecycle methods are defined
+  // before trying to resolve them
   if (!reducer) {
     return Promise.resolve(accumulator)
   }
 
-  const reducerFunction = getReducerFunction(manager, resolve, reducer.type)
-  const result = reducerFunction(accumulator, reducer)
-  return result
+  const resolveReducer = resolveFunctions[reducer.type]
+  if (!resolveReducer) {
+    throw new Error(`Reducer type '${reducer.type}' was not recognized`)
+  }
+
+  // NOTE: recursive call
+  return resolveReducer(manager, resolve, accumulator, reducer)
 }
 
 module.exports.resolve = resolve


Test patch:
diff --git a/packages/data-point/lib/reducer-types/__snapshots__/resolve.test.js.snap b/packages/data-point/lib/reducer-types/__snapshots__/resolve.test.js.snap
new file mode 100644
index 0000000..c969349
--- /dev/null
+++ b/packages/data-point/lib/reducer-types/__snapshots__/resolve.test.js.snap
@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`reducer#resolve It should throw error for invalid input 1`] = `"Reducer type 'INVALID TYPE' was not recognized"`;
diff --git a/packages/data-point/lib/reducer-types/reducer-function/resolve.test.js b/packages/data-point/lib/reducer-types/reducer-function/resolve.test.js
index 8d3e485..0732314 100644
--- a/packages/data-point/lib/reducer-types/reducer-function/resolve.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-function/resolve.test.js
@@ -1,9 +1,17 @@
 /* eslint-env jest */
 'use strict'
 
-const AccumulatorFactory = require('../../accumulator/factory')
 const reducerFactory = require('../factory')
 const resolveFunction = require('./resolve')
+const resolveReducer = require('../index').resolve
+const AccumulatorFactory = require('../../accumulator/factory')
+const FixtureStore = require('../../../test/utils/fixture-store')
+
+let dataPoint
+
+beforeAll(() => {
+  dataPoint = FixtureStore.create()
+})
 
 describe('resolve#filter.resolve', () => {
   test('resolves node style callback', () => {
@@ -15,9 +23,11 @@ describe('resolve#filter.resolve', () => {
       done(null, `${acc.value}node`)
     )
 
-    return resolveFunction.resolve(accumulator, reducer).then(result => {
-      expect(result.value).toBe('testnode')
-    })
+    return resolveFunction
+      .resolve(dataPoint, resolveReducer, accumulator, reducer)
+      .then(result => {
+        expect(result.value).toBe('testnode')
+      })
   })
 
   test('resolves a sync function', () => {
@@ -27,9 +37,11 @@ describe('resolve#filter.resolve', () => {
 
     const reducer = reducerFactory.create(acc => `${acc.value}sync`)
 
-    return resolveFunction.resolve(accumulator, reducer).then(result => {
-      expect(result.value).toBe('testsync')
-    })
+    return resolveFunction
+      .resolve(dataPoint, resolveReducer, accumulator, reducer)
+      .then(result => {
+        expect(result.value).toBe('testsync')
+      })
   })
 
   test('resolves a promise function', () => {
@@ -41,9 +53,11 @@ describe('resolve#filter.resolve', () => {
       Promise.resolve(`${acc.value}promise`)
     )
 
-    return resolveFunction.resolve(accumulator, reducer).then(result => {
-      expect(result.value).toBe('testpromise')
-    })
+    return resolveFunction
+      .resolve(dataPoint, resolveReducer, accumulator, reducer)
+      .then(result => {
+        expect(result.value).toBe('testpromise')
+      })
   })
 
   test('rejects if callback passes error as first param', () => {
@@ -56,7 +70,7 @@ describe('resolve#filter.resolve', () => {
     })
 
     return resolveFunction
-      .resolve(accumulator, reducer)
+      .resolve(dataPoint, resolveReducer, accumulator, reducer)
       .catch(err => err)
       .then(err => {
         expect(err).toHaveProperty('message', 'Test')
diff --git a/packages/data-point/lib/reducer-types/reducer-path/resolve.test.js b/packages/data-point/lib/reducer-types/reducer-path/resolve.test.js
index c6f1f44..99d60ad 100644
--- a/packages/data-point/lib/reducer-types/reducer-path/resolve.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-path/resolve.test.js
@@ -2,7 +2,15 @@
 'use strict'
 
 const reducerPath = require('./index')
+const resolveReducer = require('../index').resolve
 const AccumulatorFactory = require('../../accumulator/factory')
+const FixtureStore = require('../../../test/utils/fixture-store')
+
+let dataPoint
+
+beforeAll(() => {
+  dataPoint = FixtureStore.create()
+})
 
 describe('ReducerPath#resolve', () => {
   function resolve (value, reducerSource) {
@@ -14,7 +22,8 @@ describe('ReducerPath#resolve', () => {
       locals
     })
 
-    return reducerPath.resolve(accumulator, reducerPath.create(reducerSource))
+    const reducer = reducerPath.create(reducerSource)
+    return reducerPath.resolve(dataPoint, resolveReducer, accumulator, reducer)
   }
 
   test('resolve current value', () => {
diff --git a/packages/data-point/lib/reducer-types/resolve.test.js b/packages/data-point/lib/reducer-types/resolve.test.js
index 6aafbc5..1b1429d 100644
--- a/packages/data-point/lib/reducer-types/resolve.test.js
+++ b/packages/data-point/lib/reducer-types/resolve.test.js
@@ -5,7 +5,6 @@ const Factory = require('./factory')
 const Resolve = require('./resolve')
 
 const reducers = require('../../test/utils/reducers')
-const resolveReducerList = require('./reducer-list').resolve
 
 const fixtureStore = require('../../test/utils/fixture-store')
 const testData = require('../../test/data.json')
@@ -18,45 +17,26 @@ beforeAll(() => {
   manager = fixtureStore.create()
 })
 
-describe('reducer.getReducerFunction', () => {
-  test('resolve to ReducerPath', () => {
-    const resolver = Resolve.getReducerFunction(
-      manager,
-      resolveReducerList,
-      'ReducerPath'
-    )
-    expect(resolver).toBeInstanceOf(Function)
-  })
-  test('resolve to ReducerFunction', () => {
-    const resolver = Resolve.getReducerFunction(
-      manager,
-      resolveReducerList,
-      'ReducerFunction'
-    )
-    expect(resolver).toBeInstanceOf(Function)
-  })
-  test('resolve to ReducerEntity', () => {
-    const resolver = Resolve.getReducerFunction(
-      manager,
-      resolveReducerList,
-      'ReducerEntity'
-    )
-    expect(resolver).toBeInstanceOf(Function)
-  })
-  test('resolve to ReducerEntity', () => {
-    expect(() => {
-      Resolve.getReducerFunction(manager, resolveReducerList, 'INVALID TYPE')
-    }).toThrow()
-  })
-})
+describe('reducer#resolve', () => {
+  test('It should work for valid input', () => {
+    const accumulator = AccumulatorFactory.create({
+      value: testData.a.b.c
+    })
 
-test('resolve#reducer.resolve', () => {
-  const accumulator = AccumulatorFactory.create({
-    value: testData.a.b.c
+    const reducer = Factory.create(reducers.addCollectionValues())
+    return Resolve.resolve(manager, accumulator, reducer).then(result => {
+      expect(result.value).toEqual(6)
+    })
   })
 
-  const reducer = Factory.create(reducers.addCollectionValues())
-  return Resolve.resolve(manager, accumulator, reducer).then(result => {
-    expect(result.value).toEqual(6)
+  test('It should throw error for invalid input', () => {
+    const accumulator = AccumulatorFactory.create({
+      value: testData.a.b.c
+    })
+
+    const reducer = { type: 'INVALID TYPE' }
+    expect(() => {
+      Resolve.resolve(manager, accumulator, reducer)
+    }).toThrowErrorMatchingSnapshot()
   })
 })