ImageStream implementation #147
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Laminas\Diactoros; | ||
|
|
||
| use GdImage; | ||
|
|
||
| use function sprintf; | ||
|
|
||
| /** | ||
| * This class purposely does not override the $resource property in order to | ||
| * allow it to extend Stream. If defined here with a type, PHP will raise a | ||
| * fatal error complaining that it must not define a type for the property. | ||
| */ | ||
| class ImageStream extends Stream | ||
| { | ||
| /** | ||
| * This purposely does not explicitly override the $resource property in | ||
| * order to allow it to extend Stream. If defined here with a type, PHP will | ||
| * raise a fatal error complaining that it must not define a type for the | ||
| * property. | ||
| * | ||
| * @var null|GdImage | ||
| */ | ||
| protected $resource; | ||
|
|
||
| public function __construct( | ||
| GdImage $resource, | ||
| ) { | ||
| $this->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; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| */ | ||
|
Comment on lines
+39
to
41
There was a problem hiding this comment. I am not sure if this is good. Expanding parameter types is considered type safe. The problem is anyone needing Wouldn't that go against interoperability goal between libraries since this will be creating dependency on diactoros and this implementation? There was a problem hiding this comment.
This suggests two things to me:
For now, I don't think this patch violates interoperability, as PSR-7 and PSR-17 do talk about resources, and this is a type of resource. There was a problem hiding this comment. I would not support PSR is clear here: If some1 wants to use |
||
| public function createStreamFromResource($resource): StreamInterface | ||
| { | ||
| if ($resource instanceof GdImage) { | ||
| return new ImageStream($resource); | ||
| } | ||
|
|
||
| return new Stream($resource); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not extend Stream at all. It is not compatible with any of its methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree here, extending
Streammakes no sense as we have to handle almost every method in a specific way.Especially due to the
GdImage/resourceproblematic, we could also just provideGdImageStreamas it is forGdImageonly.Not sure if some1 wants to use this for anything else, so renaming it might be helpful but I do not insist on this.