Add offline fallback to installer loader

[hulthe]

This change is 

[linear]

DES-2195 Implement on Windows

[MarkusPettersson98]

Reviewed 21 of 21 files at r1, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions

---

installer-downloader/tests/mock.rs line 80 at r1 (raw file):

/// A cache that returns nothing.
pub type FakeAppCacheEmpty = FakeAppCache<false, FakeInstaller<true, true, true>>;

lmao

Code quote:

/// Cache for which all steps immediately succeed
pub type FakeAppCacheHappyPath = FakeAppCache<true, FakeInstaller<true, true, true>>;

/// Cache for which the verification step fails
pub type FakeAppCacheVerifyFail = FakeAppCache<true, FakeInstaller<true, false, false>>;

/// A cache that returns nothing.
pub type FakeAppCacheEmpty = FakeAppCache<false, FakeInstaller<true, true, true>>;

---

mullvad-update/src/client/app.rs line 154 at r1 (raw file):

            // Verification succeeded
            Ok(()) => Ok(InstallerFile::<true> {
                path: self.path, // TODO: remove clones?

We aren't cloning anymore, are we?:)
Comment could be removed

Code quote:

                path: self.path, // TODO: remove clones?

---

installer-downloader/src/controller.rs line 116 at r1 (raw file):

                                .to_owned(),
                            retry_button_text: resource::DOWNLOAD_FAILED_RETRY_BUTTON_TEXT
                                .to_owned(),

Maybe it's a good idea to create two new strings: CREATE_TEMPDIR_FAILED_CANCEL_BUTTON and CREATE_TEMPDIR_FAILED_RETRY_BUTTON. WDYT?

Code quote:

                            cancel_button_text: resource::DOWNLOAD_FAILED_CANCEL_BUTTON_TEXT
                                .to_owned(),
                            retry_button_text: resource::DOWNLOAD_FAILED_RETRY_BUTTON_TEXT
                                .to_owned(),

---

installer-downloader/src/controller.rs line 123 at r1 (raw file):

            };

            let metadata_path = working_dir.directory.join("metadata.json");

The metadata.json filename should be provided METADATA_FILENAME constant defined in the mullvad-update crate

pub const METADATA_FILENAME: &str = "metadata.json";

Code quote:

let metadata_path = working_dir.directory.join("metadata.json");

---

installer-downloader/src/controller.rs line 206 at r1 (raw file):

        let err = match version_provider.get_version_info(&version_params).await {
            Ok(version_info) => {
                //anyhow::anyhow!("test")

Remove before merge

Code quote:

//anyhow::anyhow!("test")

---

installer-downloader/src/controller.rs line 244 at r1 (raw file):

            });

            if let Some((version, cached_app_installer)) = cached_app.take() {

Maybe the comment from above would be suitable here?

// Check if we've already downloaded an istaller.
// If so, the user will be given the option to run it.
if let Some((version, cached_app_installer)) = cached_app.take() { .. }

Code quote:

            if let Some((version, cached_app_installer)) = cached_app.take() {

---

mullvad-update/src/client/version_provider.rs line 13 at r1 (raw file):

    ) -> impl std::future::Future<Output = anyhow::Result<VersionInfo>> + Send;

    fn set_metadata_dump_path(&mut self, path: PathBuf);

Would it be bike shedding trying to move this to some AppCache-esque trait instead? Otherwise, it is fine to leave as is:)

Code quote:

fn set_metadata_dump_path(&mut self, path: PathBuf);

---

mullvad-update/src/client/api.rs line 236 at r1 (raw file):

        let url = format!("{}/version", server.url());

        let temp_dump = TempDir::new().await.unwrap().join("metadata.json");

The metadata.json filename should be provided METADATA_FILENAME constant defined in the mullvad-update crate.

Code quote:

let temp_dump = TempDir::new().await.unwrap().join("metadata.json");

---

installer-downloader/build.rs line 7 at r1 (raw file):

    let target_os = env::var("CARGO_CFG_TARGET_OS").context("Missing 'CARGO_CFG_TARGET_OS")?;
    match target_os.as_str() {
        //"windows" => win_main(),

Don't forget to undo this comment!

Code quote:

//"windows" => win_main(),

[MarkusPettersson98]

Reviewed 1 of 2 files at r4, all commit messages.
Reviewable status: 18 of 21 files reviewed, 5 unresolved discussions (waiting on @hulthe)

[hulthe]

Reviewed 1 of 7 files at r11, 11 of 22 files at r12.
Reviewable status: 3 of 32 files reviewed, 11 unresolved discussions (waiting on @MarkusPettersson98)

---

mullvad-update/src/version.rs line 100 at r12 (raw file):

                // Find installer for the requested architecture (assumed to be unique)
                .find(|installer| params.architecture == installer.architecture)
                // Map each artifact to a [IntermediateVersion]

Comment says IntermediateVersion but looks like the type was changed to a Version

[hulthe]

Reviewed 6 of 22 files at r12, 2 of 4 files at r13, 1 of 1 files at r14, all commit messages.
Reviewable status: 3 of 32 files reviewed, 10 unresolved discussions (waiting on @MarkusPettersson98)

[MarkusPettersson98]

Reviewed 2 of 7 files at r5, 1 of 9 files at r8, 2 of 7 files at r11, 2 of 22 files at r12, 2 of 4 files at r13, all commit messages.
Reviewable status: 12 of 32 files reviewed, 9 unresolved discussions (waiting on @dlon and @hulthe)

[MarkusPettersson98]

I think the PR looks good, except for the suspicious snapshot that @dlon pointed out in installer-downloader/tests/snapshots/controller__cached_app_verify_fail.snap 😊

Reviewed all commit messages.
Reviewable status: 12 of 32 files reviewed, 5 unresolved discussions (waiting on @dlon and @hulthe)

[dlon]

Reviewed all commit messages.
Reviewable status: 12 of 32 files reviewed, all discussions resolved (waiting on @hulthe)

[hulthe]

Reviewed 3 of 21 files at r1, 2 of 7 files at r5, 1 of 9 files at r8, 1 of 7 files at r11, 1 of 1 files at r15, all commit messages.
Reviewable status: 12 of 32 files reviewed, 3 unresolved discussions

---

mullvad-update/src/client/api.rs line 235 at r12 (raw file):

        let temp_dump_dir = TempDir::new().await.unwrap();
        let temp_dump = temp_dump_dir.join("metadata.json");

⛏️ Should "metadata.json" be hard-coded?

---

mullvad-update/src/client/version_provider.rs line 5 at r15 (raw file):

use crate::version::{VersionInfo, VersionParameters};

/// See [module-level](self) docs.

⛏️ There are no module docs

---

mullvad-update/src/client/app.rs line 248 at r13 (raw file):

impl<const VERIFIED: bool> InstallerFile<VERIFIED> {
    fn launch_path(&self) -> PathBuf {

⛏️ Should launch_path and lanch_args be available if the installer isn't verified?

[dlon]

Reviewable status: 12 of 32 files reviewed, 3 unresolved discussions (waiting on @hulthe)

---

mullvad-update/src/client/api.rs line 235 at r12 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Should "metadata.json" be hard-coded?

Ideally no, but local does not compile on linux

---

mullvad-update/src/client/app.rs line 248 at r13 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Should launch_path and lanch_args be available if the installer isn't verified?

Probably should not, if at all avoidable.

[MarkusPettersson98]

Reviewable status: 12 of 32 files reviewed, 3 unresolved discussions (waiting on @dlon and @hulthe)

---

mullvad-update/src/client/api.rs line 235 at r12 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Should "metadata.json" be hard-coded?

Fixed

---

mullvad-update/src/client/app.rs line 248 at r13 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Should launch_path and lanch_args be available if the installer isn't verified?

No, that doesn't make any sense. Nice catch!

[MarkusPettersson98]

Reviewable status: 12 of 32 files reviewed, 3 unresolved discussions (waiting on @dlon and @hulthe)

---

mullvad-update/src/client/api.rs line 235 at r12 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Fixed

Reverted. ugh.

[dlon]

Reviewed all commit messages.
Reviewable status: 12 of 32 files reviewed, 1 unresolved discussion (waiting on @hulthe)