[ruby] fix oneOf handling #5706

jfeltesse-mdsol · 2020-03-25T08:41:15Z

~~I have yet to update the documentation related templates and update the samples but opening now to gather initial feedback on the direction.~~ 👌

Basically, this builds up on the work from @bkabrda on the java and go experimental clients.
I didn't go "ruby-experiemental" because the support of oneOf in the current ruby client is broken anyway. not doing this in the end.

How it works is, in the spirit of the "oneof as interfaces" this creates a module, not a class, for oneOf models and provides a set of class methods related to the functionality.
The core method is build_from_hash whose job in this case is to find the relevant oneOf item and transparently building and returning the correct model.
It does so by using the discriminator if present, mapping if present and "falls back" to the existing validation-based look up.

~~I changed some test yaml files to include more oneOf examples.~~ no need in the end.

If I try to do a before/after using the modified composed-one.yaml file, it goes like (some parts trimmed for brevity):

openapi-generator generate -i composed-oneof.yaml -g ruby --library faraday -o ./out/ruby_oneof

model files

# current master
custom_one_of_schema.rb
obj_a.rb
obj_b.rb
obj_c.rb
obj_d.rb

# this branch
custom_one_of_array_schema_one_of.rb
custom_one_of_schema.rb
obj_a.rb
obj_b.rb
obj_c.rb
obj_c_data_one_of.rb
obj_d.rb

custom_one_of_schema.rb

# current master
module OpenapiClient
  class CustomOneOfSchema
    attr_accessor :realtype

    attr_accessor :message

    attr_accessor :description

    attr_accessor :code

    # Attribute mapping from ruby-style variable name to JSON key.
    def self.attribute_map
      {
        :'realtype' => :'realtype',
        :'message' => :'message',
        :'description' => :'description',
        :'code' => :'code'
      }
    end

    # Attribute type mapping.
    def self.openapi_types
      {
        :'realtype' => :'String',
        :'message' => :'String',
        :'description' => :'String',
        :'code' => :'Integer'
      }
    end

    # List of attributes with nullable: true
    def self.openapi_nullable
      Set.new([
      ])
    end

    # List of class defined in oneOf (OpenAPI v3)
    def self.openapi_one_of
      [
      :'ObjA',
      :'ObjB'
      ]
    end

    # discriminator's property name in OpenAPI v3
    def self.openapi_discriminator_name
      :'realtype'
    end

    # Initializes the object
    # @param [Hash] attributes Model attributes in the form of hash
    def initialize(attributes = {})

# another 200 lines of model code...

# this branch
module OpenapiClient
  module CustomOneOfSchema
    # List of class defined in oneOf (OpenAPI v3)
    def self.openapi_one_of
      [
        :'ObjA',
        :'ObjB'
      ]
    end

    # discriminator's property name in OpenAPI v3
    def self.openapi_discriminator_name
      :'realtype'
    end

    # discriminator's mapping in OpenAPI v3
    def self.openapi_discriminator_mapping
      {
        :'a-type' => :'ObjA',
        :'b-type' => :'ObjB'
      }
    end

    # Builds the object from hash
    # @param [Hash] attributes Model attributes in the form of hash
    # @return [Object] Returns the model itself
    def self.build_from_hash(attributes)
      discriminator_value = attributes[openapi_discriminator_name]
      return nil unless discriminator_value

      _class = openapi_discriminator_mapping[discriminator_value.to_sym]
      return nil unless _class

      OpenapiClient.const_get(_class).build_from_hash(attributes)
    end
  end

end

Noteworthy is in the obj_c.rb the nested oneOf property has its model generated, albeit with a sligthly different name:

# current master
    def self.openapi_types
      {
        :'realtype' => :'String',
        :'data' => :'OneOfObjAObjB'  # <==== model not generated!
      }
    end

