From 40f5fdfcc872b9e72cd2e02387f0bf8b4c9f1730 Mon Sep 17 00:00:00 2001 From: FLAME Date: Sat, 11 Oct 2025 12:13:11 -0400 Subject: [PATCH 01/13] Upgrade to Marshmallow >= 4 with Superset able to be booted up and able to be logged in --- superset/advanced_data_type/schemas.py | 4 ++-- superset/themes/schemas.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/superset/advanced_data_type/schemas.py b/superset/advanced_data_type/schemas.py index d94fd19873b5..47918e5416f0 100644 --- a/superset/advanced_data_type/schemas.py +++ b/superset/advanced_data_type/schemas.py @@ -23,10 +23,10 @@ advanced_data_type_convert_schema = { "type": "object", "properties": { - "type": {"type": "string", "default": "port"}, + "type": {"type": "string", "dump_default": "port"}, "values": { "type": "array", - "items": {"default": "http"}, + "items": {"dump_default": "http"}, "minItems": 1, }, }, diff --git a/superset/themes/schemas.py b/superset/themes/schemas.py index eaa40fdd8bfd..cc9b9ea03c94 100644 --- a/superset/themes/schemas.py +++ b/superset/themes/schemas.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from contextvars import ContextVar from typing import Any from marshmallow import fields, Schema, validates, ValidationError @@ -25,6 +26,11 @@ ) from superset.utils import json +# Context variable for storing sanitized JSON data during validation +sanitized_json_context: ContextVar[str | None] = ContextVar( + "sanitized_json_data", default=None +) + def _sanitize_and_validate_theme_config(theme_config: dict[str, Any]) -> dict[str, Any]: """Sanitize and validate theme configuration. @@ -77,7 +83,7 @@ def validate_json_data(self, value: dict[str, Any], **kwargs: Any) -> None: value.clear() value.update(sanitized_config) else: - self.context["sanitized_json_data"] = json.dumps(sanitized_config) + sanitized_json_context.set(json.dumps(sanitized_config)) class ThemePostSchema(Schema): @@ -104,7 +110,7 @@ def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: # Note: This modifies the input data to ensure sanitized content is stored if sanitized_config != theme_config: # Re-serialize the sanitized config - self.context["sanitized_json_data"] = json.dumps(sanitized_config) + sanitized_json_context.set(json.dumps(sanitized_config)) class ThemePutSchema(Schema): @@ -131,7 +137,7 @@ def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: # Note: This modifies the input data to ensure sanitized content is stored if sanitized_config != theme_config: # Re-serialize the sanitized config - self.context["sanitized_json_data"] = json.dumps(sanitized_config) + sanitized_json_context.set(json.dumps(sanitized_config)) openapi_spec_methods_override = { From 416ba17c4af2a597aebb658fd881976a8a3f9b3d Mon Sep 17 00:00:00 2001 From: FLAME Date: Mon, 27 Oct 2025 00:26:11 -0400 Subject: [PATCH 02/13] Apply the compatibility patch to address incompatibilities between Flask-AppBuilder 5.0.0 and marshmallow 4.x --- superset/marshmallow_fix.py | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 superset/marshmallow_fix.py diff --git a/superset/marshmallow_fix.py b/superset/marshmallow_fix.py new file mode 100644 index 000000000000..99a6da3716c5 --- /dev/null +++ b/superset/marshmallow_fix.py @@ -0,0 +1,68 @@ +""" +Marshmallow 4.x Compatibility Fix for Flask-AppBuilder 5.0.0 + +This module provides a targeted fix for incompatibilities between +Flask-AppBuilder 5.0.0 and marshmallow 4.x, specifically handling +missing auto-generated fields during schema initialization. +""" + +from marshmallow import fields + + +def patch_marshmallow_for_flask_appbuilder(): + """ + Patches marshmallow Schema._init_fields to handle Flask-AppBuilder 5.0.0 + compatibility with marshmallow 4.x. + + Flask-AppBuilder 5.0.0 automatically generates schema fields that reference + SQL relationship fields that may not exist in marshmallow 4.x's stricter + field validation. This patch dynamically adds missing fields as Raw fields + to prevent KeyError exceptions during schema initialization. + """ + import marshmallow + + # Store the original method + original_init_fields = marshmallow.Schema._init_fields + + def patched_init_fields(self): + """Patched version that handles missing declared fields.""" + max_retries = 10 # Prevent infinite loops in case of unexpected errors + retries = 0 + + while retries < max_retries: + try: + return original_init_fields(self) + except KeyError as e: + # Extract the missing field name from the KeyError + missing_field = str(e).strip("'\"") + + # Initialize declared_fields if it doesn't exist + if not hasattr(self, "declared_fields"): + self.declared_fields = {} + + # Only add if it doesn't already exist + if missing_field not in self.declared_fields: + # Use Raw field as a safe fallback for unknown auto-generated fields + self.declared_fields[missing_field] = fields.Raw( + allow_none=True, + dump_only=True, # Prevent validation issues during serialization + ) + + print( + f"Marshmallow compatibility: Added missing field " + f"'{missing_field}' as Raw field" + ) + + retries += 1 + # Continue the loop to retry initialization + except Exception: + # For any other type of error, just propagate it + raise + + # If we've exhausted retries, something is seriously wrong + raise RuntimeError( + f"Marshmallow field initialization failed after {max_retries} retries" + ) + + # Apply the patch + marshmallow.Schema._init_fields = patched_init_fields From 4194c43779530a3ef6446b81327543caf3911f40 Mon Sep 17 00:00:00 2001 From: FLAME Date: Mon, 27 Oct 2025 22:00:44 -0400 Subject: [PATCH 03/13] Add unit tests --- tests/unit_tests/test_marshmallow_fix.py | 197 +++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 tests/unit_tests/test_marshmallow_fix.py diff --git a/tests/unit_tests/test_marshmallow_fix.py b/tests/unit_tests/test_marshmallow_fix.py new file mode 100644 index 000000000000..27505852b1a3 --- /dev/null +++ b/tests/unit_tests/test_marshmallow_fix.py @@ -0,0 +1,197 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Unit tests for marshmallow 4.x compatibility fix. + +This module tests the marshmallow_fix.py module that provides compatibility +between Flask-AppBuilder 5.0.0 and marshmallow 4.x by handling missing +auto-generated fields during schema initialization. +""" + +from unittest.mock import patch + +import pytest +from marshmallow import Schema, fields + +from superset.marshmallow_fix import patch_marshmallow_for_flask_appbuilder + + +class TestMarshmallowFix: + """Test cases for the marshmallow 4.x compatibility fix.""" + + def test_patch_marshmallow_for_flask_appbuilder_applies_patch(self): + """Test that the patch function correctly replaces Schema._init_fields.""" + # Store original method + original_method = Schema._init_fields + + # Apply patch + patch_marshmallow_for_flask_appbuilder() + + # Verify the method was replaced + assert Schema._init_fields != original_method + assert callable(Schema._init_fields) + + # Restore original for other tests + Schema._init_fields = original_method + + def test_patch_functionality_with_real_schema_creation(self): + """Test that the patch works with actual schema creation scenarios.""" + # Store original method + original_method = Schema._init_fields + + try: + # Apply the patch + patch_marshmallow_for_flask_appbuilder() + + # Create a simple schema - this should work without errors + class TestSchema(Schema): + name = fields.Str() + age = fields.Int() + + # Schema creation should succeed + schema = TestSchema() + assert "name" in schema.declared_fields + assert "age" in schema.declared_fields + assert isinstance(schema.declared_fields["name"], fields.Str) + assert isinstance(schema.declared_fields["age"], fields.Int) + + finally: + # Restore original method + Schema._init_fields = original_method + + def test_patch_handles_schema_with_no_fields(self): + """Test that the patch works with schemas that have no declared fields.""" + # Store original method + original_method = Schema._init_fields + + try: + # Apply the patch + patch_marshmallow_for_flask_appbuilder() + + # Create an empty schema + class EmptySchema(Schema): + pass + + # Schema creation should succeed + schema = EmptySchema() + # Should have at least a declared_fields attribute + assert hasattr(schema, "declared_fields") + + finally: + # Restore original method + Schema._init_fields = original_method + + def test_raw_field_creation_and_configuration(self): + """Test that Raw fields can be created with the expected configuration.""" + # Test creating a Raw field with our configuration + raw_field = fields.Raw(allow_none=True, dump_only=True) + + assert isinstance(raw_field, fields.Raw) + assert raw_field.allow_none is True + assert raw_field.dump_only is True + + @patch("builtins.print") + def test_print_function_can_be_mocked(self, mock_print): + """Test that print function can be mocked (for testing log output).""" + test_message = ( + "Marshmallow compatibility: Added missing field 'test' as Raw field" + ) + print(test_message) + mock_print.assert_called_once_with(test_message) + + def test_keyerror_exception_handling(self): + """Test that KeyError exceptions can be caught and handled.""" + try: + raise KeyError("test_field") + except KeyError as e: + # Verify we can extract the field name + field_name = str(e).strip("'\"") + assert field_name == "test_field" + + def test_schema_declared_fields_manipulation(self): + """Test that we can manipulate schema declared_fields.""" + + class TestSchema(Schema): + existing_field = fields.Str() + + schema = TestSchema() + + # Verify initial state + assert "existing_field" in schema.declared_fields + assert isinstance(schema.declared_fields["existing_field"], fields.Str) + + # Test adding a new field + schema.declared_fields["new_field"] = fields.Raw( + allow_none=True, dump_only=True + ) + + # Verify the new field was added + assert "new_field" in schema.declared_fields + assert isinstance(schema.declared_fields["new_field"], fields.Raw) + assert schema.declared_fields["new_field"].allow_none is True + assert schema.declared_fields["new_field"].dump_only is True + + def test_flask_appbuilder_field_names_list(self): + """Test that we have the correct list of Flask-AppBuilder field names.""" + # Common Flask-AppBuilder auto-generated field names that our fix handles + expected_fab_fields = [ + "permission_id", + "view_menu_id", + "db_id", + "chart_id", + "dashboard_id", + "user_id", + ] + + # Verify these are strings (field names) + for field_name in expected_fab_fields: + assert isinstance(field_name, str) + assert len(field_name) > 0 + assert "_id" in field_name + + def test_patch_function_is_callable(self): + """Test that the patch function can be called without errors.""" + # This should not raise any exceptions + patch_marshmallow_for_flask_appbuilder() + + # Calling it multiple times should also be safe + patch_marshmallow_for_flask_appbuilder() + patch_marshmallow_for_flask_appbuilder() + + def test_marshmallow_schema_basic_functionality(self): + """Test basic marshmallow schema functionality still works.""" + + class UserSchema(Schema): + name = fields.Str(required=True) + email = fields.Email() + age = fields.Int(validate=lambda x: x > 0) + + schema = UserSchema() + + # Test serialization + data = {"name": "John Doe", "email": "john@example.com", "age": 30} + result = schema.load(data) + assert result["name"] == "John Doe" + assert result["email"] == "john@example.com" + assert result["age"] == 30 + + from marshmallow import ValidationError + + # Test validation - missing required field should raise error + with pytest.raises(ValidationError): + schema.load({"email": "john@example.com", "age": 30}) # Missing name From 6130614236c6d0519654f2cceb32fc964dae6328 Mon Sep 17 00:00:00 2001 From: FLAME Date: Mon, 27 Oct 2025 22:11:08 -0400 Subject: [PATCH 04/13] Rename "marshmallow_fix" to "marshmallow_compatibility" --- superset/marshmallow_fix.py | 68 ------------------- ...x.py => test_marshmallow_compatibility.py} | 10 +-- 2 files changed, 5 insertions(+), 73 deletions(-) delete mode 100644 superset/marshmallow_fix.py rename tests/unit_tests/{test_marshmallow_fix.py => test_marshmallow_compatibility.py} (95%) diff --git a/superset/marshmallow_fix.py b/superset/marshmallow_fix.py deleted file mode 100644 index 99a6da3716c5..000000000000 --- a/superset/marshmallow_fix.py +++ /dev/null @@ -1,68 +0,0 @@ -""" -Marshmallow 4.x Compatibility Fix for Flask-AppBuilder 5.0.0 - -This module provides a targeted fix for incompatibilities between -Flask-AppBuilder 5.0.0 and marshmallow 4.x, specifically handling -missing auto-generated fields during schema initialization. -""" - -from marshmallow import fields - - -def patch_marshmallow_for_flask_appbuilder(): - """ - Patches marshmallow Schema._init_fields to handle Flask-AppBuilder 5.0.0 - compatibility with marshmallow 4.x. - - Flask-AppBuilder 5.0.0 automatically generates schema fields that reference - SQL relationship fields that may not exist in marshmallow 4.x's stricter - field validation. This patch dynamically adds missing fields as Raw fields - to prevent KeyError exceptions during schema initialization. - """ - import marshmallow - - # Store the original method - original_init_fields = marshmallow.Schema._init_fields - - def patched_init_fields(self): - """Patched version that handles missing declared fields.""" - max_retries = 10 # Prevent infinite loops in case of unexpected errors - retries = 0 - - while retries < max_retries: - try: - return original_init_fields(self) - except KeyError as e: - # Extract the missing field name from the KeyError - missing_field = str(e).strip("'\"") - - # Initialize declared_fields if it doesn't exist - if not hasattr(self, "declared_fields"): - self.declared_fields = {} - - # Only add if it doesn't already exist - if missing_field not in self.declared_fields: - # Use Raw field as a safe fallback for unknown auto-generated fields - self.declared_fields[missing_field] = fields.Raw( - allow_none=True, - dump_only=True, # Prevent validation issues during serialization - ) - - print( - f"Marshmallow compatibility: Added missing field " - f"'{missing_field}' as Raw field" - ) - - retries += 1 - # Continue the loop to retry initialization - except Exception: - # For any other type of error, just propagate it - raise - - # If we've exhausted retries, something is seriously wrong - raise RuntimeError( - f"Marshmallow field initialization failed after {max_retries} retries" - ) - - # Apply the patch - marshmallow.Schema._init_fields = patched_init_fields diff --git a/tests/unit_tests/test_marshmallow_fix.py b/tests/unit_tests/test_marshmallow_compatibility.py similarity index 95% rename from tests/unit_tests/test_marshmallow_fix.py rename to tests/unit_tests/test_marshmallow_compatibility.py index 27505852b1a3..57f2170f7f6d 100644 --- a/tests/unit_tests/test_marshmallow_fix.py +++ b/tests/unit_tests/test_marshmallow_compatibility.py @@ -16,9 +16,9 @@ # under the License. """ -Unit tests for marshmallow 4.x compatibility fix. +Unit tests for marshmallow 4.x compatibility module. -This module tests the marshmallow_fix.py module that provides compatibility +This module tests the marshmallow_compatibility.py module that provides compatibility between Flask-AppBuilder 5.0.0 and marshmallow 4.x by handling missing auto-generated fields during schema initialization. """ @@ -28,11 +28,11 @@ import pytest from marshmallow import Schema, fields -from superset.marshmallow_fix import patch_marshmallow_for_flask_appbuilder +from superset.marshmallow_compatibility import patch_marshmallow_for_flask_appbuilder -class TestMarshmallowFix: - """Test cases for the marshmallow 4.x compatibility fix.""" +class TestMarshmallowCompatibility: + """Test cases for the marshmallow 4.x compatibility module.""" def test_patch_marshmallow_for_flask_appbuilder_applies_patch(self): """Test that the patch function correctly replaces Schema._init_fields.""" From 0d0b541c2f1c6795c97518f2749d748fa83f205d Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Sun, 2 Nov 2025 17:52:18 -0500 Subject: [PATCH 05/13] Move patch fn to app.py --- superset/app.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/superset/app.py b/superset/app.py index 391d633488d3..a9e73bbc203f 100644 --- a/superset/app.py +++ b/superset/app.py @@ -47,6 +47,16 @@ logger = logging.getLogger(__name__) +# Apply marshmallow 4.x compatibility patch for Flask-AppBuilder +try: + from superset.marshmallow_compatibility import ( + patch_marshmallow_for_flask_appbuilder, + ) + + patch_marshmallow_for_flask_appbuilder() +except ImportError: + logger.debug("marshmallow_compatibility module not found, skipping patch") + def create_app( superset_config_module: Optional[str] = None, From 501d2585cd3588a1719d9daf9b606abbd4b9698c Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Mon, 3 Nov 2025 09:04:07 -0500 Subject: [PATCH 06/13] empty From 5b677926ca9aad28d942ca5107a0a21289e7a1fd Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Wed, 12 Nov 2025 19:09:59 -0500 Subject: [PATCH 07/13] Fix validates decorator --- superset/models/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 72ce09a390d9..2bd5c0d593ae 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -976,6 +976,7 @@ def ensure_extra_json_is_not_none( self, _: str, value: Optional[dict[str, Any]], + **kwargs: Any, ) -> Any: if value is None: return "{}" From 04c6bf20ea0378d796ca22dce585fd822c1bb328 Mon Sep 17 00:00:00 2001 From: Eyang0612 Date: Sat, 22 Nov 2025 22:06:45 -0500 Subject: [PATCH 08/13] Apply JSON Schema Change --- superset/advanced_data_type/schemas.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/advanced_data_type/schemas.py b/superset/advanced_data_type/schemas.py index 47918e5416f0..d94fd19873b5 100644 --- a/superset/advanced_data_type/schemas.py +++ b/superset/advanced_data_type/schemas.py @@ -23,10 +23,10 @@ advanced_data_type_convert_schema = { "type": "object", "properties": { - "type": {"type": "string", "dump_default": "port"}, + "type": {"type": "string", "default": "port"}, "values": { "type": "array", - "items": {"dump_default": "http"}, + "items": {"default": "http"}, "minItems": 1, }, }, From 0b07fe359f9dba9d5f04ed507f62443060b9c332 Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Sun, 23 Nov 2025 19:21:57 -0500 Subject: [PATCH 09/13] Allow optional fields to load as None --- superset/marshmallow_compatibility.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/marshmallow_compatibility.py b/superset/marshmallow_compatibility.py index 852627b40854..7aa06014142b 100644 --- a/superset/marshmallow_compatibility.py +++ b/superset/marshmallow_compatibility.py @@ -72,7 +72,10 @@ def patched_init_fields(self: "marshmallow.Schema") -> Any: name, type(self).__name__, ) - self.declared_fields[name] = marshmallow.fields.Raw() + self.declared_fields[name] = marshmallow.fields.Raw( + allow_none=True, + load_default=None, + ) return original_init_fields(self) patched_init_fields._fab_patched = True # type: ignore[attr-defined] From 999594045be8a1120e0c76dc3c15416f9ce2fe66 Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Mon, 1 Jun 2026 18:38:40 -0400 Subject: [PATCH 10/13] Fix ruff --- tests/unit_tests/test_marshmallow_compatibility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_marshmallow_compatibility.py b/tests/unit_tests/test_marshmallow_compatibility.py index 57f2170f7f6d..745b2f56f741 100644 --- a/tests/unit_tests/test_marshmallow_compatibility.py +++ b/tests/unit_tests/test_marshmallow_compatibility.py @@ -26,7 +26,7 @@ from unittest.mock import patch import pytest -from marshmallow import Schema, fields +from marshmallow import fields, Schema from superset.marshmallow_compatibility import patch_marshmallow_for_flask_appbuilder From 5258a00545f0a4a9ada23d7562e1f7f3714dab02 Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Mon, 1 Jun 2026 20:09:43 -0400 Subject: [PATCH 11/13] address bot review --- superset/marshmallow_compatibility.py | 11 +- superset/themes/schemas.py | 29 ++++- .../test_marshmallow_compatibility.py | 117 ++++++++---------- tests/unit_tests/themes/api_test.py | 45 ++++++- 4 files changed, 127 insertions(+), 75 deletions(-) diff --git a/superset/marshmallow_compatibility.py b/superset/marshmallow_compatibility.py index 7aa06014142b..e15028abf888 100644 --- a/superset/marshmallow_compatibility.py +++ b/superset/marshmallow_compatibility.py @@ -52,7 +52,9 @@ def patch_marshmallow_for_flask_appbuilder() -> None: """ import marshmallow - if getattr(marshmallow.Schema._init_fields, "_fab_patched", False): + if getattr(marshmallow.Schema._init_fields, "_fab_patched", False) or getattr( + marshmallow.Schema._init_fields, "__superset_compat_patched__", False + ): return # already patched original_init_fields = marshmallow.Schema._init_fields @@ -65,20 +67,23 @@ def patched_init_fields(self: "marshmallow.Schema") -> Any: # otherwise choke on; unknown non-FAB names are left to raise so genuine # schema bugs still surface. candidates = set(opts.fields or ()) | set(getattr(opts, "additional", ()) or ()) + declared_fields = dict(getattr(self, "declared_fields", {})) for name in candidates: - if name not in self.declared_fields and _looks_like_fab_field(name): + if name not in declared_fields and _looks_like_fab_field(name): logger.debug( "marshmallow FAB compat: stubbing Raw field for %r on %s", name, type(self).__name__, ) - self.declared_fields[name] = marshmallow.fields.Raw( + declared_fields[name] = marshmallow.fields.Raw( allow_none=True, load_default=None, ) + self.declared_fields = declared_fields return original_init_fields(self) patched_init_fields._fab_patched = True # type: ignore[attr-defined] + patched_init_fields.__superset_compat_patched__ = True # type: ignore[attr-defined] marshmallow.Schema._init_fields = patched_init_fields diff --git a/superset/themes/schemas.py b/superset/themes/schemas.py index cc9b9ea03c94..099586d025c4 100644 --- a/superset/themes/schemas.py +++ b/superset/themes/schemas.py @@ -17,7 +17,7 @@ from contextvars import ContextVar from typing import Any -from marshmallow import fields, Schema, validates, ValidationError +from marshmallow import fields, post_load, Schema, validates, ValidationError from superset.themes.utils import ( is_valid_theme, @@ -32,6 +32,12 @@ ) +def _consume_sanitized_json_data(default: str) -> str: + sanitized_json_data = sanitized_json_context.get() + sanitized_json_context.set(None) + return sanitized_json_data or default + + def _sanitize_and_validate_theme_config(theme_config: dict[str, Any]) -> dict[str, Any]: """Sanitize and validate theme configuration. @@ -97,6 +103,7 @@ def validate_theme_name(self, value: str, **kwargs: Any) -> None: @validates("json_data") def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: + sanitized_json_context.set(None) # Parse JSON try: theme_config = json.loads(value) if isinstance(value, str) else value @@ -112,6 +119,17 @@ def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: # Re-serialize the sanitized config sanitized_json_context.set(json.dumps(sanitized_config)) + def _apply_sanitized_json_data(self, data: dict[str, Any]) -> dict[str, Any]: + data["json_data"] = _consume_sanitized_json_data(data["json_data"]) + return data + + @post_load + def apply_sanitized_json_data( + self, data: dict[str, Any], **kwargs: Any + ) -> dict[str, Any]: + del kwargs + return self._apply_sanitized_json_data(data) + class ThemePutSchema(Schema): theme_name = fields.String(required=True, allow_none=False) @@ -124,6 +142,7 @@ def validate_theme_name(self, value: str, **kwargs: Any) -> None: @validates("json_data") def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: + sanitized_json_context.set(None) # Parse JSON try: theme_config = json.loads(value) if isinstance(value, str) else value @@ -139,6 +158,14 @@ def validate_and_sanitize_json_data(self, value: str, **kwargs: Any) -> None: # Re-serialize the sanitized config sanitized_json_context.set(json.dumps(sanitized_config)) + @post_load + def apply_sanitized_json_data( + self, data: dict[str, Any], **kwargs: Any + ) -> dict[str, Any]: + del kwargs + data["json_data"] = _consume_sanitized_json_data(data["json_data"]) + return data + openapi_spec_methods_override = { "get": {"get": {"summary": "Get a theme"}}, diff --git a/tests/unit_tests/test_marshmallow_compatibility.py b/tests/unit_tests/test_marshmallow_compatibility.py index 745b2f56f741..5eece333caa4 100644 --- a/tests/unit_tests/test_marshmallow_compatibility.py +++ b/tests/unit_tests/test_marshmallow_compatibility.py @@ -15,13 +15,7 @@ # specific language governing permissions and limitations # under the License. -""" -Unit tests for marshmallow 4.x compatibility module. - -This module tests the marshmallow_compatibility.py module that provides compatibility -between Flask-AppBuilder 5.0.0 and marshmallow 4.x by handling missing -auto-generated fields during schema initialization. -""" +"""Unit tests for the marshmallow compatibility shim.""" from unittest.mock import patch @@ -31,70 +25,74 @@ from superset.marshmallow_compatibility import patch_marshmallow_for_flask_appbuilder +@pytest.fixture(autouse=True) +def restore_schema_init_fields(): + """Restore Schema._init_fields after each test.""" + original_method = Schema._init_fields + try: + yield + finally: + Schema._init_fields = original_method + + class TestMarshmallowCompatibility: """Test cases for the marshmallow 4.x compatibility module.""" def test_patch_marshmallow_for_flask_appbuilder_applies_patch(self): """Test that the patch function correctly replaces Schema._init_fields.""" - # Store original method original_method = Schema._init_fields - # Apply patch patch_marshmallow_for_flask_appbuilder() - # Verify the method was replaced - assert Schema._init_fields != original_method assert callable(Schema._init_fields) - - # Restore original for other tests - Schema._init_fields = original_method + assert getattr(Schema._init_fields, "__superset_compat_patched__", False) + if not getattr(original_method, "__superset_compat_patched__", False): + assert Schema._init_fields != original_method def test_patch_functionality_with_real_schema_creation(self): """Test that the patch works with actual schema creation scenarios.""" - # Store original method - original_method = Schema._init_fields - - try: - # Apply the patch - patch_marshmallow_for_flask_appbuilder() - - # Create a simple schema - this should work without errors - class TestSchema(Schema): - name = fields.Str() - age = fields.Int() + patch_marshmallow_for_flask_appbuilder() - # Schema creation should succeed - schema = TestSchema() - assert "name" in schema.declared_fields - assert "age" in schema.declared_fields - assert isinstance(schema.declared_fields["name"], fields.Str) - assert isinstance(schema.declared_fields["age"], fields.Int) + class TestSchema(Schema): + name = fields.Str() + age = fields.Int() - finally: - # Restore original method - Schema._init_fields = original_method + schema = TestSchema() + assert "name" in schema.declared_fields + assert "age" in schema.declared_fields + assert isinstance(schema.declared_fields["name"], fields.Str) + assert isinstance(schema.declared_fields["age"], fields.Int) def test_patch_handles_schema_with_no_fields(self): """Test that the patch works with schemas that have no declared fields.""" - # Store original method - original_method = Schema._init_fields + patch_marshmallow_for_flask_appbuilder() + + class EmptySchema(Schema): + pass + + schema = EmptySchema() + assert hasattr(schema, "declared_fields") - try: - # Apply the patch + def test_patch_adds_missing_field_and_retries_initialization(self): + """Test that missing fields are injected as Raw fields and retried.""" + + def fake_init_fields(self: Schema): + if "missing_field" not in self.declared_fields: + raise KeyError("missing_field") + return "initialized" + + with patch.object(Schema, "_init_fields", fake_init_fields): patch_marshmallow_for_flask_appbuilder() - # Create an empty schema - class EmptySchema(Schema): - pass + schema = object.__new__(Schema) + schema.declared_fields = {} - # Schema creation should succeed - schema = EmptySchema() - # Should have at least a declared_fields attribute - assert hasattr(schema, "declared_fields") + result = Schema._init_fields(schema) - finally: - # Restore original method - Schema._init_fields = original_method + assert result == "initialized" + assert isinstance(schema.declared_fields["missing_field"], fields.Raw) + assert schema.declared_fields["missing_field"].allow_none is True + assert schema.declared_fields["missing_field"].load_default is None def test_raw_field_creation_and_configuration(self): """Test that Raw fields can be created with the expected configuration.""" @@ -105,24 +103,6 @@ def test_raw_field_creation_and_configuration(self): assert raw_field.allow_none is True assert raw_field.dump_only is True - @patch("builtins.print") - def test_print_function_can_be_mocked(self, mock_print): - """Test that print function can be mocked (for testing log output).""" - test_message = ( - "Marshmallow compatibility: Added missing field 'test' as Raw field" - ) - print(test_message) - mock_print.assert_called_once_with(test_message) - - def test_keyerror_exception_handling(self): - """Test that KeyError exceptions can be caught and handled.""" - try: - raise KeyError("test_field") - except KeyError as e: - # Verify we can extract the field name - field_name = str(e).strip("'\"") - assert field_name == "test_field" - def test_schema_declared_fields_manipulation(self): """Test that we can manipulate schema declared_fields.""" @@ -166,13 +146,14 @@ def test_flask_appbuilder_field_names_list(self): def test_patch_function_is_callable(self): """Test that the patch function can be called without errors.""" - # This should not raise any exceptions patch_marshmallow_for_flask_appbuilder() + patched_method = Schema._init_fields - # Calling it multiple times should also be safe patch_marshmallow_for_flask_appbuilder() patch_marshmallow_for_flask_appbuilder() + assert Schema._init_fields is patched_method + def test_marshmallow_schema_basic_functionality(self): """Test basic marshmallow schema functionality still works.""" diff --git a/tests/unit_tests/themes/api_test.py b/tests/unit_tests/themes/api_test.py index fad439fcceb5..d74ea30b02a1 100644 --- a/tests/unit_tests/themes/api_test.py +++ b/tests/unit_tests/themes/api_test.py @@ -15,8 +15,9 @@ # specific language governing permissions and limitations # under the License. - from superset.themes.api import ThemeRestApi +from superset.themes.schemas import ThemePostSchema, ThemePutSchema +from superset.utils import json class TestThemeRestApi: @@ -104,8 +105,6 @@ def test_bulk_delete_enabled(self): def test_custom_schemas_configured(self): """Test that custom schemas are properly configured""" - from superset.themes.schemas import ThemePostSchema, ThemePutSchema - api = ThemeRestApi() assert isinstance(api.add_model_schema, ThemePostSchema) assert isinstance(api.edit_model_schema, ThemePutSchema) @@ -121,3 +120,43 @@ def test_list_columns_include_new_fields(self): expected_new_fields = ["is_system", "uuid"] for field in expected_new_fields: assert field in ThemeRestApi.list_columns + + def test_theme_post_schema_uses_sanitized_json_data(self): + """Test that sanitized theme JSON is written back during load.""" + schema = ThemePostSchema() + + result = schema.load( + { + "theme_name": "test_theme", + "json_data": json.dumps( + { + "token": { + "brandSpinnerUrl": "javascript:alert('xss')", + "colorPrimary": "#ff0000", + } + } + ), + } + ) + + assert json.loads(result["json_data"])["token"]["brandSpinnerUrl"] == "" + + def test_theme_put_schema_uses_sanitized_json_data(self): + """Test that update schema also writes back sanitized theme JSON.""" + schema = ThemePutSchema() + + result = schema.load( + { + "theme_name": "test_theme", + "json_data": json.dumps( + { + "token": { + "brandSpinnerUrl": "javascript:alert('xss')", + "colorPrimary": "#ff0000", + } + } + ), + } + ) + + assert json.loads(result["json_data"])["token"]["brandSpinnerUrl"] == "" From a1d81d06529f51c10b6101d262320a86562f2fce Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Thu, 25 Jun 2026 10:52:49 -0400 Subject: [PATCH 12/13] remove duplicate patch call in app.py --- superset/app.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/superset/app.py b/superset/app.py index a9e73bbc203f..391d633488d3 100644 --- a/superset/app.py +++ b/superset/app.py @@ -47,16 +47,6 @@ logger = logging.getLogger(__name__) -# Apply marshmallow 4.x compatibility patch for Flask-AppBuilder -try: - from superset.marshmallow_compatibility import ( - patch_marshmallow_for_flask_appbuilder, - ) - - patch_marshmallow_for_flask_appbuilder() -except ImportError: - logger.debug("marshmallow_compatibility module not found, skipping patch") - def create_app( superset_config_module: Optional[str] = None, From 1c04644e30e0dc73a5f4e89b12fc7c6bb3338e7d Mon Sep 17 00:00:00 2001 From: Your Full Name Date: Thu, 25 Jun 2026 10:55:51 -0400 Subject: [PATCH 13/13] fix unit test --- .../test_marshmallow_compatibility.py | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/test_marshmallow_compatibility.py b/tests/unit_tests/test_marshmallow_compatibility.py index 5eece333caa4..8a85c4b98b0d 100644 --- a/tests/unit_tests/test_marshmallow_compatibility.py +++ b/tests/unit_tests/test_marshmallow_compatibility.py @@ -73,12 +73,11 @@ class EmptySchema(Schema): schema = EmptySchema() assert hasattr(schema, "declared_fields") - def test_patch_adds_missing_field_and_retries_initialization(self): - """Test that missing fields are injected as Raw fields and retried.""" + def test_patch_pre_stubs_missing_fab_fields_before_initialization(self): + """Test that FAB-looking missing fields are stubbed before init runs.""" def fake_init_fields(self: Schema): - if "missing_field" not in self.declared_fields: - raise KeyError("missing_field") + assert "missing_field" in self.declared_fields return "initialized" with patch.object(Schema, "_init_fields", fake_init_fields): @@ -86,6 +85,11 @@ def fake_init_fields(self: Schema): schema = object.__new__(Schema) schema.declared_fields = {} + schema.opts = type( + "Opts", + (), + {"fields": ("missing_field",), "additional": ()}, + )() result = Schema._init_fields(schema) @@ -97,11 +101,11 @@ def fake_init_fields(self: Schema): def test_raw_field_creation_and_configuration(self): """Test that Raw fields can be created with the expected configuration.""" # Test creating a Raw field with our configuration - raw_field = fields.Raw(allow_none=True, dump_only=True) + raw_field = fields.Raw(allow_none=True, load_default=None) assert isinstance(raw_field, fields.Raw) assert raw_field.allow_none is True - assert raw_field.dump_only is True + assert raw_field.load_default is None def test_schema_declared_fields_manipulation(self): """Test that we can manipulate schema declared_fields.""" @@ -117,14 +121,14 @@ class TestSchema(Schema): # Test adding a new field schema.declared_fields["new_field"] = fields.Raw( - allow_none=True, dump_only=True + allow_none=True, load_default=None ) # Verify the new field was added assert "new_field" in schema.declared_fields assert isinstance(schema.declared_fields["new_field"], fields.Raw) assert schema.declared_fields["new_field"].allow_none is True - assert schema.declared_fields["new_field"].dump_only is True + assert schema.declared_fields["new_field"].load_default is None def test_flask_appbuilder_field_names_list(self): """Test that we have the correct list of Flask-AppBuilder field names."""