Replace first_run with update check cooldown logic

Wait 12 hours before actually checking again for updates. Setting stored
as last_check. Starts at None. In the first run it changes to 0
(since it was just installed we assume it's the latest version). Having
been None before also allows us to know that it was the first run. After
that, it changes to the time since the last check (counted in unix epoch)

dangerzone/gui/updater.py

@@ -3,6 +3,7 @@
 import logging
 import platform
 import typing
+from datetime import datetime
 from typing import Any, Dict, Optional
 
 if typing.TYPE_CHECKING:
@@ -32,6 +33,8 @@
 If you prefer another way of getting notified about new releases, we suggest adding to your RSS reader our Mastodon feed: https://fosstodon.org/@dangerzone.rss. For more information about updates, check our wiki page: https://github.com/freedomofpress/dangerzone/wiki/Updates
 """
 
+UPDATE_CHECK_PERIOD_SECS = 60 * 60 * 12  # Check for updates at most every 12
+
 
 class Updater(QtCore.QThread):
     """Check asynchronously for Dangerzone updates.
@@ -72,14 +75,6 @@ def _save_me_maybe(self, key: str, val: Any) -> None:
         self.dangerzone.settings.updater_set(key, val)
         self.dangerzone.settings.save()
 
-    @property
-    def first_run(self) -> bool:
-        return self.dangerzone.settings.updater_get("first_run")
-
-    @first_run.setter
-    def first_run(self, val: bool) -> None:
-        self._save_me_maybe("first_run", val)
-
     @property
     def check(self) -> Optional[bool]:
         return self.dangerzone.settings.updater_get("check")
@@ -114,23 +109,28 @@ def should_check_for_updates(self) -> bool:
         #     return False
 
         log.debug("Checking if first run of Dangerzone")
-        if self.first_run:
+        if self.dangerzone.settings.updater_get("last_check") is None:
             log.debug("Dangerzone is running for the first time, updates are stalled")
-            self.first_run = False
+            self.dangerzone.settings.updater_set("last_check", 0)
+            self.dangerzone.settings.save()
             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()
-            # TODO: Should we always check for updates in the third run, or can we check
-            # them immediately, if the user wants to enable them?
             return False
+
         elif not self.check:
             log.debug("User has expressed that they don't want to check for updates")
             return False
 
-        return True
+        current_time = self._get_now_timestamp()
+        last_check = self.dangerzone.settings.updater_get("last_check")
+        if current_time >= last_check + UPDATE_CHECK_PERIOD_SECS:
+            return True
+        else:
+            return False
 
     def can_update(self, cur_version: str, latest_version: str) -> bool:
         if cur_version == latest_version:
@@ -148,6 +148,14 @@ def create_update_report(self, version: str, changelog: str) -> dict:
             "error": None,
         }
 
