From 0bd08c72a74edd9e37c42a7a2c3688c8c973295c Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Mon, 27 Jul 2026 05:51:22 +0200 Subject: [PATCH 1/4] fix(crystal): keep null branch of 3.1 nullable compositions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An OpenAPI 3.1 property written as `anyOf: [{type: string}, {type: null}]` was emitted as a non-nullable Crystal type. When such a property was also listed in `required`, the generated model was mandatory and non-nullable, so deserialising a response carrying `null` raised at runtime. The null branch was not lost in shared code: the normalizer collapses the composition and sets `nullable`, so `isNullable` reaches the property. No Crystal template read it — the `?` suffix was tied to optionalVars alone. - partial_model_generic.mustache: suffix required properties with `?` when nullable, in both the property declaration and the constructor, so `required` and `nullable` stay independent axes; pass the nilable flag to the validates macro when the property is optional or nullable - CrystalClientCodegen.java: add x-cr-has-required-non-nullable, true when a model still has a required non-nullable property - model_test.mustache: gate the required-field enforcement spec on that extension — JSON::Serializable accepts a missing key for a nilable field, so a model whose required properties are all nullable parses `{}` - CrystalClientCodegenTest.java: cover nullable+required, nullable+optional, the type-as-array form and a multi-member union that must stay a union - 3_1/crystal/nullable-composition.yaml: fixture for the above - samples/client/others/crystal-qdrant: regenerate Co-Authored-By: Claude Opus 5 (1M context) --- .../languages/CrystalClientCodegen.java | 13 +++ .../resources/crystal/model_test.mustache | 11 +-- .../crystal/partial_model_generic.mustache | 6 +- .../crystal/CrystalClientCodegenTest.java | 61 +++++++++++++ .../3_1/crystal/nullable-composition.yaml | 87 +++++++++++++++++++ .../spec/models/context_query_spec.cr | 21 +++-- .../src/qdrant-api/models/context_query.cr | 4 +- .../src/qdrant-api/models/discover_input.cr | 4 +- 8 files changed, 187 insertions(+), 20 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_1/crystal/nullable-composition.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java index 4acd6a559667..262added2ad9 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java @@ -626,6 +626,19 @@ public ModelsMap postProcessModels(ModelsMap objs) { if (notJsonSerializable) { cm.vendorExtensions.put("x-cr-not-json-serializable", Boolean.TRUE); } + + // A required property that is also nullable (OpenAPI 3.1 `anyOf: [T, {type: null}]`, + // or `type: [T, "null"]`) is emitted as `T?`, and JSON::Serializable accepts a + // document that omits a nilable field. Only a required *non-nullable* property makes + // deserialisation of `{}` fail, so the generated spec asserts that only when one exists. + boolean hasRequiredNonNullable = false; + for (CodegenProperty p : cm.getRequiredVars()) { + if (!p.isNullable) { + hasRequiredNonNullable = true; + break; + } + } + cm.vendorExtensions.put("x-cr-has-required-non-nullable", hasRequiredNonNullable); } // process enum in models (sets isEnum flags on properties) ModelsMap processed = postProcessModelsEnum(objs); diff --git a/modules/openapi-generator/src/main/resources/crystal/model_test.mustache b/modules/openapi-generator/src/main/resources/crystal/model_test.mustache index 0c3da07c9c98..6be3c454cc6d 100644 --- a/modules/openapi-generator/src/main/resources/crystal/model_test.mustache +++ b/modules/openapi-generator/src/main/resources/crystal/model_test.mustache @@ -31,7 +31,7 @@ Spectator.describe {{moduleName}}::{{classname}} do end {{/vendorExtensions.x-cr-discriminator-map}} {{^vendorExtensions.x-cr-discriminator-map}} -{{^hasRequired}} +{{^vendorExtensions.x-cr-has-required-non-nullable}} describe "JSON round-trip" do it "parses an empty JSON object and re-serialises to valid JSON" do instance = {{moduleName}}::{{classname}}.from_json("{}") @@ -46,18 +46,19 @@ Spectator.describe {{moduleName}}::{{classname}} do expect(instance.to_h).to be_a(Hash(String, JSON::Any)) end end -{{/hasRequired}} -{{#hasRequired}} +{{/vendorExtensions.x-cr-has-required-non-nullable}} +{{#vendorExtensions.x-cr-has-required-non-nullable}} describe "required-field enforcement" do # A required, non-nilable property without a default makes JSON::Serializable # reject a document that omits it. (Assumes at least one required field has no # default; models where every required field has a default are not present in - # the generated samples.) + # the generated samples.) A required *nullable* property is emitted as `T?`, which + # JSON::Serializable happily defaults to nil, so it cannot carry this assertion. it "raises when required properties are missing" do expect { {{moduleName}}::{{classname}}.from_json("{}") }.to raise_error(JSON::SerializableError) end end -{{/hasRequired}} +{{/vendorExtensions.x-cr-has-required-non-nullable}} {{/vendorExtensions.x-cr-discriminator-map}} {{/vendorExtensions.x-cr-not-json-serializable}} {{/anyOf}} diff --git a/modules/openapi-generator/src/main/resources/crystal/partial_model_generic.mustache b/modules/openapi-generator/src/main/resources/crystal/partial_model_generic.mustache index 77a47cb71d75..08e0fe6aead9 100644 --- a/modules/openapi-generator/src/main/resources/crystal/partial_model_generic.mustache +++ b/modules/openapi-generator/src/main/resources/crystal/partial_model_generic.mustache @@ -26,7 +26,7 @@ # {{{.}}} {{/description}} @[JSON::Field(key: "{{{baseName}}}", emit_null: false)] - property {{{name}}} : {{{dataType}}} + property {{{name}}} : {{{dataType}}}{{#isNullable}}?{{/isNullable}} {{/vendorExtensions.x-cr-inherited}} {{/requiredVars}} @@ -43,7 +43,7 @@ {{/vendorExtensions.x-cr-inherited}} {{/optionalVars}} -{{#vars}}{{#vendorExtensions.x-cr-validated}}{{^isContainer}}{{^vendorExtensions.x-cr-inherited}} validates({{{name}}}, {{{dataType}}}, {{#required}}false{{/required}}{{^required}}true{{/required}}{{#isEnum}}, enum: [{{#allowableValues}}{{#enumVars}}{{{value}}}{{^-last}}, {{/-last}}{{/enumVars}}{{/allowableValues}}]{{/isEnum}}{{#maxLength}}, max_length: {{{maxLength}}}{{/maxLength}}{{#minLength}}, min_length: {{{minLength}}}{{/minLength}}{{#maximum}}, maximum: {{{maximum}}}{{#exclusiveMaximum}}, exclusive_maximum: true{{/exclusiveMaximum}}{{/maximum}}{{#minimum}}, minimum: {{{minimum}}}{{#exclusiveMinimum}}, exclusive_minimum: true{{/exclusiveMinimum}}{{/minimum}}{{#pattern}}, pattern: {{{pattern}}}{{/pattern}}{{#maxItems}}, max_items: {{{maxItems}}}{{/maxItems}}{{#minItems}}, min_items: {{{minItems}}}{{/minItems}}) +{{#vars}}{{#vendorExtensions.x-cr-validated}}{{^isContainer}}{{^vendorExtensions.x-cr-inherited}} validates({{{name}}}, {{{dataType}}}, {{#required}}{{#isNullable}}true{{/isNullable}}{{^isNullable}}false{{/isNullable}}{{/required}}{{^required}}true{{/required}}{{#isEnum}}, enum: [{{#allowableValues}}{{#enumVars}}{{{value}}}{{^-last}}, {{/-last}}{{/enumVars}}{{/allowableValues}}]{{/isEnum}}{{#maxLength}}, max_length: {{{maxLength}}}{{/maxLength}}{{#minLength}}, min_length: {{{minLength}}}{{/minLength}}{{#maximum}}, maximum: {{{maximum}}}{{#exclusiveMaximum}}, exclusive_maximum: true{{/exclusiveMaximum}}{{/maximum}}{{#minimum}}, minimum: {{{minimum}}}{{#exclusiveMinimum}}, exclusive_minimum: true{{/exclusiveMinimum}}{{/minimum}}{{#pattern}}, pattern: {{{pattern}}}{{/pattern}}{{#maxItems}}, max_items: {{{maxItems}}}{{/maxItems}}{{#minItems}}, min_items: {{{minItems}}}{{/minItems}}) {{/vendorExtensions.x-cr-inherited}}{{/isContainer}}{{/vendorExtensions.x-cr-validated}}{{/vars}} {{#allOf}} {{#-first}} @@ -69,7 +69,7 @@ {{/discriminator}} # Initializes the object # @param [Hash] attributes Model attributes in the form of hash - def initialize({{#requiredVars}}{{^vendorExtensions.x-cr-inherited}}@{{/vendorExtensions.x-cr-inherited}}{{{name}}} : {{{dataType}}}{{^-last}}, {{/-last}}{{/requiredVars}}{{#hasRequired}}{{#hasOptional}}, {{/hasOptional}}{{/hasRequired}}{{#optionalVars}}{{^vendorExtensions.x-cr-inherited}}@{{/vendorExtensions.x-cr-inherited}}{{{name}}} : {{{dataType}}}? = {{#vendorExtensions.x-cr-default}}{{{vendorExtensions.x-cr-default}}}{{/vendorExtensions.x-cr-default}}{{^vendorExtensions.x-cr-default}}nil{{/vendorExtensions.x-cr-default}}{{^-last}}, {{/-last}}{{/optionalVars}}) + def initialize({{#requiredVars}}{{^vendorExtensions.x-cr-inherited}}@{{/vendorExtensions.x-cr-inherited}}{{{name}}} : {{{dataType}}}{{#isNullable}}?{{/isNullable}}{{^-last}}, {{/-last}}{{/requiredVars}}{{#hasRequired}}{{#hasOptional}}, {{/hasOptional}}{{/hasRequired}}{{#optionalVars}}{{^vendorExtensions.x-cr-inherited}}@{{/vendorExtensions.x-cr-inherited}}{{{name}}} : {{{dataType}}}? = {{#vendorExtensions.x-cr-default}}{{{vendorExtensions.x-cr-default}}}{{/vendorExtensions.x-cr-default}}{{^vendorExtensions.x-cr-default}}nil{{/vendorExtensions.x-cr-default}}{{^-last}}, {{/-last}}{{/optionalVars}}) {{#parent}} super({{{vendorExtensions.x-cr-parent-args}}}) {{/parent}} end diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java index 88b0e2f1df97..64f6a66f1487 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java @@ -752,4 +752,65 @@ public void testConfigurationExposesSignRequestSeam() throws Exception { } assertTrue(configSeen && connSeen, "configuration.cr and connection.cr must be generated"); } + + /** + * OpenAPI 3.1 / JSON Schema 2020-12 spells "T or null" as a composition holding an explicit + * `{"type": "null"}` member. The null branch must survive as a nullable Crystal type, and it + * must stay independent of `required`: a property that is both required and nullable is + * mandatory-but-nullable (`T?`), not optional and not `T`. + */ + @Test + public void testNullableCompositionIsNullableInCrystal() throws Exception { + final File output = Files.createTempDirectory("test").toFile(); + output.deleteOnExit(); + // parseSpec (not parseFlattenSpec): DefaultGenerator runs the inline model resolver itself, + // so this matches exactly what the CLI feeds the generator. + final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/crystal/nullable-composition.yaml"); + CodegenConfig codegen = new CrystalClientCodegen(); + codegen.setOutputDir(output.getAbsolutePath()); + List files = new DefaultGenerator().opts( + new ClientOptInput().openAPI(openAPI).config(codegen)).generate(); + + File model = files.stream() + .filter(f -> f.getName().equals("message.cr") && + f.getPath().replace(File.separatorChar, '/').contains("/models/")) + .findFirst().orElseThrow(() -> new AssertionError("models/message.cr missing")); + String src = FileUtils.readFileToString(model, StandardCharsets.UTF_8); + + // required + non-nullable stays mandatory and non-nullable + assertTrue(src.contains("property id : String\n"), + "required non-nullable property must stay `String`, got:\n" + src); + + // required + nullable (anyOf composition) -> mandatory but nullable + assertTrue(src.contains("property stop_sequence : String?"), + "required nullable anyOf property must be `String?`, got:\n" + src); + // required + nullable (type-as-array form) -> mandatory but nullable + assertTrue(src.contains("property tag : String?"), + "required nullable type-as-array property must be `String?`, got:\n" + src); + // required + nullable over a $ref -> mandatory but nullable + assertTrue(src.contains("property usage : Usage?"), + "required nullable $ref property must be `Usage?`, got:\n" + src); + + // nullable required properties stay in the mandatory (positional) part of the constructor + assertTrue(src.contains("def initialize(@id : String, @stop_sequence : String?, " + + "@tag : String?, @usage : Usage?,"), + "nullable required properties must remain positional constructor arguments, got:\n" + src); + + // optional + nullable is still optional and nullable (no double `??`) + assertTrue(src.contains("property note : String?"), "optional nullable property must be `String?`"); + assertTrue(src.contains("property count : Int32?"), "optional nullable oneOf property must be `Int32?`"); + Assert.assertFalse(src.contains("??"), "must never emit a doubled nullable marker `??`"); + + // a genuine multi-member union keeps its wrapper class + assertTrue(src.contains("property payload : Payload?"), "union-typed property must reference the wrapper"); + File union = files.stream() + .filter(f -> f.getName().equals("payload.cr") && + f.getPath().replace(File.separatorChar, '/').contains("/models/")) + .findFirst().orElseThrow(() -> new AssertionError("models/payload.cr missing")); + String unionSrc = FileUtils.readFileToString(union, StandardCharsets.UTF_8); + assertTrue(unionSrc.contains("def self.openapi_one_of"), + "a multi-member oneOf must keep generating the union wrapper class, got:\n" + unionSrc); + assertTrue(unionSrc.contains("TextBlock") && unionSrc.contains("ImageBlock"), + "the union wrapper must list both members"); + } } diff --git a/modules/openapi-generator/src/test/resources/3_1/crystal/nullable-composition.yaml b/modules/openapi-generator/src/test/resources/3_1/crystal/nullable-composition.yaml new file mode 100644 index 000000000000..4a46b7a25f74 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/crystal/nullable-composition.yaml @@ -0,0 +1,87 @@ +openapi: 3.1.0 +info: + title: Nullable composition + description: >- + JSON Schema 2020-12 spells "T or null" as a composition holding an explicit + `{"type": "null"}` member. This fixture covers every combination the Crystal + generator has to keep apart: nullable + required, nullable + optional, the + type-as-array form, and a genuine multi-member union that must stay a union. + version: 1.0.0 +paths: + /messages: + get: + operationId: getMessage + responses: + '200': + description: a message + content: + application/json: + schema: + $ref: '#/components/schemas/Message' +components: + schemas: + Message: + type: object + required: + - id + - stop_sequence + - tag + - usage + properties: + id: + type: string + # nullable + required, composition form (anyOf) + stop_sequence: + anyOf: + - type: string + - type: 'null' + default: null + description: Which custom stop sequence was generated, if any. + # nullable + required, type-as-array form + tag: + type: + - string + - 'null' + # nullable + required, composition over a $ref + usage: + anyOf: + - $ref: '#/components/schemas/Usage' + - type: 'null' + # nullable + optional, composition form (anyOf) + note: + anyOf: + - type: string + - type: 'null' + # nullable + optional, composition form (oneOf) + count: + oneOf: + - type: integer + - type: 'null' + # multi-member union: must keep generating the union wrapper class + payload: + $ref: '#/components/schemas/Payload' + Usage: + type: object + properties: + input_tokens: + type: integer + Payload: + oneOf: + - $ref: '#/components/schemas/TextBlock' + - $ref: '#/components/schemas/ImageBlock' + # The union members carry a required field each, so `try each member in order` can + # actually tell them apart. + TextBlock: + type: object + required: + - text + properties: + text: + type: string + ImageBlock: + type: object + required: + - url + properties: + url: + type: string diff --git a/samples/client/others/crystal-qdrant/spec/models/context_query_spec.cr b/samples/client/others/crystal-qdrant/spec/models/context_query_spec.cr index b9602df273c4..4583460ba8f2 100644 --- a/samples/client/others/crystal-qdrant/spec/models/context_query_spec.cr +++ b/samples/client/others/crystal-qdrant/spec/models/context_query_spec.cr @@ -3,7 +3,7 @@ #The version of the OpenAPI document: master #Contact: andrey@vasnetsov.com #Generated by: https://openapi-generator.tech -#Generator version: 7.24.0-SNAPSHOT +#Generator version: 7.25.0-SNAPSHOT # require "../spec_helper" @@ -12,13 +12,18 @@ require "../spec_helper" # Automatically generated by openapi-generator (https://openapi-generator.tech) # Please update as you see appropriate Spectator.describe Qdrant::Api::ContextQuery do - describe "required-field enforcement" do - # A required, non-nilable property without a default makes JSON::Serializable - # reject a document that omits it. (Assumes at least one required field has no - # default; models where every required field has a default are not present in - # the generated samples.) - it "raises when required properties are missing" do - expect { Qdrant::Api::ContextQuery.from_json("{}") }.to raise_error(JSON::SerializableError) + describe "JSON round-trip" do + it "parses an empty JSON object and re-serialises to valid JSON" do + instance = Qdrant::Api::ContextQuery.from_json("{}") + expect(instance).to be_a(Qdrant::Api::ContextQuery) + expect(JSON.parse(instance.to_json)).to be_a(JSON::Any) + end + end + + describe "#to_h" do + it "returns a Hash representation" do + instance = Qdrant::Api::ContextQuery.from_json("{}") + expect(instance.to_h).to be_a(Hash(String, JSON::Any)) end end end diff --git a/samples/client/others/crystal-qdrant/src/qdrant-api/models/context_query.cr b/samples/client/others/crystal-qdrant/src/qdrant-api/models/context_query.cr index e7f04ff89702..65cff74aae3c 100644 --- a/samples/client/others/crystal-qdrant/src/qdrant-api/models/context_query.cr +++ b/samples/client/others/crystal-qdrant/src/qdrant-api/models/context_query.cr @@ -15,12 +15,12 @@ module Qdrant::Api # Required properties @[JSON::Field(key: "context", emit_null: false)] - property context : ContextInput + property context : ContextInput? # Initializes the object # @param [Hash] attributes Model attributes in the form of hash - def initialize(@context : ContextInput) + def initialize(@context : ContextInput?) end # Show invalid properties with the reasons. Usually used together with valid? diff --git a/samples/client/others/crystal-qdrant/src/qdrant-api/models/discover_input.cr b/samples/client/others/crystal-qdrant/src/qdrant-api/models/discover_input.cr index 522580599626..97486b26be91 100644 --- a/samples/client/others/crystal-qdrant/src/qdrant-api/models/discover_input.cr +++ b/samples/client/others/crystal-qdrant/src/qdrant-api/models/discover_input.cr @@ -18,12 +18,12 @@ module Qdrant::Api property target : VectorInput @[JSON::Field(key: "context", emit_null: false)] - property context : DiscoverInputContext + property context : DiscoverInputContext? # Initializes the object # @param [Hash] attributes Model attributes in the form of hash - def initialize(@target : VectorInput, @context : DiscoverInputContext) + def initialize(@target : VectorInput, @context : DiscoverInputContext?) end # Show invalid properties with the reasons. Usually used together with valid? From 72fee5e78c1ad31bef0843476943857ca661e428 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Mon, 27 Jul 2026 05:52:22 +0200 Subject: [PATCH 2/4] fix(InlineModelResolver): rewrite discriminator mappings on dedup deduplicateComponents() removes a structural duplicate of a titled schema and rewrites every $ref to it. A discriminator mapping value is a schema reference too, but it is not stored in a $ref field, so rewriteSchemaRefs() never touched it: the mapping kept naming a schema that had just been removed. No model was generated for that name, and the generated polymorphic dispatch referenced an undefined type. Observed on a public OpenAPI 3.1 spec with 928 schemas: 104 "Failed to lookup the schema" errors covering 38 distinct names, each of them a discriminator target that ended up undefined in the generated client. The client still type-checked, because a language that only analyses called methods never reaches an uncalled union dispatch. - InlineModelResolver.java: add rewriteDiscriminatorMapping(), called for every schema visited by rewriteSchemaRefs(). Handles both forms the spec allows for a mapping value, a full reference and a bare schema name, and preserves the form each entry was written in - InlineModelResolverTest.java: cover both forms, plus a control asserting an unrelated mapping entry is left untouched Co-Authored-By: Claude Opus 5 (1M context) --- .../codegen/InlineModelResolver.java | 36 +++++++++++ .../codegen/InlineModelResolverTest.java | 61 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java index d5d9b348c44e..bc31f7699319 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java @@ -1286,6 +1286,7 @@ private void rewriteSchemaRefs(Schema schema, Map refReplacement schema.set$ref(replacement); } } + rewriteDiscriminatorMapping(schema, refReplacements); if (schema.getProperties() != null) { for (Object prop : schema.getProperties().values()) { rewriteSchemaRefs((Schema) prop, refReplacements); @@ -1317,6 +1318,41 @@ private void rewriteSchemaRefs(Schema schema, Map refReplacement } } + /** + * Rewrites the values of a schema's {@code discriminator.mapping} according to the same + * replacement map used for {@code $ref}s. A mapping value is a reference to a schema, but it + * is not stored in a {@code $ref} field, so it is missed by plain {@code $ref} rewriting; a + * mapping left pointing at a deduplicated-away schema names a model that is never generated. + *

+ * Per the OpenAPI specification a mapping value is either a full schema reference + * ("#/components/schemas/Foo") or a bare schema name ("Foo"). Both forms are rewritten, and + * each keeps the form it was written in. + */ + private void rewriteDiscriminatorMapping(Schema schema, Map refReplacements) { + if (schema.getDiscriminator() == null || schema.getDiscriminator().getMapping() == null) { + return; + } + Map mapping = schema.getDiscriminator().getMapping(); + for (Map.Entry entry : mapping.entrySet()) { + String value = entry.getValue(); + if (value == null) { + continue; + } + if (value.indexOf('/') >= 0) { + String replacement = refReplacements.get(value); + if (replacement != null) { + entry.setValue(replacement); + } + } else { + // bare schema name: match against the full ref, then write the bare name back + String replacement = refReplacements.get("#/components/schemas/" + value); + if (replacement != null) { + entry.setValue(replacement.substring(replacement.lastIndexOf('/') + 1)); + } + } + } + } + /** * Rewrites $refs in all operations reachable from a PathItem (request bodies, parameters, * responses, and nested callbacks). diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java index 81b30649f368..7b6363e166f8 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java @@ -1487,4 +1487,65 @@ public void deduplicateComponentsRemovesNumberedDuplicateOfTitledSchemaAndRewrit assertEquals("$ref must be rewritten from Widget_1 to Widget", "#/components/schemas/Widget", responseSchema.get$ref()); } + + @Test + public void deduplicateComponentsRewritesDiscriminatorMappings() { + // Regression test: deduplicateComponents() removes a structural duplicate and rewrites + // every $ref to it. A discriminator mapping is a reference too, but it lives in + // `discriminator.mapping` rather than in a `$ref` field. When it is not rewritten, the + // mapping keeps naming a schema that no longer exists, so no model is generated for it + // and the generated polymorphic dispatch references an undefined type. + OpenAPI openapi = new OpenAPI(); + openapi.setComponents(new Components()); + openapi.setPaths(new Paths()); + + // ApiError and BetaApiError share a title and a structure, so BetaApiError is removed and + // ApiError (alphabetically first) is kept as canonical. + Schema apiError = new ObjectSchema() + .title("ApiError") + .addProperty("message", new StringSchema()); + Schema betaApiError = new ObjectSchema() + .title("ApiError") + .addProperty("message", new StringSchema()); + Schema notFound = new ObjectSchema() + .title("NotFound") + .addProperty("detail", new StringSchema()); + + openapi.getComponents().addSchemas("ApiError", apiError); + openapi.getComponents().addSchemas("BetaApiError", betaApiError); + openapi.getComponents().addSchemas("NotFound", notFound); + + // A discriminated union whose mapping points at the schema that is about to be removed. + Schema errorResponse = new ObjectSchema() + .title("ErrorResponse") + .discriminator(new Discriminator() + .propertyName("type") + .mapping("api_error", "#/components/schemas/BetaApiError") + // the spec also allows a bare schema name as a mapping value + .mapping("legacy_api_error", "BetaApiError") + .mapping("not_found", "#/components/schemas/NotFound")); + errorResponse.setOneOf(List.of( + new Schema<>().$ref("#/components/schemas/BetaApiError"), + new Schema<>().$ref("#/components/schemas/NotFound"))); + openapi.getComponents().addSchemas("ErrorResponse", errorResponse); + + new InlineModelResolver().flatten(openapi); + + Map schemas = openapi.getComponents().getSchemas(); + assertNotNull("Canonical ApiError must survive deduplication", schemas.get("ApiError")); + assertNull("Duplicate BetaApiError must be removed", schemas.get("BetaApiError")); + + Schema union = schemas.get("ErrorResponse"); + // Control: the oneOf $ref is rewritten (this already worked). + assertEquals("oneOf $ref must be rewritten to the canonical schema", + "#/components/schemas/ApiError", ((Schema) union.getOneOf().get(0)).get$ref()); + // The defect: the discriminator mapping must follow the same rewrite. + assertEquals("discriminator mapping must be rewritten to the canonical schema", + "#/components/schemas/ApiError", union.getDiscriminator().getMapping().get("api_error")); + // a bare-name mapping value is rewritten too, and stays a bare name + assertEquals("bare-name discriminator mapping must be rewritten, keeping the bare form", + "ApiError", union.getDiscriminator().getMapping().get("legacy_api_error")); + assertEquals("an untouched mapping entry must be left alone", + "#/components/schemas/NotFound", union.getDiscriminator().getMapping().get("not_found")); + } } From 9324ade640ade26531f59f111135bb97ba169efc Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Mon, 27 Jul 2026 06:23:39 +0200 Subject: [PATCH 3/4] fix(InlineModelResolver): rewrite refs in every schema-bearing carrier Rewriting $refs after deduplicateComponents() removes a schema was incomplete in two ways, so a reference to the removed schema could survive and name a component that no longer exists. rewriteSchemaRefs() walked only properties, items, allOf/anyOf/oneOf, not and additionalProperties. Every other JSON Schema 2020-12 sub-schema container was left untouched. The rewrite loop visited components/schemas, paths and webhooks only, and inside a path it skipped response headers and the headers carried by a media type's encoding object. A reusable response, parameter, request body or header was never visited at all. - InlineModelResolver.java: walk patternProperties, dependentSchemas, prefixItems, if/then/else, contains, propertyNames, unevaluatedItems, unevaluatedProperties, additionalItems and contentSchema; extract rewriteContentRefs/rewriteHeaderRefs/rewriteParameterRefs/ rewriteApiResponseRefs so content, headers and encoding headers are covered wherever they appear; extend the rewrite to components/responses, parameters, requestBodies, headers, callbacks and pathItems - InlineModelResolverTest.java: one test per gap, each carrying a control assertion on an already-covered carrier Co-Authored-By: Claude Opus 5 (1M context) --- .../codegen/InlineModelResolver.java | 120 ++++++++++++++++-- .../codegen/InlineModelResolverTest.java | 118 +++++++++++++++++ 2 files changed, 227 insertions(+), 11 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java index bc31f7699319..4add57db345a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java @@ -25,6 +25,7 @@ import io.swagger.v3.oas.models.*; import io.swagger.v3.oas.models.PathItem.HttpMethod; import io.swagger.v3.oas.models.callbacks.Callback; +import io.swagger.v3.oas.models.headers.Header; import io.swagger.v3.oas.models.media.*; import io.swagger.v3.oas.models.parameters.Parameter; import io.swagger.v3.oas.models.parameters.RequestBody; @@ -1256,6 +1257,39 @@ private void deduplicateComponents() { rewriteSchemaRefs(schema, refReplacements); } + // Rewrite all $refs in the other component containers that can hold a schema. Only + // components/schemas used to be visited, so a reusable response, parameter, request body, + // header, callback or path item kept naming the schema that was just removed. + Components components = openAPI.getComponents(); + if (components.getResponses() != null) { + for (ApiResponse response : components.getResponses().values()) { + rewriteApiResponseRefs(response, refReplacements); + } + } + if (components.getParameters() != null) { + for (Parameter parameter : components.getParameters().values()) { + rewriteParameterRefs(parameter, refReplacements); + } + } + if (components.getRequestBodies() != null) { + for (RequestBody requestBody : components.getRequestBodies().values()) { + rewriteContentRefs(requestBody.getContent(), refReplacements); + } + } + rewriteHeaderRefs(components.getHeaders(), refReplacements); + if (components.getCallbacks() != null) { + for (Callback callback : components.getCallbacks().values()) { + for (PathItem callbackPathItem : callback.values()) { + rewritePathItemRefs(callbackPathItem, refReplacements); + } + } + } + if (components.getPathItems() != null) { + for (PathItem componentPathItem : components.getPathItems().values()) { + rewritePathItemRefs(componentPathItem, refReplacements); + } + } + // Rewrite all $refs in paths if (openAPI.getPaths() != null) { for (PathItem pathItem : openAPI.getPaths().values()) { @@ -1316,6 +1350,76 @@ private void rewriteSchemaRefs(Schema schema, Map refReplacement if (schema.getAdditionalProperties() instanceof Schema) { rewriteSchemaRefs((Schema) schema.getAdditionalProperties(), refReplacements); } + // Remaining JSON Schema 2020-12 sub-schema containers. A $ref held in any of these is a + // reference like any other and must follow the same rewrite. + if (schema.getPatternProperties() != null) { + for (Object s : schema.getPatternProperties().values()) { + rewriteSchemaRefs((Schema) s, refReplacements); + } + } + if (schema.getDependentSchemas() != null) { + for (Object s : schema.getDependentSchemas().values()) { + rewriteSchemaRefs((Schema) s, refReplacements); + } + } + if (schema.getPrefixItems() != null) { + for (Object s : schema.getPrefixItems()) { + rewriteSchemaRefs((Schema) s, refReplacements); + } + } + rewriteSchemaRefs(schema.getIf(), refReplacements); + rewriteSchemaRefs(schema.getThen(), refReplacements); + rewriteSchemaRefs(schema.getElse(), refReplacements); + rewriteSchemaRefs(schema.getContains(), refReplacements); + rewriteSchemaRefs(schema.getPropertyNames(), refReplacements); + rewriteSchemaRefs(schema.getUnevaluatedItems(), refReplacements); + rewriteSchemaRefs(schema.getUnevaluatedProperties(), refReplacements); + rewriteSchemaRefs(schema.getAdditionalItems(), refReplacements); + rewriteSchemaRefs(schema.getContentSchema(), refReplacements); + } + + /** Rewrites $refs in every media type of a Content, including the headers of its encodings. */ + private void rewriteContentRefs(Content content, Map refReplacements) { + if (content == null) { + return; + } + for (MediaType mediaType : content.values()) { + rewriteSchemaRefs(mediaType.getSchema(), refReplacements); + if (mediaType.getEncoding() != null) { + for (Encoding encoding : mediaType.getEncoding().values()) { + rewriteHeaderRefs(encoding.getHeaders(), refReplacements); + } + } + } + } + + /** Rewrites $refs carried by a map of Headers (schema and content forms). */ + private void rewriteHeaderRefs(Map headers, Map refReplacements) { + if (headers == null) { + return; + } + for (Header header : headers.values()) { + rewriteSchemaRefs(header.getSchema(), refReplacements); + rewriteContentRefs(header.getContent(), refReplacements); + } + } + + /** Rewrites $refs carried by a Parameter (schema and content forms). */ + private void rewriteParameterRefs(Parameter parameter, Map refReplacements) { + if (parameter == null) { + return; + } + rewriteSchemaRefs(parameter.getSchema(), refReplacements); + rewriteContentRefs(parameter.getContent(), refReplacements); + } + + /** Rewrites $refs carried by an ApiResponse (its content and its headers). */ + private void rewriteApiResponseRefs(ApiResponse response, Map refReplacements) { + if (response == null) { + return; + } + rewriteContentRefs(response.getContent(), refReplacements); + rewriteHeaderRefs(response.getHeaders(), refReplacements); } /** @@ -1364,30 +1468,24 @@ private void rewritePathItemRefs(PathItem pathItem, Map refRepla // Path-level parameters if (pathItem.getParameters() != null) { for (Parameter p : pathItem.getParameters()) { - rewriteSchemaRefs(p.getSchema(), refReplacements); + rewriteParameterRefs(p, refReplacements); } } // Operations for (Operation operation : pathItem.readOperations()) { if (operation.getParameters() != null) { for (Parameter p : operation.getParameters()) { - rewriteSchemaRefs(p.getSchema(), refReplacements); + rewriteParameterRefs(p, refReplacements); } } RequestBody requestBody = operation.getRequestBody(); - if (requestBody != null && requestBody.getContent() != null) { - for (MediaType mediaType : requestBody.getContent().values()) { - rewriteSchemaRefs(mediaType.getSchema(), refReplacements); - } + if (requestBody != null) { + rewriteContentRefs(requestBody.getContent(), refReplacements); } ApiResponses responses = operation.getResponses(); if (responses != null) { for (ApiResponse response : responses.values()) { - if (response.getContent() != null) { - for (MediaType mediaType : response.getContent().values()) { - rewriteSchemaRefs(mediaType.getSchema(), refReplacements); - } - } + rewriteApiResponseRefs(response, refReplacements); } } if (operation.getCallbacks() != null) { diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java index 7b6363e166f8..caf66155197d 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java @@ -23,6 +23,7 @@ import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.oas.models.Paths; +import io.swagger.v3.oas.models.headers.Header; import io.swagger.v3.oas.models.media.*; import io.swagger.v3.oas.models.parameters.Parameter; import io.swagger.v3.oas.models.parameters.RequestBody; @@ -33,6 +34,7 @@ import org.testng.Assert; import org.testng.annotations.Test; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -1548,4 +1550,120 @@ public void deduplicateComponentsRewritesDiscriminatorMappings() { assertEquals("an untouched mapping entry must be left alone", "#/components/schemas/NotFound", union.getDiscriminator().getMapping().get("not_found")); } + + /** A schema that is a structural duplicate of "Canonical" and therefore gets removed. */ + private static void addCanonicalAndDuplicate(OpenAPI openapi) { + openapi.getComponents().addSchemas("Canonical", + new ObjectSchema().title("Thing").addProperty("name", new StringSchema())); + openapi.getComponents().addSchemas("Duplicate", + new ObjectSchema().title("Thing").addProperty("name", new StringSchema())); + } + + private static Schema refToDuplicate() { + return new Schema<>().$ref("#/components/schemas/Duplicate"); + } + + private static void assertRewritten(String carrier, Schema schema) { + assertNotNull("no schema found for carrier " + carrier, schema); + assertEquals("$ref in " + carrier + " must be rewritten to the canonical schema", + "#/components/schemas/Canonical", schema.get$ref()); + } + + @Test + public void deduplicateComponentsRewritesRefsInEverySubSchemaContainer() { + // deduplicateComponents() rewrites $refs by walking a schema's sub-schemas. The walk used + // to stop at properties/items/allOf/anyOf/oneOf/not/additionalProperties, so a $ref held in + // any other JSON Schema 2020-12 container survived pointing at the removed schema. + OpenAPI openapi = new OpenAPI(); + openapi.setComponents(new Components()); + openapi.setPaths(new Paths()); + addCanonicalAndDuplicate(openapi); + + Schema holder = new ObjectSchema().title("Holder"); + // control: an already-walked container, so a total failure of the probe is visible + holder.addProperty("control", refToDuplicate()); + holder.setPatternProperties(new HashMap<>(Map.of("^x-", refToDuplicate()))); + holder.setDependentSchemas(new HashMap<>(Map.of("a", refToDuplicate()))); + holder.setPrefixItems(new ArrayList<>(List.of(refToDuplicate()))); + holder.setIf(refToDuplicate()); + holder.setThen(refToDuplicate()); + holder.setElse(refToDuplicate()); + holder.setContains(refToDuplicate()); + holder.setPropertyNames(refToDuplicate()); + holder.setUnevaluatedItems(refToDuplicate()); + holder.setUnevaluatedProperties(refToDuplicate()); + holder.setAdditionalItems(refToDuplicate()); + holder.setContentSchema(refToDuplicate()); + openapi.getComponents().addSchemas("Holder", holder); + + new InlineModelResolver().flatten(openapi); + + assertNull("the duplicate must be removed", openapi.getComponents().getSchemas().get("Duplicate")); + Schema h = openapi.getComponents().getSchemas().get("Holder"); + assertRewritten("properties (control)", (Schema) h.getProperties().get("control")); + assertRewritten("patternProperties", (Schema) h.getPatternProperties().get("^x-")); + assertRewritten("dependentSchemas", (Schema) h.getDependentSchemas().get("a")); + assertRewritten("prefixItems", (Schema) h.getPrefixItems().get(0)); + assertRewritten("if", h.getIf()); + assertRewritten("then", h.getThen()); + assertRewritten("else", h.getElse()); + assertRewritten("contains", h.getContains()); + assertRewritten("propertyNames", h.getPropertyNames()); + assertRewritten("unevaluatedItems", h.getUnevaluatedItems()); + assertRewritten("unevaluatedProperties", h.getUnevaluatedProperties()); + assertRewritten("additionalItems", h.getAdditionalItems()); + assertRewritten("contentSchema", h.getContentSchema()); + } + + @Test + public void deduplicateComponentsRewritesRefsOutsideComponentsSchemas() { + // The rewrite used to visit components/schemas, paths and webhooks only, and inside a path + // it skipped response headers and encoding headers. A $ref to the removed schema held in + // any of those places kept naming a schema that no longer exists. + OpenAPI openapi = new OpenAPI(); + openapi.setComponents(new Components()); + openapi.setPaths(new Paths()); + addCanonicalAndDuplicate(openapi); + + openapi.getComponents().addResponses("SharedResponse", new ApiResponse().description("d") + .content(new Content().addMediaType("application/json", + new MediaType().schema(refToDuplicate())))); + openapi.getComponents().addParameters("SharedParameter", + new Parameter().name("p").in("query").schema(refToDuplicate())); + openapi.getComponents().addRequestBodies("SharedBody", new RequestBody() + .content(new Content().addMediaType("application/json", + new MediaType().schema(refToDuplicate())))); + openapi.getComponents().addHeaders("SharedHeader", new Header().schema(refToDuplicate())); + + // response headers, and headers carried by a media type's encoding object + Content responseContent = new Content().addMediaType("application/json", + new MediaType() + .schema(new ObjectSchema().addProperty("ok", new StringSchema())) + .encoding(new HashMap<>(Map.of("ok", new Encoding() + .headers(new HashMap<>(Map.of("X-Encoding", + new Header().schema(refToDuplicate())))))))); + openapi.getPaths().addPathItem("/probe", new PathItem().get(new Operation() + .operationId("probe") + .responses(new ApiResponses().addApiResponse("200", new ApiResponse() + .description("d") + .headers(new HashMap<>(Map.of("X-Response", new Header().schema(refToDuplicate())))) + .content(responseContent))))); + + new InlineModelResolver().flatten(openapi); + + assertNull("the duplicate must be removed", openapi.getComponents().getSchemas().get("Duplicate")); + assertRewritten("components/responses", openapi.getComponents().getResponses() + .get("SharedResponse").getContent().get("application/json").getSchema()); + assertRewritten("components/parameters", openapi.getComponents().getParameters() + .get("SharedParameter").getSchema()); + assertRewritten("components/requestBodies", openapi.getComponents().getRequestBodies() + .get("SharedBody").getContent().get("application/json").getSchema()); + assertRewritten("components/headers", openapi.getComponents().getHeaders() + .get("SharedHeader").getSchema()); + + ApiResponse response = openapi.getPaths().get("/probe").getGet().getResponses().get("200"); + assertRewritten("response headers", response.getHeaders().get("X-Response").getSchema()); + assertRewritten("encoding headers", response.getContent().get("application/json") + .getEncoding().get("ok").getHeaders().get("X-Encoding").getSchema()); + } } From 095f02b5cbedc483441a8ea67aba9e4d59ec59f8 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Mon, 27 Jul 2026 06:49:30 +0200 Subject: [PATCH 4/4] fix(crystal): generate valid api classes for 3.1 webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenAPI 3.1 declares webhooks in a top-level `webhooks` object, which the generator routes through postProcessWebhooksWithModels. CrystalClientCodegen only overrode postProcessOperationsWithModels, so none of the Crystal vendor extensions the api template reads were computed for a webhook. The result was api files that are not valid Crystal — `class` with no name, `def create(event : )` with no parameter type — plus a spec file whose require was the empty string. The shard entrypoint did not load them either, so they were dead as well as broken. On a public 3.1 spec declaring 43 webhooks that is 43 invalid source files and 43 uncompilable specs. - CrystalClientCodegen.java: extract the per-api-group post-processing into processApiGroup() and call it from both the operations and the webhooks hook, so a webhook group gets its class name, its qualified parameter and return types, its examples and its spec helper path - shard_name.mustache: require the webhook api files from the shard entrypoint, without which their classes are undefined at use site - CrystalClientCodegenTest.java + 3_1/crystal/webhooks.yaml: assert the generated class is named, its body parameter typed and module-qualified, the entrypoint requires it and its spec requires a real path Co-Authored-By: Claude Opus 5 (1M context) --- .../languages/CrystalClientCodegen.java | 35 +++++++++--- .../resources/crystal/shard_name.mustache | 3 ++ .../crystal/CrystalClientCodegenTest.java | 53 +++++++++++++++++++ .../test/resources/3_1/crystal/webhooks.yaml | 51 ++++++++++++++++++ 4 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_1/crystal/webhooks.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java index 262added2ad9..5a22b0c708fe 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CrystalClientCodegen.java @@ -801,8 +801,33 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc @Override public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List allModels) { objs = super.postProcessOperationsWithModels(objs, allModels); + processApiGroup(objs, objs.getOperations(), allModels); + return objs; + } - OperationMap operations0 = objs.getOperations(); + /** + * OpenAPI 3.1 declares webhooks in a top-level {@code webhooks} object, which the generator + * routes here instead of through {@link #postProcessOperationsWithModels}. They are rendered by + * the same api template, so they need the same Crystal-specific vendor extensions: without them + * the generated class has no name and its parameters no type. + */ + @Override + public org.openapitools.codegen.model.WebhooksMap postProcessWebhooksWithModels( + org.openapitools.codegen.model.WebhooksMap objs, List allModels) { + objs = super.postProcessWebhooksWithModels(objs, allModels); + processApiGroup(objs, objs.getWebhooks(), allModels); + return objs; + } + + /** + * Shared post-processing for one generated api class, whether its operations come from + * {@code paths} or from {@code webhooks}. + * + * @param objs the template bundle for the api file (also carries specHelperPath) + * @param operations the operations to process, or null when the group is empty + * @param allModels every generated model, used to qualify model types and build examples + */ + private void processApiGroup(Map objs, OperationMap operations0, List allModels) { String classname = (operations0 != null) ? operations0.getClassname() : ""; // The api classname is "::" (toApiName prefixes the configured @@ -837,11 +862,11 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List modelMaps = ModelMap.toCodegenModelMap(allModels); HashMap processedModelMaps = new HashMap<>(); @@ -923,8 +948,6 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List modelMaps, HashMap processedModelMap) { diff --git a/modules/openapi-generator/src/main/resources/crystal/shard_name.mustache b/modules/openapi-generator/src/main/resources/crystal/shard_name.mustache index ec6008ccc211..731eed33e213 100644 --- a/modules/openapi-generator/src/main/resources/crystal/shard_name.mustache +++ b/modules/openapi-generator/src/main/resources/crystal/shard_name.mustache @@ -21,3 +21,6 @@ require "./{{shardName}}/validation" # APIs {{#apiInfo}}{{#apis}}require "./{{shardName}}/api/{{classFilename}}" {{/apis}}{{/apiInfo}} +{{#webhooks}}{{#-first}}# Webhooks (OpenAPI 3.1): api classes generated from the top-level `webhooks` object. +{{/-first}}require "./{{shardName}}/api/{{classFilename}}" +{{/webhooks}} diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java index 64f6a66f1487..0ac50dbe76ed 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/crystal/CrystalClientCodegenTest.java @@ -813,4 +813,57 @@ public void testNullableCompositionIsNullableInCrystal() throws Exception { assertTrue(unionSrc.contains("TextBlock") && unionSrc.contains("ImageBlock"), "the union wrapper must list both members"); } + + /** + * OpenAPI 3.1 declares webhooks in a top-level `webhooks` object. They are routed through + * postProcessWebhooksWithModels, not postProcessOperationsWithModels, so the Crystal-specific + * vendor extensions the api template relies on (class name, parameter types, spec helper path) + * have to be computed on that path too — otherwise the generated api class has no name, its + * parameters have no type, and its spec requires the empty string. + */ + @Test + public void testWebhooksGenerateValidApiClasses() throws Exception { + final File output = Files.createTempDirectory("test").toFile(); + output.deleteOnExit(); + final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/crystal/webhooks.yaml"); + CodegenConfig codegen = new CrystalClientCodegen(); + codegen.setOutputDir(output.getAbsolutePath()); + List files = new DefaultGenerator().opts( + new ClientOptInput().openAPI(openAPI).config(codegen)).generate(); + + File webhookApi = files.stream() + .filter(f -> f.getName().equals("thing_created.cr") && + f.getPath().replace(File.separatorChar, '/').contains("/api/")) + .findFirst().orElseThrow(() -> new AssertionError("api/thing_created.cr missing")); + String src = FileUtils.readFileToString(webhookApi, StandardCharsets.UTF_8); + + // the class must actually be named + Assert.assertFalse(src.contains("class \n") || src.contains("class "), + "webhook api class must have a name, got:\n" + src); + assertTrue(src.contains("module Api") && src.contains("class ThingCreated"), + "expected `module Api` + `class ThingCreated`, got:\n" + src); + + // the body parameter must be typed, and qualified so it resolves to the model + Assert.assertFalse(src.contains(" : )"), "webhook parameter must have a type, got:\n" + src); + assertTrue(src.contains("thing_created_event : OpenAPIClient::ThingCreatedEvent"), + "webhook body parameter must be typed and module-qualified, got:\n" + src); + + // the shard entrypoint must load the webhook api file, else its class is undefined + File entrypoint = files.stream() + .filter(f -> f.getName().equals("openapi_client.cr")) + .findFirst().orElseThrow(() -> new AssertionError("shard entrypoint missing")); + assertTrue(FileUtils.readFileToString(entrypoint, StandardCharsets.UTF_8) + .contains("require \"./openapi_client/api/thing_created\""), + "the shard entrypoint must require the webhook api file"); + + // the generated spec must require a real path, not "" + File webhookSpec = files.stream() + .filter(f -> f.getName().equals("thing_created_spec.cr")) + .findFirst().orElseThrow(() -> new AssertionError("thing_created_spec.cr missing")); + String specSrc = FileUtils.readFileToString(webhookSpec, StandardCharsets.UTF_8); + Assert.assertFalse(specSrc.contains("require \"\""), + "webhook spec must not require the empty string, got:\n" + specSrc); + assertTrue(specSrc.contains("require \"../spec_helper\""), + "webhook spec must require the spec helper, got:\n" + specSrc); + } } diff --git a/modules/openapi-generator/src/test/resources/3_1/crystal/webhooks.yaml b/modules/openapi-generator/src/test/resources/3_1/crystal/webhooks.yaml new file mode 100644 index 000000000000..1b19ef36d589 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/crystal/webhooks.yaml @@ -0,0 +1,51 @@ +openapi: 3.1.0 +info: + title: Webhook demo + description: >- + OpenAPI 3.1 moves webhooks to a top-level `webhooks` object. The generator + routes those through postProcessWebhooksWithModels rather than + postProcessOperationsWithModels, so a generator that only overrides the + latter emits api classes with no name and untyped parameters. + version: 1.0.0 +paths: + /things: + get: + operationId: listThings + responses: + '200': + description: the things + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/Thing' +webhooks: + thing.created: + post: + operationId: thingCreated + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/ThingCreatedEvent' + responses: + '200': + description: acknowledged +components: + schemas: + Thing: + type: object + required: + - id + properties: + id: + type: string + ThingCreatedEvent: + type: object + required: + - thing + properties: + thing: + $ref: '#/components/schemas/Thing'