From c715b7a317929971c0ceb6afb519db1077246f88 Mon Sep 17 00:00:00 2001 From: Norbert Hartl Date: Thu, 30 Jul 2026 11:47:54 +0200 Subject: [PATCH 1/2] Dispatch request-body encoding by content-type; fix response-error arity; add Client-Tests package Stripe's v1 API (the part ApptiveAccount currently uses) is entirely application/x-www-form-urlencoded, but OAOperation>>applyBody:builder: always looked up 'application/json' regardless of what the operation actually declares. Added OARequestBody>>preferredContentType (prefers form-urlencoded, falls back to json, else whatever is there) and OAMediaTypeObject>>writeFormBody:builder: as the form-writing counterpart to the existing JSON writer; applyBody:builder: now picks the writer to match. Also, while driving real requests through OpenApiClient against the Stripe spec + stripe-mock: - OAMediaTypeObject was missing #encoding: - its own NeoJSON mapping declares an accessor pair for #encoding but only the getter existed, so parsing any spec using the OpenAPI "encoding" keyword crashed. - JSONSchemaAnyObject had no #acceptOpenApi: at all (unlike Object/Array, which at least have the recurse-later stub) - hit by Stripe's metadata schemas, which collapse to an any-type schema. - OpenApiClient>>operation:arguments:body:beforeSendDo: calls #handleError:request:response: but only a 2-arg #handleError:response: was defined, so any non-2xx response crashed with doesNotUnderstand instead of raising the intended OAUnspecifiedError. Also fixes OARequestBuilder>>flattenFormParameters:prefix:into: (from the prior commit) to recognize OrderedDictionary, not just Dictionary - schema-validated request bodies are read back as NeoJSONObject, which subclasses OrderedDictionary and does NOT inherit from Dictionary in Pharo, so it was silently falling through to the scalar branch and serializing the whole object as one bogus nil-keyed field. Added the OpenAPI-Client-Tests package (did not exist before - Client had zero test coverage) with OARequestBuilderTest covering the flatten function (scalars, nested Dictionary, nested OrderedDictionary/ NeoJSONObject, arrays of objects, deep nesting) and addFormBody:. All of this was found and verified by actually loading the real Stripe OpenAPI spec (spec3.json) and driving requests through a live stripe-mock instance - not just reasoning about the code. POST /v1/customers, DELETE /v1/customers/{customer} (full round trip), and POST /v1/subscriptions with nested items[0][price] bracket-notation all build and send correctly now. Response-body reading for the full Customer/Subscription response schemas still hits separate, deeper issues (nullable is not threaded through nested property schemas at all - see the JSONSchema fixes commit - and likely more undiscovered gaps); that is left for a follow-up rather than solved here. --- .../BaselineOfOpenAPI/BaselineOfOpenAPI.class.st | 7 +++---- source/OpenAPI-Client/OARequestBuilder.class.st | 2 +- source/OpenAPI-Client/OpenApiClient.class.st | 5 +++++ .../OpenAPI-Core/JSONSchemaAnyObject.extension.st | 6 ++++++ source/OpenAPI-Core/OAMediaTypeObject.class.st | 12 ++++++++++++ source/OpenAPI-Core/OAOperation.class.st | 14 +++++++++----- source/OpenAPI-Core/OARequestBody.class.st | 8 ++++++++ source/OpenAPI-Core/OASchemaDefinition.class.st | 15 +++++++++++++-- 8 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 source/OpenAPI-Core/JSONSchemaAnyObject.extension.st diff --git a/source/BaselineOfOpenAPI/BaselineOfOpenAPI.class.st b/source/BaselineOfOpenAPI/BaselineOfOpenAPI.class.st index b4f9039..6a033d1 100644 --- a/source/BaselineOfOpenAPI/BaselineOfOpenAPI.class.st +++ b/source/BaselineOfOpenAPI/BaselineOfOpenAPI.class.st @@ -16,15 +16,14 @@ BaselineOfOpenAPI >> baseline: spec [ package: 'OpenAPI-Core-Tests' with: [ spec requires: #('OpenAPI-Core' 'JSONSchema Tests'). ]; package: 'OpenAPI-REST' with: [ spec requires: #('OpenAPI-Core' 'ZincHTTPComponents') ]; package: 'OpenAPI-REST-Tests' with: [ spec requires: #('OpenAPI-REST') ]; - package: 'OpenAPI-Client' with: [ spec requires: #('OpenAPI-Core') ]. - + package: 'OpenAPI-Client' with: [ spec requires: #('OpenAPI-Core') ]; + package: 'OpenAPI-Client-Tests' with: [ spec requires: #('OpenAPI-Client') ]. spec group: 'default' with: #('Core' 'REST' 'Client' 'Tests'); group: 'Core' with: #('OpenAPI-Core'); group: 'REST' with: #('Core' 'OpenAPI-REST'); group: 'Client' with: #('Core' 'OpenAPI-Client'); - group: 'Tests' with: #('OpenAPI-Core-Tests' 'OpenAPI-REST-Tests'). - + group: 'Tests' with: #('OpenAPI-Core-Tests' 'OpenAPI-REST-Tests' 'OpenAPI-Client-Tests'). self neoJson: spec; zinHTTPComponents: spec; diff --git a/source/OpenAPI-Client/OARequestBuilder.class.st b/source/OpenAPI-Client/OARequestBuilder.class.st index 758ed29..5945dbf 100644 --- a/source/OpenAPI-Client/OARequestBuilder.class.st +++ b/source/OpenAPI-Client/OARequestBuilder.class.st @@ -76,7 +76,7 @@ OARequestBuilder >> flattenFormParameters: anObject prefix: aPrefixString into: anObject isString ifTrue: [ aDictionary at: aPrefixString put: anObject. ^ self ]. - (anObject isKindOf: Dictionary) ifTrue: [ + ((anObject isKindOf: Dictionary) or: [ anObject isKindOf: OrderedDictionary ]) ifTrue: [ anObject keysAndValuesDo: [ :k :v | self flattenFormParameters: v diff --git a/source/OpenAPI-Client/OpenApiClient.class.st b/source/OpenAPI-Client/OpenApiClient.class.st index 580b493..150c8fc 100644 --- a/source/OpenAPI-Client/OpenApiClient.class.st +++ b/source/OpenAPI-Client/OpenApiClient.class.st @@ -33,6 +33,11 @@ OpenApiClient >> call: aString withArguments: aCollection [ ] +{ #category : 'requests' } +OpenApiClient >> handleError: anObject request: aRequest response: aResponse [ + ^ self handleError: anObject response: aResponse +] + { #category : 'as yet unclassified' } OpenApiClient >> handleError: anObject response: aResponse [ (OAUnspecifiedError new diff --git a/source/OpenAPI-Core/JSONSchemaAnyObject.extension.st b/source/OpenAPI-Core/JSONSchemaAnyObject.extension.st new file mode 100644 index 0000000..176ce5c --- /dev/null +++ b/source/OpenAPI-Core/JSONSchemaAnyObject.extension.st @@ -0,0 +1,6 @@ +Extension { #name : 'JSONSchemaAnyObject' } + +{ #category : '*OpenAPI-Core' } +JSONSchemaAnyObject >> acceptOpenApi: aVisitor [ + ^ self +] diff --git a/source/OpenAPI-Core/OAMediaTypeObject.class.st b/source/OpenAPI-Core/OAMediaTypeObject.class.st index 3e0cff4..290842c 100644 --- a/source/OpenAPI-Core/OAMediaTypeObject.class.st +++ b/source/OpenAPI-Core/OAMediaTypeObject.class.st @@ -39,6 +39,11 @@ OAMediaTypeObject >> encoding [ ^ encoding ] +{ #category : 'accessing' } +OAMediaTypeObject >> encoding: aDictionary [ + encoding := aDictionary +] + { #category : 'as yet unclassified' } OAMediaTypeObject >> example [ ^ example @@ -108,3 +113,10 @@ OAMediaTypeObject >> writeBody: aDictionary builder: builder [ ifFalse: [ schema readObject: aDictionary ]) ] + +{ #category : 'as yet unclassified' } +OAMediaTypeObject >> writeFormBody: aDictionary builder: builder [ + builder addFormBody: (schema isAnyObject + ifTrue: [ aDictionary ] + ifFalse: [ schema readObject: aDictionary ]) +] diff --git a/source/OpenAPI-Core/OAOperation.class.st b/source/OpenAPI-Core/OAOperation.class.st index 8a1c0a8..e47a261 100644 --- a/source/OpenAPI-Core/OAOperation.class.st +++ b/source/OpenAPI-Core/OAOperation.class.st @@ -52,11 +52,15 @@ OAOperation >> api [ ] { #category : 'as yet unclassified' } -OAOperation >> applyBody: body builder: builder [ - self hasBody ifTrue: [ | mediaTypeObject | - body ifNil: [ Error signal: 'cannot use nil for body' ]. - mediaTypeObject := self mediaTypeObjectFor: 'application/json'. - mediaTypeObject writeBody: body builder: builder ] +OAOperation >> applyBody: body builder: builder [ + | contentType mediaTypeObject | + self hasBody ifFalse: [ ^ self ]. + body ifNil: [ Error signal: 'cannot use nil for body' ]. + contentType := requestBody preferredContentType. + mediaTypeObject := self mediaTypeObjectFor: contentType. + contentType = 'application/x-www-form-urlencoded' + ifTrue: [ mediaTypeObject writeFormBody: body builder: builder ] + ifFalse: [ mediaTypeObject writeBody: body builder: builder ] ] { #category : 'as yet unclassified' } diff --git a/source/OpenAPI-Core/OARequestBody.class.st b/source/OpenAPI-Core/OARequestBody.class.st index 2c72914..4fa46e3 100644 --- a/source/OpenAPI-Core/OARequestBody.class.st +++ b/source/OpenAPI-Core/OARequestBody.class.st @@ -76,6 +76,14 @@ OARequestBody >> initialize [ required := false ] +{ #category : 'accessing' } +OARequestBody >> preferredContentType [ + content ifNil: [ ^ nil ]. + (content includesKey: 'application/x-www-form-urlencoded') ifTrue: [ ^ 'application/x-www-form-urlencoded' ]. + (content includesKey: 'application/json') ifTrue: [ ^ 'application/json' ]. + ^ content keys anyOne +] + { #category : 'instance creation' } OARequestBody >> readFrom: aCall object: object [ | mimeType bodyString | diff --git a/source/OpenAPI-Core/OASchemaDefinition.class.st b/source/OpenAPI-Core/OASchemaDefinition.class.st index 1f9eb82..8a8ff8f 100644 --- a/source/OpenAPI-Core/OASchemaDefinition.class.st +++ b/source/OpenAPI-Core/OASchemaDefinition.class.st @@ -2,7 +2,8 @@ Class { #name : 'OASchemaDefinition', #superclass : 'JSONSchemaDefinition', #instVars : [ - 'default' + 'default', + 'nullable' ], #category : 'OpenAPI-Core-Model', #package : 'OpenAPI-Core', @@ -13,7 +14,7 @@ Class { OASchemaDefinition class >> neoJsonMapping: mapper [ super neoJsonMapping: mapper. mapper for: self do: [ :mapping | - mapping mapInstVars: #( default ) ] + mapping mapInstVars: #( default nullable ) ] ] { #category : 'visiting' } @@ -30,3 +31,13 @@ OASchemaDefinition >> default [ OASchemaDefinition >> default: anObject [ default := anObject ] + +{ #category : 'accessing' } +OASchemaDefinition >> nullable [ + ^ nullable +] + +{ #category : 'accessing' } +OASchemaDefinition >> nullable: aBoolean [ + nullable := aBoolean +] From 86381cad0efe0269d18ecd52dfb10163ef93bbbb Mon Sep 17 00:00:00 2001 From: Norbert Hartl Date: Thu, 30 Jul 2026 11:49:37 +0200 Subject: [PATCH 2/2] Add OARequestBuilderTest (missed from prior commit - new package needed explicit Iceberg registration) --- .../OARequestBuilderTest.class.st | 78 +++++++++++++++++++ source/OpenAPI-Client-Tests/package.st | 1 + 2 files changed, 79 insertions(+) create mode 100644 source/OpenAPI-Client-Tests/OARequestBuilderTest.class.st create mode 100644 source/OpenAPI-Client-Tests/package.st diff --git a/source/OpenAPI-Client-Tests/OARequestBuilderTest.class.st b/source/OpenAPI-Client-Tests/OARequestBuilderTest.class.st new file mode 100644 index 0000000..3ae1123 --- /dev/null +++ b/source/OpenAPI-Client-Tests/OARequestBuilderTest.class.st @@ -0,0 +1,78 @@ +Class { + #name : 'OARequestBuilderTest', + #superclass : 'TestCase', + #category : 'OpenAPI-Client-Tests-Tests', + #package : 'OpenAPI-Client-Tests', + #tag : 'Tests' +} + +{ #category : 'tests' } +OARequestBuilderTest >> testAddFormBodyBuildsFormUrlEncodedEntity [ + | client builder entity | + client := ZnClient new. + builder := OARequestBuilder new client: client. + builder addFormBody: (Dictionary new at: 'email' put: 'test@example.com'; yourself). + entity := client request entity. + self assert: entity contentType sub equals: 'x-www-form-urlencoded'. + self assert: (entity contents at: 'email') equals: 'test@example.com' +] + +{ #category : 'tests' } +OARequestBuilderTest >> testFlattenArrayOfObjects [ + | builder result items | + items := OrderedCollection new + add: (Dictionary new at: 'price' put: 'price_abc'; at: 'quantity' put: '1'; yourself); + yourself. + builder := OARequestBuilder new. + result := builder flattenFormParameters: (Dictionary new at: 'items' put: items; yourself). + self assert: (result at: 'items[0][price]') equals: 'price_abc'. + self assert: (result at: 'items[0][quantity]') equals: '1' +] + +{ #category : 'tests' } +OARequestBuilderTest >> testFlattenDeeplyNestedObject [ + | builder result nested | + nested := Dictionary new + at: 'end_behavior' put: (Dictionary new at: 'missing_payment_method' put: 'cancel'; yourself); + yourself. + builder := OARequestBuilder new. + result := builder flattenFormParameters: (Dictionary new at: 'trial_settings' put: nested; yourself). + self assert: (result at: 'trial_settings[end_behavior][missing_payment_method]') equals: 'cancel' +] + +{ #category : 'tests' } +OARequestBuilderTest >> testFlattenNestedDictionary [ + | builder result | + builder := OARequestBuilder new. + result := builder flattenFormParameters: (Dictionary new + at: 'metadata' put: (Dictionary new at: 'apptiveid' put: 'acc_42'; yourself); + yourself). + self assert: (result at: 'metadata[apptiveid]') equals: 'acc_42' +] + +{ #category : 'tests' } +OARequestBuilderTest >> testFlattenNestedOrderedDictionary [ + "NeoJSON-produced objects (NeoJSONObject/OrderedDictionary) must flatten the + same way as plain Dictionary - this is what schema-validated request bodies + actually are, not a hand-built Dictionary." + | builder result neoObject | + neoObject := NeoJSONObject new. + neoObject at: 'apptiveid' put: 'acc_42'. + builder := OARequestBuilder new. + result := builder flattenFormParameters: (Dictionary new + at: 'metadata' put: neoObject; + yourself). + self assert: (result at: 'metadata[apptiveid]') equals: 'acc_42' +] + +{ #category : 'tests' } +OARequestBuilderTest >> testFlattenScalarValues [ + | builder result | + builder := OARequestBuilder new. + result := builder flattenFormParameters: (Dictionary new + at: 'customer' put: 'cus_123'; + at: 'trial_end' put: 'now'; + yourself). + self assert: (result at: 'customer') equals: 'cus_123'. + self assert: (result at: 'trial_end') equals: 'now' +] diff --git a/source/OpenAPI-Client-Tests/package.st b/source/OpenAPI-Client-Tests/package.st new file mode 100644 index 0000000..83e6c8f --- /dev/null +++ b/source/OpenAPI-Client-Tests/package.st @@ -0,0 +1 @@ +Package { #name : 'OpenAPI-Client-Tests' }