[BUG] [Ruby] Model property validation can be improved

[ckoegel]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

In the ruby-client generator, schema properties that are required or have other constraints are generated with custom attribute assignment functions that validate said properties. The way this is currently implemented can lead to NoMethodErrors inside the list_invalid_properties method. If a property is required and has another validation (e.g. maximum), but that attribute is not passed in to the initialize function, the if attributes.key?(:'{{{name}}}') block does not assign the attribute, which skips the validation for that attribute. If the user then calls the list_invalid_properties method, 'invalid value for "{{{name}}}", {{{name}}} cannot be nil.') is pushed to the invalid_properties Array, but the function does not return. A property that also has a maximum will then attempt to run @{{{name}}} >{{#exclusiveMaximum}}={{/exclusiveMaximum}} {{{maximum}}} due to the fact that all of the {{required}} checks in this file (e.g line 168) are prefixed with ^. This leads to a NoMethodError as the NilClass does not support this method.

Having the function return early or always prefixing the checks with the !nil? block would fix this issue, but I don't see a reason for the list_invalid_properties or valid? functions to exist for these models at all. It seems the design was supposed to validate all attributes upon initialization, via the build_from_hash method, and via direct assignment (=). If this is that case, is an invalid model ever supposed to exist? Currently the only way I can get a model to have an invalid attribute is by leaving an attribute nil by never declaring that attribute. Only in that case will the attribute be invalid, but this can be solved by adding an else to assign the attribute to nil if it does not have a default value in the if statement surrounding the attribute assignment in the initialize method. This should cause any validation errors to be caught in the assignment validation.

I may be misunderstanding the function of the list_invalid_properties and valid? methods, so if I am, could I get some clarity here?

openapi-generator version

6.6.0

OpenAPI declaration file content or url

Spec File

Generation Details

our openapi-config.yml

gemAuthor: Bandwidth
gemAuthorEmail: [email protected]
gemDescription: The official client SDK for Bandwidth's Voice, Messaging, MFA, and WebRTC APIs
gemHomepage: https://github.com/Bandwidth/ruby-sdk
gemLicense: MIT
gemName: bandwidth-sdk
gemRequiredRubyVersion: '>=2.7'
gemSummary: Bandwidth Ruby SDK
gemVersion: 11.0.0
library: faraday
moduleName: Bandwidth

Steps to reproduce

Generate client using openapi-generator-cli generate -g ruby -i bandwidth.yml -c openapi-config.yml -o ./
Then create a new Bandwidth::VerifyCodeRequest with no arguments and call its list_invalid_properties method.

verify_code_request_invalid = Bandwidth::VerifyCodeRequest.new
verify_code_request_invalid.list_invalid_properties

This causes the NoMethodError

Failure/Error: if @expiration_time_in_minutes > 15
     
 NoMethodError:
   undefined method `>' for nil:NilClass
     
             if @expiration_time_in_minutes > 15

Related issues/PRs

N/A

Suggest a fix

The if statements in the list_invalid_properties method should be always prefixed with the !nil? check to fix the NoMethodError. If the attribute validation is not working as intended as I've described above, then the else self.name = nil should be added to the if statements around the attribute assignments in the initialize method. A final else can be added to the build_from_hash method to mimic the new behavior of the initialize method, and the list_invalid_properties and valid? methods can likely be removed. This would be a breaking change to the SDK and may be unnecessary, so please let me know if this work is worth doing and I'll gladly open a PR to do some or all of the work mentioned.

[wing328]

Agreed. As a starting point we can improve it by adding getter, setter with validations to the Ruby models.