Refactor APNS struct to be a single instance

[mpdifran]

Rather than having a different type for Request and Application, share the same type.

Migration Guide:

Request.APNS -> APNS
Application.APNS -> APNS
application.apns.configuration -> application.apnsConfiguration

[mpdifran]

Rather than computing these properties, pass them into the init, allowing Application and Request to use their own versions.

[mpdifran]

This was a huge pain when I was developing with this library. This change should make it easier to pass APNS around and extend it.

[gwynne]

@mpdifran Unfortunately, this PR would break source compatibility by changing public API with no automatic migration path - in other words, it is a semver-major change. Your change would also make properties inherent to the application-global APNS object (configuration and pool) available on Request, but these are not meaningful in a per-Request context. The existence of these two layers of abstraction expresses via the type system an inherent property of their usage - that per-Request context is not the same as Application-global context. What use case do you have which makes the existing setup so awkward?

[mpdifran]

There should still be a way to configure things to keep those methods from Request, if that's desired.

Yeah I realize it's a breaking change, but IMO I think the change is worth it.

The problems I'm running into is passing APNS into other utility classes. With the separate types, it's impossible to build common flows around APNS. If I want to add an extension to send a silent notification, for example, I'd have to duplicate my code and extend both Application.APNS and Request.APNS, which is not great. The API is simple enough that it should be possible to use a single type for both Application and Request.

Let me try updating my PR to hide the extra methods from Request.

[mpdifran]

Moved this from the APNS object to the root Application.

[mpdifran]

Moved this from the APNS object to the root Application.

[madsodgaard]

Besides the fact that it's a breaking change, I must agree with @gwynne.

I also think that app.apns.configuration is a nicer API (imo)

I think the issues you are having stems from the fact that you're trying to do stuff at the wrong abstraction level. You'll probably want to pass APNSSwiftClient around from and not APNS. If you need common helpers, you should extend APNSSwiftClient or hide the implementation details of this package behind a common interface.

Could show some concrete examples of what you are trying to do, that is causing you issues?

[mpdifran]

Here's the class I've made to help me send a notification to all devices for a user (this is based off of my proposed changes here). You are right though, I can pass around APNSwiftClient instead.

Just seems like a very odd API decision to name these properties the same on both objects when they're actually different types. Definitely tripped me up compared to other properties like database: Database and logger: Logger on both Application and Request.

struct NotificationPusher {
    private let deviceManipulator: DevicesDatabaseManipulator
    private let eventLoop: EventLoop
    private let apns: APNS

    init(request: Request) {
        self.deviceManipulator = DevicesDatabaseManipulator(database: request.db)
        self.eventLoop = request.eventLoop
        self.apns = request.apns
    }

    init(application: Application) {
        self.deviceManipulator = DevicesDatabaseManipulator(database: application.db)
        self.eventLoop = application.eventLoopGroup.next()
        self.apns = application.apns
    }
}

extension NotificationPusher {

    func send(_ alert: APNSwiftAlert, to userIdentifier: UserIdentifier) -> EventLoopFuture<Void> {
        deviceManipulator.devices(for: userIdentifier)
            .flatMapEach(on: eventLoop) { [logger] (device) -> EventLoopFuture<Void> in
                guard let apnsDeviceToken = device.apnsDeviceToken else {
                    return eventLoop.makeSucceededVoidFuture()
                }

                return apns.send(alert, to: apnsDeviceToken.value)
            }
            .flattenToVoid()
    }
}

[mpdifran]

Regardless, I can make due with your workaround, so I'll close this PR. Thanks for taking a look!