Mongoose: Question. Why findOne, create don't work properly in a forEach loop?

[YannisMarios]

Hi,

Maybe I am making a mistake but here's the thing. I am using a forEach loop to create invoices in the database:

res.forEach(async r => {
 let invoiceData = await createInvoice(r);
});

const createInvoice = async (data) => {
    let invoiceIndex;
    let lastInvoice = await db.invoice.model.findOne({company: data.company.id}).sort({invoiceIndex: -1});
    console.log(lastInvoice); // null if invoice collection is empty
    if(lastInvoice) {
      invoiceIndex = lastInvoice.invoiceIndex + 1;
    } else {
      invoiceIndex = data.company.invoiceStart;
    }

    let invoiceData = {
      invoiceIndex: invoiceIndex,
           //... rest of data
    };

    let invoice = await db.invoice.model.create(invoiceData);
    return invoice;
};

The problem is that if the invoice collection is empty the findOne query returns invoiceData = null not only the first time but on every iteration in the loop. This is strange because after findOne I am creating the invoice and so the second time in the loop the findOne query should return the invoice with invoiceIndex : 1 and so on. Instead all invoices are saved with invoiceIndex: 1 (data.company.invoiceStart). What am I doing wrong here? Any help would be greatly appreciated.

Thank you.

[lineus]

@YannisMarios I know we discussed this on the mongoose slack channel, but In case someone stumbles on this later I wanted the answer to be here as well.

the forEach loop runs the function ( even an async function ) in parallel for each element in res. this will not behave as you intended. Here is one potential solution:

6556.js

#!/usr/bin/env node
'use strict';

const mongoose = require('mongoose');
mongoose.connect('mongodb://localhost/test');
const conn = mongoose.connection;
const Schema = mongoose.Schema;

const schema = new Schema({
  name: String,
  invoiceIndex: Number,
  company: {
    invoiceStart: Number
  }
});

const Test = mongoose.model('test', schema);

let res = [
  { 
    company: {
      invoiceStart: 0
    }
  },
  {
    company: {
      invoiceStart: 0
    }
  },
  {
    company: {
      invoiceStart: 0
    }
  }
];

async function run() {
  let startIndex = await Test.findOne({}).sort({ invoiceIndex: -1 });
  if (!startIndex) {
    startIndex = res[0].company.invoiceStart;
  } else {
    startIndex = startIndex.invoiceIndex + 1;
  }
  let docs = res.map(doc => {
    let thisIndex = startIndex++;
    let ret = Object.assign({}, doc);
    ret.name = `test${thisIndex}`;
    ret.invoiceIndex = thisIndex;
    return ret;
  });

  await Test.create(docs);
  let found = await Test.find();
  console.log(found);
  return conn.close();
}

run();

Output of 3 consecutive runs:

slack: ./ym.js
[ { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914477,
    name: 'test1',
    invoiceIndex: 1,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914476,
    name: 'test0',
    invoiceIndex: 0,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914478,
    name: 'test2',
    invoiceIndex: 2,
    __v: 0 } ]
slack: ./ym.js
[ { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914477,
    name: 'test1',
    invoiceIndex: 1,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914476,
    name: 'test0',
    invoiceIndex: 0,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914478,
    name: 'test2',
    invoiceIndex: 2,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6d,
    name: 'test3',
    invoiceIndex: 3,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6e,
    name: 'test4',
    invoiceIndex: 4,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6f,
    name: 'test5',
    invoiceIndex: 5,
    __v: 0 } ]
slack: ./ym.js
[ { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914477,
    name: 'test1',
    invoiceIndex: 1,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914476,
    name: 'test0',
    invoiceIndex: 0,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166624d532ff1443914478,
    name: 'test2',
    invoiceIndex: 2,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6d,
    name: 'test3',
    invoiceIndex: 3,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6e,
    name: 'test4',
    invoiceIndex: 4,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166626b55703144ebacf6f,
    name: 'test5',
    invoiceIndex: 5,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166628bb655d1459600832,
    name: 'test6',
    invoiceIndex: 6,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166628bb655d1459600833,
    name: 'test7',
    invoiceIndex: 7,
    __v: 0 },
  { company: { invoiceStart: 0 },
    _id: 5b166628bb655d1459600834,
    name: 'test8',
    invoiceIndex: 8,
    __v: 0 } ]
slack:

[YannisMarios]

Actually the answer is much simpler:

Instead of using forEach, or map I used for/of loop. This loop awaits for async operation to complete which is what I really want in this case because I want to find the next invoiceIndex from the database;

so this:

for(const r of res) {
 let invoiceData = await createInvoice(r);
}

works and runs async operations in series;

Thanks

[vkarpov15]

Yeah this is just how async functions work, better to use a for loop. I wrote more about the quirks with forEach and async/await here: http://thecodebarbarian.com/basic-functional-programming-with-async-await