From d91a09a2990e7a4d93433b274abc7553772cb882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 27 Feb 2025 17:55:00 +0100 Subject: [PATCH] Split updater GUI code from the code checking for release updates The code making the actual requests and checks now lives in the `updater.releases` module. The code should be easier to read and to reason about. Tests have been updated to reflect this. --- dangerzone/gui/__init__.py | 11 +- dangerzone/gui/main_window.py | 2 +- dangerzone/gui/updater.py | 270 +++++---------------------------- dangerzone/settings.py | 1 + dangerzone/updater/errors.py | 6 + dangerzone/updater/releases.py | 191 +++++++++++++++++++++++ dangerzone/util.py | 1 + tests/gui/test_main_window.py | 18 ++- tests/gui/test_updater.py | 129 ++++++++-------- 9 files changed, 320 insertions(+), 309 deletions(-) create mode 100644 dangerzone/updater/releases.py diff --git a/dangerzone/gui/__init__.py b/dangerzone/gui/__init__.py index 5a4c26c..248954c 100644 --- a/dangerzone/gui/__init__.py +++ b/dangerzone/gui/__init__.py @@ -24,6 +24,8 @@ from ..document import Document from ..isolation_provider.container import Container from ..isolation_provider.dummy import Dummy from ..isolation_provider.qubes import Qubes, is_qubes_native_conversion +from ..updater import errors as updater_errors +from ..updater import releases from ..util import get_resource_path, get_version from .logic import DangerzoneGui from .main_window import MainWindow @@ -161,16 +163,15 @@ def gui_main(dummy_conversion: bool, filenames: Optional[List[str]]) -> bool: window.register_update_handler(updater.finished) log.debug("Consulting updater settings before checking for updates") - if updater.should_check_for_updates(): + should_check = updater.should_check_for_updates() + + if should_check: log.debug("Checking for updates") updater.start() else: log.debug("Will not check for updates, based on updater settings") - # Ensure the status of the toggle updates checkbox is updated, after the user is - # prompted to enable updates. - window.toggle_updates_action.setChecked(bool(updater.check)) - + window.toggle_updates_action.setChecked(should_check) if filenames: open_files(filenames) diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index fd86817..a17cb1a 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -26,9 +26,9 @@ else: from .. import errors from ..document import SAFE_EXTENSION, Document from ..isolation_provider.qubes import is_qubes_native_conversion +from ..updater.releases import UpdateReport from ..util import format_exception, get_resource_path, get_version from .logic import Alert, CollapsibleBox, DangerzoneGui, UpdateDialog -from .updater import UpdateReport log = logging.getLogger(__name__) diff --git a/dangerzone/gui/updater.py b/dangerzone/gui/updater.py index 396de21..f4dae0b 100644 --- a/dangerzone/gui/updater.py +++ b/dangerzone/gui/updater.py @@ -1,15 +1,7 @@ -"""A module that contains the logic for checking for updates.""" - -import json import logging -import platform -import sys -import time import typing from typing import Optional -from packaging import version - if typing.TYPE_CHECKING: from PySide2 import QtCore, QtWidgets else: @@ -18,36 +10,33 @@ else: except ImportError: from PySide2 import QtCore, QtWidgets -# XXX implict import for "markdown" module required for Cx_Freeze to build on Windows -# See https://github.com/freedomofpress/dangerzone/issues/501 -import html.parser # noqa: F401 - -import markdown -import requests - -from ..util import get_version +from ..updater import errors, releases from .logic import Alert, DangerzoneGui log = logging.getLogger(__name__) - MSG_CONFIRM_UPDATE_CHECKS = """\ -

Do you want Dangerzone to automatically check for updates?

+

+ Do you want Dangerzone to automatically check for updates and apply them? +

-

If you accept, Dangerzone will check the +

If you accept, Dangerzone will check for updates of the sandbox and apply them +automatically. This will ensure that you always have the latest version of the sandbox, +which is critical for the software to operate securely.

+ +

Sandbox updates may include security patches and bug fixes, but won't include new features.

+ +

Additionally, Dangerzone will check the latest releases page -in github.com on startup. Otherwise it will make no network requests and -won't inform you about new releases.

+in github.com, and inform you about new releases. + +Otherwise it will make no network requests and won't inform you about new releases.

