refactor: move NetworkStack to WireNetwork for re-use - WPB-17719 #3257

johnxnguyen · 2025-06-24T11:02:25Z

Issue

In WireAuthentication we defined a NetworkStack that is initialized with a backend environment and is handles the validation of the environment (e.g checking if proxy credentials are required, resolving backend metadata). For multibackend support, we want to do much of the same thing when setting up the user session (also for the share extension and notification extension). This PR moves NetworkStack from WireAuthentication to WireNetwork so that it can be reused. It also updates the implementation so that it can be reused more generally and updates the main app and extensions to use the NetworkStack.

Some changes include:

Split out a WireNetworkInterface target that contains public types so other targets can depend only on types and not all the implementations of WireNetwork-
Delete some dead code (such as the WireNetwork.Assembly)

Testing

TODO

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

Make sure you use the API for UI elements that support large fonts.
All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
New UI elements have Accessibility strings for VoiceOver.

johnxnguyen · 2025-06-24T11:07:18Z

WireAPI/Sources/WireAPI/Use cases/ResolveBackendMetadata/ResolveBackendMetadataUseCase.swift

Moved to WireNetwork

johnxnguyen · 2025-06-24T11:09:29Z

WireAuthentication/Sources/WireAuthentication/Components/DetermineAuthMethodComponent.swift

@@ -55,12 +56,13 @@ class DetermineAuthMethodComponent: Component<DetermineAuthMethodComponentDepend
        email: String?,
        canCreateAccount: Bool,
        didDetectDomainConflict: Bool,
-        backendInfo: BackendInfo


Here and elsewhere you'll see that BackendInfo has been replaced by BackendEnvironment2 (which will be renamed to BackendEnvironment), which will be the sole environment model going forward.

johnxnguyen · 2025-06-24T11:11:28Z

WireAuthentication/Sources/WireAuthentication/WireAuthenticationAssembly.swift

-public typealias WireAuthenticationBackendEnvironment = WireAuthenticationAPI.WireAuthenticationBackendEnvironment
-public typealias BackendEnvironmentType = WireAuthenticationAPI.BackendEnvironmentType
-public typealias BackendConfig = WireAuthenticationAPI.BackendConfig
-public typealias Endpoints = WireAuthenticationAPI.Endpoints
-public typealias ProxySettings = WireAuthenticationAPI.UnresolvedProxySettings
-public typealias TrustData = WireAuthenticationAPI.TrustData
-public typealias BackendMetadata = WireAuthenticationAPI.BackendMetadata


These were all old models that are no longer needed, replaced by BackendEnvironment2

johnxnguyen · 2025-06-24T11:18:29Z

...tication/Sources/WireAuthenticationUI/Views/Login/LoginViaEmail/LoginViaEmailViewModel.swift

    }

