Skip to content

Commit

Permalink
fix more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired committed Sep 1, 2023
1 parent 3acd356 commit 6c650ec
Show file tree
Hide file tree
Showing 13 changed files with 662 additions and 384 deletions.
2 changes: 2 additions & 0 deletions @types/ember-data-qunit-asserts/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ declare global {
expectNoWarning(callback: () => unknown): Promise<void>;
expectAssertion(callback: () => unknown, matcher: string | RegExp): Promise<void>;
expectNoAssertion(callback: () => unknown): Promise<void>;
arrayStrictEquals<T>(actual: T[], expected: T[], message: string): void;
}

namespace QUnit {
Expand All @@ -33,6 +34,7 @@ declare global {
expectNoWarning(callback: () => unknown): Promise<void>;
expectAssertion(callback: () => unknown, matcher: string | RegExp): Promise<void>;
expectNoAssertion(callback: () => unknown): Promise<void>;
arrayStrictEquals<T>(actual: T[], expected: T[], message: string): void;
}
}

Expand Down
88 changes: 85 additions & 3 deletions packages/graph/src/-private/-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ export function _addLocal(
graph: Graph,
record: StableRecordIdentifier,
relationship: CollectionEdge,
value: StableRecordIdentifier
value: StableRecordIdentifier,
index: number | null
): boolean {
const { remoteMembers, removals } = relationship;
let additions = relationship.additions;
Expand Down Expand Up @@ -256,7 +257,25 @@ export function _addLocal(
}
}

relationship.isDirty = true;
// if we have existing localState
// and we have an index
// apply the change, as this is more efficient
// than recomputing localState and
// it allows us to preserve local ordering
// to a small extend. Local ordering should not
// be relied upon as any remote change will blow it away
if (relationship.localState) {
if (index !== null) {
relationship.localState.splice(index, 0, value);
} else {
relationship.localState.push(value);
}
}
assert(
`Expected relationship to be dirty when adding a local mutation`,
relationship.localState || relationship.isDirty
);

return true;
}

Expand Down Expand Up @@ -284,6 +303,69 @@ export function _removeLocal(relationship: CollectionEdge, value: StableRecordId
removals.add(value);
}

relationship.isDirty = true;
// if we have existing localState
// apply the change, as this is more efficient
// than recomputing localState and
// it allows us to preserve local ordering
// to a small extend. Local ordering should not
// be relied upon as any remote change will blow it away
if (relationship.localState) {
const index = relationship.localState.indexOf(value);
assert(`Cannot remove a resource that is not present`, index !== -1);
relationship.localState.splice(index, 1);
}
assert(
`Expected relationship to be dirty when performing a local mutation`,
relationship.localState || relationship.isDirty
);

return true;
}