If you prefer another way of getting notified about new releases, we suggest adding to your RSS reader our -Mastodon feed. For more information -about updates, check -this webpage.

+Dangerzone News feed.

""" -UPDATE_CHECK_COOLDOWN_SECS = 60 * 60 * 12 # Check for updates at most every 12 hours. - class UpdateCheckPrompt(Alert): """The prompt that asks the users if they want to enable update checks.""" @@ -55,7 +44,7 @@ class UpdateCheckPrompt(Alert): x_pressed = False def closeEvent(self, event: QtCore.QEvent) -> None: - """Detect when a user has pressed "X" in the title bar. + """Detect when a user has pressed "X" in the title bar (to close the dialog). This function is called when a user clicks on "X" in the title bar. We want to differentiate between the user clicking on "Cancel" and clicking on "X", since @@ -76,72 +65,32 @@ class UpdateCheckPrompt(Alert): return buttons_layout -class UpdateReport: - """A report for an update check.""" - - def __init__( - self, - version: Optional[str] = None, - changelog: Optional[str] = None, - error: Optional[str] = None, - ): - self.version = version - self.changelog = changelog - self.error = error - - def empty(self) -> bool: - return self.version is None and self.changelog is None and self.error is None - - class UpdaterThread(QtCore.QThread): """Check asynchronously for Dangerzone updates. - The Updater class is mainly responsible for the following: - - 1. Asking the user if they want to enable update checks or not. - 2. Determining when it's the right time to check for updates. - 3. Hitting the GitHub releases API and learning about updates. + The Updater class is mainly responsible for + asking the user if they want to enable update checks or not. Since checking for updates is a task that may take some time, we perform it - asynchronously, in a Qt thread. This thread then triggers a signal, and informs - whoever has connected to it. + asynchronously, in a Qt thread. + + When finished, this thread triggers a signal with the results. """ - finished = QtCore.Signal(UpdateReport) - - GH_RELEASE_URL = ( - "https://api.github.com/repos/freedomofpress/dangerzone/releases/latest" - ) - REQ_TIMEOUT = 15 + finished = QtCore.Signal(releases.UpdateReport) def __init__(self, dangerzone: DangerzoneGui): super().__init__() self.dangerzone = dangerzone - ########### - # Helpers for updater settings - # - # These helpers make it easy to retrieve specific updater-related settings, as well - # as save the settings file, only when necessary. - - @property - def check(self) -> Optional[bool]: - return self.dangerzone.settings.get("updater_check") - - @check.setter - def check(self, val: bool) -> None: - self.dangerzone.settings.set("updater_check", val, autosave=True) - def prompt_for_checks(self) -> Optional[bool]: """Ask the user if they want to be informed about Dangerzone updates.""" log.debug("Prompting the user for update checks") - # FIXME: Handle the case where a user clicks on "X", instead of explicitly - # making a choice. We should probably ask them again on the next run. prompt = UpdateCheckPrompt( self.dangerzone, message=MSG_CONFIRM_UPDATE_CHECKS, - ok_text="Check Automatically", - cancel_text="Don't Check", + ok_text="Enable sandbox updates", + cancel_text="Do not make any requests", ) check = prompt.launch() if not check and prompt.x_pressed: @@ -149,167 +98,18 @@ class UpdaterThread(QtCore.QThread): return bool(check) def should_check_for_updates(self) -> bool: - """Determine if we can check for updates based on settings and user prefs. - - Note that this method only checks if the user has expressed an interest for - learning about new updates, and not whether we should actually make an update - check. Those two things are distinct, actually. For example: - - * A user may have expressed that they want to learn about new updates. - * A previous update check may have found out that there's a new version out. - * Thus we will always show to the user the cached info about the new version, - and won't make a new update check. - """ - log.debug("Checking platform type") - # TODO: Disable updates for Homebrew installations. - if platform.system() == "Linux" and not getattr(sys, "dangerzone_dev", False): - log.debug("Running on Linux, disabling updates") - if not self.check: # if not overidden by user - self.check = False - return False - - log.debug("Checking if first run of Dangerzone") - if self.dangerzone.settings.get("updater_last_check") is None: - log.debug("Dangerzone is running for the first time, updates are stalled") - self.dangerzone.settings.set("updater_last_check", 0, autosave=True) - return False - - log.debug("Checking if user has already expressed their preference") - if self.check is None: - log.debug("User has not been asked yet for update checks") - self.check = self.prompt_for_checks() - return bool(self.check) - elif not self.check: - log.debug("User has expressed that they don't want to check for updates") - return False - - return True - - def can_update(self, cur_version: str, latest_version: str) -> bool: - if version.parse(cur_version) == version.parse(latest_version): - return False - elif version.parse(cur_version) > version.parse(latest_version): - # FIXME: This is a sanity check, but we should improve its wording. - raise Exception("Received version is older than the latest version") - else: - return True - - def _get_now_timestamp(self) -> int: - return int(time.time()) - - def _should_postpone_update_check(self) -> bool: - """Consult and update cooldown timer. - - If the previous check happened before the cooldown period expires, do not check - again. - """ - current_time = self._get_now_timestamp() - last_check = self.dangerzone.settings.get("updater_last_check") - if current_time < last_check + UPDATE_CHECK_COOLDOWN_SECS: - log.debug("Cooling down update checks") - return True - else: - return False - - def get_latest_info(self) -> UpdateReport: - """Get the latest release info from GitHub. - - Also, render the changelog from Markdown format to HTML, so that we can show it - to the users. - """ try: - res = requests.get(self.GH_RELEASE_URL, timeout=self.REQ_TIMEOUT) - except Exception as e: - raise RuntimeError( - f"Encountered an exception while checking {self.GH_RELEASE_URL}: {e}" + should_check: Optional[bool] = releases.should_check_for_releases( + self.dangerzone.settings ) - - if res.status_code != 200: - raise RuntimeError( - f"Encountered an HTTP {res.status_code} error while checking" - f" {self.GH_RELEASE_URL}" - ) - - try: - info = res.json() - except json.JSONDecodeError: - raise ValueError(f"Received a non-JSON response from {self.GH_RELEASE_URL}") - - try: - version = info["tag_name"].lstrip("v") - changelog = markdown.markdown(info["body"]) - except KeyError: - raise ValueError( - f"Missing required fields in JSON response from {self.GH_RELEASE_URL}" - ) - - return UpdateReport(version=version, changelog=changelog) - - # XXX: This happens in parallel with other tasks. DO NOT alter global state! - def _check_for_updates(self) -> UpdateReport: - """Check for updates locally and remotely. - - Check for updates in two places: - - 1. In our settings, in case we have cached the latest version/changelog from a - previous run. - 2. In GitHub, by hitting the latest releases API. - """ - log.debug("Checking for Dangerzone updates") - latest_version = self.dangerzone.settings.get("updater_latest_version") - if version.parse(get_version()) < version.parse(latest_version): - log.debug("Determined that there is an update due to cached results") - return UpdateReport( - version=latest_version, - changelog=self.dangerzone.settings.get("updater_latest_changelog"), - ) - - # If the previous check happened before the cooldown period expires, do not - # check again. Else, bump the last check timestamp, before making the actual - # check. This is to ensure that even failed update checks respect the cooldown - # period. - if self._should_postpone_update_check(): - return UpdateReport() - else: - self.dangerzone.settings.set( - "updater_last_check", self._get_now_timestamp(), autosave=True - ) - - log.debug("Checking the latest GitHub release") - report = self.get_latest_info() - log.debug(f"Latest version in GitHub is {report.version}") - if report.version and self.can_update(latest_version, report.version): - log.debug( - f"Determined that there is an update due to a new GitHub version:" - f" {latest_version} < {report.version}" - ) - return report - - log.debug("No need to update") - return UpdateReport() - - ################## - # Logic for running update checks asynchronously - - def check_for_updates(self) -> UpdateReport: - """Check for updates and return a report with the findings: - - There are three scenarios when we check for updates, and each scenario returns a - slightly different answer: - - 1. No new updates: Return an empty update report. - 2. Updates are available: Return an update report with the latest version and - changelog, in HTML format. - 3. Update check failed: Return an update report that holds just the error - message. - """ - try: - res = self._check_for_updates() - except Exception as e: - log.exception("Encountered an error while checking for upgrades") - res = UpdateReport(error=str(e)) - - return res + except errors.NeedUserInput: + should_check = self.prompt_for_checks() + if should_check is not None: + self.dangerzone.settings.set( + "updater_check", should_check, autosave=True + ) + return bool(should_check) def run(self) -> None: - self.finished.emit(self.check_for_updates()) + has_updates = releases.check_for_updates(self.dangerzone.settings) + self.finished.emit(has_updates) diff --git a/dangerzone/settings.py b/dangerzone/settings.py index 0e30896..619b458 100644 --- a/dangerzone/settings.py +++ b/dangerzone/settings.py @@ -1,6 +1,7 @@ import json import logging import os +import platform from pathlib import Path from typing import TYPE_CHECKING, Any, Dict diff --git a/dangerzone/updater/errors.py b/dangerzone/updater/errors.py index 6b75c0e..e7f20b9 100644 --- a/dangerzone/updater/errors.py +++ b/dangerzone/updater/errors.py @@ -56,3 +56,9 @@ class CosignNotInstalledError(SignatureError): class InvalidLogIndex(SignatureError): pass + + +class NeedUserInput(UpdaterError): + """The user has not yet been prompted to know if they want to check for updates.""" + + pass diff --git a/dangerzone/updater/releases.py b/dangerzone/updater/releases.py new file mode 100644 index 0000000..82cb05f --- /dev/null +++ b/dangerzone/updater/releases.py @@ -0,0 +1,191 @@ +import json +import platform +import sys +import time +from typing import Optional + +import markdown +import requests +from packaging import version + +from .. import util +from ..settings import Settings +from . import errors, log + +# Check for updates at most every 12 hours. +UPDATE_CHECK_COOLDOWN_SECS = 60 * 60 * 12 + +GH_RELEASE_URL = ( + "https://api.github.com/repos/freedomofpress/dangerzone/releases/latest" +) +REQ_TIMEOUT = 15 + + +class UpdateReport: + """A report for an update check.""" + + def __init__( + self, + version: Optional[str] = None, + changelog: Optional[str] = None, + error: Optional[str] = None, + ): + self.version = version + self.changelog = changelog + self.error = error + + def empty(self) -> bool: + return self.version is None and self.changelog is None and self.error is None + + +def _get_now_timestamp() -> int: + return int(time.time()) + + +def _should_postpone_update_check(settings) -> bool: + """Consult and update cooldown timer. + + If the previous check happened before the cooldown period expires, do not check + again. + """ + current_time = _get_now_timestamp() + last_check = settings.get("updater_last_check") + if current_time < last_check + UPDATE_CHECK_COOLDOWN_SECS: + log.debug("Cooling down update checks") + return True + else: + return False + + +def ensure_sane_update(cur_version: str, latest_version: str) -> bool: + if version.parse(cur_version) == version.parse(latest_version): + return False + elif version.parse(cur_version) > version.parse(latest_version): + # FIXME: This is a sanity check, but we should improve its wording. + raise Exception("Received version is older than the latest version") + else: + return True + + +def fetch_release_info() -> UpdateReport: + """Get the latest release info from GitHub. + + Also, render the changelog from Markdown format to HTML, so that we can show it + to the users. + """ + try: + res = requests.get(GH_RELEASE_URL, timeout=REQ_TIMEOUT) + except Exception as e: + raise RuntimeError( + f"Encountered an exception while checking {GH_RELEASE_URL}: {e}" + ) + + if res.status_code != 200: + raise RuntimeError( + f"Encountered an HTTP {res.status_code} error while checking" + f" {GH_RELEASE_URL}" + ) + + try: + info = res.json() + except json.JSONDecodeError: + raise ValueError(f"Received a non-JSON response from {GH_RELEASE_URL}") + + try: + version = info["tag_name"].lstrip("v") + changelog = markdown.markdown(info["body"]) + except KeyError: + raise ValueError( + f"Missing required fields in JSON response from {GH_RELEASE_URL}" + ) + + return UpdateReport(version=version, changelog=changelog) + + +def should_check_for_releases(settings: Settings) -> bool: + """Determine if we can check for release updates based on settings and user prefs. + + Note that this method only checks if the user has expressed an interest for + learning about new updates, and not whether we should actually make an update + check. Those two things are distinct, actually. For example: + + * A user may have expressed that they want to learn about new updates. + * A previous update check may have found out that there's a new version out. + * Thus we will always show to the user the cached info about the new version, + and won't make a new update check. + """ + check = settings.get("updater_check") + + log.debug("Checking platform type") + # TODO: Disable updates for Homebrew installations. + if platform.system() == "Linux" and not getattr(sys, "dangerzone_dev", False): + log.debug("Running on Linux, disabling updates") + if not check: # if not overidden by user + settings.set("updater_check", False, autosave=True) + return False + + log.debug("Checking if first run of Dangerzone") + if settings.get("updater_last_check") is None: + log.debug("Dangerzone is running for the first time, updates are stalled") + settings.set("updater_last_check", 0, autosave=True) + return False + + log.debug("Checking if user has already expressed their preference") + if check is None: + log.debug("User has not been asked yet for update checks") + raise errors.NeedUserInput() + elif not check: + log.debug("User has expressed that they don't want to check for updates") + return False + + return True + + +def check_for_updates(settings) -> UpdateReport: + """Check for updates locally and remotely. + + Check for updates (locally and remotely) and return a report with the findings: + + There are three scenarios when we check for updates, and each scenario returns a + slightly different answer: + + 1. No new updates: Return an empty update report. + 2. Updates are available: Return an update report with the latest version and + changelog, in HTML format. + 3. Update check failed: Return an update report that holds just the error + message. + """ + try: + log.debug("Checking for Dangerzone updates") + latest_version = settings.get("updater_latest_version") + if version.parse(util.get_version()) < version.parse(latest_version): + log.debug("Determined that there is an update due to cached results") + return UpdateReport( + version=latest_version, + changelog=settings.get("updater_latest_changelog"), + ) + + # If the previous check happened before the cooldown period expires, do not + # check again. Else, bump the last check timestamp, before making the actual + # check. This is to ensure that even failed update checks respect the cooldown + # period. + if _should_postpone_update_check(settings): + return UpdateReport() + else: + settings.set("updater_last_check", _get_now_timestamp(), autosave=True) + + log.debug("Checking the latest GitHub release") + report = fetch_release_info() + log.debug(f"Latest version in GitHub is {report.version}") + if report.version and ensure_sane_update(latest_version, report.version): + log.debug( + f"Determined that there is an update due to a new GitHub version:" + f" {latest_version} < {report.version}" + ) + return report + + log.debug("No need to update") + return UpdateReport() + except Exception as e: + log.exception("Encountered an error while checking for upgrades") + return UpdateReport(error=str(e)) diff --git a/dangerzone/util.py b/dangerzone/util.py index 6cae643..212652e 100644 --- a/dangerzone/util.py +++ b/dangerzone/util.py @@ -69,6 +69,7 @@ def get_tessdata_dir() -> Path: def get_version() -> str: + """Returns the Dangerzone version string.""" try: with get_resource_path("version.txt").open() as f: version = f.read().strip() diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index e4fc127..75a11d3 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -24,9 +24,10 @@ from dangerzone.gui.main_window import ( QtGui, WaitingWidgetContainer, ) -from dangerzone.gui.updater import UpdateReport, UpdaterThread +from dangerzone.gui.updater import UpdaterThread from dangerzone.isolation_provider.container import Container from dangerzone.isolation_provider.dummy import Dummy +from dangerzone.updater import releases from .test_updater import assert_report_equal, default_updater_settings @@ -147,7 +148,7 @@ def test_no_update( # Check that the callback function gets an empty report. handle_updates_spy.assert_called_once() - assert_report_equal(handle_updates_spy.call_args.args[0], UpdateReport()) + assert_report_equal(handle_updates_spy.call_args.args[0], releases.UpdateReport()) # Check that the menu entries remain exactly the same. menu_actions_after = window.hamburger_button.menu().actions() @@ -171,8 +172,8 @@ def test_update_detected( # Make requests.get().json() return the following dictionary. mock_upstream_info = {"tag_name": "99.9.9", "body": "changelog"} - mocker.patch("dangerzone.gui.updater.requests.get") - requests_mock = updater_module.requests.get + mocker.patch("dangerzone.updater.releases.requests.get") + requests_mock = releases.requests.get requests_mock().status_code = 200 # type: ignore [call-arg] requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] @@ -191,7 +192,8 @@ def test_update_detected( # Check that the callback function gets an update report. handle_updates_spy.assert_called_once() assert_report_equal( - handle_updates_spy.call_args.args[0], UpdateReport("99.9.9", "

changelog

") + handle_updates_spy.call_args.args[0], + releases.UpdateReport("99.9.9", "

changelog

"), ) # Check that the settings have been updated properly. @@ -281,9 +283,9 @@ def test_update_error( qt_updater.dangerzone.settings.set("updater_last_check", 0) qt_updater.dangerzone.settings.set("updater_errors", 0) - # Make requests.get() return an errorthe following dictionary. - mocker.patch("dangerzone.gui.updater.requests.get") - requests_mock = updater_module.requests.get + # Make requests.get() return an error + mocker.patch("dangerzone.updater.releases.requests.get") + requests_mock = releases.requests.get requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] window = MainWindow(qt_updater.dangerzone) diff --git a/tests/gui/test_updater.py b/tests/gui/test_updater.py index 9bf544a..114dac5 100644 --- a/tests/gui/test_updater.py +++ b/tests/gui/test_updater.py @@ -12,7 +12,9 @@ from pytestqt.qtbot import QtBot from dangerzone import settings from dangerzone.gui import updater as updater_module -from dangerzone.gui.updater import UpdateReport, UpdaterThread +from dangerzone.gui.updater import UpdaterThread +from dangerzone.updater import releases +from dangerzone.updater.releases import UpdateReport from dangerzone.util import get_version from ..test_settings import default_settings_0_4_1, save_settings @@ -116,6 +118,7 @@ def test_linux_no_check(updater: UpdaterThread, monkeypatch: MonkeyPatch) -> Non def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: """Test prompting users to ask them if they want to enable update checks.""" + settings = updater.dangerzone.settings # First run # # When Dangerzone runs for the first time, users should not be asked to enable @@ -124,7 +127,7 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: expected_settings["updater_check"] = None expected_settings["updater_last_check"] = 0 assert updater.should_check_for_updates() is False - assert updater.dangerzone.settings.get_updater_settings() == expected_settings + assert settings.get_updater_settings() == expected_settings # Second run # @@ -138,14 +141,14 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: prompt_mock().launch.return_value = False # type: ignore [attr-defined] expected_settings["updater_check"] = False assert updater.should_check_for_updates() is False - assert updater.dangerzone.settings.get_updater_settings() == expected_settings + assert settings.get_updater_settings() == expected_settings # Reset the "updater_check" field and check enabling update checks. - updater.dangerzone.settings.set("updater_check", None) + settings.set("updater_check", None) prompt_mock().launch.return_value = True # type: ignore [attr-defined] expected_settings["updater_check"] = True assert updater.should_check_for_updates() is True - assert updater.dangerzone.settings.get_updater_settings() == expected_settings + assert settings.get_updater_settings() == expected_settings # Third run # @@ -153,7 +156,7 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: # checks. prompt_mock().side_effect = RuntimeError("Should not be called") # type: ignore [attr-defined] for check in [True, False]: - updater.dangerzone.settings.set("updater_check", check) + settings.set("updater_check", check) assert updater.should_check_for_updates() == check @@ -161,43 +164,44 @@ def test_update_checks( updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture ) -> None: """Test version update checks.""" + settings = updater.dangerzone.settings # This dictionary will simulate GitHub's response. mock_upstream_info = {"tag_name": f"v{get_version()}", "body": "changelog"} # Make requests.get().json() return the above dictionary. - mocker.patch("dangerzone.gui.updater.requests.get") - requests_mock = updater_module.requests.get + mocker.patch("dangerzone.updater.releases.requests.get") + requests_mock = updater_module.releases.requests.get requests_mock().status_code = 200 # type: ignore [call-arg] requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] # Always assume that we can perform multiple update checks in a row. - monkeypatch.setattr(updater, "_should_postpone_update_check", lambda: False) + mocker.patch( + "dangerzone.updater.releases._should_postpone_update_check", return_value=False + ) # Test 1 - Check that the current version triggers no updates. - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert_report_equal(report, UpdateReport()) # Test 2 - Check that a newer version triggers updates, and that the changelog is # rendered from Markdown to HTML. mock_upstream_info["tag_name"] = "v99.9.9" - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert_report_equal( report, UpdateReport(version="99.9.9", changelog="

changelog

") ) # Test 3 - Check that HTTP errors are converted to error reports. requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] - report = updater.check_for_updates() - error_msg = ( - f"Encountered an exception while checking {updater.GH_RELEASE_URL}: failed" - ) + report = releases.check_for_updates(settings) + error_msg = f"Encountered an exception while checking {updater_module.releases.GH_RELEASE_URL}: failed" assert_report_equal(report, UpdateReport(error=error_msg)) # Test 4 - Check that cached version/changelog info do not trigger an update check. - updater.dangerzone.settings.set("updater_latest_version", "99.9.9") - updater.dangerzone.settings.set("updater_latest_changelog", "

changelog

") + settings.set("updater_latest_version", "99.9.9") + settings.set("updater_latest_changelog", "

changelog

") - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert_report_equal( report, UpdateReport(version="99.9.9", changelog="

changelog

") ) @@ -205,14 +209,16 @@ def test_update_checks( def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) -> None: """Make sure Dangerzone only checks for updates every X hours""" - updater.dangerzone.settings.set("updater_check", True) - updater.dangerzone.settings.set("updater_last_check", 0) + settings = updater.dangerzone.settings + + settings.set("updater_check", True) + settings.set("updater_last_check", 0) # Mock some functions before the tests start - cooldown_spy = mocker.spy(updater, "_should_postpone_update_check") - timestamp_mock = mocker.patch.object(updater, "_get_now_timestamp") - mocker.patch("dangerzone.gui.updater.requests.get") - requests_mock = updater_module.requests.get + cooldown_spy = mocker.spy(updater_module.releases, "_should_postpone_update_check") + timestamp_mock = mocker.patch.object(updater_module.releases, "_get_now_timestamp") + mocker.patch("dangerzone.updater.releases.requests.get") + requests_mock = updater_module.releases.requests.get # # Make requests.get().json() return the version info that we want. mock_upstream_info = {"tag_name": "99.9.9", "body": "changelog"} @@ -225,9 +231,9 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - curtime = int(time.time()) timestamp_mock.return_value = curtime - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is False - assert updater.dangerzone.settings.get("updater_last_check") == curtime + assert settings.get("updater_last_check") == curtime assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) # Test 2: Advance the current time by 1 second, and ensure that no update will take @@ -236,41 +242,39 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - curtime += 1 timestamp_mock.return_value = curtime requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] - updater.dangerzone.settings.set("updater_latest_version", get_version()) - updater.dangerzone.settings.set("updater_latest_changelog", None) + settings.set("updater_latest_version", get_version()) + settings.set("updater_latest_changelog", None) - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is True - assert updater.dangerzone.settings.get("updater_last_check") == curtime - 1 # type: ignore [unreachable] + assert settings.get("updater_last_check") == curtime - 1 # type: ignore [unreachable] assert_report_equal(report, UpdateReport()) # Test 3: Advance the current time by seconds. Ensure that # Dangerzone checks for updates again, and the last check timestamp gets bumped. - curtime += updater_module.UPDATE_CHECK_COOLDOWN_SECS + curtime += updater_module.releases.UPDATE_CHECK_COOLDOWN_SECS timestamp_mock.return_value = curtime requests_mock.side_effect = None - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is False - assert updater.dangerzone.settings.get("updater_last_check") == curtime + assert settings.get("updater_last_check") == curtime assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) # Test 4: Make Dangerzone check for updates again, but this time, it should # encounter an error while doing so. In that case, the last check timestamp # should be bumped, so that subsequent checks don't take place. - updater.dangerzone.settings.set("updater_latest_version", get_version()) - updater.dangerzone.settings.set("updater_latest_changelog", None) + settings.set("updater_latest_version", get_version()) + settings.set("updater_latest_changelog", None) - curtime += updater_module.UPDATE_CHECK_COOLDOWN_SECS + curtime += updater_module.releases.UPDATE_CHECK_COOLDOWN_SECS timestamp_mock.return_value = curtime requests_mock.side_effect = Exception("failed") - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is False - assert updater.dangerzone.settings.get("updater_last_check") == curtime - error_msg = ( - f"Encountered an exception while checking {updater.GH_RELEASE_URL}: failed" - ) + assert settings.get("updater_last_check") == curtime + error_msg = f"Encountered an exception while checking {updater_module.releases.GH_RELEASE_URL}: failed" assert_report_equal(report, UpdateReport(error=error_msg)) @@ -278,16 +282,17 @@ def test_update_errors( updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture ) -> None: """Test update check errors.""" + settings = updater.dangerzone.settings # Mock requests.get(). - mocker.patch("dangerzone.gui.updater.requests.get") - requests_mock = updater_module.requests.get + mocker.patch("dangerzone.updater.releases.requests.get") + requests_mock = releases.requests.get # Always assume that we can perform multiple update checks in a row. - monkeypatch.setattr(updater, "_should_postpone_update_check", lambda: False) + monkeypatch.setattr(releases, "_should_postpone_update_check", lambda: False) # Test 1 - Check that request exceptions are being detected as errors. requests_mock.side_effect = Exception("bad url") # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None assert "bad url" in report.error assert "Encountered an exception" in report.error @@ -298,7 +303,7 @@ def test_update_errors( requests_mock.return_value = MockResponse500() # type: ignore [attr-defined] requests_mock.side_effect = None # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None assert "Encountered an HTTP 500 error" in report.error @@ -310,7 +315,7 @@ def test_update_errors( return json.loads("bad json") requests_mock.return_value = MockResponseBadJSON() # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None assert "Received a non-JSON response" in report.error @@ -322,7 +327,7 @@ def test_update_errors( return {} requests_mock.return_value = MockResponseEmpty() # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None assert "Missing required fields in JSON" in report.error @@ -334,7 +339,7 @@ def test_update_errors( return {"tag_name": "vbad_version", "body": "changelog"} requests_mock.return_value = MockResponseBadVersion() # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None assert "Invalid version" in report.error @@ -346,7 +351,7 @@ def test_update_errors( return {"tag_name": "v99.9.9", "body": ["bad", "markdown"]} requests_mock.return_value = MockResponseBadMarkdown() # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert report.error is not None # Test 7 - Check that a valid response passes. @@ -357,7 +362,7 @@ def test_update_errors( return {"tag_name": "v99.9.9", "body": "changelog"} requests_mock.return_value = MockResponseValid() # type: ignore [attr-defined] - report = updater.check_for_updates() + report = releases.check_for_updates(settings) assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) @@ -367,24 +372,28 @@ def test_update_check_prompt( ) -> None: """Test that the prompt to enable update checks works properly.""" # Force Dangerzone to check immediately for updates - qt_updater.dangerzone.settings.set("updater_last_check", 0) + settings = qt_updater.dangerzone.settings + settings.set("updater_last_check", 0) # Test 1 - Check that on the second run of Dangerzone, the user is prompted to # choose if they want to enable update checks. def check_button_labels() -> None: dialog = qt_updater.dangerzone.app.activeWindow() - assert dialog.ok_button.text() == "Check Automatically" # type: ignore [attr-defined] - assert dialog.cancel_button.text() == "Don't Check" # type: ignore [attr-defined] + assert dialog.ok_button.text() == "Enable sandbox updates" # type: ignore [attr-defined] + assert dialog.cancel_button.text() == "Do not make any requests" # type: ignore [attr-defined] dialog.ok_button.click() # type: ignore [attr-defined] QtCore.QTimer.singleShot(500, check_button_labels) + mocker.patch( + "dangerzone.updater.releases._should_postpone_update_check", return_value=False + ) res = qt_updater.should_check_for_updates() assert res is True # Test 2 - Check that when the user chooses to enable update checks, we # store that decision in the settings. - qt_updater.check = None + settings.set("updater_check", None, autosave=True) def click_ok() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -394,11 +403,11 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is True - assert qt_updater.check is True + assert settings.get("updater_check") is True # Test 3 - Same as the previous test, but check that clicking on cancel stores the # opposite decision. - qt_updater.check = None # type: ignore [unreachable] + settings.set("updater_check", None) # type: ignore [unreachable] def click_cancel() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -408,11 +417,11 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is False - assert qt_updater.check is False + assert settings.get("updater_check") is False # Test 4 - Same as the previous test, but check that clicking on "X" does not store # any decision. - qt_updater.check = None + settings.set("updater_check", None, autosave=True) def click_x() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -422,4 +431,4 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is False - assert qt_updater.check is None + assert settings.get("updater_check") is None