Function returned undefined, expected Promise or value and unable to delete old data from firebase database using cloud functions

Question

I'm trying to delete multiple nodes on my database that are older than 12hrs. I"m using a pub/sub function to trigger this event. I don't know if my code is actually looping through all nodes as I'm not using the onWrite, onCreate database triggers on specific. Here is the image sample of the database

this is the pub/sub code

 exports.deletejob = functions.pubsub.topic('Oldtask').onPublish(() => {

           deleteOldItem();
})

and the deleteOldItem function

function deleteOldItem(){
const CUT_OFF_TIME =  12 * 60 * 1000; // 12 Hours in milliseconds.
//var ref = admin.database().ref(`/articles/${id}`);
const ref = admin.database().ref(`/articles`);
 const updates = {};
ref.orderByChild('id').limitToLast(100).on('value', function (response) {
    var index = 0;

   response.forEach(function (child) {
    var element = child.val();

     const datetime = element.timestamp;

         const now = Date.now();

         const cutoff = now - datetime;

if (CUT_OFF_TIME < cutoff){

     updates[element.key] = null;
}

  });
//This is supposed to be the returened promise
 return ref.child(response.key).update(updates);

});

If there's something I'm doing wrong, I'll like to know. The pub/sub is triggered with a JobScheduler already setup on google cloud scheduler

Answer 1

You had several problems in your code that were giving you trouble.

• The handling of promises wasn't correct. In particular, your top level function never actually returned a promise, it just called deleteOldItems().
• You should use the promise form of once() instead of calling on() with a callback since you don't want to install a listener in this case, you just need the result a single time, and you want to handle it as part of a promise chain.
• To delete nodes, you should call remove() on a reference to that node. It also generates a promise for you to use here.
• You didn't calculate 12 hours in milliseconds properly, you calculated 12 minutes in milliseconds :)

Here's what I came up with. It uses an http function instead of a pubsub function as well as adding a log statement for my testing, but the modification you need should be trivial/obvious (just change the prototype and remove the response after deleteOldItems, but do make sure you keep returning the result of deleteOldItems()):

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

function deleteOldItems() {
  const CUT_OFF_TIME =  12 * 60 * 60 * 1000; // 12 Hours in milliseconds.
  const ref = admin.database().ref('/articles');
  return ref.orderByChild('id').limitToLast(100).once('value')
    .then((response) => {
      const updatePromises = [];
      const now = Date.now();

      response.forEach((child) => {
        const datetime = child.val().timestamp;
        const cutoff = now - datetime;

        console.log(`processing ${datetime} my cutoff is ${CUT_OFF_TIME} and ${cutoff}`);

        if (CUT_OFF_TIME < cutoff){
          updatePromises.push(child.ref.remove())
        }
      });

      return Promise.all(updatePromises);
    });
}

exports.doIt = functions.https.onRequest((request, response) => {
    return deleteOldItems().then(() => { return response.send('ok') });
}

While I have not tested it, I'm pretty sure this will work to include inside your original function call for cloud scheduler:

exports.deletejob = functions.pubsub.topic('Oldtask').onPublish(() => {
    return deleteOldItems();
})

Of course, this is still more complicated than you need, since ordering by id doesn't really gain you anything here. Instead, why not just use the query to return the earliest items before the cut off time (e.g. exactly the ones you want to remove)? I've also switched to limitToFirst to ensure the earliest entries get thrown out, which seems more natural and ensures fairness:

function deleteOldItems() {
  const cutOffTime =  Date.now() - (12 * 60 * 60 * 1000); // 12 Hours earlier in milliseconds.
  const ref = admin.database().ref('/articles');
  return ref.orderByChild('timestamp').endAt(cutOffTime).limitToFirst(100).once('value')
    .then((response) => {
      const updatePromises = [];

      response.forEach((child) => {
          updatePromises.push(child.ref.remove())
      });

      return Promise.all(updatePromises);
    });
}

If you do this on more than a few items, of course, you probably want to add an index on the timestamp field so the range query is more efficient.