7

I'm trying to initialize two input with autocomplete with this library. When I load my page, I will trigger an Ajax to initialize two input text.

But I don't know how I can detect when all my mongoose find are completed.

Here is my server side code :

app.post('/init/autocomplete', function(req, res){
    var autocomplete = {
        companies: [],
        offices: []
    };

    // Find all companies
    Company.find({}, function(err, companies) {
        if (err) throw err;

        companies.forEach(function(company) {
            autocomplete.companies.push({value: company.name})
        });

        console.log('One');
    });

    // Find all offices
    Office.find({}, function(err, offices) {
        if (err) throw err;

        offices.forEach(function(office) {
            autocomplete.offices.push({value: office.name})
        });

        console.log('Two');
    });

    console.log('Three');

    // res.json(autocomplete);
});

I know than the find method is async. That is why I see my console.log() in this order :

Three
One
Two

How can I do to trigger console.log('Three'); when the Company.find and Office.find are finished ?

I want to see the console.log('Three'); at the last position.

Edit :

I think I can do this way :

app.post('/init/autocomplete', function(req, res){
    var autocomplete = {
        companies: [],
        offices: []
    };

    // Find all companies
    Company.find({}, function(err, companies) {
        if (err) throw err;

        companies.forEach(function(company) {
            autocomplete.companies.push({value: company.name})
        });

        // Find all offices
        Office.find({}, function(err, offices) {
            if (err) throw err;

            offices.forEach(function(office) {
                autocomplete.offices.push({value: office.name})
            });

            res.json(autocomplete);
        });
    });
});

But I don't know if it's the good way. Maybe using promise will be better ? I'm open for all suggestions.

John
  • 4,711
  • 9
  • 51
  • 101

5 Answers5

5

Mongoose has built-in support for promises which provide a clean way to wait for the completion of multiple async query operations with Promise.all:

// Tell Mongoose to use the native Node.js promise library.
mongoose.Promise = global.Promise;

app.post('/init/autocomplete', function(req, res){
    var autocomplete = {
        companies: [],
        offices: []
    };

    // Call .exec() on each query without a callback to return its promise.
    Promise.all([Company.find({}).exec(), Office.find({}).exec()])
        .then(results => {
            // results is an array of the results of each promise, in order.
            autocomplete.companies = results[0].map(c => ({value: c.name}));
            autocomplete.offices = results[1].map(o => ({value: o.name}));
            res.json(autocomplete);
        })
        .catch(err => {
            throw err; // res.sendStatus(500) might be better here.
        });
});
JohnnyHK
  • 305,182
  • 66
  • 621
  • 471
  • nice, but how does mongoose handle it's internal error? does it break when there is an error? or does it returns everything to results? Actually in some cases I would prefer to gather all the results, and let the client handle which query that success or failed – vdj4y Sep 13 '16 at 16:28
  • @vdj4y If the query fails the promise is rejected and the `catch` handler is invoked. You wouldn't use `Promise.all` in a case where a partial success needs to be handled differently. You're basically saying "all or nothing" with `Promise.all`. – JohnnyHK Sep 13 '16 at 16:32
  • I see, sometime, it would be nice though. I think if manually create the Promise, I can choose to resolve(err) and return immediately. but this should be my preferred code for other cases. thanks – vdj4y Sep 13 '16 at 16:34
  • Thank you for your answer. This is a good idea for my situation – John Sep 14 '16 at 07:44
  • .map() has better performance than forEach ? I never seen it and it looks very nice – John Sep 14 '16 at 08:05
  • @John [`map`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map) performance should be about the same as `forEach`, but `map` is just a cleaner, more idiomatic fit for mapping the result array into the form you need for the JSON response. – JohnnyHK Sep 14 '16 at 12:51
4

use Promise. There are ways to control parallel and series. and your code tends to be much readable. My method of dealing with parallel is to execute the async part first, then when the result have been collected, do the synchronous part.

app.post('/init/autocomplete', function(req, res){
    // Find all companies
    // the query action below is not executed, just return PromiseObject for now
    var findCompanies = new Promise((resolve, reject) => {
      Company.find({}, function(err, companies) {
          if (err) reject(err);
          resolve(companies)                   
       });
    })

    // Find all offices
    // the query action below is not executed, just return PromiseObject for now
    var findOffices = new Promise((resolve, reject) => {
      Office.find({}, function(err, offices) {
          if (err) reject(err);
          
          resolve(offices)
      });
    })
    
    // this is the execution part, in OOP world, you can think this is main()
    // execute and wait until each action(Promise Object) is complete before finally returning an array.
    return Promise.all([findCompanies, findOffices])
                  .then(array => {
                       console.log(array) // [ [companies] , [offices]  ]
                       //the difficult async part is done, with just few lines
                                                                           
                       console.log(array[0]) // [companies] array of companies
                       console.log(array[1]) // [offices] array of offices
                      
                       // now you can safely manipulate using forEach.
                       // basically, this part is for the synchronous action               
                       
                        var autocomplete = {};

                        array[0].forEach(function(company) {
                            autocomplete.companies.push({value: company.name})
                        });
                        array[1].forEach(function(office) {
                            autocomplete.office.push({value: office.name})
                        });
                       res.json(autocomplete)
                  })
});

the code above, findCompanies and FindOffices are Promise Object store in variable(it is not yet executed). Next, with Promise.all(), we run all the Promise Objects, and it will wait until each action is complete before finally returning an array.

the returned array contains another two array. The sequence of this array is the same as the sequence of actions you pass to Promise.all()

if you findOffices first, then findCompanies. It will return [[offices],[companies]] Promise.all([findOffices, findCompanies]) will return [[offices], [companies]]

vice versa

Promise.all([findCompanies, findOffices]) will return [[companies], [offices]]

