From 2aa1f27e237e270fc024d22b615416cf48cdd8ea Mon Sep 17 00:00:00 2001 From: bmac Date: Mon, 8 Jun 2015 17:07:43 -0400 Subject: [PATCH 1/2] Deprecate the preload argument to `find`/`fetchById` and move in into the `preload` key on the options argument. --- packages/ember-data/lib/system/store.js | 53 ++++++++++---- .../adapter/build-url-mixin-test.js | 2 +- .../tests/unit/store/adapter-interop-test.js | 73 +++++++++++++++++-- 3 files changed, 108 insertions(+), 20 deletions(-) diff --git a/packages/ember-data/lib/system/store.js b/packages/ember-data/lib/system/store.js index a1a58dc7786..9ae07dd64aa 100644 --- a/packages/ember-data/lib/system/store.js +++ b/packages/ember-data/lib/system/store.js @@ -445,7 +445,7 @@ Store = Service.extend({ without fetching the post you can pass in the post to the `find` call: ```javascript - store.find('comment', 2, { post: 1 }); + store.find('comment', 2, { preload: { post: 1 } }); ``` If you have access to the post model you can also pass the model itself: @@ -476,7 +476,7 @@ Store = Service.extend({ @method find @param {String} modelName @param {(Object|String|Integer|null)} id - @param {Object} preload - optional set of attributes and relationships passed in either as IDs or as actual models + @param {Object} options @return {Promise} promise */ find: function(modelName, id, preload) { @@ -494,8 +494,8 @@ Store = Service.extend({ Ember.deprecate('Calling store.find() with a query object is deprecated. Use store.query() instead.'); return this.query(modelName, id); } - - return this.findRecord(modelName, coerceId(id), preload); + var options = deprecatePreload(preload, this.modelFor(modelName), 'find'); + return this.findRecord(modelName, coerceId(id), options); }, /** @@ -523,15 +523,16 @@ Store = Service.extend({ @method fetchById @param {String} modelName @param {(String|Integer)} id - @param {Object} preload - optional set of attributes and relationships passed in either as IDs or as actual models + @param {Object} options @return {Promise} promise */ fetchById: function(modelName, id, preload) { Ember.assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string'); + var options = deprecatePreload(preload, this.modelFor(modelName), 'fetchById'); if (this.hasRecordForId(modelName, id)) { return this.getById(modelName, id).reload(); } else { - return this.find(modelName, id, preload); + return this.findRecord(modelName, id, options); } }, @@ -569,12 +570,13 @@ Store = Service.extend({ @private @param {String} modelName @param {(String|Integer)} id - @param {Object} preload - optional set of attributes and relationships passed in either as IDs or as actual models + @param {Object} options @return {Promise} promise */ findById: function(modelName, id, preload) { Ember.deprecate('Using store.findById() has been deprecated. Use store.findRecord() to return a record for a given type and id combination.'); - return this.findRecord(modelName, id, preload); + var options = deprecatePreload(preload, this.modelFor(modelName), 'findById'); + return this.findRecord(modelName, id, options); }, /** @@ -583,21 +585,23 @@ Store = Service.extend({ @method findRecord @param {String} modelName @param {(String|Integer)} id - @param {Object} preload - optional set of attributes and relationships passed in either as IDs or as actual models + @param {Object} options @return {Promise} promise */ - findRecord: function(modelName, id, preload) { + findRecord: function(modelName, id, options) { Ember.assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string'); var internalModel = this._internalModelForId(modelName, id); - return this._findByInternalModel(internalModel, preload); + return this._findByInternalModel(internalModel, options); }, - _findByInternalModel: function(internalModel, preload) { + _findByInternalModel: function(internalModel, options) { var fetchedInternalModel; + options = options || {}; + - if (preload) { - internalModel._preloadData(preload); + if (options.preload) { + internalModel._preloadData(options.preload); } if (internalModel.isEmpty()) { @@ -2172,5 +2176,26 @@ function setupRelationships(store, record, data) { }); } +function deprecatePreload(preloadOrOptions, type, methodName) { + if (preloadOrOptions) { + var modelProperties = []; + var fields = Ember.get(type, 'fields'); + fields.forEach(function(fieldType, key) { + modelProperties.push(key); + }); + var preloadDetected = modelProperties.reduce(function(memo, key) { + return typeof preloadOrOptions[key] !== 'undefined' || memo; + }, false); + if (preloadDetected) { + Ember.deprecate(`Passing a preload argument to \`store.${methodName}\` is deprecated. Please move it to the preload key on the ${methodName} \`options\` argument.`); + var preload = preloadOrOptions; + return { + preload: preload + }; + } + } + return preloadOrOptions; +} + export { Store }; export default Store; diff --git a/packages/ember-data/tests/integration/adapter/build-url-mixin-test.js b/packages/ember-data/tests/integration/adapter/build-url-mixin-test.js index cdc8008f614..dc2a1b75619 100644 --- a/packages/ember-data/tests/integration/adapter/build-url-mixin-test.js +++ b/packages/ember-data/tests/integration/adapter/build-url-mixin-test.js @@ -178,7 +178,7 @@ test('buildURL - buildURL takes a record from find', function() { }); run(function() { - store.find('comment', 1, { post: post }).then(async(function(post) { + store.find('comment', 1, { preload: { post: post } }).then(async(function(post) { equal(passedUrl, "/posts/2/comments/1"); })); }); diff --git a/packages/ember-data/tests/unit/store/adapter-interop-test.js b/packages/ember-data/tests/unit/store/adapter-interop-test.js index 2747cd1f1e4..d6585e92867 100644 --- a/packages/ember-data/tests/unit/store/adapter-interop-test.js +++ b/packages/ember-data/tests/unit/store/adapter-interop-test.js @@ -391,7 +391,7 @@ test("initial values of attributes can be passed in as the third argument to fin }); run(function() { - store.find('test', 1, { name: 'Test' }); + store.find('test', 1, { preload: { name: 'Test' } }); }); }); @@ -419,7 +419,7 @@ test("initial values of belongsTo can be passed in as the third argument to find run(function() { tom = store.push('person', { id: 2, name: 'Tom' }); - store.find('person', 1, { friend: tom }); + store.find('person', 1, { preload: { friend: tom } }); }); }); @@ -445,7 +445,7 @@ test("initial values of belongsTo can be passed in as the third argument to find env.registry.register('model:person', Person); run(function() { - store.find('person', 1, { friend: 2 }).then(async(function() { + store.find('person', 1, { preload: { friend: 2 } }).then(async(function() { store.getById('person', 1).get('friend').then(async(function(friend) { equal(friend.get('id'), '2', 'Preloaded belongsTo set'); })); @@ -477,7 +477,7 @@ test("initial values of hasMany can be passed in as the third argument to find a run(function() { tom = store.push('person', { id: 2, name: 'Tom' }); - store.find('person', 1, { friends: [tom] }); + store.find('person', 1, { preload: { friends: [tom] } }); }); }); @@ -504,7 +504,7 @@ test("initial values of hasMany can be passed in as the third argument to find a env.registry.register('model:person', Person); run(function() { - store.find('person', 1, { friends: [2] }); + store.find('person', 1, { preload: { friends: [2] } }); }); }); @@ -814,3 +814,66 @@ test("store.fetchRecord reject records that were not found, even when those requ }); }, /expected to find records with the following ids in the adapter response but they were missing/); }); + +module("unit/store/adapter_interop - find preload deprecations", { + setup: function() { + var Person = DS.Model.extend({ + name: DS.attr('string') + }); + + var TestAdapter = DS.Adapter.extend({ + find: function(store, type, id, snapshot) { + equal(snapshot.attr('name'), 'Tom'); + return Ember.RSVP.resolve({ id: id }); + } + }); + store = createStore({ + adapter: TestAdapter, + person: Person + }); + }, + teardown: function() { + run(function() { + if (store) { store.destroy(); } + }); + } +}); + +test("Using store#find with preload is deprecated", function() { + expect(2); + + expectDeprecation( + function() { + run(function() { + store.find('person', 1, { name: 'Tom' }); + }); + }, + /Passing a preload argument to `store.find` is deprecated./ + ); +}); + +test("Using store#fetchById with preload is deprecated", function() { + expect(2); + + expectDeprecation( + function() { + run(function() { + store.fetchById('person', 1, { name: 'Tom' }); + }); + }, + /Passing a preload argument to `store.fetchById` is deprecated./ + ); +}); + +test("Using store#findById with preload is deprecated", function() { + expect(2); + + expectDeprecation( + function() { + run(function() { + store.findById('person', 1, { name: 'Tom' }); + }); + }, + /Passing a preload argument to `store.findById` is deprecated/ + ); +}); From f6d6263b848866e0f86e87275b22f2f121917cfb Mon Sep 17 00:00:00 2001 From: Eric Kelly Date: Tue, 9 Jun 2015 23:34:58 -0400 Subject: [PATCH 2/2] Add test for deprecated find with preload --- .../tests/unit/store/adapter-interop-test.js | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/ember-data/tests/unit/store/adapter-interop-test.js b/packages/ember-data/tests/unit/store/adapter-interop-test.js index d6585e92867..2a3892b5444 100644 --- a/packages/ember-data/tests/unit/store/adapter-interop-test.js +++ b/packages/ember-data/tests/unit/store/adapter-interop-test.js @@ -827,6 +827,7 @@ module("unit/store/adapter_interop - find preload deprecations", { return Ember.RSVP.resolve({ id: id }); } }); + store = createStore({ adapter: TestAdapter, person: Person @@ -839,6 +840,28 @@ module("unit/store/adapter_interop - find preload deprecations", { } }); +test("store#find with deprecated preload passes correct options to store#findRecord", function() { + expect(2); + + var expectedOptions = { preload: { name: 'Tom' } }; + + store.reopen({ + findRecord: function(modelName, id, options) { + deepEqual(options, expectedOptions, + 'deprecated preload transformed to new options store#findRecord'); + } + }); + + expectDeprecation( + function() { + run(function() { + store.find('person', 1, { name: 'Tom' }); + }); + }, + /Passing a preload argument to `store.find` is deprecated./ + ); +}); + test("Using store#find with preload is deprecated", function() { expect(2);