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

[BUGFIX release] Validate JSON API docs returned by normalizeResponse #3608

Merged
merged 1 commit into from
Aug 3, 2015
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
65 changes: 64 additions & 1 deletion packages/ember-data/lib/system/store/serializer-response.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,65 @@ import Model from 'ember-data/system/model/model';

const get = Ember.get;

/**
This is a helper method that validates a JSON API top-level document

The format of a document is described here:
http://jsonapi.org/format/#document-top-level

@method validateDocumentStructure
@param {Object} doc JSON API document
@return {array} An array of errors found in the document structure
*/
export function validateDocumentStructure(doc) {
let errors = [];
if (!doc || typeof doc !== 'object') {
errors.push('Top level of a JSON API document must be an object');
} else {
if (!('data' in doc) &&
!('errors' in doc) &&
!('meta' in doc)) {
errors.push('One or more of the following keys must be present: "data", "errors", "meta".');
} else {
if (('data' in doc) && ('errors' in doc)) {
Copy link
Member

Choose a reason for hiding this comment

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

do we also need to check that the properties have object values, or are they allow to be of any type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

arrays or objects are allowed as values. I'm not sure where we want this to end, because we could then validate the objects (or arrays) themselves, including relationships, etc...

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with just checking that the keys exists for now though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I'll go ahead and implement these validations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, now I have validations in place for meta, data, errors, links, jsonapi and included. This should provide complete validation of the top-level of a JSON API document

errors.push('Top level keys "errors" and "data" cannot both be present in a JSON API document');
}
}
if ('data' in doc) {
if (!(doc.data === null || Ember.isArray(doc.data) || typeof doc.data === 'object')) {
errors.push('data must be null, an object, or an array');
Copy link
Member

Choose a reason for hiding this comment

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

Whats the difference between this error and the one on line 31?

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 29-33 seems identical with 34-38.

}
}
if ('meta' in doc) {
if (typeof doc.meta !== 'object') {
errors.push('meta must be an object');
}
}
if ('errors' in doc) {
if (!Ember.isArray(doc.errors)) {
errors.push('errors must be an array');
}
}
if ('links' in doc) {
if (typeof doc.links !== 'object') {
errors.push('links must be an object');
}
}
if ('jsonapi' in doc) {
if (typeof doc.jsonapi !== 'object') {
errors.push('jsonapi must be an object');
}
}
if ('included' in doc) {
if (typeof doc.included !== 'object') {
errors.push('included must be an array');
}
}
}

return errors;
}

/**
This is a helper method that always returns a JSON-API Document.

Expand All @@ -16,7 +75,11 @@ const get = Ember.get;
*/
export function normalizeResponseHelper(serializer, store, modelClass, payload, id, requestType) {
let normalizedResponse = serializer.normalizeResponse(store, modelClass, payload, id, requestType);

let validationErrors = [];
Ember.runInDebug(() => {
validationErrors = validateDocumentStructure(normalizedResponse);
});
Ember.assert(`normalizeResponse must return a valid JSON API document:\n\t* ${validationErrors.join('\n\t* ')}`, Ember.isEmpty(validationErrors));
// TODO: Remove after metadata refactor
if (normalizedResponse.meta) {
store._setMetadataFor(modelClass.modelName, normalizedResponse.meta);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
var Person, store, env;
var run = Ember.run;

module("integration/store/json-validation", {
setup: function() {
Person = DS.Model.extend({
updatedAt: DS.attr('string'),
name: DS.attr('string'),
firstName: DS.attr('string'),
lastName: DS.attr('string')
});

env = setupStore({
person: Person
});
store = env.store;
},

teardown: function() {
run(store, 'destroy');
}
});

test("when normalizeResponse returns undefined (or doesn't return), throws an error", function() {

env.registry.register('serializer:person', DS.Serializer.extend({
normalizeResponse() {}
}));

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

throws(function () {
run(function() {
store.find('person', 1);
});
}, /Top level of a JSON API document must be an object/);
});

test("when normalizeResponse returns null, throws an error", function() {

env.registry.register('serializer:person', DS.Serializer.extend({
normalizeResponse() {return null;}
}));

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

throws(function () {
run(function() {
store.find('person', 1);
});
}, /Top level of a JSON API document must be an object/);
});


test("when normalizeResponse returns an empty object, throws an error", function() {

env.registry.register('serializer:person', DS.Serializer.extend({
normalizeResponse() {return {};}
}));

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

throws(function () {
run(function() {
store.find('person', 1);
});
}, /One or more of the following keys must be present/);
});

test("when normalizeResponse returns a document with both data and errors, throws an error", function() {

env.registry.register('serializer:person', DS.Serializer.extend({
normalizeResponse() {
return {
data: [],
errors: []
};
}
}));

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

throws(function () {
run(function() {
store.find('person', 1);
});
}, /cannot both be present/);
});

function testPayloadError(payload, expectedError) {
env.registry.register('serializer:person', DS.Serializer.extend({
normalizeResponse(store, type, pld) {
return pld;
}
}));
env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return Ember.RSVP.resolve(payload);
}
}));
throws(function () {
run(function() {
store.find('person', 1);
});
}, expectedError, `Payload ${JSON.stringify(payload)} should throw error ${expectedError}`);
env.registry.unregister('serializer:person');
env.registry.unregister('adapter:person');
}