export function _removeRemote(relationship: CollectionEdge, value: StableRecordIdentifier): boolean {
assert(`expected an identifier to remove from the collection relationship`, value);
const { remoteMembers, additions, removals, remoteState } = relationship;

assert(`Cannot remove a resource that is not present`, remoteMembers.has(value));
if (!remoteMembers.has(value)) {
return false;
}

// remove from remote state
remoteMembers.delete(value);
let index = remoteState.indexOf(value);
assert(`Cannot remove a resource that is not present`, index !== -1);
remoteState.splice(index, 1);

// remove from removals if present
if (removals?.has(value)) {
removals.delete(value);

// nothing more to do this was our state already
return false;
}

assert(
`Remote state indicated removal of a resource that was present only as a local mutation`,
!additions?.has(value)
);

// if we have existing localState
// and we have an index
// apply the change, as this is more efficient
// than recomputing localState and
// it allows us to preserve local ordering
// to a small extend. Local ordering should not
// be relied upon as any remote change will blow it away
if (relationship.localState) {
index = relationship.localState.indexOf(value);
assert(`Cannot remove a resource that is not present`, index !== -1);
relationship.localState.splice(index, 1);
}
assert(
`Expected relationship to be dirty when performing a local mutation`,
relationship.localState || relationship.isDirty
);

return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function addRelatedRecord(
) {
assert(`expected an identifier to add to the collection relationship`, value);

if (_addLocal(graph, record, relationship, value)) {
if (_addLocal(graph, record, relationship, value, index ?? null)) {
addToInverse(graph, value, relationship.definition.inverseKey, record, isRemote);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export default function removeFromRelatedRecords(graph: Graph, op: RemoveFromRel
`You can only '${op.op}' on a hasMany relationship. ${record.type}.${op.field} is a ${relationship.definition.kind}`,
isHasMany(relationship)
);
// TODO we should potentially thread the index information through here
// when available as it may make it faster to remove from the local state
// when trying to patch more efficiently without blowing away the entire
// local state array
if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) {
removeRelatedRecord(graph, relationship, record, value[i], isRemote);
Expand Down
63 changes: 57 additions & 6 deletions packages/graph/src/-private/operations/replace-related-record.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { assert } from '@ember/debug';
import { assert, deprecate } from '@ember/debug';

import { DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE } from '@ember-data/deprecations';
import { DEBUG } from '@ember-data/env';
import type { StableRecordIdentifier } from '@ember-data/types/q/identifier';

Expand Down Expand Up @@ -81,10 +82,31 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
}
if (existingState && localState === existingState) {
notifyInverseOfPotentialMaterialization(graph, existingState, definition.inverseKey, op.record, isRemote);
} else {
relationship.localState = existingState;

notifyChange(graph, relationship.identifier, relationship.definition.key);
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
// if localState does not match existingState then we know
// we have a local mutation that has not been persisted yet
if (localState !== op.value) {
relationship.localState = existingState;

deprecate(
`EmberData is changing the default semantics of updates to the remote state of relationships.\n\nThe following local state was cleared from the <${
relationship.identifier.type
}>.${
relationship.definition.key
} belongsTo relationship but will not be once this deprecation is resolved:\n\n\t${
localState ? 'Added: ' + localState.lid + '\n\t' : ''
}${existingState ? 'Removed: ' + existingState.lid : ''}`,
false,
{
id: 'ember-data:deprecate-relationship-remote-update-clearing-local-state',
for: 'ember-data',
since: { enabled: '5.3', available: '5.3' },
until: '6.0',
}
);

notifyChange(graph, relationship.identifier, relationship.definition.key);
}
}
}
return;
Expand Down Expand Up @@ -127,9 +149,38 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
if (localState && isNew(localState) && !remoteState) {
return;
}
if (localState !== remoteState) {
// when localState does not match the new remoteState and
// localState === existingState then we had no local mutation
// and we can safely sync the new remoteState to local
if (localState !== remoteState && localState === existingState) {
relationship.localState = remoteState;
notifyChange(graph, relationship.identifier, relationship.definition.key);
// But when localState does not match the new remoteState and
// and localState !== existingState then we know we have a local mutation
// that has not been persisted yet.
} else if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
if (localState !== remoteState && localState !== existingState) {
relationship.localState = existingState;

deprecate(
`EmberData is changing the default semantics of updates to the remote state of relationships.\n\nThe following local state was cleared from the <${
relationship.identifier.type
}>.${
relationship.definition.key
} belongsTo relationship but will not be once this deprecation is resolved:\n\n\t${
localState ? 'Added: ' + localState.lid + '\n\t' : ''
}${existingState ? 'Removed: ' + existingState.lid : ''}`,
false,
{
id: 'ember-data:deprecate-relationship-remote-update-clearing-local-state',
for: 'ember-data',
since: { enabled: '5.3', available: '5.3' },
until: '6.0',
}
);

notifyChange(graph, relationship.identifier, relationship.definition.key);
}
}
} else {
notifyChange(graph, relationship.identifier, relationship.definition.key);
Expand Down
42 changes: 21 additions & 21 deletions packages/graph/src/-private/operations/replace-related-records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE } from '@embe
import { DEBUG } from '@ember-data/env';
import type { StableRecordIdentifier } from '@ember-data/types/q/identifier';

import { _addLocal, _removeLocal, diffCollection } from '../-diff';
import { _addLocal, _removeLocal, _removeRemote, diffCollection } from '../-diff';
import type { ReplaceRelatedRecordsOperation } from '../-operations';
import { isBelongsTo, isHasMany, isNew, notifyChange } from '../-utils';
import { assertPolymorphicType } from '../debug/assert-polymorphic-type';
Expand Down Expand Up @@ -178,12 +178,16 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
relationship.remoteMembers = diff.finalSet;
relationship.remoteState = diff.finalState;

// changed also indicates a change in order
if (diff.changed) {
relationship.isDirty = true;
}

// TODO unsure if we need this but it
// may allow us to more efficiently patch
// the associated ManyArray
relationship._diff = diff;

debugger;
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
// only do this for legacy hasMany, not collection
// and provide a way to incrementally migrate
Expand All @@ -198,9 +202,9 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
triggered: false,
};
if (relationship.removals) {
deprecationInfo.triggered = true;
relationship.isDirty = true;
relationship.removals.forEach((identifier) => {
deprecationInfo.triggered = true;
deprecationInfo.removals.push(identifier);
// reverse the removal
// if we are still in removals at this point then
Expand Down Expand Up @@ -232,7 +236,11 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper

if (deprecationInfo.triggered) {
deprecate(
`EmberData is changing the default semantics of updates to the remote state of relationships.\n\nThe following local state was cleared but will not be once this deprecation is resolved:\n\n\tAdded: [${deprecationInfo.additions
`EmberData is changing the default semantics of updates to the remote state of relationships.\n\nThe following local state was cleared from the <${
relationship.identifier.type
}>.${
relationship.definition.key
} hasMany relationship but will not be once this deprecation is resolved:\n\n\tAdded: [${deprecationInfo.additions
.map((i) => i.lid)
.join(', ')}]\n\tRemoved: [${deprecationInfo.removals.map((i) => i.lid).join(', ')}]`,
false,
Expand All @@ -247,14 +255,7 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
}
}

if (diff.changed) {
flushCanonical(graph, relationship);
} else {
// TODO
// this is because we used to eagerly notify on remote updates which
// has the effect of clearing certain kinds of local changes.
// preserve legacy behavior we want to change but requires some sort
// of deprecation.
if (relationship.isDirty) {
flushCanonical(graph, relationship);
}
}
Expand Down Expand Up @@ -297,6 +298,11 @@ export function addToInverse(
}
} else if (isHasMany(relationship)) {
if (isRemote) {
// TODO this needs to alert stuffs
// And patch state better
// This is almost definitely wrong
// WARNING WARNING WARNING

if (!relationship.remoteMembers.has(value)) {
graph._addToTransaction(relationship);
relationship.remoteState.push(value);
Expand All @@ -310,7 +316,7 @@ export function addToInverse(
}
}
} else {
if (_addLocal(graph, identifier, relationship, value)) {
if (_addLocal(graph, identifier, relationship, value, null)) {
notifyChange(graph, identifier, key);
}
}
Expand Down Expand Up @@ -363,15 +369,9 @@ export function removeFromInverse(
}
} else if (isHasMany(relationship)) {
if (isRemote) {
// TODO this needs to alert stuffs
// And patch state better
// This is almost definitely wrong
// WARNING WARNING WARNING
graph._addToTransaction(relationship);
let index = relationship.remoteState.indexOf(value);
if (index !== -1) {
relationship.remoteMembers.delete(value);
relationship.remoteState.splice(index, 1);
if (_removeRemote(relationship, value)) {
notifyChange(graph, identifier, key);
}
} else {
if (_removeLocal(relationship, value)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import QUnit from 'qunit';

let HAS_REGISTERED = false;

function refFromIndex(index: number, suffix: string): string {
return `<ref:@${index}${suffix}>`;
}
function getRefForItem<T>(map: Map<T, string>, item: T, index: number): string {
let ref = map.get(item);
if (ref === undefined) {
ref = refFromIndex(index, 'b');
}
return ref;
}

export function configureBetterAsserts() {
if (HAS_REGISTERED === true) {
throw new Error(`Attempting to re-register better assertion tools`);
}
HAS_REGISTERED = true;

QUnit.assert.arrayStrictEquals = function <T>(actual: T[], expected: T[], message: string): void {
let passed = actual.length === expected.length;

let actualRefs = new Map<T, string>();
let actualSerialized: string[] = actual.map((item, index) => {
let ref = refFromIndex(index, '');
actualRefs.set(item, ref);
return ref;
});
let expectedSerialized: string[] = expected.map((item, index) => {
return getRefForItem(actualRefs, item, index);
});

if (passed) {
for (let i = 0; i < actual.length; i++) {
if (actual[i] !== expected[i]) {
passed = false;
break;
}
}
}

this.pushResult({
result: passed,
actual: actualSerialized,
expected: expectedSerialized,
message,
});
};
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { configureAssertionHandler } from './assert-assertion';
import { configureBetterAsserts } from './assert-better';
import { configureDeprecationHandler } from './assert-deprecation';
import { configureWarningHandler } from './assert-warning';

export default function configureAsserts() {
configureAssertionHandler();
configureDeprecationHandler();
configureWarningHandler();
configureBetterAsserts();
}
Loading

0 comments on commit 6c650ec

Please sign in to comment.