Remove global API endpoint

[pinkisemils]

This particular changeset started off with adding a unit test to see why the iOS end-to-end API client is failing. I fixed it and added a test to ensure it doesn't get broken again. Adding a test implied removing some global state, namely the global ApiEndpoint. The test itself is using httpmock and exercising the FFI that the Swift test client is using. I have tested these changes with various Android builds, will try out the app with stagemole later too. I am not entirely certain if removing the global state has made the code any more readable or better, but it certainly has improved the testability.

This then bubbled over into refactoring the daemon parameters just one slight bit - I am not certain it is a change we need, and thus can drop the last commit.

---

This change is 

[linear]

IOS-952 Investigate why `testCreateAccount` started failing on main

[Rawa]

Reviewed 1 of 23 files at r1.
Reviewable status: 1 of 24 files reviewed, 2 unresolved discussions (waiting on @pinkisemils)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

    viewModel { VpnSettingsViewModel(get(), get(), get(), get(), get()) }
    viewModel { WelcomeViewModel(get(), get(), get(), get(), isPlayBuild = IS_PLAY_BUILD) }
    viewModel { ReportProblemViewModel(get(), get(), get<DaemonConfig>().apiEndpointOverride) }

We should inject it directly to the ProblemReporter instead of the ViewModel, it should not have to worry about the apiEndpointOverrides. Or maybe just migrate the problem reporter for the usecase of android? (Could be a separate issue.)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

    viewModel { VpnSettingsViewModel(get(), get(), get(), get(), get()) }
    viewModel { WelcomeViewModel(get(), get(), get(), get(), isPlayBuild = IS_PLAY_BUILD) }
    viewModel { ReportProblemViewModel(get(), get(), get<DaemonConfig>().apiEndpointOverride) }

Right now this override does not take into account the ApiEndpointFromIntentHolder overrides. Lets discuss how we can make it a bit more unified.

[pinkisemils]

Reviewable status: 1 of 24 files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

We should inject it directly to the ProblemReporter instead of the ViewModel, it should not have to worry about the apiEndpointOverrides. Or maybe just migrate the problem reporter for the usecase of android? (Could be a separate issue.)

Definitely not migrating the problem report to grpc here.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Right now this override does not take into account the ApiEndpointFromIntentHolder overrides. Lets discuss how we can make it a bit more unified.

Should it? Do you test problem reports in e2e tests? I'm happy to have a better solution here - I did not author the kotlin parts here. I can hack on it a bit, but this soup of getgetgetgetget is most certainly just rune gibberish to me - that is to say that I welcome any and all help.

[dlon]

Reviewed 14 of 23 files at r1, 4 of 4 files at r3, 2 of 3 files at r4, 1 of 4 files at r5, all commit messages.
Reviewable status: 20 of 26 files reviewed, 3 unresolved discussions (waiting on @pinkisemils and @Rawa)

---

mullvad-api/src/rest.rs line 455 at r3 (raw file):

        }

        println!("rest - {:?}", self.request.uri());

This should be removed

[Rawa]

Reviewable status: 20 of 26 files reviewed, 3 unresolved discussions (waiting on @dlon and @pinkisemils)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

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

Definitely not migrating the problem report to grpc here.

I'm all fine with postponing migration to gRPC to another issue, but we should move it to the problem reporter component.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

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

Should it? Do you test problem reports in e2e tests? I'm happy to have a better solution here - I did not author the kotlin parts here. I can hack on it a bit, but this soup of getgetgetgetget is most certainly just rune gibberish to me - that is to say that I welcome any and all help.

This would be for mockApi tests. No right now we don't the problem reports specifically but if we want to test it later it would be weird that the daemon uses two different API endpoints at the same time. I can help out and make it happen.

[Rawa]

Reviewable status: 18 of 26 files reviewed, 1 unresolved discussion (waiting on @dlon and @pinkisemils)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

I'm all fine with postponing migration to gRPC to another issue, but we should move it to the problem reporter component.

Fixed.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 228 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

This would be for mockApi tests. No right now we don't the problem reports specifically but if we want to test it later it would be weird that the daemon uses two different API endpoints at the same time. I can help out and make it happen.

Fixed.

[Rawa]

⭐ Kotlin parts

Reviewed 14 of 23 files at r1, 2 of 4 files at r3, 1 of 3 files at r4, 4 of 4 files at r5, 1 of 3 files at r6, 1 of 2 files at r7, 4 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

[dlon]

Might be more readable if we get rid of some conditional compilation that's all over the place right now. I kind of think that api-override really only needs to affect the behavior of ApiEndpoint::from_env_vars() (which should probably be replaced by ApiEndpoint::default()).

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[Pururun]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/dataproxy/MullvadProblemReport.kt line 60 at r8 (raw file):

                val apiOverride =
                    if (BuildConfig.DEBUG) {
                        apiEndpointFromIntentHolder.apiEndpointOverride ?: apiEndpointOverride

This will break stagemole and devmole builds. It should be possible to override api on release builds.

[Pururun]

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

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/dataproxy/MullvadProblemReport.kt line 60 at r8 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

This will break stagemole and devmole builds. It should be possible to override api on release builds.

It should be something like this:

intentApiOverride = apiEndpointFromIntentHolder.apiEndpointOverride
if (BuildConfig.DEBUG && intentApiOverride != null) {
                intentApiOverride
            } else {
                apiEndpointOverride
            }

[pinkisemils]

Reviewable status: 24 of 27 files reviewed, 1 unresolved discussion (waiting on @Pururun and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/dataproxy/MullvadProblemReport.kt line 60 at r8 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

It should be something like this:

intentApiOverride = apiEndpointFromIntentHolder.apiEndpointOverride
if (BuildConfig.DEBUG && intentApiOverride != null) {
                intentApiOverride
            } else {
                apiEndpointOverride
            }

Seems to work as expected now :)

[pinkisemils]

Whilst that is a worthwhile change, let's get @faern's approval once he's back.

Reviewable status: 22 of 27 files reviewed, 1 unresolved discussion (waiting on @dlon, @Pururun, and @Rawa)

[Rawa]

Reviewed 3 of 3 files at r9, 2 of 2 files at r10, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)