Correct the version and make the config merge deeply
The package version had been left at the scaffolded 0.1.0 while the released tag was v5.4.0. Nothing in the code chose 0.1.0 -- it was npm-init's equivalent, never updated -- and it made every version comparison meaningless: the window title, the About dialog and the update check all read it. ConfigManager merged a stored config over the defaults with dict.update(), which is shallow. A config written by an older build carries partial "player" and "feed" objects, and a shallow update replaces the whole nested default with the partial one, so keys added since came back missing. The `or 15` and `or "auto"` fallbacks at the call sites were load-bearing because of it. The merge is now recursive, and keys present only in the stored file are kept so a downgrade cannot destroy settings. Configs now carry a config_version. A file written before 5.4.0 -- including one inherited from an upstream YTSage install -- has its stored check_app_updates cleared once, because that setting used to point at PyPI's `ytsage` package and oop7/YTSage's releases, neither of which is this program. The settings tab read a missing check_app_updates as enabled and persisted that reading on OK, so merely opening Custom Options turned the checker back on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+10
-4
@@ -1,8 +1,14 @@
|
||||
"""
|
||||
YTSage - YouTube Video Downloader
|
||||
SageTube - watch-first YouTube client
|
||||
|
||||
A modern, user-friendly YouTube video downloader built with PySide6.
|
||||
Search, browse, subscribe and stream in an embedded mpv player, with the full
|
||||
yt-dlp download feature set inherited from YTSage. Built with PySide6.
|
||||
|
||||
The version below is the single source of truth for the running application:
|
||||
the window title, the About dialog and the update checker all read it. It must
|
||||
match `version` in pyproject.toml and the repository's latest signed tag --
|
||||
this package's tag line continues YTSage's, which is why it starts at 5.x.
|
||||
"""
|
||||
|
||||
__version__ = "0.1.0"
|
||||
__author__ = "oop7"
|
||||
__version__ = "5.4.0"
|
||||
__author__ = "Houmeres"
|
||||
|
||||
@@ -688,9 +688,11 @@ class UpdaterTabWidget(QWidget):
|
||||
beta_enabled = ConfigManager.get("check_beta_updates") or False
|
||||
self.beta_updates_checkbox.setChecked(beta_enabled)
|
||||
|
||||
# Load app update checker setting (default enabled for older configs)
|
||||
# Load app update checker setting. `is not False` would have read a
|
||||
# missing key as enabled, and since this dialog persists the value
|
||||
# unconditionally on OK, merely opening it re-enabled the checker.
|
||||
app_updates_enabled = ConfigManager.get("check_app_updates")
|
||||
self.app_updates_checkbox.setChecked(app_updates_enabled is not False)
|
||||
self.app_updates_checkbox.setChecked(bool(app_updates_enabled))
|
||||
|
||||
# Set current selection based on saved settings
|
||||
current_frequency = auto_settings["frequency"]
|
||||
|
||||
@@ -72,6 +72,9 @@ class ConfigManager:
|
||||
_config_file: Path = APP_CONFIG_FILE
|
||||
_settings: Dict[str, Any] = {}
|
||||
_default_config: Dict[str, Any] = {
|
||||
# Bumped whenever a stored config needs rewriting rather than merely
|
||||
# merging; see _migrate(). Absent means "written before 5.4.0".
|
||||
"config_version": 2,
|
||||
"download_path": str(USER_HOME_DIR / "Downloads"),
|
||||
"generic_mode": True,
|
||||
"speed_limit_value": None,
|
||||
@@ -122,6 +125,56 @@ class ConfigManager:
|
||||
},
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _deep_merge(cls, defaults: Dict[str, Any], stored: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""
|
||||
Recursively layer `stored` on top of `defaults`.
|
||||
|
||||
A shallow `dict.update()` here would be a data-loss bug: a config file
|
||||
written by an older build contains a *partial* "player" or "feed"
|
||||
object, and updating shallowly replaces the whole nested default with
|
||||
it -- so every key added since would come back missing.
|
||||
|
||||
Rules:
|
||||
- dict + dict recurse
|
||||
- anything else (lists, scalars, type mismatches) replaces wholesale
|
||||
- keys present only in `stored` are kept, never pruned; a user who
|
||||
downgrades must not have their settings destroyed by the older build
|
||||
"""
|
||||
merged = copy.deepcopy(defaults)
|
||||
for key, value in stored.items():
|
||||
existing = merged.get(key)
|
||||
if isinstance(value, dict) and isinstance(existing, dict):
|
||||
merged[key] = cls._deep_merge(existing, value)
|
||||
else:
|
||||
merged[key] = copy.deepcopy(value)
|
||||
return merged
|
||||
|
||||
@classmethod
|
||||
def _migrate(cls, stored: Dict[str, Any]) -> bool:
|
||||
"""
|
||||
Bring a stored config up to CONFIG_VERSION in place.
|
||||
|
||||
Returns True when something changed, so the caller knows to re-save.
|
||||
Migrations must be idempotent and must never raise -- a config that
|
||||
cannot be migrated is still a config the app has to start with.
|
||||
"""
|
||||
changed = False
|
||||
version = stored.get("config_version")
|
||||
|
||||
if not isinstance(version, int) or version < 2:
|
||||
# Pre-5.4.0, and possibly inherited from an upstream YTSage
|
||||
# install. Such a file carries "check_app_updates": true, which
|
||||
# used to point at PyPI's `ytsage` package and oop7/YTSage's
|
||||
# releases -- neither of which is this application. Dropping the
|
||||
# stored value lets SageTube's own default decide.
|
||||
if stored.pop("check_app_updates", None) is not None:
|
||||
logger.info("Config migration: cleared inherited 'check_app_updates'.")
|
||||
stored["config_version"] = 2
|
||||
changed = True
|
||||
|
||||
return changed
|
||||
|
||||
@classmethod
|
||||
def _load(cls) -> None:
|
||||
"""
|
||||
@@ -134,11 +187,21 @@ class ConfigManager:
|
||||
try:
|
||||
with open(cls._config_file, "r", encoding="utf-8") as f:
|
||||
stored = json.load(f)
|
||||
if not isinstance(stored, dict):
|
||||
raise json.JSONDecodeError("config root is not an object", "", 0)
|
||||
|
||||
migrated = False
|
||||
try:
|
||||
migrated = cls._migrate(stored)
|
||||
except Exception as e:
|
||||
logger.exception(f"Config migration failed, continuing unmigrated: {e}")
|
||||
|
||||
# Merge on top of defaults so keys added in newer versions
|
||||
# exist without call sites needing `or <default>` fallbacks
|
||||
cls._settings = copy.deepcopy(cls._default_config)
|
||||
cls._settings.update(stored)
|
||||
cls._settings = cls._deep_merge(cls._default_config, stored)
|
||||
logger.info("Config loaded from file.")
|
||||
if migrated:
|
||||
cls._save()
|
||||
except json.JSONDecodeError:
|
||||
cls._settings = copy.deepcopy(cls._default_config)
|
||||
logger.warning("Config file corrupt, loaded defaults.")
|
||||
|
||||
Reference in New Issue
Block a user