search

googleads / googleads-php-lib

Google Ad Manager SOAP API Client Library for PHP
Apache License 2.0
656 stars 769 forks source link

PHP 8.2 - Creation of dynamic property Google\AdsApi\AdManager\v202211\ReportService::$__last_request is deprecated #773

Closed nunoperalta closed 1 year ago

nunoperalta commented 1 year ago

Dynamic properties in classes are deprecated in PHP 8.2.

There is an attribute that can be added (#[AllowDynamicProperties]), but best is to define the property in advance.

PHP Fatal error [8192]: Creation of dynamic property Google\AdsApi\AdManager\v202211\ReportService::$__last_request is deprecated Stack trace:

0 /.../vendor/googleads/googleads-php-lib/src/Google/AdsApi/Common/AdsSoapClient.php(101)

1 [internal function]: Google\AdsApi\Common\AdsSoapClient->__doRequest('...', 'https://ads.google.com/apis/ads/publisher/v202211/ReportService?wsdl', '', 1, false)

2 /.../vendor/googleads/googleads-php-lib/src/Google/AdsApi/Common/AdsSoapClient.php(151): SoapClient->__soapCall('getSavedQueriesByStatement', Array, NULL, Array, Array)

3 /.../vendor/googleads/googleads-php-lib/src/Google/AdsApi/AdManager/v202211/ReportService.php(155): Google\AdsApi\Common\AdsSoapClient->__soapCall('getSavedQueriesByStatement', Array)

PierrickVoulet commented 1 year ago

This issue is in generated code. A fix would be required in the wsdl2phpgenerator project.

Naki commented 1 year ago

@PierrickVoulet this error is from AdsSoapClient not generated ReportService.

https://github.com/googleads/googleads-php-lib/blob/main/src/Google/AdsApi/Common/AdsSoapClient.php#L101

$this->__last_request = $request;

I think this is a bug because this variable is private in SoapClient. It is retrieved using $this->__getLastRequest(), which will return a variable from the SoapClient not AdsSoapClient.

The value is already set in SoapClient::__doRequest(). Therefore, the solution is to either remove this line or create a __getLastRequest() function and object property. I suggested the deletion because the code currently behaves as if it is not there. However, I don't know what the purpose of this piece of code was.

I also added, some other changes for PHP 8.2. Currently, everything works in my case. There are also no errors in PHPUnit or PHPStan analysis.

Pull request: https://github.com/googleads/googleads-php-lib/pull/774

PierrickVoulet commented 1 year ago

Thanks for the additional analysis, @Naki. I agree that removing this line seems to be the right thing to do, it would be consistent with what we currently do for the response data which works similarly (see logSoapCall). For the record, the getLastRequest and getLastResponse methods only work when the trace option is set to true (see PHP doc) which is our case (see service factory).

PierrickVoulet commented 1 year ago

Fixed in 62.0.0.