test("normalizeResponse 'data' cannot be undefined, a number, a string or a boolean", function() {

testPayloadError({ data: undefined }, /data must be/);
testPayloadError({ data: 1 }, /data must be/);
testPayloadError({ data: 'lollerskates' }, /data must be/);
testPayloadError({ data: true }, /data must be/);

});

test("normalizeResponse 'meta' cannot be an array, undefined, a number, a string or a boolean", function() {

testPayloadError({ meta: undefined }, /meta must be an object/);
testPayloadError({ meta: [] }, /meta must be an object/);
testPayloadError({ meta: 1 }, /meta must be an object/);
testPayloadError({ meta: 'lollerskates' }, /meta must be an object/);
testPayloadError({ meta: true }, /meta must be an object/);

});

test("normalizeResponse 'links' cannot be an array, undefined, a number, a string or a boolean", function() {

testPayloadError({ data: [], links: undefined }, /links must be an object/);
testPayloadError({ data: [], links: [] }, /links must be an object/);
testPayloadError({ data: [], links: 1 }, /links must be an object/);
testPayloadError({ data: [], links: 'lollerskates' }, /links must be an object/);
testPayloadError({ data: [], links: true }, /links must be an object/);

});

test("normalizeResponse 'jsonapi' cannot be an array, undefined, a number, a string or a boolean", function() {

testPayloadError({ data: [], jsonapi: undefined }, /jsonapi must be an object/);
testPayloadError({ data: [], jsonapi: [] }, /jsonapi must be an object/);
testPayloadError({ data: [], jsonapi: 1 }, /jsonapi must be an object/);
testPayloadError({ data: [], jsonapi: 'lollerskates' }, /jsonapi must be an object/);
testPayloadError({ data: [], jsonapi: true }, /jsonapi must be an object/);

});

test("normalizeResponse 'included' cannot be an object, undefined, a number, a string or a boolean", function() {

testPayloadError({ included: undefined }, /included must be an array/);
testPayloadError({ included: {} }, /included must be an array/);
testPayloadError({ included: 1 }, /included must be an array/);
testPayloadError({ included: 'lollerskates' }, /included must be an array/);
testPayloadError({ included: true }, /included must be an array/);

});

test("normalizeResponse 'errors' cannot be an object, undefined, a number, a string or a boolean", function() {

testPayloadError({ errors: undefined }, /errors must be an array/);
testPayloadError({ errors: {} }, /errors must be an array/);
testPayloadError({ errors: 1 }, /errors must be an array/);
testPayloadError({ errors: 'lollerskates' }, /errors must be an array/);
testPayloadError({ errors: true }, /errors must be an array/);

});