Repository: ViacomInc/data-point
Pull request #127 (2018-01-08 18:16:37)

Normalize reducer factory function signatures
**What**: The reducer factories all take (createReducer, source) as parameters now.

<!-- Why are these changes necessary? -->
**Why**: Consistency, which makes it easier to add new reducer types.

<!-- 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/14952203/badge)](https://coveralls.io/builds/14952203)

Coverage remained the same at 100.0% when pulling **ea26ed5a87a7c3591e2183791c4843c21d70cc04 on raingerber:normalize-reducer-factory-functions** into **2f14968c5340c9f9f4d6d590d057f76905126286 on ViacomInc:master**.


Patch:
diff --git a/packages/data-point/lib/reducer-types/factory.js b/packages/data-point/lib/reducer-types/factory.js
index 6a15dc3..3ff00bc 100644
--- a/packages/data-point/lib/reducer-types/factory.js
+++ b/packages/data-point/lib/reducer-types/factory.js
@@ -9,7 +9,13 @@ const ReducerList = require('./reducer-list')
 const ReducerObject = require('./reducer-object')
 const ReducerPath = require('./reducer-path')
 
-const reducerTypes = [ReducerPath, ReducerFunction, ReducerEntity]
+const reducerTypes = [
+  ReducerEntity,
+  ReducerFunction,
+  ReducerList,
+  ReducerObject,
+  ReducerPath
+]
 
 /**
  * this is here because ReducerLists can be arrays or | separated strings
@@ -34,15 +40,6 @@ function dealWithPipeOperators (source) {
  */
 function createReducer (source) {
   source = dealWithPipeOperators(source)
-
-  if (ReducerObject.isType(source)) {
-    return ReducerObject.create(createReducer, source)
-  }
-
-  if (ReducerList.isType(source)) {
-    return ReducerList.create(createReducer, source)
-  }
-
   const reducer = _.find(reducerTypes, r => r.isType(source))
 
   if (_.isUndefined(reducer)) {
@@ -57,7 +54,8 @@ function createReducer (source) {
     throw new Error(message)
   }
 
-  return reducer.create(source)
+  // NOTE: recursive call
+  return reducer.create(createReducer, source)
 }
 
 module.exports.create = createReducer
diff --git a/packages/data-point/lib/reducer-types/reducer-entity/factory.js b/packages/data-point/lib/reducer-types/reducer-entity/factory.js
index 4eae6b4..5de6d86 100644
--- a/packages/data-point/lib/reducer-types/reducer-entity/factory.js
+++ b/packages/data-point/lib/reducer-types/reducer-entity/factory.js
@@ -37,11 +37,11 @@ function isType (source) {
 module.exports.isType = isType
 
 /**
- * parse reducer
- * @param  {string} reducerRaw raw reducer path
+ * @param {Function} createReducer
+ * @param {string} source
  * @return {reducer}
  */
-function create (source) {
+function create (createReducer, source) {
   const reducer = new ReducerEntity()
   const tokens = source.split(':')
 
diff --git a/packages/data-point/lib/reducer-types/reducer-function/factory.js b/packages/data-point/lib/reducer-types/reducer-function/factory.js
index 62c00b6..34aa18b 100644
--- a/packages/data-point/lib/reducer-types/reducer-function/factory.js
+++ b/packages/data-point/lib/reducer-types/reducer-function/factory.js
@@ -49,11 +49,11 @@ function validateFunction (fn) {
 module.exports.validateFunction = validateFunction
 
 /**
- * parse reducer
- * @param  {string} reducerRaw raw reducer path
+ * @param {Function} createReducer
+ * @param {Function} source
  * @return {reducer}
  */
-function create (source) {
+function create (createReducer, source) {
   validateFunction(source)
   const reducer = new ReducerFunction()
   reducer.body = source
diff --git a/packages/data-point/lib/reducer-types/reducer-path/factory.js b/packages/data-point/lib/reducer-types/reducer-path/factory.js
index eaf8cc9..f7e689c 100644
--- a/packages/data-point/lib/reducer-types/reducer-path/factory.js
+++ b/packages/data-point/lib/reducer-types/reducer-path/factory.js
@@ -102,11 +102,11 @@ function getPathReducerFunction (jsonPath, asCollection) {
 module.exports.getPathReducerFunction = getPathReducerFunction
 
 /**
- * parse reducer
- * @param {string} source - raw reducer path
+ * @param {Function} createReducer
+ * @param {string} source
  * @return {reducer}
  */
-function create (source) {
+function create (createReducer, source) {
   const reducer = new ReducerPath()
 
   reducer.asCollection = source.slice(-2) === '[]'


Test patch:
diff --git a/packages/data-point/lib/entity-types/base-entity/resolve.test.js b/packages/data-point/lib/entity-types/base-entity/resolve.test.js
index 3fd3161..b2eda58 100644
--- a/packages/data-point/lib/entity-types/base-entity/resolve.test.js
+++ b/packages/data-point/lib/entity-types/base-entity/resolve.test.js
@@ -2,9 +2,9 @@
 'use strict'
 
 const ResolveEntity = require('./resolve')
-const createReducerEntity = require('../../reducer-types/reducer-entity').create
 const createReducer = require('../../reducer-types').create
 const resolveReducer = require('../../reducer-types').resolve
+const createReducerEntity = require('../../reducer-types/reducer-entity').create
 
 const FixtureStore = require('../../../test/utils/fixture-store')
 const helpers = require('../../helpers')
@@ -68,7 +68,7 @@ describe('ResolveEntity.resolveErrorReducers', () => {
 describe('ResolveEntity.createCurrentAccumulator', () => {
   let acc
   beforeAll(() => {
-    const reducerEntity = createReducerEntity('hash:base')
+    const reducerEntity = createReducerEntity(createReducer, 'hash:base')
     const accumulator = helpers.createAccumulator({
       foo: 'bar'
     })
@@ -136,7 +136,7 @@ describe('ResolveEntity.resolveEntity', () => {
 
   const resolveEntity = (entityId, input, options, resolver) => {
     const racc = helpers.createAccumulator.call(null, input, options)
-    const reducer = createReducerEntity(entityId)
+    const reducer = createReducerEntity(createReducer, entityId)
     return ResolveEntity.resolveEntity(
       dataPoint,
       resolveReducerBound,
@@ -203,7 +203,7 @@ describe('ResolveEntity.resolveEntity', () => {
 describe('ResolveEntity.resolve', () => {
   const resolve = resolver => (entityId, input, options) => {
     const racc = helpers.createAccumulator.call(null, input, options)
-    const reducer = createReducerEntity(entityId)
+    const reducer = createReducerEntity(createReducer, entityId)
     return ResolveEntity.resolve(
       dataPoint,
       resolveReducer,
diff --git a/packages/data-point/lib/reducer-types/reducer-entity/factory.test.js b/packages/data-point/lib/reducer-types/reducer-entity/factory.test.js
index 958b5a6..3046e7e 100644
--- a/packages/data-point/lib/reducer-types/reducer-entity/factory.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-entity/factory.test.js
@@ -2,6 +2,7 @@
 'use strict'
 
 const factory = require('./factory')
+const createReducer = require('../index').create
 
 test('reducer/reducer-entity#isType', () => {
   expect(factory.isType('a')).toBe(false)
@@ -26,7 +27,7 @@ test('reducer/reducer-entity#isType', () => {
 
 describe('create', function () {
   test('default create', () => {
-    const reducer = factory.create('foo:abc')
+    const reducer = factory.create(createReducer, 'foo:abc')
     expect(reducer.hasEmptyConditional).toBe(false)
     expect(reducer.asCollection).toBe(false)
     expect(reducer.type).toBe('ReducerEntity')
@@ -35,7 +36,7 @@ describe('create', function () {
   })
 
   test('as collection', () => {
-    const reducer = factory.create('foo:abc[]')
+    const reducer = factory.create(createReducer, 'foo:abc[]')
     expect(reducer.asCollection).toBe(true)
     expect(reducer.hasEmptyConditional).toBe(false)
     expect(reducer.type).toBe('ReducerEntity')
@@ -44,7 +45,7 @@ describe('create', function () {
   })
 
   test('with conditional', () => {
-    const reducer = factory.create('?foo:abc')
+    const reducer = factory.create(createReducer, '?foo:abc')
     expect(reducer.hasEmptyConditional).toBe(true)
     expect(reducer.asCollection).toBe(false)
     expect(reducer.type).toBe('ReducerEntity')
@@ -53,7 +54,7 @@ describe('create', function () {
   })
 
   test('with conditional and as collection', () => {
-    const reducer = factory.create('?foo:abc[]')
+    const reducer = factory.create(createReducer, '?foo:abc[]')
     expect(reducer.hasEmptyConditional).toBe(true)
     expect(reducer.asCollection).toBe(true)
     expect(reducer.type).toBe('ReducerEntity')
diff --git a/packages/data-point/lib/reducer-types/reducer-function/factory.test.js b/packages/data-point/lib/reducer-types/reducer-function/factory.test.js
index 657e3ab..ad28be5 100644
--- a/packages/data-point/lib/reducer-types/reducer-function/factory.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-function/factory.test.js
@@ -1,27 +1,28 @@
 /* eslint-env jest */
 'use strict'
 
-const rf = require('./factory')
+const factory = require('./factory')
+const createReducer = require('../index').create
 
 test('reducer/reducer-function#isType', () => {
-  expect(rf.isType('$foo')).toBe(false)
-  expect(rf.isType('foo()')).toBe(false)
-  expect(rf.isType(() => true)).toBe(true)
+  expect(factory.isType('$foo')).toBe(false)
+  expect(factory.isType('foo()')).toBe(false)
+  expect(factory.isType(() => true)).toBe(true)
 })
 
 describe('reducer/reducer-function#validateFunction', () => {
-  expect(rf.validateFunction(() => true)).toBe(true)
-  expect(rf.validateFunction(a => true)).toBe(true)
-  expect(rf.validateFunction((a, b) => true)).toBe(true)
+  expect(factory.validateFunction(() => true)).toBe(true)
+  expect(factory.validateFunction(a => true)).toBe(true)
+  expect(factory.validateFunction((a, b) => true)).toBe(true)
   expect(() =>
-    rf.validateFunction((a, b, c) => true)
+    factory.validateFunction((a, b, c) => true)
   ).toThrowErrorMatchingSnapshot()
 })
 
 describe('reducer/reducer-function#create', () => {
   test('function body', () => {
     const reducerFunction = () => (acc, done) => done(null, acc.value * 2)
-    const reducer = rf.create(reducerFunction)
+    const reducer = factory.create(createReducer, reducerFunction)
     expect(reducer.body).toEqual(reducerFunction)
   })
 })
diff --git a/packages/data-point/lib/reducer-types/reducer-path/factory.test.js b/packages/data-point/lib/reducer-types/reducer-path/factory.test.js
index 14387c3..141da59 100644
--- a/packages/data-point/lib/reducer-types/reducer-path/factory.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-path/factory.test.js
@@ -2,6 +2,7 @@
 'use strict'
 
 const factory = require('./factory')
+const createReducer = require('../index').create
 
 it('reducer/reducer-path#isType', () => {
   expect(factory.isType('#a')).not.toBe('is not path')
@@ -64,33 +65,33 @@ describe('reducer/reducer-path#getPathReducerFunction', () => {
 
 describe('reducer/reducer-path#create', () => {
   it('basic path', () => {
-    const reducer = factory.create('$a')
+    const reducer = factory.create(createReducer, '$a')
     expect(reducer.type).toBe('ReducerPath')
     expect(reducer.name).toBe('a')
     expect(reducer.asCollection).toBe(false)
   })
 
   it('empty path', () => {
-    const reducer = factory.create('$')
+    const reducer = factory.create(createReducer, '$')
     expect(reducer.type).toBe('ReducerPath')
     expect(reducer.name).toBe('.')
     expect(reducer.asCollection).toBe(false)
   })
 
   it('compound path', () => {
-    const reducer = factory.create('$foo.bar')
+    const reducer = factory.create(createReducer, '$foo.bar')
     expect(reducer.name).toBe('foo.bar')
     expect(reducer.asCollection).toBe(false)
   })
 
   it('compound path', () => {
-    const reducer = factory.create('$foo.bar[0]')
+    const reducer = factory.create(createReducer, '$foo.bar[0]')
     expect(reducer.name).toBe('foo.bar[0]')
     expect(reducer.asCollection).toBe(false)
   })
 
   it('path with asCollection', () => {
-    const reducer = factory.create('$foo.bar[]')
+    const reducer = factory.create(createReducer, '$foo.bar[]')
     expect(reducer.name).toBe('foo.bar')
     expect(reducer.asCollection).toBe(true)
   })
@@ -101,7 +102,7 @@ describe('ReducerPath#body', () => {
     const acc = {
       value: 'test'
     }
-    const result = factory.create('$').body(acc)
+    const result = factory.create(createReducer, '$').body(acc)
     expect(result).toBe('test')
   })
 
@@ -109,7 +110,7 @@ describe('ReducerPath#body', () => {
     const acc = {
       value: 'test'
     }
-    const result = factory.create('$.').body(acc)
+    const result = factory.create(createReducer, '$.').body(acc)
     expect(result).toBe('test')
   })
 
@@ -120,9 +121,9 @@ describe('ReducerPath#body', () => {
       },
       locals: 'test2'
     }
-    let result = factory.create('$..value.a[0]').body(acc)
+    let result = factory.create(createReducer, '$..value.a[0]').body(acc)
     expect(result).toBe('test')
-    result = factory.create('$..locals').body(acc)
+    result = factory.create(createReducer, '$..locals').body(acc)
     expect(result).toBe('test2')
   })
 
@@ -132,7 +133,7 @@ describe('ReducerPath#body', () => {
         a: ['test']
       }
     }
-    const result = factory.create('$a[0]').body(acc)
+    const result = factory.create(createReducer, '$a[0]').body(acc)
     expect(result).toBe('test')
   })
 
@@ -162,7 +163,7 @@ describe('ReducerPath#body', () => {
         }
       ]
     }
-    const result = factory.create('$a.b.c[]').body(acc)
+    const result = factory.create(createReducer, '$a.b.c[]').body(acc)
     expect(result).toEqual([1, 2, 3])
   })
 
@@ -192,7 +193,7 @@ describe('ReducerPath#body', () => {
         }
       ]
     }
-    const result = factory.create('$a.b.d[]').body(acc)
+    const result = factory.create(createReducer, '$a.b.d[]').body(acc)
     expect(result).toEqual([undefined, undefined, undefined])
   })
 
@@ -204,7 +205,7 @@ describe('ReducerPath#body', () => {
         }
       }
     }
-    const result = factory.create('$a.b.c[]').body(acc)
+    const result = factory.create(createReducer, '$a.b.c[]').body(acc)
     expect(result).toBe(undefined)
   })
 })
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 99d60ad..bac0977 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,6 +2,7 @@
 'use strict'
 
 const reducerPath = require('./index')
+const createReducer = require('../index').create
 const resolveReducer = require('../index').resolve
 const AccumulatorFactory = require('../../accumulator/factory')
 const FixtureStore = require('../../../test/utils/fixture-store')
@@ -22,7 +23,7 @@ describe('ReducerPath#resolve', () => {
       locals
     })
 
-    const reducer = reducerPath.create(reducerSource)
+    const reducer = reducerPath.create(createReducer, reducerSource)
     return reducerPath.resolve(dataPoint, resolveReducer, accumulator, reducer)
   }