-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/implement server side push token cleanup #102
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… method return types - Introduce PushNotificationResult class to encapsulate the result of bulk push notification send operations - Update sendNotification and sendBulkNotifications methods to return PushNotificationResult instead of void - These changes allow for better error handling and cleanup of invalid device tokens
… result handling - Refactor sendNotification and sendBulkNotifications to return PushNotificationResult - Enhance _sendBatch to provide detailed success/failure information - Log invalid tokens at info level for cleanup purposes - Reduce log spam by downgrading some log levels - Simplify error handling and reporting
… results - Update sendNotification and sendBulkNotifications to return PushNotificationResult - Implement logic to handle failed notifications within batches - Process OneSignal API response to identify invalid player IDs - Refactor _sendBatch to return detailed result of the send operation - Improve
…ing device tokens This commit improves the performance of the push notification dispatch process by grouping device tokens by user ID before sending notifications. This change avoids iterating through all devices for each user, which reduces the time complexity from O(N^2) to O(N), significantly speeding up the dispatch process.
…ling - Refactor error handling logic for better clarity and consistency - Add detailed comments to explain error handling decisions - Introduce nuanced error handling for different exception types: - Treat NotFoundException as a permanent failure and mark token for cleanup - Log other HTTP errors as severe but do not mark for deletion - Improve log messages with more specific information and context
- Add a new bullet point describing the notification engine's ability to automatically detect and prune invalid device tokens - This feature ensures long-term reliability and efficient notification delivery
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the push notification system by implementing server-side cleanup of invalid device tokens. This new feature ensures that the system remains efficient and reliable by automatically identifying and removing tokens that are no longer valid (e.g., due to app uninstalls), thereby preventing notifications from being sent to non-existent or inactive devices. The changes involve modifying the push notification clients to report detailed send results and integrating a cleanup mechanism into the core notification service. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a robust self-healing mechanism for push notifications by cleaning up invalid device tokens. The changes are well-structured, introducing a PushNotificationResult to propagate failure information from notification clients up to the service layer, which then triggers a cleanup process. The refactoring of FirebasePushNotificationClient and OneSignalPushNotificationClient to return this result is a key part of the implementation. Additionally, a performance improvement has been made in DefaultPushNotificationService by pre-grouping device tokens. My feedback includes suggestions to improve error handling consistency, code conciseness, and performance in the cleanup process.
…deletion process - Use `putIfAbsent` for concise and efficient population of userDeviceTokensMap - Replace sequential device deletion with parallel deletion using `Future.wait` for improved performance
Status
READY
Description
This pull request introduces a significant enhancement to the push notification system by implementing server-side cleanup of invalid device tokens. This new feature ensures that the system remains efficient and reliable by automatically identifying and removing tokens that are no longer valid (e.g., due to app uninstalls), thereby preventing notifications from being sent to non-existent or inactive devices. The changes involve modifying the push notification clients to report detailed send results and integrating a cleanup mechanism into the core notification service.
Type of Change