Skip to content

Commit 9d0828b

Browse files
wiebrenclaude
andauthored
[ruby] fix: build_from_hash drops every attribute inherited from an allOf parent (#24892)
* [ruby] fix: build_from_hash maps attributes inherited from allOf parents A discriminator allOf child generates as a real subclass, but build_from_hash iterated only openapi_types/attribute_map - class methods that dispatch on the child class in every frame of the ancestry, so a parent's attributes were never mapped from the wire hash: a required inherited attribute made deserialization raise ArgumentError, an optional one came back silently nil. The pre-existing super(attributes) call could not help: its result was discarded, and it iterated the child's own types anyway. Walk the ancestry merging each ancestor's openapi_types and attribute_map (the child wins on a name clash) and drop the discarded-result super call. RubyClientCodegenTest asserts the walk on the generated two-level Lizard < Reptile < Pet fixture (fails without the template change), and the three petstore custom base_object specs now deserialize a Cat that carries its parent's attributes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * update ruby samples Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [ruby] fix: to_hash loses the same inherited attributes The serialization direction has the identical defect: attribute_map and openapi_nullable dispatch on the child class in every ancestor frame, so the inherited super chain only repeated the child's own attributes and a round-tripped allOf child lost its parent's fields again on the way out. Walk the ancestry the same way build_from_hash now does; for a model without a generated parent the walk does not run and the output is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * update ruby samples Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [ruby] fix: ancestor nullability defers to a child's redeclaration, and Set loads Two findings from review: the merged-nullable union let a parent's nullable flag survive a child redeclaring the attribute as non-nullable - an ancestor's entry now only applies to attributes no nearer class declares - and the walk evaluates openapi_nullable eagerly for every to_hash call, so the generated models require 'set' explicitly instead of leaning on the Set builtin autoload rubies before 3.2 do not have. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [ruby] fix: only hand an initializer the attributes it accepts, and load Set only where used build_from_hash and to_hash now take their attribute map from acceptable_attribute_map, the same map the generated initializer validates against. For a generated allOf child it is identical to the ancestry walk; for a consumer subclass that narrows attribute_map it keeps the attributes to the ones the subclass accepts, as on master, instead of passing its parent's fields to an initializer that rejects them. The ancestry walk remains for openapi_types and openapi_nullable. require 'set' is emitted only for generic models, the only ones whose openapi_nullable uses Set; enum classes and oneOf/anyOf modules no longer carry it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [ruby] refactor: merge inherited types and nullability with the acceptable_attribute_map idiom acceptable_openapi_types and acceptable_openapi_nullable merge along the allOf ancestry the way acceptable_attribute_map already does, so build_from_hash and to_hash lose their while loops and the require 'set' block. Also drops the redundant map.key? guard, shortens the generated comments and specs, and reverts the stray ruby-faraday Gemfile.lock bump. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * [ruby] fix: models whose superclass is not a generated model do not delegate to it An alias to an array (ArrayAlias < Array) has a parent, so the ancestry helpers and the initializer called superclass methods Array does not provide. Gate the delegation on parentModel, so every model has a base case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent f81c416 commit 9d0828b

220 files changed

Lines changed: 4699 additions & 1764 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎modules/openapi-generator/src/main/resources/ruby-client/base_object.mustache‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,21 @@
33
# @return [Object] Returns the model itself
44
def self.build_from_hash(attributes)
55
return nil unless attributes.is_a?(Hash)
6-
{{#parent}}
7-
super(attributes)
8-
{{/parent}}
96
attributes = attributes.transform_keys(&:to_sym)
7+
# include attributes inherited from allOf parents (the nearest declaration wins)
8+
map = acceptable_attribute_map
109
transformed_hash = {}
11-
openapi_types.each_pair do |key, type|
12-
if attributes.key?(attribute_map[key]) && attributes[attribute_map[key]].nil?
10+
acceptable_openapi_types.each_pair do |key, type|
11+
if attributes.key?(map[key]) && attributes[map[key]].nil?
1312
transformed_hash["#{key}"] = nil
1413
elsif type =~ /\AArray<(.*)>/i
1514
# check to ensure the input is an array given that the attribute
1615
# is documented as an array but the input is not
17-
if attributes[attribute_map[key]].is_a?(Array)
18-
transformed_hash["#{key}"] = attributes[attribute_map[key]].map { |v| _deserialize($1, v) }
16+
if attributes[map[key]].is_a?(Array)
17+
transformed_hash["#{key}"] = attributes[map[key]].map { |v| _deserialize($1, v) }
1918
end
20-
elsif !attributes[attribute_map[key]].nil?
21-
transformed_hash["#{key}"] = _deserialize(type, attributes[attribute_map[key]])
19+
elsif !attributes[map[key]].nil?
20+
transformed_hash["#{key}"] = _deserialize(type, attributes[map[key]])
2221
end
2322
end
2423
new(transformed_hash)
@@ -27,11 +26,12 @@
2726
# Returns the object in the form of hash
2827
# @return [Hash] Returns the object in the form of hash
2928
def to_hash
30-
hash = {{^parent}}{}{{/parent}}{{#parent}}super{{/parent}}
31-
self.class.attribute_map.each_pair do |attr, param|
29+
hash = {}
30+
# include attributes inherited from allOf parents (the nearest declaration wins)
31+
self.class.acceptable_attribute_map.each_pair do |attr, param|
3232
value = self.send(attr)
3333
if value.nil?
34-
is_nullable = self.class.openapi_nullable.include?(attr)
34+
is_nullable = self.class.acceptable_openapi_nullable.include?(attr)
3535
next if !is_nullable || (is_nullable && !instance_variable_defined?(:"@#{attr}"))
3636
end
3737

‎modules/openapi-generator/src/main/resources/ruby-client/partial_model_generic.mustache‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@
4242
}
4343
end
4444

45-
# Returns attribute mapping this model knows about{{#parent}}, including the ones defined in its parent(s){{/parent}}
45+
# Returns attribute mapping this model knows about{{#parentModel}}, including the ones defined in its parent(s){{/parentModel}}
4646
def self.acceptable_attribute_map
47-
{{^parent}}
47+
{{^parentModel}}
4848
attribute_map
49-
{{/parent}}
50-
{{#parent}}
49+
{{/parentModel}}
50+
{{#parentModel}}
5151
superclass.acceptable_attribute_map.merge(attribute_map)
52-
{{/parent}}
52+
{{/parentModel}}
5353
end
5454

55-
# Returns all the JSON keys this model knows about{{#parent}}, including the ones defined in its parent(s){{/parent}}
55+
# Returns all the JSON keys this model knows about{{#parentModel}}, including the ones defined in its parent(s){{/parentModel}}
5656
def self.acceptable_attributes
5757
acceptable_attribute_map.values
5858
end
@@ -77,6 +77,26 @@
7777
])
7878
end
7979

80+
# Returns attribute type mapping{{#parentModel}}, including the ones defined in its parent(s){{/parentModel}}
81+
def self.acceptable_openapi_types
82+
{{^parentModel}}
83+
openapi_types
84+
{{/parentModel}}
85+
{{#parentModel}}
86+
superclass.acceptable_openapi_types.merge(openapi_types)
87+
{{/parentModel}}
88+
end
89+
90+
# Returns the nullable attributes{{#parentModel}}, including the ones defined in its parent(s) that it does not redeclare{{/parentModel}}
91+
def self.acceptable_openapi_nullable
92+
{{^parentModel}}
93+
openapi_nullable
94+
{{/parentModel}}
95+
{{#parentModel}}
96+
(superclass.acceptable_openapi_nullable - attribute_map.keys) | openapi_nullable
97+
{{/parentModel}}
98+
end
99+
80100
{{#anyOf}}
81101
{{#-first}}
82102
# List of class defined in anyOf (OpenAPI v3)
@@ -127,11 +147,11 @@
127147
end
128148
h[k.to_sym] = v
129149
}
130-
{{#parent}}
150+
{{#parentModel}}
131151

132152
# call parent's initialize
133153
super(attributes)
134-
{{/parent}}
154+
{{/parentModel}}
135155
{{#vars}}
136156

137157
if attributes.key?(:'{{{name}}}')

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/ruby/RubyClientCodegenTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@
2626
import org.openapitools.codegen.model.ModelMap;
2727
import org.openapitools.codegen.model.OperationMap;
2828
import org.openapitools.codegen.model.OperationsMap;
29+
import org.openapitools.codegen.utils.ModelUtils;
2930
import org.testng.Assert;
3031
import org.testng.annotations.Test;
3132

3233
import java.io.File;
3334
import java.nio.charset.StandardCharsets;
3435
import java.nio.file.Files;
36+
import java.nio.file.Path;
3537
import java.util.*;
3638
import java.util.stream.Collectors;
3739

@@ -799,4 +801,61 @@ public void testQueryParamJsonSerializationSetsQueryIsJsonMimeType() {
799801
assertTrue(op.queryParams.stream().allMatch(p -> p.queryIsJsonMimeType),
800802
"All content:application/json query params should have queryIsJsonMimeType=true");
801803
}
804+
805+
@Test(description = "an allOf child's build_from_hash and to_hash include the attributes inherited from its parents")
806+
public void testBuildFromHashMapsInheritedAttributes() throws Exception {
807+
final File output = Files.createTempDirectory("test").toFile();
808+
output.deleteOnExit();
809+
810+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/allOf_composition_discriminator.yaml");
811+
CodegenConfig codegenConfig = new RubyClientCodegen();
812+
codegenConfig.setOutputDir(output.getAbsolutePath());
813+
814+
ClientOptInput clientOptInput = new ClientOptInput().openAPI(openAPI).config(codegenConfig);
815+
new DefaultGenerator().opts(clientOptInput).generate();
816+
817+
// Lizard < Reptile < Pet: types and nullability merge along the ancestry, the nearest declaration wins
818+
Path lizard = new File(output, "lib/openapi_client/models/lizard.rb").toPath();
819+
TestUtils.assertFileContains(lizard,
820+
"superclass.acceptable_openapi_types.merge(openapi_types)",
821+
"(superclass.acceptable_openapi_nullable - attribute_map.keys) | openapi_nullable");
822+
// build_from_hash and to_hash use the merged maps
823+
TestUtils.assertFileContains(lizard,
824+
"acceptable_openapi_types.each_pair do |key, type|",
825+
"self.class.acceptable_attribute_map.each_pair do |attr, param|",
826+
"is_nullable = self.class.acceptable_openapi_nullable.include?(attr)");
827+
// the super call that built and discarded a second instance is gone
828+
TestUtils.assertFileNotContains(lizard, "super(attributes)\n attributes = attributes.transform_keys(&:to_sym)");
829+
// a root model has no parent to merge
830+
TestUtils.assertFileNotContains(new File(output, "lib/openapi_client/models/pet.rb").toPath(), "superclass.acceptable_openapi_types");
831+
// a generated parent still receives the initializer call
832+
TestUtils.assertFileContains(lizard, "# call parent's initialize\n super(attributes)");
833+
}
834+
835+
@Test(description = "a model whose superclass is not a generated model (an alias to an array) does not delegate to it")
836+
public void testAliasModelDoesNotDelegateToItsBuiltInSuperclass() throws Exception {
837+
final File output = Files.createTempDirectory("test").toFile();
838+
output.deleteOnExit();
839+
840+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/features/generate-alias-as-model.yaml");
841+
CodegenConfig codegenConfig = new RubyClientCodegen();
842+
codegenConfig.setOutputDir(output.getAbsolutePath());
843+
844+
ClientOptInput clientOptInput = new ClientOptInput().openAPI(openAPI).config(codegenConfig);
845+
ModelUtils.setGenerateAliasAsModel(true);
846+
try {
847+
new DefaultGenerator().opts(clientOptInput).generate();
848+
} finally {
849+
ModelUtils.setGenerateAliasAsModel(false);
850+
}
851+
852+
// ArrayAlias < Array: Array has none of the helpers, so each one is its own base case
853+
Path arrayAlias = new File(output, "lib/openapi_client/models/array_alias.rb").toPath();
854+
TestUtils.assertFileContains(arrayAlias,
855+
"class ArrayAlias < Array",
856+
"def self.acceptable_attribute_map\n attribute_map\n end",
857+
"def self.acceptable_openapi_types\n openapi_types\n end",
858+
"def self.acceptable_openapi_nullable\n openapi_nullable\n end");
859+
TestUtils.assertFileNotContains(arrayAlias, "superclass.", "super(attributes)");
860+
}
802861
}

‎samples/client/echo_api/ruby-faraday/lib/openapi_client/models/bird.rb‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,16 @@ def self.openapi_nullable
5151
])
5252
end
5353

54+
# Returns attribute type mapping
55+
def self.acceptable_openapi_types
56+
openapi_types
57+
end
58+
59+
# Returns the nullable attributes
60+
def self.acceptable_openapi_nullable
61+
openapi_nullable
62+
end
63+
5464
# Initializes the object
5565
# @param [Hash] attributes Model attributes in the form of hash
5666
def initialize(attributes = {})
@@ -118,18 +128,20 @@ def hash
118128
def self.build_from_hash(attributes)
119129
return nil unless attributes.is_a?(Hash)
120130
attributes = attributes.transform_keys(&:to_sym)
131+
# include attributes inherited from allOf parents (the nearest declaration wins)
132+
map = acceptable_attribute_map
121133
transformed_hash = {}
122-
openapi_types.each_pair do |key, type|
123-
if attributes.key?(attribute_map[key]) && attributes[attribute_map[key]].nil?
134+
acceptable_openapi_types.each_pair do |key, type|
135+
if attributes.key?(map[key]) && attributes[map[key]].nil?
124136
transformed_hash["#{key}"] = nil
125137
elsif type =~ /\AArray<(.*)>/i
126138
# check to ensure the input is an array given that the attribute
127139
# is documented as an array but the input is not
128-
if attributes[attribute_map[key]].is_a?(Array)
129-
transformed_hash["#{key}"] = attributes[attribute_map[key]].map { |v| _deserialize($1, v) }
140+
if attributes[map[key]].is_a?(Array)
141+
transformed_hash["#{key}"] = attributes[map[key]].map { |v| _deserialize($1, v) }
130142
end
131-
elsif !attributes[attribute_map[key]].nil?
132-
transformed_hash["#{key}"] = _deserialize(type, attributes[attribute_map[key]])
143+
elsif !attributes[map[key]].nil?
144+
transformed_hash["#{key}"] = _deserialize(type, attributes[map[key]])
133145
end
134146
end
135147
new(transformed_hash)
@@ -139,10 +151,11 @@ def self.build_from_hash(attributes)
139151
# @return [Hash] Returns the object in the form of hash
140152
def to_hash
141153
hash = {}
142-
self.class.attribute_map.each_pair do |attr, param|
154+
# include attributes inherited from allOf parents (the nearest declaration wins)
155+
self.class.acceptable_attribute_map.each_pair do |attr, param|
143156
value = self.send(attr)
144157
if value.nil?
145-
is_nullable = self.class.openapi_nullable.include?(attr)
158+
is_nullable = self.class.acceptable_openapi_nullable.include?(attr)
146159
next if !is_nullable || (is_nullable && !instance_variable_defined?(:"@#{attr}"))
147160
end
148161

‎samples/client/echo_api/ruby-faraday/lib/openapi_client/models/category.rb‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,16 @@ def self.openapi_nullable
5151
])
5252
end
5353

54+
# Returns attribute type mapping
55+
def self.acceptable_openapi_types
56+
openapi_types
57+
end
58+
59+
# Returns the nullable attributes
60+
def self.acceptable_openapi_nullable
61+
openapi_nullable
62+
end
63+
5464
# Initializes the object
5565
# @param [Hash] attributes Model attributes in the form of hash
5666
def initialize(attributes = {})
@@ -118,18 +128,20 @@ def hash
118128
def self.build_from_hash(attributes)
119129
return nil unless attributes.is_a?(Hash)
120130
attributes = attributes.transform_keys(&:to_sym)
131+
# include attributes inherited from allOf parents (the nearest declaration wins)
132+
map = acceptable_attribute_map
121133
transformed_hash = {}
122-
openapi_types.each_pair do |key, type|
123-
if attributes.key?(attribute_map[key]) && attributes[attribute_map[key]].nil?
134+
acceptable_openapi_types.each_pair do |key, type|
135+
if attributes.key?(map[key]) && attributes[map[key]].nil?
124136
transformed_hash["#{key}"] = nil
125137
elsif type =~ /\AArray<(.*)>/i
126138
# check to ensure the input is an array given that the attribute
127139
# is documented as an array but the input is not
128-
if attributes[attribute_map[key]].is_a?(Array)
129-
transformed_hash["#{key}"] = attributes[attribute_map[key]].map { |v| _deserialize($1, v) }
140+
if attributes[map[key]].is_a?(Array)
141+
transformed_hash["#{key}"] = attributes[map[key]].map { |v| _deserialize($1, v) }
130142
end
131-
elsif !attributes[attribute_map[key]].nil?
132-
transformed_hash["#{key}"] = _deserialize(type, attributes[attribute_map[key]])
143+
elsif !attributes[map[key]].nil?
144+
transformed_hash["#{key}"] = _deserialize(type, attributes[map[key]])
133145
end
134146
end
135147
new(transformed_hash)
@@ -139,10 +151,11 @@ def self.build_from_hash(attributes)
139151
# @return [Hash] Returns the object in the form of hash
140152
def to_hash
141153
hash = {}
142-
self.class.attribute_map.each_pair do |attr, param|
154+
# include attributes inherited from allOf parents (the nearest declaration wins)
155+
self.class.acceptable_attribute_map.each_pair do |attr, param|
143156
value = self.send(attr)
144157
if value.nil?
145-
is_nullable = self.class.openapi_nullable.include?(attr)
158+
is_nullable = self.class.acceptable_openapi_nullable.include?(attr)
146159
next if !is_nullable || (is_nullable && !instance_variable_defined?(:"@#{attr}"))
147160
end
148161

‎samples/client/echo_api/ruby-faraday/lib/openapi_client/models/data_query.rb‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,16 @@ def self.openapi_nullable
8080
])
8181
end
8282

83+
# Returns attribute type mapping, including the ones defined in its parent(s)
84+
def self.acceptable_openapi_types
85+
superclass.acceptable_openapi_types.merge(openapi_types)
86+
end
87+
88+
# Returns the nullable attributes, including the ones defined in its parent(s) that it does not redeclare
89+
def self.acceptable_openapi_nullable
90+
(superclass.acceptable_openapi_nullable - attribute_map.keys) | openapi_nullable
91+
end
92+
8393
# List of class defined in allOf (OpenAPI v3)
8494
def self.openapi_all_of
8595
[
@@ -161,20 +171,21 @@ def hash
161171
# @return [Object] Returns the model itself
162172
def self.build_from_hash(attributes)
163173
return nil unless attributes.is_a?(Hash)
164-
super(attributes)
165174
attributes = attributes.transform_keys(&:to_sym)
175+
# include attributes inherited from allOf parents (the nearest declaration wins)
176+
map = acceptable_attribute_map
166177
transformed_hash = {}
167-
openapi_types.each_pair do |key, type|
168-
if attributes.key?(attribute_map[key]) && attributes[attribute_map[key]].nil?
178+
acceptable_openapi_types.each_pair do |key, type|
179+
if attributes.key?(map[key]) && attributes[map[key]].nil?
169180
transformed_hash["#{key}"] = nil
170181
elsif type =~ /\AArray<(.*)>/i
171182
# check to ensure the input is an array given that the attribute
172183
# is documented as an array but the input is not
173-
if attributes[attribute_map[key]].is_a?(Array)
174-
transformed_hash["#{key}"] = attributes[attribute_map[key]].map { |v| _deserialize($1, v) }
184+
if attributes[map[key]].is_a?(Array)
185+
transformed_hash["#{key}"] = attributes[map[key]].map { |v| _deserialize($1, v) }
175186
end
176-
elsif !attributes[attribute_map[key]].nil?
177-
transformed_hash["#{key}"] = _deserialize(type, attributes[attribute_map[key]])
187+
elsif !attributes[map[key]].nil?
188+
transformed_hash["#{key}"] = _deserialize(type, attributes[map[key]])
178189
end
179190
end
180191
new(transformed_hash)
@@ -183,11 +194,12 @@ def self.build_from_hash(attributes)
183194
# Returns the object in the form of hash
184195
# @return [Hash] Returns the object in the form of hash
185196
def to_hash
186-
hash = super
187-
self.class.attribute_map.each_pair do |attr, param|
197+
hash = {}
198+
# include attributes inherited from allOf parents (the nearest declaration wins)
199+
self.class.acceptable_attribute_map.each_pair do |attr, param|
188200
value = self.send(attr)
189201
if value.nil?
190-
is_nullable = self.class.openapi_nullable.include?(attr)
202+
is_nullable = self.class.acceptable_openapi_nullable.include?(attr)
191203
next if !is_nullable || (is_nullable && !instance_variable_defined?(:"@#{attr}"))
192204
end
193205

0 commit comments

Comments
 (0)