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

During normalization, use property lookup instead of hasOwnProp checks #4313

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 2 additions & 2 deletions addon/serializers/json-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ const JSONAPISerializer = JSONSerializer.extend({
if (resourceHash.attributes) {
modelClass.eachAttribute((key) => {
let attributeKey = this.keyForAttribute(key, 'deserialize');
if (resourceHash.attributes.hasOwnProperty(attributeKey)) {
if (resourceHash.attributes[attributeKey] !== undefined) {
attributes[key] = resourceHash.attributes[attributeKey];
}
});
Expand Down Expand Up @@ -260,7 +260,7 @@ const JSONAPISerializer = JSONSerializer.extend({
if (resourceHash.relationships) {
modelClass.eachRelationship((key, relationshipMeta) => {
let relationshipKey = this.keyForRelationship(key, relationshipMeta.kind, 'deserialize');
if (resourceHash.relationships.hasOwnProperty(relationshipKey)) {
if (resourceHash.relationships[relationshipKey] !== undefined) {

let relationshipHash = resourceHash.relationships[relationshipKey];
relationships[key] = this.extractRelationship(relationshipHash);
Expand Down
18 changes: 9 additions & 9 deletions addon/serializers/json.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ export default Serializer.extend({

modelClass.eachAttribute((key) => {
attributeKey = this.keyForAttribute(key, 'deserialize');
if (resourceHash.hasOwnProperty(attributeKey)) {
if (resourceHash[attributeKey] !== undefined) {
attributes[key] = resourceHash[attributeKey];
}
});
Expand Down Expand Up @@ -649,7 +649,7 @@ export default Serializer.extend({
modelClass.eachRelationship((key, relationshipMeta) => {
let relationship = null;
let relationshipKey = this.keyForRelationship(key, relationshipMeta.kind, 'deserialize');
if (resourceHash.hasOwnProperty(relationshipKey)) {
if (resourceHash[relationshipKey] !== undefined) {
let data = null;
let relationshipHash = resourceHash[relationshipKey];
if (relationshipMeta.kind === 'belongsTo') {
Expand All @@ -675,7 +675,7 @@ export default Serializer.extend({
}

let linkKey = this.keyForLink(key, relationshipMeta.kind);
if (resourceHash.links && resourceHash.links.hasOwnProperty(linkKey)) {
if (resourceHash.links && resourceHash.links[linkKey] !== undefined) {
let related = resourceHash.links[linkKey];
relationship = relationship || {};
relationship.links = { related };
Expand Down Expand Up @@ -710,7 +710,7 @@ export default Serializer.extend({
typeClass.eachAttribute((key) => {
payloadKey = this.keyForAttribute(key, 'deserialize');
if (key === payloadKey) { return; }
if (!hash.hasOwnProperty(payloadKey)) { return; }
if (hash[payloadKey] === undefined) { return; }

hash[key] = hash[payloadKey];
delete hash[payloadKey];
Expand All @@ -729,7 +729,7 @@ export default Serializer.extend({
typeClass.eachRelationship((key, relationship) => {
payloadKey = this.keyForRelationship(key, relationship.kind, 'deserialize');
if (key === payloadKey) { return; }
if (!hash.hasOwnProperty(payloadKey)) { return; }
if (hash[payloadKey] === undefined) { return; }

hash[key] = hash[payloadKey];
delete hash[payloadKey];
Expand All @@ -749,7 +749,7 @@ export default Serializer.extend({
for (key in attrs) {
normalizedKey = payloadKey = this._getMappedKey(key, modelClass);

if (!hash.hasOwnProperty(payloadKey)) { continue; }
if (hash[payloadKey] === undefined) { continue; }

if (get(modelClass, 'attributes').has(key)) {
normalizedKey = this.keyForAttribute(key);
Expand Down Expand Up @@ -1266,7 +1266,7 @@ export default Serializer.extend({
@param {Object} payload
*/
extractMeta(store, modelClass, payload) {
if (payload && payload.hasOwnProperty('meta')) {
if (payload && payload['meta'] !== undefined) {
let meta = payload.meta;
delete payload.meta;
return meta;
Expand Down Expand Up @@ -1366,15 +1366,15 @@ export default Serializer.extend({

typeClass.eachAttribute((name) => {
let key = this.keyForAttribute(name, 'deserialize');
if (key !== name && payload.hasOwnProperty(key)) {
if (key !== name && payload[key] !== undefined) {
payload[name] = payload[key];
delete payload[key];
}
});

typeClass.eachRelationship((name) => {
let key = this.keyForRelationship(name, 'deserialize');
if (key !== name && payload.hasOwnProperty(key)) {
if (key !== name && payload[key] !== undefined) {
payload[name] = payload[key];
delete payload[key];
}
Expand Down
10 changes: 5 additions & 5 deletions addon/serializers/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ var RESTSerializer = JSONSerializer.extend({
* With `App.Comment`, `"comments"` and `{ id: 2, body: "Rails is unagi" }`

You can use this method, for example, to normalize underscored keys to camelized
or other general-purpose normalizations. You will only need to implement
or other general-purpose normalizations. You will only need to implement
`normalize` and manipulate the payload as desired.

For example, if the `IDs` under `"comments"` are provided as `_id` instead of
Expand All @@ -136,16 +136,16 @@ var RESTSerializer = JSONSerializer.extend({
normalize(model, hash, prop) {
if (prop === 'comments') {
hash.id = hash._id;
delete hash._id;
delete hash._id;
}

return this._super(...arguments);
}
});
```

On each call to the `normalize` method, the third parameter (`prop`) is always
one of the keys that were in the original payload or in the result of another
On each call to the `normalize` method, the third parameter (`prop`) is always
one of the keys that were in the original payload or in the result of another
normalization as `normalizeResponse`.

@method normalize
Expand Down Expand Up @@ -785,7 +785,7 @@ var RESTSerializer = JSONSerializer.extend({
var isPolymorphic = relationshipMeta.options.polymorphic;
var typeProperty = this.keyForPolymorphicType(key, relationshipType, 'deserialize');

if (isPolymorphic && resourceHash.hasOwnProperty(typeProperty) && typeof relationshipHash !== 'object') {
if (isPolymorphic && (resourceHash[typeProperty] !== undefined) && (typeof relationshipHash !== 'object')) {
let type = this.modelNameFromPayloadKey(resourceHash[typeProperty]);
return {
id: relationshipHash,
Expand Down
11 changes: 2 additions & 9 deletions tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -906,26 +906,23 @@ AssertionPrototype.convertsWhenSet = function(type, provided, expected) {
};

test("a DS.Model can describe String attributes", function(assert) {
assert.expect(6);
assert.expect(4);

assert.converts('string', "Scumbag Tom", "Scumbag Tom");
assert.converts('string', 1, "1");
assert.converts('string', "", "");
assert.converts('string', null, null);
assert.converts('string', undefined, null);
assert.convertsFromServer('string', undefined, null);
});

test("a DS.Model can describe Number attributes", function(assert) {
assert.expect(9);
assert.expect(8);

assert.converts('number', "1", 1);
assert.converts('number', "0", 0);
assert.converts('number', 1, 1);
assert.converts('number', 0, 0);
assert.converts('number', "", null);
assert.converts('number', null, null);
assert.converts('number', undefined, null);
assert.converts('number', true, 1);
assert.converts('number', false, 0);
});
Expand All @@ -938,18 +935,14 @@ test("a DS.Model can describe Boolean attributes", function(assert) {

if (isEnabled('ds-transform-pass-options') && isEnabled('ds-boolean-transform-allow-null')) {
assert.converts('boolean', null, null, { allowNull: true });
assert.converts('boolean', undefined, null, { allowNull: true });

assert.converts('boolean', null, false, { allowNull: false });
assert.converts('boolean', undefined, false, { allowNull: false });

// duplicating the tests from the else branch here, so once the feature is
// enabled and the else branch is deleted, those assertions are kept
assert.converts('boolean', null, false);
assert.converts('boolean', undefined, false);
} else {
assert.converts('boolean', null, false);
assert.converts('boolean', undefined, false);
}

assert.converts('boolean', true, true);
Expand Down