# this branch
    def self.openapi_types
      {
        :'realtype' => :'String',
        :'data' => :'ObjCDataOneOf'  # <==== "interface" module generated
      }
    end

Limitations / bugs / food for thought

Interestingly, a custom_one_of_array_schema_one_of.rb "interface" module is now generated but the oneOf is taking place inside the items. I'm not sure this solution is going to work here. In this case maybe a regular model should be generated alongside some custom_one_of_array_schema_one_of_items.rb "interface" module?
@bkabrda how do the java and experimental go clients fare in this case?
In java the interface pattern used in this PR makes the models referenced in a oneOf carry the "dynamic" props illustrated in ObjD. [core] Provide User feedback on confusing use of oneOf #4695 sort of discourages that pattern but I guess if the tool can do it, why not. What could be improved in the case of ruby though is to make some "dynamicProps" module in the interface module and dynamically include the props at runtime upon deserializing. Performance may not be stellar so it's trade off between "pureness" of the models and speed I guess.

PR checklist

Read the contribution guidelines.
If contributing template-only or documentation-only changes which will change sample output, build the project before.
Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@cliffano (2017/07) @zlx (2017/09) @autopp (2019/02)

auto-labeler · 2020-03-25T08:41:21Z

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

gommo · 2020-03-26T04:30:58Z

Does this PR address the below part in the original bug?

Another issue related to oneOf for the Ruby client is the generation of objects within a response schema. If you have the following OpenAPI spec, the class type gets generated as ExampleClient::OneOfsimpleObjectcomplexObject, and that class does not exist.

jfeltesse-mdsol · 2020-03-26T08:12:10Z

@gommo ah right, that's not addressed. Even the java generator with improved oneOf handling fails in this case. In both languages it refers to some "EndpointGET200" model that doesn't exist.

@bkabrda is this a known issue of the current implementation or should I open a new issue specifically for this?

wing328 · 2020-04-02T03:08:38Z

is this a known issue of the current implementation or should I open a new issue specifically for this?

Better open an issue with details (e.g. mininal spec) to track the issue.

jfeltesse-mdsol · 2020-04-13T02:44:17Z

@wing328 I just opened #5903

wing328 · 2020-08-17T02:07:38Z

@jfeltesse-mdsol thanks for the PR. When you've time, can you please PM me via Slack to discuss this PR?

https://join.slack.com/t/openapi-generator/shared_invite/enQtNzAyNDMyOTU0OTE1LTY5ZDBiNDI5NzI5ZjQ1Y2E5OWVjMjZkYzY1ZGM2MWQ4YWFjMzcyNDY5MGI4NjQxNDBiMTlmZTc5NjY2ZTQ5MGM

jfeltesse-mdsol · 2020-09-08T06:42:21Z

any chance to get this reviewed/merged?

wing328 · 2020-09-20T16:27:11Z

Tested with a spec with an oneOf schema containing null:

    def self.openapi_one_of
      [
        :'Cat',
        :'DogDot',
        :'Null'
      ]
    end

Something that we need to address later (not this PR) to properly handle nullable oneOf schemas.

wing328 · 2020-09-20T16:43:38Z

I did a test with primary types in oneOf schemas and looks like your current approach assumes the schemas defined in oneOf are models (with properties). Something we need to handle/address in future PRs.

wing328 · 2020-09-20T17:05:14Z

If discriminator.propertyName is set to petType, I got the following code in the oneOf model:

    # discriminator's property name in OpenAPI v3
    def self.openapi_discriminator_name
      :'pet_type'
    end

I don't think this will work as the pet_type won't match the attribute/field petType in the JSON payload. Can you please take a look when you've time?

Example definition:

    One.Of.Response:
      discriminator:
        propertyName: petType
      oneOf:
      - $ref: '#/components/schemas/dog.dot'
      - $ref: '#/components/schemas/Cat'

jfeltesse-mdsol · 2020-09-21T01:51:20Z

