Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,23 @@ module {{moduleName}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}.to_json
{{/queryIsJsonMimeType}}
{{^queryIsJsonMimeType}}
{{#isMap}}
{{#isExplode}}
{{^isDeepObject}}
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
{{{paramName}}}.compact.each { |name, value| query_params[name.to_s] = value }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A required exploded map passed as nil with client_side_validation disabled now raises NoMethodError before the request is sent. Add the same nil guard used by the optional branch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/ruby-client/api.mustache, line 187:

<comment>A required exploded map passed as `nil` with `client_side_validation` disabled now raises `NoMethodError` before the request is sent. Add the same nil guard used by the optional branch.</comment>

<file context>
@@ -180,7 +180,23 @@ module {{moduleName}}
+      {{#isExplode}}
+      {{^isDeepObject}}
+      # form style, explode: one query parameter per entry, keyed by the property name; nil is left out
+      {{{paramName}}}.compact.each { |name, value| query_params[name.to_s] = value }
+      {{/isDeepObject}}
+      {{#isDeepObject}}
</file context>
Suggested change
{{{paramName}}}.compact.each { |name, value| query_params[name.to_s] = value }
{{{paramName}}}.compact.each { |name, value| query_params[name.to_s] = value } if !{{{paramName}}}.nil?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but it only arises for a required parameter passed as nil with client_side_validation turned off, which is a caller error the validation exists to catch. With validation on (the default) the call fails earlier with the usual "Missing the required parameter" message. I have left it rather than add a nil guard to the required branch, since a guard there would silently send the request without a required parameter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Hash#compact requires Ruby 2.4, but gemRequiredRubyVersion supports values as low as >= 1.9; clients targeting Ruby 1.9–2.3 raise NoMethodError for exploded maps. Replace compact with an each that skips nil values in both new branches, or raise the advertised minimum to 2.4.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/ruby-client/api.mustache, line 187:

<comment>`Hash#compact` requires Ruby 2.4, but `gemRequiredRubyVersion` supports values as low as `>= 1.9`; clients targeting Ruby 1.9–2.3 raise `NoMethodError` for exploded maps. Replace `compact` with an `each` that skips nil values in both new branches, or raise the advertised minimum to 2.4.</comment>

<file context>
@@ -180,7 +180,23 @@ module {{moduleName}}
+      {{#isExplode}}
+      {{^isDeepObject}}
+      # form style, explode: one query parameter per entry, keyed by the property name; nil is left out
+      {{{paramName}}}.compact.each { |name, value| query_params[name.to_s] = value }
+      {{/isDeepObject}}
+      {{#isDeepObject}}
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated client already needs a newer ruby than that: master's base_object.mustache calls attributes.transform_keys(&:to_sym) in every model's build_from_hash, and Hash#transform_keys is Ruby 2.5. So Hash#compact (2.4) does not raise the real floor; the >= 1.9 default of gemRequiredRubyVersion is already out of date on master, which is worth its own fix rather than avoiding 2.4 methods here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. will file a pr to update ruby to newer version instead.

{{/isDeepObject}}
{{#isDeepObject}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}
{{/isExplode}}
{{/isMap}}
{{^isMap}}
query_params[:'{{{baseName}}}'] = {{#collectionFormat}}@api_client.build_collection_param({{{paramName}}}, :{{{collectionFormat}}}){{/collectionFormat}}{{^collectionFormat}}{{{paramName}}}{{/collectionFormat}}
{{/isMap}}
{{/queryIsJsonMimeType}}
{{/required}}
{{/queryParams}}
Expand All @@ -190,7 +206,23 @@ module {{moduleName}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'].to_json if !opts[:'{{{paramName}}}'].nil?
{{/queryIsJsonMimeType}}
{{^queryIsJsonMimeType}}
{{#isMap}}
{{#isExplode}}
{{^isDeepObject}}
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
opts[:'{{{paramName}}}'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'{{{paramName}}}'].nil?
{{/isDeepObject}}
{{#isDeepObject}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'] if !opts[:'{{{paramName}}}'].nil?
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'] if !opts[:'{{{paramName}}}'].nil?
{{/isExplode}}
{{/isMap}}
{{^isMap}}
query_params[:'{{{baseName}}}'] = {{#collectionFormat}}@api_client.build_collection_param(opts[:'{{{paramName}}}'], :{{{collectionFormat}}}){{/collectionFormat}}{{^collectionFormat}}opts[:'{{{paramName}}}']{{/collectionFormat}} if !opts[:'{{{paramName}}}'].nil?
{{/isMap}}
{{/queryIsJsonMimeType}}
{{/required}}
{{/queryParams}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,43 @@ public void testGenerateRubyClientWithHtmlEntity() throws Exception {
}
}

@Test(description = "Verify a form style, exploded map query parameter goes on the wire one entry per parameter")
public void testExplodedObjectQueryParameter() throws Exception {
final File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/exploded-object-query-param.yaml");
RubyClientCodegen codegen = new RubyClientCodegen();
codegen.setOutputDir(output.getAbsolutePath());

ClientOptInput clientOptInput = new ClientOptInput().openAPI(openAPI).config(codegen);
DefaultGenerator generator = new DefaultGenerator();
List<File> files = generator.opts(clientOptInput).generate();
files.forEach(File::deleteOnExit);

File apiFile = files.stream()
.filter(f -> f.getName().equals("default_api.rb"))
.findFirst()
.orElseThrow(() -> new AssertionError("default_api.rb not found in generated files"));

// form style with explode - the default - puts every entry on the wire under its own
// property name. Assigning the whole hash under the parameter name left the http
// library to serialize it in bracket style, which is what used to happen. A nil entry is
// left out.
TestUtils.assertFileContains(apiFile.toPath(),
"opts[:'filter'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'filter'].nil?");
TestUtils.assertFileNotContains(apiFile.toPath(), "query_params[:'filter']");

// a declared map behaves the same way
TestUtils.assertFileContains(apiFile.toPath(),
"opts[:'typed_filter'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'typed_filter'].nil?");

// deepObject and form without explode both keep a single parameter
TestUtils.assertFileContains(apiFile.toPath(),
"query_params[:'deepFilter'] = opts[:'deep_filter'] if !opts[:'deep_filter'].nil?",
"query_params[:'flatFilter'] = opts[:'flat_filter'] if !opts[:'flat_filter'].nil?");
}

@Test
public void testInitialConfigValues() {
final RubyClientCodegen codegen = new RubyClientCodegen();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
openapi: 3.0.3
info:
title: Exploded object query parameters
description: >
Object typed query parameters under form/explode (as a free-form object and as a typed map), deepObject, and form without explode. The free-form variant matters because it is flagged isMap but not isContainer.
version: 1.0.0
servers:
- url: localhost:8080
paths:
/items:
get:
operationId: listItems
parameters:
# style and explode both left out, so the form/true defaults apply: every entry
# becomes its own parameter, keyed by the property name alone.
- in: query
name: filter
schema:
type: object
# the same, but declared as a map rather than as a free-form object
- in: query
name: typedFilter
schema:
type: object
additionalProperties:
type: string
# deepObject nests each entry under the parameter name: deepFilter[key]=value
- in: query
name: deepFilter
style: deepObject
explode: true
schema:
type: object
# form without explode keeps a single parameter carrying the whole object
- in: query
name: flatFilter
style: form
explode: false
schema:
type: object
responses:
'200':
description: a list of items
content:
application/json:
schema:
type: array
items:
type: string
Original file line number Diff line number Diff line change
Expand Up @@ -1556,7 +1556,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
opts[:'language'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
opts[:'language'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
opts[:'language'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
3 changes: 2 additions & 1 deletion samples/client/petstore/ruby/lib/petstore/api/fake_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style, explode: one query parameter per entry, keyed by the property name; nil is left out
opts[:'language'].compact.each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
Loading