Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion
Hey Larry, That's fine for the main vote, but the others are all either/or votes, not > yes/no votes, so 2/3 majority doesn't mean anything. Do you mean those are > 50/50 votes, or something else? > Thanks for your insight again, You are right, it was a silly bug in the RFC. I've just fixed it so that the vote for the implementation itself is now a primary vote requiring 2/3 majority, while the rest of the votes are now secondary ones requiring a simple majority. Thanks, Máté
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
On Wed, Jan 3, 2024, at 7:11 AM, Nicolas Grekas wrote:
> Hi Max,
>
> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
>> functionality is already available for internal classes - userspace should
>> benefit from it, too.
>>
>> The RFC: https://wiki.php.net/rfc/not_serializable
>> Proposed implementation: https://github.com/php/php-src/pull/12788
>>
>> Please let me know what you think.
>>
>
> Regarding the inheritance-related behavior ("The non-serializable flag is
> inherited by descendants"), this is very unlike any other attributes, and
> this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
>
> To me, if this is really the behavior we want, then the attribute should be
> replaced by a maker interface.
> Then, a simple "instanceof NotSerializable" would be enough instead of
> adding yet another method to ReflectionClass.
>
> Cheers,
> Nicolas
I think I'm inclined to agree with Nicolas. If the intent here is to impact
mainly serialize()/unserialize(), then a marker interface seems like it has the
desired logic "built in" more than an attribute does. It would only work at
the object level, but that seems correct in this case. For excluding
individual properties, __serialize()/__unserialize() already support that use
case. That's one of the main uses for them.
User-space serializers (Crell/Serde, JMS, Symfony, etc.) could easily be
modified to honor the interface as well for external serialization.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion
On Wed, Jan 3, 2024, at 7:36 AM, Máté Kocsis wrote: > Hi Everyone, > > If there are no further complaints, I intend to start the votes ( > https://wiki.php.net/rfc/resource_to_object_conversion) the day after > tomorrow. > > Regards, > Máté "Each vote requires 2/3 majority in order to be accepted. " That's fine for the main vote, but the others are all either/or votes, not yes/no votes, so 2/3 majority doesn't mean anything. Do you mean those are 50/50 votes, or something else? --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
śr., 3 sty 2024 o 15:57 Gina P. Banyard napisał(a):
> On Wednesday, 3 January 2024 at 14:38, Michał Marcin Brzuchalski <
> michal.brzuchal...@gmail.com> wrote:
>
> Hi Gina,
>
> śr., 3 sty 2024 o 14:41 Gina P. Banyard napisał(a):
>
> Hello internals,
>
> I would like to propose an RFC to add the functions
> http_get_last_request_headers() and http_clear_last_request_headers() to
> PHP to replace the magic variable $http_response_header.
>
> Link: https://wiki.php.net/rfc/http-last-response-headers
>
>
> I was on the specific documentation page describing this feature Today and
> was thinking that it is inappropriate as well.
> But my thinking was whether it shouldn't be a part of a stream context
> instead, something like this:
>
> $response_headers;
> $context = stream_context_create([
> 'http' => ['response_headers' => &$response_headers]
> ]);
> $result = file_get_contents('http://example.com/submit.php', false,
> $context);
>
> This way by ref you get a specific HTTP wrapper running under the hood
> response headers instead of just the last one.
> Any thoughts about that?
>
>
> I don't understand why the response should be part of the context of a
> stream.
> Especially as I'm not aware of anything within the context that changes
> values after a request?
>
My thinking was like passing as part of the context a placeholder by ref
where the headers of requests done in this context are to be populated.
This means instead of calling http_get_last_request_headers() as you
propose just read them from $response_headers array which got populated
during the HTTP call.
In general the same to what Aleksander asked.
Cheers
Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
On Wednesday, 3 January 2024 at 14:38, Michał Marcin Brzuchalski
wrote:
> Hi Gina,
> śr., 3 sty 2024 o 14:41 Gina P. Banyard napisał(a):
>
>> Hello internals,
>>
>> I would like to propose an RFC to add the functions
>> http_get_last_request_headers() and http_clear_last_request_headers() to PHP
>> to replace the magic variable $http_response_header.
>>
>> Link: https://wiki.php.net/rfc/http-last-response-headers
>
> I was on the specific documentation page describing this feature Today and
> was thinking that it is inappropriate as well.
> But my thinking was whether it shouldn't be a part of a stream context
> instead, something like this:
>
> $response_headers;
> $context = stream_context_create([
> 'http' => ['response_headers' => &$response_headers]
> ]);
> $result = file_get_contents('http://example.com/submit.php', false, $context);
>
> This way by ref you get a specific HTTP wrapper running under the hood
> response headers instead of just the last one.
> Any thoughts about that?
I don't understand why the response should be part of the context of a stream.
Especially as I'm not aware of anything within the context that changes values
after a request?
Best regards,
Gina P. Banyard
Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
On Wednesday, 3 January 2024 at 14:34, Aleksander Machniak wrote:
> On 3.01.2024 14:41, Gina P. Banyard wrote:
>
> > Link: https://wiki.php.net/rfc/http-last-response-headers
>
>
> Wrong function name in the subject (should be "response" not "request")
Ah yes indeed, the RFC title is also incorrect.
> I don't think we need the clearing function. Do we?
Yes we do, because this is effectively global state whereas the variable is
created in the *local* scope.
> I don't like that this is HTTP specific feature while we have other
> protocol wrappers. Here's a different approach. Use stream context with
> extended context parameters feature. Something like:
>
> $context = stream_context_create();
>
> $file = file_get_contents('http://www.example.com/', false, $context);
>
> $headers = stream_context_get_params($context)['response_headers'];
>
> Or something like that. I don't know.
>
> While on this I found out that we already have stream_get_meta_data()
> and `wrapper_data` there. So, maybe we should/could make it more unified.
>
> Maybe it should be mentioned in the RFC.
The whole point of this RFC is to be able to remove the local variable,
requiring to drop to the stream layer, is the reason this got removed from the
8.1 mass deprecation RFC.
That RFC also shows how this can be achieved currently, but it is way more
cumbersome.
You cannot use any of the stream related functions if the request fails unless
one ignores errors, which requires parsing every single response to know if it
is valid or not instead of just checking for false.
Moreover, this means one cannot use file_get_contents().
As such, I am not going to change anything in the RFC as my objective is to get
rid of the local variable creation in the long term.
Best regards,
Gina P. Banyard
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
Hi Gina,
śr., 3 sty 2024 o 14:41 Gina P. Banyard napisał(a):
> Hello internals,
>
> I would like to propose an RFC to add the functions
> http_get_last_request_headers() and http_clear_last_request_headers() to
> PHP to replace the magic variable $http_response_header.
>
> Link: https://wiki.php.net/rfc/http-last-response-headers
I was on the specific documentation page describing this feature Today and
was thinking that it is inappropriate as well.
But my thinking was whether it shouldn't be a part of a stream context
instead, something like this:
$response_headers;
$context = stream_context_create([
'http' => ['response_headers' => &$response_headers]
]);
$result = file_get_contents('http://example.com/submit.php', false,
$context);
This way by ref you get a specific HTTP wrapper running under the hood
response headers instead of just the last one.
Any thoughts about that?
Cheers,
Michał Marcin Brzuchalski
Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
On 3.01.2024 14:41, Gina P. Banyard wrote:
Link: https://wiki.php.net/rfc/http-last-response-headers
Wrong function name in the subject (should be "response" not "request")
I don't think we need the clearing function. Do we?
I don't like that this is HTTP specific feature while we have other
protocol wrappers. Here's a different approach. Use stream context with
extended context parameters feature. Something like:
$context = stream_context_create();
$file = file_get_contents('http://www.example.com/', false, $context);
$headers = stream_context_get_params($context)['response_headers'];
Or something like that. I don't know.
While on this I found out that we already have stream_get_meta_data()
and `wrapper_data` there. So, maybe we should/could make it more unified.
Maybe it should be mentioned in the RFC.
--
Aleksander Machniak
Kolab Groupware Developer[https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
śr., 3 sty 2024 o 11:10 Nicolas Grekas
napisał(a):
>
śr., 3 sty 2024 o 08:12 Nicolas Grekas
napisał(a):
> Hi Max,
>
> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
> > functionality is already available for internal classes - userspace
> should
> > benefit from it, too.
> >
> > The RFC: https://wiki.php.net/rfc/not_serializable
> > Proposed implementation: https://github.com/php/php-src/pull/12788
> >
> > Please let me know what you think.
> >
>
> Regarding the inheritance-related behavior ("The non-serializable flag
> is
> inherited by descendants"), this is very unlike any other attributes,
> and
> this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
>
> To me, if this is really the behavior we want, then the attribute
> should be
> replaced by a maker interface.
> Then, a simple "instanceof NotSerializable" would be enough instead of
> adding yet another method to ReflectionClass.
>
This should be possible without ReflectionClass, see
https://3v4l.org/N3fmO
>>>
>>> Sure.
>>>
>>> My main point is : why use an attribute? this should be an interface to
>>> me. All semantics match an interface.
>>>
>>>
From the serialization libraries you can find similar attributes
>>>
>>> Those a very different, because they tackle the problem from the
>>> *external* angle: the attributes there describe how a system *external* to
>>> the class itself should best serialize an object. There, attributes make
>>> sense, because they enrich the description of the class without forcibly
>>> telling what to do with the object
>>>
>>> But in the RFC, we're talking about the object deciding itself how it
>>> should be (not) serialized. This is enforced and thus belongs to the
>>> typesystem - not to an attribute.
>>>
>>
>> But then what should implement the NotSerializable interface?
>> If you want to ignore a string-typed property there would be no option to
>> mark it with a NotSerializable interface
>> Consider "baz" property in this example:
>>
>> class Foo {
>> protected int $bar = 1;
>> #[NotSerializable]
>> protected string $baz = 2;
>> }
>>
>
>
> The attribute is #[Attribute(Attribute::TARGET_CLASS] so I'm not sure why
> you consider this as it's not mentioned in the RFC (and I'm not sure it
> would make sense).
>
Apologies, apparently I didn't read.
Cheers
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
On Wed, 3 Jan 2024 at 13:17, Max Semenik wrote: > > this actively prevents writing a child class that'd make a parent > serializable if it wants to. > > Wouldn't this violate LSP? > No it doesn't. Making a child class unserializable if the parent is serializable however is violating LSP, and probably a test should be added for this. Best regards, Gina P. Banyard
[PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function
Hello internals, I would like to propose an RFC to add the functions http_get_last_request_headers() and http_clear_last_request_headers() to PHP to replace the magic variable $http_response_header. Link: https://wiki.php.net/rfc/http-last-response-headers Best regards, Gina P. Banyard
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
On Wed, Jan 3, 2024 at 10:11 AM Nicolas Grekas
wrote:
>
> Regarding the inheritance-related behavior ("The non-serializable flag is
> inherited by descendants"), this is very unlike any other attributes, and
> this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
>
> To me, if this is really the behavior we want, then the attribute should
> be replaced by a maker interface.
> Then, a simple "instanceof NotSerializable" would be enough instead of
> adding yet another method to ReflectionClass.
>
As explained in the RFC, this implementation hooks to the already existing
feature. Additionally, checking a bit in zend_class_entry::ce_flags is much
faster than checking inheritance.
> this actively prevents writing a child class that'd make a parent
serializable if it wants to.
Wouldn't this violate LSP?
--
Best regards,
Max Semenik
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
>
>
>>> śr., 3 sty 2024 o 08:12 Nicolas Grekas
>>> napisał(a):
>>>
Hi Max,
Hi, I'd like to propose a new attribute, #[NotSerializable]. This
> functionality is already available for internal classes - userspace
should
> benefit from it, too.
>
> The RFC: https://wiki.php.net/rfc/not_serializable
> Proposed implementation: https://github.com/php/php-src/pull/12788
>
> Please let me know what you think.
>
Regarding the inheritance-related behavior ("The non-serializable flag
is
inherited by descendants"), this is very unlike any other attributes,
and
this actively prevents writing a child class that'd make a parent
serializable if it wants to.
To me, if this is really the behavior we want, then the attribute
should be
replaced by a maker interface.
Then, a simple "instanceof NotSerializable" would be enough instead of
adding yet another method to ReflectionClass.
>>>
>>> This should be possible without ReflectionClass, see
>>> https://3v4l.org/N3fmO
>>>
>>
>> Sure.
>>
>> My main point is : why use an attribute? this should be an interface to
>> me. All semantics match an interface.
>>
>>
>>> From the serialization libraries you can find similar attributes
>>>
>>
>> Those a very different, because they tackle the problem from the
>> *external* angle: the attributes there describe how a system *external* to
>> the class itself should best serialize an object. There, attributes make
>> sense, because they enrich the description of the class without forcibly
>> telling what to do with the object
>>
>> But in the RFC, we're talking about the object deciding itself how it
>> should be (not) serialized. This is enforced and thus belongs to the
>> typesystem - not to an attribute.
>>
>
> But then what should implement the NotSerializable interface?
> If you want to ignore a string-typed property there would be no option to
> mark it with a NotSerializable interface
> Consider "baz" property in this example:
>
> class Foo {
> protected int $bar = 1;
> #[NotSerializable]
> protected string $baz = 2;
> }
>
The attribute is #[Attribute(Attribute::TARGET_CLASS] so I'm not sure why
you consider this as it's not mentioned in the RFC (and I'm not sure it
would make sense).
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
śr., 3 sty 2024 o 10:09 Nicolas Grekas
napisał(a):
> Hi Nicolas,
>>
>> śr., 3 sty 2024 o 08:12 Nicolas Grekas
>> napisał(a):
>>
>>> Hi Max,
>>>
>>> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
>>> > functionality is already available for internal classes - userspace
>>> should
>>> > benefit from it, too.
>>> >
>>> > The RFC: https://wiki.php.net/rfc/not_serializable
>>> > Proposed implementation: https://github.com/php/php-src/pull/12788
>>> >
>>> > Please let me know what you think.
>>> >
>>>
>>> Regarding the inheritance-related behavior ("The non-serializable flag is
>>> inherited by descendants"), this is very unlike any other attributes, and
>>> this actively prevents writing a child class that'd make a parent
>>> serializable if it wants to.
>>>
>>> To me, if this is really the behavior we want, then the attribute should
>>> be
>>> replaced by a maker interface.
>>> Then, a simple "instanceof NotSerializable" would be enough instead of
>>> adding yet another method to ReflectionClass.
>>>
>>
>> This should be possible without ReflectionClass, see
>> https://3v4l.org/N3fmO
>>
>
> Sure.
>
> My main point is : why use an attribute? this should be an interface to
> me. All semantics match an interface.
>
>
>> From the serialization libraries you can find similar attributes
>>
>
> Those a very different, because they tackle the problem from the
> *external* angle: the attributes there describe how a system *external* to
> the class itself should best serialize an object. There, attributes make
> sense, because they enrich the description of the class without forcibly
> telling what to do with the object
>
> But in the RFC, we're talking about the object deciding itself how it
> should be (not) serialized. This is enforced and thus belongs to the
> typesystem - not to an attribute.
>
But then what should implement the NotSerializable interface?
If you want to ignore a string-typed property there would be no option to
mark it with a NotSerializable interface
Consider "baz" property in this example:
class Foo {
protected int $bar = 1;
#[NotSerializable]
protected string $baz = 2;
}
Cheers,
Michał Marcin Brzuchalski
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
> Hi Nicolas,
>
> śr., 3 sty 2024 o 08:12 Nicolas Grekas
> napisał(a):
>
>> Hi Max,
>>
>> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
>> > functionality is already available for internal classes - userspace
>> should
>> > benefit from it, too.
>> >
>> > The RFC: https://wiki.php.net/rfc/not_serializable
>> > Proposed implementation: https://github.com/php/php-src/pull/12788
>> >
>> > Please let me know what you think.
>> >
>>
>> Regarding the inheritance-related behavior ("The non-serializable flag is
>> inherited by descendants"), this is very unlike any other attributes, and
>> this actively prevents writing a child class that'd make a parent
>> serializable if it wants to.
>>
>> To me, if this is really the behavior we want, then the attribute should
>> be
>> replaced by a maker interface.
>> Then, a simple "instanceof NotSerializable" would be enough instead of
>> adding yet another method to ReflectionClass.
>>
>
> This should be possible without ReflectionClass, see
> https://3v4l.org/N3fmO
>
Sure.
My main point is : why use an attribute? this should be an interface to me.
All semantics match an interface.
> From the serialization libraries you can find similar attributes
>
Those a very different, because they tackle the problem from the *external*
angle: the attributes there describe how a system *external* to the class
itself should best serialize an object. There, attributes make sense,
because they enrich the description of the class without forcibly telling
what to do with the object
But in the RFC, we're talking about the object deciding itself how it
should be (not) serialized. This is enforced and thus belongs to the
typesystem - not to an attribute.
IMHO
Cheers,
Nicolas
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
Hi Nicolas,
śr., 3 sty 2024 o 08:12 Nicolas Grekas
napisał(a):
> Hi Max,
>
> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
> > functionality is already available for internal classes - userspace
> should
> > benefit from it, too.
> >
> > The RFC: https://wiki.php.net/rfc/not_serializable
> > Proposed implementation: https://github.com/php/php-src/pull/12788
> >
> > Please let me know what you think.
> >
>
> Regarding the inheritance-related behavior ("The non-serializable flag is
> inherited by descendants"), this is very unlike any other attributes, and
> this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
>
> To me, if this is really the behavior we want, then the attribute should be
> replaced by a maker interface.
> Then, a simple "instanceof NotSerializable" would be enough instead of
> adding yet another method to ReflectionClass.
>
This should be possible without ReflectionClass, see https://3v4l.org/N3fmO
A property redeclare doesn't inherit the attributes, therefore this might
be a solution for the problem you presented.
Another ad-hoc way could be implementing `__serialize()` in the child class.
I like this feature and the only thing I'd like to consider is different
naming options for voting.
>From the serialization libraries you can find similar attributes, these are:
* https://github.com/Crell/Serde `#[Crell\Serde\Attributes\Field(exclude:
true)]`
* https://github.com/symfony/serializer
`#[Symfony\Component\Serializer\Attribute\Ignore]`
* https://github.com/schmittjoh/serializer
`#[JMS\Serializer\Annotation\Exclude]`
In the Java world also:
* Java EE or Jakarta EE and using JAX-RS `@Transient`
* Jackson `@JsonIgnore`
In the C# world
* .NET _System.Text.Json.Serialization_ `[JsonIgnore]`
So there might be some other naming proposals like maybe just `Ignore` or
`Exclude` ?
What is worth mentioning are solutions allowing to change the strategy on
inheritance which JMS/Serializer supports
https://jmsyst.com/libs/serializer/master/reference/annotations#exclusionpolicy
The `#[ExclusionPolicy]` allows to declare that all properties should be
excluded or none.
Worth mentioning a future scope could introduce more interesting solutions
like:
* Exclude/Ignore conditionally on class or property level, for eg:
a. if the value is null,
b. or is undefined or
c. its value is the default value - which allows to skip serialization of
properties with default values that will be unserialized back with the
default value - see https://3v4l.org/6b1Y6
Similar solutions used by:
* JMS/Serializer `#[JMS\Serializer\Annotation\SkipWhenEmpty]
* .NET `[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]`
see
https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonignorecondition?view=net-8.0
Cheers,
Michał Marcin Brzuchalski
Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute
On Jan 2, 2024, at 23:11, Nicolas Grekas wrote:
> Regarding the inheritance-related behavior ("The non-serializable flag is
> inherited by descendants"), this is very unlike any other attributes, and
> this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
That seems appropriate. Consider a class which has set itself as
#[NotSerializable] because it contains something which mustn't be serialized
(like a secret or an identifier returned by an FFI library), or because the
lifecycle of objects of that class needs to be tightly controlled (like a
singleton or a scope guard). It shouldn't be possible to make any of these
serializable by creating a subclass.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