I did a test with primary types in oneOf schemas and looks like your current approach assumes the schemas defined in oneOf are models (with properties). Something we need to handle/address in future PRs.

Are you referring to the case where the oneOf members are other types like array or scalars? How are the other languages handling this?

wing328 · 2020-09-21T02:11:33Z

Example:

    One.Of.Primitive.Types:
      oneOf:
      - type: string
      - type: integer

Java (jersey2), Go, PowerShell generator should be able to handle this without issues.

wing328 · 2020-09-21T02:12:34Z

If I understand correctly, your current approach relies on discriminator (which is optional) so we will need to come up with the implementation to support oneOf without the discriminator.

jfeltesse-mdsol · 2020-09-21T09:35:20Z

If I understand correctly, your current approach relies on discriminator (which is optional) so we will need to come up with the implementation to support oneOf without the discriminator.

Actually it does work without a discriminator, the build_from_hash logic goes in this order:

discriminator + mapped model
discriminator alone (e.g. discriminator value is a model name)
if no discriminator, for each oneOf item try to validate the model and return the first one that validates.

wing328 · 2020-09-21T12:41:28Z

That's good to know.

if no discriminator, for each oneOf item try to validate the model and return the first one that validates.

Should it go through the rest to ensure there's only one match (to fulfill the definitions of oneOf)?

wing328 · 2020-09-21T12:54:24Z

For the spec used by the Ruby petstore samples listed below, can you please switch it to modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml instead which covers many more test cases for oneOf, anyOf models?

jfeltesse-mdsol · 2020-09-24T05:06:43Z

Should it go through the rest to ensure there's only one match (to fulfill the definitions of oneOf)?

I don't think so, the logic I described is used when deserizalizing the response and doing validation at that stage brings no benefit. On the request side you wouldn't use the module directly but rather pass the relevant data directly. There, alongside the existing client side validations, checking the oneOf promise may make sense but it should be done in a separate PR to keep things bite-sized.

wing328 · 2020-09-24T16:28:45Z

I don't think so, the logic I described is used when deserizalizing the response

So anyOf and oneOf are essentially the same with respect to the logic you described?

jfeltesse-mdsol · 2020-09-25T05:29:38Z

I don't think so, the logic I described is used when deserizalizing the response

So anyOf and oneOf are essentially the same with respect to the logic you described?

Correct.

As a user though, I have a hard time picturing how validation when receiving can be useful. Imagine I'm using an autogenerated client to access an external API I have no control over. What good does it do to me that even though I received some data, the client throws an error (and prevents me from accessing the data?) because the API's openapi spec file incorrectly advertises oneOf when it really should be anyOf?
As long as the data I receive can be deserialized properly, I don't really care whether it happens to also match another schema. It's a bit arbitrary because you might expect the 2nd item in the oneOf list and get the 1st one (because it checks in order) but then the clients are not AI-powered, there's a minimum expectation that the openapi spec matches the server side implementation, isn't there?

Having said that though, it's not a huge challenge to validate that part. How the client should behave when encountering such cases though? Throw an error? Return null? Return the first valid item and log a warning? some other way??

wing328 · 2020-09-28T09:01:07Z

What good does it do to me that even though I received some data, the client throws an error (and prevents me from accessing the data?)

The user can then report the issue to the API owner to look into the error as the response (payload) doesn't conform to the contract/spec (oneOf in this case). If multiple matches are expected, then anyOf should be used instead.

there's a minimum expectation that the openapi spec matches the server side implementation

Sorry that it's not something I can agree with. Both client and server should conform to the contract (openapi spec).

How the client should behave when encountering such cases though?

In Java, Go, PowerShell implementation, the client throws an exception or returns an error.

zippolyte · 2020-10-01T08:39:04Z

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

+    {{/discriminator}}
+    {{^discriminator}}
+      openapi_one_of.each do |_class|
+        model = {{moduleName}}.const_get(_class).build_from_hash(attributes)


