0

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

enter image description here

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

XY-JOE
  • 1,574
  • 1
  • 11
  • 9
  • 1
    You probably want to use the [promise form](https://firebase.google.com/docs/reference/admin/node/admin.database.Query#once) of `once()` instead of `on()` (since you don't want a persistent listener on that key), and then return the whole promise chain at the top level of the function. Notably your top level function doesn't return anything -- it just calls `deleteOldItem()`. – robsiemb Nov 02 '19 at 14:48
  • @robsiemb are you saying i should do something like this `return deleteOldItem()` on the first pub/sub function ? – XY-JOE Nov 02 '19 at 14:55
  • Yes, but also that `deleteOldItem` needs to actually return a promise, which it doesn't currently -- it creates a bunch of callbacks. – robsiemb Nov 02 '19 at 14:56
  • @robsiemb you've seen the code samples alongside the database image sample. Could you drop a sample code showing how the expected return promise is supposed to be – XY-JOE Nov 02 '19 at 14:58
  • added as an answer. – robsiemb Nov 02 '19 at 15:25

1 Answers1

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.

robsiemb
  • 6,157
  • 7
  • 32
  • 46
  • you switched the function to an Http function which is currently giving new issues while setting up the cloud scheduler. These three fields: **Http Method**, **Body** and **Url**. I think the url is the one from the firebase but what of the other two. This is my first time using Http function – XY-JOE Nov 02 '19 at 16:14
  • Like I said, just replace it with your original prototype for the scheduler function, and make sure to return the result of `deleteOldItems()` – robsiemb Nov 02 '19 at 16:19
  • added the code that I expect you need to use, but I haven't tested it. – robsiemb Nov 02 '19 at 16:21