Fix broadcast "list all channels" to use /all-channels endpoint#240
Merged
Conversation
The `.listAll` broadcast operation was targeting `/1/apps/{bundleID}/channels`,
but Apple exposes "read all channels" on a distinct endpoint:
`GET /1/apps/{bundleID}/all-channels`. Against real APNs the old path is not the
list endpoint, so `readAllChannelIDs()` would not return the channel list.
- Route `.listAll` to `/all-channels` in APNSBroadcastRequest.
- Update APNSTestServer to serve `/all-channels` for listing and to require the
`apns-channel-id` header on `GET /channels` (so it can no longer silently mirror
the old, incorrect behaviour and mask a regression).
- Add `testOperationPaths` pinning the path contract for every operation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
.listAllbroadcast operation targeted/1/apps/{bundleID}/channels, but Apple's channel-management API exposes "read all channels" on a distinct endpoint:GET /1/apps/{bundleID}/all-channels. Against real APNs,readAllChannelIDs()was hitting the wrong path and would not return the channel list.This was masked by the mock
APNSTestServer, which treated anyGET …/channelswithout a channel-id header as "list" — mirroring the client's incorrect assumption, so the existing test passed.Ref: Sending channel management requests to APNs.
Changes
.listAllto/all-channelsinAPNSBroadcastRequest.Operation.path.APNSTestServer:GET /1/apps/{bundleID}/all-channelsfor listingapns-channel-idheader onGET /channelsso it can no longer silently mirror the old behaviour (a regression would now fail the test).testOperationPathspinning the path contract for create/read/delete/listAll.Testing
swift test --filter APNSBroadcastClientTests— 10 tests pass, including the existingtestListAllChannels/testListAllChannels_emptynow exercising/all-channels.