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

DS.Store type presence checks #4178

Merged
merged 1 commit into from
Mar 18, 2016
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
22 changes: 18 additions & 4 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ var get = Ember.get;
var set = Ember.set;
var once = Ember.run.once;
var isNone = Ember.isNone;
var isPresent = Ember.isPresent;
var Promise = Ember.RSVP.Promise;
var copy = Ember.copy;
var Store;
Expand Down Expand Up @@ -281,6 +282,7 @@ Store = Service.extend({
@return {DS.Model} record
*/
createRecord(modelName, inputProperties) {
assert("You need to pass a model name to the store's createRecord method", isPresent(modelName));
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${Ember.inspect(modelName)}`, typeof modelName === 'string');
var typeClass = this.modelFor(modelName);
var properties = copy(inputProperties) || new EmptyObject();
Expand Down Expand Up @@ -457,6 +459,7 @@ Store = Service.extend({
@return {Promise} promise
*/
findRecord(modelName, id, options) {
assert("You need to pass a model name to the store's findRecord method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
assert(badIdFormatAssertion, (typeof id === 'string' && id.length > 0) || (typeof id === 'number' && !isNaN(id)));

Expand Down Expand Up @@ -531,6 +534,7 @@ Store = Service.extend({
@return {Promise} promise
*/
findByIds(modelName, ids) {
assert("You need to pass a model name to the store's findByIds method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
let promises = new Array(ids.length);

Expand Down Expand Up @@ -721,6 +725,7 @@ Store = Service.extend({
@return {DS.Model|null} record
*/
peekRecord(modelName, id) {
assert("You need to pass a model name to the store's peekRecord method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
if (this.hasRecordForId(modelName, id)) {
return this._internalModelForId(modelName, id).getRecord();
Expand Down Expand Up @@ -762,6 +767,7 @@ Store = Service.extend({
@return {Boolean}
*/
hasRecordForId(modelName, inputId) {
assert("You need to pass a model name to the store's hasRecordForId method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var typeClass = this.modelFor(modelName);
var id = coerceId(inputId);
Expand All @@ -780,6 +786,7 @@ Store = Service.extend({
@return {DS.Model} record
*/
recordForId(modelName, id) {
assert("You need to pass a model name to the store's recordForId method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
return this._internalModelForId(modelName, id).getRecord();
},
Expand Down Expand Up @@ -913,7 +920,7 @@ Store = Service.extend({
},

_query(modelName, query, array) {
assert("You need to pass a type to the store's query method", modelName);
assert("You need to pass a model name to the store's query method", isPresent(modelName));
assert("You need to pass a query hash to the store's query method", query);
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var typeClass = this.modelFor(modelName);
Expand Down Expand Up @@ -945,7 +952,7 @@ Store = Service.extend({
@return {Promise} promise
*/
queryRecord(modelName, query) {
assert("You need to pass a type to the store's queryRecord method", modelName);
assert("You need to pass a model name to the store's queryRecord method", isPresent(modelName));
assert("You need to pass a query hash to the store's queryRecord method", query);
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');

Expand Down Expand Up @@ -981,6 +988,7 @@ Store = Service.extend({
@return {Promise} promise
*/
findAll(modelName, options) {
assert("You need to pass a model name to the store's findAll method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var typeClass = this.modelFor(modelName);

Expand Down Expand Up @@ -1050,6 +1058,7 @@ Store = Service.extend({
@return {DS.RecordArray}
*/
peekAll(modelName) {
assert("You need to pass a model name to the store's peekAll method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var typeClass = this.modelFor(modelName);

Expand Down Expand Up @@ -1155,6 +1164,7 @@ Store = Service.extend({
@deprecated
*/
filter(modelName, query, filter) {
assert("You need to pass a model name to the store's filter method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');

if (!Ember.ENV.ENABLE_DS_FILTER) {
Expand Down Expand Up @@ -1206,6 +1216,7 @@ Store = Service.extend({
@return {boolean}
*/
recordIsLoaded(modelName, id) {
assert("You need to pass a model name to the store's recordIsLoaded method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
return this.hasRecordForId(modelName, id);
},
Expand Down Expand Up @@ -1462,6 +1473,7 @@ Store = Service.extend({
@return {DS.Model}
*/
modelFor(modelName) {
assert("You need to pass a model name to the store's modelFor method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');

var factory = this.modelFactoryFor(modelName);
Expand All @@ -1478,6 +1490,7 @@ Store = Service.extend({
},

modelFactoryFor(modelName) {
assert("You need to pass a model name to the store's modelFactoryFor method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var normalizedKey = normalizeModelName(modelName);

Expand Down Expand Up @@ -1808,6 +1821,7 @@ Store = Service.extend({
@return {Object} The normalized payload
*/
normalize(modelName, payload) {
assert("You need to pass a model name to the store's normalize method", isPresent(modelName));
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${Ember.inspect(modelName)}`, typeof modelName === 'string');
var serializer = this.serializerFor(modelName);
var model = this.modelFor(modelName);
Expand Down Expand Up @@ -1901,7 +1915,7 @@ Store = Service.extend({
@return DS.Adapter
*/
adapterFor(modelName) {

assert("You need to pass a model name to the store's adapterFor method", isPresent(modelName));
assert(`Passing classes to store.adapterFor has been removed. Please pass a dasherized string instead of ${Ember.inspect(modelName)}`, typeof modelName === 'string');

return this.lookupAdapter(modelName);
Expand Down Expand Up @@ -1937,7 +1951,7 @@ Store = Service.extend({
@return {DS.Serializer}
*/
serializerFor(modelName) {

assert("You need to pass a model name to the store's serializerFor method", isPresent(modelName));
assert(`Passing classes to store.serializerFor has been removed. Please pass a dasherized string instead of ${Ember.inspect(modelName)}`, typeof modelName === 'string');

var fallbacks = [
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/adapter/queries-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module("integration/adapter/queries - Queries", {
testInDebug("It raises an assertion when no type is passed", function(assert) {
assert.expectAssertion(function() {
store.query();
}, "You need to pass a type to the store's query method");
}, "You need to pass a model name to the store's query method");
});

testInDebug("It raises an assertion when no query hash is passed", function(assert) {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/store/query-record-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module("integration/store/query-record - Query one record with a query hash", {
testInDebug("It raises an assertion when no type is passed", function(assert) {
assert.expectAssertion(function() {
store.queryRecord();
}, "You need to pass a type to the store's queryRecord method");
}, "You need to pass a model name to the store's queryRecord method");
});

testInDebug("It raises an assertion when no query hash is passed", function(assert) {
Expand Down
36 changes: 36 additions & 0 deletions tests/unit/store/asserts-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {module} from 'qunit';
import testInDebug from 'dummy/tests/helpers/test-in-debug';
import {createStore} from 'dummy/tests/helpers/store';

module("unit/store/asserts - DS.Store methods produce useful assertion messages");

const MODEL_NAME_METHODS = [
'createRecord',
'findRecord',
'findByIds',
'peekRecord',
'hasRecordForId',
'recordForId',
'query',
'queryRecord',
'findAll',
'peekAll',
'filter',
'recordIsLoaded',
'modelFor',
'modelFactoryFor',
Copy link
Member

Choose a reason for hiding this comment

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

recordIsLoaded is marked as private in the API docs; modelFactoryFor currently has no public documentation (though the name suggests it is public).

Tiny nitpick: I wonder if those methods should be excluded from the assertion tests here, since they are not public API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at a few existing asserts and saw type so I matched that. I'll change to modelName and take a look for existing asserts that would be good to change as well.

I think asserts in private API can still be useful occasionally when bad values (erroneously) sneak through into internal code. If they're usually avoided in ED though I can remove them.
https://github.com/emberjs/data/pull/4178/files#r55129914

Copy link
Member

Choose a reason for hiding this comment

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

I think its ok to leave the asserts on private APIs. If these methods were ever marked public in the future its unlikely they someone would remember to come back and update this test.

Copy link
Member

Choose a reason for hiding this comment

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

If these methods were ever marked public in the future its unlikely they someone would remember to come back and update this test.

👍

'normalize',
'adapterFor',
'serializerFor'
];

testInDebug("Calling Store methods with no type asserts", function(assert) {
assert.expect(MODEL_NAME_METHODS.length);
let store = createStore();

MODEL_NAME_METHODS.forEach(function(methodName) {
assert.expectAssertion(function() {
store[methodName](null);
}, new RegExp(`You need to pass a model name to the store's ${methodName} method`));
});
});