Repository: ViacomInc/data-point (js)

Issue:
Remove predefined execution order for hashes and collections
**Problem description:**

Hash and collection entities don't always execute in the order they're defined. @YiningChen explains this problem in more depth here #24, but instead of warning users, I propose that we remove this behavior entirely. This would make it easier to read entity definitions without needing to consult the documentation to know the execution order for each of the keys.

**Suggested solution:**

If users want a hash or collection to use more than one modifier (e.g. `addValues`, `mapKeys`, etc), they should be required to use `compose`.

For example, this would be valid:

```js
dataPoint.addEntities({
  'hash:example': {
    addValues: {
      b: { text: 'See You Later' }
    }
  }
})
```

...and this would be valid:

```js
dataPoint.addEntities({
  'hash:example': {
    compose: [
      addValues: {
        b: { text: 'See You Later' }
      },
      mapKeys: {
        a: '$a.text',
        b: '$b.text',
      }
    ]
  }
})
```

...but this would **NOT** be valid, because `addValues` and `mapKeys` are both defined on the root object instead of using `compose`:

```js
dataPoint.addEntities({
  'hash:example': {
    addValues: {
      b: { text: 'See You Later' }
    },
    mapKeys: {
      a: '$a.text',
      b: '$b.text',
    }
  }
})
```

Pull request description:
feat(data-point): Remove predefined hash and collection execution order
**What**: closes #73, and has a codemod because it's a breaking change

<!-- Why are these changes necessary? -->
**Why**: Makes it easier to understand `hash` and `collection` entity definitions without consulting the readme, because the execution order for "modifiers" is always defined by the order of the `compose` array

<!-- How were these changes implemented? -->
**How**: Refactored `parse-compose.js`

<!-- 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" -->
- [x] Has Breaking changes
- [x] Documentation
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->

Patch:
diff --git a/packages/data-point-codemods/transforms/hash-and-collection-execution-order.js b/packages/data-point-codemods/transforms/hash-and-collection-execution-order.js
new file mode 100644
index 0000000..ac3b2d4
--- /dev/null
+++ b/packages/data-point-codemods/transforms/hash-and-collection-execution-order.js
@@ -0,0 +1,65 @@
+const partition = require('lodash/partition')
+const findLastIndex = require('lodash/findLastIndex')
+
+const { getEntityObjects } = require('./utils')
+
+function isMatchingKey (key, keys) {
+  const id = key.type === 'Identifier' ? key.name : key.value
+  return keys.includes(id)
+}
+
+module.exports = (file, api, options) => {
+  const j = api.jscodeshift
+  const root = j(file.source)
+
+  function updateEntity (node, keys) {
+    let [props, otherProps] = partition(node.value.properties, prop => {
+      return isMatchingKey(prop.key, keys)
+    })
+
+    if (props.length < 2) {
+      return
+    }
+
+    props = keys.reduce((acc, key) => {
+      const prop = props.find(p => isMatchingKey(p.key, [key]))
+      if (prop) {
+        acc.push(prop)
+      }
+
+      return acc
+    }, [])
+
+    const composeProp = j.property(
+      'init',
+      j.identifier('compose'),
+      j.arrayExpression(
+        props.map(prop => {
+          return j.objectExpression([prop])
+        })
+      )
+    )
+
+    const index = findLastIndex(otherProps, prop => {
+      return isMatchingKey(prop.key, ['inputType', 'before', 'value'])
+    })
+
+    if (index === -1) {
+      node.value.properties = [composeProp].concat(otherProps)
+    } else {
+      otherProps.splice(index + 1, 0, composeProp)
+      node.value.properties = otherProps
+    }
+  }
+
+  function execute (entityId, keys) {
+    const nodes = getEntityObjects(entityId, j, root)
+    nodes[0].forEach(node => updateEntity(node, keys))
+    nodes[1].forEach(node => updateEntity(node, keys))
+  }
+
+  execute('collection', ['filter', 'map', 'find'])
+  execute('hash', ['omitKeys', 'pickKeys', 'mapKeys', 'addValues', 'addKeys'])
+
+  return root.toSource()
+}
diff --git a/packages/data-point-codemods/transforms/utils.js b/packages/data-point-codemods/transforms/utils.js
new file mode 100644
index 0000000..ad981e2
--- /dev/null
+++ b/packages/data-point-codemods/transforms/utils.js
@@ -0,0 +1,59 @@
+function getEntityKeyRegexFn (entityId) {
+  const r = new RegExp(`^${entityId}:.+`)
+  return key => r.test(key)
+}
+
+/*
+find entities that are defined with object literals:
+
+const data = {
+  'hash:thing1': {
+    value: {}
+  },
+  entities: {
+    'hash:thing2: {
+      value: {}
+    }
+  }
+}
+*/
+function getEntityObjectFromProperties (entityId, j, root) {
+  return root
+    .find(j.Property, {
+      key: {
+        type: 'Literal',
+        value: getEntityKeyRegexFn(entityId)
+      }
+    })
+    .map(node => node.get('value'))
+}
+
+/*
+find entities that are defined by assignment:
+
+const data = {}
+data['hash:thing'] = {
+  value: {}
+}
+*/
+function getEntityObjectFromAssignments (entityId, j, root) {
+  return root
+    .find(j.AssignmentExpression, {
+      left: {
+        type: 'MemberExpression',
+        property: {
+          value: getEntityKeyRegexFn(entityId)
+        }
+      }
+    })
+    .map(path => path.get('right'))
+}
+
+function getEntityObjects (entityId, j, root) {
+  return [
+    getEntityObjectFromProperties(entityId, j, root),
+    getEntityObjectFromAssignments(entityId, j, root)
+  ]
+}
+
+module.exports.getEntityObjects = getEntityObjects
diff --git a/packages/data-point/README.md b/packages/data-point/README.md
index 11494dd..66f67c6 100644
--- a/packages/data-point/README.md
+++ b/packages/data-point/README.md
@@ -2054,14 +2054,6 @@ A Hash entity transforms a _Hash_ like data structure. It enables you to manipul
 
 To prevent unexpected results, a **Hash** can only return **Plain Objects**, which are objects created by the Object constructor. If a hash resolves to a different type, it will throw an error. This type check occurs *before* the value is passed to the (optional) `outputType` reducer.
 
