Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# Changelog

## 0.1.0rc11 — 2026-07-28

Free-text fields are no longer type-coerced during parsing (closes ClickUp 869cqbpxa).

### Fixed
- `CuemsParser.str_to_value` no longer coerces values whose key names a string-typed field. It previously ran every scalar through `int` → `float` → `strtobool` → `Uuid` regardless of key, so free text was silently rewritten. Because `strtobool` accepts the truth abbreviations, a cue named `n`/`N`/`f`/`F` was persisted as `False`, `y`/`Y`/`t`/`T` as `True`, and any bare digit as an `int` — 18 of the 62 alphanumeric single characters were corrupted, along with the words `yes`/`no`/`true`/`false`/`on`/`off`. Sergio reported the symptom as "can't name a cue with a single letter"; there was never a length rule, the failing characters were exactly `strtobool`'s vocabulary. The affected keys reachable in practice are `name`, `description` and `file_name`.
- A cue named lowercase `none` or `null` hit the `['none', 'null', '']` → `None` branch, serialised to `<name/>`, and failed `NameStringType`'s `minLength=1` — a hard `XMLSchemaValidationError` at save time rather than silent corruption. The new short-circuit precedes that branch, so both failure modes are fixed together.

### Added
- `STRING_TYPED_KEYS` in `xml/Parsers.py` — the set of keys exempt from coercion. `name`, `description` and `file_name` are the ones reachable today; `output_name`, `parameter_name`, `icon`, `color` and `unix_name` are defensive entries, currently shielded by unrelated bypasses in `outputsParser`, `_normalize_fade_parameters` and the `GenericDict` fallback, listed so that fixing any of those bypasses cannot silently reintroduce this bug.
- `str_to_value` takes an optional `key` argument, threaded through all four call sites (`CuemsScriptParser`, `CueListParser`, `GenericParser`, `fade_profileParser`). The argument is optional, so existing single-argument callers are unaffected.
- `tests/test_name_coercion.py` — exhaustive sweep over all 62 alphanumeric single characters for each reachable key, the boolean/nullish word set, a full XML round-trip, and negative tests pinning that `enabled`/`autoload`/`timecode`/`loop` still coerce and that `id` still parses to a `Uuid`.

### Notes
- `id` is deliberately **not** in the allowlist: the `Uuid()` branch inside `str_to_value` is the only thing that produces `Uuid` objects on parse (the parsers assign via raw `dict.__setitem__` and never reach the property setters), so adding it would downgrade every cue, script and media id to a plain `str`. The consequence is that `DmxSceneType.id` (`script.xsd:403`, declared `xs:string`) cannot be protected by this mechanism — accepted, since DMX scene ids are system-assigned rather than operator-typed.
- Projects saved before this release have the corrupted name baked into their `cue_script.xml`; the original text is unrecoverable (`n`, `no`, `N`, `off` all collapse to `False`) and must be renamed by hand.
- `cuems-nodeconf` calls the inherited `str_to_value` without a key (`NodeXmlBuilders.py:80`), so node names remain exposed to the identical bug. Tracked separately.

## 0.1.0rc8 — 2026-05-20

Production call-site migration to the `CTimecode` v2 API, removal of a long-deprecated method, and a new required settings field for `gradient-motiond` integration.
Expand Down
2 changes: 1 addition & 1 deletion src/cuemsutils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: 2025-present Adrià (StageLab) <adria@stagelab.coop>
#
# SPDX-License-Identifier: GPL-3.0
__version__ = "0.1.0rc10"
__version__ = "0.1.0rc11"
47 changes: 42 additions & 5 deletions src/cuemsutils/xml/Parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,28 @@
#TODO: XML_ROOT_TAG get from constants storage
XML_ROOT_TAG = 'CuemsScript'

# Keys that must never be type-coerced by str_to_value(). Without this, a cue named
# "n" is saved as False, one named "1" as int 1, and one named "none" becomes None ->
# <name/> -> XSD minLength violation, i.e. a hard save error. See ClickUp 869cqbpxa.
#
# 'name', 'description' and 'file_name' are the keys actually reachable today. The
# rest are defensive only: they are currently shielded by bypasses in outputsParser
# (builds output objects directly), _normalize_fade_parameters (diverts 'parameters'
# before the scalar branch) and the GenericDict fallback in GenericParser.parse()
# (get_class('ui_properties') misses because the class is UI_properties). They are
# listed so that fixing any of those bypasses cannot silently reintroduce this bug.
#
# 'id' is deliberately ABSENT: the Uuid() branch in str_to_value is the only thing
# that produces Uuid objects on parse (parsers assign via raw dict.__setitem__ and so
# never hit the property setters). Adding 'id' here would downgrade every cue, script
# and media id to a plain str.
STRING_TYPED_KEYS = frozenset({
# reachable today
'name', 'description', 'file_name',
# defensive -- see above
'output_name', 'parameter_name', 'icon', 'color', 'unix_name',
})