This should be wrapped in a try/rescue. build_from_hash for enum values can raise. And when that's the case, the loop should continue to try the next model.

build_from_hash for enum values can raise

can you provide an example?

It's in here https://github.com/OpenAPITools/openapi-generator/blob/a0a7464786eb727d1ca4b483ad711bc79bc2bdf6/modules/openapi-generator/src/main/resources/ruby-client/partial_model_enum_class.mustache#L17

Oh one more thing I noticed, as I'm trying out this PR on our spec: the oneOf types can be primitive types like Float, String, ... so we can't call build_from_hash on those. Maybe you could add some special cases, comparing the openapi_one_of value to primitive types, and try to directly parse the value in that case ?

Yes, this has been pointed out and it is something I plan to address #5706 (comment)

Ah right, sorry for the noise 🙈

jfeltesse-mdsol · 2020-10-02T06:57:52Z

heads up here, I've not abandoned this PR, it's just that I got super busy at work. I might be able to go back to this in a couple of weeks.

wing328 · 2020-10-05T15:27:45Z

@jfeltesse-mdsol please take your time. We appreciate your contributions to this.

jfeltesse-mdsol · 2020-10-09T08:46:56Z

@wing328 About #5706 (comment), I rebased my branch with latest master, re-built the generator and updated the inputSpec with

--- a/bin/configs/ruby-faraday.yaml
+++ b/bin/configs/ruby-faraday.yaml
@@ -1,7 +1,7 @@
 generatorName: ruby
 outputDir: samples/client/petstore/ruby-faraday
 library: faraday
-inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml
+inputSpec: modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
 templateDir: modules/openapi-generator/src/main/resources/ruby-client
 additionalProperties:
   gemVersion: 1.0.0

But then running ./bin/generate-samples.sh bin/configs/ruby-faraday.yaml fails with:

[main] WARN  o.o.c.languages.RubyClientCodegen - 200_response (model name starts with number) cannot be used as model name. Renamed to Model200Response
[main] WARN  o.o.c.languages.RubyClientCodegen - Return (reserved word) cannot be used as model name. Renamed to ModelReturn
[main] WARN  o.o.c.languages.RubyClientCodegen - 200_response (model name starts with number) cannot be used as model name. Renamed to Model200Response
[main] WARN  o.o.c.languages.RubyClientCodegen - Return (reserved word) cannot be used as model name. Renamed to ModelReturn
[main] WARN  o.o.c.languages.RubyClientCodegen - 200_response (model name starts with number) cannot be used as model name. Renamed to Model200Response
[main] INFO  o.o.codegen.DefaultGenerator - Model inline_object_3 (marked as unused due to form parameters) is generated due to the system property skipFormModel=false (default)
Exception in thread "main" java.lang.RuntimeException: Could not process model 'inline_object_3'.Please make sure that your schema is correct!
	at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:499)
	at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:879)
	at org.openapitools.codegen.cmd.Generate.execute(Generate.java:432)
	at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32)
	at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)
Caused by: java.lang.ClassCastException: class java.time.OffsetDateTime cannot be cast to class java.lang.String (java.time.OffsetDateTime and java.lang.String are in module java.base of loader 'bootstrap')
	at org.openapitools.codegen.languages.AbstractRubyCodegen.toDefaultValue(AbstractRubyCodegen.java:150)
	at org.openapitools.codegen.DefaultCodegen.fromProperty(DefaultCodegen.java:3083)
	at org.openapitools.codegen.DefaultCodegen.addVars(DefaultCodegen.java:4853)
	at org.openapitools.codegen.DefaultCodegen.addVars(DefaultCodegen.java:4802)
	at org.openapitools.codegen.DefaultCodegen.fromModel(DefaultCodegen.java:2550)
	at org.openapitools.codegen.DefaultGenerator.processModels(DefaultGenerator.java:1259)
	at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:494)