Community
  • 1
  • 1
vdj4y
  • 2,649
  • 2
  • 21
  • 31
  • Very nice answer thanks. So I can do my foreach on `company.name` with array[0].name ? this array will contains my mongodb json object ? And it's better to use Native Promise or bluebird ? – John Sep 13 '16 at 13:56
  • array[0] is [companies] , you could do array[0].forEach( (company) => { //push to autocomplete }) – vdj4y Sep 13 '16 at 13:58
  • the sequence of array is the sequence you pass to Promise.all(), if you pass [offices, companies] it will return [ [offices], [companies] ]. vice versa – vdj4y Sep 13 '16 at 14:02
  • I have to remove the return keyword because I had this error : `var findCompanies = return new Promise(function(resolve, reject){` `SyntaxError: Unexpected token return`. And to make the forEach loop I have to initialize my autocomplete variable with `companies` and `offices` array else the loop is stopped. – John Sep 13 '16 at 15:01
  • oh yeah, sorry, I forget it is a variable not a function. will update the code – vdj4y Sep 13 '16 at 15:02
  • So I implemented your way because this is really what I want. It works perfeclty. – John Sep 13 '16 at 15:20
  • It would be better to avoid the anti-pattern of manually creating promises here and directly using the promises returned by `Company.find({}).exec()` and `Office.find({}).exec()`. – JohnnyHK Sep 13 '16 at 15:33
  • perhaps, you could show a code. I am just guessing, but maybe what you mean is to return Promise.resolve()?? – vdj4y Sep 13 '16 at 15:39
  • I choose to structure like above to prepare for error handling. If there is better pattern, please show an example. thanks – vdj4y Sep 13 '16 at 15:59
  • `return Promise.all([Company.find({}).exec(), Office.find({}).exec()]).then(...` – JohnnyHK Sep 13 '16 at 15:59
  • Wow, this seems to be nice – John Sep 13 '16 at 16:03
  • I see, because mongoose find() query use Promise, but how do you check if there is error in query? it seems this code will continue to run even when there is an error, and we need to loop the result-array to see which one fails. – vdj4y Sep 13 '16 at 16:03
2

With Promises you'll have a more clear code to maintain with not so much overhead, using Mongoose you have two options. Using the native Mongoose promises which is fine for basic use cases. But Mongoose promises are deprecated at this time as a warning will show up.

So to switch to native Promises:

// Native JS Promise
mongoose.Promise = global.Promise;

Or to the more advanced Bluebird Promise library:

// Bluebird
mongoose.Promise = require('bluebird');

An MVCE with native JS promises and Bluebird commented out.

const mongoose = require('mongoose');
const Schema = mongoose.Schema;
const assert = require('assert');

mongoose.connect("mongodb://localhost:33023/test_1");

// Native JS Promise
mongoose.Promise = global.Promise;

// Bluebird
// mongoose.Promise = require('bluebird');


// Schema creation
var Company = mongoose.model('Company', new Schema({
  name: String
}));

var Office = mongoose.model('Office', new Schema({
  name: String
}));


var acme = new Company({
  name: 'ACME'
});

var HR = new Office({
  name: 'Human Resources'
});

acme
  .save()
  .then(HR.save());

var query = Company.find({});
assert.equal(query.exec().constructor, global.Promise);
// assert.equal(query.exec().constructor, require('bluebird'));

query.exec()
.then(function(results) {
  console.log(results);
})
.then(Office.find({})
.then(function(results) {
  console.log(results);
}));

Documentation for Mongoose Promises.

Community
  • 1
  • 1
Marcs
  • 3,768
  • 5
  • 33
  • 42
  • `Promise.all()` is also useful in this situation. – robertklep Sep 13 '16 at 13:40
  • Thank you for your answer. I never use promise in node js this will be my first time. There is two approach with @vdj4y's answer and your answer. What is the difference ? – John Sep 13 '16 at 13:46
  • Basically [Promise.all](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all) returns a promise when all the promises have resolved. If any of the passed in promises rejects, the all Promise immediately rejects with the value of the promise that rejected, discarding all the other promises whether or not they have resolved. So if you want this behaviour is the way to go. In my case you can handle rejection in the flow. – Marcs Sep 13 '16 at 13:58
  • Also I'm using synchronous approach instead of @vdj4y that is using an asynchronous approach which is good for performance but you lose control on the execution flow. A good explanation: http://stackoverflow.com/questions/28066429/promise-all-order-of-resolved-values – Marcs Sep 13 '16 at 14:22
  • In my situation I prefer to be asynchronous. I need than all my promises are resolve. But I keep in mind your answer because I think this will really help me in the future. Thank you – John Sep 13 '16 at 15:23
0

The method you specified can be used but for a large number of async operation, it will result in callback from hell. In order to avoid this it is always better to write code in a sequential and orderly manner. In your case you can use a library like async to achieve this, since mongoose doesn't return a Promise Object. You can refer to this link for more information on async library.

async.series([
    function(callback) {
        // do some stuff ...
        console.log('one');
        callback(null, 'one');
    },
    function(callback) {
        // do some more stuff ...
        console.log('two');
        callback(null, 'two');
    }
],
// optional callback
function(err, results) {
    // results is now equal to ['one', 'two']
    console.log('three');
});

The third log will only be written once tasks one and two are completed. series function runs tasks one by one. You can use the eachSeries to run all tasks parallely.

Community
  • 1
  • 1
Ananth Pai
  • 1,939
  • 14
  • 15
-2

I'm quite new to mongoose but I just had a similar problem and I solved it by using a cursor()

let cursor = Company.find().cursor();
    cursor.on('data', function(doc){
        //do something with doc
    })


    cursor.on('close', function() {
        console.log("done");
    })