diff --git a/test/web_interface/test_normalize_nullable_structural_types.py b/test/web_interface/test_normalize_nullable_structural_types.py new file mode 100644 index 00000000..9649b5be --- /dev/null +++ b/test/web_interface/test_normalize_nullable_structural_types.py @@ -0,0 +1,164 @@ +"""Tests for the union-type branch of normalize_config_values, which the +plugin-config save path (_prepare_plugin_config_for_save in api_v3/plugins.py) +runs before validation. + +``customization.modes.`` overrides are generated by +``element_style._nullable``, which widens every field's declared type with +``'null'`` so a blank means "inherit the base". That turns a colour declared +``"array"`` into ``["array", "null"]``. The union branch only knew how to +convert null/integer/number/boolean, so a valid ``[0, 249, 0]`` matched nothing: +it logged "Could not normalize field ..." and, because it then skipped the +single-type handling, never normalized the array's items either. +""" + +import logging + +import pytest + +from web_interface.blueprints.api_v3.plugins import ( + _prepare_plugin_config_for_save, +) + + +class _SchemaMgr: + """Minimal stand-in for the real schema manager. + + Defaults are empty and validation always passes, so these tests isolate the + normalization pass rather than re-testing the validator. + """ + + def generate_default_config(self, plugin_id, use_cache=True): + return {} + + def validate_config_against_schema(self, config, schema, plugin_id): + return True, [] + + +def _rgb_prop(nullable): + """An RGB colour field as the base schema (plain) or a mode override.""" + prop = {"type": "array", "items": {"type": "integer"}, + "minItems": 3, "maxItems": 3} + if nullable: + prop = dict(prop, type=["array", "null"], default=None) + return prop + + +def _schema(nullable): + """A scoreboard-shaped schema: customization.modes.upcoming.odds_text.""" + return { + "type": "object", + "properties": { + "enabled": {"type": "boolean"}, + "customization": { + "type": "object", + "properties": { + "modes": { + "type": "object", + "properties": { + "upcoming": { + "type": "object", + "properties": { + "odds_text": { + "type": "object", + "properties": { + "text_color": _rgb_prop(nullable), + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + +def _save(config, nullable=True): + regular, _secrets, error = _prepare_plugin_config_for_save( + "baseball-scoreboard", config, _schema(nullable), _SchemaMgr(), + is_json=True, + ) + assert error is None + return regular + + +def _colour(config): + return (config["customization"]["modes"]["upcoming"] + ["odds_text"]["text_color"]) + + +def _submit(value): + return {"enabled": True, + "customization": {"modes": {"upcoming": { + "odds_text": {"text_color": value}}}}} + + +class TestNullableArrayOverride: + """['array', 'null'] holding a real list is valid, not a failure.""" + + def test_valid_rgb_survives_the_save(self) -> None: + """The exact value from the bug report is stored unchanged.""" + assert _colour(_save(_submit([0, 249, 0]))) == [0, 249, 0] + + def test_valid_rgb_does_not_warn(self, caplog) -> None: + """No "Could not normalize" for a value that already matches.""" + with caplog.at_level(logging.WARNING): + _save(_submit([0, 249, 0])) + assert "Could not normalize field" not in caplog.text + + def test_items_are_normalized_like_a_plain_array(self) -> None: + """The real defect: form data posts strings, and a nullable array used + to keep them while a plain 'array' turned them into ints.""" + nullable = _colour(_save(_submit(["0", "249", "0"]))) + plain = _colour(_save(_submit(["0", "249", "0"]), nullable=False)) + assert nullable == [0, 249, 0] + assert nullable == plain + + def test_null_still_means_inherit(self) -> None: + """Widening the type must not break the blank-means-inherit case.""" + assert _colour(_save(_submit(None))) is None + + @pytest.mark.parametrize("blank", ["", " ", "null", "None"]) + def test_blank_strings_still_become_null(self, blank) -> None: + """A cleared form field arrives as a string and still means inherit.""" + assert _colour(_save(_submit(blank))) is None + + +class TestFractionalItemsReachTheValidator: + """int(2.5) == 2 would store a corrected number where the client sent a + wrong one, so the save looks successful and the config is quietly wrong. + Routing nullable arrays into the shared item handling made this reachable + for them too, so both paths convert only genuinely integral values.""" + + @pytest.mark.parametrize("nullable", [True, False]) + def test_a_fractional_item_is_not_truncated(self, nullable) -> None: + """2.5 reaches the validator intact, which then refuses it for an + integer item. Storing 2 would make the save look successful.""" + assert _colour(_save(_submit([2.5, 249, 0]), + nullable=nullable)) == [2.5, 249, 0] + + @pytest.mark.parametrize("nullable", [True, False]) + def test_a_whole_float_still_converts(self, nullable) -> None: + """JSON has no int/float distinction, so 2.0 is a legitimate integer.""" + assert _colour(_save(_submit([2.0, 249.0, 0.0]), + nullable=nullable)) == [2, 249, 0] + + @pytest.mark.parametrize("nullable", [True, False]) + def test_a_fractional_string_is_preserved_too(self, nullable) -> None: + """int('2.5') raises, so this path always preserved the value -- + pinned so the numeric and string cases cannot drift apart again.""" + assert _colour(_save(_submit(["2.5", "249", "0"]), + nullable=nullable)) == ["2.5", 249, 0] + + +class TestUnionFallbackStillReports: + """A union the value genuinely does not match must still warn.""" + + def test_unmatched_value_warns_and_passes_through(self, caplog) -> None: + """An object where ['array', 'null'] is declared is a real mismatch, + so it keeps reaching the validator with the diagnostic intact.""" + with caplog.at_level(logging.WARNING): + saved = _save(_submit({"r": 0})) + assert "Could not normalize field" in caplog.text + assert _colour(saved) == {"r": 0} diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index a5ebefbf..6fce4411 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -2080,10 +2080,29 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr normalized[key] = value.strip().lower() in ('true', '1', 'on', 'yes') continue - # Nothing converted: keep the value for validation to report. - logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}") - normalized[key] = value - continue + # The scalar conversions above are the only ones this branch + # knows, so a union naming a structural or string type fell + # through here even when the value already matched it -- + # customization.modes. makes every override nullable + # (see element_style._nullable), so every per-mode colour is + # ['array', 'null'] and warned on a perfectly valid [r, g, b]. + # Worse than the noise: `continue` skipped the single-type + # handling below, so a nullable array never had its items + # normalized and form-posted ["0", "249", "0"] stayed strings + # where a plain 'array' field would have become ints. Re-enter + # that handling with the matched member instead. + if isinstance(value, list) and 'array' in prop_type: + prop_type = 'array' + elif isinstance(value, dict) and 'object' in prop_type: + prop_type = 'object' + elif isinstance(value, str) and 'string' in prop_type: + normalized[key] = value + continue + else: + # Nothing converted: keep the value for validation to report. + logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}") + normalized[key] = value + continue if isinstance(value, dict) and prop_type == 'object' and 'properties' in prop_schema: # Recursively normalize nested objects @@ -2112,7 +2131,16 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr except (ValueError, TypeError, OverflowError): pass elif isinstance(v, (int, float)): - normalized_array.append(int(v)) + # Only a genuinely integral value converts. + # int(2.5) == 2 would store a silently + # corrected number where the client sent a + # wrong one; leaving it lets the validator + # reject it. A whole float (2.0 out of JSON) + # is integral and still converts. + if isinstance(v, int) or float(v).is_integer(): + normalized_array.append(int(v)) + else: + normalized_array.append(v) continue elif 'number' in item_type: if isinstance(v, str): @@ -2138,7 +2166,11 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr except (ValueError, TypeError, OverflowError): normalized_array.append(v) elif isinstance(v, (int, float)): - normalized_array.append(int(v)) + # Integral only -- see the union branch above. + if isinstance(v, int) or float(v).is_integer(): + normalized_array.append(int(v)) + else: + normalized_array.append(v) else: normalized_array.append(v) normalized[key] = normalized_array