From 63ae55921553d1a8286c2b994c7a53eecc24c2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Pe=C5=88=C3=A1z?= Date: Tue, 10 Nov 2026 14:30:12 +0100 Subject: [PATCH] fix(gooddata-api-client): accept primitive-only oneOf union values (#1934) * 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 * 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(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 * 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 --------- 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 +- ...declarative_analytics_model_documents.json | 10 +- .../demo_documents_store_load_and_put.yaml | 100 +++++++++++- .../catalog/test_catalog_workspace_content.py | 5 + .../unit_tests/test_composed_oneof_unions.py | 154 ++++++++++++++++++ .../test_declarative_analytics_documents.py | 69 +++++++- 15 files changed, 457 insertions(+), 15 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 000000000..e6ad5c235 --- /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 a73fe96ec..94338b211 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 03ba372b1..245afa22a 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 06393e075..04af7fbe9 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 91f1295e1..9dcd8c700 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 8ee0034a0..d96bcdca0 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 fc327adac..403f5a800 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 d5611e5a9..3c4e5a527 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 1b234dff2..5cfa7d3ed 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 8663eaee8..e2ee362ae 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/expected/declarative_analytics_model_documents.json b/packages/gooddata-sdk/tests/catalog/expected/declarative_analytics_model_documents.json index ad31b77d8..9b4ac1bf6 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/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 1640263be..a3df93b39 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 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 5e214c2fd..24b1cc172 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) 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 5119432ad..c866fa3e9 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 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 948edeea7..31c2c8a7a 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, )