Any clue? The input file doesn't have any inline_object_3.

jfeltesse-mdsol · 2020-10-12T10:11:55Z

@wing328 About the logic and ensuring the oneOf promise is fullfilled, how about verifying it only when a discriminator is not set? This is to conform to the openapi spec's explanation about discriminator:

a discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema. We can then describe exactly which field tells us which schema to use:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#discriminator-object

IMHO, if we always validate all the schemas then it makes the discriminator property pointless and undermines the work of the API spec author.

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

jfeltesse-mdsol · 2020-10-19T08:06:02Z

@wing328 So I had a look at validating the models that may be defined in the oneOf array so that when looping through those we can double check the oneOf promise. The take away is this is not as simple as it sounds and it should be done in a different PR.

Basically, a bare bones implementation may look like

    {{^discriminator}}
      valid_models = []
      openapi_one_of.each do |_class|
        model = {{moduleName}}.const_get(_class).build_from_hash(attributes)
        valid_models << model if model.valid?
      end
      return valid_models[0] if valid_models.size == 1

      raise "{{classname}} is using oneOf but more than 1 schema validated #{attributes} => #{valid_models.inspect}"
    {{/discriminator}}

Looks good right? Except the valid? method does nothing most of the time...
The current code's validation only looks at required fields, min/max lengths, nullable, enums, patterns... but that's about it and if none of those are declared in the schema, it only ever returns true. Just with that we can't identify the models that may be breaking the oneOf promise, for that we'd need to actually do a proper JSON schema validation. A concrete example, with the code above, in the following spec all 3 schemas are considered valid if I pass say { "realtype" => "a", "code" => "b" } to build_from_hash:

    ObjA:
      type: object
      properties:
        realtype:
          type: string
        message:
          type: string
    ObjB:
      type: object
      properties:
        realtype:
          type: string
        code:
          type: integer
    ObjC:
      type: object
      properties:
        realtype:
          type: string
        code:
          type: string
    ObjAOrObjB:
      oneOf:
        - $ref: '#/components/schemas/ObjA'
        - $ref: '#/components/schemas/ObjB'
        - $ref: '#/components/schemas/ObjC'

In this case, that's because the valid? method doesn't check the types or the property names.

So yeah, not opening that pandora's box just now.

I'll try to complete the PR with support for scalar values though, stay tuned!

jfeltesse-mdsol · 2020-10-27T07:37:42Z

Unless the CI reveals bugs, I think this is feature complete. The most interesting bits are in d1c599d (#5706).

Now oneOf supports scalars, arrays, hashes and the combinations you may think of. Maybe not perfect but should support most of the cases. And yeah, it doesn't try to check the oneOf promise. I got it to actually check whether the properties passed existed (see the changes in base_object, before it'd build_from_hash without passing the data to the constructor, bypassing the property name checks...) but the values in base_object are cast without checking the type. If someone's pumped about fixing this, it should be the topic of a separate PR. I can't fix all the issues in one PR 😅

jfeltesse-mdsol · 2020-10-27T08:26:01Z

@wing328 I tried to update the default ruby version that's incorrect here d1c599d (#5706) (by default the generator would use >= 2.4) but the ensure-up-to-date script reverts that change. How to update those docs?

wing328 · 2020-10-27T11:08:36Z

Travis CI reported the following errors:

An error occurred while loading spec_helper.
Failure/Error:
    class ChildCat < ParentPet
      attr_accessor :name
  
      attr_accessor :pet_type
  
      class EnumAttributeValidator
        attr_reader :datatype
        attr_reader :allowable_values
  
        def initialize(datatype, allowable_values)
NameError:
  uninitialized constant Petstore::ParentPet
