Skip to content

Conversation

@homersimpsons
Copy link
Collaborator

@homersimpsons homersimpsons commented Dec 11, 2025

Fixes #259

Note that there are two alternative implementations:

  1. e9445ba which uses an anonymous type and requires response type juggling (between symfony and psr7).
    Note that the anonymous class declaration is rather verbose and would have been simplified by https://wiki.php.net/rfc/property-capture
  2. 5114346 (current) which uses concrete classes with a DummyResponseWithRequest allowing us to extract the ecodev/graphql-upload psr7 request from the middleware chain to allow passing it in our previous symfony flow.

@andrew-demb note that I would like this for the v6 of this package, how would you like to proceed?

@homersimpsons homersimpsons force-pushed the fix/support-ecodev-graphql-upload-v8 branch 2 times, most recently from 07a4a76 to 00e934b Compare December 11, 2025 09:17
@homersimpsons homersimpsons force-pushed the fix/support-ecodev-graphql-upload-v8 branch from 00e934b to 5114346 Compare December 11, 2025 09:21
*/
class RequestExtractorMiddleware implements RequestHandlerInterface
{
public function handle(ServerRequestInterface $request): ResponseInterface

This comment was marked as resolved.

use Psr\Http\Message\ServerRequestInterface;

/**
* Class used to allow extraction of request from PSR-15 middleware
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please mark introduced classes with @internal tag to avoid using them from the outside

$psr7Request = $uploadMiddleware->processRequest($psr7Request);
\assert($psr7Request instanceof ServerRequestInterface);
$dummyResponseWithRequest = $uploadMiddleware->process($psr7Request, new RequestExtractorMiddleware());
assert($dummyResponseWithRequest instanceof DummyResponseWithRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is not so strict as to be sure that you will have a DummyResponseWithRequest instance.

I suggest to use if + throw in case of unexpected result


/**
* Listens to every single request and forward Graphql requests to Graphql Webonix standardServer.
* Listens to every single request and forwards GraphQL requests to Webonyx's \GraphQL\Server\StandardServer.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Listens to every single request and forwards GraphQL requests to Webonyx's \GraphQL\Server\StandardServer.
* Listens to every single request and forwards GraphQL requests to Webonyx's {@see \GraphQL\Server\StandardServer}.

@andrew-demb
Copy link
Collaborator

@andrew-demb note that I would like this for the v6 of this package, how would you like to proceed?

I'm OK with introducing it for the v6, we too had problems with update graphqlite to v8.

Let's retarget to 6.x branch then.

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.

Support ecodev/graphql-upload v8

3 participants