Nested Javascript promises - fetching data from firestore

Question

I have been stuck with this error for the past 3 days and i have tried literally everything, tried to structure the promises in a 1000 ways but nothing seems to work. Maybe I am losing the "big picture" so hopefully new eyes will help. Thanks for reading:

I have a scheduled function running in Firebase Cloud Functions. What the code tries to accomplish is

1. checking if a document expired and changing it to "inactive" >> this part works
2. if a document was set to inactive, i want to see if i have any other docs in the firestore database of the same "type". if there is no other document of the same type, then i want to remove that type from my document "types".

In my latest attempt (copied below) I check if there is a document in the snapshot (which would mean that there is another document of the same type, therefore the doc doesnt have to be deleted). Then if res!== true, I would delete the document.

The problem is that for some reason, res is never true.... maybe the "res" promise resolves before the "snapshot" promise?

const functions = require('firebase-functions');
const admin = require('firebase-admin');
admin.initializeApp();

exports.scheduledFunction = functions.pubsub
.schedule('0 23 * * *').timeZone('Europe/Madrid')
.onRun( async (context) => {

    const expiredDocs = await admin.firestore().collection('PROMOTIONS_INFO')
    .where('active','==',true)
    .where('expiration', '<=', new Date())
    .get()
    .then(async (snapshot) => {
        await Promise.all(snapshot.docs.map( async (doc) => {
            doc.ref.update({active: false})
            const type = doc.data().business.type
            const id = doc.data().id

            const exists = await admin.firestore().collection('PROMOTIONS_INFO')
                .where('active','==',true)
                .where('business.type','==', type)
                .where('id', '!=', id)
                .limit(1)
                .get()
                .then((snapshot) => {
                    snapshot.docs.map((doc)=>{return true})
                }).then(async (res) => {
                    res===true ? null : 
                    (await admin.firestore().collection('PROMOTIONS_INFO').doc('types')
                    .update('types', admin.firestore.FieldValue.arrayRemove(type)))
                })
            }))
        });
});

Answer 1

To achieve your desired result, you might want to consider making use of Batched Writes and splitting your code into distinct steps.

One possible set of steps is:

1. Get all expired documents that are still active
2. No expired documents? Log result & end function.
3. For each expired document:
   • Update it to inactive
   • Store it's type to check later
4. For each type to check, check if an active document with that type exists and if it doesn't, store that type to remove later.
5. No types to remove? Log result & end function.
6. Remove all the types that need to be removed.
7. Log result & end function.

In the above steps, step 3 can make use of Batched Writes and step 6 can make use of the arrayRemove() field transform which can remove multiple elements at once to ease the burden on your database.

const functions = require('firebase-functions');
const admin = require('firebase-admin');
admin.initializeApp();

