[Feedback] NIOAPNS: NIO-based Apple Push Notification Service

Okay, ByteBuffer refactor is here

We still have this issue with FileSigner where it is using Foundation and blocking reading from Disk.

Should we, change the API to be asynchronous? Do we require the user to load the key into memory before configuration is made?

Does anyone have any suggestions here?

It seems reasonable to ask users to load the key into memory first.

Of course, down the road we should implement signers that do not require the key to be in memory at all (i.e. can use HSMs or Secure Enclaves). For now though, simply only accepting a key from memory is probably fine.

Thanks @kylebrowning for looking into this!

big +1 here. IMHO that has two big advantages:

  1. We don't need to design APIs now for stuff that isn't implemented yet.
  2. We could remove the whole Signer protocol. IMHO adding protocols to APIs in Swift is very often a mistake and if done they need to be extremely carefully designed. One example is right here in the pitch: The API looked reasonable and then it turns out because it was synchronous it could only satisfy one use-case (key in memory) which invalidates the whole idea that we need a protocol in the first place. The fewer public protocols we need, the better.

If, further down the line, there is actual demand for fancier technologies (such as storing keys in HSMs), then we can always add an API for that but at least we will know what people are actually using. For now, let's keep this API narrow and focussed and as Cory says: Asking the users to provide the key at start of day is totally reasonable.

Okay, that's done here

Will update the proposal text now.

1 Like

We made one small change which adds a convenience initializer for initializing keys via a file path.

It’s important to note that this init should only be called upon the start of the app as contentsOfUrl is blocking. I’ve documented that in the file, and here on the proposal.

We’ve also made all JWT implementation internal so that when/if a proper SSS JWT lib comes around we can include that as a dependency and not have to do a semver major bump.

Keep the feedback coming if you’ve got anymore!

Thanks for the awesome work @kylebrowning!

I have an additional feature request. I have custom Encodable types in my notification payload. They need to get some information via Encoder.userInfo in their encode(to:) implementation. I haven't found an option to do this currently. Could we expose this in some way?

We could either pass a userInfo dictionary or a custom JSONEncoder. I don't have a strong preference for either, but lean towards using a userInfo dictionary. The userInfo dictionary / JSONEncoder instance could be part of APNSConfiguration or an additional parameter to APNSConnection.send().

Another thing I noticed: It's currently not possible to set the topic per notification. It needs to be set on the APNSConnection. I think it should be possible to set/overwrite the topic per notification. At least that's how we've done it in node-apn.

I think the topic suggestion is reasonable. As for the additional Encoder stuff. Can you show me an example of one of those.

The biggest problem is that [String: Any] isn't Codable so we've struggled with a way to be able to send that.

There is no need to encode [String: Any]. userInfo is a property on JSONEncoder that Encodable types can access in encode(to:).

My Encodable types encode themselves in differently depending on options set in the userInfo dictionary.

You can use userInfo to provide different encoding variants (e.g. encode Bool as true/false or 1/0), use different kinds of encoding keys or include/exclude certain properties like in the following example:

struct PersonCodingOptions {
    static let userInfoKey = CodingUserInfoKey(rawValue: "com.example.PersonCodingOptions")!
    
    let encodeFullName: Bool
}

struct Person: Encodable {
    let firstName: String
    let lastName: String
    var fullName: String {
        return "\(firstName) \(lastName)"
    }
    
    private enum CodingKeys: String, CodingKey {
        case firstName, lastName, fullName
    }
    
    func encode(to encoder: Encoder) throws {
        let codingOptions = encoder.userInfo[PersonCodingOptions.userInfoKey] as! PersonCodingOptions
        
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(self.firstName, forKey: .firstName)
        try container.encode(self.lastName, forKey: .lastName)
        if codingOptions.encodeFullName {
            try container.encode(self.fullName, forKey: .fullName)
        }
    }
}

let person = Person(firstName: "John", lastName: "Appleseed")

let encoder = JSONEncoder()
encoder.userInfo[PersonCodingOptions.userInfoKey] = PersonCodingOptions(encodeFullName: true)
let json = try! encoder.encode(person)

It's also not possible to set the dateEncodingStrategy, dataEncodingStrategy and keyEncodingStrategy on JSONEncoder. I think these and userInfo should all be exposed in some way.

And one more thing: Could you use Date for the expiration parameter in the send() method?

Sorry for the delayed reply @florianreinhart

Does this work for you?

I opted to just let JSONEncoder be passed in. This allows maximum flexibility for dateEncoding, and doesn't require engineers to adhere to standards we define as userInfo keys. You should be able to do whatever you want with it. @t089 I think this works for you as well!

1 Like

This looks good. Thanks!

Cool, merged.

As a reminder there are two days left for changes to the proposed API at which point, you'll just have to create issues/pr on github like normal engineers and wait for a dot release. :slight_smile:

1 Like

The feedback duration has now closed. SSWG will vote on this proposal at the next meeting (June 13th)

I have tagged 1.0.0-alpha.1 for ease of use.

thank you @kylebrowning

  1. would you like us to lock this thread?

  2. please submit a PR with the proposal language to sswg/proposals at master · swift-server/sswg · GitHub

No problem @tomerd.

  1. Yes please lock.
  2. Its here
1 Like