Fix #792: reconcile sync mode was clamped back to 'replace' in the backend
start_playlist_sync validated the resolved mode with 'if sync_mode not in (replace, append): sync_mode = replace' — a pre-existing clamp two lines below the config read I added, which silently downgraded a configured 'reconcile' to 'replace'. So config=reconcile resolved correctly then got clobbered, and every sync ran replace regardless of the setting (and incognito didn't help — it's backend, not cache). Replace the hand-rolled clamp with a pure normalize_sync_mode(requested, configured) helper (VALID_SYNC_MODES includes reconcile) so the resolution is testable and can't silently drop a mode again. Regression tests cover reconcile-from-config, request-overrides-config, and unknown->replace.
This commit is contained in:
parent
b105372d70
commit
65c9948c78
3 changed files with 52 additions and 4 deletions
|
|
@ -109,4 +109,26 @@ def plan_playlist_reconcile(
|
||||||
return {"add": add, "remove": remove}
|
return {"add": add, "remove": remove}
|
||||||
|
|
||||||
|
|
||||||
__all__ = ["plan_playlist_add", "remove_one_occurrence", "plan_playlist_reconcile"]
|
VALID_SYNC_MODES = ("replace", "append", "reconcile")
|
||||||
|
|
||||||
|
|
||||||
|
def normalize_sync_mode(requested, configured, default: str = "replace") -> str:
|
||||||
|
"""Resolve the effective playlist sync mode.
|
||||||
|
|
||||||
|
An explicit per-request value wins; otherwise the configured default
|
||||||
|
(Settings > Playlist sync mode); anything unrecognized falls back to
|
||||||
|
``default``. Keeping ``reconcile`` in ``VALID_SYNC_MODES`` is load-bearing —
|
||||||
|
a validation list that omits it silently downgrades reconcile to replace,
|
||||||
|
which is exactly the #792 regression this helper exists to prevent.
|
||||||
|
"""
|
||||||
|
mode = (requested or "") or (configured or "") or default
|
||||||
|
return mode if mode in VALID_SYNC_MODES else default
|
||||||
|
|
||||||
|
|
||||||
|
__all__ = [
|
||||||
|
"plan_playlist_add",
|
||||||
|
"remove_one_occurrence",
|
||||||
|
"plan_playlist_reconcile",
|
||||||
|
"normalize_sync_mode",
|
||||||
|
"VALID_SYNC_MODES",
|
||||||
|
]
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from core.sync.playlist_edit import (
|
from core.sync.playlist_edit import (
|
||||||
|
normalize_sync_mode,
|
||||||
plan_playlist_add,
|
plan_playlist_add,
|
||||||
plan_playlist_reconcile,
|
plan_playlist_reconcile,
|
||||||
remove_one_occurrence,
|
remove_one_occurrence,
|
||||||
|
|
@ -126,3 +127,28 @@ def test_reconcile_duplicate_still_desired_not_removed():
|
||||||
# a gone id is listed once even if it had duplicates.
|
# a gone id is listed once even if it had duplicates.
|
||||||
plan = plan_playlist_reconcile(['a', 'a', 'b', 'b'], ['a'])
|
plan = plan_playlist_reconcile(['a', 'a', 'b', 'b'], ['a'])
|
||||||
assert plan == {'add': [], 'remove': ['b']}
|
assert plan == {'add': [], 'remove': ['b']}
|
||||||
|
|
||||||
|
|
||||||
|
# ── normalize_sync_mode: reconcile must survive resolution (#792 regression) ──
|
||||||
|
|
||||||
|
def test_normalize_keeps_reconcile_from_config():
|
||||||
|
# The bug: a validation list omitting 'reconcile' downgraded the configured
|
||||||
|
# setting to 'replace'. No per-request override → configured value wins.
|
||||||
|
assert normalize_sync_mode(None, 'reconcile') == 'reconcile'
|
||||||
|
assert normalize_sync_mode('', 'reconcile') == 'reconcile'
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_request_overrides_config():
|
||||||
|
assert normalize_sync_mode('append', 'reconcile') == 'append'
|
||||||
|
assert normalize_sync_mode('reconcile', 'replace') == 'reconcile'
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_falls_back_for_unknown():
|
||||||
|
assert normalize_sync_mode('bogus', 'replace') == 'replace'
|
||||||
|
assert normalize_sync_mode(None, None) == 'replace'
|
||||||
|
assert normalize_sync_mode(None, 'also-bogus') == 'replace'
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_all_real_modes_pass_through():
|
||||||
|
for m in ('replace', 'append', 'reconcile'):
|
||||||
|
assert normalize_sync_mode(m, 'replace') == m
|
||||||
|
|
|
||||||
|
|
@ -23548,9 +23548,9 @@ def start_playlist_sync():
|
||||||
# /Playlists/<id>/Items, Navidrome updatePlaylist?songIdToAdd=...).
|
# /Playlists/<id>/Items, Navidrome updatePlaylist?songIdToAdd=...).
|
||||||
# Per-request sync_mode wins; otherwise use the configured default
|
# Per-request sync_mode wins; otherwise use the configured default
|
||||||
# (Settings > Playlist sync mode). Default 'replace' keeps today's behavior.
|
# (Settings > Playlist sync mode). Default 'replace' keeps today's behavior.
|
||||||
sync_mode = data.get('sync_mode') or config_manager.get('playlist_sync.mode', 'replace')
|
from core.sync.playlist_edit import normalize_sync_mode
|
||||||
if sync_mode not in ('replace', 'append'):
|
sync_mode = normalize_sync_mode(data.get('sync_mode'),
|
||||||
sync_mode = 'replace'
|
config_manager.get('playlist_sync.mode', 'replace'))
|
||||||
|
|
||||||
if not all([playlist_id, playlist_name, tracks_json]):
|
if not all([playlist_id, playlist_name, tracks_json]):
|
||||||
return jsonify({"success": False, "error": "Missing playlist_id, name, or tracks."}), 400
|
return jsonify({"success": False, "error": "Missing playlist_id, name, or tracks."}), 400
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue