Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: optimize unload and relationships #8149

Merged
merged 8 commits into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,7 @@ module('autotracking has-many', function (hooks) {

names = findAll('li').map((e) => e.textContent);
assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children');
assert.expectDeprecation({ id: 'ember-data:no-a-with-array-like', count: 6 });
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(6);

let { owner } = this;
let belongsToReturnValue = { data: { id: '1', type: 'person' } };
let belongsToReturnValue;

class RelationshipRecordData extends TestRecordData {
getBelongsTo(key: string) {
Expand Down Expand Up @@ -570,6 +570,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
belongsToReturnValue = { data: store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }) };

store.push({
data: [davidHash, runspiredHash],
Expand All @@ -591,7 +592,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(4);
let { owner } = this;

let belongsToReturnValue = { data: { id: '1', type: 'person' } };
let belongsToReturnValue;

let RelationshipRecordData;
if (V2CACHE_SINGLETON_MANAGER) {
Expand All @@ -607,7 +608,9 @@ module('integration/record-data - Custom RecordData Implementations', function (
key: string,
value: StableRecordIdentifier | null
) {
belongsToReturnValue = { data: { id: '3', type: 'person' } };
belongsToReturnValue = {
data: store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'landlord');
}
};
Expand All @@ -619,7 +622,9 @@ module('integration/record-data - Custom RecordData Implementations', function (
}

setDirtyBelongsTo(this: V1TestRecordData, key: string, recordData: this | null) {
belongsToReturnValue = { data: { id: '3', type: 'person' } };
belongsToReturnValue = {
data: store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'landlord');
}
};
Expand All @@ -638,6 +643,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
belongsToReturnValue = { data: store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }) };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand All @@ -662,8 +668,7 @@ module('integration/record-data - Custom RecordData Implementations', function (

let { owner } = this;

let hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };

let hasManyReturnValue;
class RelationshipRecordData extends TestRecordData {
getHasMany(key: string) {
return hasManyReturnValue;
Expand Down Expand Up @@ -716,6 +721,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand Down Expand Up @@ -747,8 +753,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(10);
let { owner } = this;

let hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };

let hasManyReturnValue;
class RelationshipRecordData extends TestRecordData {
getHasMany(key: string) {
return hasManyReturnValue;
Expand All @@ -770,8 +775,8 @@ module('integration/record-data - Custom RecordData Implementations', function (

hasManyReturnValue = {
data: [
{ id: '3', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -787,7 +792,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(key, 'tenants', 'Passed correct key to removeFromHasMany');
assert.strictEqual(recordDatas[0].getResourceIdentifier().id, '2', 'Passed correct RD to removeFromHasMany');
}
hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
}

Expand All @@ -796,8 +801,8 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(recordDatas[0].getResourceIdentifier().id, '3', 'Passed correct RD to addToHasMany');
hasManyReturnValue = {
data: [
{ id: '1', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -813,8 +818,8 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(values[0].id, '3', 'Passed correct RD to addToHasMany');
hasManyReturnValue = {
data: [
{ id: '1', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -834,6 +839,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { settled } from '@ember/test-helpers';

import { module, test } from 'qunit';
import { resolve } from 'rsvp';

Expand Down Expand Up @@ -85,6 +87,7 @@ module('Store.createRecord() coverage', function (hooks) {
assert.deepEqual(pets, ['Shen'], 'Precondition: Chris has Shen as a pet');

pet.unloadRecord();
await settled();
assert.strictEqual(pet.owner, null, 'Shen no longer has an owner');
// check that the relationship has been dissolved
pets = chris.pets.toArray().map((pet) => pet.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1614,7 +1614,7 @@ module('integration/relationships/one_to_many_test - OneToMany relationships', f

deprecatedTest(
'createRecord updates inverse record array which has observers',
{ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 5 },
{ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 4 },
async function (assert) {
let store = this.owner.lookup('service:store');
let adapter = store.adapterFor('application');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import EmberObject, { computed } from '@ember/object';
import { filterBy } from '@ember/object/computed';
import { settled } from '@ember/test-helpers';

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

import { setupRenderingTest } from 'ember-qunit';

Expand All @@ -14,39 +14,43 @@ import { deprecatedTest } from '@ember-data/unpublished-test-infra/test-support/
module('PromiseManyArray', (hooks) => {
setupRenderingTest(hooks);

test('PromiseManyArray is not side-affected by EmberArray', async function (assert) {
const { owner } = this;
class Person extends Model {
@attr('string') name;
}
class Group extends Model {
@hasMany('person', { async: true, inverse: null }) members;
}
owner.register('model:person', Person);
owner.register('model:group', Group);
const store = owner.lookup('service:store');
const members = ['Bob', 'John', 'Michael', 'Larry', 'Lucy'].map((name) => store.createRecord('person', { name }));
const group = store.createRecord('group', { members });

const forEachFn = group.members.forEach;
assert.strictEqual(group.members.length, 5, 'initial length is correct');

if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 4, 'updated length is correct');
}
deprecatedTest(
'PromiseManyArray is not side-affected by EmberArray',
{ id: 'ember-data:no-a-with-array-like', until: '5.0', count: 1 },
async function (assert) {
const { owner } = this;
class Person extends Model {
@attr('string') name;
}
class Group extends Model {
@hasMany('person', { async: true, inverse: null }) members;
}
owner.register('model:person', Person);
owner.register('model:group', Group);
const store = owner.lookup('service:store');
const members = ['Bob', 'John', 'Michael', 'Larry', 'Lucy'].map((name) => store.createRecord('person', { name }));
const group = store.createRecord('group', { members });

A(group.members);
const forEachFn = group.members.forEach;
assert.strictEqual(group.members.length, 5, 'initial length is correct');

assert.strictEqual(forEachFn, group.members.forEach, 'we have the same function for forEach');
if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 4, 'updated length is correct');
}

A(group.members);

assert.strictEqual(forEachFn, group.members.forEach, 'we have the same function for forEach');

if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 3, 'updated length is correct');
// we'll want to use a different test for this but will want to still ensure we are not side-affected
assert.expectDeprecation({ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 2 });
if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 3, 'updated length is correct');
// we'll want to use a different test for this but will want to still ensure we are not side-affected
assert.expectDeprecation({ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 2 });
}
}
});
);

deprecatedTest(
'PromiseManyArray can be subscribed to by computed chains',
Expand Down Expand Up @@ -130,6 +134,7 @@ module('PromiseManyArray', (hooks) => {
assert.strictEqual(memberIds.length, 6, 'memberIds length is correct');
assert.strictEqual(johnRecords.length, 2, 'johnRecords length is correct');
assert.strictEqual(group.members.length, 6, 'members length is correct');
assert.expectDeprecation({ id: 'ember-data:no-a-with-array-like', count: 2 });
}
);
});
116 changes: 0 additions & 116 deletions packages/-ember-data/tests/unit/many-array-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import { run } from '@ember/runloop';
import { module, test } from 'qunit';
import { resolve } from 'rsvp';

import { gte } from 'ember-compatibility-helpers';
import { setupTest } from 'ember-qunit';

import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import { ManyArray } from '@ember-data/model/-private';

module('unit/many_array - ManyArray', function (hooks) {
setupTest(hooks);
Expand Down Expand Up @@ -76,118 +74,4 @@ module('unit/many_array - ManyArray', function (hooks) {
});
});
});

if (!gte('4.0.0')) {
test('manyArray trigger arrayContentChange functions with the correct values', function (assert) {
assert.expect(6);
class Post extends Model {
@attr('string') title;
@hasMany('tag', { async: false, inverse: 'post' }) tags;
}

class Tag extends Model {
@attr('string') name;
@belongsTo('post', { async: false, inverse: 'tags' }) post;
}

this.owner.register('model:post', Post);
this.owner.register('model:tag', Tag);
const store = this.owner.lookup('service:store');

const TestManyArray = ManyArray.proto();

let willChangeStartIdx;
let willChangeRemoveAmt;
let willChangeAddAmt;

let originalArrayContentWillChange = TestManyArray.arrayContentWillChange;
let originalArrayContentDidChange = TestManyArray.arrayContentDidChange;
let originalInit = TestManyArray.init;

// override ManyArray temp (cleanup occures in afterTest);

TestManyArray.init = function (...args) {
// We aren't actually adding any observers in this test
// just testing the observer codepaths, so we use this to
// force the ManyArray instance to take the observer paths.
this.__hasArrayObservers = true;
originalInit.call(this, ...args);
};

TestManyArray.arrayContentWillChange = function (startIdx, removeAmt, addAmt) {
willChangeStartIdx = startIdx;
willChangeRemoveAmt = removeAmt;
willChangeAddAmt = addAmt;

return originalArrayContentWillChange.apply(this, arguments);
};

TestManyArray.arrayContentDidChange = function (startIdx, removeAmt, addAmt) {
assert.strictEqual(startIdx, willChangeStartIdx, 'WillChange and DidChange startIdx should match');
assert.strictEqual(removeAmt, willChangeRemoveAmt, 'WillChange and DidChange removeAmt should match');
assert.strictEqual(addAmt, willChangeAddAmt, 'WillChange and DidChange addAmt should match');

return originalArrayContentDidChange.apply(this, arguments);
};

try {
run(() => {
store.push({
data: [
{
type: 'tag',
id: '1',
attributes: {
name: 'Ember.js',
},
},
{
type: 'tag',
id: '2',
attributes: {
name: 'Tomster',
},
},
{
type: 'post',
id: '3',
attributes: {
title: 'A framework for creating ambitious web applications',
},
relationships: {
tags: {
data: [{ type: 'tag', id: '1' }],
},
},
},
],
});

store.peekRecord('post', 3).tags;

store.push({
data: {
type: 'post',
id: '3',
attributes: {
title: 'A framework for creating ambitious web applications',
},
relationships: {
tags: {
data: [
{ type: 'tag', id: '1' },
{ type: 'tag', id: '2' },
],
},
},
},
});
});
} finally {
TestManyArray.arrayContentWillChange = originalArrayContentWillChange;
TestManyArray.arrayContentDidChange = originalArrayContentDidChange;
TestManyArray.init = originalInit;
}
});
}
});
Loading