class GenericDict(dict):
pass

Expand Down Expand Up @@ -56,7 +78,22 @@ def get_first_key(self, _dict):
def get_contained_dict(self, _dict):
return list(_dict.values())[0]

def str_to_value(self, _string):
def str_to_value(self, _string, key = None):
"""Decode a string-encoded scalar into its Python type.

Args:
_string: The value to decode. Non-str values pass through unchanged.
key: The dict key ``_string`` was stored under, when known. Values
whose key is in :data:`STRING_TYPED_KEYS` are returned verbatim
so free-text fields are never coerced (ClickUp 869cqbpxa).

Returns:
The decoded value, or ``_string`` unchanged for string-typed keys.
"""
# Must precede every coercion branch below, including the none/null one:
# a cue legitimately named "none" would otherwise become None.
if key in STRING_TYPED_KEYS:
return _string
if not isinstance(_string, str):
return _string
if _string in ['none', 'null', '']:
Expand Down Expand Up @@ -94,7 +131,7 @@ def parse(self):
parser_class, class_string = self.get_parser_class(k)
self.item_csp[k] = parser_class(init_dict=v, class_string=class_string).parse()
else:
v = self.str_to_value(v)
v = self.str_to_value(v, key = k)
self.item_csp[k] = v

return self.item_csp
Expand Down Expand Up @@ -128,7 +165,7 @@ def parse(self):
self.item_clp[k] = value_parser_class(init_dict=v, class_string=value_class_string).parse()

else:
v = self.str_to_value(v)
v = self.str_to_value(v, key = k)
self.item_clp[k] = v
return self.item_clp

Expand Down Expand Up @@ -177,7 +214,7 @@ def parse(self):
else:
self.item_gp[dict_key] = local_list
else:
dict_value = self.str_to_value(dict_value)
dict_value = self.str_to_value(dict_value, key = dict_key)
self.item_gp[dict_key] = dict_value
return self.item_gp

Expand Down Expand Up @@ -360,7 +397,7 @@ def parse(self):
pcls(init_dict=li, class_string=pstr).parse() for li in dict_value
]
else:
d[dict_key] = self.str_to_value(dict_value)
d[dict_key] = self.str_to_value(dict_value, key = dict_key)
return FadeProfile(d)


Expand Down
184 changes: 184 additions & 0 deletions tests/test_name_coercion.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
"""Free-text fields must survive parsing verbatim.

Regression tests for ClickUp 869cqbpxa: ``CuemsParser.str_to_value`` used to
coerce every scalar regardless of its key, so a cue named ``n`` was saved as
``False``, one named ``1`` as int ``1``, and one named ``none`` collapsed to
``None`` -> ``<name/>`` -> a hard XSD ``minLength`` failure on save.
"""

import string
from datetime import datetime, timezone
from pathlib import Path

import pytest

from cuemsutils.cues import AudioCue, CueList, CuemsScript
from cuemsutils.cues.MediaCue import Media, Region
from cuemsutils.tools.Uuid import Uuid
from cuemsutils.xml import XmlReaderWriter
from cuemsutils.xml.Parsers import STRING_TYPED_KEYS, CuemsParser

TMP_DIR = Path(__file__).parent / "tmp"
TMP_DIR.mkdir(exist_ok=True)

# The strtobool truth abbreviations -- the single characters that used to break.
TRUTHY_TOKENS = ["y", "Y", "t", "T", "yes", "true", "on"]
FALSY_TOKENS = ["n", "N", "f", "F", "no", "false", "off"]
# Hit the ['none', 'null', ''] -> None branch, which raised on save.
NULLISH_TOKENS = ["none", "null"]

UUID_STR = "1f301cf8-dd03-4b40-ac17-ef0e5e7988be"


def _str_to_value(value, key=None):
"""Call the parser helper without needing a constructed parser."""
return CuemsParser.str_to_value(None, value, key=key)


def _audio_cue(name="placeholder"):
cue = AudioCue({
"Media": Media({
"file_name": "f.wav",
"id": "",
"duration": "00:00:00.000",
"regions": [
Region({"id": 0, "loop": 1, "in_time": None, "out_time": None})
],
}),
"ui_properties": {"warning": None},
})
cue.name = name
return cue


def _script(cue_name="placeholder"):
cuelist = CueList({"contents": [_audio_cue(cue_name)]})
cuelist.name = "main"
script = CuemsScript({"CueList": cuelist})
script.name = "proj"
# Dates required by the CuemsScript schema assertion (modified >= created).
now = datetime.now(timezone.utc).isoformat()
script.created = now
script.modified = now
return script


