Skip to content

Commit

Permalink
chore: improve handling of rollback scenarios (#9680)
Browse files Browse the repository at this point in the history 
imrpove handling of rollback scenarios
  • Loading branch information
@runspired @gitKrystan
runspired authored and gitKrystan committed Feb 12, 2025
1 parent 9f980dd commit bdced6c
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 98 deletions.
124 changes: 108 additions & 16 deletions packages/graph/src/-private/-diff.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { deprecate } from '@ember/debug';

import { DEPRECATE_NON_UNIQUE_PAYLOADS, DISABLE_6X_DEPRECATIONS } from '@warp-drive/build-config/deprecations';
import {
DEPRECATE_NON_UNIQUE_PAYLOADS,
DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE,
DISABLE_6X_DEPRECATIONS,
} from '@warp-drive/build-config/deprecations';
import { DEBUG } from '@warp-drive/build-config/env';
import { assert } from '@warp-drive/build-config/macros';
import type { StableRecordIdentifier } from '@warp-drive/core-types';

import { isBelongsTo } from './-utils';
import { isBelongsTo, notifyChange } from './-utils';
import { assertPolymorphicType } from './debug/assert-polymorphic-type';
import type { CollectionEdge } from './edges/collection';
import type { ResourceEdge } from './edges/resource';
Expand All @@ -20,12 +24,14 @@ function _deprecatedCompare<T>(
prevState: T[],
prevSet: Set<T>,
onAdd: (v: T) => void,
onDel: (v: T) => void
onDel: (v: T) => void,
remoteClearsLocal: boolean
): { duplicates: Map<T, number[]>; diff: Diff<T> } {
const newLength = newState.length;
const prevLength = prevState.length;
const iterationLength = Math.max(newLength, prevLength);
let changed: boolean = newMembers.size !== prevSet.size;
let remoteOrderChanged = false;
const added = new Set<T>();
const removed = new Set<T>();
const duplicates = new Map<T, number[]>();
Expand Down Expand Up @@ -70,11 +76,43 @@ function _deprecatedCompare<T>(
// detect reordering, adjusting index for duplicates
// j is always less than i and so if i < prevLength, j < prevLength
if (member !== prevState[j]) {
changed = true;
} else if (!changed && j < priorLocalLength) {
const priorLocalMember = priorLocalState![j];
if (priorLocalMember !== member) {
changed = true;
// the new remote order does not match the current remote order
// indicating a change in membership or reordering
remoteOrderChanged = true;
// however: if the new remote order matches the current local order
// we can disregard the change notification generation so long as
// we are not configured to reset on remote update (which is deprecated)
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
if (!remoteClearsLocal && i < priorLocalLength) {
const priorLocalMember = priorLocalState![j];
if (priorLocalMember !== member) {
changed = true;
}
} else {
changed = true;
}
} else {
if (i < priorLocalLength) {
const priorLocalMember = priorLocalState![j];
if (priorLocalMember !== member) {
changed = true;
}
} else {
changed = true;
}
}

// if remote order hasn't changed but local order differs
// and we are configured to reset on remote update (which is deprecated)
// then we still need to mark the relationship as changed
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
if (remoteClearsLocal) {
if (!changed && j < priorLocalLength) {
const priorLocalMember = priorLocalState![j];
if (priorLocalMember !== member) {
changed = true;
}
}
}
}

Expand All @@ -98,6 +136,7 @@ function _deprecatedCompare<T>(
finalState,
finalSet,
changed,
remoteOrderChanged,
};

return {
Expand All @@ -113,13 +152,15 @@ function _compare<T>(
prevState: T[],
prevSet: Set<T>,
onAdd: (v: T) => void,
onDel: (v: T) => void
onDel: (v: T) => void,
remoteClearsLocal: boolean
): Diff<T> {
const finalLength = finalState.length;
const prevLength = prevState.length;
const iterationLength = Math.max(finalLength, prevLength);
const equalLength = finalLength === prevLength;
let changed: boolean = finalSet.size !== prevSet.size;
let remoteOrderChanged = false;
const added = new Set<T>();
const removed = new Set<T>();
const priorLocalLength = priorLocalState?.length ?? 0;
Expand All @@ -143,11 +184,43 @@ function _compare<T>(

// detect reordering
if (equalLength && member !== prevMember) {
changed = true;
} else if (equalLength && !changed && i < priorLocalLength) {
const priorLocalMember = priorLocalState![i];
if (priorLocalMember !== prevMember) {
changed = true;
// the new remote order does not match the current remote order
// indicating a change in membership or reordering
remoteOrderChanged = true;
// however: if the new remote order matches the current local order
// we can disregard the change notification generation so long as
// we are not configured to reset on remote update (which is deprecated)
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
if (!remoteClearsLocal && i < priorLocalLength) {
const priorLocalMember = priorLocalState![i];
if (priorLocalMember !== member) {
changed = true;
}
} else {
changed = true;
}
} else {
if (i < priorLocalLength) {
const priorLocalMember = priorLocalState![i];
if (priorLocalMember !== member) {
changed = true;
}
} else {
changed = true;
}
}

// if remote order hasn't changed but local order differs
// and we are configured to reset on remote update (which is deprecated)
// then we still need to mark the relationship as changed
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
if (remoteClearsLocal) {
if (equalLength && !changed && i < priorLocalLength) {
const priorLocalMember = priorLocalState![i];
if (priorLocalMember !== prevMember) {
changed = true;
}
}
}
}

Expand All @@ -165,6 +238,7 @@ function _compare<T>(
finalState,
finalSet,
changed,
remoteOrderChanged,
};
}

Expand All @@ -174,6 +248,7 @@ type Diff<T> = {
finalState: T[];
finalSet: Set<T>;
changed: boolean;
remoteOrderChanged: boolean;
};

export function diffCollection(
Expand All @@ -194,7 +269,8 @@ export function diffCollection(
remoteState,
remoteMembers,
onAdd,
onDel
onDel,
relationship.definition.resetOnRemoteUpdate
);

if (DEBUG) {
Expand All @@ -221,7 +297,16 @@ export function diffCollection(
);
}

return _compare(priorLocalState, finalState, finalSet, remoteState, remoteMembers, onAdd, onDel);
return _compare(
priorLocalState,
finalState,
finalSet,
remoteState,
remoteMembers,
onAdd,
onDel,
relationship.definition.resetOnRemoteUpdate
);
}

export function computeLocalState(storage: CollectionEdge): StableRecordIdentifier[] {
Expand Down Expand Up @@ -421,5 +506,12 @@ export function rollbackRelationship(
},
false
);

// when the change was a "reorder" only we wont have generated
// a notification yet.
// if we give rollback a unique operation we can use the ability of
// diff to report a separate `remoteOrderChanged` flag to trigger this
// if needed to avoid the duplicate.
notifyChange(graph, relationship);
}
}
6 changes: 5 additions & 1 deletion tests/main/ember-cli-build.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

const EmberApp = require('ember-cli/lib/broccoli/ember-app');

function isEnabled(flag) {
return flag === true || flag === 'true' || flag === '1';
}

module.exports = async function (defaults) {
const { setConfig } = await import('@warp-drive/build-config');
const { macros } = await import('@warp-drive/build-config/babel-macros');
Expand Down Expand Up @@ -56,7 +60,7 @@ module.exports = async function (defaults) {
});

setConfig(app, __dirname, {
compatWith: process.env.EMBER_DATA_FULL_COMPAT ? '99.0' : null,
compatWith: isEnabled(process.env.EMBER_DATA_FULL_COMPAT) ? '99.0' : null,
deprecations: {
DISABLE_6X_DEPRECATIONS: false,
},
Expand Down
62 changes: 0 additions & 62 deletions tests/main/tests/integration/application-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
// ensure DS namespace is set
import Application from '@ember/application';
import Controller from '@ember/controller';
import Service, { inject as service } from '@ember/service';

import { module, test } from 'qunit';

import initializeEmberData from 'ember-data/setup-container';
import Store from 'ember-data/store';
import { setupTest } from 'ember-qunit';
import Resolver from 'ember-resolver';

import JSONAPIAdapter from '@ember-data/adapter/json-api';

Expand Down Expand Up @@ -135,62 +132,3 @@ module('integration/application - Using the store as a service', function (hooks
assert.notStrictEqual(store, secondService, 'the store can be used as a service');
});
});

module('integration/application - Attaching initializer', function (hooks) {
hooks.beforeEach(function () {
this.TestApplication = Application.extend({
modulePrefix: '--none',
Resolver,
});
this.TestApplication.initializer({
name: 'ember-data',
initialize: initializeEmberData,
});

this.application = null;
this.owner = null;
});

test('ember-data initializer is run', async function (assert) {
let ran = false;

this.TestApplication.initializer({
name: 'after-ember-data',
after: 'ember-data',
initialize() {
ran = true;
},
});

this.application = this.TestApplication.create({ autoboot: false });

await this.application.boot();

assert.ok(ran, 'ember-data initializer was found');
});

test('ember-data initializer does not register the store service when it was already registered', async function (assert) {
class AppStore extends Store {
isCustomStore = true;
}

this.TestApplication.initializer({
name: 'before-ember-data',
before: 'ember-data',
initialize(registry) {
registry.register('service:store', AppStore);
},
});

this.application = this.TestApplication.create({ autoboot: false });

await this.application.boot();
this.owner = this.application.buildInstance();

const store = this.owner.lookup('service:store');
assert.ok(
store && store.isCustomStore,
'ember-data initializer does not overwrite the previous registered service store'
);
});
});
Loading

0 comments on commit bdced6c

Please sign in to comment.