Repository: ViacomInc/data-point
Pull request #99 (2018-01-02 17:03:34)

Move shared code to parse-compose.js
<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**: Moved the `validateComposeVsInlineModifiers` function to `parse-compose.js` because it was defined twice (once for Hash entities and once for Collection entities). Also moved `validateCompose` and began using that for Collections (before it was only used by Hashes).

<!-- Why are these changes necessary? -->
**Why**: DRY

<!-- 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? -->

<!-- feel free to add additional comments -->


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

Coverage remained the same at 100.0% when pulling **16c1c8f72ad94544579ff168189a455cb8ba187d on raingerber:move-shared-code-to-parse-compose** into **50a25aa2ac1f3c19d174e81334858643321f43bf on ViacomInc:master**.

acatl: > **Why**: DRY 

👍  😆 

Patch:
diff --git a/packages/data-point/lib/entity-types/entity-collection/factory.js b/packages/data-point/lib/entity-types/entity-collection/factory.js
index 8d06ccd..7469562 100644
--- a/packages/data-point/lib/entity-types/entity-collection/factory.js
+++ b/packages/data-point/lib/entity-types/entity-collection/factory.js
@@ -22,34 +22,6 @@ function createCompose (composeParse) {
   })
 }
 
-function validateComposeVsInlineModifiers (spec, invalidInlinesKeys) {
-  if (!spec.compose) {
-    return true
-  }
-
-  if (!(spec.compose instanceof Array)) {
-    throw new Error(
-      `Entity ${
-        spec.id
-      } Hash.compose property is expected to be an instance of Array, but found ${
-        spec.compose
-      }`
-    )
-  }
-
-  const specKeys = Object.keys(spec)
-  const intersection = _.intersection(invalidInlinesKeys, specKeys)
-  if (intersection.length !== 0) {
-    throw new Error(
-      `Entity ${
-        spec.id
-      } Spec is invalid, when 'compose' is defined the key(s): '${intersection.join(
-        ', '
-      )}' should be inside compose.`
-    )
-  }
-}
-
 /**
  * Creates new Entity Object
  * @param  {Object} spec - spec
@@ -57,11 +29,12 @@ function validateComposeVsInlineModifiers (spec, invalidInlinesKeys) {
  * @return {EntityCollection} Entity Object
  */
 function create (spec, id) {
-  validateComposeVsInlineModifiers(spec, modifierKeys)
+  parseCompose.validateComposeModifiers(spec, modifierKeys)
 
   const entity = createBaseEntity(EntityCollection, spec, id)
 
   const compose = parseCompose.parse(spec, modifierKeys)
+  parseCompose.validateCompose(entity.id, compose, modifierKeys)
   entity.compose = createCompose(compose)
 
   return Object.freeze(entity)
diff --git a/packages/data-point/lib/entity-types/entity-hash/factory.js b/packages/data-point/lib/entity-types/entity-hash/factory.js
index 9f137e0..adae8fa 100644
--- a/packages/data-point/lib/entity-types/entity-hash/factory.js
+++ b/packages/data-point/lib/entity-types/entity-hash/factory.js
@@ -34,52 +34,13 @@ function createCompose (composeParse) {
       case 'addKeys':
         transform = createReducerObject(createTransform, modifier.spec)
     }
-    return _.assign({}, modifier, {
+
+    return Object.assign({}, modifier, {
       transform
     })
   })
 }
 
