From 46b62b93ed0e450ea947b76f2fece49455ef538b Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Fri, 29 Sep 2017 02:37:02 +0000
Subject: [PATCH 01/10] Improve performance for sorted entity adapter

Prior to this fix, collection methods in sorted_state_adapter
ran in quadratic time, because they iterated over the collection
with single-item mutators, each of which also running in linear time.

Now, we first collect all modifications (each in constant time),
then sort them in linearithmic time, and finally merge
them with the existing sorted array linearly.

As a result, we can improve performance of collection methods
to N + M log M, where N is the number of existing items
and M is the number of modifications.

Single-item methods are now expressed through those
for collections, still running linearly.
---
 modules/entity/src/sorted_state_adapter.ts | 117 +++++++++------------
 1 file changed, 52 insertions(+), 65 deletions(-)

diff --git a/modules/entity/src/sorted_state_adapter.ts b/modules/entity/src/sorted_state_adapter.ts
index 13ece9f8e5..7adf81e8d0 100644
--- a/modules/entity/src/sorted_state_adapter.ts
+++ b/modules/entity/src/sorted_state_adapter.ts
@@ -20,102 +20,89 @@ export function createSortedStateAdapter<T>(
   );
 
   function addOneMutably(entity: T, state: R): boolean {
-    const key = selectId(entity);
-
-    if (key in state.entities) {
-      return false;
-    }
-
-    const insertAt = findTargetIndex(state, entity);
-    state.ids.splice(insertAt, 0, key);
-    state.entities[key] = entity;
-
-    return true;
+    return addManyMutably([entity], state);
   }
 
   function addManyMutably(newModels: T[], state: R): boolean {
-    let didMutate = false;
-
-    for (let index in newModels) {
-      didMutate = addOneMutably(newModels[index], state) || didMutate;
-    }
+    const models = newModels.filter(
+      model => !(selectId(model) in state.entities)
+    );
 
-    return didMutate;
+    return merge(models, state);
   }
 
   function addAllMutably(models: T[], state: R): boolean {
-    const sortedModels = models.sort(sort);
-
     state.entities = {};
-    state.ids = sortedModels.map(model => {
-      const id = selectId(model);
-      state.entities[id] = model;
-      return id;
-    });
+    state.ids = [];
 
-    return true;
+    return addManyMutably(models, state);
   }
 
   function updateOneMutably(update: Update<T>, state: R): boolean {
+    return updateManyMutably([update], state);
+  }
+
+  function takeUpdatedModel(models: T[], update: Update<T>, state: R): void {
     if (!(update.id in state.entities)) {
-      return false;
+      return;
     }
 
     const original = state.entities[update.id];
-    const updated: T = Object.assign({}, original, update.changes);
-    const updatedKey = selectId(updated);
-    const result = sort(original, updated);
-
-    if (result === 0) {
-      if (updatedKey !== update.id) {
-        delete state.entities[update.id];
-        const index = state.ids.indexOf(update.id);
-        state.ids[index] = updatedKey;
-      }
+    const updated = Object.assign({}, original, update.changes);
 
-      state.entities[updatedKey] = updated;
-
-      return true;
-    }
+    delete state.entities[update.id];
 
-    const index = state.ids.indexOf(update.id);
-    state.ids.splice(index, 1);
-    state.ids.splice(findTargetIndex(state, updated), 0, updatedKey);
-
-    if (updatedKey !== update.id) {
-      delete state.entities[update.id];
-    }
-
-    state.entities[updatedKey] = updated;
-
-    return true;
+    models.push(updated);
   }
 
   function updateManyMutably(updates: Update<T>[], state: R): boolean {
-    let didMutate = false;
+    const models: T[] = [];
 
-    for (let index in updates) {
-      didMutate = updateOneMutably(updates[index], state) || didMutate;
-    }
+    updates.forEach(update => takeUpdatedModel(models, update, state));
 
-    return didMutate;
+    return merge(models, state);
   }
 