exports.scheduledFunction = functions.pubsub
.schedule('0 23 * * *').timeZone('Europe/Madrid')
.onRun( async (context) => {
    // get instance of Firestore to use below
    const db = admin.firestore();
    
    // this is reused often, so initialize it once.
    const promotionsInfoColRef = db.collection('PROMOTIONS_INFO');

    // find all documents that are active and have expired.
    const expiredDocsQuerySnapshot = await promotionsInfoColRef
        .where('active','==',true)
        .where('expiration', '<=', new Date())
        .get();

    if (expiredDocsQuerySnapshot.empty) {
        // no expired documents, log the result
        console.log(`No documents have expired recently.`);
        return; // done
    } 
    
    // initialize an object to store all the types to be checked
    // this helps ensure each type is checked only once
    const typesToCheckObj = {};
    
    // initialize a batched write to make changes all at once, rather than call out to Firestore multiple times
    // note: batches are limited to 500 read/write operations in a single batch
    const makeDocsInactiveBatch = db.batch();
    
    // for each snapshot, add their type to typesToCheckObj and update them to inactive
    expiredDocsQuerySnapshot.forEach(doc => {
        const type = doc.get("business.type"); // rather than use data(), parse only the property you need.
        typesToCheckObj[type] = true; // add this type to the ones to check
        makeDocsInactiveBatch.update(doc.ref, { active: false }); // add the "update to inactive" operation to the batch
    });
    
    // update database for all the now inactive documents all at once.
    // we update these documents first, so that the type check are done against actual "active" documents.
    await makeDocsInactiveBatch.commit();
    
    // this is a unique array of the types encountered above
    // this can now be used to check each type ONCE, instead of multiple times
    const typesToCheckArray = Object.keys(typesToCheckObj);
    
    // check each type and return types that have no active promotions
    const typesToRemoveArray = (await Promise.all(
        typesToCheckArray.map((type) => {
            return promotionsInfoColRef
                .where('active','==',true)
                .where('business.type','==', type)
                .limit(1)
                .get()
                .then((querySnapshot) => querySnapshot.empty ? type : null) // if empty, include the type for removal
        })
    ))
    .filter((type) => type !== null); // filter out the null values that represent types that don't need removal
    
    // typesToRemoveArray is now a unique list of strings, containing each type that needs to be removed
    
    if (typesToRemoveArray.length == 0) {
        // no types need removing, log the result
        console.log(`Updated ${expiredDocsQuerySnapshot.size} expired documents to "inactive" and none of the ${typesToCheckArray.length} unique types encountered needed to be removed.`);
        return; // done
    }
    
    // get the types document reference
    const typesDocRef = promotionsInfoColRef.doc('types');

    // use the arrayRemove field transform to remove all the given types at once
    await typesDocRef.update({types: admin.firestore.FieldValue.arrayRemove(...typesToRemoveArray) });

    // log the result
    console.log(`Updated ${expiredDocsQuerySnapshot.size} expired documents to "inactive" and ${typesToRemoveArray.length}/${typesToCheckArray.length} unique types encountered needed to be removed.\n\nThe types removed: ${typesToRemoveArray.sort().join(", ")}`);

Note: Error checking is omitted and should be implemented.

Batch Limits

If you ever expect to hit the 500 operations per batch limit, you can add a wrapper around batches so they automatically get split up as required. One possible wrapper is included here:

class MultiBatch {
    constructor(dbRef) {
        this.dbRef = dbRef;
        this.batchOperations = [];
        this.batches = [this.dbRef.batch()];
        this.currentBatch = this.batches[0];
        this.currentBatchOpCount = 0;
        this.committed = false;
    }
    
    /** Used when for basic update operations */
    update(ref, changesObj) {
        if (this.committed) throw new Error('MultiBatch already committed.');
        if (this.currentBatchOpCount + 1 > 500) {
            // operation limit exceeded, start a new batch
            this.currentBatch = this.dbRef.batch();
            this.currentBatchOpCount = 0;
            this.batches.push(this.currentBatch);
        }
        this.currentBatch.update(ref, changesObj);
        this.currentBatchOpCount++;
    }
    
    /** Used when an update contains serverTimestamp, arrayUnion, arrayRemove, increment or decrement (which all need to be counted as 2 operations) */
    transformUpdate(ref, changesObj) {
        if (this.committed) throw new Error('MultiBatch already committed.');
        if (this.currentBatchOpCount + 2 > 500) {
            // operation limit exceeded, start a new batch
            this.currentBatch = this.dbRef.batch();
            this.currentBatchOpCount = 0;
            this.batches.push(this.currentBatch);
        }
        this.currentBatch.update(ref, changesObj);
        this.currentBatchOpCount += 2;
    }
    
    commit() {
        this.committed = true;
        return Promise.all(this.batches.map(batch => batch.commit()));
    }
}

To use this, swap out db.batch() in the original code for new MultiBatch(db). If an update in the batch (like someBatch.update(ref, { ... })) contains a field transform (such as FieldValue.arrayRemove()), make sure to use someMultiBatch.transformUpdate(ref, { ... }) instead so that single update is correctly counted as 2 operations (a read and a write).

Answer 2

snapshot.docs.map((doc)=>{return true}) returns Array like [true, false] not boolean like true.
So .then( async (res) => { res===true ? null : await admin.firestore(... cannot work. and

Maybe you should modify like the following.

.then((snapshot) => 
    snapshot.docs.length > 0 ? null : 
        await admin.firestore(...

Answer 3

I'm assuming in this case that res is undefined and is evaluating as falsey.

Before your .then promise with the res parameter, you have a previous .then promise that returns void:

//...
.then((snapshot) => {
    snapshot.docs.map((doc)=>{return true}) // <--- This is not actually returning a resolved value
}).then(async (res) => {
    res===true ? null : 
    (await admin.firestore().collection('PROMOTIONS_INFO').doc('types')
    .update('types', admin.firestore.FieldValue.arrayRemove(type)))
})
//...

Depending on your intent, you'll need to return a value in this previous promise. It looks like you're creating an array of boolean values that is the length of the number of snapshot.docs that you have, so if you put a return statement simply in the previous .then clause, res would be something like, [true, true, true, true, /* ... */]

//...
.then((snapshot) => {
    return snapshot.docs.map((doc)=>{return true})
}).then(async (res) => {
    res===true ? null : 
    (await admin.firestore().collection('PROMOTIONS_INFO').doc('types')
    .update('types', admin.firestore.FieldValue.arrayRemove(type)))
})
//...