Skip to content

Commit

Permalink
fix(Entity): updateOne/updateMany should not change ids state on exis…
Browse files Browse the repository at this point in the history 
…ting entity

Closes #571
  • Loading branch information
@bbaia
bbaia committed Nov 18, 2017
1 parent 4125914 commit 3644258
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 49 deletions.
15 changes: 15 additions & 0 deletions modules/entity/spec/unsorted_state_adapter.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,21 @@ describe('Unsorted State Adapter', () => {
expect(withUpdates).toBe(state);
});

it('should not change ids state if you attempt to update an entity that has already been added', () => {
const withOne = adapter.addOne(TheGreatGatsby, state);
const changes = { title: 'A New Hope' };

const withUpdates = adapter.updateOne(
{
id: TheGreatGatsby.id,
changes,
},
withOne
);

expect(withOne.ids).toBe(withUpdates.ids);
});

it('should let you update the id of entity', () => {
const withOne = adapter.addOne(TheGreatGatsby, state);
const changes = { id: 'A New Id' };
Expand Down
32 changes: 16 additions & 16 deletions modules/entity/src/sorted_state_adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
EntityStateAdapter,
Update,
} from './models';
import { createStateOperator } from './state_adapter';
import { createStateOperator, DidMutate } from './state_adapter';
import { createUnsortedStateAdapter } from './unsorted_state_adapter';

export function createSortedStateAdapter<T>(
Expand All @@ -20,32 +20,32 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
selectId
);

function addOneMutably(entity: T, state: R): boolean;
function addOneMutably(entity: any, state: any): boolean {
function addOneMutably(entity: T, state: R): DidMutate;
function addOneMutably(entity: any, state: any): DidMutate {
return addManyMutably([entity], state);
}

function addManyMutably(newModels: T[], state: R): boolean;
function addManyMutably(newModels: any[], state: any): boolean {
function addManyMutably(newModels: T[], state: R): DidMutate;
function addManyMutably(newModels: any[], state: any): DidMutate {
const models = newModels.filter(
model => !(selectId(model) in state.entities)
);

return merge(models, state);
}

function addAllMutably(models: T[], state: R): boolean;
function addAllMutably(models: any[], state: any): boolean {
function addAllMutably(models: T[], state: R): DidMutate;
function addAllMutably(models: any[], state: any): DidMutate {
state.entities = {};
state.ids = [];

addManyMutably(models, state);

return true;
return DidMutate.Both;
}

function updateOneMutably(update: Update<T>, state: R): boolean;
function updateOneMutably(update: any, state: any): boolean {
function updateOneMutably(update: Update<T>, state: R): DidMutate;
function updateOneMutably(update: any, state: any): DidMutate {
return updateManyMutably([update], state);
}

Expand All @@ -63,8 +63,8 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
models.push(updated);
}

function updateManyMutably(updates: Update<T>[], state: R): boolean;
function updateManyMutably(updates: any[], state: any): boolean {
function updateManyMutably(updates: Update<T>[], state: R): DidMutate;
function updateManyMutably(updates: any[], state: any): DidMutate {
const models: T[] = [];

updates.forEach(update => takeUpdatedModel(models, update, state));
Expand All @@ -76,10 +76,10 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
return merge(models, state);
}

function merge(models: T[], state: R): boolean;
function merge(models: any[], state: any): boolean {
function merge(models: T[], state: R): DidMutate;
function merge(models: any[], state: any): DidMutate {
if (models.length === 0) {
return false;
return DidMutate.None;
}

models.sort(sort);
Expand Down Expand Up @@ -114,7 +114,7 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
state.entities[selectId(model)] = model;
});

return true;
return DidMutate.Both;
}

return {
Expand Down
19 changes: 16 additions & 3 deletions modules/entity/src/state_adapter.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { EntityState, EntityStateAdapter } from './models';

export enum DidMutate {
EntitiesOnly,
Both,
None,
}

export function createStateOperator<V, R>(
mutator: (arg: R, state: EntityState<V>) => boolean
mutator: (arg: R, state: EntityState<V>) => DidMutate
): EntityState<V>;
export function createStateOperator<V, R>(
mutator: (arg: any, state: any) => boolean
mutator: (arg: any, state: any) => DidMutate
): any {
return function operation<S extends EntityState<V>>(arg: R, state: any): S {
const clonedEntityState: EntityState<V> = {
Expand All @@ -14,10 +20,17 @@ export function createStateOperator<V, R>(

const didMutate = mutator(arg, clonedEntityState);

if (didMutate) {
if (didMutate === DidMutate.Both) {
return Object.assign({}, state, clonedEntityState);
}

if (didMutate === DidMutate.EntitiesOnly) {
return {
...state,
entities: clonedEntityState.entities,
};
}

return state;
};
}
71 changes: 41 additions & 30 deletions modules/entity/src/unsorted_state_adapter.ts
View file
Original file line number Diff line number Diff line change
@@ -1,54 +1,55 @@
import { EntityState, EntityStateAdapter, IdSelector, Update } from './models';
import { createStateOperator } from './state_adapter';
import { createStateOperator, DidMutate } from './state_adapter';

export function createUnsortedStateAdapter<T>(
selectId: IdSelector<T>
): EntityStateAdapter<T>;
export function createUnsortedStateAdapter<T>(selectId: IdSelector<T>): any {
type R = EntityState<T>;

function addOneMutably(entity: T, state: R): boolean;
function addOneMutably(entity: any, state: any): boolean {
function addOneMutably(entity: T, state: R): DidMutate;
function addOneMutably(entity: any, state: any): DidMutate {
const key = selectId(entity);

if (key in state.entities) {
return false;
return DidMutate.None;
}

state.ids.push(key);
state.entities[key] = entity;

return true;
return DidMutate.Both;
}

function addManyMutably(entities: T[], state: R): boolean;
function addManyMutably(entities: any[], state: any): boolean {
function addManyMutably(entities: T[], state: R): DidMutate;
function addManyMutably(entities: any[], state: any): DidMutate {
let didMutate = false;

for (let index in entities) {
didMutate = addOneMutably(entities[index], state) || didMutate;
didMutate =
addOneMutably(entities[index], state) !== DidMutate.None || didMutate;
}

return didMutate;
return didMutate ? DidMutate.Both : DidMutate.None;
}

function addAllMutably(entities: T[], state: R): boolean;
function addAllMutably(entities: any[], state: any): boolean {
function addAllMutably(entities: T[], state: R): DidMutate;
function addAllMutably(entities: any[], state: any): DidMutate {
state.ids = [];
state.entities = {};

addManyMutably(entities, state);

return true;
return DidMutate.Both;
}

function removeOneMutably(key: T, state: R): boolean;
function removeOneMutably(key: any, state: any): boolean {
function removeOneMutably(key: T, state: R): DidMutate;
function removeOneMutably(key: any, state: any): DidMutate {
return removeManyMutably([key], state);
}

function removeManyMutably(keys: T[], state: R): boolean;
function removeManyMutably(keys: any[], state: any): boolean {
function removeManyMutably(keys: T[], state: R): DidMutate;
function removeManyMutably(keys: any[], state: any): DidMutate {
const didMutate =
keys
.filter(key => key in state.entities)
Expand All @@ -58,7 +59,7 @@ export function createUnsortedStateAdapter<T>(selectId: IdSelector<T>): any {
state.ids = state.ids.filter((id: any) => id in state.entities);
}

return didMutate;
return didMutate ? DidMutate.Both : DidMutate.None;
}

function removeAll<S extends R>(state: S): S;
Expand All @@ -78,38 +79,48 @@ export function createUnsortedStateAdapter<T>(selectId: IdSelector<T>): any {
keys: { [id: string]: any },
update: Update<T>,
state: any
): void {
): boolean {
const original = state.entities[update.id];
const updated: T = Object.assign({}, original, update.changes);
const newKey = selectId(updated);
const hasNewKey = newKey !== update.id;

if (newKey !== update.id) {
if (hasNewKey) {
keys[update.id] = newKey;
delete state.entities[update.id];
}

state.entities[newKey] = updated;

return hasNewKey;
}

function updateOneMutably(update: Update<T>, state: R): boolean;
function updateOneMutably(update: any, state: any): boolean {
function updateOneMutably(update: Update<T>, state: R): DidMutate;
function updateOneMutably(update: any, state: any): DidMutate {
return updateManyMutably([update], state);
}

function updateManyMutably(updates: Update<T>[], state: R): boolean;
function updateManyMutably(updates: any[], state: any): boolean {
function updateManyMutably(updates: Update<T>[], state: R): DidMutate;
function updateManyMutably(updates: any[], state: any): DidMutate {
const newKeys: { [id: string]: string } = {};

const didMutate =
updates
.filter(update => update.id in state.entities)
.map(update => takeNewKey(newKeys, update, state)).length > 0;
updates = updates.filter(update => update.id in state.entities);

if (didMutate) {
state.ids = state.ids.map((id: any) => newKeys[id] || id);
const didMutateEntities = updates.length > 0;

if (didMutateEntities) {
const didMutateIds =
updates.filter(update => takeNewKey(newKeys, update, state)).length > 0;

if (didMutateIds) {
state.ids = state.ids.map((id: any) => newKeys[id] || id);
return DidMutate.Both;
} else {
return DidMutate.EntitiesOnly;
}
}

return didMutate;
return DidMutate.None;
}

return {
Expand Down

0 comments on commit 3644258

Please sign in to comment.