-  function findTargetIndex(state: R, model: T) {
-    if (state.ids.length === 0) {
-      return 0;
+  function merge(models: T[], state: R): boolean {
+    if (models.length === 0) {
+      return false;
     }
 
-    for (let i = 0; i < state.ids.length; i++) {
-      const entity = state.entities[state.ids[i]];
-      const isSmaller = sort(model, entity) < 0;
+    models.sort(sort);
+
+    state.ids = state.ids.filter(id => id in state.entities);
 
-      if (isSmaller) {
-        return i;
+    const ids: string[] = [];
+
+    let i = 0;
+    let j = 0;
+
+    while (i < models.length && j < state.ids.length) {
+      const model = models[i];
+      const modelId = selectId(model);
+      const entityId = state.ids[j];
+      const entity = state.entities[entityId];
+
+      if (sort(model, entity) <= 0) {
+        ids.push(modelId);
+        i++;
+      } else {
+        ids.push(entityId);
+        j++;
       }
     }
 
-    return state.ids.length - 1;
+    if (i < models.length) {
+      state.ids = ids.concat(models.slice(i).map(selectId));
+    } else {
+      state.ids = ids.concat(state.ids.slice(j));
+    }
+
+    models.forEach((model, i) => {
+      state.entities[selectId(model)] = model;
+    });
+
+    return true;
   }
 
   return {

From 6d2999c7c7665174d0d759e2935e00894933c77e Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Fri, 29 Sep 2017 03:16:09 +0000
Subject: [PATCH 02/10] Remove updateManyMutably() functionality from merge()
 in sorted_state_adapter

Removing "holes" created by updateManyMutably()
should be there instead of merge().
---
 modules/entity/src/sorted_state_adapter.ts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/modules/entity/src/sorted_state_adapter.ts b/modules/entity/src/sorted_state_adapter.ts
index 7adf81e8d0..8fca92f53b 100644
--- a/modules/entity/src/sorted_state_adapter.ts
+++ b/modules/entity/src/sorted_state_adapter.ts
@@ -60,6 +60,10 @@ export function createSortedStateAdapter<T>(
 
     updates.forEach(update => takeUpdatedModel(models, update, state));
 
+    if (models.length) {
+      state.ids = state.ids.filter(id => id in state.entities);
+    }
+
     return merge(models, state);
   }
 
@@ -70,8 +74,6 @@ export function createSortedStateAdapter<T>(
 
     models.sort(sort);
 
-    state.ids = state.ids.filter(id => id in state.entities);
-
     const ids: string[] = [];
 
     let i = 0;

From ee9067cc8622ba57c1581d00924cbbb50cd64d29 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Fri, 29 Sep 2017 03:55:07 +0000
Subject: [PATCH 03/10] Fix return value from addAllMutably() in
 sorted_state_adapter

It should return "true" unconditionally (probably worth a test).
---
 modules/entity/src/sorted_state_adapter.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/modules/entity/src/sorted_state_adapter.ts b/modules/entity/src/sorted_state_adapter.ts
index 8fca92f53b..ad95428e52 100644
--- a/modules/entity/src/sorted_state_adapter.ts
+++ b/modules/entity/src/sorted_state_adapter.ts
@@ -35,7 +35,9 @@ export function createSortedStateAdapter<T>(
     state.entities = {};
     state.ids = [];
 
-    return addManyMutably(models, state);
+    addManyMutably(models, state);
+
+    return true;
   }
 
   function updateOneMutably(update: Update<T>, state: R): boolean {

From 8dcb54ce82371e74fbc122725d994edba78a3c88 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Fri, 29 Sep 2017 18:02:16 +0000
Subject: [PATCH 04/10] Improve performance of collection methods for unsorted
 state adapter

These methods ran in quadratic time by interating over
the collection and applying a linear algorithm for each item.

Now, we modify entity hashes first (each in constant time),
and then update the array of their ids in linear time.

As a result, all operations now occur linearly.

We also simplified logic for single-item operations
by expressing them through those for collections.

The removal methods also apply to sorted collections
(because they preserve their order).
---
 modules/entity/src/unsorted_state_adapter.ts | 46 +++++++++++---------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/modules/entity/src/unsorted_state_adapter.ts b/modules/entity/src/unsorted_state_adapter.ts
index 3f1cad07e7..67d4d3bf10 100644
--- a/modules/entity/src/unsorted_state_adapter.ts
+++ b/modules/entity/src/unsorted_state_adapter.ts
@@ -39,23 +39,17 @@ export function createUnsortedStateAdapter<T>(
   }
 
   function removeOneMutably(key: string, state: R): boolean {
-    const index = state.ids.indexOf(key);
-
-    if (index === -1) {
-      return false;
-    }
-
-    state.ids.splice(index, 1);
-    delete state.entities[key];
-
-    return true;
+    return removeManyMutably([key], state);
   }
 
   function removeManyMutably(keys: string[], state: R): boolean {
-    let didMutate = false;
+    const didMutate =
+      keys
+        .filter(key => key in state.entities)
+        .map(key => delete state.entities[key]).length > 0;
 
-    for (let index in keys) {
-      didMutate = removeOneMutably(keys[index], state) || didMutate;
+    if (didMutate) {
+      state.ids = state.ids.filter(id => id in state.entities);
     }
 
     return didMutate;
@@ -68,10 +62,12 @@ export function createUnsortedStateAdapter<T>(
     });
   }
 
-  function updateOneMutably(update: Update<T>, state: R): boolean {
-    const index = state.ids.indexOf(update.id);
-
-    if (index === -1) {
+  function takeNewKey(
+    keys: { [id: string]: string },
+    update: Update<T>,
+    state: R
+  ): boolean {
+    if (!(update.id in state.entities)) {
       return false;
     }
 
@@ -80,7 +76,7 @@ export function createUnsortedStateAdapter<T>(
     const newKey = selectId(updated);
 
     if (newKey !== update.id) {
-      state.ids[index] = newKey;
+      keys[update.id] = newKey;
       delete state.entities[update.id];
     }
 
@@ -89,11 +85,19 @@ export function createUnsortedStateAdapter<T>(
     return true;
   }
 
+  function updateOneMutably(update: Update<T>, state: R): boolean {
+    return updateManyMutably([update], state);
+  }
+
   function updateManyMutably(updates: Update<T>[], state: R): boolean {
-    let didMutate = false;
+    const newKeys: { [id: string]: string } = {};
+
+    const didMutate = updates
+      .map(update => takeNewKey(newKeys, update, state))
+      .includes(true);
 
-    for (let index in updates) {
-      didMutate = updateOneMutably(updates[index], state) || didMutate;
+    if (didMutate) {
+      state.ids = state.ids.map(id => newKeys[id] || id);
     }
 
     return didMutate;

From 8efba7094086038aeaf92d7b90d096a9c4b4adf3 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Fri, 29 Sep 2017 18:16:40 +0000
Subject: [PATCH 05/10] Fix for unsupported includes() in
 unsorted_state_adapter

CircleCI failed the build because includes() is not supported
by its runtime yet. We use an alternative syntax to fix that.
---
 modules/entity/src/unsorted_state_adapter.ts | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/modules/entity/src/unsorted_state_adapter.ts b/modules/entity/src/unsorted_state_adapter.ts
index 67d4d3bf10..5f23f2d8e9 100644
--- a/modules/entity/src/unsorted_state_adapter.ts
+++ b/modules/entity/src/unsorted_state_adapter.ts
@@ -66,11 +66,7 @@ export function createUnsortedStateAdapter<T>(
     keys: { [id: string]: string },
     update: Update<T>,
     state: R
-  ): boolean {
-    if (!(update.id in state.entities)) {
-      return false;
-    }
-
+  ): void {
     const original = state.entities[update.id];
     const updated: T = Object.assign({}, original, update.changes);
     const newKey = selectId(updated);
@@ -81,8 +77,6 @@ export function createUnsortedStateAdapter<T>(
     }
 
     state.entities[newKey] = updated;
-
-    return true;
   }
 
   function updateOneMutably(update: Update<T>, state: R): boolean {
@@ -92,9 +86,10 @@ export function createUnsortedStateAdapter<T>(
   function updateManyMutably(updates: Update<T>[], state: R): boolean {
     const newKeys: { [id: string]: string } = {};
 
-    const didMutate = updates
-      .map(update => takeNewKey(newKeys, update, state))
-      .includes(true);
+    const didMutate =
+      updates
+        .filter(update => update.id in state.entities)
+        .map(update => takeNewKey(newKeys, update, state)).length > 0;
 
     if (didMutate) {
       state.ids = state.ids.map(id => newKeys[id] || id);

From 2b9cc50ff5a5a1eac0df48918657bf4ddcccc484 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Tue, 17 Oct 2017 01:04:23 +0000
Subject: [PATCH 06/10] Expose effectMetadata() useful in testing

This PR exposes a method to get metadata supplied
through @Effect() decorator.

This method is useful for when we want to test
that an effect was properly decorated/registered
in the store.

Related issue: https://github.com/ngrx/platform/issues/491
---
 docs/effects/testing.md                       | 42 ++++++++++++++++++-
 modules/effects/spec/effects_metadata.spec.ts | 29 +++++++++++++
 modules/effects/src/effects_metadata.ts       | 13 ++++++
 modules/effects/src/index.ts                  |  2 +-
 4 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/docs/effects/testing.md b/docs/effects/testing.md
index b3112255c0..d8996dfb91 100644
--- a/docs/effects/testing.md
+++ b/docs/effects/testing.md
@@ -47,6 +47,46 @@ describe('My Effects', () => {
     effects.someSource$.subscribe(result => {
       expect(result).toEqual(AnotherAction);
     });
-  });  
+  });
+});
+```
+
+### effectMetadata
+Returns decorator configuration for an effect in an instance of effects.
+Use this function to ensure that an effect has been properly decorated.
+
+If the decorator was not supplied, the result is `undefined`.
+
+Usage:
+```ts
+import { TestBed } from '@angular/core/testing';
+import { effectMetadata } from '@ngrx/effects';
+import { MyEffects } from './my-effects';
+
+describe('My Effects', () => {
+  let effects: MyEffects;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      providers: [
+        MyEffects,
+        // other providers
+      ],
+    });
+
+    effects = TestBed.get(MyEffects);
+  });
+
+  it('should register someSource$', () => {
+    expect(effectMetadata(effects, 'someSource$')).toEqual({
+      dispatch: true,
+    });
+  });
+
+  it('should register someOtherSource$', () => {
+    expect(effectMetadata(effects, 'someOtherSource$')).toEqual({
+      dispatch: false,
+    });
+  });
 });
 ```
\ No newline at end of file
diff --git a/modules/effects/spec/effects_metadata.spec.ts b/modules/effects/spec/effects_metadata.spec.ts
index 223927a19f..3215384278 100644
--- a/modules/effects/spec/effects_metadata.spec.ts
+++ b/modules/effects/spec/effects_metadata.spec.ts
@@ -1,5 +1,6 @@
 import {
   Effect,
+  effectMetadata,
   getSourceMetadata,
   getSourceForInstance,
 } from '../src/effects_metadata';
@@ -46,4 +47,32 @@ describe('Effect Metadata', () => {
       expect(proto).toBe(Fixture.prototype);
     });
   });
+
+  describe('effectMetadata', () => {
+    it('should get the effect metadata for a class instance with known effect name', () => {
+      class Fixture {
+        @Effect() a$: any;
+        @Effect({ dispatch: true })
+        b$: any;
+        @Effect({ dispatch: false })
+        c$: any;
+      }
+
+      const mock = new Fixture();
+
+      expect(effectMetadata(mock, 'a$')).toEqual({ dispatch: true });
+      expect(effectMetadata(mock, 'b$')).toEqual({ dispatch: true });
+      expect(effectMetadata(mock, 'c$')).toEqual({ dispatch: false });
+    });
+
+    it('should return "undefined" if the effect has not been decorated', () => {
+      class Fixture {
+        a$: any;
+      }
+
+      const mock = new Fixture();
+
+      expect(effectMetadata(mock, 'a$')).toBeUndefined();
+    });
+  });
 });
diff --git a/modules/effects/src/effects_metadata.ts b/modules/effects/src/effects_metadata.ts
index 92cc4e3bc3..ca70b1d045 100644
--- a/modules/effects/src/effects_metadata.ts
+++ b/modules/effects/src/effects_metadata.ts
@@ -43,3 +43,16 @@ export const getSourceMetadata = compose(
   getEffectMetadataEntries,
   getSourceForInstance
 );
+
+export interface PartialEffectMetadata {
+  dispatch: boolean;
+}
+
+export function effectMetadata(
+  instance: any,
+  effectName: string
+): PartialEffectMetadata {
+  return getSourceMetadata(instance)
+    .filter(({ propertyName }) => propertyName === effectName)
+    .map(({ dispatch }) => ({ dispatch }))[0];
+}
diff --git a/modules/effects/src/index.ts b/modules/effects/src/index.ts
index 691f22608a..95ca711fcb 100644
--- a/modules/effects/src/index.ts
+++ b/modules/effects/src/index.ts
@@ -1,4 +1,4 @@
-export { Effect } from './effects_metadata';
+export { Effect, effectMetadata } from './effects_metadata';
 export { mergeEffects } from './effects_resolver';
 export { Actions } from './actions';
 export { EffectsModule } from './effects_module';

From f06e744472994f377b875bab569c662737c56525 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Tue, 17 Oct 2017 03:09:08 +0000
Subject: [PATCH 07/10] Shorter syntax for effects metadata

This commit replaces earlier function effectMetadata()
with a map accessor getEffectsMetadata()
and associated EffectsMetadata class.

This syntax enables enables more concise access to metadata
in unit tests.

This is particularly nice, as we can now test
both the observables and their metadata in a similar way:
```
expect(effects.someSource$).toBeObservable(expected);
expect(metadata.someSource$).toEqual({ dispatch: true });
```
---
 docs/effects/testing.md                       | 28 +++++++++----------
 modules/effects/spec/effects_metadata.spec.ts | 28 +++++++++++--------
 modules/effects/src/effects_metadata.ts       | 21 ++++++++------
 modules/effects/src/index.ts                  |  6 +++-
 4 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/docs/effects/testing.md b/docs/effects/testing.md
index d8996dfb91..d1bf16a7d2 100644
--- a/docs/effects/testing.md
+++ b/docs/effects/testing.md
@@ -51,20 +51,19 @@ describe('My Effects', () => {
 });
 ```
 
-### effectMetadata
-Returns decorator configuration for an effect in an instance of effects.
-Use this function to ensure that an effect has been properly decorated.
-
-If the decorator was not supplied, the result is `undefined`.
+### effectsMetadata
+Returns decorator configuration for all effects in a class instance.
+Use this function to ensure that effects have been properly decorated.
 
 Usage:
 ```ts
 import { TestBed } from '@angular/core/testing';
-import { effectMetadata } from '@ngrx/effects';
+import { effectsMetadata, EffectsMetadata } from '@ngrx/effects';
 import { MyEffects } from './my-effects';
 
 describe('My Effects', () => {
   let effects: MyEffects;
+  let metadata: EffectsMetadata;
 
   beforeEach(() => {
     TestBed.configureTestingModule({
@@ -75,18 +74,19 @@ describe('My Effects', () => {
     });
 
     effects = TestBed.get(MyEffects);
+    metadata = getEffectsMetadata(effects);
   });
 
-  it('should register someSource$', () => {
-    expect(effectMetadata(effects, 'someSource$')).toEqual({
-      dispatch: true,
-    });
+  it('should register someSource$ that dispatches an action', () => {
+    expect(metadata.someSource$).toEqual({ dispatch: true });
   });
 
-  it('should register someOtherSource$', () => {
-    expect(effectMetadata(effects, 'someOtherSource$')).toEqual({
-      dispatch: false,
-    });
+  it('should register someOtherSource$ that does not dispatch an action', () => {
+    expect(metadata.someOtherSource$).toEqual({ dispatch: false });
+  });
+
+  it('should not register undecoratedSource$', () => {
+    expect(metadata.undecoratedSource$).toBeUndefined();
   });
 });
 ```
\ No newline at end of file
diff --git a/modules/effects/spec/effects_metadata.spec.ts b/modules/effects/spec/effects_metadata.spec.ts
index 3215384278..42e80deede 100644
--- a/modules/effects/spec/effects_metadata.spec.ts
+++ b/modules/effects/spec/effects_metadata.spec.ts
@@ -1,6 +1,6 @@
 import {
   Effect,
-  effectMetadata,
+  getEffectsMetadata,
   getSourceMetadata,
   getSourceForInstance,
 } from '../src/effects_metadata';
@@ -48,31 +48,35 @@ describe('Effect Metadata', () => {
     });
   });
 
-  describe('effectMetadata', () => {
-    it('should get the effect metadata for a class instance with known effect name', () => {
+  describe('getEffectsMetadata', () => {
+    it('should get map of metadata for all decorated effects in a class instance', () => {
       class Fixture {
-        @Effect() a$: any;
+        @Effect() a: any;
         @Effect({ dispatch: true })
-        b$: any;
+        b: any;
         @Effect({ dispatch: false })
-        c$: any;
+        c: any;
       }
 
       const mock = new Fixture();
 
-      expect(effectMetadata(mock, 'a$')).toEqual({ dispatch: true });
-      expect(effectMetadata(mock, 'b$')).toEqual({ dispatch: true });
-      expect(effectMetadata(mock, 'c$')).toEqual({ dispatch: false });
+      expect(getEffectsMetadata(mock)).toEqual({
+        a: { dispatch: true },
+        b: { dispatch: true },
+        c: { dispatch: false },
+      });
     });
 
-    it('should return "undefined" if the effect has not been decorated', () => {
+    it('should return an empty map if the class has not been decorated', () => {
       class Fixture {
-        a$: any;
+        a: any;
+        b: any;
+        c: any;
       }
 
       const mock = new Fixture();
 
-      expect(effectMetadata(mock, 'a$')).toBeUndefined();
+      expect(getEffectsMetadata(mock)).toEqual({});
     });
   });
 });
diff --git a/modules/effects/src/effects_metadata.ts b/modules/effects/src/effects_metadata.ts
index 1617c08e69..c5210c4f43 100644
--- a/modules/effects/src/effects_metadata.ts
+++ b/modules/effects/src/effects_metadata.ts
@@ -41,15 +41,18 @@ export const getSourceMetadata = compose(
   getSourceForInstance
 );
 
-export interface PartialEffectMetadata {
-  dispatch: boolean;
+export interface EffectsMetadata {
+  [propertyName: string]: {
+    dispatch: boolean;
+  };
 }
 
-export function effectMetadata(
-  instance: any,
-  effectName: string
-): PartialEffectMetadata {
-  return getSourceMetadata(instance)
-    .filter(({ propertyName }) => propertyName === effectName)
-    .map(({ dispatch }) => ({ dispatch }))[0];
+export function getEffectsMetadata(instance: any): EffectsMetadata {
+  const metadata: EffectsMetadata = {};
+
+  getSourceMetadata(instance).forEach(({ propertyName, dispatch }) => {
+    metadata[propertyName] = { dispatch };
+  });
+
+  return metadata;
 }
diff --git a/modules/effects/src/index.ts b/modules/effects/src/index.ts
index 95ca711fcb..f54bf2c415 100644
--- a/modules/effects/src/index.ts
+++ b/modules/effects/src/index.ts
@@ -1,4 +1,8 @@
-export { Effect, effectMetadata } from './effects_metadata';
+export {
+  Effect,
+  EffectsMetadata,
+  getEffectsMetadata,
+} from './effects_metadata';
 export { mergeEffects } from './effects_resolver';
 export { Actions } from './actions';
 export { EffectsModule } from './effects_module';

From 0673d12563f72d197ccc45d4c5bb73d5817647b6 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Tue, 17 Oct 2017 03:24:08 +0000
Subject: [PATCH 08/10] Fix docs for getEffectsMetadata()

Earlier version contained incorrect function name.
---
 docs/effects/testing.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/effects/testing.md b/docs/effects/testing.md
index d1bf16a7d2..1e466f6077 100644
--- a/docs/effects/testing.md
+++ b/docs/effects/testing.md
@@ -51,14 +51,14 @@ describe('My Effects', () => {
 });
 ```
 
-### effectsMetadata
+### getEffectsMetadata
 Returns decorator configuration for all effects in a class instance.
 Use this function to ensure that effects have been properly decorated.
 
 Usage:
 ```ts
 import { TestBed } from '@angular/core/testing';
-import { effectsMetadata, EffectsMetadata } from '@ngrx/effects';
+import { EffectsMetadata, getEffectsMetadata } from '@ngrx/effects';
 import { MyEffects } from './my-effects';
 
 describe('My Effects', () => {

From 1bc563e8e8bc933181c82d58ebc4bf6aed2125b7 Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Tue, 17 Oct 2017 19:22:02 +0000
Subject: [PATCH 09/10] Add key type checking for EffectsMetadata

This change converts EffectsMetadata to a generic type
that requires its keys to be keys of the effects class.

We also update getEffectsMetadata() accordingly.
---
 docs/effects/testing.md                 |  2 +-
 modules/effects/src/effects_metadata.ts | 18 ++++++++++--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/docs/effects/testing.md b/docs/effects/testing.md
index 1e466f6077..4e42e0c732 100644
--- a/docs/effects/testing.md
+++ b/docs/effects/testing.md
@@ -63,7 +63,7 @@ import { MyEffects } from './my-effects';
 
 describe('My Effects', () => {
   let effects: MyEffects;
-  let metadata: EffectsMetadata;
+  let metadata: EffectsMetadata<MyEffects>;
 
   beforeEach(() => {
     TestBed.configureTestingModule({
diff --git a/modules/effects/src/effects_metadata.ts b/modules/effects/src/effects_metadata.ts
index c5210c4f43..3efe9e033d 100644
--- a/modules/effects/src/effects_metadata.ts
+++ b/modules/effects/src/effects_metadata.ts
@@ -41,14 +41,16 @@ export const getSourceMetadata = compose(
   getSourceForInstance
 );
 
-export interface EffectsMetadata {
-  [propertyName: string]: {
-    dispatch: boolean;
-  };
-}
-
-export function getEffectsMetadata(instance: any): EffectsMetadata {
-  const metadata: EffectsMetadata = {};
+export type EffectsMetadata<T> = {
+  [key in keyof T]?:
+    | undefined
+    | {
+        dispatch: boolean;
+      }
+};
+
+export function getEffectsMetadata<T>(instance: T): EffectsMetadata<T> {
+  const metadata: EffectsMetadata<T> = {};
 
   getSourceMetadata(instance).forEach(({ propertyName, dispatch }) => {
     metadata[propertyName] = { dispatch };

From 9826793e8c522f467b229298692e069450fdf00c Mon Sep 17 00:00:00 2001
From: Denis Loginov <dloginov@mit.edu>
Date: Mon, 6 Nov 2017 19:58:59 +0000
Subject: [PATCH 10/10] Implement upsertOne() and upsertMany()

These methods are used to update one ore more entities
if they already exist in the state, or to add them
if they don't.

The signature of the methods is identical to that of
the corresponding update...() methods.
---
 .../entity/spec/sorted_state_adapter.spec.ts  | 68 +++++++++++++++++++
 .../spec/unsorted_state_adapter.spec.ts       | 68 +++++++++++++++++++
 modules/entity/src/models.ts                  |  3 +
 modules/entity/src/sorted_state_adapter.ts    | 28 ++++++++
 modules/entity/src/unsorted_state_adapter.ts  | 28 ++++++++
 5 files changed, 195 insertions(+)

diff --git a/modules/entity/spec/sorted_state_adapter.spec.ts b/modules/entity/spec/sorted_state_adapter.spec.ts
index ca4fe2ef48..21f61acd5a 100644
--- a/modules/entity/spec/sorted_state_adapter.spec.ts
+++ b/modules/entity/spec/sorted_state_adapter.spec.ts
@@ -231,4 +231,72 @@ describe('Sorted State Adapter', () => {
       },
     });
   });
+
+  it('should let you add one entity to the state with upsert()', () => {
+    const withOneEntity = adapter.upsertOne(
+      {
+        id: TheGreatGatsby.id,
+        changes: TheGreatGatsby,
+      },
+      state
+    );
+
+    expect(withOneEntity).toEqual({
+      ids: [TheGreatGatsby.id],
+      entities: {
+        [TheGreatGatsby.id]: TheGreatGatsby,
+      },
+    });
+  });
+
+  it('should let you update an entity in the state with upsert()', () => {
+    const withOne = adapter.addOne(TheGreatGatsby, state);
+    const changes = { title: 'A New Hope' };
+
+    const withUpdates = adapter.upsertOne(
+      {
+        id: TheGreatGatsby.id,
+        changes,
+      },
+      withOne
+    );
+
+    expect(withUpdates).toEqual({
+      ids: [TheGreatGatsby.id],
+      entities: {
+        [TheGreatGatsby.id]: {
+          ...TheGreatGatsby,
+          ...changes,
+        },
+      },
+    });
+  });
+
+  it('should let you upsert many entities in the state', () => {
+    const firstChange = { title: 'Zack' };
+    const secondChange = { title: 'Aaron' };
+    const withMany = adapter.addAll([TheGreatGatsby], state);
+
+    const withUpserts = adapter.upsertMany(
+      [
+        { id: TheGreatGatsby.id, changes: firstChange },
+        { id: AClockworkOrange.id, changes: secondChange },
+      ],
+      withMany
+    );
+
+    expect(withUpserts).toEqual({
+      ids: [AClockworkOrange.id, TheGreatGatsby.id],
+      entities: {
+        [TheGreatGatsby.id]: {
+          ...TheGreatGatsby,
+          ...firstChange,
+        },
+        [AClockworkOrange.id]: {
+          ...AClockworkOrange,
+          ...secondChange,
+        },
+      },
+    });
+  });
 });
diff --git a/modules/entity/spec/unsorted_state_adapter.spec.ts b/modules/entity/spec/unsorted_state_adapter.spec.ts
index 941ab87088..e61a7cb281 100644
--- a/modules/entity/spec/unsorted_state_adapter.spec.ts
+++ b/modules/entity/spec/unsorted_state_adapter.spec.ts
@@ -202,4 +202,72 @@ describe('Unsorted State Adapter', () => {
       },
     });
   });
+
+  it('should let you add one entity to the state with upsert()', () => {
+    const withOneEntity = adapter.upsertOne(
+      {
+        id: TheGreatGatsby.id,
+        changes: TheGreatGatsby,
+      },
+      state
+    );
+
+    expect(withOneEntity).toEqual({
+      ids: [TheGreatGatsby.id],
+      entities: {
+        [TheGreatGatsby.id]: TheGreatGatsby,
+      },
+    });
+  });
+
+  it('should let you update an entity in the state with upsert()', () => {
+    const withOne = adapter.addOne(TheGreatGatsby, state);
+    const changes = { title: 'A New Hope' };
+
+    const withUpdates = adapter.upsertOne(
+      {
+        id: TheGreatGatsby.id,
+        changes,
+      },
+      withOne
+    );
+
+    expect(withUpdates).toEqual({
+      ids: [TheGreatGatsby.id],
+      entities: {
+        [TheGreatGatsby.id]: {
+          ...TheGreatGatsby,
+          ...changes,
+        },
+      },
+    });
+  });
+
+  it('should let you upsert many entities in the state', () => {
+    const firstChange = { title: 'First Change' };
+    const secondChange = { title: 'Second Change' };
+    const withMany = adapter.addAll([TheGreatGatsby], state);
+
+    const withUpserts = adapter.upsertMany(
+      [
+        { id: TheGreatGatsby.id, changes: firstChange },
+        { id: AClockworkOrange.id, changes: secondChange },
+      ],
+      withMany
+    );
+
+    expect(withUpserts).toEqual({
+      ids: [TheGreatGatsby.id, AClockworkOrange.id],
+      entities: {
+        [TheGreatGatsby.id]: {
+          ...TheGreatGatsby,
+          ...firstChange,
+        },
+        [AClockworkOrange.id]: {
+          ...AClockworkOrange,
+          ...secondChange,
+        },
+      },
+    });
+  });
 });
diff --git a/modules/entity/src/models.ts b/modules/entity/src/models.ts
index 416bb6ae30..9f763391b9 100644
--- a/modules/entity/src/models.ts
+++ b/modules/entity/src/models.ts
@@ -63,6 +63,9 @@ export interface EntityStateAdapter<T> {
 
   updateOne<S extends EntityState<T>>(update: Update<T>, state: S): S;
   updateMany<S extends EntityState<T>>(updates: Update<T>[], state: S): S;
+
+  upsertOne<S extends EntityState<T>>(update: Update<T>, state: S): S;
+  upsertMany<S extends EntityState<T>>(updates: Update<T>[], state: S): S;
 }
 
 export type EntitySelectors<T, V> = {
diff --git a/modules/entity/src/sorted_state_adapter.ts b/modules/entity/src/sorted_state_adapter.ts
index 4ae8e694b6..c98dcaaec8 100644
--- a/modules/entity/src/sorted_state_adapter.ts
+++ b/modules/entity/src/sorted_state_adapter.ts
@@ -76,6 +76,32 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
     return merge(models, state);
   }
 
+  function upsertOneMutably(update: Update<T>, state: R): boolean;
+  function upsertOneMutably(update: any, state: any): boolean {
+    return upsertManyMutably([update], state);
+  }
+
+  function upsertManyMutably(updates: Update<T>[], state: R): boolean;
+  function upsertManyMutably(updates: any[], state: any): boolean {
+    const added: T[] = [];
+    const updated: Update<T>[] = [];
+
+    for (let index in updates) {
+      const update = updates[index];
+      if (update.id in state.entities) {
+        updated.push(update);
+      } else {
+        added.push({
+          ...update.changes,
+          id: update.id,
+        });
+      }
+    }
+
+    const didMutate = updateManyMutably(updated, state);
+    return addManyMutably(added, state) || didMutate;
+  }
+
   function merge(models: T[], state: R): boolean;
   function merge(models: any[], state: any): boolean {
     if (models.length === 0) {
@@ -123,8 +149,10 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
     removeAll,
     addOne: createStateOperator(addOneMutably),
     updateOne: createStateOperator(updateOneMutably),
+    upsertOne: createStateOperator(upsertOneMutably),
     addAll: createStateOperator(addAllMutably),
     addMany: createStateOperator(addManyMutably),
     updateMany: createStateOperator(updateManyMutably),
+    upsertMany: createStateOperator(upsertManyMutably),
   };
 }
diff --git a/modules/entity/src/unsorted_state_adapter.ts b/modules/entity/src/unsorted_state_adapter.ts
index 37057137ab..43186684be 100644
--- a/modules/entity/src/unsorted_state_adapter.ts
+++ b/modules/entity/src/unsorted_state_adapter.ts
@@ -112,6 +112,32 @@ export function createUnsortedStateAdapter<T>(selectId: IdSelector<T>): any {
     return didMutate;
   }
 
+  function upsertOneMutably(update: Update<T>, state: R): boolean;
+  function upsertOneMutably(update: any, state: any): boolean {
+    return upsertManyMutably([update], state);
+  }
+
+  function upsertManyMutably(updates: Update<T>[], state: R): boolean;
+  function upsertManyMutably(updates: any[], state: any): boolean {
+    const added: T[] = [];
+    const updated: Update<T>[] = [];
+
+    for (let index in updates) {
+      const update = updates[index];
+      if (update.id in state.entities) {
+        updated.push(update);
+      } else {
+        added.push({
+          ...update.changes,
+          id: update.id,
+        });
+      }
+    }
+
+    const didMutate = updateManyMutably(updated, state);
+    return addManyMutably(added, state) || didMutate;
+  }
+
   return {
     removeAll,
     addOne: createStateOperator(addOneMutably),
@@ -119,6 +145,8 @@ export function createUnsortedStateAdapter<T>(selectId: IdSelector<T>): any {
     addAll: createStateOperator(addAllMutably),
     updateOne: createStateOperator(updateOneMutably),
     updateMany: createStateOperator(updateManyMutably),
+    upsertOne: createStateOperator(upsertOneMutably),
+    upsertMany: createStateOperator(upsertManyMutably),
     removeOne: createStateOperator(removeOneMutably),
     removeMany: createStateOperator(removeManyMutably),
   };