Mongoose inserting same data three times instead of iterating to next data

Question

I am trying to seed the following data to my MongoDB server:

const userRole = {
    role: 'user',
    permissions: ['readPost', 'commentPost', 'votePost']
}
const authorRole = {
    role: 'author',
    permissions: ['readPost', 'createPost', 'editPostSelf', 'commentPost',
'votePost']
}
const adminRole = {
    role: 'admin',
    permissions: ['readPost', 'createPost', 'editPost', 'commentPost',
    'votePost', 'approvePost', 'approveAccount']
}
const data = [
    {
        model: 'roles',
        documents: [
            userRole, authorRole, adminRole
        ]
    }
]

When I try to iterate through this object / array, and to insert this data into the database, I end up with three copies of 'adminRole', instead of the three individual roles. I feel very foolish for being unable to figure out why this is happening.

My code to actually iterate through the object and seed it is the following, and I know it's actually getting every value, since I've done the console.log testing and can get all the data properly:

for (i in data) {
        m = data[i]
        const Model = mongoose.model(m.model)
        for (j in m.documents) {
            var obj = m.documents[j]

            Model.findOne({'role':obj.role}, (error, result) => {
                if (error) console.error('An error occurred.')
                else if (!result) {
                    Model.create(obj, (error) => {
                        if (error) console.error('Error seeding. ' + error)
                        console.log('Data has been seeded: ' + obj)
                    })
                }
            })
        }
    }

Update:

Here is the solution I came up with after reading everyone's responses. Two private functions generate Promise objects for both checking if the data exists, and inserting the data, and then all Promises are fulfilled with Promise.all.

// Stores all promises to be resolved
var deletionPromises = []
var insertionPromises = []
// Fetch the model via its name string from mongoose
const Model = mongoose.model(data.model)
// For each object in the 'documents' field of the main object
data.documents.forEach((item) => {
    deletionPromises.push(promiseDeletion(Model, item))
    insertionPromises.push(promiseInsertion(Model, item))
})

console.log('Promises have been pushed.')
// We need to fulfil the deletion promises before the insertion promises.
Promise.all(deletionPromises).then(()=> {
    return Promise.all(insertionPromises).catch(()=>{})
}).catch(()=>{})

I won't include both promiseDeletion and promiseInsertion as they're functionally the same.

const promiseDeletion = function (model, item) {
    console.log('Promise Deletion ' + item.role)
    return new Promise((resolve, reject) => {
        model.findOneAndDelete(item, (error) => {
            if (error) reject()
            else resolve()
        })
    })
}

Update 2: You should ignore my most recent update. I've modified the result I posted a bit, but even then, half of the time the roles are deleted and not inserted. It's very random as to when it will actually insert the roles into the server. I'm very confused and frustrated at this point.

score 0 · Answer 1 · answered Jan 23 '19 at 08:02

You ran into a very common problem when using Javascript: You shouldn't define (async) functions in a regular for (-in) loop. What happens, is that while you loop through the three values the first async find is being called. Since your code is async, nodejs does not wait for it to finish, before it continues to the next loop iteration and counts up to the third value, here the admin rule. Now, since you defined your functions in the loop, when the first async call is over, the for-loop already looped to the last value, which is why admin is being inserted three times.

To avoid this, you can just move the async functions out of the loop to force a call by value rather than reference. Still, this can bring up a lot of other problems, so I'd recommend you to rather have a look at promises and how to chain them (e.g. Put all mongoose promises in an array and the await them using Promise.all) or use the more modern async/await syntax together with the for-of loop that allows for both easy readability as well as sequential async command instructions.

Check this very similar question: Calling an asynchronous function within a for loop in JavaScript

Note: for-of is being discussed as to performance heavy, so check if this applies to your use-case or not.

Yes after doing a lot of research I concluded it was due to asynchronous function calls, I just had no idea how to actually go about fixing it. That link is helpful, and with the other response I know how to tackle it. Thanks a ton! — , Jan 24 '19 at 13:00

score 0 · Answer 2 · answered Jan 23 '19 at 10:03

When using async functions in loops could cause some problems.

You should change the way you work with findOne to make it synchronous function

First you need to set your function to async, and then use the findOne like so:

async function myFucntion() {
  let res = await Model.findOne({'role':obj.role}).exec();//Exec will fire the function and give back a promise which the await can handle.
  //do what you need to do here with the result..
}

Mongoose inserting same data three times instead of iterating to next data

2 Answers2