Skip to content

Commit

Permalink
Replace AppDownloaderFactory trait with From impl (#7606)
Browse files Browse the repository at this point in the history
Small refactoring that replaces a custom trait with a `From` implementation.
  • Loading branch information
Serock3 authored and dlon committed Feb 7, 2025
1 parent 5f7c95c commit 5133b1d
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 46 deletions.
40 changes: 14 additions & 26 deletions installer-downloader/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ use crate::delegate::{AppDelegate, AppDelegateQueue};
use crate::resource;
use crate::ui_downloader::{UiAppDownloader, UiAppDownloaderParameters, UiProgressUpdater};

use mullvad_update::api::Version;
use mullvad_update::{
api::{self, VersionInfoProvider},
app::{self, AppDownloaderFactory},
api::{self, Version, VersionInfoProvider},
app::{self, AppDownloader},
};

use std::future::Future;
Expand All @@ -25,8 +24,7 @@ pub struct AppController {}

/// Public entry function for registering a [AppDelegate].
pub fn initialize_controller<T: AppDelegate + 'static>(delegate: &mut T) {
use mullvad_update::api::LatestVersionInfoProvider;
use mullvad_update::app::HttpAppDownloader;
use mullvad_update::{api::LatestVersionInfoProvider, app::HttpAppDownloader};

// App downloader (factory) to use
type DownloaderFactory<T> = HttpAppDownloader<UiProgressUpdater<T>, UiProgressUpdater<T>>;
Expand All @@ -41,12 +39,11 @@ impl AppController {
///
/// Providing the downloader and version info fetcher as type arguments, they're decoupled from
/// the logic of [AppController], allowing them to be mocked.
pub fn initialize<Delegate, DownloaderFactory, VersionProvider>(delegate: &mut Delegate)
pub fn initialize<D, A, V>(delegate: &mut D)
where
Delegate: AppDelegate + 'static,
VersionProvider: VersionInfoProvider + 'static,
DownloaderFactory:
AppDownloaderFactory<Parameters = UiAppDownloaderParameters<Delegate>> + 'static,
D: AppDelegate + 'static,
V: VersionInfoProvider + 'static,
A: From<UiAppDownloaderParameters<D>> + AppDownloader + 'static,
{
delegate.hide_download_progress();
delegate.show_download_button();
Expand All @@ -55,15 +52,9 @@ impl AppController {
delegate.hide_beta_text();

let (task_tx, task_rx) = mpsc::channel(1);
tokio::spawn(handle_action_messages::<Delegate, DownloaderFactory>(
delegate.queue(),
task_rx,
));
tokio::spawn(handle_action_messages::<D, A>(delegate.queue(), task_rx));
delegate.set_status_text(resource::FETCH_VERSION_DESC);
tokio::spawn(fetch_app_version_info::<Delegate, VersionProvider>(
delegate,
task_tx.clone(),
));
tokio::spawn(fetch_app_version_info::<D, V>(delegate, task_tx.clone()));
Self::register_user_action_callbacks(delegate, task_tx);
}

Expand Down Expand Up @@ -107,13 +98,10 @@ where

/// Async worker that handles actions such as initiating a download, cancelling it, and updating
/// labels.
async fn handle_action_messages<Delegate, DownloaderFactory>(
queue: Delegate::Queue,
mut rx: mpsc::Receiver<TaskMessage>,
) where
Delegate: AppDelegate + 'static,
DownloaderFactory:
AppDownloaderFactory<Parameters = UiAppDownloaderParameters<Delegate>> + 'static,
async fn handle_action_messages<D, A>(queue: D::Queue, mut rx: mpsc::Receiver<TaskMessage>)
where
D: AppDelegate + 'static,
A: From<UiAppDownloaderParameters<D>> + AppDownloader + 'static,
{
let mut version_info = None;
let mut active_download = None;
Expand Down Expand Up @@ -158,7 +146,7 @@ async fn handle_action_messages<Delegate, DownloaderFactory>(
self_.enable_cancel_button();
self_.show_download_progress();

let downloader = DownloaderFactory::new_downloader(UiAppDownloaderParameters {
let downloader = A::from(UiAppDownloaderParameters {
signature_url: signature_url.to_owned(),
app_url: app_url.to_owned(),
app_size,
Expand Down
8 changes: 3 additions & 5 deletions installer-downloader/tests/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use installer_downloader::delegate::{AppDelegate, AppDelegateQueue};
use installer_downloader::resource;
use installer_downloader::ui_downloader::UiAppDownloaderParameters;
use mullvad_update::api::{Version, VersionInfo, VersionInfoProvider};
use mullvad_update::app::{AppDownloader, AppDownloaderFactory, DownloadError};
use mullvad_update::app::{AppDownloader, DownloadError};
use mullvad_update::fetch::ProgressUpdater;
use std::sync::{Arc, LazyLock, Mutex};
use std::time::Duration;
Expand Down Expand Up @@ -36,12 +36,10 @@ pub type FakeAppDownloaderFactoryHappyPath = FakeAppDownloader<true, true, true>
/// Downloader for which all but the final verification step succeed
pub type FakeAppDownloaderFactoryVerifyFail = FakeAppDownloader<true, true, false>;

impl<const A: bool, const B: bool, const C: bool> AppDownloaderFactory
impl<const A: bool, const B: bool, const C: bool> From<UiAppDownloaderParameters<FakeAppDelegate>>
for FakeAppDownloader<A, B, C>
{
type Parameters = UiAppDownloaderParameters<FakeAppDelegate>;

fn new_downloader(params: Self::Parameters) -> Self {
fn from(params: UiAppDownloaderParameters<FakeAppDelegate>) -> Self {
FakeAppDownloader { params }
}
}
Expand Down
18 changes: 3 additions & 15 deletions mullvad-update/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ pub trait AppDownloader: Send {
async fn verify(&mut self) -> Result<(), DownloadError>;
}

/// Trait for constructing some [AppDownloader].
pub trait AppDownloaderFactory: AppDownloader {
type Parameters;

/// Instantiate a new [AppDownloader].
fn new_downloader(parameters: Self::Parameters) -> Self;
}

/// Download the app and signature, and verify the app's signature
pub async fn install_and_upgrade(mut downloader: impl AppDownloader) -> Result<(), DownloadError> {
downloader.download_signature().await?;
Expand Down Expand Up @@ -78,15 +70,11 @@ impl<SigProgress, AppProgress> HttpAppDownloader<SigProgress, AppProgress> {
}
}

impl<SigProgress: ProgressUpdater, AppProgress: ProgressUpdater> AppDownloaderFactory
impl<SigProgress: ProgressUpdater, AppProgress: ProgressUpdater>
From<AppDownloaderParameters<SigProgress, AppProgress>>
for HttpAppDownloader<SigProgress, AppProgress>
{
type Parameters = AppDownloaderParameters<SigProgress, AppProgress>;

fn new_downloader(parameters: Self::Parameters) -> Self
where
Self: Sized,
{
fn from(parameters: AppDownloaderParameters<SigProgress, AppProgress>) -> Self {
HttpAppDownloader::new(parameters)
}
}
Expand Down

0 comments on commit 5133b1d

Please sign in to comment.