search

googleads / google-ads-php

Google Ads API Client Library for PHP
https://developers.google.com/google-ads/api/docs/client-libs/php
Apache License 2.0
277 stars 260 forks source link

Remove final from all classes of this repository #1026

Open gmponos opened 2 months ago

gmponos commented 2 months ago

Your client library and Google Ads API versions:

Your environment:

Description of the bug:

It has been raised in various issues that since (I think) v21.0 where you marked many classes as final it is a trouble for many developers to mock/test their applications.

You can find other related GH issues here:

https://github.com/googleads/google-ads-php/issues/1008 https://github.com/googleads/google-ads-php/issues/639 https://github.com/googleads/google-ads-php/issues/294 https://github.com/googleads/google-ads-php/issues/347

It is mentioned in these issues that the practice of marking classes as final should be done when you are following some very specific design/coding practices. More info when to mark as final your classes can be found here:

https://ocramius.github.io/blog/when-to-declare-classes-final/

As a result our application is not able to move from v15 to v16 because currently we are unable to mock classes of this package. If we are not able to mock the classes then our Unit tests will end up performing actual HTTP calls over the internet. This has various defects as our pipelines/jobs depend on external resources.

More specifically this is the class that we are trying to mock https://github.com/googleads/google-ads-php/blob/main/src/Google/Ads/GoogleAds/V16/Services/Client/ConversionUploadServiceClient.php but we can't.

I would advise to remove the final from all your classes since the package in many cases does not follow design and coding practices to support having classes as final and it would be troublesome to come back and report each class every time there is a problem with that one.

gmponos commented 1 month ago

Hi, any response on this one?

fiboknacky commented 1 month ago

@gmponos

Sorry for the delayed response! I need to talk to the source generation tool owner and we may be able to overwrite its behavior for the Google Ads API library. I know that it's not ideal for you, but in the meantime, you can consider using this while waiting for the change.