ably / ably-cocoa

iOS, tvOS and macOS Objective-C and Swift client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
46 stars 24 forks source link

Umbrella issue for the Apple location push related functionality across different repositaries #1763

Open maratal opened 1 year ago

maratal commented 1 year ago

Apple article on how to implement location pushes: https://developer.apple.com/documentation/corelocation/creating_a_location_push_service_extension

ably-cocoa: https://github.com/ably/ably-cocoa/issues/1762

Server:

  1. Handle additional dictionary parameter in POST "/push/deviceRegistrations". a) In addition to "push.recipient.deviceToken", "push.recipient.locationDeviceToken" should be read from client's request and saved. b) Implement sending location push according to https://developer.apple.com/documentation/corelocation/creating_a_location_push_service_extension#3875733 and https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns

Docs:

  1. Update APNs tutorial at https://ably.com/tutorials/ios-location-push-notifications on how to receive and save location push token.

Specs:

  1. Update RSH8i (or create additional) spec point for location pushes. Make a note that it's an iOS and iPadOS only.

┆Issue is synchronized with this Jira Task by Unito

sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3643

maratal commented 1 year ago

https://ably.com/docs/api/rest-api#post-device-registration

curl -X POST https://rest.ably.io/push/deviceRegistrations \
 -u "******"
 -H "Content-Type: application/json" \
 --data \
'{
  "id": "*****",
  "platform": "ios",
  "formFactor": "phone",
  "push": {
    "recipient": {
      "transportType": "apns",
      "deviceToken": "***"
    }
  }
}

I would suggest instead of "transportType" to use "apns" (or "fcm") dictionary. It will help to eliminate potential problems if you need to change something on one platform, and not to mess up with the other.

"push": {
   "recipient": {
      "apns": {
         "deviceTokens": {
            "default": "***",
            "location": "****",
            "voip": "*****",
            "pushtotalk": "******"
         }
      }
    }
  }

Device tokens are different, as well as API used for their obtain. Keys in the "deviceTokens" (plural) are values from Apple's documentation for "apns-push-type" header field: https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns WDYT @lawrence-forooghian

lawrence-forooghian commented 1 year ago

I think the idea of being able to specify a separate token to be used for a specific push type is a sensible one.

It will help to eliminate potential problems if you need to change something on one platform, and not to mess up with the other.

Could you please elaborate on what scenario you're suggesting this helps us avoid, @maratal? A device registration applies to a single device and hence a single platform (note the top-level platform key).

maratal commented 1 year ago

I've meant having keys for different platforms in one dictionary is probably not the best recipe to handle on the server, but someone from a server side team can have a different opinion on it @lawrence-forooghian

Peter-Kubik commented 1 year ago

So just to clarify, for each different type of apns-push-type, the iOS app needs to generate a separate token, where the API to generate this token is different of each type?

Certainly changing the API completely might be quite difficult at the backend, especially because we would need to handle both types for ongoing support.

maratal commented 1 year ago

where the API to generate this token is different of each type

Correct. All those tokens come from different iOS frameworks.

might be quite difficult at the backend

Agree, what about adding just one extra token to speed up process and then think of more neat solution? @Peter-Kubik @lawrence-forooghian

Peter-Kubik commented 1 year ago

@maratal Yeah, I think adding deviceLocationToken should be sufficient for now at least. Just to clarify, for the the location push notifications, our server should send the notification to /3/device/<locationDeviceToken> instead of /3/device/<deviceToken>?

maratal commented 1 year ago

At least it's what Apple says here @Peter-Kubik

maratal commented 1 year ago

@stmoreau @lawrence-forooghian WDYT of adding just one additional token for simplicity?

maratal commented 1 year ago

@Peter-Kubik Some more information on how to handle device tokens here: https://developer.apple.com/documentation/pushkit/supporting_pushkit_notifications_in_your_app#3377584

Because users can run your app on multiple devices, be prepared to handle multiple tokens for each notification type your app supports.

Also see https://developer.apple.com/documentation/pushkit/pkpushtype-swift.struct

Peter-Kubik commented 1 year ago

@maratal I have talked to Paddy, and he said that for now, we should do the bare minimum to support the new feature of the GM app, which will require location push notification, so I think for now just sending the location token is fine.

Also, thanks for the docs!

maratal commented 1 year ago

Would you be able to read this dictionary? Where "deviceToken" is an old token, and "deviceTokens" (plural) - is the new one? You just read it like recipient["deviceTokens"]["location"]. Or it's important to keep it as a simple additional variable?

"push": {
   "recipient": {
       "transportType": "apns",
       "deviceToken": "***",
       "deviceTokens": {
           "default": "***",
           "location": "****"
        }
    }
}

I mean it's two different dictionaries - one without "deviceTokens" is sent by old clients and with "deviceTokens" (but without "deviceToken" (singular)) by the new ones. @Peter-Kubik

Peter-Kubik commented 1 year ago

@maratal I don't think it would be an issue. I am just cautious of the very small difference in the name deviceToken and deviceTokens. Would deviceTokens.default be the same as deviceToken ?

maratal commented 1 year ago

Would deviceTokens.default be the same as deviceToken

Yes, but as I've mentioned above, not in the same request.

Peter-Kubik commented 1 year ago

Ah, I got you. I think if we were to do it this way, we should consider maybe apnsDeviceTokens instead of deviceTokens to make it clearly distinguished from deviceToken. What do you think?

maratal commented 1 year ago

Yeah, it's less confusing this way, will do.