[PHP] By default should use static Configuration class instance

[DarkSide666]

For PHP.

By default should use static Configuration class instance, otherwise new instance is created every time you make an API call.

Before this PR every time you create new API object and do not pass Configuration object as 2nd argument to API class constructor it creates new Configuration instance. This is wrong, because Configuration class is meant to be Singleton - so Configuration::getDefaultConfiguration() should be used instead.

Please review: @jebentier @dkarlovi @mandrean @jfastnacht @ybelenko @renepardon

[wing328]

https://github.com/OpenAPITools/openapi-generator/actions/runs/11168507264/job/31063629502?pr=19775

please update the sample when you've time

[DarkSide666]

@wing328 could you please do this for me?
I'm on Windows and was trying to run generate_samples script from windows linux console (WSL), but still no luck.
It throws Error: Unable to access jarfile /mnt/c/wamp/www/DarkSide666/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar I guess this is not gonna work on Windows :(
Maybe I can just merge latest commits from your master branch into my PR and then it should be ok, isn't it?

[dkarlovi]

I'm OK with this change, using static methods and properties is a code smell in 99% of the cases, but here the assumption is you should get the (supposedly pre-configured) default config instead of just an empty config object.

Not because of performance but because of the behavior, the solution is still the same. 👍

[DarkSide666]

I'm OK with this change, using static methods and properties is a code smell in 99% of the cases, but here the assumption is you should get the (supposedly pre-configured) default config instead of just an empty config object.

Not because of performance but because of the behavior, the solution is still the same. 👍

Yes exactly. Normally because Configuration is static object, then it's meant to be pre-configured and then can be automatically be reused in all API calls done later in code.
But now you have to always do like this to use this "global" Configuration in your API classes:

$api = new \Example\Client\Api\PaymentsApi(
    null, // uses default \GuzzleHttp\Client()
    \Example\Client\Configuration::getDefaultConfiguration()
);

[wing328]

no worry i'll update the sample over the weekend and get this PR merged

have a nice weekend

[wing328]

tested updated samples locally and the result is good

PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

...............................................................  63 / 222 ( 28%)
............................................................... 126 / 222 ( 56%)
............................................................... 189 / 222 ( 85%)
.................................                               222 / 222 (100%)

Time: 00:01.512, Memory: 12.00 MB

OK (222 tests, 384 assertions)

will update the samples after merging this PR into master

thanks again for the PR

[Pittiplatsch]

Just my 2 cts to this closed issue:

This change of behavior is a severe BC break!

In a dynamic environment (with underlying caching of API instances), I explicitly relied on receiving a new config instance every time:

$apiClass = 'Some/Dynamically/Determined/GeneratedApi';

$api = new ($apiClass)(/* no configuration passed */);

// Fetch correct config instance from $api
// - don't want to do the 'Some/Dynamically/Determined/...' stuff for configuration again
$apiConfig = $api->getConfig();
$apiConfig->setHost($specificHost);

return $api->callDaStuff(); // API call uses $specificHost

With this PR in place, changing $apiConfig as in following example (which worked before this PR) the default configuration instance, and consequently the configuration of all API instances using the default configuration instance:

$gugl = new FinderApi();
$gugl->getConfig()->setHost('https://www.gugl.com');
$guglEaster = $gugle->find('easteregg');     // "GUGL's easteregg

$yehu = new FinderApi();
$yehu->getConfig()->setHost('https://www.yehu.com');
$yehuEaster = $yehu->find('easteregg');     // "YEHU's easteregg


// Now, use $gugl again:
$guglXmas = $gugle->find('santa');     // "YEHU's santa (!!!!!!)

---

Lazy workaround:
Update documentation 😉

Fix:
The paradigm of proper encapsulation expects every API instance to hold its own configuration which may be manipulated without affecting other API instances.

API class (api.mustache) should clone the received default config:

--- $this->config = $config ?: Configuration::getDefaultConfiguration();
+++ $this->config = $config ?: clone Configuration::getDefaultConfiguration();

However, not sure if Configuration needs a magic __clone() for deep cloning...
And: this fix recovers former behavior, but obviously itself is a BC break to current behavior, requiring documentation 😉

---

Any opinions on this? @wing328