-function validateComposeVsInlineModifiers (spec, invalidInlinesKeys) {
-  if (!spec.compose) {
-    return true
-  }
-
-  if (spec.compose && !(spec.compose instanceof Array)) {
-    throw new Error(
-      `Entity ${
-        spec.id
-      } Hash.compose property is expected to be of instance of Array and found ${
-        spec.compose
-      }`
-    )
-  }
-
-  const specKeys = Object.keys(spec)
-  const intersection = _.intersection(invalidInlinesKeys, specKeys)
-  if (intersection.length !== 0) {
-    throw new Error(
-      `Entity ${
-        spec.id
-      } is invalid, when 'compose' is defined the key(s): '${intersection.join(
-        ', '
-      )}' should be inside compose.`
-    )
-  }
-}
-
-function validateCompose (entityId, compose, validKeys) {
-  compose.forEach(modifier => {
-    if (validKeys.indexOf(modifier.type) === -1) {
-      throw new Error(
-        `Modifier '${modifier.type}' in ${
-          entityId
-        } doesn't match any of the registered Modifiers: ${validKeys}`
-      )
-    }
-  })
-}
-
 /**
  * Creates new Entity Object
  * @param  {Object} spec - spec
@@ -87,12 +48,12 @@ function validateCompose (entityId, compose, validKeys) {
  * @return {EntityHash} Entity Object
  */
 function create (spec, id) {
-  validateComposeVsInlineModifiers(spec, modifierKeys)
+  parseCompose.validateComposeModifiers(spec, modifierKeys)
 
   const entity = createBaseEntity(EntityHash, spec, id)
 
   const compose = parseCompose.parse(spec, modifierKeys)
-  validateCompose(entity.id, compose, modifierKeys)
+  parseCompose.validateCompose(entity.id, compose, modifierKeys)
   entity.compose = createCompose(compose)
 
   return Object.freeze(entity)
diff --git a/packages/data-point/lib/entity-types/parse-compose/parse-compose.js b/packages/data-point/lib/entity-types/parse-compose/parse-compose.js
index 66db290..f291918 100644
--- a/packages/data-point/lib/entity-types/parse-compose/parse-compose.js
+++ b/packages/data-point/lib/entity-types/parse-compose/parse-compose.js
@@ -1,5 +1,71 @@
-'use strict'
+const intersection = require('lodash/intersection')
 
+/**
+ * @param {Object} spec
+ * @param {Array<string>} validKeys
+ * @throws if the spec is not valid
+ * @returns {boolean}
+ */
+function validateComposeModifiers (spec, validKeys) {
+  if (!spec.compose) {
+    return true
+  }
+
+  if (!(spec.compose instanceof Array)) {
+    throw new Error(
+      `Entity ${
+        spec.id
+      } compose property is expected to be an instance of Array, but found ${
+        spec.compose
+      }`
+    )
+  }
+
+  const specKeys = Object.keys(spec)
+  const invalidKeys = intersection(validKeys, specKeys)
+  if (invalidKeys.length !== 0) {
+    throw new Error(
+      `Entity ${
+        spec.id
+      } is invalid; when 'compose' is defined the keys: '${invalidKeys.join(
+        ', '
+      )}' should be inside compose.`
+    )
+  }
+
+  return true
+}
+
+module.exports.validateComposeModifiers = validateComposeModifiers
+
+/**
+ * @param {string} entityId
+ * @param {Array<Object>} compose
+ * @param {Array<string>} validKeys
+ * @throws if compose contains an invalid modifier type
+ * @returns {boolean}
+ */
+function validateCompose (entityId, compose, validKeys) {
+  compose.forEach(modifier => {
+    if (validKeys.indexOf(modifier.type) === -1) {
+      throw new Error(
+        `Modifier '${modifier.type}' in ${
+          entityId
+        } doesn't match any of the registered Modifiers: ${validKeys}`
+      )
+    }
+  })
+
+  return true
+}
+
+module.exports.validateCompose = validateCompose
+
+/**
+ * @param {string} type
+ * @param {Object} spec
+ * @returns {Object}
+ */
 function createComposeReducer (type, spec) {
   return {
     type,
@@ -9,44 +75,64 @@ function createComposeReducer (type, spec) {
 
 module.exports.createComposeReducer = createComposeReducer
 
+/**
+ * @param {Object} modifierSpec
+ * @returns {Object}
+ */
 function parseModifierSpec (modifierSpec) {
   const keys = Object.keys(modifierSpec)
   if (keys.length !== 1) {
     throw new Error(
-      `Compose Modifiers may only contain one key, found: ${
-        keys.length
-      }(${keys.join(', ')})`
+      `Compose Modifiers may only contain one key, but found ${keys.length}${
+        keys.length ? ` (${keys.join(', ')})` : ''
+      }`
     )
   }
+
   const type = keys[0]
   return createComposeReducer(keys[0], modifierSpec[type])
 }
 
 module.exports.parseModifierSpec = parseModifierSpec
 
+/**
+ * @param {Array<Object>} composeSpec
+ * @returns {Array<Object>}
+ */
 function parseComposeSpecProperty (composeSpec) {
   return composeSpec.map(parseModifierSpec)
 }
 
 module.exports.parseComposeSpecProperty = parseComposeSpecProperty
 
+/**
+ * @param {Object} entitySpec
+ * @param {Array<string>} modifierKeys
+ * @returns {Array<Object>}
+ */
 function parseComposeFromEntitySpec (entitySpec, modifierKeys) {
   return modifierKeys.reduce((acc, modifierKey) => {
-    if (typeof entitySpec[modifierKey] === 'undefined') {
-      return acc
+    const modifierSpec = entitySpec[modifierKey]
+    if (modifierSpec) {
+      const modifier = createComposeReducer(modifierKey, modifierSpec)
+      acc.push(modifier)
     }
-    const modifier = createComposeReducer(modifierKey, entitySpec[modifierKey])
-    acc.push(modifier)
+
     return acc
   }, [])
 }
 
 module.exports.parseComposeFromEntitySpec = parseComposeFromEntitySpec
 
-function parse (spec, modifierKeys) {
-  return spec.compose
-    ? parseComposeSpecProperty(spec.compose)
-    : parseComposeFromEntitySpec(spec, modifierKeys)
+/**
+ * @param {Object} entitySpec
+ * @param {Array<string>} modifierKeys
+ * @returns {Array<Object>}
+ */
+function parse (entitySpec, modifierKeys) {
+  return entitySpec.compose
+    ? parseComposeSpecProperty(entitySpec.compose, modifierKeys)
+    : parseComposeFromEntitySpec(entitySpec, modifierKeys)
 }
 
 module.exports.parse = parse


Test patch:
diff --git a/packages/data-point/lib/entity-types/parse-compose/parse-compose.test.js b/packages/data-point/lib/entity-types/parse-compose/parse-compose.test.js
index db569d3..140e6d3 100644
--- a/packages/data-point/lib/entity-types/parse-compose/parse-compose.test.js
+++ b/packages/data-point/lib/entity-types/parse-compose/parse-compose.test.js
@@ -36,7 +36,7 @@ describe('parseComposeSpecProperty', () => {
   test('should throw error if does not contain keys', () => {
     expect(() => {
       parseCompose.parseComposeSpecProperty([{}])
-    }).toThrow(/found: 0/)
+    }).toThrow(/found 0/)
   })
   test('should throw error if contains more than 1 key', () => {
     expect(() => {
@@ -46,7 +46,7 @@ describe('parseComposeSpecProperty', () => {
           filter: '$a'
         }
       ])
-    }).toThrow(/found: 2/)
+    }).toThrow(/found 2/)
   })
 })