Skip to content

Commit

Permalink
Merge pull request #77 from embermap/assert-must-preload-and-load-all
Browse files Browse the repository at this point in the history
Assert must preload and loadRecords
  • Loading branch information
ryanto authored Apr 10, 2019
2 parents 4a9789c + 370d8e1 commit 4c26a29
Show file tree
Hide file tree
Showing 9 changed files with 246 additions and 17 deletions.
13 changes: 13 additions & 0 deletions addon/-private/record-array-query.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,17 @@ export default class RecordArrayQuery {
return promise;
}

trackIncludes() {
let includes = this.params && this.params.include;
let models = this.value;

if (includes && models) {
models
.filter(model => model.trackLoadedIncludes)
.forEach((model) => {
model.trackLoadedIncludes(includes);
});
}
}

}
111 changes: 107 additions & 4 deletions addon/mixins/loadable-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,112 @@ export default Mixin.create({
},

/**
A list of models for a given relationship. It's always normalized to a list,
even for belongsTo, null, or unloaded relationships.
@method _getRelationshipModels
@private
*/
_getRelationshipModels(name) {
let reference = this._getReference(name);
let info = this._getRelationshipInfo(name);
let models;

if (info.kind === 'hasMany') {
models = reference.value() || [];
} else if (info.kind === 'belongsTo') {
models = reference.value() ? [ reference.value() ] : [];
}

return models;
},

/**
This is a private method because we may refactor it in the future to have
a difference signature. However, this method is used by other
storefront objects. So, it's really public, but don't use it in app code!
@method trackLoadedIncludes
@param {String} includes A full include path. Example: "author,comments.author,tags"
@private
*/
trackLoadedIncludes(includes) {
includes.split(",").forEach(path => this._trackLoadedIncludePath(path));
},

/**
Tracks a single include path as being loaded.
@method _trackLoadedIncludePath
@param {String} path A single include path. Example: "comments.author"
@private
*/
_trackLoadedIncludePath(path) {
let [firstInclude, ...rest] = path.split(".");
let relationship = camelize(firstInclude);
let reference = this._getReference(relationship);

if (reference) {
this._loadedReferences[relationship] = true;

if (rest.length) {
this._getRelationshipModels(relationship)
.filter(model => model.trackLoadedIncludes)
.forEach(model => model.trackLoadedIncludes(rest.join('.')));
}
}
},

/**
This method can take an include string and see if the graph of objects
in the store related to this model have all loaded each of the elements
in that include string.
@method _graphHasLoaded
@param {String} includes A full include path. Example: "author,comments.author,tags"
@return {Boolean} True if the includes have been loaded, false if not
@private
*/
_graphHasLoaded(includes) {
return includes
.split(",")
.every(path => this._graphHasLoadedPath(path));
},

/**
Checks wether a single include path has been loaded.
@method _graphHasLoadedPath
@param {String} path A single include path. Example: "comments.author"
@return {Boolean} True if the path has been loaded, false if not
@private
*/
_graphHasLoadedPath(includePath) {
let [firstInclude, ...rest] = includePath.split(".");
let relationship = camelize(firstInclude);
let reference = this._getReference(relationship);
let hasLoaded = reference && this._hasLoadedReference(relationship);

if (rest.length === 0) {
return hasLoaded;

} else {
let models = this._getRelationshipModels(relationship);

let childrenHaveLoaded = models.every(model => {
return model.trackLoadedIncludes && model._graphHasLoaded(rest.join("."));
});

return hasLoaded && childrenHaveLoaded;
}
},

/**
Checks if storefront has ever loaded this reference.
@method _hasLoadedReference
@param {String} name Reference or relationshipname name.
@return {Boolean} True if storefront has loaded the reference.
@private
*/
_hasLoadedReference(name) {
Expand All @@ -242,10 +347,8 @@ export default Mixin.create({
*/
hasLoaded(includesString) {
let modelName = this.constructor.modelName;
let hasSideloaded = this.get('store').hasLoadedIncludesForRecord(modelName, this.get('id'), includesString);
let hasLoaded = this._hasLoadedReference(camelize(includesString));

return hasLoaded || hasSideloaded;
return this.get('store').hasLoadedIncludesForRecord(modelName, this.get('id'), includesString) ||
this._graphHasLoaded(includesString);
}

});
8 changes: 5 additions & 3 deletions addon/mixins/loadable-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,20 @@ export default Mixin.create({
let shouldBlock = options.reload || !query.value;
let shouldBackgroundReload = !options.hasOwnProperty('backgroundReload') || options.backgroundReload;
let promise;
let fetcher;

if (shouldBlock) {
promise = query.run();
fetcher = promise;

} else {
promise = resolve(query.value);

if (shouldBackgroundReload) {
query.run();
}
fetcher = shouldBackgroundReload ? query.run() : resolve();
}

fetcher.then(() => query.trackIncludes());

return promise;
},

Expand Down
7 changes: 5 additions & 2 deletions tests/dummy/app/models/author.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import DS from 'ember-data';

export default DS.Model.extend({

comments: DS.hasMany()

name: DS.attr('string'),

comments: DS.hasMany(),
post: DS.belongsTo()

});
2 changes: 2 additions & 0 deletions tests/dummy/app/models/comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import DS from 'ember-data';

export default DS.Model.extend({

text: DS.attr('string'),

post: DS.belongsTo(),
author: DS.belongsTo()

Expand Down
2 changes: 1 addition & 1 deletion tests/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
{{content-for "head-footer"}}
{{content-for "test-head-footer"}}
</head>
<body>
<body style="background-color: white !important;">
{{content-for "body"}}
{{content-for "test-body"}}

Expand Down
63 changes: 58 additions & 5 deletions tests/integration/components/assert-must-preload-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ module('Integration | Component | assert must preload', function(hooks) {
// the next line doesn't work in 2.x due to an eslint rule
// eslint-disable-next-line
[ this.major, this.minor ] = Ember.VERSION.split(".");

// setup a bunch of data that our tests will load
let author = this.server.create('author');
let post = this.server.create('post', {
id: 1,
title: 'Post title',
author
});
let comments = this.server.createList('comment', 3, { post });

comments.forEach(comment => {
server.create('author', { comments: [comment] });
});
});

hooks.afterEach(function() {
Expand All @@ -41,7 +54,6 @@ module('Integration | Component | assert must preload', function(hooks) {
});

test('it errors if the relationship has not yet be loaded', async function(assert) {
this.server.create('post');
this.post = await run(() => {
return this.store.loadRecord('post', 1);
});
Expand All @@ -64,7 +76,6 @@ module('Integration | Component | assert must preload', function(hooks) {
});

test('it errors if one of the relationships has not yet be loaded', async function(assert) {
this.server.create('post');
this.post = await run(() => {
return this.store.loadRecord('post', 1, { include: 'author' });
});
Expand All @@ -87,7 +98,6 @@ module('Integration | Component | assert must preload', function(hooks) {
});

test('it errors if a nested relationship has not yet be loaded', async function(assert) {
this.server.create('post');
this.post = await run(() => {
return this.store.loadRecord('post', 1, { include: 'comments' });
});
Expand All @@ -110,7 +120,6 @@ module('Integration | Component | assert must preload', function(hooks) {
});

test('it does not error if the relationship was loaded', async function(assert) {
this.server.create('post');
this.post = await run(() => {
return this.store.loadRecord('post', 1, { include: 'comments' });
});
Expand All @@ -119,8 +128,52 @@ module('Integration | Component | assert must preload', function(hooks) {
{{assert-must-preload post "comments"}}
`);

// if nothing renders, we're ok
// if anything renders, we're ok
assert.dom('*').hasText('');
});

module('Data loaded with loadRecords', function() {
test('it should not error when all data is loaded', async function(assert) {
let posts = await run(() => {
return this.store.loadRecords('post', { include: 'comments' });
});

this.post = posts.get('firstObject');

await render(hbs`
{{assert-must-preload post "comments"}}
<div data-test-id="title">
{{post.title}}
</div>
`);

assert.dom('[data-test-id="title"]').hasText("Post title");
});

test('it should error is not all data is loaded', async function(assert) {
let posts = await run(() => {
return this.store.loadRecords('post', { include: 'comments,author' });
});

this.post = posts.get('firstObject');

let assertError = function(e) {
let regexp = /You tried to render a .+ that accesses relationships off of a post, but that model didn't have all of its required relationships preloaded ('comments.author')*/;
assert.ok(e.message.match(regexp));
};

if (this.major === "2" && (this.minor === "12" || this.minor === "16")) {
Ember.Logger.error = function() {};
Ember.Test.adapter.exception = assertError;
} else {
Ember.onerror = assertError;
}

await render(hbs`
{{assert-must-preload post "author,comments.author"}}
`);
});
});

});
8 changes: 7 additions & 1 deletion tests/integration/mixins/loadable-store/load-record-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ module('Integration | Mixins | LoadableStore | loadRecord', function(hooks) {
models: {
post: Model.extend({
comments: hasMany(),
author: belongsTo(),
tags: hasMany()
}),
comment: Model.extend({
post: belongsTo()
post: belongsTo(),
author: belongsTo()
}),
tag: Model.extend({
posts: hasMany()
}),
author: Model.extend({
comments: hasMany(),
posts: hasMany()
})
},
baseConfig() {
Expand Down
49 changes: 48 additions & 1 deletion tests/integration/mixins/loadable-store/load-records-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@ module('Integration | Mixins | LoadableStore | loadRecords', function(hooks) {
models: {
post: Model.extend({
comments: hasMany(),
author: belongsTo(),
tags: hasMany()
}),
comment: Model.extend({
post: belongsTo()
post: belongsTo(),
author: belongsTo()
}),
tag: Model.extend({
posts: hasMany()
}),
author: Model.extend({
comments: hasMany(),
posts: hasMany()
})
},
baseConfig() {
Expand Down Expand Up @@ -139,4 +145,45 @@ module('Integration | Mixins | LoadableStore | loadRecords', function(hooks) {
assert.equal(posts.get('firstObject.id'), serverPost.id);
assert.equal(posts.get('firstObject.comments.length'), 2);
});

module('Tracking includes', function() {
test('it will track an include', async function(assert) {
let serverPost = this.server.create('post', { title: 'My post' });
this.server.createList('comment', 3, { post: serverPost });

let posts = await this.store.loadRecords('post', { include: 'comments' });

assert.ok(posts.get('firstObject').hasLoaded('comments'));
});

test('it will track a dot path include', async function(assert) {
let serverPost = this.server.create('post', { title: 'My post' });
let serverComments = this.server.createList('comment', 3, { post: serverPost });

serverComments.forEach(comment => {
this.server.create('author', { comments: [comment] });
});

let posts = await this.store.loadRecords('post', { include: 'comments.author' });

assert.ok(posts.get('firstObject').hasLoaded('comments.author'));
});

test('it will track multiple includes', async function(assert) {
let serverAuthor = this.server.create('author');
let serverPost = this.server.create('post', {
title: 'My post',
author: serverAuthor
});
let serverComments = this.server.createList('comment', 3, { post: serverPost });

serverComments.forEach(comment => {
this.server.create('author', { comments: [comment] });
});

let posts = await this.store.loadRecords('post', { include: 'author,comments.author' });

assert.ok(posts.get('firstObject').hasLoaded('author,comments.author'));
});
});
});

0 comments on commit 4c26a29

Please sign in to comment.