Created by: thomasphansen
The current PHP client has no support for server variables inside operations. This PR offers a solution for that, based on the same code used for server variables inside the Configuration
class.
Current logic
The Configuration
class creates an array of server settings, where each server's index is used as the selector for the different servers. A method called getHostFromSettings()
will pick the chosen server, interpolate the variables and return the host string based on that. This works fine for global servers and server variables.
For operation-specific servers, the current code only implements the choice of a server: the API class implements a method called setHostIndex()
, which will be used inside the Request*()
methods to choose a server. There is no way you can set values for eventual server variables, and calling and endpoint with them may result in a call to a host like 'https://{server}.address.com:{port}/{some_variable}/endpoint', with no interpolation of the variables.
This approach has (at least) two critical problems. Apart from the obvious one (variables don't work), calling two different endpoints from the same API without calling setHostIndex()
between them may result in unexpected behavior. Consider the following fragment:
# scenario: OperationA() has two servers, but OperationB() has only one.
# set host index to the second server for operation OperationA()
$api->setHostIndex(1);
$result = $api->OperationA();
$result = $api->OperationB(); # this will throw `InvalidArgumentException`, as the index is out of range
The second call will fail because the $api object holds the state for the host index between the calls; OperationB() will look for a server with index 1, which doesn't exist.
Solution for Operation-specific servers and variables
In order to find a solution for these problems, I started with three basic premises:
- No breaking changes: the current behavior, including its fallacies, must be preserved
- The solution must be independent for each operation: no states saving, no "global" settings
- Follow the current behavior as much as possible.
To achieve that, we need to add two extra parameters to each operation: the $hostIndex and the $variables array. The code is using the same logic in the Configuration
class, which was slightly reorganized to make it reusable:
-
Configuration::getHostFromSettings()
has been split into two methods: a staticgetHostString()
, that interpolates the variables based on a given array of settings, and thegetHostFromSettings()
itself, that calls the previous one to preserve the old behavior. - For each operation having specific servers, we add a new protected method to build the array of server settings.
- This array is given to
Configuration::getHostString()
, together with the $hostIndex and $variables array, to get the correct host string. - If no $hostIndex/$variables are provided, then we fallback to the old behavior, using the API's
hostIndex
to choose the server, making the code backward-compatible.
A call to the addPet()
operation would look like:
$api = new PetApi();
$pet = new Pet();
$api->addPet($pet, 1); # will call the operation-specific second server
$api->addPet($pet, 2, ['server' => 'petstore', 'port' => '8080']); # will call the third server , interpolating variables
# Old behavior
$api->setHostIndex(1);
$api->addPet($pet); # will call the operation-specific second server
Please notice that the "old" behavior is still present, and still saves state in $api. This needs to be preserved, in order to avoid breaking changes.
I have added a third server to the addPet
operation in petstore-with-fake-endpoints-models-for-testing.yaml, to include a test case for operation-specific variables. I've also added a new test file ('ServerVariablesInOperationTest.php').
Finally, looks like the PHP generator never overwrites files in the 'Test' folder: because of that, there were many old (from vesion 5.0) files there, including some for classes that don't exist anymore. So, I've deleted both lib and test folders before running the generate_samples script, to guarantee that all files are rebuilt from scratch and the old ones are purged.
Hope you appreciate!
PR checklist
-
Read the contribution guidelines. -
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community. -
Run the following to build the project and update samples: ./mvnw clean package ./bin/generate-samples.sh ./bin/utils/export_docs_generators.sh
./bin/generate-samples.sh bin/configs/java*
. For Windows users, please run the script in Git BASH. -
File the PR against the correct branch: master
(6.1.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks) -
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. @jebentier, @dkarlovi, @mandrean, @jfastnacht, @ybelenko, @renepardon