0

I am trying to retrieve data in a tree format from Mongodb recursively using Mongoose in a Node Express app. I'm trying to do it with promises and I am clearly not doing it right.

Say you have the mongodb table foos':

{
    "_id": {
        "$oid": "5b5fb27232db7b13a1090577"
    },
    "bazs": [
        {
            "$oid": "5b626a2e325c181bdf4fba08"
        }
    ],
    "bars": [
        {
            "$oid": "5b5fb3cc32db7b13a1090579"
        }
    ],
    "name": "Parent1",
    "accountId": "5b5fb27132db7b13a1090576",
    "__v": 0
}

So this has two different kind of child objects. Also, the baz objects have other baz objects as children, as well as bar objects. Say this is the table that the id for bars:

{
    "_id": {
        "$oid": "5b626a2e325c181bdf4fba08"
    },
    "bazs": [
        {
            "$oid": "5b5fb3cc32db7b75fa01"
        }
    ],
    "bars": [
        {
            "$oid": "5b5fb3c7cdaf740a500a"
        }
    ],
    "accountId": "5b5fb27132db7b13a1090576",
    "parent": {
        "$oid": "5b5fb27232db7b13a1090577"
    },
    "__v": 0
}

I've been using a recursive function in a thunk waiting for an object type to retrieve using Mongoose.

const getObjectRecursive = (type, id)=>{     //'type' will be 'Foo' or 'Bar'
    return new Promise((resolve, reject)=>{
        type.findOne.call(type, {_id:id},{}, err=>{
            if(err){
                reject(err);
            }
        })
            .then((rslt)=>{
                const result = rslt;
                const bars = result.bars;
                const bazs = result.bazs;
                result.children = {};
                result.children.bars = tasks.map(async barId=>{
                    const barIdString = barId.toString();
                    await getBarRecursive(barIdString, err=>{
                        if(err){
                            reject(err);
                        }
                    });
                });
                result.children.bazs = bazs.map(async eventId=>{
                    const bazIdString = bazId.toString();
                    await Baz.findOne({_id:eventIdString}, {},err=>{
                        if(err){
                            reject(err);
                        }
                    });
                });
                resolve(result);
            })
    });

}

And I use it in these two functions:

const getBarRecursive = (taskId)=>{
    return getObjectRecursive(Bar, taskId);

}

const getFooRecursive=(categoryId)=>{
    return getObjectRecursive(Foo,categoryId);
};

Which are called by this route:

router.get('/:id', verifyToken, (req,res)=>{
    Branch.getFooRecursive(req.params.id)
        .then(
            (foo)=>{
                res.status(200).send(cat);
            },
            (err)=>{
                res.status(500).send(err);
            }
        )
});

This is meant to return the tree object recursively and all the objects' children. What's happening is that the children objects result.children.bars and result.children.bazs become promises that never get fulfilled.

This is not the best way to do this, I know. I'm clearly not doing promises right. I also thought about retrieving the table using a $graphLookup, but I can't figure out how to do that since the children come from different collections.

How can I make the app retrieve the children to any depth?

UPDATE: This is still not working. I've changed the recursive function to this:

const getObjectRecursive = async (type, id)=>{     
    const rslt = await type.findOne.call(type, {_id:id},{}, {}, err=>{
        if(err){
            return({err: err});
        }
    });

    const result = rslt.toObject();
    const bars = result.bars;
    const bazs = result.bazs;
    result.children = {};
    result.children.bars = tasks.map(async barId=>{
        const barIdString = barId.toString();
        await getBarRecursive(barIdString, err=>{
            if(err){
                reject(err);
            }
        });
    });
    result.children.bazs = bazs.map(async eventId=>{
        const bazIdString = bazId.toString();
        await Baz.findOne({_id:eventIdString}, {},err=>{
            if(err){
                reject(err);
            }
        });
    });
    return result
};

Everything else is the same. What's happening now is that the promises containing the bar result isn't being fulfilled until AFTER the result is sent to the client, so I am getting an empty object in it's place, like this:

{
    "bars": [
        "5b626a2e325c181bdf4fba08"
    ],
    "bazs": [],
    "_id": "5b5fb27232db7b13a1090577",
    "__v": 0,
    "children": {
        "bars": [
            {}
        ],
        "bazs": []
    }
}

Some things I've tried:

  • placing the bars.map(async barId=>... etc. inside a Promises.all statement and assigning it to result.children in a .then(...) statement
  • placing the bars.map(async barId=>... etc. inside a separate async function and then saying result.children.bars = await getAllTasks(tasks)

How can I make sure my promise is fulfilled before the result is sent?

jimboweb
  • 4,362
  • 3
  • 22
  • 45
  • 1
    The example heavily uses https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it . Mongoose supports promises. Why do you keep using findOne callback? And you're mixing these things with async/await. The problem would likely be solved if you were using async/await everywhere. – Estus Flask Aug 04 '18 at 00:09
  • So you're saying instead of making findObjectRecursive return a promise, make it and getFooRecursive and getBarRecursive into async functions? – jimboweb Aug 04 '18 at 00:23
  • 1
    You still can use recursion if you need but with much cleaner control flow that leaves less space for error. `const result = await type.findOne(...)`, etc. – Estus Flask Aug 04 '18 at 00:25
  • I'll try this tomorrow – jimboweb Aug 04 '18 at 06:43
  • So this has succeeded in getting my promises fulfilled, thank you. It looks like I have another problem where the stuff I'm adding to my returned objects isn't being included in what's returned but that's something else. Thank you. – jimboweb Aug 05 '18 at 00:02
  • For anyone who's wondering, the problem was that I was trying to add properties onto a Mongoose object, so they weren't being returned. I changed the line `const result = rslt` to `const result = rslt.toObject()` which turned it into a plain (non-mongoose) object so I could add things to it. – jimboweb Aug 05 '18 at 00:08
  • 1
    Btw, it's a good idea to do this by default, unless you need the opposite. This is done like `const result = await type.findOne(...).lean()`. – Estus Flask Aug 05 '18 at 00:11
  • So it's still not quite working because some of my promises aren't being fulfilled until after the result is sent. The problem's described in the update above. – jimboweb Aug 06 '18 at 23:48
  • There are numerous problems here. I tried to explain them. I cannot guarantee that this will work because the logic depends on actual data structure. Since there's a bunch of requests and nested promises, it may be better to move some of these requests to Mongo side or restructure data. The abundance of `findOne` is a clue that there could be a single `find` instead. – Estus Flask Aug 07 '18 at 10:15

1 Answers1

0

Mongoose supports promises for a long time, there is no need to use callbacks. reject function is undefined, its use will result in exception. Also, call is unjustified here. There are also several inconsistencies, tasks vs bars and eventId vs bazId. All of them will prevent the code from working normally

It should be something like:

const getObjectRecursive = async (type, id)=>{     
    const result = await type.findOne({_id:id}).lean();
    const barsPromises = result.bars.map(barId=>{
        return getBarRecursive(barId.toString()); // shouldn't it accept 2 args?
    });
    const bazsPromises = result.bazs.map(bazId =>{
        return Baz.findOne({_id:bazId.toString()}).lean();
    });

    const [bars, bazs] = await Promise.all([
      Promise.all(barsPromises),
      Promise.all(bazsPromises)
    ]);
    result.children = { bars, bazs };

    return result;
};
Estus Flask
  • 206,104
  • 70
  • 425
  • 565