-    var proxyServer: String {
-        backendInfo.backendConfig.endpoints.backendURL.absoluteString


This was wrong (it's the backend rest api endpoint), I believe it should be the proxy host (right @KaterinaWire ?)

johnxnguyen · 2025-06-24T11:19:36Z

.../Resources/ReferenceImages/DetermineAuthMethodViewTests/testDynamicTypeVariants.xxxLarge.png

These and the following deleted snapshots don't have a corresponding test, so the snapshots have been deleted.

johnxnguyen · 2025-06-24T11:21:23Z

...eferenceImages/LoginViaEmailViewTests/testDynamicTypeVariantsWithoutCreateAccount.xSmall.png

These snapshot tests changed because I fixed a typo (there were some other tests that needed re-recording for non visible changes)

johnxnguyen · 2025-06-24T11:24:09Z

WireDomain/Sources/WireDomain/Components/ClientSessionComponent.swift

The client session component current holds a lot of the "new stack" for the user session. The changes here are:

we don't inject small pieces like network service, api version, etc...

instead we inject in what is needed which is AuthenticatedRESTAPI (see later in the pull request) to provide the api clients.

johnxnguyen · 2025-06-24T11:25:23Z

WireDomain/Sources/WireDomain/Components/ClientSessionComponent.swift

+    public lazy var accountsAPI: some AccountsAPI = authenticatedRESTAPI.accountsAPI()
+    public lazy var conversationsAPI: some ConversationsAPI = authenticatedRESTAPI.conversationsAPI()


These two are public because they're still needed outside this component. There was some code in UI project that creates api clients to make use cases. For now I made these public, but in the future the UI project shouldn't be concerned with api clients.

johnxnguyen · 2025-06-24T11:25:57Z

WireDomain/Sources/WireDomain/Components/ClientSessionComponent.swift

@@ -316,7 +260,7 @@ public final class ClientSessionComponent {
        store: userLocalStore
    )

-    private lazy var pullSelfUserClientsSync = PullSelfUserClientsSync(
+    public lazy var pullSelfUserClientsSync = PullSelfUserClientsSync(


Similar to the last comment, it's public because there's some other target that is using it.

johnxnguyen · 2025-06-24T11:47:22Z

WireDomain/Sources/WireDomain/Components/UserSessionComponent.swift

-    private let backendEnvironment: WireNetwork.BackendEnvironment
-    private let minTLSVersion: WireNetwork.TLSVersion
-    private let apiVersion: WireNetwork.APIVersion


We not just inject a NetworkStack which encapsulate all the set up.

johnxnguyen · 2025-06-24T11:48:37Z

WireDomain/Sources/WireDomain/Components/UserSessionComponent.swift

+    ) async throws -> ClientSessionComponent {
+        let authenticatedRESTAPI = try await networkStack.authenticatedRESTAPI(
+            userID: selfUserID,
+            clientID: clientID,
+            cookieEncryptionKey: UserDefaults.cookiesKey()
+        )


This needs to be async now, but I think it's only temporary. Deterministic session loading will ensure that the network stack is fully set up (which is async) before we have a user session.

johnxnguyen · 2025-06-24T12:03:11Z

WireDomain/Sources/WireDomain/Notifications/Components/NotificationServiceExtensionFlow.swift

This is the entry point for the new NSE. I've moved some of the dependencies around (e.g up to higher components and passing through to children via Needle).

johnxnguyen · 2025-06-24T12:04:18Z

WireDomain/Sources/WireDomain/Notifications/Components/NotificationServiceExtensionFlow.swift

+
+public extension BackendEnvironment2 {
+
+    init(_ legacyEnvironment: WireTransport.BackendEnvironment) {


All these mappings are temporary until we implement multibackend support and remove the legacy environments).

johnxnguyen · 2025-06-24T12:05:30Z

WireNetwork/Sources/WireNetwork/Assembly.swift

This was dead

johnxnguyen · 2025-06-24T12:06:01Z

WireNetwork/Sources/WireNetwork/Models/Backend/BackendEnvironment2.swift

Moved to WireNetworkInterface

johnxnguyen · 2025-06-24T12:06:13Z

WireNetwork/Sources/WireNetwork/Models/Backend/ProxySettings.swift

Moved to WireNetworkInterface

johnxnguyen · 2025-06-24T12:06:51Z

WireNetwork/Sources/WireNetwork/Network/NetworkStack/NetworkStack+REST.swift

+            clientID: clientID,
+            cookieStorage: cookieStorage,
+            networkService: networkServices.rest,
+            onAuthenticationFailure: {} // TODO: network stack should bubble this up


issue: still TODO

johnxnguyen · 2025-06-24T12:07:12Z

WireNetwork/Sources/WireNetwork/Network/NetworkStack/NetworkStack.swift

+import WireLogging
+import WireNetworkInterface
+
+// TODO: doc


issue: Still TODO

johnxnguyen · 2025-06-24T12:07:41Z

WireNetwork/Sources/WireNetwork/Network/NetworkStack/NetworkStack.swift

+
+}
+
+// TODO: move to network service


issue: still TODO

johnxnguyen · 2025-06-24T12:09:28Z

WireNetwork/Sources/WireNetwork/Network/NetworkStack/REST API/AuthenticatedRESTAPI.swift

+// along with this program. If not, see http://www.gnu.org/licenses/.
+//
+
+public struct AuthenticatedRESTAPI {


This is a factory for all api clients that need an APIService (which requires an AuthenticationManager). It's an async throws operation to create this factory because the api version needs to be resolved.

But once we have it we can create api clients when needed.

johnxnguyen · 2025-06-24T12:10:28Z

WireNetwork/Sources/WireNetwork/Network/NetworkStack/REST API/UnauthenticatedRESTAPI.swift

@@ -16,18 +16,14 @@
 // along with this program. If not, see http://www.gnu.org/licenses/.
 //

-import Foundation
+public struct UnauthenticatedRESTAPI: Sendable {


Similar to AuthenticatedRESTAPI but doesn't need an api service, just a network service...

johnxnguyen · 2025-06-24T12:10:52Z

WireNetwork/Sources/WireNetwork/Network/URLSessionConfigurationFactory.swift

+
+extension ProxySettings {
+
+    func proxyDictionary() -> [AnyHashable: Any] {


Moved from the model definition, it's only needed here.

johnxnguyen · 2025-06-24T12:11:17Z

...ork/Sources/WireNetwork/Use cases/ResolveBackendMetadata/ResolveBackendMetadataUseCase.swift

Moved from WireAuthentication.

johnxnguyen · 2025-06-24T12:13:50Z

wire-ios-share-engine/Sources/SharingSession.swift

+
+private extension BackendEnvironment2 {
+
+    init(_ legacyEnvironment: WireTransport.BackendEnvironment) {


This is some duplication, but we'll remove when the legacy environment is retired when multibackend support is added.

minor suggestion: Add TODO with ticket in order not forget remove it here

johnxnguyen · 2025-06-24T12:15:45Z

wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift

+                            asyncStreamEnabled: selfUserClient.asyncStreamCapable
+                        )
+                    } catch {
+                        fatalError("failed to set up sync agent: \(error)")


issue: intending to address this with deterministic session loading, but maybe need to an intermediate solution. For instance, what happens if there's no network? Will this fatalError?

…rk-wpb-17719 # Conflicts: # WireAuthentication/Tests/WireAuthenticationUITests/Resources/ReferenceImages/PersonalAccountCreationViewTests/testDynamicTypeVariants.xxxLarge.png # WireNetwork/Sources/WireNetwork/Models/Backend/BackendEnvironment2.swift # WireNetwork/Sources/WireNetworkInterface/Models/PinnedKey.swift # wire-ios-sync-engine/Source/SessionManager/SessionFactories.swift # wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift

github-actions · 2025-06-24T14:34:18Z

Test Results

4 files 449 suites 5m 6s ⏱️
3 171 tests 3 136 ✅ 27 💤 8 ❌
3 171 runs 3 138 ✅ 27 💤 6 ❌

For more details on these failures, see this check.

Results for commit ba33885.

datadog-wireapp · 2025-06-24T14:35:59Z

Datadog Report

Branch report: refactor/move-network-stack-to-wire-network-wpb-17719
Commit report: 763e16f
Test service: wire-ios-mono

❌ 6 Failed (0 Known Flaky), 3138 Passed, 27 Skipped, 2m 6.64s Total Time

❌ Failed Tests (6)

This report shows up to 5 failed tests.

test_didRegisterSelfUserClient_withConsumableNotificationsCapabableEnablesSyncV3() - UnitTests.ZMUserSessionTests - Details
Expand for error
```
 Crash: WireSyncEngine Test Host (17682) <external symbol>
```
test_itCreatesMLSClientIfNeeded_AfterQuickSync() - UnitTests.ZMUserSessionTests - Details
Expand for error
```
 Crash: WireSyncEngine Test Host (17739) <external symbol>
```
test_itPerformsPeriodicMLSUpdates_AfterQuickSync() - UnitTests.ZMUserSessionTests - Details
Expand for error
```
 Crash: WireSyncEngine Test Host (17740) <external symbol>
```
testThatItNotfiesTheTransportSessionWhenSelfUserClientIsRegistered() - UnitTests.ZMUserSessionTests - Details
Expand for error
```
 Crash: WireSyncEngine Test Host (17743) <external symbol>
```
testThatIsLoggedInIsFalseAtStartup() - UnitTests.ZMUserSessionTests_Authentication

dmitrysimkin

Great work! Left couple of comments

dmitrysimkin · 2025-06-25T08:51:55Z

WireAuthentication/Sources/WireAuthentication/Components/DetermineAuthMethodComponent.swift

+        let authenticationAPI = try await networkStack
+            .unauthenticatedRESTAPI()
+            .authenticationAPI()


issue: this looks odd, is it really needed unauthenticatedRESTAPI to make authenticationAPI? usually it's either authenticated client or unauthenticated, and you don't need one to create another.

And from client side if authenticated client needed, I'd hide unauthenticated detail to implementation. Not need for client to know that

ah yes the naming is confusing... on one hand we have unauthenticatedRESTAPI as a container of api clients that don't need an authentication cookie or access token... i.e to make unauthenticated requests. One such api client is AuthenticationAPI that is used to login or register.

Probably though this is the only 'unauthenticated' api client so maybe there is no need to have an UnauthenticatedRESTAPI container. On the other hand, the AuthenticatedRESTAPI container has a lot more api clients and needs a user id.

Or perhaps we can find some better names to avoid the confusion. What do you think?

I'm ok if authenticated and unauthenticated share some code and depend on each other (or one on another) in implementation part.
But not for clients, i think clients should not know about that, only thing they should care if it needs authenticated and unauthenticated client. You can hide this detail under some abstraction right?

dmitrysimkin · 2025-06-25T08:54:40Z

WireAuthentication/Sources/WireAuthentication/Components/LoginViaEmailComponent.swift

-        let authenticationAPI = try await networkStack.makeAuthenticationAPI()
+        let authenticationAPI = try await networkStack
+            .unauthenticatedRESTAPI()
+            .authenticationAPI()


suggestion: I like make prefix, it adds clarity what it does which is being factory method. Just unauthenticatedRESTAPI() or authenticationAPI() is unclear to me is it wrapper, adapter or doing some side affect

dmitrysimkin · 2025-06-25T09:20:26Z

wire-ios-share-engine/Sources/SharingSession.swift

+
+private extension BackendEnvironment2 {
+
+    init(_ legacyEnvironment: WireTransport.BackendEnvironment) {


minor suggestion: Add TODO with ticket in order not forget remove it here

johnxnguyen added 7 commits June 19, 2025 12:00

move resolve backend metadata use case to wire api

31e382c

factor out some types to WireNetworkInterface target

aecc343

move network stack to WireNetwork

5539072

fix ui tests

a9e4db8

move network stack to WireNetwork and re-use it

f5e0fe9

fix tests

056a474

clean up

976c578

github-actions bot assigned johnxnguyen Jun 24, 2025

johnxnguyen added the DO NOT MERGE label Jun 24, 2025

johnxnguyen commented Jun 24, 2025

View reviewed changes

WireNetwork/Sources/WireNetwork/Assembly.swift Outdated

Copy link

Collaborator Author

johnxnguyen Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dead

johnxnguyen commented Jun 24, 2025

View reviewed changes

WireNetwork/Sources/WireNetwork/Models/Backend/BackendEnvironment2.swift Outdated

Copy link

Collaborator Author

johnxnguyen Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to WireNetworkInterface

johnxnguyen commented Jun 24, 2025

View reviewed changes

WireNetwork/Sources/WireNetwork/Models/Backend/ProxySettings.swift Outdated

Copy link

Collaborator Author

johnxnguyen Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to WireNetworkInterface

johnxnguyen commented Jun 24, 2025

View reviewed changes

johnxnguyen added 2 commits June 24, 2025 14:47

format

d5c3191

dmitrysimkin reviewed Jun 25, 2025

View reviewed changes

johnxnguyen marked this pull request as draft June 26, 2025 09:00

		public lazy var accountsAPI: some AccountsAPI = authenticatedRESTAPI.accountsAPI()
		public lazy var conversationsAPI: some ConversationsAPI = authenticatedRESTAPI.conversationsAPI()


		public extension BackendEnvironment2 {

		init(_ legacyEnvironment: WireTransport.BackendEnvironment) {


		extension ProxySettings {

		func proxyDictionary() -> [AnyHashable: Any] {


		private extension BackendEnvironment2 {

		init(_ legacyEnvironment: WireTransport.BackendEnvironment) {

refactor: move NetworkStack to WireNetwork for re-use - WPB-17719 #3257

Are you sure you want to change the base?

refactor: move NetworkStack to WireNetwork for re-use - WPB-17719 #3257

Uh oh!

Conversation

johnxnguyen commented Jun 24, 2025

Issue

Testing

Checklist

UI accessibility checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 24, 2025

Test Results

Uh oh!

datadog-wireapp bot commented Jun 24, 2025

Datadog Report

❌ Failed Tests (6)

Uh oh!

dmitrysimkin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment