fix non-object attributes being passed to model.parse()

[brendantelz]

• Related Issues: .save({method: 'insert') is attempting to parse() undefined objects #2026

Introduction

When model.save() is executed with method: 'update', the resp argument is first the number of rows that were updated. This causes undefined to be passed into this.parse on line 1183 in model.save(). Any custom model.parse functions on a model are expecting an object of attributes to be passed.

Also, this branch can be reached if the save method is 'insert' and this.id is not null, which we have found to occur when query.options.returning was set to 'id' instead of the default '*' which bookshelf expects.

Motivation

Any custom model.parse functions are not expecting to be receiving any argument other than an object, potentially causing unexpected errors.

Proposed solution

Check that the response is an object before passing it to model.parse and merging with this.attributes.

[ricardograca]

Won't this fix mean that we will still be left without the updated attributes?

[brendantelz]

@ricardograca When the query returns the updated attributes in an object, yes it will add the updated attributes. Added a unit test for that behavior.

Perhaps you know more about why this is, but I found that this code gets called twice on a model.save() - the first time resp is number of rows updated, the second time it receives an array containing the object with all model attributes.

Thank you for the review!