Page MenuHomePhabricator

[Tunnelbroker] implement creating APNs headers
ClosedPublic

Authored by kamil on Fri, Jun 28, 5:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 29, 12:47 AM
Unknown Object (File)
Sat, Jun 29, 12:47 AM
Unknown Object (File)
Sat, Jun 29, 12:46 AM
Unknown Object (File)
Fri, Jun 28, 7:47 AM

Details

Summary

Implementing building request headers.

Apple docs: Send a POST request to APNs.

Not sure about PushType - perhaps we can just use String but I prefer more detailed type definition. (cc. @marcin).

Depends on D12611

Test Plan

Tested calling this function with different params

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Fri, Jun 28, 6:30 AM
kamil added inline comments.
services/tunnelbroker/src/notifs/apns/headers.rs
32 ↗(On Diff #41790)

that's how headers are typed in the keyserver codebase, cc. @marcin

61 ↗(On Diff #41790)

forgot to make lines 80 chars width - I'll update before landing

bartek added 1 blocking reviewer(s): marcin.

Rust looks ok, letting @marcin review as well, I have no context on some of these

services/tunnelbroker/src/notifs/apns/mod.rs
36–39 ↗(On Diff #41790)

Not sure about your preference, just indicating there's another way of doing this:

impl TryFrom<NotificationHeaders> for HeaderMap {
  type Error = error::Error;
  fn try_from(notif_headers: NotificationHeaders) -> Result<Self, Self::Error> {
    // ...
  }
}

A good alternative if you feel impl APNsClient becomes bloated and difficult to navigate

71 ↗(On Diff #41790)

The push_type_str should be a &'static str so using HeaderValue::from_static() should be possible

This revision is now accepted and ready to land.Tue, Jul 2, 2:04 AM
services/tunnelbroker/src/notifs/apns/mod.rs
36–39 ↗(On Diff #41790)

I like this but this could get messy because of self.token.get_bearer().await? below