-Hash entities expose a set of reducers: [mapKeys](#hash-mapKeys), [omitKeys](#hash-omitKeys), [pickKeys](#hash-pickKeys), [addKeys](#hash-addKeys), [addValues](#hash-addValues). You may apply one or more of these reducers to a Hash entity. Keep in mind that those reducers will always be executed in a specific order:
-
-```js
-omitKeys -> pickKeys -> mapKeys -> addValues -> addKeys
-```
-
-If you want to have more control over the order of execution, you may use the [compose](#entity-compose-reducer) reducer.
-
 **SYNOPSIS**
 
 ```js
@@ -2102,6 +2094,11 @@ dataPoint.addEntities({
 | *error*   | [Reducer](#reducers) | reducer to be resolved in case of an error |
 | *params*    | `Object` | User-defined Hash that will be passed to every reducer within the context of the transform function's execution |
 
+##### <a name="hash-entity-reducers">Hash Reducers</a>
+
+Hash entities expose a set of optional reducers: [mapKeys](#hash-mapKeys), [omitKeys](#hash-omitKeys), [pickKeys](#hash-pickKeys), [addKeys](#hash-addKeys), and [addValues](#hash-addValues). When using more than one of these reducers, they should be defined through the `compose` property.
+
+
 ##### <a name="hash-value">Hash.value</a>
 
 <details>
@@ -2386,8 +2383,6 @@ For examples of hash entities, see the [Examples](examples), on the unit tests:
 
 A Collection entity enables you to operate over an array. Its API provides basic reducers to manipulate the elements in the array.
 
-Collection entities expose a set of reducers that you may apply to them: [map](#collection-map), [find](#collection-find), [filter](#collection-filter). These reducers are executed in a [specific order](#collection-reducers-order). If you want to have more control over the order of execution, use the [compose](#compose-reducer) reducer.
-
 To prevent unexpected results, a **Collection** can only return arrays. If a collection resolves to a different type, it will throw an eror. This type check occurs *before* the value is passed to the (optional) `outputType` reducer.
 
 **IMPORTANT:** Keep in mind that in DataPoint, **all** operations are asynchronous. If your operations do NOT need to be asynchronous, iterating over a large array might result in slower execution. In such cases, consider using a reducer function where you can implement a synchronous solution.
@@ -2428,11 +2423,9 @@ dataPoint.addEntities({
 | *error* | [Reducer](#reducers) | reducer to be resolved in case of an error |
 | *params* | `Object` | User-defined Hash that will be passed to every reducer within the context of the transform function's execution |
 
-<a name="collection-reducers-order">The order of execution of is:</a>
+##### <a name="collection-entity-reducers">Collection Reducers</a>
 
-```js
-filter -> map -> find
-```
+Collection entities expose a set of optional reducers: [map](#collection-map), [find](#collection-find), and [filter](#collection-filter). When using more than one of these reducers, they should be defined with the `compose` property.
 
 ##### <a name="collection-map">Collection.map</a>
 
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 eca575b..3a3d4c0 100644
--- a/packages/data-point/lib/entity-types/entity-collection/factory.js
+++ b/packages/data-point/lib/entity-types/entity-collection/factory.js
@@ -43,7 +43,6 @@ function createCompose (composeSpec) {
  */
 function create (spec, id) {
   validateModifiers(id, spec, modifierKeys.concat('compose'))
-  parseCompose.validateComposeModifiers(id, spec, modifierKeys)
 
   const outputType = getTypeCheckSourceWithDefault(
     'collection',
@@ -53,11 +52,9 @@ function create (spec, id) {
   spec = Object.assign({}, spec, { outputType })
 
   const entity = createBaseEntity(EntityCollection, spec, id)
-
-  const composeSpec = parseCompose.parse(spec, modifierKeys)
-  parseCompose.validateCompose(entity.id, composeSpec, modifierKeys)
-  if (composeSpec.length) {
-    entity.compose = createCompose(composeSpec)
+  const compose = parseCompose.parse(id, modifierKeys, spec)
+  if (compose.length) {
+    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 91374db..60ea752 100644
--- a/packages/data-point/lib/entity-types/entity-hash/factory.js
+++ b/packages/data-point/lib/entity-types/entity-hash/factory.js
@@ -66,7 +66,6 @@ function createCompose (composeSpec) {
  */
 function create (spec, id) {
   validateModifiers(id, spec, modifierKeys.concat('compose'))
-  parseCompose.validateComposeModifiers(id, spec, modifierKeys)
 
   const outputType = getTypeCheckSourceWithDefault(
     'hash',
@@ -76,9 +75,7 @@ function create (spec, id) {
   spec = Object.assign({}, spec, { outputType })
 
   const entity = createBaseEntity(EntityHash, spec, id)
-
-  const compose = parseCompose.parse(spec, modifierKeys)
-  parseCompose.validateCompose(entity.id, compose, modifierKeys)
+  const compose = parseCompose.parse(id, modifierKeys, spec)
   if (compose.length) {
     entity.compose = createCompose(compose)
   }
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 18fb295..0fa9f20 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,83 +1,90 @@
 const intersection = require('lodash/intersection')
+const _ = require('lodash')
+const { format, inspect } = require('util')
+
+const composeLink =
+  'https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer'
 
 /**
- * @param {string} id - Entity's Id
- * @param {Object} spec
+ * @param {string} entityId
  * @param {Array<string>} validKeys
+ * @param {Object} spec
  * @throws if the spec is not valid
- * @returns {boolean}
+ * @returns {Array<Object>}
  */
-function validateComposeModifiers (id, spec, validKeys) {
-  if (!spec.compose) {
-    return true
-  }
-
-  if (!(spec.compose instanceof Array)) {
-    throw new Error(
-      `Entity ${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) {
+function parse (entityId, validKeys, spec) {
+  const specKeys = intersection(Object.keys(spec), validKeys)
+  if (specKeys.length > 1) {
     throw new Error(
-      `Entity ${id} is invalid; when 'compose' is defined the keys: '${invalidKeys.join(
-        ', '
-      )}' should be inside compose.`
+      format(
+        'Entity "%s" has invalid modifiers, when using multiple keys they should be added through the "compose" property.\nValid modifiers: %s.\nFor more info: %s',
+        entityId,
+        specKeys.join(', '),
+        composeLink
+      )
     )
   }
 
-  return true
-}
-
-module.exports.validateComposeModifiers = validateComposeModifiers
+  if (spec.compose) {
+    if (!Array.isArray(spec.compose)) {
+      throw new Error(
+        format(
+          'Entity "%s" compose property is expected to be an instance of Array, but found: %s\nFor more info: %s',
+          entityId,
+          _.truncate(inspect(spec.compose, { breakLength: Infinity }), {
+            length: 30
+          }),
+          composeLink
+        )
+      )
+    }
 
-/**
- * @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) {
+    if (specKeys.length !== 0) {
       throw new Error(
-        `Modifier '${
-          modifier.type
-        }' in ${entityId} doesn't match any of the registered Modifiers: ${validKeys}`
+        format(
+          'Entity "%s" is invalid; when "compose" is defined the keys: %s should be inside compose.\nFor more info: %s',
+          entityId,
+          specKeys.join(', '),
+          composeLink
+        )
       )
     }
-  })
 
-  return true
+    return parseComposeSpec(entityId, validKeys, spec.compose)
+  }
+
+  if (specKeys.length === 1) {
+    const key = specKeys[0]
+    return [createComposeReducer(key, spec[key])]
+  }
+
+  return []
 }
 
-module.exports.validateCompose = validateCompose
+module.exports.parse = parse
 
 /**
- * @param {string} type
- * @param {Object} spec
- * @returns {Object}
+ * @param {string} entityId
+ * @param {Array<string>} modifierKeys
+ * @param {Array<Object>} composeSpec
+ * @returns {Array<Object>}
  */
-function createComposeReducer (type, spec) {
-  return {
-    type,
-    spec
-  }
+function parseComposeSpec (entityId, modifierKeys, composeSpec) {
+  return composeSpec.map(modifierSpec => {
+    return parseModifierSpec(entityId, modifierKeys, modifierSpec)
+  })
 }
 
-module.exports.createComposeReducer = createComposeReducer
+module.exports.parseComposeSpec = parseComposeSpec
 
 /**
+ * @param {string} entityId
+ * @param {Array<string>} validKeys
  * @param {Object} modifierSpec
- * @throws if the spec does not contain exactly one key
+ * @throws if the spec is not valid
  * @returns {Object}
  */
-function parseModifierSpec (modifierSpec) {
+function parseModifierSpec (entityId, validKeys, modifierSpec) {
   const keys = Object.keys(modifierSpec)
   if (keys.length !== 1) {
     throw new Error(
@@ -88,49 +95,26 @@ function parseModifierSpec (modifierSpec) {
   }
 
   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) => {
-    const modifierSpec = entitySpec[modifierKey]
-    if (modifierSpec) {
-      const modifier = createComposeReducer(modifierKey, modifierSpec)
-      acc.push(modifier)
-    }
+  if (!validKeys.includes(type)) {
+    throw new Error(
+      `Modifier '${type}' in "${entityId}" does not match any of the valid modifiers: ${validKeys.join(
+        ', '
+      )}`
+    )
+  }
 
-    return acc
-  }, [])
+  return createComposeReducer(type, modifierSpec[type])
 }
 
-module.exports.parseComposeFromEntitySpec = parseComposeFromEntitySpec
+module.exports.parseModifierSpec = parseModifierSpec
 
 /**
- * @param {Object} entitySpec
- * @param {Array<string>} modifierKeys
- * @returns {Array<Object>}
+ * @param {string} type
+ * @param {Object} spec
+ * @returns {Object}
  */
-function parse (entitySpec, modifierKeys) {
-  return entitySpec.compose
-    ? parseComposeSpecProperty(entitySpec.compose, modifierKeys)
-    : parseComposeFromEntitySpec(entitySpec, modifierKeys)
+function createComposeReducer (type, spec) {
+  return { type, spec }
 }
 
-module.exports.parse = parse
+module.exports.createComposeReducer = createComposeReducer


Test patch:
diff --git a/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.input.js b/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.input.js
new file mode 100644
index 0000000..738515b
--- /dev/null
+++ b/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.input.js
@@ -0,0 +1,86 @@
+/* eslint-disable */
+/* eslint-disable prettier */
+
+const e1 = {}
+
+// prettier-ignore
+e1 = {
+  entities: {
+    'collection:nested': {
+      map: '$b',
+      filter: '$a',
+      after: () => true
+    }
+  }
+}
+
+const e2 = {}
+
+// prettier-ignore
+e2['hash:example'] = {
+  'after': () => ({}),
+  addKeys: {},
+  'error': () => ({}),
+  'params': {},
+  'omitKeys': ['a', 'b'],
+  'before': () => ({}),
+  pickKeys: [],
+  value: {},
+  'mapKeys': () => true,
+  'inputType': 'object',
+  'addValues': () => ({}),
+  'outputType': 'object'
+}
+
+// prettier-ignore
+const e3 = {
+  'collection:example': {
+    before: () => [],
+    outputType: 'array',
+    inputType: 'array',
+    find: '$find',
+    filter: '$filter',
+    params: {},
+    after: () => [],
+    map: '$map',
+    value: '$c',
+    error: () => []
+  }
+}
+
+const e4 = {}
+
+// should not be changed
+e4['collection:example'] = {
+  mapKeys: '$a'
+}
+
+// should not be changed
+const e5 = {
+  entities: {
+    'hash:example': {
+      mapKeys: '$a'
+    }
+  }
+}
+
+// should not be changed
+const e6 = {
+  'collection:example': {
+    before: () => [],
+    inputType: 'array',
+    find: '$find'
+  }
+}
+
+// should not be changed
+// prettier-ignore
+const e7 = {
+  'hash:example': {
+    before: () => [],
+    compose: [{
+      invalidKey: '$invalidKey'
+    }],
+    inputType: 'array'
+  }
+}
diff --git a/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.output.js b/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.output.js
new file mode 100644
index 0000000..3d1ca24
--- /dev/null
+++ b/packages/data-point-codemods/transforms/__testfixtures__/hash-and-collection-execution-order.output.js
@@ -0,0 +1,104 @@
+/* eslint-disable */
+/* eslint-disable prettier */
+
+const e1 = {}
+
+// prettier-ignore
+e1 = {
+  entities: {
+    'collection:nested': {
+      compose: [{
+        filter: '$a'
+      }, {
+        map: '$b'
+      }],
+
+      after: () => true
+    }
+  }
+}
+
+const e2 = {}
+
+// prettier-ignore
+e2['hash:example'] = {
+  'after': () => ({}),
+  'error': () => ({}),
+  'params': {},
+  'before': () => ({}),
+  value: {},
+  'inputType': 'object',
+
+  compose: [{
+    'omitKeys': ['a', 'b']
+  }, {
+    pickKeys: []
+  }, {
+    'mapKeys': () => true
+  }, {
+    'addValues': () => ({})
+  }, {
+    addKeys: {}
+  }],
+
+  'outputType': 'object'
+}
+
+// prettier-ignore
+const e3 = {
+  'collection:example': {
+    before: () => [],
+    outputType: 'array',
+    inputType: 'array',
+    params: {},
+    after: () => [],
+    value: '$c',
+
+    compose: [{
+      filter: '$filter'
+    }, {
+      map: '$map'
+    }, {
+      find: '$find'
+    }],
+
+    error: () => []
+  }
+}
+
+const e4 = {}
+
+// should not be changed
+e4['collection:example'] = {
+  mapKeys: '$a'
+}
+
+// should not be changed
+const e5 = {
+  entities: {
+    'hash:example': {
+      mapKeys: '$a'
+    }
+  }
+}
+
+// should not be changed
+const e6 = {
+  'collection:example': {
+    before: () => [],
+    inputType: 'array',
+    find: '$find'
+  }
+}
+
+// should not be changed
+// prettier-ignore
+const e7 = {
+  'hash:example': {
+    before: () => [],
+    compose: [{
+      invalidKey: '$invalidKey'
+    }],
+    inputType: 'array'
+  }
+}
diff --git a/packages/data-point-codemods/transforms/__tests__/hash-and-collection-execution-order-test.js b/packages/data-point-codemods/transforms/__tests__/hash-and-collection-execution-order-test.js
new file mode 100644
index 0000000..6e704a2
--- /dev/null
+++ b/packages/data-point-codemods/transforms/__tests__/hash-and-collection-execution-order-test.js
@@ -0,0 +1,10 @@
+/* eslint-env jest */
+
+const defineTest = require('jscodeshift/dist/testUtils').defineTest
+
+defineTest(
+  __dirname,
+  'hash-and-collection-execution-order',
+  null,
+  'hash-and-collection-execution-order'
+)
diff --git a/packages/data-point/lib/entity-types/entity-collection/__snapshots__/factory.test.js.snap b/packages/data-point/lib/entity-types/entity-collection/__snapshots__/factory.test.js.snap
index c5ecd8b..affc7e9 100644
--- a/packages/data-point/lib/entity-types/entity-collection/__snapshots__/factory.test.js.snap
+++ b/packages/data-point/lib/entity-types/entity-collection/__snapshots__/factory.test.js.snap
@@ -1,3 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`parse compose modifier throw error if compose is not an array 1`] = `"Entity undefined compose property is expected to be an instance of Array, but found [object Object]"`;
+exports[`parse compose modifier throw error if compose is mixed with inline modifiers (map, filter, ..)  1`] = `
+"Entity \\"test-id\\" has invalid modifiers, when using multiple keys they should be added through the \\"compose\\" property.
+Valid modifiers: map, filter.
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
+
+exports[`parse compose modifier throw error if compose is not an array 1`] = `
+"Entity \\"test-id\\" compose property is expected to be an instance of Array, but found: { map: '$a' }
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
diff --git a/packages/data-point/lib/entity-types/entity-collection/factory.test.js b/packages/data-point/lib/entity-types/entity-collection/factory.test.js
index b5927fa..680676d 100644
--- a/packages/data-point/lib/entity-types/entity-collection/factory.test.js
+++ b/packages/data-point/lib/entity-types/entity-collection/factory.test.js
@@ -15,7 +15,7 @@ test('modelFactory#create default', () => {
 })
 
 describe('parse loose modifiers', () => {
-  test('modelFactory#create default | checks that an entitiy containing one reducer has that respective property', () => {
+  test('modelFactory#create default | checks that an entity containing one reducer has that respective property', () => {
     const result = modelFactory.create({
       map: '$a'
     })
@@ -24,20 +24,6 @@ describe('parse loose modifiers', () => {
     expect(result.compose).toHaveProperty('type', 'ReducerMap')
     expect(result.compose.reducer).toHaveProperty('type', 'ReducerPath')
   })
-
-  test('modelFactory#create default | checks multiple reducers in an entitiy to have matching properties', () => {
-    const result = modelFactory.create({
-      map: '$a',
-      find: '$a',
-      filter: '$a'
-    })
-
-    expect(helpers.isReducer(result.compose)).toBe(true)
-    expect(result.compose).toHaveProperty('type', 'ReducerList')
-    expect(result.compose.reducers[0]).toHaveProperty('type', 'ReducerFilter')
-    expect(result.compose.reducers[1]).toHaveProperty('type', 'ReducerMap')
-    expect(result.compose.reducers[2]).toHaveProperty('type', 'ReducerFind')
-  })
 })
 
 describe('parse compose modifier', () => {
@@ -53,9 +39,12 @@ describe('parse compose modifier', () => {
 
   test('throw error if compose is not an array', () => {
     expect(() => {
-      modelFactory.create({
-        compose: { map: '$a' }
-      })
+      modelFactory.create(
+        {
+          compose: { map: '$a' }
+        },
+        'test-id'
+      )
     }).toThrowErrorMatchingSnapshot()
   })
 
@@ -67,9 +56,9 @@ describe('parse compose modifier', () => {
           map: '$a',
           filter: '$a'
         },
-        ['filter', 'map', 'find']
+        'test-id'
       )
-    }).toThrow(/filter, map/)
+    }).toThrowErrorMatchingSnapshot()
   })
 
   test('parses multiple modifiers, respect order', () => {
diff --git a/packages/data-point/lib/entity-types/entity-hash/__snapshots__/factory.test.js.snap b/packages/data-point/lib/entity-types/entity-hash/__snapshots__/factory.test.js.snap
index 8966287..b7a5e43 100644
--- a/packages/data-point/lib/entity-types/entity-hash/__snapshots__/factory.test.js.snap
+++ b/packages/data-point/lib/entity-types/entity-hash/__snapshots__/factory.test.js.snap
@@ -1,7 +1,13 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`factory#parse composed modifiers throw error if compose and loose modifiers are mixed 1`] = `"Entity hash:invalid is invalid; when 'compose' is defined the keys: 'mapKeys' should be inside compose."`;
+exports[`factory#parse composed modifiers throw error if compose and loose modifiers are mixed 1`] = `
+"Entity \\"hash:invalid\\" is invalid; when \\"compose\\" is defined the keys: mapKeys should be inside compose.
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
 
-exports[`factory#parse composed modifiers throw error if compose non array 1`] = `"Entity hash:invalid compose property is expected to be an instance of Array, but found [object Object]"`;
+exports[`factory#parse composed modifiers throw error if compose non array 1`] = `
+"Entity \\"hash:invalid\\" compose property is expected to be an instance of Array, but found: {}
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
 
-exports[`factory#parse composed modifiers throw error if modifier is invalid 1`] = `"Modifier 'invalidKey' in hash:invalid doesn't match any of the registered Modifiers: omitKeys,pickKeys,mapKeys,addValues,addKeys"`;
+exports[`factory#parse composed modifiers throw error if modifier is invalid 1`] = `"Modifier 'invalidKey' in \\"hash:invalid\\" does not match any of the valid modifiers: omitKeys, pickKeys, mapKeys, addValues, addKeys"`;
diff --git a/packages/data-point/lib/entity-types/parse-compose/__snapshots__/parse-compose.test.js.snap b/packages/data-point/lib/entity-types/parse-compose/__snapshots__/parse-compose.test.js.snap
new file mode 100644
index 0000000..327924f
--- /dev/null
+++ b/packages/data-point/lib/entity-types/parse-compose/__snapshots__/parse-compose.test.js.snap
@@ -0,0 +1,40 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`parse should parse a single key 1`] = `
+Array [
+  Object {
+    "spec": "$a",
+    "type": "map",
+  },
+]
+`;
+
+exports[`parse should parse multiple keys inside of compose 1`] = `
+Array [
+  Object {
+    "spec": "$a",
+    "type": "map",
+  },
+  Object {
+    "spec": "$b",
+    "type": "find",
+  },
+]
+`;
+
+exports[`parse should throw error for invalid key inside compose 1`] = `"Modifier 'find' in \\"id\\" does not match any of the valid modifiers: map"`;
+
+exports[`parse should throw error when multiple keys are not inside compose 1`] = `
+"Entity \\"id\\" has invalid modifiers, when using multiple keys they should be added through the \\"compose\\" property.
+Valid modifiers: map, find.
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
+
+exports[`parse should throw error when the compose key and individual modifier keys are used together 1`] = `
+"Entity \\"id\\" is invalid; when \\"compose\\" is defined the keys: map should be inside compose.
+For more info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#entity-compose-reducer"
+`;
+
+exports[`parseComposeSpec should throw error if contains more than 1 key 1`] = `"Compose Modifiers may only contain one key, but found 2 (map, filter)"`;
+
+exports[`parseComposeSpec should throw error if does not contain keys 1`] = `"Compose Modifiers may only contain one key, but found 0"`;
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 0a9e844..e3ce865 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
@@ -1,6 +1,5 @@
 /* eslint-env jest */
 const parseCompose = require('./parse-compose')
-const _ = require('lodash')
 
 function expectModifier (result) {
   expect(result).toHaveProperty('type')
@@ -16,55 +15,106 @@ describe('createComposeReducer', () => {
 
 describe('parseModifierSpec', () => {
   test('it should parse a modifier spec', () => {
-    const result = parseCompose.parseModifierSpec({
+    const result = parseCompose.parseModifierSpec('id', ['map'], {
       map: '$a'
     })
     expectModifier(result)
   })
 })
 
-describe('parseComposeSpecProperty', () => {
+describe('parseComposeSpec', () => {
   test('it should parse a single modifier spec', () => {
-    const result = parseCompose.parseComposeSpecProperty([
-      {
-        map: '$a'
-      }
-    ])
+    const result = parseCompose.parseComposeSpec(
+      'id',
+      ['map'],
+      [
+        {
+          map: '$a'
+        }
+      ]
+    )
 
     expect(result).toHaveLength(1)
     result.forEach(expectModifier)
   })
+
   test('should throw error if does not contain keys', () => {
     expect(() => {
-      parseCompose.parseComposeSpecProperty([{}])
-    }).toThrow(/found 0/)
+      parseCompose.parseComposeSpec('id', [], [{}])
+    }).toThrowErrorMatchingSnapshot()
   })
+
   test('should throw error if contains more than 1 key', () => {
     expect(() => {
-      parseCompose.parseComposeSpecProperty([
-        {
-          map: '$a',
-          filter: '$a'
-        }
-      ])
-    }).toThrow(/found 2/)
+      parseCompose.parseComposeSpec(
+        'id',
+        ['map', 'filter'],
+        [
+          {
+            map: '$a',
+            filter: '$a'
+          }
+        ]
+      )
+    }).toThrowErrorMatchingSnapshot()
   })
 })
 
-describe('parseComposeFromEntitySpec', () => {
-  test('should parse modifiers from entitySpec keys', () => {
-    const result = parseCompose.parseComposeFromEntitySpec(
-      {
-        shouldIgnore: '$a',
+describe('parse', () => {
+  test('should parse a single key', () => {
+    return expect(
+      parseCompose.parse('id', ['map'], {
+        map: '$a'
+      })
+    ).toMatchSnapshot()
+  })
+
+  test('should parse multiple keys inside of compose', () => {
+    return expect(
+      parseCompose.parse('id', ['map', 'find'], {
+        compose: [
+          {
+            map: '$a'
+          },
+          {
+            find: '$b'
+          }
+        ]
+      })
+    ).toMatchSnapshot()
+  })
+
+  test('should throw error for invalid key inside compose', () => {
+    return expect(() =>
+      parseCompose.parse('id', ['map'], {
+        compose: [
+          {
+            find: '$a'
+          }
+        ]
+      })
+    ).toThrowErrorMatchingSnapshot()
+  })
+
+  test('should throw error when multiple keys are not inside compose', () => {
+    return expect(() =>
+      parseCompose.parse('id', ['map', 'find'], {
         map: '$a',
-        filter: '$a'
-      },
-      ['filter', 'map']
-    )
+        find: '$a'
+      })
+    ).toThrowErrorMatchingSnapshot()
+  })
 
-    expect(result).toBeInstanceOf(Array)
-    expect(result).toHaveLength(2)
-    result.forEach(expectModifier)
-    expect(_.map(result, 'type')).toEqual(['filter', 'map'])
+  test('should throw error when the compose key and individual modifier keys are used together', () => {
+    return expect(() =>
+      parseCompose.parse('id', ['map', 'find'], {
+        map: '$a',
+        compose: [
+          {
+            find: '$a'
+          }
+        ]
+      })
+    ).toThrowErrorMatchingSnapshot()
   })
 })
diff --git a/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap b/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap
index c8fe917..24e9e8e 100644
--- a/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap
+++ b/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`add It should throw error with information if item already exist | error containing spec will return and entitiy containing spec object 1`] = `
+exports[`add It should throw error with information if item already exist | error containing spec will return and entity containing spec object 1`] = `
 [Error: factory error
 Entity "id1": {
   "spec": "mySpec"
diff --git a/packages/data-point/lib/stores/store-manager.test.js b/packages/data-point/lib/stores/store-manager.test.js
index 7f6ffb2..2c3a7be 100644
--- a/packages/data-point/lib/stores/store-manager.test.js
+++ b/packages/data-point/lib/stores/store-manager.test.js
@@ -30,7 +30,7 @@ describe('add', () => {
     expect(error).toMatchSnapshot()
   })
 
-  test('It should throw error with information if item already exist | error containing spec will return and entitiy containing spec object', () => {
+  test('It should throw error with information if item already exist | error containing spec will return and entity containing spec object', () => {
     const mgr = createManager()
     const errorInfoCb = () => ({
       name: 'Foo',
diff --git a/packages/data-point/test/definitions/hash.js b/packages/data-point/test/definitions/hash.js
index 213a01a..3e53e9c 100644
--- a/packages/data-point/test/definitions/hash.js
+++ b/packages/data-point/test/definitions/hash.js
@@ -69,15 +69,23 @@ module.exports = {
   },
   'hash:g.1': {
     value: '$a.g',
-    mapKeys: {
-      g: ['$g2', reducers.multiplyBy(2)]
-    },
-    addValues: {
-      g4: 4
-    },
-    addKeys: {
-      g4: ['$g1', reducers.multiplyBy(4)]
-    }
+    compose: [
+      {
+        mapKeys: {
+          g: ['$g2', reducers.multiplyBy(2)]
+        }
+      },
+      {
+        addValues: {
+          g4: 4
+        }
+      },
+      {
+        addKeys: {
+          g4: ['$g1', reducers.multiplyBy(4)]
+        }
+      }
+    ]
   },
   'hash:h.1': {
     value: '$a.e.e1',