This fixes duplicate key issue #649

[BerkeleyTrue]

change.rectify will call save without __persisted flag set.
This causes DAO to treat model as new instead of as
an existing model to update.

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[bajtos]

@slnode ok to test

[bajtos]

Hi @BerkeleyTrue, thank you for submitting the patch. The solution looks too hacky to me, as it is using an internal juggler property. However, I don't know enough about the replication internals, I'll leave it up to @ritch to judge that.

[BerkeleyTrue]

Agreed on the hacky-ness, but I could not find exactly where this flag is to be set. This got me thinking about how that flag is normally set. Turns out if you change line 415 in change.js

change = new Change(change);

to

change = new Change(change, { persisted: true });

This also works, but I don't know what else this would affect.

[bajtos]

The new solution looks better to me. Please add a unit-test that fails without your patch and passes with your patch in place.

[bajtos]

Two more thoughts:

This looks like a bug in the juggler to me. I would expect that new Change(change).save() does update instead of create. /cc @raymondfeng

I don't understand why we are creating a new change object in the first place, since we already have one. Is there any reason why the following should not work?

this.find(function(err, changes) {
  if (err) return cb(err);
  changes.forEach(function(change) {
    change.rectify();
  });
});

[BerkeleyTrue]

Removed the instantiation line. All seems to work as expected. Are there test that need to be added?

[bajtos]

Are there test that need to be added?

I am don't know if there is an easy way how to test your changes in isolation against in-memory database. We should ideally run the test suite against MongoDB, but that's out of scope of this PR.

Could you please squash the commits and update the commit message?

[bajtos]

(Other than that, the patch LGTM.)

[BerkeleyTrue]

Sorry, squash the commits? Not sure what that means.

[bajtos]

@BerkeleyTrue run git rebase -i master and follow the instructions in the editor opened by git. Once done, run git push -f to rewrite the history on the server too. More details can be found here: http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

Let me know if you need more help.

[desmondmorris]

+1 this patch does the trick

[BerkeleyTrue]

That was fun. How does that look?

[bajtos]

@BerkeleyTrue Much better :)

One more thing - I need you to sign our CLA, see https://cla.strongloop.com/agreements/strongloop/loopback.

[bajtos]

@slnode ok to test

[bajtos]

Landed, thank you for the contribution!

[BerkeleyTrue]

Woot!