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.