Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make httpClient() a public, static method #1810

Merged
merged 2 commits into from
Jan 29, 2025

Conversation

dejagersh
Copy link
Contributor

@dejagersh dejagersh commented Jan 29, 2025

Why?

I want to do something like this with ApiRequestor::setHttpClient():

in tests:

ApiRequestor::setHttpClient($this->createMock(...));

in application code:

ApiRequestor::setHttpClient(new LoggingHttpClient($this->logger))

If you first execute the ApiRequestor::setHttpClient in test, and then afterwards the ApiRequestor::setHttpClient() in application code, then the mocked http client is gone and this goes wrong. What I think would be a nice solution: allow me to decorate the configured http client:

class LoggingHttpClient implements ClientInterface
{
    public function __construct(
        private LoggerInterface $logger,
        private ClientInterface $decorated,
    ) {
    }

    /**
     * @see ClientInterface::request()
     */
    public function request($method, $absUrl, $headers, $params, $hasFile): array
    {
        [$rawResponseBody, $statusCode, $responseHeaders] = $this->decorated->request(
            $method,
            $absUrl,
            $headers,
            $params,
            $hasFile,
        );

        // $this->logger->...

        return [$rawResponseBody, $statusCode, $responseHeaders];
    }
}

And then wherever I configure the logging http client I can instantiate this class decorating the previously configured http client:

ApiRequestor::setHttpClient(new LoggingHttpClient($this->logger, ApiRequestor::httpClient());

Now LoggingHttpClient decorates the mock client and things work as intended. It kinda becomes a middleware pattern, and opens the door for other middleware, e.g.:

ApiRequestor::setHttpClient(
    new MetricsHttpClient(
        new RetryingHttpClient(
            new CachingHttpClient(
                new LoggingHttpClient(
                    $this->logger,
                    ApiRequestor::httpClient()
                ),
                $this->cache
            )
        ),
        $this->metrics
    )
);

What?

  • Make httpClient() public and static

See Also

NA

Copy link

cla-assistant bot commented Jan 29, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

cla-assistant bot commented Jan 29, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
Copy link

cla-assistant bot commented Jan 29, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@helenye-stripe helenye-stripe merged commit 4dd0463 into stripe:master Jan 29, 2025
22 checks passed
xavdid-stripe added a commit that referenced this pull request Feb 7, 2025
* Update generated code (#1793)

* Update generated code for v1399

* Update generated code for v1409

* Update generated code for v1412

---------

Co-authored-by: Stripe OpenAPI <105521251+stripe-openapi[bot]@users.noreply.github.com>

* Bump version to 16.4.0

* Added pull request template (#1797)

* Add justfile, remove coveralls, and fix AUTOLOAD in CI (#1801)

* add justfile and tweak CI + readme

* debug test

* Debugging

* further debugging

* restore original composer json

* add more logging

* maybe fix ci

* Fix typo

* fix test naming and pass autoload directly to recipe as arugment

* Remove unused logline and fix typo

* restore composer

* remove extra log line

* update justfile

* add comments

* update ci

* revert to gh action

* ensure dependencies are installed for format and test recipes (#1802)

* Added CONTRIBUTING.md file (#1806)

* minor justfile fixes (#1807)

* made v2 event class concrete, and changed convertToStripeObject to use it if we cannot find the identified event subclass (#1805)

* Update generated code for v1461

* Update generated code (#1804)

* Update generated code for v1441

* Update generated code for v1452

* Update generated code for v1455

---------

Co-authored-by: Stripe OpenAPI <105521251+stripe-openapi[bot]@users.noreply.github.com>
Co-authored-by: helenye-stripe <[email protected]>

* Bump version to 16.5.0

* Make `httpClient()` a public, static method (#1810)

* Make `httpClient()` a public, static method

* Make `streamingHttpClient()` a public, static method

* Update generated code for v1467

* Raise a more descriptive error if null bytes are found in the path (#1811)

* raise a more descriptive error if null bytes are found in the path

* formatting

* change error thrown for null byte issues (#1813)

* Update generated code for v1472

---------

Co-authored-by: stripe-openapi[bot] <105521251+stripe-openapi[bot]@users.noreply.github.com>
Co-authored-by: Jesse Rosalia <[email protected]>
Co-authored-by: David Brownman <[email protected]>
Co-authored-by: helenye-stripe <[email protected]>
Co-authored-by: Helen Ye <[email protected]>
Co-authored-by: Johan de Jager <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants