diff --git a/aw_core/dirs.py b/aw_core/dirs.py index e79270d..2527d7f 100644 --- a/aw_core/dirs.py +++ b/aw_core/dirs.py @@ -7,6 +7,105 @@ GetDirFunc = Callable[[Optional[str]], str] +_DEFAULT_APPNAME = "activitywatch" +_TESTING_PROFILE = "testing" +_TESTING_APPNAME = f"{_DEFAULT_APPNAME}-{_TESTING_PROFILE}" + +# Filenames that mark a machine as still using the pre-profile shared-root +# testing layout (ActivityWatch/activitywatch#1399). Keep this list specific: +# a false positive would pin a fresh install to the legacy layout forever. +_LEGACY_TESTING_FILENAME_MARKERS = ( + "peewee-sqlite-testing", + "sqlite-testing", + "settings-testing", + "config-testing", + "-testing.db", + "-testing.toml", + "-testing.json", + "_testing_", +) + + +def get_profile() -> str: + """Return the active ``AW_PROFILE`` value, or ``""`` for the default profile.""" + return os.environ.get("AW_PROFILE", "") or "" + + +def _new_testing_root_exists() -> bool: + """True if any platform dir for ``activitywatch-testing`` already exists. + + Must not create directories: existence is the signal that a previous run + already adopted the isolated testing root. + """ + for getter in ( + platformdirs.user_data_dir, + platformdirs.user_config_dir, + platformdirs.user_cache_dir, + ): + if os.path.isdir(getter(_TESTING_APPNAME)): + return True + return False + + +def _is_legacy_testing_filename(name: str) -> bool: + lower = name.lower() + return any(marker in lower for marker in _LEGACY_TESTING_FILENAME_MARKERS) + + +def _legacy_testing_artifacts_exist() -> bool: + """True if testing data still lives under the shared ``activitywatch`` root.""" + roots = ( + platformdirs.user_data_dir(_DEFAULT_APPNAME), + platformdirs.user_config_dir(_DEFAULT_APPNAME), + platformdirs.user_cache_dir(_DEFAULT_APPNAME), + ) + for root in roots: + if not os.path.isdir(root): + continue + for dirpath, dirnames, filenames in os.walk(root): + for filename in filenames: + if _is_legacy_testing_filename(filename): + return True + # Descend one level (activitywatch/aw-server/...) not further. + if os.path.relpath(dirpath, root) != ".": + dirnames.clear() + return False + + +def using_legacy_testing_root() -> bool: + """Whether ``AW_PROFILE=testing`` should stay on the shared ``activitywatch`` root. + + Resolution rule (ActivityWatch/activitywatch#1399), identical on python and rust: + + 1. If ``activitywatch-testing/`` already exists: use it (new layout). + 2. Else if legacy testing artifacts exist in the bare ``activitywatch/`` + root: stay in legacy mode (old paths, old filenames). + 3. Else (fresh setup): create and use ``activitywatch-testing/``. + """ + if get_profile() != _TESTING_PROFILE: + return False + if _new_testing_root_exists(): + return False + return _legacy_testing_artifacts_exist() + + +def legacy_testing_suffix(testing: bool) -> str: + """Return ``"-testing"`` only when testing data shares the default root. + + Isolated profile roots (including new-style ``activitywatch-testing/``) use + bare filenames: the directory already isolates. Suffixed names remain only + in legacy mode so existing ``peewee-sqlite-testing.v2.db`` files keep working. + """ + if not testing: + return "" + profile = get_profile() + if profile == _TESTING_PROFILE and not using_legacy_testing_root(): + return "" + if profile and profile != _TESTING_PROFILE: + # Named isolated profile — directory already isolates. + return "" + return "-testing" + def _get_appname() -> str: """Return the platformdirs appname, optionally suffixed by the active profile. @@ -17,14 +116,22 @@ def _get_appname() -> str: default profile. An unset or empty ``AW_PROFILE`` returns the bare ``"activitywatch"`` name, which is identical to the pre-profile behaviour. + The ``testing`` profile is special: see :func:`using_legacy_testing_root`. + Existing testing data in the shared root is not orphaned; fresh setups and + machines that already have ``activitywatch-testing/`` use the isolated root. + This is the single authoritative place where profile isolation is applied. Every module that uses :func:`get_data_dir`, :func:`get_config_dir`, :func:`get_cache_dir` or :func:`get_log_dir` automatically inherits the correct root for the running profile; no ``profile=`` parameter needs to be threaded through the call chain. """ - profile = os.environ.get("AW_PROFILE", "") - return f"activitywatch-{profile}" if profile else "activitywatch" + profile = get_profile() + if not profile: + return _DEFAULT_APPNAME + if profile == _TESTING_PROFILE and using_legacy_testing_root(): + return _DEFAULT_APPNAME + return f"{_DEFAULT_APPNAME}-{profile}" def ensure_path_exists(path: str) -> None: diff --git a/aw_core/log.py b/aw_core/log.py index e0787e0..a0fd551 100644 --- a/aw_core/log.py +++ b/aw_core/log.py @@ -63,12 +63,16 @@ def _get_latest_log_files(name, testing=False) -> List[str]: # pragma: no cover """ log_dir = dirs.get_log_dir(name) files = filter(lambda filename: name in filename, os.listdir(log_dir)) - files = filter( - lambda filename: "testing" in filename - if testing - else "testing" not in filename, - files, - ) + # Isolated testing roots already separate logs by directory; the + # filename filter is only needed in the shared-root legacy layout. + isolated_testing = testing and not dirs.legacy_testing_suffix(testing) + if not isolated_testing: + files = filter( + lambda filename: ( + "testing" in filename if testing else "testing" not in filename + ), + files, + ) return [os.path.join(log_dir, filename) for filename in sorted(files, reverse=True)] @@ -100,7 +104,10 @@ def _create_file_handler( # $LOG_DIR/aw-server_testing_2017-01-05T00:21:39.log file_ext = ".log.json" if log_json else ".log" now_str = str(datetime.now().replace(microsecond=0).isoformat()).replace(":", "-") - log_name = name + "_" + ("testing_" if testing else "") + now_str + file_ext + testing_prefix = ( + "testing_" if testing and dirs.legacy_testing_suffix(testing) else "" + ) + log_name = name + "_" + testing_prefix + now_str + file_ext log_file_path = os.path.join(log_dir, log_name) # Create rotating logfile handler, max 10MB per file, 3 files max diff --git a/aw_datastore/migration.py b/aw_datastore/migration.py index 29045af..887b7b4 100644 --- a/aw_datastore/migration.py +++ b/aw_datastore/migration.py @@ -2,7 +2,7 @@ import os from typing import List, Optional -from aw_core.dirs import get_data_dir +from aw_core.dirs import get_data_dir, legacy_testing_suffix from .storages import AbstractStorage @@ -31,7 +31,7 @@ def check_for_migration(datastore: AbstractStorage): if datastore.sid == "sqlite": peewee_type = "peewee-sqlite" - peewee_name = peewee_type + ("-testing" if datastore.testing else "") + peewee_name = peewee_type + legacy_testing_suffix(datastore.testing) # Migrate from peewee v2 peewee_db_v2 = detect_db_files(data_dir, peewee_name, 2) if len(peewee_db_v2) > 0: diff --git a/aw_datastore/storages/peewee.py b/aw_datastore/storages/peewee.py index b003cc9..d114b9c 100644 --- a/aw_datastore/storages/peewee.py +++ b/aw_datastore/storages/peewee.py @@ -10,7 +10,7 @@ ) import iso8601 -from aw_core.dirs import get_data_dir +from aw_core.dirs import get_data_dir, legacy_testing_suffix from aw_core.models import Event from playhouse.migrate import SqliteMigrator, migrate @@ -171,7 +171,7 @@ def __init__(self, testing: bool = True, filepath: Optional[str] = None) -> None if not filepath: filename = ( "peewee-sqlite" - + ("-testing" if testing else "") + + legacy_testing_suffix(testing) + f".v{LATEST_VERSION}" + ".db" ) diff --git a/aw_datastore/storages/sqlite.py b/aw_datastore/storages/sqlite.py index 9d2fd61..f139697 100644 --- a/aw_datastore/storages/sqlite.py +++ b/aw_datastore/storages/sqlite.py @@ -5,7 +5,7 @@ from datetime import datetime, timedelta, timezone from typing import Iterable, List, Optional -from aw_core.dirs import get_data_dir +from aw_core.dirs import get_data_dir, legacy_testing_suffix from aw_core.models import Event from .abstract import AbstractStorage @@ -77,7 +77,7 @@ def __init__( # Ignore the migration check if custom filepath is set ignore_migration_check = filepath is not None - ds_name = self.sid + ("-testing" if testing else "") + ds_name = self.sid + legacy_testing_suffix(testing) if not filepath: data_dir = get_data_dir("aw-server") filename = ds_name + f".v{LATEST_VERSION}" + ".db" diff --git a/tests/test_dirs.py b/tests/test_dirs.py index a26f2c3..482bb04 100644 --- a/tests/test_dirs.py +++ b/tests/test_dirs.py @@ -3,45 +3,153 @@ Three profiles (default / testing / research) must yield completely disjoint directory roots so that an ActivityWatch research build cannot read from or write to a participant's personal datastore. + +The ``testing`` profile additionally follows the new-root-plus-legacy-fallback +rule from ActivityWatch/activitywatch#1399. """ import os +from pathlib import Path from unittest.mock import patch -from aw_core.dirs import _get_appname, get_cache_dir, get_config_dir, get_data_dir +import pytest + +from aw_core.dirs import ( + _get_appname, + get_cache_dir, + get_config_dir, + get_data_dir, + legacy_testing_suffix, + using_legacy_testing_root, +) from . import context # noqa: F401 +@pytest.fixture +def fake_dirs(tmp_path, monkeypatch): + """Point platformdirs at a tmp tree so tests never touch the real home.""" + data = tmp_path / "data" + config = tmp_path / "config" + cache = tmp_path / "cache" + + def _join(root: Path, appname: str) -> str: + return str(root / appname) + + monkeypatch.setattr( + "aw_core.dirs.platformdirs.user_data_dir", + lambda appname, *a, **k: _join(data, appname), + ) + monkeypatch.setattr( + "aw_core.dirs.platformdirs.user_config_dir", + lambda appname, *a, **k: _join(config, appname), + ) + monkeypatch.setattr( + "aw_core.dirs.platformdirs.user_cache_dir", + lambda appname, *a, **k: _join(cache, appname), + ) + monkeypatch.setattr( + "aw_core.dirs.platformdirs.user_cache_path", + lambda appname, *a, **k: cache / appname, + ) + monkeypatch.setattr( + "aw_core.dirs.platformdirs.user_log_dir", + lambda appname, *a, **k: str(cache / appname / "log"), + ) + monkeypatch.delenv("AW_PROFILE", raising=False) + return {"data": data, "config": config, "cache": cache, "root": tmp_path} + + +def _plant_legacy_testing_db(fake_dirs) -> Path: + aw_server = fake_dirs["data"] / "activitywatch" / "aw-server" + aw_server.mkdir(parents=True) + marker = aw_server / "peewee-sqlite-testing.v2.db" + marker.write_text("") + return marker + + # --------------------------------------------------------------------------- # _get_appname # --------------------------------------------------------------------------- class TestGetAppname: - def test_unset_returns_bare_name(self): + def test_unset_returns_bare_name(self, fake_dirs): """AW_PROFILE absent → bare 'activitywatch', identical to legacy.""" with patch.dict(os.environ, {"AW_PROFILE": ""}): assert _get_appname() == "activitywatch" - def test_empty_string_returns_bare_name(self): + def test_empty_string_returns_bare_name(self, fake_dirs): """AW_PROFILE='' is treated the same as unset.""" with patch.dict(os.environ, {"AW_PROFILE": ""}): assert _get_appname() == "activitywatch" - def test_testing_profile(self): + def test_testing_profile_fresh_uses_new_root(self, fake_dirs): with patch.dict(os.environ, {"AW_PROFILE": "testing"}): assert _get_appname() == "activitywatch-testing" - def test_research_profile(self): + def test_research_profile(self, fake_dirs): with patch.dict(os.environ, {"AW_PROFILE": "research"}): assert _get_appname() == "activitywatch-research" - def test_arbitrary_profile(self): + def test_arbitrary_profile(self, fake_dirs): with patch.dict(os.environ, {"AW_PROFILE": "myproject"}): assert _get_appname() == "activitywatch-myproject" +# --------------------------------------------------------------------------- +# testing-root fallback (ActivityWatch/activitywatch#1399) +# --------------------------------------------------------------------------- + + +class TestTestingRootFallback: + def test_fresh_setup_uses_new_root(self, fake_dirs): + with patch.dict(os.environ, {"AW_PROFILE": "testing"}): + assert using_legacy_testing_root() is False + assert _get_appname() == "activitywatch-testing" + assert legacy_testing_suffix(True) == "" + data = get_data_dir() + assert "activitywatch-testing" in data + assert (fake_dirs["data"] / "activitywatch-testing").is_dir() + + def test_legacy_artifacts_keep_shared_root(self, fake_dirs): + _plant_legacy_testing_db(fake_dirs) + with patch.dict(os.environ, {"AW_PROFILE": "testing"}): + assert using_legacy_testing_root() is True + assert _get_appname() == "activitywatch" + assert legacy_testing_suffix(True) == "-testing" + data = get_data_dir() + assert "activitywatch-testing" not in data + assert data.endswith("activitywatch") or data.endswith("activitywatch/") + assert not (fake_dirs["data"] / "activitywatch-testing").exists() + + def test_new_root_wins_over_legacy_artifacts(self, fake_dirs): + _plant_legacy_testing_db(fake_dirs) + (fake_dirs["data"] / "activitywatch-testing").mkdir() + with patch.dict(os.environ, {"AW_PROFILE": "testing"}): + assert using_legacy_testing_root() is False + assert _get_appname() == "activitywatch-testing" + assert legacy_testing_suffix(True) == "" + + def test_config_testing_toml_is_a_legacy_marker(self, fake_dirs): + cfg = fake_dirs["config"] / "activitywatch" + cfg.mkdir(parents=True) + (cfg / "config-testing.toml").write_text("") + with patch.dict(os.environ, {"AW_PROFILE": "testing"}): + assert using_legacy_testing_root() is True + assert _get_appname() == "activitywatch" + + def test_testing_without_aw_profile_keeps_filename_suffix(self, fake_dirs): + """``testing=True`` with no profile still shares the default root.""" + assert legacy_testing_suffix(True) == "-testing" + assert legacy_testing_suffix(False) == "" + + def test_named_profile_never_suffixes_filenames(self, fake_dirs): + with patch.dict(os.environ, {"AW_PROFILE": "research"}): + assert legacy_testing_suffix(True) == "" + assert legacy_testing_suffix(False) == "" + + # --------------------------------------------------------------------------- # Directory isolation # --------------------------------------------------------------------------- @@ -56,19 +164,19 @@ def _data_dir(profile: str) -> str: class TestDirsIsolation: """Three profiles must yield fully disjoint directory roots.""" - def test_default_has_no_profile_suffix(self): + def test_default_has_no_profile_suffix(self, fake_dirs): d = _data_dir("") assert "activitywatch" in d # The bare appname must not contain a dash after "activitywatch" assert "activitywatch-" not in d - def test_testing_suffix_present(self): + def test_testing_suffix_present(self, fake_dirs): assert "activitywatch-testing" in _data_dir("testing") - def test_research_suffix_present(self): + def test_research_suffix_present(self, fake_dirs): assert "activitywatch-research" in _data_dir("research") - def test_three_profiles_are_disjoint(self): + def test_three_profiles_are_disjoint(self, fake_dirs): """default, testing, research all produce distinct, non-nested paths.""" default = _data_dir("") testing = _data_dir("testing") @@ -82,12 +190,12 @@ def test_three_profiles_are_disjoint(self): if a != b: assert not a.startswith(b + os.sep), f"{a!r} is a subpath of {b!r}" - def test_config_dir_isolated(self): + def test_config_dir_isolated(self, fake_dirs): with patch.dict(os.environ, {"AW_PROFILE": "research"}): cfg = get_config_dir() assert "activitywatch-research" in cfg - def test_cache_dir_isolated(self): + def test_cache_dir_isolated(self, fake_dirs): with patch.dict(os.environ, {"AW_PROFILE": "testing"}): cache = get_cache_dir() assert "activitywatch-testing" in cache