# ./lib/petstore/models/child_cat.rb:17:in `<module:Petstore>'
# ./lib/petstore/models/child_cat.rb:16:in `<top (required)>'
# ./lib/petstore.rb:35:in `require'
# ./lib/petstore.rb:35:in `<top (required)>'
# ./spec/spec_helper.rb:14:in `require'
# ./spec/spec_helper.rb:14:in `<top (required)>'
No examples found.
Finished in 0.00003 seconds (files took 0.1754 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

Ref: https://travis-ci.org/github/OpenAPITools/openapi-generator/builds/739218479

Please take a look when you've time.

wing328 · 2020-10-27T11:09:52Z

but the ensure-up-to-date script reverts that change. How to update those docs?

CircleCI, which runs the ensure-up-to-date script, didn't report any failure so I assume it's no longer an issue, right?

jfeltesse-mdsol · 2020-10-27T14:39:23Z

@wing328 I'll have a look at the Travis failure tomorrow.

As for the Circle CI one, it's still a problem, I reverted the change so it passes, that's it. The doc here is still going to advertise the default ruby version for generated clients as ">= 1.9" when in fact it's ">= 2.4". How to update the doc and make the CI not change it back?

wing328 · 2020-10-27T15:28:36Z

How to update the doc and make the CI not change it back?

One way is to run ./bin/utils/ensure-up-to-date locally and commit the changes (in the doc).

jfeltesse-mdsol · 2020-10-28T06:58:24Z

One way is to run ./bin/utils/ensure-up-to-date locally and commit the changes (in the doc).

Not sure if I explain this correctly but locally the script will always revert to the previous value.

I commit this:

commit ab351a70fb (HEAD -> ruby/fix_oneof)
Author: Julien Feltesse <[email protected]>
Date:   Wed Oct 28 15:34:55 2020 +0900

    use ruby 2.4 by default

diff --git a/docs/generators/ruby.md b/docs/generators/ruby.md
index 4c1cbca4e2..24b340eeec 100644
--- a/docs/generators/ruby.md
+++ b/docs/generators/ruby.md
@@ -16,7 +16,7 @@
 |gemHomepage|gem homepage. | |http://org.openapitools|
 |gemLicense|gem license. | |unlicense|
 |gemName|gem name (convention: underscore_case).| |openapi_client|
-|gemRequiredRubyVersion|gem required Ruby version. | |&gt;= 1.9|
+|gemRequiredRubyVersion|gem required Ruby version. | |&gt;= 2.4|
 |gemSummary|gem summary. | |A ruby wrapper for the REST APIs|
 |gemVersion|gem version.| |1.0.0|
 |hideGenerationTimestamp|Hides the generation timestamp when files are generated.| |true|

Run the script:

./bin/utils/ensure-up-to-date

And I get that diff:

diff --git a/docs/generators/ruby.md b/docs/generators/ruby.md
index 24b340eeec..4c1cbca4e2 100644
--- a/docs/generators/ruby.md
+++ b/docs/generators/ruby.md
@@ -16,7 +16,7 @@
 |gemHomepage|gem homepage. | |http://org.openapitools|
 |gemLicense|gem license. | |unlicense|
 |gemName|gem name (convention: underscore_case).| |openapi_client|
-|gemRequiredRubyVersion|gem required Ruby version. | |&gt;= 2.4|
+|gemRequiredRubyVersion|gem required Ruby version. | |&gt;= 1.9|
 |gemSummary|gem summary. | |A ruby wrapper for the REST APIs|
 |gemVersion|gem version.| |1.0.0|
 |hideGenerationTimestamp|Hides the generation timestamp when files are generated.| |true|

Where the heck the 1.9 value comes from I do not know. I even tried to modify the value in modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java, rebuild and re-run: same result. Giving up because I can't spend hours fighting that script.

wing328 · 2020-10-28T07:07:49Z

Where the heck the 1.9 value comes from I do not know. I even tried to modify the value in modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java, rebuild and re-run: same result. Giving up because I can't spend hours fighting that script.

The doc is auto-generated. We'll take care of it in a separate PR instead.

jfeltesse-mdsol · 2020-10-28T09:01:08Z

As mentioned in the last commit, I reverted to the old configs because the new ones would run into the issue described at #4690
We actually have a fix for this one and we'll submit a PR soon.

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

jfeltesse-mdsol · 2020-10-29T12:12:09Z

~~CI passes but please do not merge yet, thought more about my last fix and I think it's not going to work in all cases. Will post an update, probably tomorrow.~~ 🙆‍♂️

jfeltesse-mdsol · 2020-10-30T08:18:43Z

fails on the .net things which I'm pretty sure aren't related to the PR here... 😑

jfeltesse-mdsol · 2020-10-31T13:54:03Z

@wing328 This PR is ready to be merged but some CIs are failing randomly. I tried to force-push a couple of times but it's one CI failing once, another CI failing afterwards, and so on. On unrelated stuff.

zippolyte · 2020-11-17T09:56:37Z

hey @wing328, any chance this can get merged in soon ? We're excited to start using this feature for generating our ruby client.

wing328 · 2020-11-23T08:38:56Z

We were busy with the v5.0.0-beta3 release: https://twitter.com/oas_generator/status/1329828788264321025. Let me try to review today or tomorrow.

Is it correct to say that you've tested the change locally and it works in your use cases?

zippolyte · 2020-11-23T10:01:21Z

Yeah it works for the few use cases I tried

wing328 · 2020-11-23T14:42:55Z

@zippolyte thanks for testing it. I did a quick look and the PR looks good overall. I will merge this instead of being the bottleneck to get this PR merged. Will open an issue or file a PR if I find anything later.

zippolyte · 2020-11-24T13:48:44Z

Just found a minor bug in the generated tests. They fail for oneOf containing primitive types because of the generated following code

    it 'lists the models referenced in the oneOf array' do
      expect(described_class.openapi_one_of).to_not be_empty
      described_class.openapi_one_of.each { |klass| expect { DatadogAPIClient::V2.const_get(klass) }.to_not raise_error }
    end

Primitive types are not constants of the API client module.
I propose to just remove this test, WDYT ?
Or maybe not change anything, since test are meant to be manually edited anyway.

jfeltesse-mdsol · 2020-11-24T14:00:33Z

ah, probably an oversight of mine when I refactored the code to accept scalar types and not just schemas. I think removing the 2nd line is ok since we wouldn't be able to make advanced tests in the generated code (lack of context). "Real" oneOf specs should be added in the samples but I didn't do that since the config need to be updated first.

auto-labeler bot added the Client: Ruby label Mar 25, 2020

jfeltesse-mdsol mentioned this pull request Mar 25, 2020

[BUG][Ruby] Using oneOf generated code has unexpected results #3630

Closed

6 tasks

jfeltesse-mdsol mentioned this pull request Apr 13, 2020

[BUG][java] oneOf in "anonymous" schemas: model not generated. #5903

Open

6 tasks

wing328 added Enhancement: Feature Feature: OAS 3.0 spec support labels Sep 20, 2020

wing328 added this to the 5.0.0 milestone Sep 20, 2020

zippolyte reviewed Oct 1, 2020

View reviewed changes

zippolyte reviewed Oct 12, 2020

View reviewed changes

modules/openapi-generator/src/main/resources/ruby-client/model_test.mustache Show resolved Hide resolved

jfeltesse-mdsol mentioned this pull request Oct 27, 2020

[ruby] Fix init/serialization/deserialization of map/array model aliases #7627

Closed

6 tasks

zippolyte reviewed Oct 28, 2020

View reviewed changes

modules/openapi-generator/src/main/resources/ruby-client/base_object.mustache Show resolved Hide resolved

jfeltesse-mdsol added 4 commits November 4, 2020 15:40

[ruby] fix oneOf handling

64998c0

use previous ruby configs due to issue #4690

c4e632b

check for oneOf model in base_object

01d0eee

validate the attributes in partial_oneof_module

ba2aaf8

wing328 merged commit 522faf8 into OpenAPITools:master Nov 23, 2020

jfeltesse-mdsol deleted the ruby/fix_oneof branch November 24, 2020 02:34

jfeltesse-mdsol mentioned this pull request Nov 25, 2020

[ruby] make the generated ruby oneOf specs pass #8019

Merged

6 tasks

[ruby] fix oneOf handling #5706

[ruby] fix oneOf handling #5706

Conversation

jfeltesse-mdsol commented Mar 25, 2020 • edited Loading

model files

custom_one_of_schema.rb

Limitations / bugs / food for thought

PR checklist

auto-labeler bot commented Mar 25, 2020

gommo commented Mar 26, 2020

jfeltesse-mdsol commented Mar 26, 2020

wing328 commented Apr 2, 2020

jfeltesse-mdsol commented Apr 13, 2020

wing328 commented Aug 17, 2020

jfeltesse-mdsol commented Sep 8, 2020

wing328 commented Sep 20, 2020

wing328 commented Sep 20, 2020

wing328 commented Sep 20, 2020 • edited Loading

jfeltesse-mdsol commented Sep 21, 2020

wing328 commented Sep 21, 2020

wing328 commented Sep 21, 2020

jfeltesse-mdsol commented Sep 21, 2020

wing328 commented Sep 21, 2020

wing328 commented Sep 21, 2020

jfeltesse-mdsol commented Sep 24, 2020

wing328 commented Sep 24, 2020

jfeltesse-mdsol commented Sep 25, 2020

wing328 commented Sep 28, 2020 • edited Loading

zippolyte Oct 1, 2020

Choose a reason for hiding this comment

jfeltesse-mdsol Oct 2, 2020

Choose a reason for hiding this comment

zippolyte Oct 2, 2020

Choose a reason for hiding this comment

zippolyte Oct 8, 2020

Choose a reason for hiding this comment

jfeltesse-mdsol Oct 8, 2020

Choose a reason for hiding this comment

zippolyte Oct 8, 2020

Choose a reason for hiding this comment

jfeltesse-mdsol commented Oct 2, 2020

wing328 commented Oct 5, 2020

jfeltesse-mdsol commented Oct 9, 2020

jfeltesse-mdsol commented Oct 12, 2020

jfeltesse-mdsol commented Oct 19, 2020

jfeltesse-mdsol commented Oct 27, 2020 • edited Loading

jfeltesse-mdsol commented Oct 27, 2020 • edited Loading

wing328 commented Oct 27, 2020

wing328 commented Oct 27, 2020

jfeltesse-mdsol commented Oct 27, 2020

wing328 commented Oct 27, 2020

jfeltesse-mdsol commented Oct 28, 2020

wing328 commented Oct 28, 2020

jfeltesse-mdsol commented Oct 28, 2020

jfeltesse-mdsol commented Oct 29, 2020 • edited Loading

jfeltesse-mdsol commented Oct 30, 2020

jfeltesse-mdsol commented Oct 31, 2020

zippolyte commented Nov 17, 2020

wing328 commented Nov 23, 2020

zippolyte commented Nov 23, 2020

wing328 commented Nov 23, 2020

zippolyte commented Nov 24, 2020 • edited Loading

jfeltesse-mdsol commented Nov 24, 2020

jfeltesse-mdsol commented Mar 25, 2020 •

edited

Loading

wing328 commented Sep 20, 2020 •

edited

Loading

wing328 commented Sep 28, 2020 •

edited

Loading

jfeltesse-mdsol commented Oct 27, 2020 •

edited

Loading

jfeltesse-mdsol commented Oct 27, 2020 •

edited

Loading

jfeltesse-mdsol commented Oct 29, 2020 •

edited

Loading

zippolyte commented Nov 24, 2020 •

edited

Loading