How do I properly update models with Bookshelf.js?

Question

I'm sure I am missing something but I find the Bookshelf API to be relentlessly confusing for me. Here's what I am trying to do:

• I have a model called Radio with an application-assigned string primary key named serial and, for the purpose of this example, two fields named example1 and example2. I have specified the custom ID with idAttribute: 'serial' in the model definition.
• I'm trying to perform an upsert with Bookshelf (not with Knex directly, in my actual application that query becomes rather complex).
• I've got the insert case working but can't seem to get the update case working.
• For simplicity I'm not caring about transactions or atomicity right now. I am satisfied to get a simple select → insert/update to work.

And specifically in this example:

• On insert set example1 and example2.
• On update set example1 and leave example2 unchanged.

So I've got something like this in the Bookshelf model, as a class (i.e. "static") method ("info" has fields "serial", "example1", and "example2"):

insertOrUpdate: function (info) {
    return new Radio({'serial':info.serial}).fetch().then(function (model) {
        if (model) {
            model.set('example1', info.example1);
            return model.save({}, {
                method: 'update',
                patch: true
            })
        } else {
            return new Radio({
                serial: info.serial,
                example1: info.example1,
                example2: info.example2
            }).save({}, {
                method: 'insert'
            })
        }
    }).then(function (model) {
        console.log("SUCCESS");
    }).catch(function (err) {
        console.log("ERROR", err);
    });
}

Example call:

Radio.insertOrUpdate({
    serial: ...,
    example1: ...,
    example2: ...
})

The problem I'm running into here is that while the "insert" case works, the "update" case fails with:

ERROR { Error: ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'where `serial` = '123223'' at line 1

Which is obvious with Knex debugging turned on, where the generated query is missing the set clause:

update `radios` set  where `serial` = ?

Now, I'm focused on the Bookshelf docs for fetch and save, and I'm wondering if I'm headed in the wrong direction.

I know I'm using the API wrong but I can't figure it out. A couple of weird things I noticed / had to do just to get it into its semi-working state:

• I don't understand the first parameter of save. It would make sense to me if save was a static method of Model, but it isn't. It's an instance method and you can already pass attributes to the Model constructor (e.g. what would new X(a:1).save({a:2}) mean...?), and you can already set attributes with set prior to the save. So I can't make sense of this. I had to pass {} as a placeholder to let me specify the options.

• There is this forge thing but I'm not sure what its purpose is since you can already pass attributes to the Model constructor (unless the authors found some benefit to X.forge({a:1}) vs. new X({a:1})...?).

• I found that I had to explicitly specify the method to save because of an apparent Bookshelf quirk: Bookshelf bases its choice of method on isNew(), but isNew() is always true when you pass the id to the model constructor, which you have to do in the application-assigned ID case. Therefore for application-assigned IDs Bookshelf will always do an "insert" since it always thinks the model "is new". So you have to force the method to "update"... this just adds to my Bookshelf confusion.

Anyways, how do I do this properly? How do I make this insert and update work?

More importantly, where is this documented? In good faith I assume that it is documented clearly somewhere and I just can't see the forest for the trees right now, so I'd actually appreciate some direction to follow in the docs even more than a direct answer, because I do need to figure this out. I've been spending a lot of time on Bookshelf instead of actual development, so much that I almost wish I would have just stuck to direct SQL queries from the start.

Answer 1

That was an interesting one and took me some time to understand what was going on.

As you seem to have found out, the save() method documentation regarding the patch option states that it

Only save attributes supplied in arguments to save.

So you just need to change your code to

if (model) {
    model.set('example1', info.example1);
    return model.save();
}

and the set attributes will be saved.

BUT BUT BUT BUT

ALL attributes will get into the update statement, even the id!

This is a common behavior for ORMs, its rationale is that if we got the data from one transaction and are saving from another one (bad, bad practice!), the data may have been changed by some other client. So saving only part of the attributes may lead to inconsistent state.

But the very existence of the patch attribute violates this concept. So Bookshelf could be improved by:

• Simply deprecating the patch option. (I could prefer that)
• Since Bookshelf models keep track of changed attributes I think it should be trivial to make the updates smarter on this regard. This change could lead too to the deprecation of the patch option.
• Another approach could be making the patch semantics relate to the changed attributes instead to just the ones supplied on the save(). But such change could unfortunately break some use cases.
• Or finally introducing a new option to act upon all changed attributes. But that feels messy.

Answer 2

After a few iterations of guessing I seem to have gotten it working, but I have no idea if this is right or how I could have determined this without guessing, and I definitely do not stand by its correctness.

Basically I was able to get it to work by modifying the "update" case to:

• Pass the attributes to save as its first parameter, rather than setting them with set.

Leading to a final solution of:

insertOrUpdate: function (info) {
    return new Radio({'serial':info.serial}).fetch().then(function (model) {
        if (model) {
            // pass params to save instead of set()
            var params = { 'example1' : info.example1 }
            return model.save(params, {
                method: 'update',
                patch: true
            })
        } else {
            return new Radio({
                serial: info.serial,
                example1: info.example1,
                example2: info.example2
            }).save({}, {
                method: 'insert'
            })
        }
    }).then(function (model) {
        console.log("SUCCESS");
    }).catch(function (err) {
        console.log("ERROR", err);
    });
}

I'm still not sure how/if forge fits in here or what the deal should be with the first parameter to save in the "insert" case.

More importantly, I'm not entirely sure what set is for, now. One of the main benefits of an ORM framework is supposed to be making that kind of stuff transparent (i.e. "save" works correctly while letting you use the model without thinking about it, and without you having to have knowledge of what has changed at the point you "save" -- I should be able to have unknown arbitrary code set things ahead of time then be able to save it without knowing what changed, but it looks like I can't), so I'm not sure what I've actually gained from Bookshelf here. There must be a better approach.