-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
+249
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) { | ||
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'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whats the difference between this error and the one on line 31? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
||
|
@@ -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); | ||
|
185 changes: 185 additions & 0 deletions
185
packages/ember-data/tests/integration/store/json-api-validation-test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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/); | ||
|
||
}); | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta
has to be an objectdata
can be an object, null or an arrayerrors
has to be an arrayI'm fine with just checking that the keys exists for now though.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
andincluded
. This should provide complete validation of the top-level of a JSON API document