Uniqueness not working, fails with E11000 duplicate key error

[Miyou]

Hi. I can't seem to get the uniqueness to work. Here is a code sample that reproduces the problem. Any ideas on how to fix this?

var mongoose = require('mongoose');
var Promise = require('bluebird');
mongoose.Promise = Promise;
var slugify = require('mongoose-url-slugs');

var TestSchema = new mongoose.Schema({
    name: String
});
TestSchema.plugin(slugify('name'));
var Test = mongoose.model('Test', TestSchema);

mongoose.connect('mongodb://localhost/test-db');

var testPromises = [];
for (var i = 0; i < 20; i++) {
    var test = new Test({
        name: 'test'
    });

    testPromises.push(test.save());
};

Promise.all(testPromises).then(function() {
    console.log('done');
})
.catch(function(error) {
    console.error(error);
});

[talha-asad]

Are you able to save any out of the 20 that you try or all fail?

[Miyou]

Only the first is saved. It fails when trying to save the second document.

[talha-asad]

Don't know how I missed it. You have incorrect code, the first one isn't saved when the next one is called for saving. You see, your promises don't wait for the .save to finish.

[Miyou]

Well surely it would be better to catch the error and retry as to not compromise the async nature of mongoose? What if I have an express route that saves a document to the database and two users access the url at roughly the same time?

[talha-asad]

Well it's highly unlikely, retry feature could be added to this plugin however I think, this can be equally handled from the usage side.

[Miyou]

Well in my opinion the whole reason of having a plugin like this would be so that these things are handled automatically. I'm going to see if I can write a retry function. Would you want me to create a pull request or should I fork?

[talha-asad]

I'd accept a pull request :)

[Miyou]

Alright here is a pull request that seems to work perfectly! What do you think of this approach? Pull Request

[Miyou]

Just realized I made a big mistake and had to rework the code. Although now it will no longer be possible to use the regular save function, so maybe you don't want this functionality in your plugin? Let me know what you think: https://github.com/Miyou/mongoose-url-slugs/commit/fcbf46ead0de19d6af903a68fe8ad51ee28a864d

[talha-asad]

Why is it no longer possible to use the regular save function? I thought you were going to add a retry mechanism, so it would try to save an item and if it got duplicate index error, it would retry it in a configurable time. That approach would have been simple and configurable.

Can you explain what your code does ATM? I glimpsed quickly and think you implemented some sort of queue?

Also you shouldn't change tab spacing of the entire file when you submit PRs.

[Miyou]

Yeah I thought about doing a retry but that would just slow everything down as you would have to retry multiple times in the loop example. So instead I figured it would be much better to have a queue where documents with the same initial slug are queued so that they are saved one at a time. This way all other documents can be saved in an async way without waiting. The only problem here is that we can't do this with the pre('validate) approach that you are using because we need a callback from the save method to trigger the next document in the queue. So my solution was to create a saveUnique method which will use this queue and run the regular save method on each of the documents in the queue, now however we need to remove the pre('validate') hook otherwise we will be validating multiple times. Does that make sense?

I guess maybe you don't want such a backwards compatible breaking change to your package though? Sorry about the tabs, forgot to change back when I was done.

[Miyou]

I made some more changes and added updateUnique and findOneAndUpdateUnique functions. Everything seems to be working now. So if you want me to create a pull request I can, otherwise I'll just use my fork. https://github.com/Miyou/mongoose-url-slugs/commit/f40c9311632a455e5c73b5c397d5ab764ab25c3b

[talha-asad]

I haven't been able to fully review your fork, but i will be doing it sometime later this week hopefully. I suggest you continue to use it and i'll write a few test cases for this and we can merge in your changes if they work well when battle-tested. I'll ask you to submit a PR, after review. Thanks!

[Miyou]

Sounds good! I found two small bugs yesterday so refer to the latest version when you do your review.

[talha-asad]

I reviewed your fork, looks like you have done a lot of work, Thanks. Can you perhaps add a few test cases for the new functionality you added.