def _roundtrip_cue_name(name, tmp_name):
"""Write a script whose cue is named ``name``, read it back, return the name."""
path = str(TMP_DIR / tmp_name)
writer = XmlReaderWriter(schema_name="script", xmlfile=path)
writer.write_from_object(_script(name))
assert writer.validate() is None
loaded = XmlReaderWriter(schema_name="script", xmlfile=path).read_to_objects()
return loaded.cuelist.contents[0].name


# ---------------------------------------------------------------------------
# str_to_value -- string-typed keys are never coerced
# ---------------------------------------------------------------------------

@pytest.mark.parametrize("char", list(string.ascii_letters + string.digits))
@pytest.mark.parametrize("key", ["name", "description", "file_name"])
def test_single_characters_survive_for_string_typed_keys(char, key):
"""All 62 alphanumeric single characters must pass through untouched.

18 of them (y/Y/t/T/n/N/f/F and the ten digits) used to be corrupted.
"""
result = _str_to_value(char, key=key)
assert result == char
assert isinstance(result, str)


@pytest.mark.parametrize("token", TRUTHY_TOKENS + FALSY_TOKENS + NULLISH_TOKENS)
def test_boolean_and_nullish_words_survive_as_names(token):
result = _str_to_value(token, key="name")
assert result == token
assert isinstance(result, str)


def test_every_string_typed_key_is_protected():
"""Guards the allowlist itself, including the defensive entries."""
for key in STRING_TYPED_KEYS:
assert _str_to_value("n", key=key) == "n"
assert _str_to_value("1", key=key) == "1"
assert _str_to_value("none", key=key) == "none"


# ---------------------------------------------------------------------------
# ...but coercion still happens everywhere it must
# ---------------------------------------------------------------------------

@pytest.mark.parametrize("key", ["enabled", "autoload", "timecode", "loop"])
@pytest.mark.parametrize(
"value,expected",
[("true", True), ("false", False), ("True", True), ("False", False)],
)
def test_boolean_keys_still_coerce(key, value, expected):
assert _str_to_value(value, key=key) is expected


def test_numeric_and_nullish_keys_still_coerce():
assert _str_to_value("1", key="loop") == 1
assert _str_to_value("42", key="master_vol") == 42
assert _str_to_value("none", key="target") is None
assert _str_to_value("", key="target") is None


def test_id_is_not_allowlisted_and_still_becomes_a_uuid():
"""'id' must stay coercible -- the Uuid() branch is the only thing that
produces Uuid objects on parse."""
assert "id" not in STRING_TYPED_KEYS
assert isinstance(_str_to_value(UUID_STR, key="id"), Uuid)


def test_unkeyed_calls_are_unchanged():
"""The key argument is optional; existing callers pass one positional arg."""
assert _str_to_value("n") is False
assert _str_to_value("1") == 1
assert _str_to_value("none") is None


# ---------------------------------------------------------------------------
# Full XML roundtrip through the editor's save path
# ---------------------------------------------------------------------------

@pytest.mark.parametrize("name", ["a", "n", "y", "t", "f", "N", "1", "0", "no", "on"])
def test_cue_name_survives_xml_roundtrip(name):
assert _roundtrip_cue_name(name, f"test_name_coercion_{name}.xml") == name


@pytest.mark.parametrize("name", NULLISH_TOKENS)
def test_nullish_cue_name_no_longer_fails_validation(name):
"""These used to raise XMLSchemaValidationError (minLength=1) on save."""
assert _roundtrip_cue_name(name, f"test_name_coercion_null_{name}.xml") == name


def test_parser_preserves_name_through_the_editor_save_path():
"""CuemsParser is what CuemsDBProject.update() runs on the frontend payload."""
parsed = CuemsParser({"AudioCue": {"name": "n", "description": "off"}}).parse()
assert parsed["name"] == "n"
assert parsed["description"] == "off"


# ---------------------------------------------------------------------------
# output_name: currently shielded by outputsParser, but assert the consumer
# that would break first if that bypass is ever removed (plan section 4b/5).
# ---------------------------------------------------------------------------

def test_get_all_output_names_handles_numeric_output_name():
"""MediaCue.get_all_output_names slices output_name -- an int would raise
TypeError: 'int' object is not subscriptable."""
parsed = CuemsParser({
"AudioCue": {
"name": "cue",
"outputs": {
"AudioCueOutput": [
{"output_name": "1", "output_vol": "80", "channels": {}}
]
},
}
}).parse()
# Returns (node_id, output_id) tuples split at the UUID boundary.
names = parsed.get_all_output_names()
assert names == [("1", "")]
assert all(isinstance(part, str) for pair in names for part in pair)
Loading