model.save() attempts to update and then throws error when hasChanged() is false

[coolaj86]

I would prefer that model.save() graciously become a noop and neglect to run the sql statement when hasChanged() returns false, but still call the success callback.

Pretty:

model.save().then(function () {
  // do stuff
});

vs

Ugly:

function onSuccess(model) {
  // do stuff
}

if (model.hasChanged) {
model.save().then(onSuccess);
} else {
  onSuccess(model);
}

Perhaps there's another method saveUnlessUnchanged() or perhaps a unlessChanged: true option I could pass to save() that I don't know about?

The practical reason behind this is that I'm taking data in from a third party source that may or may not have changed since the last time I queried. Also, there are many cases where an array of user data may come in with some elements changed and some left as they were.

[bendrucker]

Where would this help you avoid a query in the real world? hasChanged becomes true any time an attribute is set that's not a noop. A brand new model is seen as having changed. Doesn't matter that its values might be already persisted.

[coolaj86]

Here's my use case:

Everytime a user logs in with oauth I have to query the logins table using typedUid which is a manual composite of their login id (i.e. 1274648392) and the login provider (i.e. facebook).

If it doesn't fetch a record, I create a new one.

If the record doesn't have an associated account, I create one of those as well.

On the account I'm going to loop through a limited number of fields and set them. 99% of the time it's setting them to the same thing it was already set to (my facebook url probably didn't change, my preferred name probabably didn't change, my facebook email probably didn't change, etc).

If I'm creating the account, I'm going to attach() it to the login.

On the login I'm going to loop over all of the fields and set them. Most of them actually get stored as a json column called xattrs, but 90% of the time they don't change and the string is the same string and hasChanged() returns false (or at least changed was an empty object).

At this point nothing has actually changed most of the time because most of the time it's returning users. But sometimes it's new users. And occasionally the returning user has changed their info.

Then I want to call save().

I'm familiar with shortcuts like findOrCreate() and save() that behaves as a noop when nothing has changed. I understand that you don't have tie to code the world and everyone wants a pony, but if you think those are good and practical ideas then, since I have to write the logic anyway, I may figure out enough of framework to do a pull request for that at some point.

Side question: Is it possible to attach() a new model? It seemed like last night I had to make sure I saved it first.

In generally I've just got a lot of new/dirty/exists if-branching that feels (to me) like it belongs in the ORM rather than in the application.

Another aside: If you have time for an hour of consulting sometime next week, I'd really like to have you take a look at my code, tell me what I'm doing "the wrong way" and explan the "right way" to me. I'm imagining that when I'm done with this specific part of my project there will only be 3 tables and maybe 500 lines of actual code.

[bendrucker]

Hmm. Read through all that and I think I at least understand the gist of what you're doing.

I'd actually contend the opposite on built in versus userland in this case. Detecting changes is tricky because you need to fetch the entire model first in order to really know for sure whether a set is creating values that will result in a change to the database. That's a decision you may be able to make based on a whole host of complicated factors specific to the business logic of your app. Getting it to work in a way that is sensible for everyone would be tough.

Oftentimes it's not a bad idea to just try to keep operations idempotent and ignore the possibility of unnecessary writes. But unless it's a huge issue, it's often better to just accept the additional response time and address the issue comprehensively in the future with some sort of caching strategy.

Re: consulting, I'm happy to spend some time digging in. Email's on my profile.

Also, be careful with your conditional in your example. You may be releasing zalgo.

[crunchtime-ali]

The project leadership of Bookshelf recently changed. In an effort to advance the project we close all issues older than one year.

If you think this issue needs to be re-evaluated please post a comment on why this is still important and we will re-open it.

We also started an open discussion about the future of Bookshelf.js here #1600. Feel free to drop by and give us your opinion.
Let's make Bookshelf great again