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

[11.x] Introduce method Http::createPendingRequest() #50980

Merged
merged 3 commits into from Apr 9, 2024

Conversation

Jacobs63
Copy link
Contributor

@Jacobs63 Jacobs63 commented Apr 9, 2024

This PR introduces a new public method to the HTTP Factory.

Currently, the HTTP factory does not provide any method to return an instance of PendingRequest which would not have side-effects.

Imagine a scenario, where we'd like to encapsulate clients for multiple services, which require API calls. To do that, we'd like to use the PendingRequest and have each client configure the underlying PendingRequest, if it needs any special configuration.

The sample code might look as follows:

<?php

declare(strict_types=1);

namespace App\Client;

use Illuminate\Http\Client\Factory;
use Illuminate\Http\Client\PendingRequest;
use Illuminate\Support\Traits\ForwardsCalls;

/**
 * @mixin PendingRequest
 */
abstract readonly class AbstractPendingRequestBackedClient
{
    use ForwardsCalls;

    public function __construct(private Factory $factory)
    {
    }

    public function __call(string $name, array $arguments): mixed
    {
        return $this->forwardCallTo(
            $this->newPendingRequest(),
            $name,
            $arguments,
        );
    }

    public function getPendingRequest(): PendingRequest
    {
        return $this->newPendingRequest();
    }

    private function newPendingRequest(): PendingRequest
    {
        return tap(
            $this->factory->throw(), // <<< the issue, there's no method without side-effects
            $this->configurePendingRequest(...),
        );
    }

    abstract protected function configurePendingRequest(PendingRequest $pendingRequest): void;
}

Currently, there is no way to instantiate a PendingRequest without calling a method from the underlying Factory. However, none of these methods return just a PendingRequest instance, all of them have a side-effect.
At the same time, we cannot simply use dependency injection or the container to instantiate a new PendingRequest, because we'd lose the global middleware, global options, stubs & preventing stray requests, which are provided by the underlying Factory through its __call & newPendingRequest methods.

Hence why I suggest to either add a new method to the factory, or make the newPendingRequest method public (which, however, would be a breaking change and likely not a viable solution, unless targeting 12.x).

@Jacobs63 Jacobs63 changed the title Introduce method \Illuminate\Http\Client\Factory::createPendingRequest() [11.x] Introduce method \Illuminate\Http\Client\Factory::createPendingRequest() Apr 9, 2024
@Jacobs63 Jacobs63 changed the title [11.x] Introduce method \Illuminate\Http\Client\Factory::createPendingRequest() [11.x] Introduce method Http::createPendingRequest() Apr 9, 2024
@taylorotwell taylorotwell merged commit e0f2f6c into laravel:11.x Apr 9, 2024
28 checks passed
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.

None yet

2 participants