Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Bug Report: Expired targets not automatically deleted #8159

Open
2 tasks done
ayushpathak-48 opened this issue May 19, 2024 · 4 comments 路 May be fixed by #8239
Open
2 tasks done

馃悰 Bug Report: Expired targets not automatically deleted #8159

ayushpathak-48 opened this issue May 19, 2024 · 4 comments 路 May be fixed by #8239
Assignees
Labels
bug Something isn't working community PR or issues handled by the community members who may need guidance from core product / messaging Fixes and upgrades for the Appwrite Messaging.

Comments

@ayushpathak-48
Copy link

馃憻 Reproduction steps

Go to console

Create FCM provider for push messaging

Create a token of a device and save it into appwrite

Then re create the token and again save it to appwrite

Then send notification to the web target

Then you will get an error for the expired device token

Again after 1 day th same error occurs.

But appwrite says it should be removed after a days if it's expired

馃憤 Expected behavior

It should be removed

馃憥 Actual Behavior

It's giving same errror everyday

馃幉 Appwrite version

Appwrite Cloud

馃捇 Operating system

Windows

馃П Your Environment

I am using node-appwrite to create push target and for creating push notification

馃憖 Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find similar issue

馃彚 Have you read the Code of Conduct?

@ayushpathak-48 ayushpathak-48 added the bug Something isn't working label May 19, 2024
@stnguyen90
Copy link
Contributor

@stnguyen90 stnguyen90 self-assigned this May 20, 2024
@stnguyen90 stnguyen90 added the product / messaging Fixes and upgrades for the Appwrite Messaging. label May 20, 2024
@stnguyen90
Copy link
Contributor

@ayushpathak-48, thanks for raising this issue! 馃檹馃徏 We do mark expired targets as such:

if (($result['error'] ?? '') === 'Expired device token') {
$target = $dbForProject->findOne('targets', [
Query::equal('identifier', [$result['recipient']])
]);
if ($target instanceof Document && !$target->isEmpty()) {
$dbForProject->updateDocument(
'targets',
$target->getId(),
$target->setAttribute('expired', true)
);
}
}

but it seems like the maintenance task that deletes expired targets isn't triggered per project:

Console::loop(function () use ($interval, $cacheRetention, $schedulesDeletionRetention, $usageStatsRetentionHourly, $dbForConsole, $queueForDeletes, $queueForCertificates) {
$time = DateTime::now();
Console::info("[{$time}] Notifying workers with maintenance tasks every {$interval} seconds");
$this->foreachProject($dbForConsole, function (Document $project) use ($queueForDeletes, $usageStatsRetentionHourly) {
$queueForDeletes->setProject($project);
$this->notifyDeleteExecutionLogs($queueForDeletes);
$this->notifyDeleteAbuseLogs($queueForDeletes);
$this->notifyDeleteAuditLogs($queueForDeletes);
$this->notifyDeleteUsageStats($usageStatsRetentionHourly, $queueForDeletes);
$this->notifyDeleteExpiredSessions($queueForDeletes);
});
$this->notifyDeleteConnections($queueForDeletes);
$this->renewCertificates($dbForConsole, $queueForCertificates);
$this->notifyDeleteCache($cacheRetention, $queueForDeletes);
$this->notifyDeleteSchedules($schedulesDeletionRetention, $queueForDeletes);
$this->notifyDeleteTargets($queueForDeletes);
}, $interval, $delay);

so the delete worker doesn't delete the expired targets per project:

private function deleteExpiredTargets(Document $project, callable $getProjectDB): void
{
$this->deleteByGroup(
'targets',
[
Query::equal('expired', [true])
],
$getProjectDB($project),
function (Document $target) use ($getProjectDB, $project) {
$this->deleteTargetSubscribers($project, $getProjectDB, $target);
}
);
}

@stnguyen90 stnguyen90 changed the title 馃悰 Bug Report: Targets not automatically deleted 馃悰 Bug Report: Expired targets not automatically deleted May 20, 2024
@ItzNotABug
Copy link
Contributor

@stnguyen90 is this issue open for assignment?

@stnguyen90
Copy link
Contributor

@ItzNotABug, sure, ya. Assigned!

@stnguyen90 stnguyen90 added the community PR or issues handled by the community members who may need guidance from core label Jun 5, 2024
@ItzNotABug ItzNotABug linked a pull request Jun 6, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community PR or issues handled by the community members who may need guidance from core product / messaging Fixes and upgrades for the Appwrite Messaging.
Projects
Status: In Review
Development

Successfully merging a pull request may close this issue.

3 participants