Skip to content

Commit

Permalink
fix: dont orphan new records in implicit relationships
Browse files Browse the repository at this point in the history
  • Loading branch information
@runspired
runspired committed Aug 26, 2023
1 parent db1efc8 commit f2afc14
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 33 deletions.
2 changes: 1 addition & 1 deletion packages/graph/src/-private/graph.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export class Graph {
continue;
}
assert(`Expected a relationship`, relationship);
if (relationship.definition.inverseIsAsync) {
if (relationship.definition.inverseIsAsync && !isNew(identifier)) {
if (LOG_GRAPH) {
// eslint-disable-next-line no-console
console.log(`graph: <<NOT>> RELEASABLE ${String(identifier)}`);
Expand Down
15 changes: 2 additions & 13 deletions packages/json-api/src/-private/cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,11 +1025,7 @@ export default class JSONAPICache implements Cache {
}

if (cached.isNew) {
this.__graph.push({
op: 'deleteRecord',
record: identifier,
isNew: true,
});
// > Note: Graph removal handled by unloadRecord
cached.isDeleted = true;
cached.isNew = false;
}
Expand Down Expand Up @@ -1083,14 +1079,7 @@ export default class JSONAPICache implements Cache {
setIsDeleted(identifier: StableRecordIdentifier, isDeleted: boolean): void {
const cached = this.__peek(identifier, false);
cached.isDeleted = isDeleted;
if (cached.isNew) {
// TODO can we delete this since we will do this in unload?
this.__graph.push({
op: 'deleteRecord',
record: identifier,
isNew: true,
});
}
// > Note: Graph removal for isNew handled by unloadRecord
this.__storeWrapper.notifyChange(identifier, 'state');
}

Expand Down
6 changes: 4 additions & 2 deletions tests/main/tests/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@
// the reporter is configured to ignore them. This results
// in extremely high memory and cpu utilization and eventually
// will crash the console.
const ignoredLogTypes = new Set(['log', 'warn', 'info', 'group']);
const ignoredLogTypes = new Set(['log', 'warn', 'info', 'group', 'debug', 'groupEnd', 'error', 'trace', 'dir']);
// restore original functionality
ignoredLogTypes.forEach((method) => {
console[method] = Testem.console[method];
if (Testem.console[method]) {
console[method] = Testem.console[method];
}
});

// enable debugging memory over time
Expand Down
65 changes: 48 additions & 17 deletions tests/main/tests/integration/records/new-record-unload-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { module, test } from 'qunit';

import { setupTest } from 'ember-qunit';

import Model, { attr, hasMany } from '@ember-data/model';
import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import testInDebug from '@ember-data/unpublished-test-infra/test-support/test-in-debug';

class Person extends Model {
@attr name;
@hasMany('person', { async: true, inverse: 'friends' }) friends;
@belongsTo('person', { async: true, inverse: null }) bestFriend;
}

module('Integration | Records | New Record Unload', function (hooks) {
Expand Down Expand Up @@ -60,13 +61,12 @@ module('Integration | Records | New Record Unload', function (hooks) {
assert.strictEqual(people.length, 1, 'precond - one person left in the store');
});

test('Rolling Back Attributes on multiple New (related) Records unloads them safely', async function (assert) {
test('Rolling Back Attributes on multiple New (related via async self-reflexive HasMany) Records unloads them safely', async function (assert) {
const store = this.owner.lookup('service:store');
let Pat = store.createRecord('person', { name: 'Patrick Wachter' });
let Matt = store.createRecord('person', { name: 'Matthew Seidel' });
const Pat = store.createRecord('person', { name: 'Patrick Wachter' });
const Matt = store.createRecord('person', { name: 'Matthew Seidel', friends: [Pat] });
const friends = Matt.hasMany('friends').value();
friends.push(Pat);
let people = store.peekAll('person');
const people = store.peekAll('person');

assert.strictEqual(friends.length, 1, 'Matt has friends');
assert.strictEqual(people.length, 2, 'precond - two people records in the store');
Expand All @@ -77,28 +77,59 @@ module('Integration | Records | New Record Unload', function (hooks) {

Pat.rollbackAttributes();

assert.false(Pat.isDestroyed, 'record is not yet destroyed');
assert.true(Pat.isDestroying, 'record is destroying');
assert.false(Pat.isDestroyed, 'Pat record is not yet destroyed');
assert.true(Pat.isDestroying, 'Pat record is destroying');
assert.strictEqual(friends.length, 0, 'Matt has no friends');
assert.strictEqual(people.length, 1, 'precond - one person left in the store');

Matt.rollbackAttributes();
assert.false(Matt.isDestroyed, 'Matt record is not yet destroyed');
assert.true(Matt.isDestroying, 'Matt record is destroying');
assert.strictEqual(people.length, 0, 'precond - no people left in the store');

await settled();

assert.true(Pat.isDestroyed, 'record is destroyed');
assert.true(Pat.isDestroying, 'record is destroying');
assert.strictEqual(friends.length, 0, 'Matt has no friends');
assert.true(Pat.isDestroyed, 'Pat record is destroyed');
assert.true(Pat.isDestroying, 'Pat record is destroying');
assert.true(Matt.isDestroyed, 'Matt record is destroyed');
assert.true(Matt.isDestroying, 'Matt record is destroying');
assert.strictEqual(people.length, 0, 'precond - no people left in the store');
});

test('Rolling Back Attributes on multiple New (related via async belongsTo with no inverse) Records unloads them safely', async function (assert) {
const store = this.owner.lookup('service:store');
const Pat = store.createRecord('person', { name: 'Patrick Wachter' });
const Matt = store.createRecord('person', { name: 'Matthew Seidel', bestFriend: Pat });
let bestFriend = Matt.belongsTo('bestFriend').value();
const people = store.peekAll('person');

assert.strictEqual(people.length, 2, 'precond - two people records in the store');
assert.strictEqual(bestFriend, Pat, 'Matt has a best friend');
assert.true(Matt.hasDirtyAttributes, 'precond - record has dirty attributes');
assert.true(Matt.isNew, 'precond - record is new');
assert.true(Pat.hasDirtyAttributes, 'precond - record has dirty attributes');
assert.true(Pat.isNew, 'precond - record is new');

Pat.rollbackAttributes();

bestFriend = Matt.belongsTo('bestFriend').value();
assert.strictEqual(bestFriend, null, 'Matt has no best friend');
assert.false(Pat.isDestroyed, 'Pat record is not yet destroyed');
assert.true(Pat.isDestroying, 'Pat record is destroying');
assert.strictEqual(people.length, 1, 'precond - one person left in the store');

Matt.rollbackAttributes();
assert.false(Matt.isDestroyed, 'record is not yet destroyed');
assert.true(Matt.isDestroying, 'record is destroying');
assert.strictEqual(people.length, 0, 'precond - no person left in the store');
assert.false(Matt.isDestroyed, 'Matt record is not yet destroyed');
assert.true(Matt.isDestroying, 'Matt record is destroying');
assert.strictEqual(people.length, 0, 'precond - no people left in the store');

await settled();

assert.true(Matt.isDestroyed, 'record is destroyed');
assert.true(Matt.isDestroying, 'record is destroying');
assert.strictEqual(people.length, 0, 'precond - one person left in the store');
assert.true(Pat.isDestroyed, 'Pat record is destroyed');
assert.true(Pat.isDestroying, 'Pat record is destroying');
assert.true(Matt.isDestroyed, 'Matt record is destroyed');
assert.true(Matt.isDestroying, 'Matt record is destroying');
assert.strictEqual(people.length, 0, 'precond - no people left in the store');
});

test('Unload on a New Record unloads that record safely', async function (assert) {
Expand Down

0 comments on commit f2afc14

Please sign in to comment.