Skip to content

Commit

Permalink
Coalesce find requests, add support for preloading data
Browse files Browse the repository at this point in the history
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
  • Loading branch information
igorT committed Jul 24, 2014
1 parent f7db7bc commit 51728a8
Show file tree
Hide file tree
Showing 14 changed files with 942 additions and 165 deletions.
143 changes: 128 additions & 15 deletions packages/ember-data/lib/adapters/rest_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,50 @@ var forEach = Ember.ArrayPolyfills.forEach;
*/
export default Adapter.extend({
defaultSerializer: '-rest',

/**
By default the RESTAdapter will send each find request coming from a `store.find`
or from accessing a relationship separately to the server. If your server supports passing
ids as a query string, you can set coalesceFindRequests to true to coalesce all find requests
within a single runloop.
For example, if you have an initial payload of
```javascript
post: {
id:1,
comments: [1,2]
}
```
By default calling `post.get('comments')` will trigger the following requests(assuming the
comments haven't been loaded before):
```
GET /comments/1
GET /comments/2
```
If you set coalesceFindRequests to `true` it will instead trigger the following request:
```
GET /comments?ids[]=1&ids[]=2
```
Setting coalesceFindRequests to `true` also works for `store.find` requests and `belongsTo`
relationships accessed within the same runloop. If you set `coalesceFindRequests: true`
```javascript
store.find('comment', 1);
store.find('comment', 2);
```
will also send a request to: `GET /comments?ids[]=1&ids[]=2`
@property coalesceFindRequests
@type {boolean}
*/
coalesceFindRequests: false,

/**
Endpoint paths can be prefixed with a `namespace` by setting the namespace
property on the adapter:
Expand Down Expand Up @@ -205,10 +249,11 @@ export default Adapter.extend({
@param {DS.Store} store
@param {subclass of DS.Model} type
@param {String} id
@param {DS.Model} record
@return {Promise} promise
*/
find: function(store, type, id) {
return this.ajax(this.buildURL(type.typeKey, id), 'GET');
find: function(store, type, id, record) {
return this.ajax(this.buildURL(type.typeKey, id, record), 'GET');
},

/**
Expand Down Expand Up @@ -257,9 +302,7 @@ export default Adapter.extend({
},

/**
Called by the store in order to fetch a JSON array for
the unloaded records in a has-many relationship that were originally
specified as IDs.
Called by the store in order to fetch several records together if `coalesceFindRequests` is true
For example, if the original payload looks like:
Expand Down Expand Up @@ -288,10 +331,11 @@ export default Adapter.extend({
@param {DS.Store} store
@param {subclass of DS.Model} type
@param {Array} ids
@param {Array} records
@return {Promise} promise
*/
findMany: function(store, type, ids) {
return this.ajax(this.buildURL(type.typeKey), 'GET', { data: { ids: ids } });
findMany: function(store, type, ids, records) {
return this.ajax(this.buildURL(type.typeKey, ids, records), 'GET', { data: { ids: ids } });
},

/**
Expand Down Expand Up @@ -391,7 +435,7 @@ export default Adapter.extend({

serializer.serializeIntoHash(data, type, record, { includeId: true });

return this.ajax(this.buildURL(type.typeKey), "POST", { data: data });
return this.ajax(this.buildURL(type.typeKey, null, record), "POST", { data: data });
},

/**
Expand All @@ -418,7 +462,7 @@ export default Adapter.extend({

var id = get(record, 'id');

return this.ajax(this.buildURL(type.typeKey, id), "PUT", { data: data });
return this.ajax(this.buildURL(type.typeKey, id, record), "PUT", { data: data });
},

/**
Expand All @@ -435,7 +479,7 @@ export default Adapter.extend({
deleteRecord: function(store, type, record) {
var id = get(record, 'id');

return this.ajax(this.buildURL(type.typeKey, id), "DELETE");
return this.ajax(this.buildURL(type.typeKey, id, record), "DELETE");
},

/**
Expand All @@ -451,15 +495,20 @@ export default Adapter.extend({
@method buildURL
@param {String} type
@param {String} id
@param {DS.Model} record
@return {String} url
*/
buildURL: function(type, id) {
var url = [];
var host = get(this, 'host');
var prefix = this.urlPrefix();
buildURL: function(type, id, record) {
var url = [],
host = get(this, 'host'),
prefix = this.urlPrefix();

if (type) { url.push(this.pathForType(type)); }
if (id) { url.push(id); }

//We might get passed in an array of ids from findMany
//in which case we don't want to modify the url, as the
//ids will be passed in through a query param
if (id && !Ember.isArray(id)) { url.push(id); }

if (prefix) { url.unshift(prefix); }

Expand Down Expand Up @@ -504,6 +553,60 @@ export default Adapter.extend({
return url.join('/');
},

_stripIDFromURL: function(store, record) {
var type = store.modelFor(record);
var url = this.buildURL(type.typeKey, record.get('id'), record);

var expandedURL = url.split('/');
//Case when the url is of the format ...something/:id
var lastSegment = expandedURL[ expandedURL.length - 1 ];
var id = record.get('id');
if (lastSegment === id) {
expandedURL[expandedURL.length - 1] = "";
} else if(endsWith(lastSegment, '?id=' + id)) {
//Case when the url is of the format ...something?id=:id
expandedURL[expandedURL.length - 1] = lastSegment.substring(0, lastSegment.length - id.length - 1);
}

return expandedURL.join('/');
},

/**
Organize records into groups, each of which is to be passed to separate
calls to `findMany`.
This implementation groups together records that have the same base URL but
differing ids. For example `/comments/1` and `/comments/2` will be grouped together
because we know findMany can coalesce them together as `/comments?ids[]=1&ids[]=2`
It also supports urls where ids are passed as a query param, such as `/comments?id=1`
but not those where there is more than 1 query param such as `/comments?id=2&name=David`
Currently only the query param of `id` is supported. If you need to support others, please
override this or the `_stripIDFromURL` method.
It does not group records that have differing base urls, such as for example: `/posts/1/comments/2`
and `/posts/2/comments/3`
@method groupRecordsForFindMany
@param {Array} records
@returns {Array} an array of arrays of records, each of which is to be
loaded separately by `findMany`.
*/
groupRecordsForFindMany: function (store, records) {
var groups = Ember.MapWithDefault.create({defaultValue: function(){return [];}});
var adapter = this;
forEach.call(records, function(record){
var baseUrl = adapter._stripIDFromURL(store, record);
groups.get(baseUrl).push(record);
});
var groupsArray = [];
groups.forEach(function(key, group){
groupsArray.push(group);
});

return groupsArray;
},

/**
Determines the pathname for a given type.
Expand Down Expand Up @@ -649,3 +752,13 @@ export default Adapter.extend({
return hash;
}
});

//From http://stackoverflow.com/questions/280634/endswith-in-javascript
function endsWith(string, suffix){
if (typeof String.prototype.endsWith !== 'function') {
return string.indexOf(suffix, string.length - suffix.length) !== -1;
} else {
return string.endsWith(suffix);
}
}

53 changes: 27 additions & 26 deletions packages/ember-data/lib/system/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,42 +417,43 @@ var Adapter = Ember.Object.extend({
deleteRecord: Ember.required(Function),

/**
Find multiple records at once.
By default the store will try to coalesce all `fetchRecord` calls within the same runloop
into as few requests as possible by calling groupRecordsForFindMany and passing it into a findMany call.
You can opt out of this behaviour by either not implementing the findMany hook or by setting
coalesceFindRequests to false
By default, it loops over the provided ids and calls `find` on each.
May be overwritten to improve performance and reduce the number of
server requests.
Example
@property coalesceFindRequests
@type {boolean}
*/
coalesceFindRequests: true,

```javascript
App.ApplicationAdapter = DS.Adapter.extend({
findMany: function(store, type, ids) {
var url = type;
return new Ember.RSVP.Promise(function(resolve, reject) {
jQuery.getJSON(url, {ids: ids}).then(function(data) {
Ember.run(null, resolve, data);
}, function(jqXHR) {
jqXHR.then = null; // tame jQuery's ill mannered promises
Ember.run(null, reject, jqXHR);
});
});
}
});
```
/**
Find multiple records at once if coalesceFindRequests is true
@method findMany
@param {DS.Store} store
@param {subclass of DS.Model} type the DS.Model class of the records
@param {Array} ids
@param {Array} records
@return {Promise} promise
*/
findMany: function(store, type, ids) {
var promises = map.call(ids, function(id) {
return this.find(store, type, id);
}, this);

return Ember.RSVP.all(promises);
/**
Organize records into groups, each of which is to be passed to separate
calls to `findMany`.
For example, if your api has nested URLs that depend on the parent, you will
want to group records by their parent.
The default implementation returns the records as a single group.
@method groupRecordsForFindMany
@param {Array} records
@returns {Array} an array of arrays of records, each of which is to be
loaded separately by `findMany`.
*/
groupRecordsForFindMany: function (store, records) {
return [records];
}
});

Expand Down
62 changes: 62 additions & 0 deletions packages/ember-data/lib/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var get = Ember.get;
var set = Ember.set;
var merge = Ember.merge;
var Promise = Ember.RSVP.Promise;
var forEach = Ember.ArrayPolyfills.forEach;

var JSONSerializer;
var retrieveFromCurrentState = Ember.computed('currentState', function(key, value) {
Expand Down Expand Up @@ -629,6 +630,67 @@ var Model = Ember.Object.extend(Ember.Evented, {
get(this, 'store').dataWasUpdated(this.constructor, this);
},

/**
When a find request is triggered on the store, the user can optionally passed in
attributes and relationships to be preloaded. These are meant to behave as if they
came back from the server, expect the user obtained them out of band and is informing
the store of their existence. The most common use case is for supporting client side
nested URLs, such as `/posts/1/comments/2` so the user can do
`store.find('comment', 2, {post:1})` without having to fetch the post.
Preloaded data can be attributes and relationships passed in either as IDs or as actual
models.
@method _preloadData
@private
@param {Object} preload
*/
_preloadData: function(preload) {
var record = this;
//TODO(Igor) consider the polymorphic case
forEach.call(Ember.keys(preload), function(key) {
var preloadValue = get(preload, key);
var relationshipMeta = record.constructor.metaForProperty(key);
if (relationshipMeta.isRelationship) {
record._preloadRelationship(key, preloadValue);
} else {
get(record, '_data')[key] = preloadValue;
}
});
},

_preloadRelationship: function(key, preloadValue) {
var relationshipMeta = this.constructor.metaForProperty(key);
var type = relationshipMeta.type;
if (relationshipMeta.kind === 'hasMany'){
this._preloadHasMany(key, preloadValue, type);
} else {
this._preloadBelongsTo(key, preloadValue, type);
}
},

_preloadHasMany: function(key, preloadValue, type) {
Ember.assert("You need to pass in an array to set a hasMany property on a record", Ember.isArray(preloadValue));
var record = this;

forEach.call(preloadValue, function(recordToPush) {
recordToPush = record._convertStringOrNumberIntoRecord(recordToPush, type);
get(record, key).pushObject(recordToPush);
});
},

_preloadBelongsTo: function(key, preloadValue, type){
var recordToPush = this._convertStringOrNumberIntoRecord(preloadValue, type);
set(this, key, recordToPush);
},

_convertStringOrNumberIntoRecord: function(value, type) {
if (Ember.typeOf(value) === 'string' || Ember.typeOf(value) === 'number'){
return this.store.recordForId(type, value);
}
return value;
},

/**
Returns an object, whose keys are changed properties, and value is
an [oldProp, newProp] array.
Expand Down
8 changes: 8 additions & 0 deletions packages/ember-data/lib/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ var DirtyState = {
// EVENTS
didSetProperty: didSetProperty,

//TODO(Igor) reloading now triggers a
//loadingData event, though it seems fine?
loadingData: Ember.K,

propertyWasReset: function(record, name) {
var stillDirty = false;

Expand Down Expand Up @@ -536,6 +540,10 @@ var RootState = {
// FLAGS
isLoaded: true,

//TODO(Igor) Reloading now triggers a loadingData event,
//but it should be ok?
loadingData: Ember.K,

// SUBSTATES

// If there are no local changes to a record, it remains
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-data/lib/system/record_arrays/many_array.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export default RecordArray.extend({
var owner = get(this, 'owner');

var unloadedRecords = records.filterBy('isEmpty', true);
store.fetchMany(unloadedRecords, owner);
store.scheduleFetchMany(unloadedRecords, owner);
},

// Overrides Ember.Array's replace method to implement
Expand Down
Loading

1 comment on commit 51728a8

@stefanpenner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.