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.

Sign up for GitHub

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

Store._find asserts adapterPayload not empty #3916

Merged
merged 1 commit into from
Nov 20, 2015
Merged

Store._find asserts adapterPayload not empty #3916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion addon/system/store/finders.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ import {

var Promise = Ember.RSVP.Promise;

function payloadIsNotBlank(adapterPayload) {
if (Ember.isArray(adapterPayload)) {
return true;
} else {
return Object.keys(adapterPayload || {}).length;
}
}

export function _find(adapter, store, typeClass, id, internalModel, options) {
var snapshot = internalModel.createSnapshot(options);
var promise = adapter.findRecord(store, typeClass, id, snapshot);
Expand All @@ -24,7 +32,7 @@ export function _find(adapter, store, typeClass, id, internalModel, options) {
promise = _guard(promise, _bind(_objectIsAlive, store));

return promise.then(function(adapterPayload) {
Ember.assert("You made a request for a " + typeClass.typeClassKey + " with id " + id + ", but the adapter's response did not have any data", adapterPayload);
Ember.assert("You made a `find` request for a " + typeClass.typeClassKey + " with id " + id + ", but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload));
return store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, id, 'findRecord');
//TODO Optimize
Expand Down Expand Up @@ -56,6 +64,7 @@ export function _findMany(adapter, store, typeClass, ids, internalModels) {
promise = _guard(promise, _bind(_objectIsAlive, store));

return promise.then(function(adapterPayload) {
Ember.assert("You made a `findMany` request for a " + typeClass.typeClassKey + " with ids " + ids + ", but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac how can I exercise this assertion?

Copy link
Member

Choose a reason for hiding this comment

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

@seanpdoyle I would follow the pattern used in this test of stubbing the adapter method to return a value that triggers the assertion then using the expectAssertion helper to test that it is called.

data/tests/integration/adapter/queries-test.js

Lines 47 to 63 in 1d5dfa1

test("The store asserts when query is made and the adapter responses with a single record.", function(assert) {
env = setupStore({ person: Person, adapter: DS.RESTAdapter });
store = env.store;
adapter = env.adapter;
adapter.query = function(store, type, query, recordArray) {
assert.equal(type, Person, "the query method is called with the correct type");
return Ember.RSVP.resolve({ people: { id: 1, name: "Peter Wagenet" } });
};
assert.expectAssertion(function() {
Ember.run(function() {
store.query('person', { page: 1 });
});
}, /The response to store.query is expected to be an array but it was a single record/);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac unfortunately, there isn't a store operation that maps directly to this one like the others.

For instance, adapter.findRecord maps to store.find, adapter.query maps to store.query (and maybe store.queryRecord?).

In this case, isn't findMany used to coalesce find requests?

Would I be able to exercise adapter.findMany with a store.query({ ids: [1,2,3] })?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac I've tried the following:

adapter.findMany = () => Ember.RSVP.resolve({});

assert.expectAssertion(() => {
  run(() => store.query('person', { ids: [1, 2] }));
}, /the adapter's response did not have any data/);

But it didn't raise an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac which store operation maps to a findMany?

Copy link
Member

Choose a reason for hiding this comment

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

I believe you can trigger it by setting coalesceFindRequests: true on the adapter then making 2 findRecord requests in the same run loop.

assert.expectAssertion(() => {
  run(() => {
    store.findRecord('person', 1);
    store.findRecord('person', 2);
  });
}, /the adapter's response did not have any data/);

return store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'findMany');
//TODO Optimize, no need to materialize here
Expand All @@ -77,6 +86,7 @@ export function _findHasMany(adapter, store, internalModel, link, relationship)
promise = _guard(promise, _bind(_objectIsAlive, internalModel));

return promise.then(function(adapterPayload) {
Ember.assert("You made a `findHasMany` request for a " + internalModel.modelName + "'s `" + relationship.key + "` relationship, using link " + link + ", but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac how can I exercise this assertion?

return store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'findHasMany');
//TODO Use a non record creating push
Expand Down Expand Up @@ -126,6 +136,7 @@ export function _findAll(adapter, store, typeClass, sinceToken, options) {
promise = _guard(promise, _bind(_objectIsAlive, store));

return promise.then(function(adapterPayload) {
Ember.assert("You made a `findAll` request for " + typeClass.typeClassKey + "records, but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload));
store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'findAll');
//TODO Optimize
Expand Down Expand Up @@ -172,6 +183,7 @@ export function _queryRecord(adapter, store, typeClass, query) {
promise = _guard(promise, _bind(_objectIsAlive, store));

return promise.then(function(adapterPayload) {
Ember.assert("You made a `queryRecord` request for " + typeClass.typeClassKey + "records, with query `" + query + "`, but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload));
var record;
store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'queryRecord');
Expand Down
10 changes: 10 additions & 0 deletions tests/integration/adapter/find-all-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,13 @@ test("When all records for a type are requested, records that are created on the
assert.equal(get(allRecords, 'length'), 1, "the record array's length is 1");
assert.equal(allRecords.objectAt(0).get('name'), "Carsten Nielsen", "the first item in the record array is Carsten Nielsen");
});

test('When all records are requested, assert the payload is not blank', (assert) => {
env.registry.register('adapter:person', DS.Adapter.extend({
findAll: () => Ember.RSVP.resolve({})
}));

assert.expectAssertion(() => {
run(() => store.findAll('person'));
}, /the adapter's response did not have any data/);
});
24 changes: 24 additions & 0 deletions tests/integration/adapter/find-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,27 @@ test("When a single record is requested, and the promise is rejected, the record
});

});

test('When a single record is requested, and the payload is blank', (assert) => {
env.registry.register('adapter:person', DS.Adapter.extend({
findRecord: () => Ember.RSVP.resolve({})
}));

assert.expectAssertion(() => {
run(() => store.find('person', 'the-id'));
}, /the adapter's response did not have any data/);
});

test('When multiple records are requested, and the payload is blank', (assert) => {
env.registry.register('adapter:person', DS.Adapter.extend({
coalesceFindRequests: true,
findMany: () => Ember.RSVP.resolve({})
}));

assert.expectAssertion(() => {
run(() => {
store.findRecord('person', '1');
store.findRecord('person', '2');
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmac 👍 that did the trick.

}, /the adapter's response did not have any data/);
});
8 changes: 4 additions & 4 deletions tests/integration/store/json-api-validation-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test("when normalizeResponse returns undefined (or doesn't return), throws an er

env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return Ember.RSVP.resolve({});
return Ember.RSVP.resolve({ data: {} });
}
}));

Expand All @@ -53,7 +53,7 @@ test("when normalizeResponse returns null, throws an error", function(assert) {

env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return Ember.RSVP.resolve({});
return Ember.RSVP.resolve({ data: {} });
}
}));

Expand All @@ -73,7 +73,7 @@ test("when normalizeResponse returns an empty object, throws an error", function

env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return Ember.RSVP.resolve({});
return Ember.RSVP.resolve({ data: {} });
}
}));

Expand All @@ -97,7 +97,7 @@ test("when normalizeResponse returns a document with both data and errors, throw

env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return Ember.RSVP.resolve({});
return Ember.RSVP.resolve({ data: {} });
}
}));

Expand Down