Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122150
MIME-Version: 1.0
References: <CAH5C8xXPNz1e2EARB5HpuiWpuRh=qRj8r1ZgpCH-=UFRJ_9O=Q@mail.gmail.com>
In-Reply-To: <CAH5C8xXPNz1e2EARB5HpuiWpuRh=qRj8r1ZgpCH-=UFRJ_9O=Q@mail.gmail.com>
Date: Sat, 13 Jan 2024 16:04:29 +0000
Message-ID: <CAEKnhAEHuA-r+fwNVK7Mv8P43N4F8z+4QtXz1DnQawrtQyCw+w@mail.gmail.com>
To: =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000336c17060ed5f0fa"
Subject: Re: [PHP-DEV] Dedicated StreamBucket class
From: bukka@php.net (Jakub Zelenka)

--000000000000336c17060ed5f0fa
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Sat, Jan 13, 2024 at 9:29=E2=80=AFAM M=C3=A1t=C3=A9 Kocsis <kocsismate90=
@gmail.com> wrote:

> Hi Everyone,
>
> Recently, I realized that the stream_bucket_new() and
> stream_bucket_make_writeable() functions
> create stdClass instances by dynamically adding a "bucket", a "data" and =
a
> "datalen" property to it.
>
> A few days ago, I submitted a PR which makes the above mentioned function=
s
> return a dedicated StreamBucket class which has these parameters properly
> declared (https://github.com/php/php-src/pull/13111/). Furthermore,
> stream_bucket_prepend() and stream_bucket_append() would accept objects o=
f
> this class as their second parameter from now on. Before, they accepted a=
ny
> kind of objects as long as they had a "bucket" property containing a vali=
d
> stream.
>
>
I just submitted feedback to the PR but will also mention it here as it's
probably more an API thing. The problem that I see is that it combines two
distinct things and create quite ugly self reference inside the proposed
StreamBucket object. I would prefer we split it and introduce
StreamBucketHandle opaque object that will completely replace the current
use of bucket resource. The actual StreamBucket could be introduced later
(ideally in PHP 9 as it's a sort of breaking change - change of class).


> As far as I see, my changes are backward compatible as long as people use
> the stream bucket API properly (i.e. create stream buckets via
> stream_bucket_new() and stream_bucket_make_writeable()). If they manually
> construct such objects (i.e. $bucket =3D new stdClass(); $bucket->bucket =
=3D
> ....) then obviously, they would start to face type errors.
>

I think it's more typing issue if someone passes this object for further
processing and type hint stdClass. Possibly the pattern above could be used
for copying but seems quite unlikely. Still I would see this as a BC break
and it is not really related to resource to object migration.

>
> So my question is whether anyone has any use-case/preexisting code which
> falls into the second case? If no one knows about the invalid usage
> pattern, my next question would be whether I have to create an RFC for th=
is
> change?
>
>
If you want to change the API and replace stdClass with StreamBucket, then
I think it should have a separate RFC. If you just introduce
StreamBucketHandle as suggested, then I think it's already covered by other
RFC as it's just a simple resource to object migration.

Regards

Jakub

--000000000000336c17060ed5f0fa--