From 015b69f84c10505aad731c94b8379500d75c8e86 Mon Sep 17 00:00:00 2001 From: Pavlo Pavliukovych Date: Sun, 31 May 2026 12:17:33 +0200 Subject: [PATCH 1/4] Add multipart/form-data support via request data extractors --- .../DefaultRequestDataExtractor.php | 49 ++++ .../MultipartRequestDataExtractor.php | 81 ++++++ .../RequestDataExtractorInterface.php | 28 ++ ArgumentResolver/ServiceRequestResolver.php | 114 ++++---- Resources/config/services.yml | 16 + ...odeDenormalizeAwareSerializerInterface.php | 22 -- .../DefaultRequestDataExtractorTest.php | 114 ++++++++ .../MultipartRequestDataExtractorTest.php | 148 ++++++++++ .../ServiceRequestResolverTest.php | 275 +++++++++--------- composer.json | 12 +- 10 files changed, 651 insertions(+), 208 deletions(-) create mode 100644 ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php create mode 100644 ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php create mode 100644 ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php delete mode 100644 Tests/ArgumentResolver/DecodeDenormalizeAwareSerializerInterface.php create mode 100644 Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php create mode 100644 Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php diff --git a/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php new file mode 100644 index 0000000..c5ee756 --- /dev/null +++ b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php @@ -0,0 +1,49 @@ +decoder = $decoder; + } + + public function supports(EndpointInterface $endpoint): bool + { + return true; + } + + public function extract(Request $request, EndpointInterface $endpoint): array + { + $body = $request->getContent(); + $decoded = !empty($body) + ? $this->decoder->decode($body, $endpoint->getRequestFormat()) + : []; + + return array_merge( + $decoded, + $request->attributes->all(), + $request->query->all() + ); + } +} diff --git a/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php new file mode 100644 index 0000000..162d483 --- /dev/null +++ b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php @@ -0,0 +1,81 @@ +streamFactory = $streamFactory; + } + + public function supports(EndpointInterface $endpoint): bool + { + return self::FORMAT === $endpoint->getRequestFormat(); + } + + public function extract(Request $request, EndpointInterface $endpoint): array + { + if (null === $this->streamFactory) { + throw new LogicException( + sprintf( + 'A PSR-17 "%s" must be wired to handle multipart/form-data endpoints. ' + . 'Install a PSR-7 implementation (e.g. guzzlehttp/psr7, nyholm/psr7) ' + . 'and register its stream factory.', + StreamFactoryInterface::class + ) + ); + } + + return array_merge( + $request->request->all(), + $this->wrapFiles($request->files->all()), + $request->attributes->all(), + $request->query->all() + ); + } + + private function wrapFiles(array $files): array + { + $wrapped = []; + foreach ($files as $key => $value) { + if (is_array($value)) { + $wrapped[$key] = $this->wrapFiles($value); + continue; + } + + if ($value instanceof UploadedFile) { + $stream = $this->streamFactory->createStreamFromFile($value->getRealPath(), 'r'); + $wrapped[$key] = new UploadedFileStream($stream, $value); + } + } + + return $wrapped; + } +} diff --git a/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php b/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php new file mode 100644 index 0000000..7da6676 --- /dev/null +++ b/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php @@ -0,0 +1,28 @@ + */ - private $serviceResponseListener; + private iterable $requestDataExtractors; /** - * ServiceRequestResolver constructor. - * - * @param SerializerInterface $serializer - * @param EndpointRegistryInterface $endpointRegistry - * @param ServiceResponseListener $serviceResponseListener + * @param iterable $requestDataExtractors */ public function __construct( - SerializerInterface $serializer, + DenormalizerInterface $denormalizer, EndpointRegistryInterface $endpointRegistry, - ServiceResponseListener $serviceResponseListener + ServiceResponseListener $serviceResponseListener, + iterable $requestDataExtractors ) { - $this->serializer = $serializer; + $this->denormalizer = $denormalizer; $this->endpointRegistry = $endpointRegistry; $this->serviceResponseListener = $serviceResponseListener; + $this->requestDataExtractors = $requestDataExtractors; } /** - * {@inheritdoc} + * {@inheritdoc} */ public function resolve(Request $request, ArgumentMetadata $argument): iterable { @@ -70,48 +69,65 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable return []; } - $endpoint = $this->endpointRegistry->getEndpoint( - (new \ReflectionClass($argument->getType()))->newInstanceWithoutConstructor() - ); + $endpoint = $this->endpointRegistry + ->getEndpoint( + (new ReflectionClass($argument->getType())) + ->newInstanceWithoutConstructor() + ) + ; if ($endpoint->getRequestClass() !== $argument->getType()) { - throw new \LogicException('Incorrect resolving'); + throw new LogicException('Incorrect resolving'); } try { - $requestVars = array_merge( - !empty($request->getContent()) - ? $this->serializer->decode( - $request->getContent(), - $endpoint->getRequestFormat() - ) - : [] - , - $request->attributes->all(), - $request->query->all() - ); + $requestVars = $this->resolveExtractor($endpoint)->extract($request, $endpoint); $this->serviceResponseListener->addExpectedRequestEndpoint($request, $endpoint); - yield $this->serializer->denormalize( - $requestVars, - $endpoint->getRequestClass(), - $endpoint->getRequestFormat(), - [ - AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true, - ] - ); + yield $this->denormalizer + ->denormalize( + $requestVars, + $endpoint->getRequestClass(), + $endpoint->getRequestFormat(), + [ + AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true, + ] + ) + ; } catch (ExceptionInterface $exception) { - $this->getLogger()->warning( - 'Request deserialization exception', - [ - 'exception_message' => $exception->getMessage(), - ] - ); + $this->getLogger() + ->warning( + sprintf( + 'Request deserialization exception: %s', + $exception->getMessage() + ), + [ + 'exception' => $exception, + ] + ) + ; + throw new BadRequestHttpException('Request deserialization error'); } } + private function resolveExtractor(EndpointInterface $endpoint): RequestDataExtractorInterface + { + foreach ($this->requestDataExtractors as $extractor) { + if ($extractor->supports($endpoint)) { + return $extractor; + } + } + + throw new LogicException( + sprintf( + 'No request data extractor supports endpoint with format "%s".', + $endpoint->getRequestFormat() + ) + ); + } + private function supports(ArgumentMetadata $argument): bool { return is_subclass_of($argument->getType(), ServiceRequestInterface::class, true); diff --git a/Resources/config/services.yml b/Resources/config/services.yml index fc9165c..9d72ec0 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -18,6 +18,21 @@ services: tags: - {name: 'nelmio_api_doc.route_describer', priority: -400} + # Request data extractors + auto1.api_handler.request_data_extractor.multipart: + class: Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\MultipartRequestDataExtractor + arguments: + - '@?Psr\Http\Message\StreamFactoryInterface' + tags: + - { name: 'auto1.api_handler.request_data_extractor', priority: 100 } + + auto1.api_handler.request_data_extractor.default: + class: Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\DefaultRequestDataExtractor + arguments: + - '@auto1.api.request.serializer' + tags: + - { name: 'auto1.api_handler.request_data_extractor', priority: -100 } + # Argument resolver auto1.api_handler.argument_resolver.service_request: class: Auto1\ServiceAPIHandlerBundle\ArgumentResolver\ServiceRequestResolver @@ -25,6 +40,7 @@ services: - '@auto1.api.request.serializer' - '@auto1.api.endpoint.registry' - '@auto1.api_handler.response_listener.service_response' + - !tagged_iterator auto1.api_handler.request_data_extractor tags: - { name: 'controller.argument_value_resolver', priority: 150 } diff --git a/Tests/ArgumentResolver/DecodeDenormalizeAwareSerializerInterface.php b/Tests/ArgumentResolver/DecodeDenormalizeAwareSerializerInterface.php deleted file mode 100644 index c3ef7f0..0000000 --- a/Tests/ArgumentResolver/DecodeDenormalizeAwareSerializerInterface.php +++ /dev/null @@ -1,22 +0,0 @@ -decoder = $this->createMock(DecoderInterface::class); + $this->endpoint = $this->createMock(EndpointInterface::class); + } + + private function getCut(): DefaultRequestDataExtractor + { + return new DefaultRequestDataExtractor($this->decoder); + } + + public function testSupportsIsAlwaysTrue(): void + { + $extractor = $this->getCut(); + + $result = $extractor->supports($this->endpoint); + + self::assertTrue($result); + } + + public function testExtractMergesDecodedBodyAttributesAndQuery(): void + { + $targetBody = 'foobar'; + $targetQuery = ['targetQueryKey' => 'targetQueryValue']; + $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; + $targetDecoded = ['targetBodyKey' => 'targetBodyValue']; + + $request = new Request( + $targetQuery, + [], + $targetAttributes, + [], + [], + [], + $targetBody + ); + + $this->endpoint + ->method('getRequestFormat') + ->willReturn(self::TARGET_FORMAT) + ; + + $this->decoder + ->expects($this->once()) + ->method('decode') + ->with($targetBody, self::TARGET_FORMAT) + ->willReturn($targetDecoded) + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + self::assertSame( + array_merge($targetDecoded, $targetAttributes, $targetQuery), + $result + ); + } + + public function testExtractSkipsDecodeWhenBodyIsEmpty(): void + { + $targetQuery = ['targetQueryKey' => 'targetQueryValue']; + $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; + + $request = new Request($targetQuery, [], $targetAttributes); + + $this->decoder + ->expects(self::never()) + ->method('decode') + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + self::assertSame(array_merge($targetAttributes, $targetQuery), $result); + } +} diff --git a/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php new file mode 100644 index 0000000..7299a69 --- /dev/null +++ b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php @@ -0,0 +1,148 @@ +streamFactory = $this->createMock(StreamFactoryInterface::class); + $this->endpoint = $this->createMock(EndpointInterface::class); + } + + private function getCut(): MultipartRequestDataExtractor + { + return new MultipartRequestDataExtractor($this->streamFactory); + } + + public function testSupportsMultipartFormat(): void + { + $this->endpoint + ->method('getRequestFormat') + ->willReturn(self::TARGET_FORMAT) + ; + + $extractor = $this->getCut(); + + $result = $extractor->supports($this->endpoint); + + self::assertTrue($result); + } + + public function testDoesNotSupportOtherFormats(): void + { + $targetOtherFormat = 'json'; + + $this->endpoint + ->method('getRequestFormat') + ->willReturn($targetOtherFormat) + ; + + $extractor = $this->getCut(); + + $result = $extractor->supports($this->endpoint); + + self::assertFalse($result); + } + + public function testExtractMergesTextFieldsFilesAttributesAndQuery(): void + { + $targetFileName = 'avatar.png'; + $targetMimeType = 'image/png'; + $targetFileFieldKey = 'avatar'; + $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; + $targetQuery = ['targetQueryKey' => 'targetQueryValue']; + $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; + $targetFileContent = 'hello'; + + $targetTmp = tempnam(sys_get_temp_dir(), 'multipart-test-'); + file_put_contents($targetTmp, $targetFileContent); + $targetUploadedFile = new UploadedFile($targetTmp, $targetFileName, $targetMimeType, null, true); + + $request = new Request( + $targetQuery, + $targetTextFields, + $targetAttributes, + [], + [$targetFileFieldKey => $targetUploadedFile], + ['CONTENT_TYPE' => 'multipart/form-data; boundary=test'] + ); + + $targetStream = $this->createMock(StreamInterface::class); + $this->streamFactory + ->expects(self::once()) + ->method('createStreamFromFile') + ->with($targetTmp, 'r') + ->willReturn($targetStream) + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + $this->assertSame($targetTextFields['targetTextFieldKey'], $result['targetTextFieldKey']); + $this->assertSame($targetQuery['targetQueryKey'], $result['targetQueryKey']); + $this->assertSame($targetAttributes['targetAttributeKey'], $result['targetAttributeKey']); + $this->assertInstanceOf(UploadedFileStream::class, $result[$targetFileFieldKey]); + + unlink($targetTmp); + } + + public function testExtractThrowsWhenStreamFactoryMissing(): void + { + $this->streamFactory = null; + $request = new Request(); + + $extractor = $this->getCut(); + + $this->expectException(LogicException::class); + $extractor->extract($request, $this->endpoint); + } + + public function testExtractRequiresFactoryEvenWhenNoFilesPresent(): void + { + $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; + + $this->streamFactory = null; + $request = new Request([], $targetTextFields); + + $extractor = $this->getCut(); + + $this->expectException(LogicException::class); + $extractor->extract($request, $this->endpoint); + } +} diff --git a/Tests/ArgumentResolver/ServiceRequestResolverTest.php b/Tests/ArgumentResolver/ServiceRequestResolverTest.php index 365ce81..0e024fd 100644 --- a/Tests/ArgumentResolver/ServiceRequestResolverTest.php +++ b/Tests/ArgumentResolver/ServiceRequestResolverTest.php @@ -15,135 +15,113 @@ use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointRegistryInterface; +use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\RequestDataExtractorInterface; use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\ServiceRequestResolver; use Auto1\ServiceAPIHandlerBundle\EventListener\ServiceResponseListener; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\Prophecy\ObjectProphecy; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -use Symfony\Component\Serializer\Encoder\DecoderInterface; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; -use Symfony\Component\Serializer\Exception\UnexpectedValueException; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; -use Symfony\Component\Serializer\SerializerInterface; class ServiceRequestResolverTest extends TestCase { - /** - * @var SerializerInterface|DecoderInterface|DenormalizerInterface|ObjectProphecy - */ - private $serializerProphecy; - - /** - * @var EndpointRegistryInterface|ObjectProphecy - */ - private $endpointRegistryProphecy; - - /** - * @var ServiceResponseListener|ObjectProphecy - */ - private $serviceResponseListenerProphecy; - - /** - * @var ServiceRequestResolver - */ - private $serviceRequestResolver; - - /** - * {@inheritDoc} - */ + private const TARGET_FORMAT = 'json'; + private const TARGET_MULTIPART_FORMAT = 'multipart'; + + /** @var DenormalizerInterface&MockObject */ + private DenormalizerInterface $denormalizer; + + /** @var EndpointRegistryInterface&MockObject */ + private EndpointRegistryInterface $endpointRegistry; + + /** @var ServiceResponseListener&MockObject */ + private ServiceResponseListener $serviceResponseListener; + + /** @var RequestDataExtractorInterface&MockObject */ + private RequestDataExtractorInterface $extractor; + + /** @var iterable */ + private iterable $extractors; + protected function setUp(): void { - $this->serializerProphecy = $this->prophesize(DecodeDenormalizeAwareSerializerInterface::class); - $this->endpointRegistryProphecy = $this->prophesize(EndpointRegistryInterface::class); - $this->serviceResponseListenerProphecy = $this->prophesize(ServiceResponseListener::class); - $this->serviceRequestResolver = new ServiceRequestResolver( - $this->serializerProphecy->reveal(), - $this->endpointRegistryProphecy->reveal(), - $this->serviceResponseListenerProphecy->reveal() + $this->denormalizer = $this->createMock(DenormalizerInterface::class); + $this->endpointRegistry = $this->createMock(EndpointRegistryInterface::class); + $this->serviceResponseListener = $this->createMock(ServiceResponseListener::class); + $this->extractor = $this->createMock(RequestDataExtractorInterface::class); + $this->extractors = [$this->extractor]; + } + + private function getCut(): ServiceRequestResolver + { + return new ServiceRequestResolver( + $this->denormalizer, + $this->endpointRegistry, + $this->serviceResponseListener, + $this->extractors ); } - /** - * @return void - */ + private function createMetadata(): ArgumentMetadata + { + return new ArgumentMetadata('foo', RequestStub::class, false, false, null); + } + public function testResolveWrongRequestClass(): void { - $generator = $this->serviceRequestResolver->resolve( - new Request(), - $this->createMetadata() - ); - $endpointProphecy = $this->prophesize(EndpointInterface::class); - $this->endpointRegistryProphecy->getEndpoint(Argument::type(RequestStub::class)) - ->willReturn($endpointProphecy->reveal()) - ->shouldBeCalled(); - $endpointProphecy->getRequestClass() - ->willReturn(\stdClass::class) - ->shouldBeCalled(); + $endpoint = $this->createMock(EndpointInterface::class); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $endpoint->method('getRequestClass')->willReturn(\stdClass::class); + + $cut = $this->getCut(); + + $generator = $cut->resolve(new Request(), $this->createMetadata()); $this->expectException(\LogicException::class); $generator->current(); } - /** - * @return void - */ - public function testResolveDecodeException(): void + public function testResolveNoSupportingExtractor(): void { - $generator = $this->serviceRequestResolver->resolve( - new Request([], [], ['baz' => 'qux'], [], [], [], 'foobar'), - $this->createMetadata() - ); - $endpointProphecy = $this->prophesize(EndpointInterface::class); - $this->endpointRegistryProphecy->getEndpoint(Argument::type(RequestStub::class)) - ->willReturn($endpointProphecy->reveal()) - ->shouldBeCalled(); - $endpointProphecy->getRequestClass() - ->willReturn(RequestStub::class) - ->shouldBeCalled(); - $endpointProphecy->getRequestFormat() - ->willReturn('json') - ->shouldBeCalled(); - $this->serializerProphecy->decode('foobar', 'json', Argument::cetera()) - ->willThrow(UnexpectedValueException::class) - ->shouldBeCalled(); + $endpoint = $this->createMock(EndpointInterface::class); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $endpoint->method('getRequestClass')->willReturn(RequestStub::class); + $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); + $this->extractor->method('supports')->with($endpoint)->willReturn(false); - $this->expectException(BadRequestHttpException::class); + $cut = $this->getCut(); + + $generator = $cut->resolve(new Request(), $this->createMetadata()); + + $this->expectException(\LogicException::class); $generator->current(); } - /** - * @return void - */ - public function testResolveDeserializationException() + public function testResolveDeserializationException(): void { - $generator = $this->serviceRequestResolver->resolve( - new Request([], [], ['baz' => 'qux'], [], [], [], 'foobar'), - $this->createMetadata() - ); - $endpointProphecy = $this->prophesize(EndpointInterface::class); - $this->endpointRegistryProphecy->getEndpoint(Argument::type(RequestStub::class)) - ->willReturn($endpointProphecy->reveal()) - ->shouldBeCalled(); - $endpointProphecy->getRequestClass() - ->willReturn(RequestStub::class) - ->shouldBeCalled(); - $endpointProphecy->getRequestFormat() - ->willReturn('json') - ->shouldBeCalled(); - $this->serializerProphecy->decode('foobar', 'json', Argument::cetera()) - ->willReturn(['foo' => 'bar']) - ->shouldBeCalled(); - $this->serializerProphecy->denormalize( - ['foo' => 'bar', 'baz' => 'qux'], - RequestStub::class, - 'json', - Argument::cetera() - ) - ->willThrow(NotNormalizableValueException::class) - ->shouldBeCalled(); + $targetBody = 'foobar'; + $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; + $targetExtracted = ['targetBodyKey' => 'targetBodyValue', 'targetAttributeKey' => 'targetAttributeValue']; + + $request = new Request([], [], $targetAttributes, [], [], [], $targetBody); + + $endpoint = $this->createMock(EndpointInterface::class); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $endpoint->method('getRequestClass')->willReturn(RequestStub::class); + $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); + $this->extractor->method('supports')->with($endpoint)->willReturn(true); + $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); + $this->denormalizer + ->method('denormalize') + ->with($targetExtracted, RequestStub::class, self::TARGET_FORMAT) + ->willThrowException(new NotNormalizableValueException()); + + $cut = $this->getCut(); + + $generator = $cut->resolve($request, $this->createMetadata()); $this->expectException(BadRequestHttpException::class); $generator->current(); @@ -151,43 +129,70 @@ public function testResolveDeserializationException() public function testResolve(): void { - $generator = $this->serviceRequestResolver->resolve( - new Request(['asd' => 'dsa'], [], ['baz' => 'qux'], [], [], [], 'foobar'), - $this->createMetadata() - ); - $endpointProphecy = $this->prophesize(EndpointInterface::class); - $this->endpointRegistryProphecy->getEndpoint(Argument::type(RequestStub::class)) - ->willReturn($endpointProphecy->reveal()) - ->shouldBeCalled(); - $endpointProphecy->getRequestClass() - ->willReturn(RequestStub::class) - ->shouldBeCalled(); - $endpointProphecy->getRequestFormat() - ->willReturn('json') - ->shouldBeCalled(); - $this->serializerProphecy->decode('foobar', 'json', Argument::cetera()) - ->willReturn(['foo' => 'bar']) - ->shouldBeCalled(); - $this->serializerProphecy->denormalize( - ['foo' => 'bar', 'baz' => 'qux', 'asd' => 'dsa'], - RequestStub::class, - 'json', - Argument::cetera() - ) - ->willReturn(new RequestStub()) - ->shouldBeCalled(); - - $this->assertInstanceOf(RequestStub::class, $generator->current()); + $targetBody = 'foobar'; + $targetQuery = ['targetQueryKey' => 'targetQueryValue']; + $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; + $targetExtracted = [ + 'targetBodyKey' => 'targetBodyValue', + 'targetAttributeKey' => 'targetAttributeValue', + 'targetQueryKey' => 'targetQueryValue', + ]; + $targetDenormalized = new RequestStub(); + + $request = new Request($targetQuery, [], $targetAttributes, [], [], [], $targetBody); + + $endpoint = $this->createMock(EndpointInterface::class); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $endpoint->method('getRequestClass')->willReturn(RequestStub::class); + $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); + $this->extractor->method('supports')->with($endpoint)->willReturn(true); + $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); + $this->denormalizer + ->method('denormalize') + ->with($targetExtracted, RequestStub::class, self::TARGET_FORMAT) + ->willReturn($targetDenormalized); + + $cut = $this->getCut(); + + $generator = $cut->resolve($request, $this->createMetadata()); + + $this->assertSame($targetDenormalized, $generator->current()); } - public static function getDataForTestSupports(): \Generator + public function testResolvePicksFirstSupportingExtractor(): void { - yield 'supported' => [self::createMetadata(), true]; - yield 'not supported' => [new ArgumentMetadata('bar', \stdClass::class, false, false, null), false]; - } + $targetExtracted = ['targetKey' => 'targetValue']; + $targetDenormalized = new RequestStub(); - private static function createMetadata(): ArgumentMetadata - { - return new ArgumentMetadata('foo', RequestStub::class, false, false, null); + $skippedExtractor = $this->createMock(RequestDataExtractorInterface::class); + $matchingExtractor = $this->createMock(RequestDataExtractorInterface::class); + $this->extractors = [$skippedExtractor, $matchingExtractor]; + + $request = new Request(); + $endpoint = $this->createMock(EndpointInterface::class); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $endpoint->method('getRequestClass')->willReturn(RequestStub::class); + $endpoint->method('getRequestFormat')->willReturn(self::TARGET_MULTIPART_FORMAT); + + $skippedExtractor->expects($this->once())->method('supports')->with($endpoint)->willReturn(false); + $skippedExtractor->expects($this->never())->method('extract'); + + $matchingExtractor->expects($this->once())->method('supports')->with($endpoint)->willReturn(true); + $matchingExtractor + ->expects($this->once()) + ->method('extract') + ->with($request, $endpoint) + ->willReturn($targetExtracted); + + $this->denormalizer + ->method('denormalize') + ->with($targetExtracted, RequestStub::class, self::TARGET_MULTIPART_FORMAT) + ->willReturn($targetDenormalized); + + $cut = $this->getCut(); + + $generator = $cut->resolve($request, $this->createMetadata()); + + $this->assertSame($targetDenormalized, $generator->current()); } -} +} \ No newline at end of file diff --git a/composer.json b/composer.json index f26b930..1af1194 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,9 @@ "require": { "php": "^8.1", "auto1-oss/service-api-request": "^1.0", - "auto1-oss/service-api-components-bundle": "^1.0", + "auto1-oss/service-api-components-bundle": "dev-support-multiform-data", + "psr/http-message": "^1.1|^2.0", + "psr/http-factory": "^1.0", "symfony/serializer" : "~6.4|~7.0", "symfony/monolog-bridge": "~6.4|~7.0", "symfony/dependency-injection": "~6.4|~7.0", @@ -37,5 +39,11 @@ }, "autoload-dev": { "psr-4": { "Tests\\Auto1\\ServiceAPIHandlerBundle\\": "Tests"} - } + }, + "repositories": [ + { + "type": "vcs", + "url": "git@github.com:aviator-ua/service-api-components-bundle.git" + } + ] } From 04c77fcc01dc36ee2611bc800f396a11796f8168 Mon Sep 17 00:00:00 2001 From: Pavlo Pavliukovych Date: Wed, 29 Jul 2026 18:58:07 +0200 Subject: [PATCH 2/4] Address review feedback for multipart request data extractors - Reject invalid uploads with 400 instead of TypeError; use getPathname() instead of getRealPath() (also fixes macOS test failures) - Reject non-POST methods and non-multipart content types with 400 - Check the stream factory lazily when a file is actually wrapped - Fail container compilation when a controller-served multipart endpoint is registered but no PSR-17 stream factory is available - Autoconfigure RequestDataExtractorInterface implementations with the request data extractor tag - Decode "0" request bodies in DefaultRequestDataExtractor - Move psr/http-message to require-dev, suggest nyholm/psr7 - Document multipart endpoints, custom extractors and the constructor BC break in README - Extend test coverage: nested file arrays, invalid uploads, unfilled optional inputs, method/content-type guards, decode failures, compiler pass; clean up temp files in tearDown() --- .../DefaultRequestDataExtractor.php | 8 +- .../MultipartRequestDataExtractor.php | 58 ++++- .../RequestDataExtractorInterface.php | 2 +- Auto1ServiceAPIHandlerBundle.php | 2 + .../Auto1ServiceAPIHandlerExtension.php | 6 + .../MultipartStreamFactoryCompilerPass.php | 101 ++++++++ README.md | 52 +++++ .../DefaultRequestDataExtractorTest.php | 61 ++++- .../MultipartRequestDataExtractorTest.php | 220 ++++++++++++++++-- .../ServiceRequestResolverTest.php | 96 +++++--- .../CompilerPass/EndpointProviderStub.php | 38 +++ ...MultipartStreamFactoryCompilerPassTest.php | 114 +++++++++ composer.json | 7 +- 13 files changed, 682 insertions(+), 83 deletions(-) create mode 100644 DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php create mode 100644 Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php create mode 100644 Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php diff --git a/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php index c5ee756..aec3a1d 100644 --- a/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php +++ b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php @@ -35,10 +35,12 @@ public function supports(EndpointInterface $endpoint): bool public function extract(Request $request, EndpointInterface $endpoint): array { + $decoded = []; + $body = $request->getContent(); - $decoded = !empty($body) - ? $this->decoder->decode($body, $endpoint->getRequestFormat()) - : []; + if ('' !== $body) { + $decoded = $this->decoder->decode($body, $endpoint->getRequestFormat()); + } return array_merge( $decoded, diff --git a/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php index 162d483..1519c85 100644 --- a/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php +++ b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php @@ -19,14 +19,16 @@ use Psr\Http\Message\StreamFactoryInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use function array_merge; use function is_array; use function sprintf; +use function str_starts_with; class MultipartRequestDataExtractor implements RequestDataExtractorInterface { - public const FORMAT = 'multipart'; + private const CONTENT_TYPE = 'multipart/form-data'; private ?StreamFactoryInterface $streamFactory; @@ -37,18 +39,29 @@ public function __construct(?StreamFactoryInterface $streamFactory = null) public function supports(EndpointInterface $endpoint): bool { - return self::FORMAT === $endpoint->getRequestFormat(); + return EndpointInterface::FORMAT_MULTIPART === $endpoint->getRequestFormat(); } public function extract(Request $request, EndpointInterface $endpoint): array { - if (null === $this->streamFactory) { - throw new LogicException( + // PHP populates $_POST / $_FILES for POST requests only; for any other method the + // multipart body would be silently ignored and the payload would come out empty. + if (!$request->isMethod(Request::METHOD_POST)) { + throw new BadRequestHttpException( sprintf( - 'A PSR-17 "%s" must be wired to handle multipart/form-data endpoints. ' - . 'Install a PSR-7 implementation (e.g. guzzlehttp/psr7, nyholm/psr7) ' - . 'and register its stream factory.', - StreamFactoryInterface::class + 'multipart/form-data endpoints only support POST, got "%s".', + $request->getMethod() + ) + ); + } + + $contentType = (string) $request->headers->get('CONTENT_TYPE'); + if (!str_starts_with($contentType, self::CONTENT_TYPE)) { + throw new BadRequestHttpException( + sprintf( + 'Expected "%s" content type, got "%s".', + self::CONTENT_TYPE, + $contentType ) ); } @@ -70,12 +83,33 @@ private function wrapFiles(array $files): array continue; } - if ($value instanceof UploadedFile) { - $stream = $this->streamFactory->createStreamFromFile($value->getRealPath(), 'r'); - $wrapped[$key] = new UploadedFileStream($stream, $value); + // An unfilled optional file input arrives as null and is intentionally + // omitted, so the request DTO keeps its default value. + if (!$value instanceof UploadedFile) { + continue; } + + if (!$value->isValid()) { + throw new BadRequestHttpException( + sprintf('Upload failed for field "%s": %s', $key, $value->getErrorMessage()) + ); + } + + if (null === $this->streamFactory) { + throw new LogicException( + sprintf( + 'A PSR-17 "%s" must be wired to handle multipart/form-data file uploads. ' + . 'Install a PSR-7 implementation (e.g. guzzlehttp/psr7, nyholm/psr7) ' + . 'and register its stream factory.', + StreamFactoryInterface::class + ) + ); + } + + $stream = $this->streamFactory->createStreamFromFile($value->getPathname(), 'r'); + $wrapped[$key] = new UploadedFileStream($stream, $value); } return $wrapped; } -} +} \ No newline at end of file diff --git a/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php b/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php index 7da6676..5a08fbd 100644 --- a/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php +++ b/ArgumentResolver/RequestDataExtractor/RequestDataExtractorInterface.php @@ -25,4 +25,4 @@ public function supports(EndpointInterface $endpoint): bool; * for the endpoint's request class. */ public function extract(Request $request, EndpointInterface $endpoint): array; -} \ No newline at end of file +} diff --git a/Auto1ServiceAPIHandlerBundle.php b/Auto1ServiceAPIHandlerBundle.php index 1da4ea9..74eb33b 100644 --- a/Auto1ServiceAPIHandlerBundle.php +++ b/Auto1ServiceAPIHandlerBundle.php @@ -12,6 +12,7 @@ namespace Auto1\ServiceAPIHandlerBundle; use Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass\EndpointRouterCompilerPass; +use Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass\MultipartStreamFactoryCompilerPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -22,5 +23,6 @@ public function build(ContainerBuilder $container): void parent::build($container); $container->addCompilerPass(new EndpointRouterCompilerPass()); + $container->addCompilerPass(new MultipartStreamFactoryCompilerPass()); } } diff --git a/DependencyInjection/Auto1ServiceAPIHandlerExtension.php b/DependencyInjection/Auto1ServiceAPIHandlerExtension.php index a513e21..c734de7 100644 --- a/DependencyInjection/Auto1ServiceAPIHandlerExtension.php +++ b/DependencyInjection/Auto1ServiceAPIHandlerExtension.php @@ -11,6 +11,7 @@ namespace Auto1\ServiceAPIHandlerBundle\DependencyInjection; +use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\RequestDataExtractorInterface; use Nelmio\ApiDocBundle\NelmioApiDocBundle; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -35,6 +36,11 @@ public function load(array $configs, ContainerBuilder $container) //Load config files $loader->load('services.yml'); + $container + ->registerForAutoconfiguration(RequestDataExtractorInterface::class) + ->addTag('auto1.api_handler.request_data_extractor') + ; + if (!class_exists('EXSyst\Component\Swagger\Swagger') || class_exists('OpenApi\Annotations\OpenApi')) { $container->removeDefinition('auto1.route_describers.route_metadata'); } diff --git a/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php b/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php new file mode 100644 index 0000000..b518346 --- /dev/null +++ b/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php @@ -0,0 +1,101 @@ +has(StreamFactoryInterface::class)) { + return; + } + + if (!$container->hasParameter(self::CONTROLLER_MAPPING_PARAMETER)) { + return; + } + + $handledRequestClasses = $container->getParameter(self::CONTROLLER_MAPPING_PARAMETER); + + $multipartRequestClasses = []; + foreach ($this->getEndpoints($container) as $endpoint) { + if (EndpointInterface::FORMAT_MULTIPART !== $endpoint->getRequestFormat()) { + continue; + } + + $requestClass = $endpoint->getRequestClass(); + if (!in_array($requestClass, $handledRequestClasses, true)) { + continue; + } + + $multipartRequestClasses[$requestClass] = $requestClass; + } + + if ([] === $multipartRequestClasses) { + return; + } + + throw new ConfigurationException( + sprintf( + 'A PSR-17 "%s" must be wired to handle multipart/form-data endpoints: %s. ' + . 'Install a PSR-7 implementation (e.g. guzzlehttp/psr7, nyholm/psr7) ' + . 'and register its stream factory.', + StreamFactoryInterface::class, + implode(', ', $multipartRequestClasses) + ) + ); + } + + /** + * @return EndpointInterface[] + */ + private function getEndpoints(ContainerBuilder $container): array + { + $endpoints = []; + foreach ($container->findTaggedServiceIds(self::ENDPOINT_PROVIDER_TAG) as $id => $tags) { + $provider = $container->resolveServices($container->getDefinition($id)); + + // Misconfigured providers are reported by the components-bundle compiler pass. + if (!$provider instanceof EndpointProviderInterface) { + continue; + } + + foreach ($provider->getEndpoints() as $endpoint) { + if ($endpoint instanceof EndpointInterface) { + $endpoints[] = $endpoint; + } + } + } + + return $endpoints; + } +} diff --git a/README.md b/README.md index c4214be..8263661 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,58 @@ class MyController { } ``` +## multipart/form-data endpoints +Endpoints declared with `requestFormat: 'multipart'` accept `multipart/form-data` requests. +Text fields, attributes and query parameters are merged into the request DTO as usual; +uploaded files are exposed as `Auto1\ServiceAPIComponentsBundle\Multipart\UploadedFileStream` properties. + +Constraints: +* Only `POST` is supported — PHP does not parse multipart bodies for other HTTP methods; any other method is rejected with `400 Bad Request`. +* A PSR-17 `Psr\Http\Message\StreamFactoryInterface` service must be registered in the container. + Install a PSR-7 implementation (e.g. `nyholm/psr7` or `guzzlehttp/psr7`) and register its stream factory. + This is enforced at container compile time: if the application serves a multipart endpoint + and no stream factory is available, the container fails to build with a `ConfigurationException`. + +```yaml +uploadDocument: + method: 'POST' + baseUrl: '%auto1.api.url%' + path: '/v1/document' + requestFormat: 'multipart' + requestClass: 'App\Request\UploadDocumentRequest' + responseClass: 'App\Response\Document' +``` + +## Custom request data extractors +The request payload is built by the first `RequestDataExtractorInterface` implementation +whose `supports()` returns `true` for the endpoint, evaluated in descending priority. + +Implementations are autoconfigured: any service implementing +`Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\RequestDataExtractorInterface` +is tagged with `auto1.api_handler.request_data_extractor` automatically at the default priority `0`. + +The bundle registers two extractors of its own: +* `multipart` (priority `100`) — handles `requestFormat: 'multipart'` endpoints. +* `default` (priority `-100`) — always-true fallback that decodes the raw body. + +To control the order explicitly, tag the service manually: +```yaml +App\Request\CsvRequestDataExtractor: + tags: + - { name: 'auto1.api_handler.request_data_extractor', priority: 50 } +``` +A priority at or below `-100` is unreachable — the fallback wins first. Verify the effective +order with: +```bash +bin/console debug:container --tag=auto1.api_handler.request_data_extractor +``` + +## Upgrade note +`ServiceRequestResolver::__construct()` signature changed: the first argument is now a +`DenormalizerInterface` (was `SerializerInterface`) and a required 4th argument (the tagged +iterator of request data extractors) was added. Applications decorating or overriding the +`auto1.api_handler.argument_resolver.service_request` service definition must be updated. + ## Swagger generation For `symfony:>=6.0` and `nelmio/api-doc-bundle:>=4.0` swagger json file is generated in OpenApi v3 format `"openapi": "3.0.0"`. For previous versions of `symfony` and `nelmio/api-doc-bundle` swagger json file is generated in Swagger V2 format `"swagger": "2.0"`. diff --git a/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php b/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php index 1bdb439..54a6411 100644 --- a/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php +++ b/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php @@ -13,12 +13,13 @@ namespace Tests\Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor; -use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; +use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\Endpoint; use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\DefaultRequestDataExtractor; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Serializer\Encoder\DecoderInterface; +use Symfony\Component\Serializer\Exception\NotEncodableValueException; class DefaultRequestDataExtractorTest extends TestCase { @@ -29,15 +30,12 @@ class DefaultRequestDataExtractorTest extends TestCase */ private DecoderInterface $decoder; - /** - * @var EndpointInterface&MockObject - */ - private EndpointInterface $endpoint; + private Endpoint $endpoint; protected function setUp(): void { $this->decoder = $this->createMock(DecoderInterface::class); - $this->endpoint = $this->createMock(EndpointInterface::class); + $this->endpoint = new Endpoint(); } private function getCut(): DefaultRequestDataExtractor @@ -71,13 +69,10 @@ public function testExtractMergesDecodedBodyAttributesAndQuery(): void $targetBody ); - $this->endpoint - ->method('getRequestFormat') - ->willReturn(self::TARGET_FORMAT) - ; + $this->endpoint->setRequestFormat(self::TARGET_FORMAT); $this->decoder - ->expects($this->once()) + ->expects(self::once()) ->method('decode') ->with($targetBody, self::TARGET_FORMAT) ->willReturn($targetDecoded) @@ -111,4 +106,48 @@ public function testExtractSkipsDecodeWhenBodyIsEmpty(): void self::assertSame(array_merge($targetAttributes, $targetQuery), $result); } + + public function testExtractDecodesZeroStringBody(): void + { + $targetBody = '0'; + $targetDecoded = ['targetBodyKey' => 'targetBodyValue']; + + $request = new Request([], [], [], [], [], [], $targetBody); + + $this->endpoint->setRequestFormat(self::TARGET_FORMAT); + + $this->decoder + ->expects(self::once()) + ->method('decode') + ->with($targetBody, self::TARGET_FORMAT) + ->willReturn($targetDecoded) + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + self::assertSame($targetDecoded, $result); + } + + public function testExtractPropagatesDecodeException(): void + { + $targetBody = 'not-a-valid-payload'; + $targetException = new NotEncodableValueException(); + + $request = new Request([], [], [], [], [], [], $targetBody); + + $this->endpoint->setRequestFormat(self::TARGET_FORMAT); + + $this->decoder + ->method('decode') + ->with($targetBody, self::TARGET_FORMAT) + ->willThrowException($targetException) + ; + + $extractor = $this->getCut(); + + $this->expectException(NotEncodableValueException::class); + $extractor->extract($request, $this->endpoint); + } } diff --git a/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php index 7299a69..d935c2c 100644 --- a/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php +++ b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php @@ -13,7 +13,7 @@ namespace Tests\Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor; -use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; +use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\Endpoint; use Auto1\ServiceAPIComponentsBundle\Multipart\UploadedFileStream; use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\MultipartRequestDataExtractor; use LogicException; @@ -23,25 +23,43 @@ use Psr\Http\Message\StreamInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; class MultipartRequestDataExtractorTest extends TestCase { private const TARGET_FORMAT = 'multipart'; + private const TARGET_TMP_PREFIX = 'multipart-test-'; + private const TARGET_POST_SERVER = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'multipart/form-data; boundary=test', + ]; /** * @var (StreamFactoryInterface&MockObject)|null */ private ?StreamFactoryInterface $streamFactory; + private Endpoint $endpoint; + /** - * @var EndpointInterface&MockObject + * @var string[] */ - private EndpointInterface $endpoint; + private array $tmpFiles = []; protected function setUp(): void { $this->streamFactory = $this->createMock(StreamFactoryInterface::class); - $this->endpoint = $this->createMock(EndpointInterface::class); + $this->endpoint = new Endpoint(); + } + + protected function tearDown(): void + { + foreach ($this->tmpFiles as $tmpFile) { + if (file_exists($tmpFile)) { + unlink($tmpFile); + } + } + $this->tmpFiles = []; } private function getCut(): MultipartRequestDataExtractor @@ -49,12 +67,19 @@ private function getCut(): MultipartRequestDataExtractor return new MultipartRequestDataExtractor($this->streamFactory); } + private function createTmpFile(string $content): string + { + $tmpDir = sys_get_temp_dir(); + $tmpFile = tempnam($tmpDir, self::TARGET_TMP_PREFIX); + file_put_contents($tmpFile, $content); + $this->tmpFiles[] = $tmpFile; + + return $tmpFile; + } + public function testSupportsMultipartFormat(): void { - $this->endpoint - ->method('getRequestFormat') - ->willReturn(self::TARGET_FORMAT) - ; + $this->endpoint->setRequestFormat(self::TARGET_FORMAT); $extractor = $this->getCut(); @@ -67,10 +92,7 @@ public function testDoesNotSupportOtherFormats(): void { $targetOtherFormat = 'json'; - $this->endpoint - ->method('getRequestFormat') - ->willReturn($targetOtherFormat) - ; + $this->endpoint->setRequestFormat($targetOtherFormat); $extractor = $this->getCut(); @@ -89,8 +111,7 @@ public function testExtractMergesTextFieldsFilesAttributesAndQuery(): void $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; $targetFileContent = 'hello'; - $targetTmp = tempnam(sys_get_temp_dir(), 'multipart-test-'); - file_put_contents($targetTmp, $targetFileContent); + $targetTmp = $this->createTmpFile($targetFileContent); $targetUploadedFile = new UploadedFile($targetTmp, $targetFileName, $targetMimeType, null, true); $request = new Request( @@ -99,7 +120,7 @@ public function testExtractMergesTextFieldsFilesAttributesAndQuery(): void $targetAttributes, [], [$targetFileFieldKey => $targetUploadedFile], - ['CONTENT_TYPE' => 'multipart/form-data; boundary=test'] + self::TARGET_POST_SERVER ); $targetStream = $this->createMock(StreamInterface::class); @@ -114,18 +135,129 @@ public function testExtractMergesTextFieldsFilesAttributesAndQuery(): void $result = $extractor->extract($request, $this->endpoint); - $this->assertSame($targetTextFields['targetTextFieldKey'], $result['targetTextFieldKey']); - $this->assertSame($targetQuery['targetQueryKey'], $result['targetQueryKey']); - $this->assertSame($targetAttributes['targetAttributeKey'], $result['targetAttributeKey']); - $this->assertInstanceOf(UploadedFileStream::class, $result[$targetFileFieldKey]); + self::assertSame($targetTextFields['targetTextFieldKey'], $result['targetTextFieldKey']); + self::assertSame($targetQuery['targetQueryKey'], $result['targetQueryKey']); + self::assertSame($targetAttributes['targetAttributeKey'], $result['targetAttributeKey']); + self::assertInstanceOf(UploadedFileStream::class, $result[$targetFileFieldKey]); + } + + public function testExtractWrapsNestedFileArrays(): void + { + $targetFileName = 'doc.pdf'; + $targetMimeType = 'application/pdf'; + $targetFilesFieldKey = 'docs'; + $targetFileContent = 'content'; + + $targetFirstTmp = $this->createTmpFile($targetFileContent); + $targetSecondTmp = $this->createTmpFile($targetFileContent); + $targetFirstFile = new UploadedFile($targetFirstTmp, $targetFileName, $targetMimeType, null, true); + $targetSecondFile = new UploadedFile($targetSecondTmp, $targetFileName, $targetMimeType, null, true); - unlink($targetTmp); + $request = new Request( + [], + [], + [], + [], + [$targetFilesFieldKey => [$targetFirstFile, $targetSecondFile]], + self::TARGET_POST_SERVER + ); + + $targetStream = $this->createMock(StreamInterface::class); + $this->streamFactory + ->expects(self::exactly(2)) + ->method('createStreamFromFile') + ->willReturn($targetStream) + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + self::assertCount(2, $result[$targetFilesFieldKey]); + self::assertInstanceOf(UploadedFileStream::class, $result[$targetFilesFieldKey][0]); + self::assertInstanceOf(UploadedFileStream::class, $result[$targetFilesFieldKey][1]); } - public function testExtractThrowsWhenStreamFactoryMissing(): void + public function testExtractOmitsUnfilledOptionalFileInput(): void { + $targetFileFieldKey = 'optional'; + $targetNoFileUpload = [ + 'name' => '', + 'type' => '', + 'tmp_name' => '', + 'error' => UPLOAD_ERR_NO_FILE, + 'size' => 0, + ]; + + $request = new Request( + [], + [], + [], + [], + [$targetFileFieldKey => $targetNoFileUpload], + self::TARGET_POST_SERVER + ); + + $this->streamFactory + ->expects(self::never()) + ->method('createStreamFromFile') + ; + + $extractor = $this->getCut(); + + $result = $extractor->extract($request, $this->endpoint); + + self::assertArrayNotHasKey($targetFileFieldKey, $result); + } + + public function testExtractThrowsOnInvalidUpload(): void + { + $targetFileName = 'too-big.png'; + $targetFileFieldKey = 'attachment'; + $targetFileContent = 'partial'; + + $targetTmp = $this->createTmpFile($targetFileContent); + $targetUploadedFile = new UploadedFile($targetTmp, $targetFileName, null, UPLOAD_ERR_PARTIAL, true); + + $request = new Request( + [], + [], + [], + [], + [$targetFileFieldKey => $targetUploadedFile], + self::TARGET_POST_SERVER + ); + + $this->streamFactory + ->expects(self::never()) + ->method('createStreamFromFile') + ; + + $extractor = $this->getCut(); + + $this->expectException(BadRequestHttpException::class); + $extractor->extract($request, $this->endpoint); + } + + public function testExtractThrowsWhenStreamFactoryMissingForFileUpload(): void + { + $targetFileName = 'avatar.png'; + $targetMimeType = 'image/png'; + $targetFileFieldKey = 'avatar'; + $targetFileContent = 'hello'; + + $targetTmp = $this->createTmpFile($targetFileContent); + $targetUploadedFile = new UploadedFile($targetTmp, $targetFileName, $targetMimeType, null, true); + $this->streamFactory = null; - $request = new Request(); + $request = new Request( + [], + [], + [], + [], + [$targetFileFieldKey => $targetUploadedFile], + self::TARGET_POST_SERVER + ); $extractor = $this->getCut(); @@ -133,16 +265,54 @@ public function testExtractThrowsWhenStreamFactoryMissing(): void $extractor->extract($request, $this->endpoint); } - public function testExtractRequiresFactoryEvenWhenNoFilesPresent(): void + public function testExtractDoesNotRequireFactoryWhenNoFilesPresent(): void { $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; $this->streamFactory = null; - $request = new Request([], $targetTextFields); + $request = new Request( + [], + $targetTextFields, + [], + [], + [], + self::TARGET_POST_SERVER + ); $extractor = $this->getCut(); - $this->expectException(LogicException::class); + $result = $extractor->extract($request, $this->endpoint); + + self::assertSame($targetTextFields, $result); + } + + public function testExtractRejectsNonPostRequest(): void + { + $targetPutServer = [ + 'REQUEST_METHOD' => 'PUT', + 'CONTENT_TYPE' => 'multipart/form-data; boundary=test', + ]; + + $request = new Request([], [], [], [], [], $targetPutServer); + + $extractor = $this->getCut(); + + $this->expectException(BadRequestHttpException::class); + $extractor->extract($request, $this->endpoint); + } + + public function testExtractRejectsNonMultipartContentType(): void + { + $targetJsonServer = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/json', + ]; + + $request = new Request([], [], [], [], [], $targetJsonServer); + + $extractor = $this->getCut(); + + $this->expectException(BadRequestHttpException::class); $extractor->extract($request, $this->endpoint); } } diff --git a/Tests/ArgumentResolver/ServiceRequestResolverTest.php b/Tests/ArgumentResolver/ServiceRequestResolverTest.php index 0e024fd..eb4e1b1 100644 --- a/Tests/ArgumentResolver/ServiceRequestResolverTest.php +++ b/Tests/ArgumentResolver/ServiceRequestResolverTest.php @@ -13,7 +13,7 @@ namespace Tests\Auto1\ServiceAPIHandlerBundle\ArgumentResolver; -use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; +use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\Endpoint; use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointRegistryInterface; use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\RequestDataExtractor\RequestDataExtractorInterface; use Auto1\ServiceAPIHandlerBundle\ArgumentResolver\ServiceRequestResolver; @@ -23,6 +23,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\Serializer\Exception\NotEncodableValueException; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; @@ -70,15 +71,26 @@ private function createMetadata(): ArgumentMetadata return new ArgumentMetadata('foo', RequestStub::class, false, false, null); } + private function createEndpoint(string $requestClass, string $requestFormat): Endpoint + { + $endpoint = new Endpoint(); + $endpoint->setRequestClass($requestClass); + $endpoint->setRequestFormat($requestFormat); + + return $endpoint; + } + public function testResolveWrongRequestClass(): void { - $endpoint = $this->createMock(EndpointInterface::class); + $endpoint = $this->createEndpoint(\stdClass::class, self::TARGET_FORMAT); $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $endpoint->method('getRequestClass')->willReturn(\stdClass::class); + + $request = new Request(); + $metadata = $this->createMetadata(); $cut = $this->getCut(); - $generator = $cut->resolve(new Request(), $this->createMetadata()); + $generator = $cut->resolve($request, $metadata); $this->expectException(\LogicException::class); $generator->current(); @@ -86,15 +98,16 @@ public function testResolveWrongRequestClass(): void public function testResolveNoSupportingExtractor(): void { - $endpoint = $this->createMock(EndpointInterface::class); + $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $endpoint->method('getRequestClass')->willReturn(RequestStub::class); - $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); $this->extractor->method('supports')->with($endpoint)->willReturn(false); + $request = new Request(); + $metadata = $this->createMetadata(); + $cut = $this->getCut(); - $generator = $cut->resolve(new Request(), $this->createMetadata()); + $generator = $cut->resolve($request, $metadata); $this->expectException(\LogicException::class); $generator->current(); @@ -105,23 +118,47 @@ public function testResolveDeserializationException(): void $targetBody = 'foobar'; $targetAttributes = ['targetAttributeKey' => 'targetAttributeValue']; $targetExtracted = ['targetBodyKey' => 'targetBodyValue', 'targetAttributeKey' => 'targetAttributeValue']; + $targetException = new NotNormalizableValueException(); $request = new Request([], [], $targetAttributes, [], [], [], $targetBody); + $metadata = $this->createMetadata(); - $endpoint = $this->createMock(EndpointInterface::class); + $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $endpoint->method('getRequestClass')->willReturn(RequestStub::class); - $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); $this->extractor->method('supports')->with($endpoint)->willReturn(true); $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); $this->denormalizer ->method('denormalize') ->with($targetExtracted, RequestStub::class, self::TARGET_FORMAT) - ->willThrowException(new NotNormalizableValueException()); + ->willThrowException($targetException); + + $cut = $this->getCut(); + + $generator = $cut->resolve($request, $metadata); + + $this->expectException(BadRequestHttpException::class); + $generator->current(); + } + + public function testResolveDecodeException(): void + { + $targetBody = 'not-a-valid-payload'; + $targetException = new NotEncodableValueException(); + + $request = new Request([], [], [], [], [], [], $targetBody); + $metadata = $this->createMetadata(); + + $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); + $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $this->extractor->method('supports')->with($endpoint)->willReturn(true); + $this->extractor + ->method('extract') + ->with($request, $endpoint) + ->willThrowException($targetException); $cut = $this->getCut(); - $generator = $cut->resolve($request, $this->createMetadata()); + $generator = $cut->resolve($request, $metadata); $this->expectException(BadRequestHttpException::class); $generator->current(); @@ -140,11 +177,10 @@ public function testResolve(): void $targetDenormalized = new RequestStub(); $request = new Request($targetQuery, [], $targetAttributes, [], [], [], $targetBody); + $metadata = $this->createMetadata(); - $endpoint = $this->createMock(EndpointInterface::class); + $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $endpoint->method('getRequestClass')->willReturn(RequestStub::class); - $endpoint->method('getRequestFormat')->willReturn(self::TARGET_FORMAT); $this->extractor->method('supports')->with($endpoint)->willReturn(true); $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); $this->denormalizer @@ -154,9 +190,11 @@ public function testResolve(): void $cut = $this->getCut(); - $generator = $cut->resolve($request, $this->createMetadata()); + $generator = $cut->resolve($request, $metadata); - $this->assertSame($targetDenormalized, $generator->current()); + $result = $generator->current(); + + self::assertSame($targetDenormalized, $result); } public function testResolvePicksFirstSupportingExtractor(): void @@ -169,17 +207,17 @@ public function testResolvePicksFirstSupportingExtractor(): void $this->extractors = [$skippedExtractor, $matchingExtractor]; $request = new Request(); - $endpoint = $this->createMock(EndpointInterface::class); + $metadata = $this->createMetadata(); + + $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $endpoint->method('getRequestClass')->willReturn(RequestStub::class); - $endpoint->method('getRequestFormat')->willReturn(self::TARGET_MULTIPART_FORMAT); - $skippedExtractor->expects($this->once())->method('supports')->with($endpoint)->willReturn(false); - $skippedExtractor->expects($this->never())->method('extract'); + $skippedExtractor->expects(self::once())->method('supports')->with($endpoint)->willReturn(false); + $skippedExtractor->expects(self::never())->method('extract'); - $matchingExtractor->expects($this->once())->method('supports')->with($endpoint)->willReturn(true); + $matchingExtractor->expects(self::once())->method('supports')->with($endpoint)->willReturn(true); $matchingExtractor - ->expects($this->once()) + ->expects(self::once()) ->method('extract') ->with($request, $endpoint) ->willReturn($targetExtracted); @@ -191,8 +229,10 @@ public function testResolvePicksFirstSupportingExtractor(): void $cut = $this->getCut(); - $generator = $cut->resolve($request, $this->createMetadata()); + $generator = $cut->resolve($request, $metadata); + + $result = $generator->current(); - $this->assertSame($targetDenormalized, $generator->current()); + self::assertSame($targetDenormalized, $result); } -} \ No newline at end of file +} diff --git a/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php b/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php new file mode 100644 index 0000000..5fb352b --- /dev/null +++ b/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php @@ -0,0 +1,38 @@ +requestClass = $requestClass; + $this->requestFormat = $requestFormat; + } + + public function getEndpoints(): array + { + $endpoint = new Endpoint(); + $endpoint->setRequestClass($this->requestClass); + $endpoint->setRequestFormat($this->requestFormat); + + return [$endpoint]; + } +} diff --git a/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php b/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php new file mode 100644 index 0000000..b22ff0f --- /dev/null +++ b/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php @@ -0,0 +1,114 @@ +container = new ContainerBuilder(); + } + + private function getCut(): MultipartStreamFactoryCompilerPass + { + return new MultipartStreamFactoryCompilerPass(); + } + + private function registerEndpointProvider(string $requestClass, string $requestFormat): void + { + $definition = new Definition(EndpointProviderStub::class, [$requestClass, $requestFormat]); + $definition->addTag(self::TARGET_PROVIDER_TAG); + $this->container->setDefinition(self::TARGET_PROVIDER_SERVICE_ID, $definition); + } + + public function testProcessThrowsWhenHandledMultipartEndpointAndFactoryMissing(): void + { + $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; + + $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); + $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + + $cut = $this->getCut(); + + $this->expectException(ConfigurationException::class); + $cut->process($this->container); + } + + public function testProcessSucceedsWhenFactoryRegistered(): void + { + $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; + + $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); + $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + $this->container->register(StreamFactoryInterface::class); + + $cut = $this->getCut(); + + $this->expectNotToPerformAssertions(); + $cut->process($this->container); + } + + public function testProcessIgnoresClientOnlyMultipartEndpoints(): void + { + $targetMapping = []; + + $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); + $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + + $cut = $this->getCut(); + + $this->expectNotToPerformAssertions(); + $cut->process($this->container); + } + + public function testProcessIgnoresNonMultipartEndpoints(): void + { + $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; + + $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); + $this->registerEndpointProvider(RequestStub::class, self::TARGET_JSON_FORMAT); + + $cut = $this->getCut(); + + $this->expectNotToPerformAssertions(); + $cut->process($this->container); + } + + public function testProcessSucceedsWhenMappingParameterMissing(): void + { + $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + + $cut = $this->getCut(); + + $this->expectNotToPerformAssertions(); + $cut->process($this->container); + } +} diff --git a/composer.json b/composer.json index 1af1194..87f0f00 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,6 @@ "php": "^8.1", "auto1-oss/service-api-request": "^1.0", "auto1-oss/service-api-components-bundle": "dev-support-multiform-data", - "psr/http-message": "^1.1|^2.0", "psr/http-factory": "^1.0", "symfony/serializer" : "~6.4|~7.0", "symfony/monolog-bridge": "~6.4|~7.0", @@ -29,10 +28,12 @@ "require-dev": { "symfony/console": "~6.4|~7.0", "phpunit/phpunit": "^7.5|^8.0|^9.6", - "phpspec/prophecy": "^1.7.2" + "phpspec/prophecy": "^1.7.2", + "psr/http-message": "^1.1|^2.0" }, "suggest": { - "nelmio/api-doc-bundle": "For Generating API documentations" + "nelmio/api-doc-bundle": "For Generating API documentations", + "nyholm/psr7": "For multipart/form-data endpoints (provides a PSR-17 stream factory)" }, "autoload": { "psr-4": { "Auto1\\ServiceAPIHandlerBundle\\": "" } From b13427b71fa3fb674450cd98dc86eeb269ea12d4 Mon Sep 17 00:00:00 2001 From: Pavlo Pavliukovych Date: Thu, 30 Jul 2026 23:11:47 +0200 Subject: [PATCH 3/4] Harden request data extractors and rework multipart compiler pass - DefaultRequestDataExtractor: reject scalar/null decoded bodies with a Serializer UnexpectedValueException so they resolve to 400 instead of an uncaught TypeError (500) - MultipartRequestDataExtractor: check the real wire method so method overrides on a wire POST are accepted; match the Content-Type mime case-insensitively and also accept application/x-www-form-urlencoded; reject non-empty form bodies PHP failed to parse (post_max_size) - MultipartStreamFactoryCompilerPass: read endpoints already baked into the registry definition by EndpointProviderCompilerPass instead of instantiating every tagged provider a second time; run the pass at negative priority to guarantee ordering; document the compile-time coverage boundary in the class docblock and README - Tests: cover the new behaviors, rename CUT variables to $target, break multi-call chains one call per line, name the fopen mode literal --- .../DefaultRequestDataExtractor.php | 13 ++ .../MultipartRequestDataExtractor.php | 44 +++++-- Auto1ServiceAPIHandlerBundle.php | 10 +- .../MultipartStreamFactoryCompilerPass.php | 44 +++++-- README.md | 7 + .../DefaultRequestDataExtractorTest.php | 42 ++++-- .../MultipartRequestDataExtractorTest.php | 111 +++++++++++++--- .../ServiceRequestResolverTest.php | 124 +++++++++++++----- .../CompilerPass/EndpointProviderStub.php | 38 ------ ...MultipartStreamFactoryCompilerPassTest.php | 76 ++++++++--- 10 files changed, 366 insertions(+), 143 deletions(-) delete mode 100644 Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php diff --git a/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php index aec3a1d..ea36a95 100644 --- a/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php +++ b/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractor.php @@ -16,8 +16,12 @@ use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Serializer\Encoder\DecoderInterface; +use Symfony\Component\Serializer\Exception\UnexpectedValueException; use function array_merge; +use function get_debug_type; +use function is_array; +use function sprintf; class DefaultRequestDataExtractor implements RequestDataExtractorInterface { @@ -40,6 +44,15 @@ public function extract(Request $request, EndpointInterface $endpoint): array $body = $request->getContent(); if ('' !== $body) { $decoded = $this->decoder->decode($body, $endpoint->getRequestFormat()); + + // A scalar body (e.g. JSON `0` or `"foo"`) decodes without error but cannot feed + // array_merge(); UnexpectedValueException is a Serializer ExceptionInterface, so + // ServiceRequestResolver converts it to a 400 instead of a TypeError-driven 500. + if (!is_array($decoded)) { + throw new UnexpectedValueException( + sprintf('Request body must decode to an array, got "%s".', get_debug_type($decoded)) + ); + } } return array_merge( diff --git a/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php index 1519c85..ff4860c 100644 --- a/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php +++ b/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractor.php @@ -22,13 +22,23 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use function array_merge; +use function explode; +use function in_array; use function is_array; use function sprintf; -use function str_starts_with; +use function strtolower; +use function trim; class MultipartRequestDataExtractor implements RequestDataExtractorInterface { - private const CONTENT_TYPE = 'multipart/form-data'; + private const MIME_TYPE_MULTIPART = 'multipart/form-data'; + + // Request::create()/BrowserKit default POST bodies to urlencoded even when files are + // attached, and PHP parses both form mime types into the same superglobals — accept either. + private const FORM_MIME_TYPES = [ + self::MIME_TYPE_MULTIPART, + 'application/x-www-form-urlencoded', + ]; private ?StreamFactoryInterface $streamFactory; @@ -44,28 +54,44 @@ public function supports(EndpointInterface $endpoint): bool public function extract(Request $request, EndpointInterface $endpoint): array { - // PHP populates $_POST / $_FILES for POST requests only; for any other method the - // multipart body would be silently ignored and the payload would come out empty. - if (!$request->isMethod(Request::METHOD_POST)) { + // PHP populates $_POST / $_FILES only when the wire method is POST; method overrides + // (_method / X-HTTP-METHOD-OVERRIDE) are applied after parsing, so the real method + // decides whether the body was parsed — an overridden wire POST is fine. + if (Request::METHOD_POST !== $request->getRealMethod()) { throw new BadRequestHttpException( sprintf( - 'multipart/form-data endpoints only support POST, got "%s".', - $request->getMethod() + 'multipart/form-data endpoints must be sent as POST, got "%s".', + $request->getRealMethod() ) ); } + // Media types are case-insensitive (RFC 9110) and may carry parameters (boundary). $contentType = (string) $request->headers->get('CONTENT_TYPE'); - if (!str_starts_with($contentType, self::CONTENT_TYPE)) { + $mimeTypeParts = explode(';', $contentType, 2); + $mimeType = strtolower(trim($mimeTypeParts[0])); + if (!in_array($mimeType, self::FORM_MIME_TYPES, true)) { throw new BadRequestHttpException( sprintf( 'Expected "%s" content type, got "%s".', - self::CONTENT_TYPE, + self::MIME_TYPE_MULTIPART, $contentType ) ); } + // A body PHP failed to parse (e.g. post_max_size exceeded) leaves both bags empty + // while the body itself is non-empty — reject it instead of letting the request + // through with a silently empty payload. + if (0 === $request->request->count() + && 0 === $request->files->count() + && 0 < (int) $request->headers->get('CONTENT_LENGTH') + ) { + throw new BadRequestHttpException( + 'Form body could not be parsed — the "post_max_size" limit may be exceeded.' + ); + } + return array_merge( $request->request->all(), $this->wrapFiles($request->files->all()), diff --git a/Auto1ServiceAPIHandlerBundle.php b/Auto1ServiceAPIHandlerBundle.php index 74eb33b..eefae9e 100644 --- a/Auto1ServiceAPIHandlerBundle.php +++ b/Auto1ServiceAPIHandlerBundle.php @@ -13,6 +13,7 @@ use Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass\EndpointRouterCompilerPass; use Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass\MultipartStreamFactoryCompilerPass; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -23,6 +24,13 @@ public function build(ContainerBuilder $container): void parent::build($container); $container->addCompilerPass(new EndpointRouterCompilerPass()); - $container->addCompilerPass(new MultipartStreamFactoryCompilerPass()); + + // After the components-bundle EndpointProviderCompilerPass (default priority 0), so + // the endpoints are already baked into the registry definition when the guard runs. + $container->addCompilerPass( + new MultipartStreamFactoryCompilerPass(), + PassConfig::TYPE_BEFORE_OPTIMIZATION, + -10 + ); } } diff --git a/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php b/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php index b518346..966aa71 100644 --- a/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php +++ b/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPass.php @@ -13,12 +13,14 @@ namespace Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass; +use Auto1\ServiceAPIComponentsBundle\DependencyInjection\CompilerPass\EndpointProviderCompilerPass; use Auto1\ServiceAPIComponentsBundle\Exception\Core\ConfigurationException; +use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointImmutable; use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointInterface; -use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointProviderInterface; use Psr\Http\Message\StreamFactoryInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use function implode; use function in_array; @@ -28,10 +30,16 @@ * Fails container compilation when a controller-handled multipart endpoint is registered * but no PSR-17 stream factory is available — so the misconfiguration surfaces on deploy * instead of on the first upload request. + * + * Covers endpoints wired through the generated endpoints.yaml and the `*Controller::*Action` + * convention; manually-routed handlers are the integrator's responsibility and fail at + * runtime with the LogicException thrown by MultipartRequestDataExtractor instead. + * + * Must run after the components-bundle EndpointProviderCompilerPass has baked the endpoints + * into the registry definition — hence the negative pass priority in the bundle class. */ class MultipartStreamFactoryCompilerPass implements CompilerPassInterface { - private const ENDPOINT_PROVIDER_TAG = 'auto1.api.endpoint_provider'; private const CONTROLLER_MAPPING_PARAMETER = 'auto1.api_handler.controller_request_mapping'; public function process(ContainerBuilder $container): void @@ -76,24 +84,38 @@ public function process(ContainerBuilder $container): void } /** + * Reads the endpoints the components-bundle EndpointProviderCompilerPass has already baked + * into the registry definition — instead of instantiating every tagged provider (and its + * constructor dependency graph) a second time on each compile. + * * @return EndpointInterface[] */ private function getEndpoints(ContainerBuilder $container): array { - $endpoints = []; - foreach ($container->findTaggedServiceIds(self::ENDPOINT_PROVIDER_TAG) as $id => $tags) { - $provider = $container->resolveServices($container->getDefinition($id)); + if (!$container->hasDefinition(EndpointProviderCompilerPass::SERVICE_ENDPOINT_REGISTRY)) { + return []; + } - // Misconfigured providers are reported by the components-bundle compiler pass. - if (!$provider instanceof EndpointProviderInterface) { + $registryDefinition = $container->getDefinition( + EndpointProviderCompilerPass::SERVICE_ENDPOINT_REGISTRY + ); + + $endpoints = []; + foreach ($registryDefinition->getMethodCalls() as [$methodName, $arguments]) { + if (EndpointProviderCompilerPass::METHOD_REGISTER_ENDPOINT !== $methodName) { continue; } - foreach ($provider->getEndpoints() as $endpoint) { - if ($endpoint instanceof EndpointInterface) { - $endpoints[] = $endpoint; - } + $endpointDefinition = $arguments[0] ?? null; + if (!$endpointDefinition instanceof Definition + || EndpointImmutable::class !== $endpointDefinition->getClass() + ) { + continue; } + + // Scalar constructor args only — no services resolved, no provider constructors + // run; an arg-order change in the vendor pass would fail loudly here. + $endpoints[] = new EndpointImmutable(...$endpointDefinition->getArguments()); } return $endpoints; diff --git a/README.md b/README.md index 8263661..342bd71 100644 --- a/README.md +++ b/README.md @@ -84,10 +84,17 @@ uploaded files are exposed as `Auto1\ServiceAPIComponentsBundle\Multipart\Upload Constraints: * Only `POST` is supported — PHP does not parse multipart bodies for other HTTP methods; any other method is rejected with `400 Bad Request`. + The *wire* method is what counts: a wire `POST` carrying a `_method`/`X-HTTP-METHOD-OVERRIDE` override is accepted, since PHP has already parsed its body. +* The `Content-Type` mime type is matched case-insensitively; `application/x-www-form-urlencoded` is also accepted + (PHP parses both form mime types identically, and `Request::create()`/BrowserKit default POST bodies to urlencoded even when files are attached). +* A non-empty form body that PHP could not parse (e.g. `post_max_size` exceeded) is rejected with `400 Bad Request` instead of arriving as a silently empty payload. * A PSR-17 `Psr\Http\Message\StreamFactoryInterface` service must be registered in the container. Install a PSR-7 implementation (e.g. `nyholm/psr7` or `guzzlehttp/psr7`) and register its stream factory. This is enforced at container compile time: if the application serves a multipart endpoint and no stream factory is available, the container fails to build with a `ConfigurationException`. + Compile-time enforcement covers endpoints wired through the generated `endpoints.yaml` and the + `*Controller::*Action` convention; manually-routed handlers (e.g. `__invoke` controllers) are the + integrator's responsibility and fail at runtime with a descriptive `LogicException` instead. ```yaml uploadDocument: diff --git a/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php b/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php index 54a6411..2165263 100644 --- a/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php +++ b/Tests/ArgumentResolver/RequestDataExtractor/DefaultRequestDataExtractorTest.php @@ -20,6 +20,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Serializer\Encoder\DecoderInterface; use Symfony\Component\Serializer\Exception\NotEncodableValueException; +use Symfony\Component\Serializer\Exception\UnexpectedValueException; class DefaultRequestDataExtractorTest extends TestCase { @@ -45,9 +46,9 @@ private function getCut(): DefaultRequestDataExtractor public function testSupportsIsAlwaysTrue(): void { - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->supports($this->endpoint); + $result = $target->supports($this->endpoint); self::assertTrue($result); } @@ -78,9 +79,9 @@ public function testExtractMergesDecodedBodyAttributesAndQuery(): void ->willReturn($targetDecoded) ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertSame( array_merge($targetDecoded, $targetAttributes, $targetQuery), @@ -100,9 +101,9 @@ public function testExtractSkipsDecodeWhenBodyIsEmpty(): void ->method('decode') ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertSame(array_merge($targetAttributes, $targetQuery), $result); } @@ -123,13 +124,34 @@ public function testExtractDecodesZeroStringBody(): void ->willReturn($targetDecoded) ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertSame($targetDecoded, $result); } + public function testExtractRejectsNonArrayDecodedBody(): void + { + $targetBody = '0'; + $targetDecoded = 0; + + $request = new Request([], [], [], [], [], [], $targetBody); + + $this->endpoint->setRequestFormat(self::TARGET_FORMAT); + + $this->decoder + ->method('decode') + ->with($targetBody, self::TARGET_FORMAT) + ->willReturn($targetDecoded) + ; + + $target = $this->getCut(); + + $this->expectException(UnexpectedValueException::class); + $target->extract($request, $this->endpoint); + } + public function testExtractPropagatesDecodeException(): void { $targetBody = 'not-a-valid-payload'; @@ -145,9 +167,9 @@ public function testExtractPropagatesDecodeException(): void ->willThrowException($targetException) ; - $extractor = $this->getCut(); + $target = $this->getCut(); $this->expectException(NotEncodableValueException::class); - $extractor->extract($request, $this->endpoint); + $target->extract($request, $this->endpoint); } } diff --git a/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php index d935c2c..b1a2364 100644 --- a/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php +++ b/Tests/ArgumentResolver/RequestDataExtractor/MultipartRequestDataExtractorTest.php @@ -81,9 +81,9 @@ public function testSupportsMultipartFormat(): void { $this->endpoint->setRequestFormat(self::TARGET_FORMAT); - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->supports($this->endpoint); + $result = $target->supports($this->endpoint); self::assertTrue($result); } @@ -94,9 +94,9 @@ public function testDoesNotSupportOtherFormats(): void $this->endpoint->setRequestFormat($targetOtherFormat); - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->supports($this->endpoint); + $result = $target->supports($this->endpoint); self::assertFalse($result); } @@ -123,17 +123,18 @@ public function testExtractMergesTextFieldsFilesAttributesAndQuery(): void self::TARGET_POST_SERVER ); + $targetStreamReadMode = 'r'; $targetStream = $this->createMock(StreamInterface::class); $this->streamFactory ->expects(self::once()) ->method('createStreamFromFile') - ->with($targetTmp, 'r') + ->with($targetTmp, $targetStreamReadMode) ->willReturn($targetStream) ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertSame($targetTextFields['targetTextFieldKey'], $result['targetTextFieldKey']); self::assertSame($targetQuery['targetQueryKey'], $result['targetQueryKey']); @@ -169,9 +170,9 @@ public function testExtractWrapsNestedFileArrays(): void ->willReturn($targetStream) ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertCount(2, $result[$targetFilesFieldKey]); self::assertInstanceOf(UploadedFileStream::class, $result[$targetFilesFieldKey][0]); @@ -203,9 +204,9 @@ public function testExtractOmitsUnfilledOptionalFileInput(): void ->method('createStreamFromFile') ; - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertArrayNotHasKey($targetFileFieldKey, $result); } @@ -233,10 +234,10 @@ public function testExtractThrowsOnInvalidUpload(): void ->method('createStreamFromFile') ; - $extractor = $this->getCut(); + $target = $this->getCut(); $this->expectException(BadRequestHttpException::class); - $extractor->extract($request, $this->endpoint); + $target->extract($request, $this->endpoint); } public function testExtractThrowsWhenStreamFactoryMissingForFileUpload(): void @@ -259,10 +260,10 @@ public function testExtractThrowsWhenStreamFactoryMissingForFileUpload(): void self::TARGET_POST_SERVER ); - $extractor = $this->getCut(); + $target = $this->getCut(); $this->expectException(LogicException::class); - $extractor->extract($request, $this->endpoint); + $target->extract($request, $this->endpoint); } public function testExtractDoesNotRequireFactoryWhenNoFilesPresent(): void @@ -279,9 +280,9 @@ public function testExtractDoesNotRequireFactoryWhenNoFilesPresent(): void self::TARGET_POST_SERVER ); - $extractor = $this->getCut(); + $target = $this->getCut(); - $result = $extractor->extract($request, $this->endpoint); + $result = $target->extract($request, $this->endpoint); self::assertSame($targetTextFields, $result); } @@ -295,10 +296,10 @@ public function testExtractRejectsNonPostRequest(): void $request = new Request([], [], [], [], [], $targetPutServer); - $extractor = $this->getCut(); + $target = $this->getCut(); $this->expectException(BadRequestHttpException::class); - $extractor->extract($request, $this->endpoint); + $target->extract($request, $this->endpoint); } public function testExtractRejectsNonMultipartContentType(): void @@ -310,9 +311,77 @@ public function testExtractRejectsNonMultipartContentType(): void $request = new Request([], [], [], [], [], $targetJsonServer); - $extractor = $this->getCut(); + $target = $this->getCut(); $this->expectException(BadRequestHttpException::class); - $extractor->extract($request, $this->endpoint); + $target->extract($request, $this->endpoint); + } + + public function testExtractAllowsMethodOverrideOnWirePostRequest(): void + { + $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; + $targetOverriddenServer = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'multipart/form-data; boundary=test', + 'HTTP_X_HTTP_METHOD_OVERRIDE' => 'PUT', + ]; + + $request = new Request([], $targetTextFields, [], [], [], $targetOverriddenServer); + + $target = $this->getCut(); + + $result = $target->extract($request, $this->endpoint); + + self::assertSame($targetTextFields, $result); + } + + public function testExtractAcceptsCaseVariantContentType(): void + { + $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; + $targetCaseVariantServer = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'Multipart/Form-Data; boundary=test', + ]; + + $request = new Request([], $targetTextFields, [], [], [], $targetCaseVariantServer); + + $target = $this->getCut(); + + $result = $target->extract($request, $this->endpoint); + + self::assertSame($targetTextFields, $result); + } + + public function testExtractAcceptsUrlEncodedContentType(): void + { + $targetTextFields = ['targetTextFieldKey' => 'targetTextFieldValue']; + $targetUrlEncodedServer = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', + ]; + + $request = new Request([], $targetTextFields, [], [], [], $targetUrlEncodedServer); + + $target = $this->getCut(); + + $result = $target->extract($request, $this->endpoint); + + self::assertSame($targetTextFields, $result); + } + + public function testExtractRejectsUnparsedFormBody(): void + { + $targetUnparsedServer = [ + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'multipart/form-data; boundary=test', + 'CONTENT_LENGTH' => '1048576', + ]; + + $request = new Request([], [], [], [], [], $targetUnparsedServer); + + $target = $this->getCut(); + + $this->expectException(BadRequestHttpException::class); + $target->extract($request, $this->endpoint); } } diff --git a/Tests/ArgumentResolver/ServiceRequestResolverTest.php b/Tests/ArgumentResolver/ServiceRequestResolverTest.php index eb4e1b1..3c2416f 100644 --- a/Tests/ArgumentResolver/ServiceRequestResolverTest.php +++ b/Tests/ArgumentResolver/ServiceRequestResolverTest.php @@ -83,14 +83,17 @@ private function createEndpoint(string $requestClass, string $requestFormat): En public function testResolveWrongRequestClass(): void { $endpoint = $this->createEndpoint(\stdClass::class, self::TARGET_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; $request = new Request(); $metadata = $this->createMetadata(); - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $this->expectException(\LogicException::class); $generator->current(); @@ -99,15 +102,22 @@ public function testResolveWrongRequestClass(): void public function testResolveNoSupportingExtractor(): void { $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $this->extractor->method('supports')->with($endpoint)->willReturn(false); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; + $this->extractor + ->method('supports') + ->with($endpoint) + ->willReturn(false) + ; $request = new Request(); $metadata = $this->createMetadata(); - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $this->expectException(\LogicException::class); $generator->current(); @@ -124,17 +134,29 @@ public function testResolveDeserializationException(): void $metadata = $this->createMetadata(); $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $this->extractor->method('supports')->with($endpoint)->willReturn(true); - $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; + $this->extractor + ->method('supports') + ->with($endpoint) + ->willReturn(true) + ; + $this->extractor + ->method('extract') + ->with($request, $endpoint) + ->willReturn($targetExtracted) + ; $this->denormalizer ->method('denormalize') ->with($targetExtracted, RequestStub::class, self::TARGET_FORMAT) - ->willThrowException($targetException); + ->willThrowException($targetException) + ; - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $this->expectException(BadRequestHttpException::class); $generator->current(); @@ -149,16 +171,24 @@ public function testResolveDecodeException(): void $metadata = $this->createMetadata(); $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $this->extractor->method('supports')->with($endpoint)->willReturn(true); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; + $this->extractor + ->method('supports') + ->with($endpoint) + ->willReturn(true) + ; $this->extractor ->method('extract') ->with($request, $endpoint) - ->willThrowException($targetException); + ->willThrowException($targetException) + ; - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $this->expectException(BadRequestHttpException::class); $generator->current(); @@ -180,17 +210,29 @@ public function testResolve(): void $metadata = $this->createMetadata(); $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); - $this->extractor->method('supports')->with($endpoint)->willReturn(true); - $this->extractor->method('extract')->with($request, $endpoint)->willReturn($targetExtracted); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; + $this->extractor + ->method('supports') + ->with($endpoint) + ->willReturn(true) + ; + $this->extractor + ->method('extract') + ->with($request, $endpoint) + ->willReturn($targetExtracted) + ; $this->denormalizer ->method('denormalize') ->with($targetExtracted, RequestStub::class, self::TARGET_FORMAT) - ->willReturn($targetDenormalized); + ->willReturn($targetDenormalized) + ; - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $result = $generator->current(); @@ -210,26 +252,44 @@ public function testResolvePicksFirstSupportingExtractor(): void $metadata = $this->createMetadata(); $endpoint = $this->createEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); - $this->endpointRegistry->method('getEndpoint')->willReturn($endpoint); + $this->endpointRegistry + ->method('getEndpoint') + ->willReturn($endpoint) + ; - $skippedExtractor->expects(self::once())->method('supports')->with($endpoint)->willReturn(false); - $skippedExtractor->expects(self::never())->method('extract'); + $skippedExtractor + ->expects(self::once()) + ->method('supports') + ->with($endpoint) + ->willReturn(false) + ; + $skippedExtractor + ->expects(self::never()) + ->method('extract') + ; - $matchingExtractor->expects(self::once())->method('supports')->with($endpoint)->willReturn(true); + $matchingExtractor + ->expects(self::once()) + ->method('supports') + ->with($endpoint) + ->willReturn(true) + ; $matchingExtractor ->expects(self::once()) ->method('extract') ->with($request, $endpoint) - ->willReturn($targetExtracted); + ->willReturn($targetExtracted) + ; $this->denormalizer ->method('denormalize') ->with($targetExtracted, RequestStub::class, self::TARGET_MULTIPART_FORMAT) - ->willReturn($targetDenormalized); + ->willReturn($targetDenormalized) + ; - $cut = $this->getCut(); + $target = $this->getCut(); - $generator = $cut->resolve($request, $metadata); + $generator = $target->resolve($request, $metadata); $result = $generator->current(); diff --git a/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php b/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php deleted file mode 100644 index 5fb352b..0000000 --- a/Tests/DependencyInjection/CompilerPass/EndpointProviderStub.php +++ /dev/null @@ -1,38 +0,0 @@ -requestClass = $requestClass; - $this->requestFormat = $requestFormat; - } - - public function getEndpoints(): array - { - $endpoint = new Endpoint(); - $endpoint->setRequestClass($this->requestClass); - $endpoint->setRequestFormat($this->requestFormat); - - return [$endpoint]; - } -} diff --git a/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php b/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php index b22ff0f..2aa6a39 100644 --- a/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php +++ b/Tests/DependencyInjection/CompilerPass/MultipartStreamFactoryCompilerPassTest.php @@ -13,7 +13,9 @@ namespace Tests\Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass; +use Auto1\ServiceAPIComponentsBundle\DependencyInjection\CompilerPass\EndpointProviderCompilerPass; use Auto1\ServiceAPIComponentsBundle\Exception\Core\ConfigurationException; +use Auto1\ServiceAPIComponentsBundle\Service\Endpoint\EndpointImmutable; use Auto1\ServiceAPIHandlerBundle\DependencyInjection\CompilerPass\MultipartStreamFactoryCompilerPass; use PHPUnit\Framework\TestCase; use Psr\Http\Message\StreamFactoryInterface; @@ -26,9 +28,9 @@ class MultipartStreamFactoryCompilerPassTest extends TestCase private const TARGET_MULTIPART_FORMAT = 'multipart'; private const TARGET_JSON_FORMAT = 'json'; private const TARGET_MAPPING_PARAMETER = 'auto1.api_handler.controller_request_mapping'; - private const TARGET_PROVIDER_TAG = 'auto1.api.endpoint_provider'; - private const TARGET_PROVIDER_SERVICE_ID = 'target.endpoint_provider'; private const TARGET_CONTROLLER_ACTION = 'App\Controller\UploadController::uploadAction'; + private const TARGET_HTTP_METHOD = 'POST'; + private const TARGET_PATH = '/v1/upload'; private ContainerBuilder $container; @@ -42,11 +44,31 @@ private function getCut(): MultipartStreamFactoryCompilerPass return new MultipartStreamFactoryCompilerPass(); } - private function registerEndpointProvider(string $requestClass, string $requestFormat): void + private function registerEndpoint(string $requestClass, string $requestFormat): void { - $definition = new Definition(EndpointProviderStub::class, [$requestClass, $requestFormat]); - $definition->addTag(self::TARGET_PROVIDER_TAG); - $this->container->setDefinition(self::TARGET_PROVIDER_SERVICE_ID, $definition); + $registryId = EndpointProviderCompilerPass::SERVICE_ENDPOINT_REGISTRY; + if (!$this->container->hasDefinition($registryId)) { + $newRegistryDefinition = new Definition(); + $this->container->setDefinition($registryId, $newRegistryDefinition); + } + + $endpointDefinition = new Definition(EndpointImmutable::class); + $endpointDefinition->setArguments([ + self::TARGET_HTTP_METHOD, + null, + self::TARGET_PATH, + $requestFormat, + $requestClass, + self::TARGET_JSON_FORMAT, + null, + null, + ]); + + $registryDefinition = $this->container->getDefinition($registryId); + $registryDefinition->addMethodCall( + EndpointProviderCompilerPass::METHOD_REGISTER_ENDPOINT, + [$endpointDefinition] + ); } public function testProcessThrowsWhenHandledMultipartEndpointAndFactoryMissing(): void @@ -54,12 +76,12 @@ public function testProcessThrowsWhenHandledMultipartEndpointAndFactoryMissing() $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); - $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + $this->registerEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); - $cut = $this->getCut(); + $target = $this->getCut(); $this->expectException(ConfigurationException::class); - $cut->process($this->container); + $target->process($this->container); } public function testProcessSucceedsWhenFactoryRegistered(): void @@ -67,13 +89,13 @@ public function testProcessSucceedsWhenFactoryRegistered(): void $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); - $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + $this->registerEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); $this->container->register(StreamFactoryInterface::class); - $cut = $this->getCut(); + $target = $this->getCut(); $this->expectNotToPerformAssertions(); - $cut->process($this->container); + $target->process($this->container); } public function testProcessIgnoresClientOnlyMultipartEndpoints(): void @@ -81,12 +103,12 @@ public function testProcessIgnoresClientOnlyMultipartEndpoints(): void $targetMapping = []; $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); - $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + $this->registerEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); - $cut = $this->getCut(); + $target = $this->getCut(); $this->expectNotToPerformAssertions(); - $cut->process($this->container); + $target->process($this->container); } public function testProcessIgnoresNonMultipartEndpoints(): void @@ -94,21 +116,33 @@ public function testProcessIgnoresNonMultipartEndpoints(): void $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); - $this->registerEndpointProvider(RequestStub::class, self::TARGET_JSON_FORMAT); + $this->registerEndpoint(RequestStub::class, self::TARGET_JSON_FORMAT); - $cut = $this->getCut(); + $target = $this->getCut(); $this->expectNotToPerformAssertions(); - $cut->process($this->container); + $target->process($this->container); } public function testProcessSucceedsWhenMappingParameterMissing(): void { - $this->registerEndpointProvider(RequestStub::class, self::TARGET_MULTIPART_FORMAT); + $this->registerEndpoint(RequestStub::class, self::TARGET_MULTIPART_FORMAT); - $cut = $this->getCut(); + $target = $this->getCut(); $this->expectNotToPerformAssertions(); - $cut->process($this->container); + $target->process($this->container); + } + + public function testProcessSucceedsWhenRegistryDefinitionMissing(): void + { + $targetMapping = [self::TARGET_CONTROLLER_ACTION => RequestStub::class]; + + $this->container->setParameter(self::TARGET_MAPPING_PARAMETER, $targetMapping); + + $target = $this->getCut(); + + $this->expectNotToPerformAssertions(); + $target->process($this->container); } } From 0cfbe7a04c42b9021ca78eb0ff783e73a632f769 Mon Sep 17 00:00:00 2001 From: Pavlo Pavliukovych Date: Mon, 3 Aug 2026 17:23:40 +0200 Subject: [PATCH 4/4] Add container compilation integration tests Boot a real kernel (FrameworkBundle + components bundle + this bundle) so the actual services.yml files load and all compiler passes from both bundles run in their real order: - happy path: with a PSR-17 stream factory the container compiles, the controller_request_mapping parameter maps the fixture controller action to its request class, and ServiceRequestResolver is resolvable - guard path: without a stream factory a handled multipart endpoint fails the build with ConfigurationException The fixture controller is discovered via the same Finder/require_once scan of /src that consumer applications go through, and is excluded from the classmap on purpose. symfony/framework-bundle added to require-dev; the existing CI matrix (PHP 8.1 -> Symfony 6.4, PHP 8.2+ -> Symfony 7.x) now covers compilation on Symfony updates. --- .../Integration/ContainerCompilationTest.php | 88 +++++++++++++++++ .../Fixtures/App/DocumentEndpointProvider.php | 34 +++++++ .../Fixtures/App/StreamFactoryStub.php | 39 ++++++++ .../Fixtures/App/UploadDocumentRequest.php | 20 ++++ Tests/Integration/Fixtures/TestKernel.php | 98 +++++++++++++++++++ .../Fixtures/src/UploadController.php | 26 +++++ composer.json | 6 +- 7 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 Tests/Integration/ContainerCompilationTest.php create mode 100644 Tests/Integration/Fixtures/App/DocumentEndpointProvider.php create mode 100644 Tests/Integration/Fixtures/App/StreamFactoryStub.php create mode 100644 Tests/Integration/Fixtures/App/UploadDocumentRequest.php create mode 100644 Tests/Integration/Fixtures/TestKernel.php create mode 100644 Tests/Integration/Fixtures/src/UploadController.php diff --git a/Tests/Integration/ContainerCompilationTest.php b/Tests/Integration/ContainerCompilationTest.php new file mode 100644 index 0000000..971cc97 --- /dev/null +++ b/Tests/Integration/ContainerCompilationTest.php @@ -0,0 +1,88 @@ +varDir = $tmpDir . self::TARGET_VAR_DIR_PREFIX . $uniqueSuffix; + } + + protected function tearDown(): void + { + if (null !== $this->kernel) { + $this->kernel->shutdown(); + $this->kernel = null; + } + + $filesystem = new Filesystem(); + $filesystem->remove($this->varDir); + } + + private function getCut(bool $withStreamFactory): TestKernel + { + $this->kernel = new TestKernel($withStreamFactory, $this->varDir); + + return $this->kernel; + } + + public function testContainerCompilesAndMapsHandledEndpoint(): void + { + $targetWithStreamFactory = true; + $targetControllerAction = UploadController::class . '::uploadAction'; + + $target = $this->getCut($targetWithStreamFactory); + + $target->boot(); + + $container = $target->getContainer(); + + $mapping = $container->getParameter(self::TARGET_MAPPING_PARAMETER); + self::assertArrayHasKey($targetControllerAction, $mapping); + self::assertSame(UploadDocumentRequest::class, $mapping[$targetControllerAction]); + + $testContainer = $container->get('test.service_container'); + $resolver = $testContainer->get(self::TARGET_RESOLVER_SERVICE_ID); + self::assertInstanceOf(ServiceRequestResolver::class, $resolver); + } + + public function testContainerCompilationFailsWithoutStreamFactoryForMultipartEndpoint(): void + { + $targetWithStreamFactory = false; + + $target = $this->getCut($targetWithStreamFactory); + + $this->expectException(ConfigurationException::class); + $target->boot(); + } +} diff --git a/Tests/Integration/Fixtures/App/DocumentEndpointProvider.php b/Tests/Integration/Fixtures/App/DocumentEndpointProvider.php new file mode 100644 index 0000000..a957e64 --- /dev/null +++ b/Tests/Integration/Fixtures/App/DocumentEndpointProvider.php @@ -0,0 +1,34 @@ +setMethod(Request::METHOD_POST); + $endpoint->setPath('/v1/document'); + $endpoint->setRequestFormat(EndpointInterface::FORMAT_MULTIPART); + $endpoint->setRequestClass(UploadDocumentRequest::class); + $endpoint->setResponseFormat(EndpointInterface::FORMAT_JSON); + + return [$endpoint]; + } +} diff --git a/Tests/Integration/Fixtures/App/StreamFactoryStub.php b/Tests/Integration/Fixtures/App/StreamFactoryStub.php new file mode 100644 index 0000000..16a364b --- /dev/null +++ b/Tests/Integration/Fixtures/App/StreamFactoryStub.php @@ -0,0 +1,39 @@ +withStreamFactory = $withStreamFactory; + $this->varDir = $varDir; + + parent::__construct('test', true); + } + + public function registerBundles(): iterable + { + return [ + new FrameworkBundle(), + new Auto1ServiceAPIComponentsBundle(), + new Auto1ServiceAPIHandlerBundle(), + ]; + } + + /* + * EndpointRouterCompilerPass scans /src for controllers, so the fixture + * directory acts as the consumer application root. + */ + public function getProjectDir(): string + { + return __DIR__; + } + + public function getCacheDir(): string + { + return $this->varDir . '/cache'; + } + + public function getLogDir(): string + { + return $this->varDir . '/log'; + } + + private function configureContainer(ContainerConfigurator $container): void + { + $container->extension('framework', [ + 'secret' => 'test', + 'test' => true, + 'http_method_override' => false, + 'handle_all_throwables' => true, + 'php_errors' => ['log' => true], + 'serializer' => ['enabled' => true], + 'property_access' => ['enabled' => true], + 'property_info' => ['enabled' => true], + ]); + + $services = $container->services(); + + $services + ->set(DocumentEndpointProvider::class) + ->tag('auto1.api.endpoint_provider', ['priority' => 0]) + ; + + if ($this->withStreamFactory) { + $services->set(StreamFactoryInterface::class, StreamFactoryStub::class); + } + } + + private function configureRoutes(RoutingConfigurator $routes): void + { + } +} diff --git a/Tests/Integration/Fixtures/src/UploadController.php b/Tests/Integration/Fixtures/src/UploadController.php new file mode 100644 index 0000000..e17e928 --- /dev/null +++ b/Tests/Integration/Fixtures/src/UploadController.php @@ -0,0 +1,26 @@ +/src and loads it with require_once, mirroring how consumer + * applications' controllers are found. + */ +class UploadController +{ + public function uploadAction(UploadDocumentRequest $request): void + { + } +} diff --git a/composer.json b/composer.json index 87f0f00..e2e5ac7 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,8 @@ "symfony/console": "~6.4|~7.0", "phpunit/phpunit": "^7.5|^8.0|^9.6", "phpspec/prophecy": "^1.7.2", - "psr/http-message": "^1.1|^2.0" + "psr/http-message": "^1.1|^2.0", + "symfony/framework-bundle": "~6.4|~7.0" }, "suggest": { "nelmio/api-doc-bundle": "For Generating API documentations", @@ -39,7 +40,8 @@ "psr-4": { "Auto1\\ServiceAPIHandlerBundle\\": "" } }, "autoload-dev": { - "psr-4": { "Tests\\Auto1\\ServiceAPIHandlerBundle\\": "Tests"} + "psr-4": { "Tests\\Auto1\\ServiceAPIHandlerBundle\\": "Tests"}, + "exclude-from-classmap": ["Tests/Integration/Fixtures/src/"] }, "repositories": [ {