From f96e7d84ebd7b49d176411c44d6c00e998248905 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 1 May 2023 16:28:14 -0500 Subject: [PATCH 1/3] feat: adds ImageStream implementation `ImageStream` is an extension of `Stream`, with the following key differences: - The `$resource` property is documented as being `null|GdIamge` - The constructor accepts a `GdImage` **only**, and assigns it to the `$resource` property - The `detach()` method no declares a return type of `null|GdImage` - The `attach()` method documents via annotation that the `$resource` it accepts can only be of type `GdImage`; the method raises an exception for any other argument. Signed-off-by: Matthew Weier O'Phinney --- psalm-baseline.xml | 112 ++++++++++++++++++++++----------------- src/ImageStream.php | 63 ++++++++++++++++++++++ test/ImageStreamTest.php | 84 +++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 48 deletions(-) create mode 100644 src/ImageStream.php create mode 100644 test/ImageStreamTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index cab33714..da27c326 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -28,6 +28,17 @@ __construct + + + null|GdImage + + + $resource + + + $resource + + ! is_string($name) @@ -101,7 +112,7 @@ $protocolVersion $requestTarget $uri - self::getValueFromKey($serializedRequest, 'body') + $headers @@ -154,7 +165,7 @@ $protocolVersion $reasonPhrase $statusCode - self::getValueFromKey($serializedResponse, 'body') + $headers @@ -230,7 +241,7 @@ - $requestFilter(new ServerRequest( + + ))]]> - $headers['cookie'] + $headers @@ -365,10 +376,10 @@ - $spec['error'] - $spec['name'] ?? null - $spec['tmp_name'] - $spec['type'] ?? null + + + + @@ -393,8 +404,8 @@ string - $server['REQUEST_METHOD'] ?? 'GET' - $server['REQUEST_METHOD'] ?? 'GET' + + @@ -404,7 +415,7 @@ - $server['SERVER_PROTOCOL'] + @@ -424,14 +435,14 @@ static function (string $name, array $headers, $default = null) { - $getHeaderFromArray('x-forwarded-proto', $headers, '') + $host $host $host $host $port $requestUri - $server['QUERY_STRING'] + $headers[$header] @@ -448,7 +459,7 @@ string - $server['SERVER_ADDR'] + $defaults @@ -456,7 +467,7 @@ $unencodedUrl - strrpos($host, ':') + @@ -467,13 +478,13 @@ - $apacheRequestHeaders['Authorization'] - $apacheRequestHeaders['authorization'] + + $apacheRequestHeaders - $server['HTTP_AUTHORIZATION'] - $server['HTTP_AUTHORIZATION'] + + @@ -501,25 +512,25 @@ $nameTree[$key] ?? null, $typeTree[$key] ?? null ) - $recursiveNormalize( + + )]]> array - $recursiveNormalize( + + )]]> @@ -541,6 +552,11 @@ $ret + + + $resourceToAttach + + getSize()]]> @@ -599,7 +615,7 @@ - $normalizedFiles['fooFiles'] + @@ -650,23 +666,23 @@ - $normalised['my-form']['details']['avatars'] - $normalised['slide-shows'][0]['slides'] + + - $normalised['my-form']['details']['avatar'] - $normalised['my-form']['details']['avatars'] - $normalised['my-form']['details']['avatars'] - $normalised['my-form']['details']['avatars'] - $normalised['my-form']['details']['avatars'] - $normalised['my-form']['details']['avatars'][0] - $normalised['my-form']['details']['avatars'][1] - $normalised['my-form']['details']['avatars'][2] - $normalised['slide-shows'][0]['slides'] - $normalised['slide-shows'][0]['slides'] - $normalised['slide-shows'][0]['slides'] - $normalised['slide-shows'][0]['slides'][0] - $normalised['slide-shows'][0]['slides'][1] + + + + + + + + + + + + + getClientFilename @@ -677,14 +693,14 @@ getClientFilename - $normalised['my-form'] - $normalised['my-form'] - $normalised['my-form'] - $normalised['my-form'] - $normalised['my-form'] - $normalised['slide-shows'] - $normalised['slide-shows'] - $normalised['slide-shows'] + + + + + + + + diff --git a/src/ImageStream.php b/src/ImageStream.php new file mode 100644 index 00000000..6a2037c3 --- /dev/null +++ b/src/ImageStream.php @@ -0,0 +1,63 @@ +resource = $resource; + } + + /** + * {@inheritdoc} + * @return null|GdImage + */ + public function detach(): ?GdImage + { + $resource = $this->resource; + $this->resource = null; + return $resource; + } + + /** + * Attach a new stream/resource to the instance. + * + * @param GdImage $resource + * @param string $mode Unused + * @throws Exception\InvalidArgumentException When provided a non-GdImage resource. + */ + public function attach($resource, string $mode = 'r'): void + { + if (! $resource instanceof GdImage) { + throw new Exception\InvalidArgumentException(sprintf( + 'When attaching a resource to %s, resource must be a GdImage', + $this::class, + )); + } + + $this->resource = $resource; + } +} diff --git a/test/ImageStreamTest.php b/test/ImageStreamTest.php new file mode 100644 index 00000000..a0f5b224 --- /dev/null +++ b/test/ImageStreamTest.php @@ -0,0 +1,84 @@ +assertInstanceOf(ImageStream::class, $stream); + + return $stream; + } + + /** @depends testCanInstantiateWithGDResource */ + public function testImageStreamExtendsStream(ImageStream $stream): void + { + $this->assertInstanceOf(Stream::class, $stream); + } + + public function testDetachReturnsGDResource(): void + { + $resource = imagecreate(1, 1); + assert($resource instanceof GdImage); + $stream = new ImageStream($resource); + $detached = $stream->detach(); + + $this->assertInstanceOf(GdImage::class, $detached); + $this->assertSame($resource, $detached); + } + + /** @psalm-return non-empty-array */ + public function invalidResourceProvider(): array + { + return [ + 'file' => [fopen(__FILE__, 'r')], + ]; + } + + /** + * @dataProvider invalidResourceProvider + * @param resource $resourceToAttach + */ + public function testAttachRaisesExceptionForNonGDResource($resourceToAttach): void + { + $resource = imagecreate(1, 1); + assert($resource instanceof GdImage); + + $stream = new ImageStream($resource); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('resource must be a GdImage'); + $stream->attach($resourceToAttach); + } + + public function testAttachSwitchesToNewResourceWhenSuccessful(): void + { + $resource1 = imagecreate(1, 1); + assert($resource1 instanceof GdImage); + $resource2 = imagecreate(1, 1); + assert($resource2 instanceof GdImage); + + $stream = new ImageStream($resource1); + $stream->attach($resource2); + + $detached = $stream->detach(); + + $this->assertInstanceOf(GdImage::class, $detached); + $this->assertSame($resource2, $detached); + } +} From 632265bc2dac722420d260943d3681f6b87e4815 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 2 May 2023 08:11:38 -0500 Subject: [PATCH 2/3] refactor: deprecate usage of GdImage in `Stream` class When a `GdImage` is used, the class will now issue an `E_USER_DEPRECATED` indicating the user should use `ImageStream` instead. Signed-off-by: Matthew Weier O'Phinney --- psalm-baseline.xml | 3 +++ src/ImageStream.php | 1 + src/Stream.php | 10 ++++++++++ test/StreamTest.php | 21 +++++++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index da27c326..6544e42b 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -632,6 +632,9 @@ setMethods + + $errno + diff --git a/src/ImageStream.php b/src/ImageStream.php index 6a2037c3..788e518f 100644 --- a/src/ImageStream.php +++ b/src/ImageStream.php @@ -33,6 +33,7 @@ public function __construct( /** * {@inheritdoc} + * * @return null|GdImage */ public function detach(): ?GdImage diff --git a/src/Stream.php b/src/Stream.php index 96c0982e..5700e027 100644 --- a/src/Stream.php +++ b/src/Stream.php @@ -28,7 +28,9 @@ use function stream_get_contents; use function stream_get_meta_data; use function strstr; +use function trigger_error; +use const E_USER_DEPRECATED; use const SEEK_SET; /** @@ -359,6 +361,14 @@ private function isValidStreamResourceType(mixed $resource): bool } if ($resource instanceof GdImage) { + trigger_error( + sprintf( + 'When using GdImage resources, use %s; %s will drop support for GdImage in 3.0.0', + ImageStream::class, + self::class + ), + E_USER_DEPRECATED, + ); return true; } diff --git a/test/StreamTest.php b/test/StreamTest.php index 245d7bde..4bd26b9d 100644 --- a/test/StreamTest.php +++ b/test/StreamTest.php @@ -26,14 +26,18 @@ use function fwrite; use function imagecreate; use function is_resource; +use function restore_error_handler; +use function set_error_handler; use function shmop_open; use function stream_get_meta_data; +use function strpos; use function sys_get_temp_dir; use function tempnam; use function uniqid; use function unlink; use const DIRECTORY_SEPARATOR; +use const E_USER_DEPRECATED; final class StreamTest extends TestCase { @@ -67,12 +71,29 @@ public function testCanInstantiateWithStreamResource(): void $this->assertInstanceOf(Stream::class, $stream); } + /** + * @todo For version 3, this test should verify that a GdImage raises an exception. + */ public function testCanInstantiateWithGDResource(): void { $resource = imagecreate(1, 1); self::assertInstanceOf(GdImage::class, $resource); + + $spy = (object) ['caught' => false]; + set_error_handler(function (int $errno, string $errstr) use ($spy): bool { + if (strpos($errstr, 'drop support for GdImage') !== false) { + $spy->caught = true; + return true; + } + return false; + }, E_USER_DEPRECATED); + $stream = new Stream($resource); + + restore_error_handler(); + $this->assertInstanceOf(Stream::class, $stream); + $this->assertTrue($spy->caught); } public function testIsReadableReturnsFalseIfStreamIsNotReadable(): void From 7c0003d240d41e4eae36358c4706f3741e2f611e Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 2 May 2023 08:20:29 -0500 Subject: [PATCH 3/3] refactor: update `StreamFactory::createStreamFromResource()` to work with GdImage Updates `createStreamFromResource()` to detect `GdImage` resources, and, when detected, create and return an `ImageStream` instead of a `Stream`. Signed-off-by: Matthew Weier O'Phinney --- src/StreamFactory.php | 7 +++++++ test/StreamFactoryTest.php | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 test/StreamFactoryTest.php diff --git a/src/StreamFactory.php b/src/StreamFactory.php index 54596dba..d548076c 100644 --- a/src/StreamFactory.php +++ b/src/StreamFactory.php @@ -4,6 +4,7 @@ namespace Laminas\Diactoros; +use GdImage; use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; @@ -35,9 +36,15 @@ public function createStreamFromFile(string $file, string $mode = 'r'): StreamIn /** * {@inheritDoc} + * + * @param resource|GdImage $resource */ public function createStreamFromResource($resource): StreamInterface { + if ($resource instanceof GdImage) { + return new ImageStream($resource); + } + return new Stream($resource); } } diff --git a/test/StreamFactoryTest.php b/test/StreamFactoryTest.php new file mode 100644 index 00000000..4d126428 --- /dev/null +++ b/test/StreamFactoryTest.php @@ -0,0 +1,36 @@ +createStreamFromResource($resource); + $this->assertInstanceOf(ImageStream::class, $stream); + } + + public function testPassingFileResourceToCreateStreamFromResourceReturnsStream(): void + { + $factory = new StreamFactory(); + $resource = fopen(__FILE__, 'r'); + $stream = $factory->createStreamFromResource($resource); + $this->assertInstanceOf(Stream::class, $stream); + } +}