Skip to content

Add Sendable conformance to public push notification models - #764

Open
btctcn wants to merge 1 commit into
mindbox-cloud:developfrom
btctcn:feature/sendable-push-notification-models
Open

Add Sendable conformance to public push notification models#764
btctcn wants to merge 1 commit into
mindbox-cloud:developfrom
btctcn:feature/sendable-push-notification-models

Conversation

@btctcn

@btctcn btctcn commented Aug 27, 2026

Copy link
Copy Markdown

Summary

  • Adds Sendable conformance to MBPushNotification, MBAps, MBApsAlert, and MBPushNotificationButton in both the Mindbox (Mindbox/PublicModels/MBPushNotification.swift) and MindboxNotifications (MindboxNotifications/Models/MBPushNotification.swift) targets.
  • All stored properties on these types are already value types (String?, Int?, or nested structs from the same family), so no @unchecked Sendable or other workaround is needed.

Why

These are public API types that callers may want to pass across actor/concurrency boundaries (e.g. in async notification-handling code). Without an explicit Sendable conformance, clients outside the SDK's own module can't rely on it being safe to send, even though structurally it already is.

Test plan

  • xcodebuild ... -scheme Mindbox -destination 'generic/platform=iOS Simulator' build — succeeds
  • xcodebuild ... -scheme MindboxNotifications -destination 'generic/platform=iOS Simulator' build — succeeds
  • Both schemes rebuilt with SWIFT_STRICT_CONCURRENCY=complete — no new warnings/errors related to these types
  • MindboxNotificationsTests/PushNotificationParsingTests and MindboxNotificationsTests/MindboxPushNotificationTests (25 tests) pass unchanged

Make MBPushNotification, MBAps, MBApsAlert and MBPushNotificationButton
conform to Sendable in both the Mindbox and MindboxNotifications targets.
All stored properties are value types (String?, Int?, or nested Sendable
structs), so the conformance is safe to add without any @unchecked usage.
Verified with a build under SWIFT_STRICT_CONCURRENCY=complete: no new
warnings or errors, and existing push-notification tests still pass.
@justSmK

justSmK commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

What's the rule behind this set of four types — a specific integration that hit it, or the start of a general sweep? The rest of the public API (MBConfiguration, Status, ProtocolError, ...) is still unmarked, so I'd like to know whether to
expect follow-ups and on what principle.

@btctcn

btctcn commented Aug 27, 2026

Copy link
Copy Markdown
Author

Specific integration, not a general sweep. In one of our client apps, the notification-storage layer serializes all reads/writes to the pending-notifications list through a private serial DispatchQueue. Adding a notification means capturing an MBPushNotification value inside that queue's async closure.

That code is compiled into the app's notification-service-extension and content-extension targets, both of which already build under full Swift 6 language mode. There, DispatchQueue.async closures are @Sendable, so capturing a non-Sendable struct is a hard compile error, not a warning.

I didn't extend this to MBConfiguration, Status, ProtocolError, etc. — checked, and none of them are captured across a queue/actor boundary anywhere in that app today. No further follow-up PRs planned on my side.

@justSmK

justSmK commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Specific integration, not a general sweep. In one of our client apps, the notification-storage layer serializes all reads/writes to the pending-notifications list through a private serial DispatchQueue. Adding a notification means capturing an MBPushNotification value inside that queue's async closure.

That code is compiled into the app's notification-service-extension and content-extension targets, both of which already build under full Swift 6 language mode. There, DispatchQueue.async closures are @Sendable, so capturing a non-Sendable struct is a hard compile error, not a warning.

I didn't extend this to MBConfiguration, Status, ProtocolError, etc. — checked, and none of them are captured across a queue/actor boundary anywhere in that app today. No further follow-up PRs planned on my side.

Thanks — we're taking it, but it won't be in a released version before October: release and regression capacity is committed until then, and a public conformance is a minor, not a patch.
Until then @preconcurrency import MindboxNotifications in the affected files should unblock you.

Is the Swift 6 build blocking a dated release on your side? If so, say so and we'll re-weigh the timing.

@btctcn

btctcn commented Aug 27, 2026

Copy link
Copy Markdown
Author

Sounds good — @preconcurrency import MindboxNotifications in the affected files works fine as an interim workaround, and October is no problem on our end. No dated release is blocked on this; we don't have a hard deadline for finishing the Swift 6 migration.

Thanks for the quick turnaround and for taking it on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants