Introduce NotificationIdFactory for customizing the default notification IDs
#6072
Conversation
…on ID creation logic.
…on ID creation logic.
NotificationIdFactory for customizing the default notification IDs
SDK Size Comparison 📏
|
WalkthroughAdds a new public functional interface Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant NotificationHandlerFactory
participant MessagingStyleNotificationHandler
participant MessagingStyleNotificationFactory
participant NotificationManager
App->>NotificationHandlerFactory: createNotificationHandler(..., notificationIdFactory)
NotificationHandlerFactory->>MessagingStyleNotificationHandler: instantiate(..., notificationIdFactory)
MessagingStyleNotificationHandler->>MessagingStyleNotificationFactory: buildNotification(push, notificationIdFactory)
MessagingStyleNotificationFactory->>MessagingStyleNotificationFactory: createNotificationId(chatNotification)
alt factory returns non-null
MessagingStyleNotificationFactory->>NotificationManager: notify(notificationId from factory, notification)
else factory null or returns null
MessagingStyleNotificationFactory->>MessagingStyleNotificationFactory: createDefaultNotificationId(...)
MessagingStyleNotificationFactory->>NotificationManager: notify(generatedId, notification)
end
MessagingStyleNotificationHandler->>MessagingStyleNotificationHandler: trackChannelNotificationId(channel, notificationId)
sequenceDiagram
actor System
participant MessagingStyleNotificationHandler
participant NotificationManager
participant Storage
System->>MessagingStyleNotificationHandler: dismissChannelNotifications(channel)
MessagingStyleNotificationHandler->>Storage: getChannelNotificationIds(channel)
MessagingStyleNotificationHandler->>NotificationManager: cancel(notificationId) for each id
MessagingStyleNotificationHandler->>Storage: remove channel key
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationIdFactory.kt (1)
19-49: NotificationIdFactory API and docs look good; consider minor KDoc clarificationsThe fun interface shape and nullable return to trigger default ID generation align well with the intended customization use case, and the example is clear and practical.
As a small improvement, you might extend the KDoc to clarify:
- On which threads
getNotificationIdmay be invoked (main/UI vs background), and- Any expectations around ID stability/uniqueness (e.g., whether callers should assume the SDK may reuse IDs to group notifications by channel, and whether negative/zero values are acceptable).
This would help integrators reason about concurrency and grouping behavior without digging into the implementation.
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.kt (1)
199-268: Comprehensive test coverage for notification ID factory behavior.The three test methods effectively cover:
- Null factory → default ID generation
- Factory returning null → fallback to default
- Custom factory with non-null ID → custom ID is used
Line 267 correctly verifies that the custom notification ID is used for the individual notification. The summary notification uses a separate ID derivation (channel hash), which is intentional design.
Optional: Consider testing edge cases
For additional robustness, consider testing:
- Factory returning negative IDs
- Factory throwing exceptions (if resilience is desired)
These are optional enhancements for more defensive testing.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (1)
153-163: Clean implementation of factory-based ID generation with appropriate defaults.The implementation correctly:
- Uses the factory when provided and non-null
- Falls back to default generation when factory is null or returns null
- Uses
System.nanoTime().toInt()for MessageNew/MessageUpdated (pre-M Android) to ensure each notification gets a unique ID- Uses hash-based IDs for ReactionNew and NotificationReminderDue
Note: This differs from
MessagingStyleNotificationFactorywhich uses hash-based IDs for MessageNew (API 23+). This difference is intentional—pre-M Android lacks MessagingStyle grouping capabilities, so time-based IDs prevent unwanted notification replacement.Optional: Consider extracting common structure
The
createNotificationIdandcreateDefaultNotificationIdmethods share identical structure withMessagingStyleNotificationFactory.kt(lines 77-89). While the default implementations differ intentionally, the factory delegation pattern could potentially be extracted to a shared helper to reduce structural duplication.However, given the different default strategies (time-based vs. hash-based) and that these classes serve different Android API levels, the current approach is acceptable.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt (3)
167-168: Consider consolidating channel notification tracking with global tracking.The per-channel tracking logic (lines 167-183) duplicates patterns from the global tracking methods (
getShownNotifications,trackNotificationId,removeNotificationId). Consider extracting a shared helper that can work with different key prefixes to reduce duplication.♻️ Potential refactor to reduce duplication
private fun trackNotificationIdWithKey(key: String, notificationId: Int) { sharedPreferences.edit { putStringSet( key, (getNotificationIdsForKey(key) + notificationId).map(Int::toString).toSet() ) } } private fun getNotificationIdsForKey(key: String): Set<Int> = sharedPreferences.getStringSet(key, null).orEmpty().mapNotNull { it.toIntOrNull() }.toSet() private fun trackChannelNotificationId(channelType: String, channelId: String, notificationId: Int) { trackNotificationIdWithKey(getChannelNotificationsKey(channelType, channelId), notificationId) } private fun trackNotificationId(notificationId: Int) { trackNotificationIdWithKey(KEY_NOTIFICATIONS_SHOWN, notificationId) }Also applies to: 170-183
197-208: Potential performance concern with iteration over all SharedPreferences keys.The method iterates over all SharedPreferences keys and filters by prefix. If there are many unrelated keys in SharedPreferences, this could be inefficient. However, given that this is only called during
dismissAllNotifications, which is likely infrequent, the performance impact is probably acceptable.
167-216: Consider SharedPreferences cleanup strategy for abandoned channels.Per-channel tracking creates a new SharedPreferences entry for each channel (e.g.,
KEY_CHANNEL_NOTIFICATIONS_messaging:channel123). Over time, if channels are deleted or abandoned, these entries could accumulate. WhiledismissChannelNotificationsclears the entry, it's only called when explicitly dismissing notifications for that channel. Consider documenting the expected cleanup lifecycle or adding periodic cleanup logic for stale entries.Would you like me to generate a cleanup strategy or documentation for managing per-channel SharedPreferences entries?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
CHANGELOG.mdstream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationHandlerFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationIdFactory.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationIdFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationHandlerFactory.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationIdFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationHandlerFactory.kt
**/src/test/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/src/test/**/*.kt: Use backtick test names (for example:funmessage list filters muted channels()) for readability
Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state)
Keep helper extensions private/internal in test files
Files:
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.kt
🧠 Learnings (4)
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
Applied to files:
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use deterministic tests with `runTest` + virtual time for concurrency-sensitive logic (uploads, sync, message state)
Applied to files:
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use backtick test names (for example: `fun `message list filters muted channels`()`) for readability
Applied to files:
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.kt
🧬 Code graph analysis (5)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.kt (2)
createNotificationId(77-78)createDefaultNotificationId(80-89)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (1)
randomString(94-98)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (2)
randomChannel(423-483)randomMessage(302-399)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.kt (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (2)
createDefaultNotificationId(156-163)createNotificationId(153-154)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (1)
removeNotificationId(323-332)
🔇 Additional comments (24)
CHANGELOG.md (1)
19-19: Changelog entry is clear and consistent with existing styleThe wording, placement under
✅ Added, and PR link formatting all match the rest of the changelog; no changes needed.stream-chat-android-client/api/stream-chat-android-client.api (1)
2931-2947: NotificationIdFactory wiring into public API looks consistent and backward compatibleThe additional
createNotificationHandleroverload that takesNotificationIdFactoryand the correspondingNotificationIdFactoryinterface entry correctly expose the new customization point without altering existing signatures. This should preserve compatibility while enabling the new behavior. No issues from an API-surface perspective.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandlerTest.kt (2)
44-44: LGTM!The
eqimport is properly added to support the verification on line 267 usingeq(customNotificationId).
107-107: Correctly updated to reflect the new constructor signature.The test setup properly passes
notificationIdFactory = nullto verify default behavior.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandlerTest.kt (3)
116-116: Correctly updated to reflect the new constructor signature.The test setup properly passes
notificationIdFactory = nullto verify default behavior.
141-158: Well-structured test for multi-notification dismissal.The test now properly verifies that:
- Multiple channel notifications (IDs 100, 200) are tracked per channel
- All tracked notifications are cancelled
- Channel storage is cleaned up via
remove(channelKey)This aligns with the updated implementation supporting custom notification IDs.
161-193: Comprehensive test for global and channel notification dismissal.The test effectively covers:
- Global notification dismissal (ID 1)
- Multiple channel notification dismissal (IDs 10, 20)
- Global storage cleared with
putStringSet(globalKey, emptySet())- Channel storage cleared with
remove(channelKey1)andremove(channelKey2)This provides good coverage of the complete dismissal flow.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/NotificationHandlerFactory.kt (1)
61-67: Excellent integration of the notification ID factory.The changes properly:
- Document the new parameter with clear fallback behavior explanation
- Provide a sensible default (null) for backward compatibility
- Propagate the factory to both handler implementations (MessagingStyleNotificationHandler for API 23+ and ChatNotificationHandler for older versions)
The
@see NotificationIdFactoryreference helps developers discover the new customization option.Also applies to: 86-110
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (2)
58-58: LGTM!The constructor correctly accepts the optional
notificationIdFactoryparameter.
107-107: Excellent centralization of notification ID generation.All notification creation sites now consistently use
createNotificationId(notification), which provides a single point of control for ID generation with proper factory support.Also applies to: 121-121, 133-133, 144-144
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactory.kt (3)
49-50: Clear documentation for the notification ID factory parameter.The KDoc properly explains the fallback behavior when the factory is null or returns null.
Also applies to: 66-66
77-89: Solid implementation of factory-based ID generation with appropriate defaults.The implementation correctly:
- Delegates to the factory when provided and non-null
- Falls back to default hash-based generation
- Uses
channel.type:channel.idhash for MessageNew, enabling proper grouping for MessagingStyle (API 23+)- Applies context-specific hashing for other notification types
The hash-based approach for MessageNew differs intentionally from
ChatNotificationHandler(which usesSystem.nanoTime()for pre-M Android). This design choice supports MessagingStyle's channel-based grouping on newer Android versions.
108-108: Appropriate signature change forrestoreMessagingStyle.Changing from
restoreMessagingStyle(channel: Channel)torestoreMessagingStyle(notification: ChatNotification)makes sense because:
- The notification ID is now computed from the complete
ChatNotificationcontext (not just the channel)- Line 155 correctly uses
createNotificationId(notification)to locate the matching active notificationThis ensures that custom notification IDs are properly used for MessagingStyle restoration.
Also applies to: 119-119, 153-157
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationFactoryTest.kt (4)
74-74: LGTM! Explicit null initialization supports default behavior testing.Setting
notificationIdFactory = nullexplicitly in the test setup makes it clear that the default behavior is being tested in most test cases.
215-228: LGTM! Clear test for default behavior when factory is null.The test properly validates that the default ID generation logic is used when no custom factory is provided.
230-256: LGTM! Proper coverage of fallback behavior.This test correctly validates the fallback to default ID generation when a factory is provided but returns null, ensuring robustness of the factory pattern.
258-284: LGTM! Custom factory override test is well-structured.The test properly validates that custom notification IDs are used when a factory provides non-null values, completing the coverage of all three factory scenarios.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt (7)
57-57: LGTM! Parameter addition enables factory customization.The optional
notificationIdFactoryparameter is correctly added to support custom notification ID generation.
83-83: LGTM! Factory properly wired through to notification factory.
107-111: LGTM! Channel-specific dismissal is correctly implemented.The logic properly dismisses and clears tracked notifications for a specific channel.
113-122: LGTM! Comprehensive cleanup in dismissAllNotifications.The method correctly dismisses both globally tracked notifications and all channel-specific tracked notifications, ensuring complete cleanup.
144-148: LGTM! Renamed method clarifies intent.Renaming from
addNotificationIdtotrackNotificationIdbetter reflects the purpose of the method.
221-221: LGTM! Constant properly defined for per-channel tracking keys.
131-136: Dual tracking behavior with custom notification IDs is intended and documented.The implementation correctly supports custom
notificationIdFactorystrategies. TheNotificationIdFactorydocumentation explicitly shows the intended use case: returning custom IDs for system messages "to avoid default grouping by channel". Each notification ID is independently tracked viatrackChannelNotificationId, which is the designed behavior. Existing tests inChatNotificationHandlerTestandMessagingStyleNotificationFactoryTestverify custom factory usage. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
18-20: Tighten wording to specify “notificationId” / “Android notification IDs” (optional).Current entry is accurate, but “notification IDs” can read a bit generic; consider “notificationId generation” or “Android notification IDs” for clarity/SEO in release notes.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: base-android-ci / Run static checks
- GitHub Check: base-android-ci / Run unit tests
- GitHub Check: base-android-ci / Build
- GitHub Check: Detekt
- GitHub Check: Build / compose apks
|
🎯 Goal
Introduces a new
NotificationIdFactorywhich can be used by customers to override the default notification ID generation logic. This allows customers to adjust the default grouping logic for channel notifications. For example, this can be used to display system messages outside of the standard message grouping.Example usage:
🛠 Implementation details
NotificationIdFactory- applied viaNotificationHandlerFactory.createNotificationHandler🎨 UI Changes
example.mp4
🧪 Testing
Example for system messages
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.