+    def _get_now_timestamp(self) -> int:
+        return int(datetime.timestamp(datetime.now()))
+
+    def update_checks_cooldown(self) -> None:
+        """Update cooldown timer"""
+        self.dangerzone.settings.updater_set("last_check", self._get_now_timestamp())
+        self.dangerzone.settings.save()
+
     def get_latest_info(self) -> dict:
         """Get the latest release info from GitHub.
 
@@ -163,6 +171,7 @@ def get_latest_info(self) -> dict:
 
         version = info["tag_name"].lstrip("v")
         changelog = markdown.markdown(info["body"])
+        self.update_checks_cooldown()
 
         return self.create_update_report(version, changelog)
 
@@ -185,7 +194,6 @@ def _check_for_updates(self) -> Dict[Any, Any]:
                 self.dangerzone.settings.updater_get("latest_changelog"),
             )
 
-        # TODO: We must have a back-off period (e.g., 12 hours) between each check.
         log.debug(f"Checking the latest GitHub release")
         report = self.get_latest_info()
         log.debug(f"Latest version in GitHub is {report['version']}")

dangerzone/settings.py

@@ -35,11 +35,11 @@ def generate_default_settings(cls) -> Dict[str, Any]:
             "safe_extension": SAFE_EXTENSION,
             "updater": {
                 "check": None,
-                "first_run": True,
                 # FIXME: How to invalidate those if they change upstream?
                 "latest_version": get_version(),
                 "latest_changelog": "",
                 "errors": list(),
+                "last_check": None,  # last check in UNIX epoch (secs since 1970)
             },
         }
 

tests/test_updater.py

@@ -1,6 +1,7 @@
 import json
 import os
 import platform
+from datetime import datetime
 from pathlib import Path
 
 import pytest
@@ -11,7 +12,7 @@
 from dangerzone import settings
 from dangerzone.gui import MainWindow
 from dangerzone.gui import updater as updater_mod
-from dangerzone.gui.updater import Updater
+from dangerzone.gui.updater import UPDATE_CHECK_PERIOD_SECS, Updater
 from dangerzone.util import get_version
 
 from . import generate_isolated_updater, qt_updater, updater
@@ -104,17 +105,6 @@ def test_post_0_4_2_settings(tmp_path: Path, monkeypatch: MonkeyPatch) -> None:
     assert updater.dangerzone.settings.get("updater") == expected_settings
 
 
-# FIXME: This test should not fail
-@pytest.mark.xfail(reason="We haven't enabled this behavior in Linux yet")
-def test_linux_first_run(updater: Updater) -> None:
-    """Check that Linux users are not prompted for update checks."""
-    expected_settings = default_updater_settings()
-    expected_settings["first_run"] = False
-    expected_settings["check"] = False
-    assert updater.should_check_for_updates() == False
-    assert updater.dangerzone.settings.get("updater") == expected_settings
-
-
 # FIXME: Make this check run only on Windows/MacOS platforms.
 def test_user_prompts(
     updater: Updater, monkeypatch: MonkeyPatch, mocker: MockerFixture
@@ -125,8 +115,8 @@ def test_user_prompts(
     # When Dangerzone runs for the first time, users should not be asked to enable
     # updates.
     expected_settings = default_updater_settings()
-    expected_settings["first_run"] = False
     expected_settings["check"] = None
+    expected_settings["last_check"] = 0
     assert updater.should_check_for_updates() == False
     assert updater.dangerzone.settings.get("updater") == expected_settings
 
@@ -205,14 +195,49 @@ def test_update_checks(
     }
 
 
+@pytest.mark.xfail(reason="We haven't enabled this behavior in Linux yet")
+def test_update_checks_cooldown(
+    updater: Updater, monkeypatch: MonkeyPatch, mocker: MockerFixture
+) -> None:
+    """Make sure the dangerzone only checks for updates every X hours"""
+
+    expected_settings = default_updater_settings()
+
+    timestamp_mock = mocker.MagicMock()
+    monkeypatch.setattr(updater, "_get_now_timestamp", timestamp_mock)
+
+    # Accept updates when asked
+    monkeypatch.setattr(updater, "prompt_for_checks", lambda: True)
+
+    # 1st start: last_check starts as None, indicating the 1st run
+    assert expected_settings["last_check"] == None
+    assert updater.should_check_for_updates() == False  # 1st start is already updated
+    timestamp_mock.assert_not_called()  # so it doesn't care about last update checks
+
+    expected_settings["last_check"] = 0  # last_check should be set to 0 in the 1st run
+    assert updater.dangerzone.settings.get("updater") == expected_settings
+
+    # 2nd start: prompts for update checks
+    assert updater.should_check_for_updates() == False
+    timestamp_mock.assert_not_called()  # so it doesn't yet check time information
+
+    # 3rd start: it finally checks but it's still in cooldown
+    timestamp_mock.return_value = 100  # still in cooldown (100 seconds later)
+    assert updater.should_check_for_updates() == False
+    timestamp_mock.assert_called()
+
+    # 4th start:  it should finally check for updates
+    timestamp_mock.return_value = UPDATE_CHECK_PERIOD_SECS  # out of cooldown
+    assert updater.should_check_for_updates() == True
+    timestamp_mock.assert_called()
+
+
 def test_qt(
     qtbot: QtBot, qt_updater: Updater, monkeypatch: MonkeyPatch, mocker: MockerFixture
 ) -> None:
     updater = qt_updater
     updater.dangerzone.settings.updater_set("check", True)
-    updater.dangerzone.settings.updater_set("first_run", False)
     expected_settings = default_updater_settings()
-    expected_settings["first_run"] = False
     expected_settings["check"] = True
 
     mock_upstream_info = {"tag_name": f"v{get_version()}", "body": "changelog"}
@@ -228,6 +253,9 @@ def test_qt(
         updater.start()
 
     assert len(window.hamburger_button.menu().actions()) == 3
+    expected_settings["last_check"] = updater.dangerzone.settings.updater_get(
+        "last_check"
+    )
     assert updater.dangerzone.settings.get("updater") == expected_settings
 
     mock_upstream_info["tag_name"] = "v99.9.9"