From fd2bf052899902d1087e234794620d4dfd2e390d Mon Sep 17 00:00:00 2001 From: AdamPenaz Date: Tue, 10 Nov 2026 10:24:07 +0100 Subject: [PATCH 1/4] fix(gooddata-api-client): accept primitive-only oneOf union values A declarative read failed with `ApiTypeError: Invalid type for variable 'gap'` once any Publisher document, document template or page template in the workspace carried a style value such as `gap: 20` or `padding: "{theme.spacing.padding.normal}"`. The write paths (from_dict, load from disk, to_api, put) failed the same way. The spec types these fields as a oneOf of two primitives. python-prior v6.6.0 renders such a schema as an empty model class that accepts no value, so all seven of them were broken: the publisher style gap, padding and border radius, a visualization's auto-visibility, the publisher date filter bounds, the dashboard tabular export date filter bounds and the conditional-formatting literal. The fix is in the custom templates, so it survives regeneration: - an override of the normal-model partial marks a primitive-only oneOf model with its member types (a non-empty oneOf list is the only thing telling it apart from a genuinely empty object model); - model_utils validates such a property against the member types, also admits an int on a union that contains float, and skips the int-to-float upconversion for these unions only, so 20 stays 20 through a read, the constructor and the wire. A wrong type, such as a dict, a list or a bool on a number union, is still rejected. Empty object models and plain number properties behave as before. The client is regenerated from the specs already in schemas/, so the generated diff is the seven union models and model_utils alone. The override is a full copy of the upstream partial and must be reconciled on a generator upgrade; a move to generator v7's python generator, which emits real unions, would retire it. jira: LX-3540 risk: low Co-Authored-By: Claude Opus 5.5 --- .../model_templates/model_normal.mustache | 57 +++++++ .../custom_templates/model_utils.mustache | 14 +- .../model/conditional_formatting_literal.py | 7 + .../dashboard_date_filter_date_filter_from.py | 7 + .../publisher_document_area_style_gap.py | 7 + ...lisher_document_auto_visibility_visible.py | 7 + ...ntainer_style_border_bottom_left_radius.py | 7 + ...lisher_document_container_style_padding.py | 7 + ...r_document_date_filter_date_filter_from.py | 7 + .../gooddata_api_client/model_utils.py | 14 +- .../unit_tests/test_composed_oneof_unions.py | 154 ++++++++++++++++++ 11 files changed, 286 insertions(+), 2 deletions(-) create mode 100644 .openapi-generator/custom_templates/model_templates/model_normal.mustache diff --git a/.openapi-generator/custom_templates/model_templates/model_normal.mustache b/.openapi-generator/custom_templates/model_templates/model_normal.mustache new file mode 100644 index 0000000000..e6ad5c2355 --- /dev/null +++ b/.openapi-generator/custom_templates/model_templates/model_normal.mustache @@ -0,0 +1,57 @@ +{{! A full copy of the upstream python-prior v6.6.0 partial plus the _primitive_oneof_types marker. Reconcile it with the upstream partial on every generator upgrade. }} +class {{classname}}(ModelNormal): + """NOTE: This class is auto generated by OpenAPI Generator. + Ref: https://openapi-generator.tech + + Do not edit the class manually. + + Attributes: +{{> model_templates/docstring_allowed }} + attribute_map (dict): The key is attribute name + and the value is json key in definition. + discriminator_value_class_map (dict): A dict to go from the discriminator + variable value to the discriminator class name. +{{> model_templates/docstring_openapi_validations }} + """ + +{{> model_templates/classvars }} + + attribute_map = { +{{#requiredVars}} + '{{name}}': '{{baseName}}', # noqa: E501 +{{/requiredVars}} +{{#optionalVars}} + '{{name}}': '{{baseName}}', # noqa: E501 +{{/optionalVars}} + } + + read_only_vars = { +{{#requiredVars}} +{{#isReadOnly}} + '{{name}}', # noqa: E501 +{{/isReadOnly}} +{{/requiredVars}} +{{#optionalVars}} +{{#isReadOnly}} + '{{name}}', # noqa: E501 +{{/isReadOnly}} +{{/optionalVars}} + } + + _composed_schemas = {} + +{{#oneOf}} +{{#-first}} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( +{{/-first}} + {{{.}}}, +{{#-last}} + ) + +{{/-last}} +{{/oneOf}} +{{> model_templates/method_from_openapi_data_normal}} + +{{> model_templates/method_init_normal}} \ No newline at end of file diff --git a/.openapi-generator/custom_templates/model_utils.mustache b/.openapi-generator/custom_templates/model_utils.mustache index a73fe96ecc..94338b2117 100644 --- a/.openapi-generator/custom_templates/model_utils.mustache +++ b/.openapi-generator/custom_templates/model_utils.mustache @@ -881,6 +881,12 @@ def get_discriminated_classes(cls): def get_possible_classes(cls, from_server_context): # TODO: lru_cache this possible_classes = [cls] + primitive_oneof_types = getattr(cls, '_primitive_oneof_types', None) + if primitive_oneof_types: + # an OpenAPI number also admits integers + if float in primitive_oneof_types and int not in primitive_oneof_types: + return [int, *primitive_oneof_types] + return list(primitive_oneof_types) if from_server_context: return possible_classes if hasattr(cls, 'discriminator') and cls.discriminator is not None: @@ -1340,7 +1346,13 @@ def validate_and_convert_types(input_value, required_types_mixed, path_to_item, key_type=False) # input_value's type is in valid_classes - if len(valid_classes) > 1 and configuration: + # A value that already matches a primitive oneOf member is kept as it is, + # so an int on a number|string union is not upconverted to a float. This + # also skips every other conversion for such a union: a future union with + # a `format: date` string member would keep its value a str, not a date. + is_primitive_union = any( + getattr(t, '_primitive_oneof_types', None) for t in required_types_mixed if inspect.isclass(t)) + if len(valid_classes) > 1 and configuration and not is_primitive_union: # there are valid classes which are not the current class valid_classes_coercible = remove_uncoercible( valid_classes, input_value, spec_property_naming, must_convert=False) diff --git a/gooddata-api-client/gooddata_api_client/model/conditional_formatting_literal.py b/gooddata-api-client/gooddata_api_client/model/conditional_formatting_literal.py index 03ba372b19..245afa22a6 100644 --- a/gooddata-api-client/gooddata_api_client/model/conditional_formatting_literal.py +++ b/gooddata-api-client/gooddata_api_client/model/conditional_formatting_literal.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + float, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/dashboard_date_filter_date_filter_from.py b/gooddata-api-client/gooddata_api_client/model/dashboard_date_filter_date_filter_from.py index 06393e0759..04af7fbe95 100644 --- a/gooddata-api-client/gooddata_api_client/model/dashboard_date_filter_date_filter_from.py +++ b/gooddata-api-client/gooddata_api_client/model/dashboard_date_filter_date_filter_from.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + int, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/publisher_document_area_style_gap.py b/gooddata-api-client/gooddata_api_client/model/publisher_document_area_style_gap.py index 91f1295e16..9dcd8c7004 100644 --- a/gooddata-api-client/gooddata_api_client/model/publisher_document_area_style_gap.py +++ b/gooddata-api-client/gooddata_api_client/model/publisher_document_area_style_gap.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + float, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/publisher_document_auto_visibility_visible.py b/gooddata-api-client/gooddata_api_client/model/publisher_document_auto_visibility_visible.py index 8ee0034a0e..d96bcdca0b 100644 --- a/gooddata-api-client/gooddata_api_client/model/publisher_document_auto_visibility_visible.py +++ b/gooddata-api-client/gooddata_api_client/model/publisher_document_auto_visibility_visible.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + bool, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_border_bottom_left_radius.py b/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_border_bottom_left_radius.py index fc327adac5..403f5a8006 100644 --- a/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_border_bottom_left_radius.py +++ b/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_border_bottom_left_radius.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + float, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_padding.py b/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_padding.py index d5611e5a9a..3c4e5a5277 100644 --- a/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_padding.py +++ b/gooddata-api-client/gooddata_api_client/model/publisher_document_container_style_padding.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + float, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model/publisher_document_date_filter_date_filter_from.py b/gooddata-api-client/gooddata_api_client/model/publisher_document_date_filter_date_filter_from.py index 1b234dff2a..5cfa7d3ed4 100644 --- a/gooddata-api-client/gooddata_api_client/model/publisher_document_date_filter_date_filter_from.py +++ b/gooddata-api-client/gooddata_api_client/model/publisher_document_date_filter_date_filter_from.py @@ -97,6 +97,13 @@ def discriminator(): _composed_schemas = {} + # A oneOf of primitive members only: values are validated against these + # types and kept as plain values, this class is never instantiated. + _primitive_oneof_types = ( + float, + str, + ) + @classmethod @convert_js_args_to_python_args def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 diff --git a/gooddata-api-client/gooddata_api_client/model_utils.py b/gooddata-api-client/gooddata_api_client/model_utils.py index 8663eaee81..e2ee362aed 100644 --- a/gooddata-api-client/gooddata_api_client/model_utils.py +++ b/gooddata-api-client/gooddata_api_client/model_utils.py @@ -1223,6 +1223,12 @@ def get_discriminated_classes(cls): def get_possible_classes(cls, from_server_context): # TODO: lru_cache this possible_classes = [cls] + primitive_oneof_types = getattr(cls, '_primitive_oneof_types', None) + if primitive_oneof_types: + # an OpenAPI number also admits integers + if float in primitive_oneof_types and int not in primitive_oneof_types: + return [int, *primitive_oneof_types] + return list(primitive_oneof_types) if from_server_context: return possible_classes if hasattr(cls, 'discriminator') and cls.discriminator is not None: @@ -1682,7 +1688,13 @@ def validate_and_convert_types(input_value, required_types_mixed, path_to_item, key_type=False) # input_value's type is in valid_classes - if len(valid_classes) > 1 and configuration: + # A value that already matches a primitive oneOf member is kept as it is, + # so an int on a number|string union is not upconverted to a float. This + # also skips every other conversion for such a union: a future union with + # a `format: date` string member would keep its value a str, not a date. + is_primitive_union = any( + getattr(t, '_primitive_oneof_types', None) for t in required_types_mixed if inspect.isclass(t)) + if len(valid_classes) > 1 and configuration and not is_primitive_union: # there are valid classes which are not the current class valid_classes_coercible = remove_uncoercible( valid_classes, input_value, spec_property_naming, must_convert=False) diff --git a/packages/gooddata-sdk/tests/catalog/unit_tests/test_composed_oneof_unions.py b/packages/gooddata-sdk/tests/catalog/unit_tests/test_composed_oneof_unions.py index 5119432ad7..c866fa3e94 100644 --- a/packages/gooddata-sdk/tests/catalog/unit_tests/test_composed_oneof_unions.py +++ b/packages/gooddata-sdk/tests/catalog/unit_tests/test_composed_oneof_unions.py @@ -19,19 +19,41 @@ The defect was repo-wide (25 collapsed enums and 47 collapsed types across 161 composed models), so it is worth suspecting whenever a `oneOf` variant is inexplicably rejected. + +A `oneOf` of primitive members only (`number|string`, `boolean|string`, +`integer|string`) is a different case: the generator emits an empty model class +for it. The custom templates mark such a class with its member types, and +`model_utils` validates the property against those members and keeps the value +as it is. The tests at the end of this module pin that for every such union. """ from __future__ import annotations +import json +from collections.abc import Callable +from dataclasses import dataclass, field +from types import SimpleNamespace from typing import Any import pytest +from gooddata_api_client.api_client import ApiClient +from gooddata_api_client.exceptions import ApiTypeError, ApiValueError +from gooddata_api_client.model.anomaly_detection_request import AnomalyDetectionRequest +from gooddata_api_client.model.column_location import ColumnLocation +from gooddata_api_client.model.conditional_formatting_literal_value import ConditionalFormattingLiteralValue +from gooddata_api_client.model.dashboard_date_filter_date_filter import DashboardDateFilterDateFilter from gooddata_api_client.model.declarative_publisher_document_page_template import ( DeclarativePublisherDocumentPageTemplate, ) from gooddata_api_client.model.json_api_llm_provider_in_attributes_provider_config import ( JsonApiLlmProviderInAttributesProviderConfig, ) +from gooddata_api_client.model.publisher_document_area_style import PublisherDocumentAreaStyle +from gooddata_api_client.model.publisher_document_auto_visibility import PublisherDocumentAutoVisibility +from gooddata_api_client.model.publisher_document_container_style import PublisherDocumentContainerStyle +from gooddata_api_client.model.publisher_document_date_filter_date_filter import ( + PublisherDocumentDateFilterDateFilter, +) from gooddata_api_client.model_utils import ( composed_union_allowed_values, composed_union_types, @@ -229,3 +251,135 @@ def test_image_layout_node_maps_python_names_to_json_names() -> None: layout = template.to_dict(camel_case=True)["content"]["layout"] assert layout == {"type": "image", "id": "i1", "altText": "A chart"} + + +@dataclass(frozen=True) +class _PrimitiveUnionSite: + """A property typed by a primitive-only `oneOf`, on a model that hosts it.""" + + model: type + python_name: str + json_name: str + members: tuple[Any, ...] + non_members: tuple[Any, ...] + required: dict[str, Any] = field(default_factory=dict) + + +_NUMBER_OR_STRING = (20, 4.5, "{theme.spacing.gap.normal}", "") +_NOT_NUMBER_OR_STRING = ({"a": 1}, [1], True) + +_PRIMITIVE_UNION_SITES = { + "style-gap": _PrimitiveUnionSite( + PublisherDocumentAreaStyle, "gap", "gap", _NUMBER_OR_STRING, _NOT_NUMBER_OR_STRING + ), + "style-padding": _PrimitiveUnionSite( + PublisherDocumentContainerStyle, "padding", "padding", _NUMBER_OR_STRING, _NOT_NUMBER_OR_STRING + ), + "style-border-radius": _PrimitiveUnionSite( + PublisherDocumentContainerStyle, "border_radius", "borderRadius", _NUMBER_OR_STRING, _NOT_NUMBER_OR_STRING + ), + "auto-visibility": _PrimitiveUnionSite( + PublisherDocumentAutoVisibility, "visible", "visible", (True, False, "auto", ""), ({"a": 1}, [1], 20, 1.5) + ), + "publisher-date-filter-from": _PrimitiveUnionSite( + PublisherDocumentDateFilterDateFilter, + "_from", + "from", + (-3, 0.5, "2020-01-01"), + _NOT_NUMBER_OR_STRING, + {"granularity": "GDC.time.month", "type": "relative"}, + ), + "dashboard-date-filter-from": _PrimitiveUnionSite( + DashboardDateFilterDateFilter, + "_from", + "from", + (-3, "2020-01-01"), + ({"a": 1}, [1], True, 1.5), + {"granularity": "GDC.time.month", "type": "relative"}, + ), + "conditional-formatting-literal": _PrimitiveUnionSite( + ConditionalFormattingLiteralValue, + "value", + "value", + _NUMBER_OR_STRING, + _NOT_NUMBER_OR_STRING, + {"kind": "literal"}, + ), +} + +_MEMBER_CASES = [ + pytest.param(site, value, id=f"{name}-{value!r}") + for name, site in _PRIMITIVE_UNION_SITES.items() + for value in site.members +] +_WRONG_CASES = [ + pytest.param(site, value, id=f"{name}-{value!r}") + for name, site in _PRIMITIVE_UNION_SITES.items() + for value in site.non_members +] + + +def _deserialize(body: dict[str, Any], model: type) -> Any: + """Deserializes `body` the way `ApiClient` deserializes a response.""" + return ApiClient().deserialize(SimpleNamespace(data=json.dumps(body)), (model,), True) + + +def _assert_kept(actual: Any, expected: Any) -> None: + assert type(actual) is type(expected) + assert actual == expected + + +@pytest.mark.parametrize(("site", "value"), _MEMBER_CASES) +def test_primitive_union_member_survives_response_deserialization(site: _PrimitiveUnionSite, value: Any) -> None: + """A member value from the wire is kept as it is: `20` stays an int, a string stays a string.""" + model = _deserialize({**site.required, site.json_name: value}, site.model) + + _assert_kept(model.to_dict()[site.python_name], value) + _assert_kept(ApiClient.sanitize_for_serialization(model)[site.json_name], value) + + +@pytest.mark.parametrize(("site", "value"), _MEMBER_CASES) +def test_primitive_union_member_survives_the_constructor(site: _PrimitiveUnionSite, value: Any) -> None: + model = site.model(**site.required, **{site.python_name: value}) + + _assert_kept(model.to_dict()[site.python_name], value) + _assert_kept(ApiClient.sanitize_for_serialization(model)[site.json_name], value) + + +def _assert_rejected_as_wrong_type(build: Callable[[], Any]) -> None: + """Asserts `build` fails on an `ApiTypeError`. + + A composed host such as `ConditionalFormattingLiteralValue` wraps the type + error of its `allOf` member in an `ApiValueError`, so the cause chain is + searched rather than the outermost exception alone. + """ + with pytest.raises((ApiTypeError, ApiValueError)) as info: + build() + error: BaseException | None = info.value + while error is not None and not isinstance(error, ApiTypeError): + error = error.__cause__ + assert isinstance(error, ApiTypeError) + + +@pytest.mark.parametrize(("site", "value"), _WRONG_CASES) +def test_primitive_union_rejects_a_wrong_type_in_a_response(site: _PrimitiveUnionSite, value: Any) -> None: + _assert_rejected_as_wrong_type(lambda: _deserialize({**site.required, site.json_name: value}, site.model)) + + +@pytest.mark.parametrize(("site", "value"), _WRONG_CASES) +def test_primitive_union_rejects_a_wrong_type_in_the_constructor(site: _PrimitiveUnionSite, value: Any) -> None: + _assert_rejected_as_wrong_type(lambda: site.model(**site.required, **{site.python_name: value})) + + +def test_number_property_outside_a_union_still_upconverts_an_int() -> None: + model = _deserialize({"sensitivity": 2}, AnomalyDetectionRequest) + + _assert_kept(model.sensitivity, 2.0) + + +def test_empty_object_model_still_accepts_a_free_form_object() -> None: + body = {"a": {"x": 1}, "b": [1, "two"]} + + model = _deserialize(body, ColumnLocation) + + assert model.to_dict() == body From 7966261e301c4abd670216e127c74965cee0dc29 Mon Sep 17 00:00:00 2001 From: AdamPenaz Date: Tue, 10 Nov 2026 10:26:08 +0100 Subject: [PATCH 2/4] test(gooddata-sdk): carry style values through document round trips The document unit tests carried no style, so nothing in the SDK suite exercised the number-or-string unions a real Publisher document uses. Their literal now has a page `gap: 20`, a container padding theme token, a `borderRadius: 4.5` and a visualization's `display.dataLabels.visible: "auto"`, in the document as well as in the document and page templates. The round-trip assertions compare types as well as values, because `==` treats 20 and 20.0 as equal and would not notice an int turned into a float. A new test deserializes a layout body the way an API response is deserialized and builds the SDK layer from it, which is the path the reported read failure took. The hand-built document in the period test now takes its content in the form a read holds it, with the client's python names for the keys the client models, as for any document's content in memory. jira: LX-3540 risk: nonprod Co-Authored-By: Claude Opus 5.5 --- .../test_declarative_analytics_documents.py | 69 ++++++++++++++++--- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/packages/gooddata-sdk/tests/catalog/unit_tests/test_declarative_analytics_documents.py b/packages/gooddata-sdk/tests/catalog/unit_tests/test_declarative_analytics_documents.py index 948edeea71..31c2c8a7a8 100644 --- a/packages/gooddata-sdk/tests/catalog/unit_tests/test_declarative_analytics_documents.py +++ b/packages/gooddata-sdk/tests/catalog/unit_tests/test_declarative_analytics_documents.py @@ -8,13 +8,17 @@ from __future__ import annotations +import json import shutil from datetime import date from pathlib import Path +from types import SimpleNamespace from typing import Any import pytest from cattrs import structure +from gooddata_api_client.api_client import ApiClient +from gooddata_api_client.model.declarative_analytics import DeclarativeAnalytics from gooddata_sdk import ( CatalogAbsolutePublisherDocumentPeriod, CatalogDeclarativeAnalytics, @@ -37,6 +41,14 @@ "modifiedAt": "2026-11-02 11:00", } _TEXT = {"type": "text", "id": "t1", "markdown": "Hello"} +_THEME_PADDING = "{theme.spacing.padding.normal}" +# Style values are typed number-or-string (`display.*.visible`: boolean-or-string) by the API. +_VISUALIZATION = {"type": "visualization", "id": "v1", "display": {"dataLabels": {"visible": "auto"}}} +_STYLED_ROW = { + "type": "row", + "style": {"padding": _THEME_PADDING, "borderRadius": 4.5}, + "children": [_TEXT, _VISUALIZATION], +} _DOCUMENT = { "id": "doc_1", "title": "Doc 1", @@ -44,13 +56,23 @@ "period": {"type": "relative", "granularity": "GDC.time.month", "from": -1, "to": 0}, "content": { "locale": "en-US", - "pages": [{"id": "p1", "kind": "content", "layout": {"type": "row", "children": [_TEXT]}}], + "pages": [{"id": "p1", "kind": "content", "style": {"gap": 20}, "layout": _STYLED_ROW}], "futureField": {"nested": [1, {"deep": True}]}, }, **_AUTHOR, } -_DOCUMENT_TEMPLATE = {"id": "tpl_1", "title": "Tpl 1", "version": "1", "content": {"locale": "en-US"}} -_PAGE_TEMPLATE = {"id": "pt_1", "title": "Page 1", "version": "1", "content": {"layout": _TEXT}} +_DOCUMENT_TEMPLATE = { + "id": "tpl_1", + "title": "Tpl 1", + "version": "1", + "content": {"locale": "en-US", "style": {"gap": 20}}, +} +_PAGE_TEMPLATE = { + "id": "pt_1", + "title": "Page 1", + "version": "1", + "content": {"style": {"gap": 20, "padding": _THEME_PADDING}, "layout": _STYLED_ROW}, +} _LAYER = {"documents": [_DOCUMENT], "documentTemplates": [_DOCUMENT_TEMPLATE], "pageTemplates": [_PAGE_TEMPLATE]} _METRIC = {"id": "bmk_w_fraud_charged_back", "title": "BMK", "content": {"format": "#,##0", "maql": "SELECT 1"}} _DOCUMENT_FOLDERS = ("documents", "document_templates", "page_templates") @@ -79,12 +101,38 @@ def _round_trip(layer: CatalogDeclarativeAnalyticsLayer) -> dict[str, Any]: return layer.to_api().to_dict(camel_case=True) +def _assert_same(actual: Any, expected: Any) -> None: + """Asserts equality that also tells `20` from `20.0`, which `==` treats as equal.""" + assert type(actual) is type(expected), f"{actual!r} is not {expected!r}" + if isinstance(expected, dict): + assert actual.keys() == expected.keys() + for key, value in expected.items(): + _assert_same(actual[key], value) + elif isinstance(expected, list): + assert len(actual) == len(expected) + for actual_item, expected_item in zip(actual, expected, strict=True): + _assert_same(actual_item, expected_item) + else: + assert actual == expected + + def test_documents_survive_api_round_trip() -> None: layer = CatalogDeclarativeAnalyticsLayer.from_dict(_LAYER) round_tripped = _round_trip(layer) - assert {key: round_tripped[key] for key in _LAYER} == _LAYER + _assert_same({key: round_tripped[key] for key in _LAYER}, _LAYER) + + +def test_documents_survive_response_deserialization() -> None: + """A layout read from the API keeps every style value as the backend sent it.""" + response = SimpleNamespace(data=json.dumps({"analytics": _LAYER})) + api_layout = ApiClient().deserialize(response, (DeclarativeAnalytics,), True) + + analytics = CatalogDeclarativeAnalytics.from_dict(api_layout.to_dict(camel_case=False), camel_case=False) + + round_tripped = analytics.to_api().to_dict(camel_case=True)["analytics"] + _assert_same({key: round_tripped[key] for key in _LAYER}, _LAYER) def test_document_author_fields_are_typed() -> None: @@ -117,7 +165,7 @@ def test_templates_have_no_author_attributes( assert isinstance(template, item_class) assert not hasattr(template, "created_by") round_tripped = _round_trip(layer)[layer_key] - assert round_tripped == [item] + _assert_same(round_tripped, [item]) assert not set(_AUTHOR) & set(round_tripped[0]) @@ -132,7 +180,7 @@ def test_document_without_author_fields() -> None: assert loaded.modified_by is None assert loaded.modified_at is None round_tripped = _round_trip(layer)["documents"] - assert round_tripped == [document] + _assert_same(round_tripped, [document]) assert not set(_AUTHOR) & set(round_tripped[0]) @@ -159,7 +207,7 @@ def test_documents_survive_disk_round_trip(tmp_path: Path, sort: bool) -> None: assert (analytics_model_folder / "document_templates" / "tpl_1.yaml").is_file() assert (analytics_model_folder / "page_templates" / "pt_1.yaml").is_file() round_tripped = _round_trip(CatalogDeclarativeAnalyticsLayer.load_from_disk(tmp_path)) - assert {key: round_tripped[key] for key in _LAYER} == _LAYER + _assert_same({key: round_tripped[key] for key in _LAYER}, _LAYER) def test_load_without_document_folders(tmp_path: Path) -> None: @@ -179,6 +227,11 @@ def _document_with(period: dict[str, Any]) -> dict[str, Any]: return {**{key: value for key, value in _DOCUMENT.items() if key not in _AUTHOR}, "period": period} +def _content_in_memory() -> dict[str, Any]: + """`_DOCUMENT`'s content as a read holds it: keys the client models carry its python names.""" + return CatalogDeclarativePublisherDocument.from_dict(_document_with(_RELATIVE_PERIOD)).content + + @_PERIODS def test_period_read_is_typed( period: dict[str, Any], typed_period: CatalogPublisherDocumentPeriod, client_period: dict[str, Any] @@ -205,7 +258,7 @@ def test_hand_built_document_equals_read_document( id=_DOCUMENT["id"], title=_DOCUMENT["title"], version=_DOCUMENT["version"], - content=_DOCUMENT["content"], + content=_content_in_memory(), period=typed_period, ) From 9aae1021f43674732300d63cae0d5361aaeaaeea Mon Sep 17 00:00:00 2001 From: AdamPenaz Date: Tue, 10 Nov 2026 10:36:52 +0100 Subject: [PATCH 3/4] test(gooddata-sdk): seed style values in the recorded document round trip The recorded document store-load-put test seeded a document without any style, so the real-backend round trip never carried the number-or-string values a Publisher document has. The seed layout now has a page `gap: 20`, a container padding theme token, a `borderRadius: 4.5` and a visualization's `display.dataLabels.visible: "auto"`. The test asserts that the read-back page keeps `gap` as the int 20 and the padding as the token string. The test fails on replay until its cassette is re-recorded against the backend with the new seed. jira: LX-3540 risk: nonprod Co-Authored-By: Claude Sonnet 5.5 --- .../declarative_analytics_model_documents.json | 10 +++++++++- .../tests/catalog/test_catalog_workspace_content.py | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/gooddata-sdk/tests/catalog/expected/declarative_analytics_model_documents.json b/packages/gooddata-sdk/tests/catalog/expected/declarative_analytics_model_documents.json index ad31b77d86..9b4ac1bf62 100644 --- a/packages/gooddata-sdk/tests/catalog/expected/declarative_analytics_model_documents.json +++ b/packages/gooddata-sdk/tests/catalog/expected/declarative_analytics_model_documents.json @@ -14,7 +14,15 @@ { "id": "p1", "kind": "content", - "layout": {"type": "row", "children": [{"type": "text", "id": "t1", "markdown": "Hello"}]} + "style": {"gap": 20}, + "layout": { + "type": "row", + "style": {"padding": "{theme.spacing.padding.normal}", "borderRadius": 4.5}, + "children": [ + {"type": "text", "id": "t1", "markdown": "Hello"}, + {"type": "visualization", "id": "v1", "display": {"dataLabels": {"visible": "auto"}}} + ] + } } ] }, diff --git a/packages/gooddata-sdk/tests/catalog/test_catalog_workspace_content.py b/packages/gooddata-sdk/tests/catalog/test_catalog_workspace_content.py index 5e214c2fdf..24b1cc1724 100644 --- a/packages/gooddata-sdk/tests/catalog/test_catalog_workspace_content.py +++ b/packages/gooddata-sdk/tests/catalog/test_catalog_workspace_content.py @@ -276,6 +276,11 @@ def test_documents_survive_store_load_and_put(test_config): assert document.created_at == seed_document["createdAt"] assert document.modified_by.id == seed_document["modifiedBy"]["id"] assert document.modified_at == seed_document["modifiedAt"] + page = document.to_api().to_dict(camel_case=True)["content"]["pages"][0] + assert page["style"]["gap"] == 20 + assert type(page["style"]["gap"]) is int + assert page["layout"]["style"]["padding"] == "{theme.spacing.padding.normal}" + assert page["layout"]["style"]["borderRadius"] == 4.5 finally: safe_delete(_refresh_workspaces, sdk) From 7e89826f4735541370f63ab0aacf67dc47b7fc16 Mon Sep 17 00:00:00 2001 From: AdamPenaz Date: Tue, 10 Nov 2026 11:07:06 +0100 Subject: [PATCH 4/4] chore(gooddata-sdk): re-record the document round-trip cassette The recorded document store-load-put test now seeds a page gap, a container padding token, a border radius and a visualization data label setting, so its cassette is recorded again on the SDK's staging environment, as is and unedited. The read responses carry the page `gap` as the JSON int 20 and the padding as the token string. The recording was made with the metric and visualization permission flags off, like the other document cassettes. No other cassette changes. jira: LX-3540 risk: nonprod Co-Authored-By: Claude Sonnet 5.5 --- .../demo_documents_store_load_and_put.yaml | 100 +++++++++++++++++- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/packages/gooddata-sdk/tests/catalog/fixtures/workspace_content/demo_documents_store_load_and_put.yaml b/packages/gooddata-sdk/tests/catalog/fixtures/workspace_content/demo_documents_store_load_and_put.yaml index 1640263bea..a3df93b391 100644 --- a/packages/gooddata-sdk/tests/catalog/fixtures/workspace_content/demo_documents_store_load_and_put.yaml +++ b/packages/gooddata-sdk/tests/catalog/fixtures/workspace_content/demo_documents_store_load_and_put.yaml @@ -947,7 +947,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1038,6 +1048,10 @@ interactions: markdown: Hello type: text type: row + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Brand review id: tpl_1 tags: @@ -1055,7 +1069,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1085,6 +1109,10 @@ interactions: id: t1 markdown: Hello type: text + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Cover id: pt_1 tags: @@ -1105,7 +1133,7 @@ interactions: - nosniff X-GDC-TRACE-ID: *id001 content-length: - - '1165' + - '1479' status: code: 200 message: OK @@ -1221,6 +1249,10 @@ interactions: markdown: Hello type: text type: row + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Brand review id: tpl_1 tags: @@ -1238,7 +1270,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1268,6 +1310,10 @@ interactions: id: t1 markdown: Hello type: text + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Cover id: pt_1 tags: @@ -1288,7 +1334,7 @@ interactions: - nosniff X-GDC-TRACE-ID: *id001 content-length: - - '1165' + - '1479' status: code: 200 message: OK @@ -1407,7 +1453,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1504,6 +1560,10 @@ interactions: markdown: Hello type: text type: row + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Brand review id: tpl_1 tags: @@ -1521,7 +1581,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1551,6 +1621,10 @@ interactions: id: t1 markdown: Hello type: text + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Cover id: pt_1 tags: @@ -1571,7 +1645,7 @@ interactions: - nosniff X-GDC-TRACE-ID: *id001 content-length: - - '1165' + - '1479' status: code: 200 message: OK @@ -1608,6 +1682,10 @@ interactions: markdown: Hello type: text type: row + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Brand review id: tpl_1 tags: @@ -1625,7 +1703,17 @@ interactions: - id: t1 markdown: Hello type: text + - display: + dataLabels: + visible: auto + id: v1 + type: visualization + style: + borderRadius: 4.5 + padding: '{theme.spacing.padding.normal}' type: row + style: + gap: 20 createdAt: 2000-01-01 00:00 createdBy: id: admin @@ -1655,6 +1743,10 @@ interactions: id: t1 markdown: Hello type: text + createdAt: 2000-01-01 00:00 + createdBy: + id: admin + type: user description: Cover id: pt_1 tags: @@ -1675,7 +1767,7 @@ interactions: - nosniff X-GDC-TRACE-ID: *id001 content-length: - - '1165' + - '1479' status: code: 200 message: OK