Add support for accessing dbal native connection

[simPod]

Dbal V3 contains:

/**
 * Connection interface.
 * Driver connections must implement this interface.
 *
 * @method resource|object getNativeConnection()
 */
interface Connection

[ste93cry]

Thank you for the contribution! While making the required changes, please also add a CHANGELOG entry and the missing unit tests. For the latter, you can take a look at how they got implemented for the similar methods. Finally, please retarget the PR to the develop branch as this is a feature and not a bug fix and will be released with the next minor version

[simPod]

Aight, thanks for review, you'll have it tomorrow evening CET

…

On Fri, Feb 4, 2022, 23:27 Stefano Arlandini ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Tracing/Doctrine/DBAL/TracingDriverConnection.php <#597 (comment)> : > @@ -193,6 +194,15 @@ public function getWrappedConnection(): DriverConnectionInterface return $this->decoratedConnection; } + public function getNativeConnection() + { + if (!method_exists($this->decoratedConnection, 'getNativeConnection')) { + throw new LogicException(sprintf('The decoratedConnection %s does not support accessing the native connection.', \get_class($this->decoratedConnection))); Yes please, we're always throwing a BadMethodCallException in similar cases so I prefer to keep doing it here too for consistency. Moreover, I don't think that a LogicException is appropriate to be honest as there is no error of logic here, but just a method you're calling when you should not. Message instead doesn't change much, it's just nitpicking 😃 — Reply to this email directly, view it on GitHub <#597 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQAJKJYDOPK4ZQSLD6PHLUZRHGDANCNFSM5NSEQVMA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[simPod]

@ste93cry how do you want to approach tests?

1. TracingServerInfoAwareDriverConnectionTest fails because we moved method to @method so it cannot be mocked Add support for accessing dbal native connection #597 (comment)
2. TracingDriverConnectionTest fails when run on dbal v2

[ste93cry]

I think we can declare a NativeDriverConnectionInterface interface that extends TracingDriverConnectionInterface and declares the getNativeConnection() method. This class would be declared in the test file and we should mock it instead of the original one. The Doctrine team did the same here, so it should work

[simPod]

Ok done

[ste93cry]

Can you please add the missing testGetNativeConnectionThrowsExceptionIfDecoratedConnectionDoesNotImplementMethod test method so that we cover the exception path? Note that since the class of the connection is anonymous, you should use expectExceptionMessageMatches

[simPod]

Thanks for review, appreciate the strictnesss

[simPod]

Yup, it was phpstan

…

On Mon, Feb 14, 2022, 23:33 Stefano Arlandini ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Tracing/Doctrine/DBAL/TracingDriverConnection.php <#597 (comment)> : > @@ -164,6 +165,20 @@ public function rollBack(): bool }); } + /** + * ***@***.***} + * + * @return resource|object Is PHPStan throwing this error? Weird, it should be able to read the @method annotations 🤔 Anyway, it's not that important, it was just a matter of not repeating the typehint — Reply to this email directly, view it on GitHub <#597 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQAJNVWFMKDN4M2JJBNU3U3F7LDANCNFSM5NSEQVMA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[ste93cry]

Thank you for the contribution and the patience making all the changes! LGTM 🥳