From fab4dfc19bd14ce2a5d54aba7b93cdbf1b6a534a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matou=C5=A1=20Dzivjak?= Date: Thu, 20 Aug 2026 10:05:57 +0200 Subject: [PATCH] fix(codegen): handling of nullable/optional fields Distinguish nullable and optional and handle them correctly. Required + nullable fields were incorrectly emitted by the codegen tool as required fields that wouldn't handle actual null values. Fixes: https://github.com/sumup/sumup-py/issues/384 --- .../builder/intermediate_representation.go | 4 +- codegen/pkg/builder/methods.go | 1 + codegen/pkg/builder/transform.go | 11 +++++ codegen/pkg/builder/transform_test.go | 40 ++++++++++++++++ codegen/pkg/builder/types.go | 24 ++++++++-- codegen/pkg/builder/types_test.go | 45 +++++++++++++++++ sumup/checkouts/resource.py | 4 +- sumup/readers/resource.py | 4 +- sumup/types/__init__.py | 16 +++---- tests/test_reader_checkout.py | 48 +++++++++++++++++++ 10 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 codegen/pkg/builder/transform_test.go create mode 100644 tests/test_reader_checkout.py diff --git a/codegen/pkg/builder/intermediate_representation.go b/codegen/pkg/builder/intermediate_representation.go index cd23682..a4ab3ec 100644 --- a/codegen/pkg/builder/intermediate_representation.go +++ b/codegen/pkg/builder/intermediate_representation.go @@ -43,8 +43,10 @@ type Property struct { // Type of the field, either primitive type (e.g. string) or if the field // is a schema reference then the type of the schema. Type string - // Optional field. + // Optional field. The property may be omitted from the object. Optional bool + // Nullable field. The property value may be JSON null. + Nullable bool // Schema is the OpenAPI schema used to generate this property. Schema *base.SchemaProxy diff --git a/codegen/pkg/builder/methods.go b/codegen/pkg/builder/methods.go index 677f81a..2fc3970 100644 --- a/codegen/pkg/builder/methods.go +++ b/codegen/pkg/builder/methods.go @@ -277,6 +277,7 @@ func (b *Builder) buildQueryFields(o *v3.Operation) ([]Property, error) { SerializedName: alias, Type: typeName, Optional: p.Required == nil || !*p.Required, + Nullable: schemaIsNullable(p.Schema.Schema()), Schema: p.Schema, Comment: parameterPropertyDoc(p.Schema.Schema()), }) diff --git a/codegen/pkg/builder/transform.go b/codegen/pkg/builder/transform.go index 18fce8f..cd503aa 100644 --- a/codegen/pkg/builder/transform.go +++ b/codegen/pkg/builder/transform.go @@ -389,6 +389,7 @@ func (b *Builder) createFields(properties *orderedmap.Map[string, *base.SchemaPr Type: typeName, Comment: schemaPropertyGodoc(schema.Schema()), Optional: optional, + Nullable: schemaIsNullable(schema.Schema()), Schema: schema, }) types = append(types, moreTypes...) @@ -397,6 +398,16 @@ func (b *Builder) createFields(properties *orderedmap.Map[string, *base.SchemaPr return fields, types } +func schemaIsNullable(schema *base.Schema) bool { + if schema == nil { + return false + } + if schema.Nullable != nil && *schema.Nullable { + return true + } + return slices.Contains(schema.Type, "null") +} + func createEnum(schema *base.Schema, name string) Writable { enumName := stringx.MakeSingular(name) switch { diff --git a/codegen/pkg/builder/transform_test.go b/codegen/pkg/builder/transform_test.go new file mode 100644 index 0000000..4b4a050 --- /dev/null +++ b/codegen/pkg/builder/transform_test.go @@ -0,0 +1,40 @@ +package builder + +import ( + "testing" + + "github.com/pb33f/libopenapi/datamodel/high/base" +) + +func TestSchemaIsNullable(t *testing.T) { + t.Parallel() + + nullable := true + tests := []struct { + name string + schema *base.Schema + want bool + }{ + {name: "nil schema", want: false}, + {name: "not nullable", schema: &base.Schema{Type: []string{"string"}}, want: false}, + { + name: "openapi 3.0 nullable", + schema: &base.Schema{Type: []string{"string"}, Nullable: &nullable}, + want: true, + }, + { + name: "openapi 3.1 null type", + schema: &base.Schema{Type: []string{"string", "null"}}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := schemaIsNullable(tt.schema); got != tt.want { + t.Fatalf("schemaIsNullable() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/codegen/pkg/builder/types.go b/codegen/pkg/builder/types.go index ce98933..1749aef 100644 --- a/codegen/pkg/builder/types.go +++ b/codegen/pkg/builder/types.go @@ -174,17 +174,18 @@ func (p *Property) String() string { useAlias := alias != "" && alias != fieldName + typeName := p.pythonType() if useAlias { aliasChoices := fmt.Sprintf("pydantic.AliasChoices(%q, %q)", alias, fieldName) if p.Optional { - fmt.Fprintf(buf, "%s: %s | None = pydantic.Field(default=None, serialization_alias=%q, validation_alias=%s)\n", fieldName, p.Type, alias, aliasChoices) + fmt.Fprintf(buf, "%s: %s = pydantic.Field(default=None, serialization_alias=%q, validation_alias=%s)\n", fieldName, typeName, alias, aliasChoices) } else { - fmt.Fprintf(buf, "%s: %s = pydantic.Field(serialization_alias=%q, validation_alias=%s)\n", fieldName, p.Type, alias, aliasChoices) + fmt.Fprintf(buf, "%s: %s = pydantic.Field(serialization_alias=%q, validation_alias=%s)\n", fieldName, typeName, alias, aliasChoices) } } else if p.Optional { - fmt.Fprintf(buf, "%s: %s | None = None\n", fieldName, p.Type) + fmt.Fprintf(buf, "%s: %s = None\n", fieldName, typeName) } else { - fmt.Fprintf(buf, "%s: %s\n", fieldName, p.Type) + fmt.Fprintf(buf, "%s: %s\n", fieldName, typeName) } if p.Comment != "" { fmt.Fprintf(buf, "'''\n%s\n'''\n", p.Comment) @@ -193,6 +194,13 @@ func (p *Property) String() string { return buf.String() } +func (p Property) pythonType() string { + if p.Optional || p.Nullable { + return p.Type + " | None" + } + return p.Type +} + func (p Property) FieldName() string { return pythonFieldName(p.Name) } @@ -212,6 +220,9 @@ func (p Property) MethodParameterString(allowNone bool) string { } return fmt.Sprintf("%s: %s | NotGivenType = NOT_GIVEN", p.FieldName(), typeName) } + if p.Nullable { + return fmt.Sprintf("%s: %s | None", p.FieldName(), typeName) + } return fmt.Sprintf("%s: %s", p.FieldName(), typeName) } @@ -223,7 +234,7 @@ func (p Property) MethodParameterType() string { func (p Property) BodyArgumentExpr(allowNone bool) string { name := p.FieldName() if strings.HasPrefix(p.Type, "list[") { - if allowNone && p.Optional { + if (allowNone && p.Optional) || p.Nullable { return fmt.Sprintf("(list(%s) if %s is not None else None)", name, name) } return fmt.Sprintf("list(%s)", name) @@ -234,6 +245,9 @@ func (p Property) BodyArgumentExpr(allowNone bool) string { func (p Property) TypedDictFieldString() string { typeName := inputTypeName(p.Type) + if p.Nullable { + typeName += " | None" + } if p.Comment != "" { typeName = fmt.Sprintf("typing_extensions.Annotated[%s, typing_extensions.Doc(%#v)]", typeName, p.Comment) } diff --git a/codegen/pkg/builder/types_test.go b/codegen/pkg/builder/types_test.go index d6ce483..ce4f57d 100644 --- a/codegen/pkg/builder/types_test.go +++ b/codegen/pkg/builder/types_test.go @@ -17,6 +17,51 @@ func TestGeneratedTypesUseModernUnionSyntax(t *testing.T) { }).String(), want: "expires_at: datetime.datetime | None = None\n", }, + { + name: "required property", + got: (&Property{ + Name: "checkout_id", + Type: "str", + }).String(), + want: "checkout_id: str\n", + }, + { + name: "required nullable property", + got: (&Property{ + Name: "card_type", + Type: "GetReaderCheckoutResponseDataCardType", + Nullable: true, + }).String(), + want: "card_type: GetReaderCheckoutResponseDataCardType | None\n", + }, + { + name: "required nullable aliased property", + got: (&Property{ + Name: "card_type", + SerializedName: "cardType", + Type: "str", + Nullable: true, + }).String(), + want: "card_type: str | None = pydantic.Field(serialization_alias=\"cardType\", validation_alias=pydantic.AliasChoices(\"cardType\", \"card_type\"))\n", + }, + { + name: "required nullable typed dict field", + got: (&Property{ + Name: "installments", + Type: "int", + Nullable: true, + }).TypedDictFieldString(), + want: "installments: typing_extensions.Required[int | None]", + }, + { + name: "required nullable method parameter", + got: (&Property{ + Name: "card_type", + Type: "str", + Nullable: true, + }).MethodParameterString(true), + want: "card_type: str | None", + }, { name: "optional method parameter", got: (&Property{ diff --git a/sumup/checkouts/resource.py b/sumup/checkouts/resource.py index 00311df..1ffd578 100644 --- a/sumup/checkouts/resource.py +++ b/sumup/checkouts/resource.py @@ -148,7 +148,7 @@ class CreateCheckoutBodyInput(typing_extensions.TypedDict, total=False): ] valid_until: typing_extensions.NotRequired[ typing_extensions.Annotated[ - datetime.datetime, + datetime.datetime | None, typing_extensions.Doc( "Optional expiration timestamp. The checkout must be processed before this moment, otherwise it becomes unusable.If omitted, the checkout does not have an explicit expiry time." ), @@ -203,7 +203,7 @@ class UpdateCheckoutBodyInput(typing_extensions.TypedDict, total=False): ] valid_until: typing_extensions.NotRequired[ typing_extensions.Annotated[ - datetime.datetime, + datetime.datetime | None, typing_extensions.Doc( "Updated expiration timestamp. The checkout must be processed before this moment, otherwise it becomes unusable." ), diff --git a/sumup/readers/resource.py b/sumup/readers/resource.py index 2c3cfb5..e675160 100644 --- a/sumup/readers/resource.py +++ b/sumup/readers/resource.py @@ -261,7 +261,7 @@ class CreateReaderCheckoutBodyInput(typing_extensions.TypedDict, total=False): ] affiliate: typing_extensions.NotRequired[ typing_extensions.Annotated[ - CreateReaderCheckoutBodyAffiliateInput, + CreateReaderCheckoutBodyAffiliateInput | None, typing_extensions.Doc( "Affiliate metadata for the transaction.\nIt is a field that allow for integrators to track the source of the transaction." ), @@ -283,7 +283,7 @@ class CreateReaderCheckoutBodyInput(typing_extensions.TypedDict, total=False): ] installments: typing_extensions.NotRequired[ typing_extensions.Annotated[ - int, + int | None, typing_extensions.Doc( "Number of installments for the transaction.\nIt may vary according to the merchant country.\nFor example, in Brazil, the maximum number of installments is 12.\n\nOmit if the merchant country does support installments.\nOtherwise, the checkout will be rejected.\nMin: 1" ), diff --git a/sumup/types/__init__.py b/sumup/types/__init__.py index 2bad779..d8fd888 100644 --- a/sumup/types/__init__.py +++ b/sumup/types/__init__.py @@ -1143,7 +1143,7 @@ class CheckoutCreateRequestDict(typing_extensions.TypedDict, total=False): ] valid_until: typing_extensions.NotRequired[ typing_extensions.Annotated[ - datetime.datetime, + datetime.datetime | None, typing_extensions.Doc( "Optional expiration timestamp. The checkout must be processed before this moment, otherwise it becomes unusable.If omitted, the checkout does not have an explicit expiry time." ), @@ -1435,7 +1435,7 @@ class CheckoutUpdateRequestDict(typing_extensions.TypedDict, total=False): ] valid_until: typing_extensions.NotRequired[ typing_extensions.Annotated[ - datetime.datetime, + datetime.datetime | None, typing_extensions.Doc( "Updated expiration timestamp. The checkout must be processed before this moment, otherwise it becomes unusable." ), @@ -1852,7 +1852,7 @@ class CreateReaderCheckoutRequestDict(typing_extensions.TypedDict, total=False): ] affiliate: typing_extensions.NotRequired[ typing_extensions.Annotated[ - CreateReaderCheckoutRequestAffiliateInput, + CreateReaderCheckoutRequestAffiliateInput | None, typing_extensions.Doc( "Affiliate metadata for the transaction.\nIt is a field that allow for integrators to track the source of the transaction." ), @@ -1874,7 +1874,7 @@ class CreateReaderCheckoutRequestDict(typing_extensions.TypedDict, total=False): ] installments: typing_extensions.NotRequired[ typing_extensions.Annotated[ - int, + int | None, typing_extensions.Doc( "Number of installments for the transaction.\nIt may vary according to the merchant country.\nFor example, in Brazil, the maximum number of installments is 12.\n\nOmit if the merchant country does support installments.\nOtherwise, the checkout will be rejected.\nMin: 1" ), @@ -2461,7 +2461,7 @@ class GetReaderCheckoutResponseData(pydantic.BaseModel): GetReaderCheckoutResponseData is a schema definition. """ - card_type: GetReaderCheckoutResponseDataCardType + card_type: GetReaderCheckoutResponseDataCardType | None """ Type of the card. Required for some countries """ @@ -2482,12 +2482,12 @@ class GetReaderCheckoutResponseData(pydantic.BaseModel): Checkout creation timestamp """ - installments: int + installments: int | None """ Number of installments for the transaction. Required for some countries. """ - payment_status: str + payment_status: str | None """ Payment status from payments v2 event """ @@ -2526,7 +2526,7 @@ class GetReaderCheckoutResponseData(pydantic.BaseModel): Checkout last update timestamp """ - valid_until: datetime.datetime + valid_until: datetime.datetime | None """ Checkout expiration timestamp. After this time, the checkout will be automatically cancelled. """ diff --git a/tests/test_reader_checkout.py b/tests/test_reader_checkout.py new file mode 100644 index 0000000..abd1b99 --- /dev/null +++ b/tests/test_reader_checkout.py @@ -0,0 +1,48 @@ +import datetime + +import httpx + + +def test_get_reader_checkout_accepts_pending_null_fields(sdk_factory): + captured_request: dict[str, httpx.Request] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured_request["request"] = request + return httpx.Response( + 200, + json={ + "data": { + "card_type": None, + "checkout_id": "00e33a36-c99b-4cb2-b635-b90c1455c9c8", + "client_transaction_id": "00e33a36-c99b-4cb2-b635-b90c1455c9c8", + "created_at": "2026-07-07T20:41:16.315434Z", + "installments": None, + "payment_status": None, + "payment_type": "card", + "reader_firmware_version": "3.3.3.21", + "reader_serial_number": "1234567890", + "status": "pending", + "total_amount": {"currency": "EUR", "minor_unit": 2, "value": 1000}, + "updated_at": "2026-07-07T20:42:18.117244Z", + "valid_until": None, + } + }, + ) + + sdk = sdk_factory(handler) + checkout = sdk.readers.get_checkout( + "merchant-123", + "reader-456", + "00e33a36-c99b-4cb2-b635-b90c1455c9c8", + ) + + assert "request" in captured_request + assert checkout.data.status == "pending" + assert checkout.data.card_type is None + assert checkout.data.installments is None + assert checkout.data.payment_status is None + assert checkout.data.valid_until is None + assert checkout.data.total_amount.value == 1000 + assert checkout.data.created_at == datetime.datetime( + 2026, 7, 7, 20, 41, 16, 315434, tzinfo=datetime.timezone.utc + )