Transport api requests across app/packet tunnel boundary

[rablador]

Why this needs to be done

To use the new rust client in both processes, we can no longer send URLRequest objects around to execute them in the process we want.

What needs to be done

We should introduce a new packet tunnel command that will execute requests on behalf of the app process. Then, when changing our API proxies, the proxy itself will need to decide if the request is to be executed locally or passed on to packet tunnel.

In short, the packet tunnel should handle a new message type, e.g. enum ApiRequest { Login(…), FetchApiAddresses(…), …

Acceptance criteria

Requests can be proxied from the app to the packet tunnel

Tests

We can enable the e2e test that is able to log out when in the blocked state. If the app is already using the Rust client, then the tests should pass.

---

This change is 

[linear]

IOS-1041 Transport API requests across app/packet tunnel boundary

[rablador]

There will be an amount of function/code duplication while we migrate to mullvad API. This is intentional.

Reviewable status: 0 of 24 files reviewed, all discussions resolved

[pinkisemils]

Reviewed 1 of 23 files at r1, 3 of 19 files at r2.
Reviewable status: 4 of 34 files reviewed, 1 unresolved discussion

---

ios/MullvadREST/APIRequest/APIRequest.swift line 31 at r2 (raw file):

public struct ProxyAPIResponse: Codable, Sendable {
    public let data: Data?

In the relay list case, should the response body and the etag header be encoded into the same bag of bytes?

[rablador]

Reviewable status: 4 of 34 files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

---

ios/MullvadREST/APIRequest/APIRequest.swift line 31 at r2 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

In the relay list case, should the response body and the etag header be encoded into the same bag of bytes?

That would work, as long as we provide a new type that can acommodate for that when data is decoded.

[buggmagnet]

Reviewed 13 of 23 files at r1, 16 of 19 files at r2, 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions

---

ios/MullvadREST/Transport/APITransport.swift line 21 at r4 (raw file):

public final class APITransport: APITransportProtocol {
    public var name: String {
        "app"

nit
Can we give this a more meaningful name ?

---

ios/MullvadREST/Transport/APITransportProvider.swift line 34 at r4 (raw file):

        public func makeTransport() -> APITransportProtocol? {
            return block()

nit
We can omit the return keyword here

[buggmagnet]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador)

[rablador]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)

---

ios/MullvadREST/Transport/APITransport.swift line 21 at r4 (raw file):

Previously, buggmagnet wrote…

nit
Can we give this a more meaningful name ?

Not sure how meaningful it can get, but at least we can call it a transport.

[SteffenErn]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadREST/APIRequest/APIRequestProxy.swift line 42 at r4 (raw file):

        dispatchQueue.async {
            guard let transport = self.transportProvider.makeTransport() else {
                completion(ProxyAPIResponse(data: nil, error: nil))

This is probably an edge case but should the old task be canceled here before completing? Otherwise the old task might still complete afterwards.

[rablador]

Reviewable status: 29 of 34 files reviewed, 2 unresolved discussions (waiting on @buggmagnet and @SteffenErn)

---

ios/MullvadREST/APIRequest/APIRequestProxy.swift line 42 at r4 (raw file):

Previously, SteffenErn (Steffen Ernst) wrote…

This is probably an edge case but should the old task be canceled here before completing? Otherwise the old task might still complete afterwards.

I think you're right. Good catch!

[pinkisemils]

Reviewed 4 of 23 files at r1, 2 of 19 files at r2.
Reviewable status: 29 of 34 files reviewed, 2 unresolved discussions (waiting on @buggmagnet, @rablador, and @SteffenErn)

---

ios/MullvadREST/APIRequest/APIRequest.swift line 31 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

That would work, as long as we provide a new type that can acommodate for that when data is decoded.

That's kind of what we do on desktop.

---

ios/MullvadREST/APIRequest/APIRequestProxy.swift line 42 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

I think you're right. Good catch!

Should these requests not be entirely independent? I'm torn, on one hand, I can't see a use case for sending multiple requests concurrently, but on the other, I feel like it'd be a rather surprising behavior if fetching addresses 5 times in a quick succession would only result in a single successful response due to cancellation.

---

ios/MullvadREST/RetryStrategy/RetryStrategy.swift line 131 at r4 (raw file):

        public var attoseconds: Int64

        public var toDuration: Duration {

toDuration doesn't seem like a great name.

[rablador]

Reviewable status: 29 of 34 files reviewed, 2 unresolved discussions (waiting on @buggmagnet, @pinkisemils, and @SteffenErn)

---

ios/MullvadREST/APIRequest/APIRequest.swift line 31 at r2 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

That's kind of what we do on desktop.

No need for changes in this PR though? The response would still be Data and the responsibility to decode it properly would be the response handler further up the chain.

---

ios/MullvadREST/RetryStrategy/RetryStrategy.swift line 131 at r4 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

toDuration doesn't seem like a great name.

It's the most appropiate one though, no? We could call it something like toLegacyDuration or toNonCodableDuration or something if that's better.

---

ios/MullvadREST/APIRequest/APIRequestProxy.swift line 42 at r4 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

Should these requests not be entirely independent? I'm torn, on one hand, I can't see a use case for sending multiple requests concurrently, but on the other, I feel like it'd be a rather surprising behavior if fetching addresses 5 times in a quick succession would only result in a single successful response due to cancellation.

We can still send 5 requests in a quick succession, but not the exact same one, unless we messed up somewhere. The cancellation only happens happens on identical requests, where we managed to send the exact same request multiple time, ie with the same UUID.

[pinkisemils]

Reviewed 1 of 5 files at r5.
Reviewable status: 30 of 34 files reviewed, 1 unresolved discussion (waiting on @buggmagnet)

---

ios/MullvadREST/APIRequest/APIRequest.swift line 31 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

No need for changes in this PR though? The response would still be Data and the responsibility to decode it properly would be the response handler further up the chain.

I think we can do something special for the relay list or we can encode it in a different body. I think we can move on for now.

---

ios/MullvadREST/RetryStrategy/RetryStrategy.swift line 131 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

It's the most appropiate one though, no? We could call it something like toLegacyDuration or toNonCodableDuration or something if that's better.

If it is a variable, I'd just call it duration, if it's a function, I'd call it a toDuration(), but this is just a nit.