Firestore query for loop with multiple values

Question

I am attempting to retrieve a number of Firestore documents using data held in a string. The idea is that for each value in the array, i'd use Firestore query to retrieve the document matching that query and push it to another array. I am having a few issues achieving this. So far i've tried:

exports.findMultipleItems = functions.https.onRequest((request, response) => {
    var list = ["item1", "item2", "item3", "item4"];

    var outputList = [];

    for (var i = 0; i < list.length; i++) {
        console.log("Current item: " + list[i]);
        let queryRef = db.collection("items").where('listedItems', 'array-contains', list[i]).get()
            .then(snapshot => {
                if (snapshot.empty) {
                    console.log('No matching documents.');
                }

                snapshot.forEach(doc => {
                    outputList.push(doc.data());
                });
                return;
            })
            .catch(err => {
                console.log('Error getting documents', err);
            });
    }

    response.send(JSON.stringify(outputList));

});

I'm not entirely sure but i think one of the issues is that the for loop is being completed before the queries have a chance to finish.

P.s - this is being ran through Cloud Functions using Admin SDK.

Asynchronous programming can be difficult, but there's not enough information here to know what might be going wrong. You mentioned that you're using Cloud Functions - please edit the question to show the entire function. Dealing with promises correctly in Cloud Functions is extremely important. As it stand now, all I can see is that outputList will only be populated after the queries finish, and there's nothing waiting on those queries to become complete. — Doug Stevenson
– Doug Stevenson, Commented Aug 31, 2019 at 15:45
I've added the entire function. How would i go about waiting on the queries to complete? — Sha-1
– Sha-1, Commented Aug 31, 2019 at 15:49

Doug Stevenson · Accepted Answer · 2019-08-31 15:55:16Z

7

Your queryRef is not actually a reference. It's a promise that resolves after your get/then/catch have finished. You need to use these promises to determine when they're all complete. The array will be populated only after they are all complete, and only then is it safe to send the response using that array.

Collect all the promises into an array, and use Promise.all() to get a new promise that resolves after they're all complete:

exports.findMultipleItems = functions.https.onRequest((request, response) => {
    var list = ["item1", "item2", "item3", "item4"];

    var outputList = [];
    const promises = [];

    for (var i = 0; i < list.length; i++) {
        console.log("Current item: " + list[i]);
        let promise = db.collection("items").where('listedItems', 'array-contains', list[i]).get()
            .then(snapshot => {
                if (snapshot.empty) {
                    console.log('No matching documents.');
                }

                snapshot.forEach(doc => {
                    outputList.push(doc.data());
                });
                return;
            })
            .catch(err => {
                console.log('Error getting documents', err);
            });
        promises.push(promise);
    }

    Promise.all(promises).then(() => {
        response.send(JSON.stringify(outputList));
    }
    .catch(err => {
        response.status(500);
    })

});

You might want to use these tutorials to better understand how to deal with promises in Cloud Functions:

https://firebase.google.com/docs/functions/video-series/

answered Aug 31, 2019 at 15:55

Doug Stevenson

321k37 gold badges458 silver badges474 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Holli Over a year ago

I learned that it is "favourable not to change the OP code too much" and also this that this code works, but it is not yet optimal. The "idiomatic" way is to use higher order list functions and by emitting the "return values" from within the promises, instead of relying on a side-effect array.

Steve Klock Over a year ago

Just a quick comment on what going on here to help others. You are creating an array with the name promises before the loop, but the name is not important. Then you packaging up the promise that is inside the loop into a variable called promise. Again any name will do. Then you push that promise into the promises array at the end of each loop. You end up with a array of promises. Then you call Promise.all() on the promises array, which resolves all of the promises before moving forward in your code, putting the desired data into the OutputList array. Hope that helps--this code works

Neel Rathod · Accepted Answer · 2019-08-31 16:09:11Z

You need to look into promises, Never called a promise in a loop like this. First, you need to chunk your code which returns result from the DB(asynchronously) and uses Promise.all() to handle multiple promises.

utils.getData = async (item) => {
        try {
                const result = await db.collection("items").where('listedItems', 'array-contains', item).get();
                return result;
        } catch (err) {
                throw err;
        }
};

utils.getDataFromDB = async () => {
        try {
                const list = ["item1", "item2", "item3", "item4"];

                const outputList = [];
                const promises = [];

                for (var i = 0; i < list.length; i++) {
                        console.log("Current item: " + list[i]);
                        const element = list[i];                   
                        promises.push(utils.getData(elem));
                }

                const result = await Promise.all(promises);
                result.forEach((r) => {
                        if (r.empty) {
                                console.log('No matching documents.');
                        } else {
                                snapshot.forEach(doc => {
                                        outputList.push(doc.data());
                                });
                        }
                });
                return outputList;
        } catch (err) {
                throw err;
        }
}

module.exports = utils;

Holli · Accepted Answer · 2019-08-31 17:23:16Z

Here is my attempt at fully idiomatic solution. It needs no intermediate variables (no race conditions possible) and it separates concerns nicely.

function data_for_snapshot( snapshot ) {
    if ( snapshot && !snapshot.empty )
        return snapshot.map( doc => doc.data() );

    return [];
}   

function query_data( search ) {
    return new Promise( (resolve, reject) => {
        db
        .collection("items")
        .where('listedItems', 'array-contains', search)
        .get()
        .then( snapshot => resolve(snapshot) )
        .catch( resolve( [] ) );
    });
}

function get_data( items )
{
    return new Promise( (resolve) => {
        Promise
            .all( items.map( item => query_data(item) ) )
            .then( (snapshots) => {
                resolve( snapshots.flatMap( 
                        snapshot => data_for_snapshot(snapshot) 
                ));
            });
    });
}

get_data( ["item1", "item2", "item3", "item4"] ).then( function(data) {
    console.log( JSON.stringify(data) );
});

I used a simple mockup for my testing, as i don't have access to that particular database. But it should work.

function query_data( search ) {
    return new Promise( (resolve, reject) => {
        setTimeout( () => { 
            resolve([{
                data: function() { return search.toUpperCase() },
                empty: false
            }])
        });
    });
}

Collectives™ on Stack Overflow

Firestore query for loop with multiple values

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related