[ruby] Support for per-operation servers #7415
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
jimschubert
approved these changes
Sep 21, 2020
jimschubert
added a commit
to Marvin1912/openapi-generator
that referenced
this pull request
Sep 25, 2020
* master: (1357 commits) minor formatting change (OpenAPITools#7503) [ruby] Fix generation when generateAliasAsModel is enabled (OpenAPITools#7419) [sonar][ci] Fixes some new recommended "bug" fixes found via Sonar (OpenAPITools#7508) Add option to set recursion limit (OpenAPITools#7491) regenerate java pkmst petstore samples (OpenAPITools#7502) fix issue without http signature auth (OpenAPITools#7504) [ruby] Support for per-operation servers (OpenAPITools#7415) Add a link to react native tutorial (OpenAPITools#7505) update of Objective-C model-body.mustache to support discriminator.mappedModels (OpenAPITools#7471) update samples [ci] Sonar bug fixes (OpenAPITools#7497) fixed the backslash issue (OpenAPITools#7492) Fix the range check condition (OpenAPITools#7482) [batch] Option to clean files before generation (OpenAPITools#7465) [java][groovy] Fix mangled src paths in outputs on Windows (OpenAPITools#7487) remove haskell stack installation in shippable ci (OpenAPITools#7481) remove hardcoded PetApi (OpenAPITools#7483) remove SWGApiResponse.m (not used) (OpenAPITools#7478) update samples [typescript-axios] support "x-enum-descriptions" (OpenAPITools#7477) ...
jimschubert
added a commit
to jimschubert/openapi-generator
that referenced
this pull request
Sep 25, 2020
* master: (2170 commits) [ci][test] Fix samples (OpenAPITools#7509) [bug][Java] Honor instantiationMap for default values for array, map, set (OpenAPITools#4982) Fix stringifying of ~= (OpenAPITools#7460) minor formatting change (OpenAPITools#7503) [ruby] Fix generation when generateAliasAsModel is enabled (OpenAPITools#7419) [sonar][ci] Fixes some new recommended "bug" fixes found via Sonar (OpenAPITools#7508) Add option to set recursion limit (OpenAPITools#7491) regenerate java pkmst petstore samples (OpenAPITools#7502) fix issue without http signature auth (OpenAPITools#7504) [ruby] Support for per-operation servers (OpenAPITools#7415) Add a link to react native tutorial (OpenAPITools#7505) update of Objective-C model-body.mustache to support discriminator.mappedModels (OpenAPITools#7471) update samples [ci] Sonar bug fixes (OpenAPITools#7497) fixed the backslash issue (OpenAPITools#7492) Fix the range check condition (OpenAPITools#7482) [batch] Option to clean files before generation (OpenAPITools#7465) [java][groovy] Fix mangled src paths in outputs on Windows (OpenAPITools#7487) remove haskell stack installation in shippable ci (OpenAPITools#7481) remove hardcoded PetApi (OpenAPITools#7483) ...
jimschubert
added a commit
to david1601/openapi-generator
that referenced
this pull request
Sep 28, 2020
…or-extension-x-spring-paginated * master: (29 commits) [Core, HTML2] Rendered docs are incomplete (OpenAPITools#7510) [C][Client]Support data callback function (OpenAPITools#7467) [typescript-axios] replace optional chaining (OpenAPITools#7476) [typescript-axios] fix refining `query` (OpenAPITools#7475) [powershell] Updated the Api doc with httpsiging script snippet. (OpenAPITools#7499) [go][client] Fix API docs example generation (OpenAPITools#7489) [ci][test] Fix samples (OpenAPITools#7509) [bug][Java] Honor instantiationMap for default values for array, map, set (OpenAPITools#4982) Fix stringifying of ~= (OpenAPITools#7460) minor formatting change (OpenAPITools#7503) [ruby] Fix generation when generateAliasAsModel is enabled (OpenAPITools#7419) [sonar][ci] Fixes some new recommended "bug" fixes found via Sonar (OpenAPITools#7508) Add option to set recursion limit (OpenAPITools#7491) regenerate java pkmst petstore samples (OpenAPITools#7502) fix issue without http signature auth (OpenAPITools#7504) [ruby] Support for per-operation servers (OpenAPITools#7415) Add a link to react native tutorial (OpenAPITools#7505) update of Objective-C model-body.mustache to support discriminator.mappedModels (OpenAPITools#7471) update samples [ci] Sonar bug fixes (OpenAPITools#7497) ...
| # Returns base URL for specified operation based on server settings | ||
| def base_url(operation = nil) | ||
| index = server_operation_index.fetch(operation, server_index) | ||
| return "#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '') if index == nil |
There was a problem hiding this comment.
Hi @jimschubert
By default server_index is 0 here which means by default the if index == nil will never be true. Can we change this to a better check and set the server_index to zero only if the host is NULL or something?
We have the issue where we cannot set the server because of the issue above. The caller needs to explicitly set the server_index to nil in order for host to work properly
There was a problem hiding this comment.
Running into this issue as well. As @laertispappas says: the if index == nil will never be true (well, unless people explicitly set the operation in the server_operation_index Hash with the value nil or set server_index to nil, both of which do not seem to make much sense.
I think no one is using this template successfully. It seems unlikely that everyone is using per-operation servers.
If you patch line 186 to be
index = server_operation_index[operation]then everything works fine.
There was a problem hiding this comment.
can you please file a PR with the suggested patch when you've time?
Confusion
added a commit
to Confusion/openapi-generator
that referenced
this pull request
Apr 9, 2023
…ex is defined As discussed in OpenAPITools#7415 (comment), it seems unlikely this was correct. server_operation_index is a hash table. In Ruby, `hash[key]` will return the value associated with `key`. If key is absent, `nil` is returned. Because that is sometimes undesirable, there is also `hash.fetch(key)`, which raises an error if the key is absent. It also allows you to specify a default to fall back on: `hash.fetch(key, default)` will return `default` if the key is absent. So, since not all users will specify a 'server per operation' (or at least: I'm not), the old code would usually set `index` to the `server_index`, which is initialized to 0. The subsequent `if index == nil` will usually return false (`0 != nil` in Ruby), after which the `server_url` call on line 177 constructs the url based on the `server_operation_variables` and `operation_server_settings`, assuming we are dealing with the case where a server per operation is configured. The case where the url should be constructed from `scheme`, `host`, etc. is only called if either `server_index` is explicitly set to `nil` or the key `operation` is explicitly associated with the value `nil` in the `server_operation_index` hash table, both of which seem inappropriate.
Confusion
added a commit
to Confusion/openapi-generator
that referenced
this pull request
Apr 9, 2023
As discussed in OpenAPITools#7415 (comment), it seems unlikely the code was correct. server_operation_index is a hash table. In Ruby, `hash[key]` will return the value associated with `key`. If key is absent, `nil` is returned. Because that is sometimes undesirable, there is also `hash.fetch(key)`, which raises an error if the key is absent. It also allows you to specify a default to fall back on: `hash.fetch(key, default)` will return `default` if the key is absent. So, since not all users will specify a 'server per operation' (or at least: I'm not), the old code would usually set `index` to the `server_index`, which is initialized to 0. The subsequent `if index == nil` will usually return false (`0 != nil` in Ruby), after which the `server_url` call on line 177 constructs the url based on the `server_operation_variables` and `operation_server_settings`, assuming we are dealing with the case where a server per operation is configured. The case where the url should be constructed from `scheme`, `host`, etc. is only called if either `server_index` is explicitly set to `nil` or the key `operation` is explicitly associated with the value `nil` in the `server_operation_index` hash table, both of which seem inappropriate.
6 tasks
wing328
pushed a commit
that referenced
this pull request
Apr 21, 2023
…#15162) As discussed in #7415 (comment), it seems unlikely the code was correct. server_operation_index is a hash table. In Ruby, `hash[key]` will return the value associated with `key`. If key is absent, `nil` is returned. Because that is sometimes undesirable, there is also `hash.fetch(key)`, which raises an error if the key is absent. It also allows you to specify a default to fall back on: `hash.fetch(key, default)` will return `default` if the key is absent. So, since not all users will specify a 'server per operation' (or at least: I'm not), the old code would usually set `index` to the `server_index`, which is initialized to 0. The subsequent `if index == nil` will usually return false (`0 != nil` in Ruby), after which the `server_url` call on line 177 constructs the url based on the `server_operation_variables` and `operation_server_settings`, assuming we are dealing with the case where a server per operation is configured. The case where the url should be constructed from `scheme`, `host`, etc. is only called if either `server_index` is explicitly set to `nil` or the key `operation` is explicitly associated with the value `nil` in the `server_operation_index` hash table, both of which seem inappropriate.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for configurable servers per operation.
Related PRs for other languages
Addresses #590
PR checklist
./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.master