Skip to content

Commit cfc8292

Browse files
runspiredgitKrystan
runspired
authored and
gitKrystan
committed
chore: improve handling of rollback scenarios (#9680)
imrpove handling of rollback scenarios
1 parent bec2db2 commit cfc8292

File tree

5 files changed

+136
-98
lines changed

5 files changed

+136
-98
lines changed

‎packages/graph/src/-private/-diff.ts

+108-16
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { deprecate } from '@ember/debug';
22

3-
import { DEPRECATE_NON_UNIQUE_PAYLOADS, DISABLE_6X_DEPRECATIONS } from '@warp-drive/build-config/deprecations';
3+
import {
4+
DEPRECATE_NON_UNIQUE_PAYLOADS,
5+
DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE,
6+
DISABLE_6X_DEPRECATIONS,
7+
} from '@warp-drive/build-config/deprecations';
48
import { DEBUG } from '@warp-drive/build-config/env';
59
import { assert } from '@warp-drive/build-config/macros';
610
import type { StableRecordIdentifier } from '@warp-drive/core-types';
711

8-
import { isBelongsTo } from './-utils';
12+
import { isBelongsTo, notifyChange } from './-utils';
913
import { assertPolymorphicType } from './debug/assert-polymorphic-type';
1014
import type { CollectionEdge } from './edges/collection';
1115
import type { ResourceEdge } from './edges/resource';
@@ -20,12 +24,14 @@ function _deprecatedCompare<T>(
2024
prevState: T[],
2125
prevSet: Set<T>,
2226
onAdd: (v: T) => void,
23-
onDel: (v: T) => void
27+
onDel: (v: T) => void,
28+
remoteClearsLocal: boolean
2429
): { duplicates: Map<T, number[]>; diff: Diff<T> } {
2530
const newLength = newState.length;
2631
const prevLength = prevState.length;
2732
const iterationLength = Math.max(newLength, prevLength);
2833
let changed: boolean = newMembers.size !== prevSet.size;
34+
let remoteOrderChanged = false;
2935
const added = new Set<T>();
3036
const removed = new Set<T>();
3137
const duplicates = new Map<T, number[]>();
@@ -70,11 +76,43 @@ function _deprecatedCompare<T>(
7076
// detect reordering, adjusting index for duplicates
7177
// j is always less than i and so if i < prevLength, j < prevLength
7278
if (member !== prevState[j]) {
73-
changed = true;
74-
} else if (!changed && j < priorLocalLength) {
75-
const priorLocalMember = priorLocalState![j];
76-
if (priorLocalMember !== member) {
77-
changed = true;
79+
// the new remote order does not match the current remote order
80+
// indicating a change in membership or reordering
81+
remoteOrderChanged = true;
82+
// however: if the new remote order matches the current local order
83+
// we can disregard the change notification generation so long as
84+
// we are not configured to reset on remote update (which is deprecated)
85+
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
86+
if (!remoteClearsLocal && i < priorLocalLength) {
87+
const priorLocalMember = priorLocalState![j];
88+
if (priorLocalMember !== member) {
89+
changed = true;
90+
}
91+
} else {
92+
changed = true;
93+
}
94+
} else {
95+
if (i < priorLocalLength) {
96+
const priorLocalMember = priorLocalState![j];
97+
if (priorLocalMember !== member) {
98+
changed = true;
99+
}
100+
} else {
101+
changed = true;
102+
}
103+
}
104+
105+
// if remote order hasn't changed but local order differs
106+
// and we are configured to reset on remote update (which is deprecated)
107+
// then we still need to mark the relationship as changed
108+
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
109+
if (remoteClearsLocal) {
110+
if (!changed && j < priorLocalLength) {
111+
const priorLocalMember = priorLocalState![j];
112+
if (priorLocalMember !== member) {
113+
changed = true;
114+
}
115+
}
78116
}
79117
}
80118

@@ -98,6 +136,7 @@ function _deprecatedCompare<T>(
98136
finalState,
99137
finalSet,
100138
changed,
139+
remoteOrderChanged,
101140
};
102141

103142
return {
@@ -113,13 +152,15 @@ function _compare<T>(
113152
prevState: T[],
114153
prevSet: Set<T>,
115154
onAdd: (v: T) => void,
116-
onDel: (v: T) => void
155+
onDel: (v: T) => void,
156+
remoteClearsLocal: boolean
117157
): Diff<T> {
118158
const finalLength = finalState.length;
119159
const prevLength = prevState.length;
120160
const iterationLength = Math.max(finalLength, prevLength);
121161
const equalLength = finalLength === prevLength;
122162
let changed: boolean = finalSet.size !== prevSet.size;
163+
let remoteOrderChanged = false;
123164
const added = new Set<T>();
124165
const removed = new Set<T>();
125166
const priorLocalLength = priorLocalState?.length ?? 0;
@@ -143,11 +184,43 @@ function _compare<T>(
143184

144185
// detect reordering
145186
if (equalLength && member !== prevMember) {
146-
changed = true;
147-
} else if (equalLength && !changed && i < priorLocalLength) {
148-
const priorLocalMember = priorLocalState![i];
149-
if (priorLocalMember !== prevMember) {
150-
changed = true;
187+
// the new remote order does not match the current remote order
188+
// indicating a change in membership or reordering
189+
remoteOrderChanged = true;
190+
// however: if the new remote order matches the current local order
191+
// we can disregard the change notification generation so long as
192+
// we are not configured to reset on remote update (which is deprecated)
193+
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
194+
if (!remoteClearsLocal && i < priorLocalLength) {
195+
const priorLocalMember = priorLocalState![i];
196+
if (priorLocalMember !== member) {
197+
changed = true;
198+
}
199+
} else {
200+
changed = true;
201+
}
202+
} else {
203+
if (i < priorLocalLength) {
204+
const priorLocalMember = priorLocalState![i];
205+
if (priorLocalMember !== member) {
206+
changed = true;
207+
}
208+
} else {
209+
changed = true;
210+
}
211+
}
212+
213+
// if remote order hasn't changed but local order differs
214+
// and we are configured to reset on remote update (which is deprecated)
215+
// then we still need to mark the relationship as changed
216+
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
217+
if (remoteClearsLocal) {
218+
if (equalLength && !changed && i < priorLocalLength) {
219+
const priorLocalMember = priorLocalState![i];
220+
if (priorLocalMember !== prevMember) {
221+
changed = true;
222+
}
223+
}
151224
}
152225
}
153226

@@ -165,6 +238,7 @@ function _compare<T>(
165238
finalState,
166239
finalSet,
167240
changed,
241+
remoteOrderChanged,
168242
};
169243
}
170244

@@ -174,6 +248,7 @@ type Diff<T> = {
174248
finalState: T[];
175249
finalSet: Set<T>;
176250
changed: boolean;
251+
remoteOrderChanged: boolean;
177252
};
178253

179254
export function diffCollection(
@@ -194,7 +269,8 @@ export function diffCollection(
194269
remoteState,
195270
remoteMembers,
196271
onAdd,
197-
onDel
272+
onDel,
273+
relationship.definition.resetOnRemoteUpdate
198274
);
199275

200276
if (DEBUG) {
@@ -221,7 +297,16 @@ export function diffCollection(
221297
);
222298
}
223299

224-
return _compare(priorLocalState, finalState, finalSet, remoteState, remoteMembers, onAdd, onDel);
300+
return _compare(
301+
priorLocalState,
302+
finalState,
303+
finalSet,
304+
remoteState,
305+
remoteMembers,
306+
onAdd,
307+
onDel,
308+
relationship.definition.resetOnRemoteUpdate
309+
);
225310
}
226311

227312
export function computeLocalState(storage: CollectionEdge): StableRecordIdentifier[] {
@@ -421,5 +506,12 @@ export function rollbackRelationship(
421506
},
422507
false
423508
);
509+
510+
// when the change was a "reorder" only we wont have generated
511+
// a notification yet.
512+
// if we give rollback a unique operation we can use the ability of
513+
// diff to report a separate `remoteOrderChanged` flag to trigger this
514+
// if needed to avoid the duplicate.
515+
notifyChange(graph, relationship);
424516
}
425517
}

‎tests/main/ember-cli-build.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

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

5+
function isEnabled(flag) {
6+
return flag === true || flag === 'true' || flag === '1';
7+
}
8+
59
module.exports = async function (defaults) {
610
const { setConfig } = await import('@warp-drive/build-config');
711
const { macros } = await import('@warp-drive/build-config/babel-macros');
@@ -56,7 +60,7 @@ module.exports = async function (defaults) {
5660
});
5761

5862
setConfig(app, __dirname, {
59-
compatWith: process.env.EMBER_DATA_FULL_COMPAT ? '99.0' : null,
63+
compatWith: isEnabled(process.env.EMBER_DATA_FULL_COMPAT) ? '99.0' : null,
6064
deprecations: {
6165
DISABLE_6X_DEPRECATIONS: false,
6266
},

‎tests/main/tests/integration/application-test.js

-62
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
// ensure DS namespace is set
2-
import Application from '@ember/application';
32
import Controller from '@ember/controller';
43
import Service, { inject as service } from '@ember/service';
54

65
import { module, test } from 'qunit';
76

8-
import initializeEmberData from 'ember-data/setup-container';
97
import Store from 'ember-data/store';
108
import { setupTest } from 'ember-qunit';
11-
import Resolver from 'ember-resolver';
129

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

@@ -135,62 +132,3 @@ module('integration/application - Using the store as a service', function (hooks
135132
assert.notStrictEqual(store, secondService, 'the store can be used as a service');
136133
});
137134
});
138-
139-
module('integration/application - Attaching initializer', function (hooks) {
140-
hooks.beforeEach(function () {
141-
this.TestApplication = Application.extend({
142-
modulePrefix: '--none',
143-
Resolver,
144-
});
145-
this.TestApplication.initializer({
146-
name: 'ember-data',
147-
initialize: initializeEmberData,
148-
});
149-
150-
this.application = null;
151-
this.owner = null;
152-
});
153-
154-
test('ember-data initializer is run', async function (assert) {
155-
let ran = false;
156-
157-
this.TestApplication.initializer({
158-
name: 'after-ember-data',
159-
after: 'ember-data',
160-
initialize() {
161-
ran = true;
162-
},
163-
});
164-
165-
this.application = this.TestApplication.create({ autoboot: false });
166-
167-
await this.application.boot();
168-
169-
assert.ok(ran, 'ember-data initializer was found');
170-
});
171-
172-
test('ember-data initializer does not register the store service when it was already registered', async function (assert) {
173-
class AppStore extends Store {
174-
isCustomStore = true;
175-
}
176-
177-
this.TestApplication.initializer({
178-
name: 'before-ember-data',
179-
before: 'ember-data',
180-
initialize(registry) {
181-
registry.register('service:store', AppStore);
182-
},
183-
});
184-
185-
this.application = this.TestApplication.create({ autoboot: false });
186-
187-
await this.application.boot();
188-
this.owner = this.application.buildInstance();
189-
190-
const store = this.owner.lookup('service:store');
191-
assert.ok(
192-
store && store.isCustomStore,
193-
'ember-data initializer does not overwrite the previous registered service store'
194-
);
195-
});
196-
});

0 commit comments

Comments
 (0)