[Cache] Errata regarding \DateTimeInterface

379 views
Skip to first unread message

Larry Garfield

unread,
Jul 26, 2016, 3:46:53 PM7/26/16
to PHP Framework Interoperability Group
Hi folks. Based an a recent discussion with someone writing a PSR-6
implementation, I would like to offer the following errata for PSR-6:

https://github.com/php-fig/fig-standards/pull/787

I think the text of it is rather self-explanatory, and I link to the
original GitHub discussion that spawned the question.

Comments/feedback/etc. welcome. As it's a non-trivial change (although
not API changing) it would probably need a vote, but likely an
uncontroversial one.

--Larry Garfield

Brent Shaffer

unread,
Jul 26, 2016, 4:08:20 PM7/26/16
to PHP Framework Interoperability Group
As mentioned by @dwsupplee in this PR commentthe PHP docs for assert state:

Assertions should be used as a debugging feature only. You may use them for sanity-checks that test for conditions that should always be TRUE and that indicate some programming errors if not or to check for the presence of certain features like extension functions or certain system limits and features.

Assertions should not be used for normal runtime operations like input parameter checks. As a rule of thumb your code should always be able to work correctly if assertion checking is not activated.


Wouldn't it be better to use E_USER_WARNING instead?

- Brent

Larry Garfield

unread,
Jul 26, 2016, 5:21:55 PM7/26/16
to php...@googlegroups.com
One of the difficulties is that in PHP 5, a failed type check will trigger an E_RECOVERABLE_ERROR, but on PHP 7 it will now throw a TypeError.  So if the goal is to mimic a type check, it will be wrong in one version or the other.  An assertion, though, can behave the same in both versions.

--Larry Garfield

Daniel Plainview

unread,
Jul 26, 2016, 5:30:41 PM7/26/16
to PHP Framework Interoperability Group
> Throwing `\InvalidArgumentException` is
> another widely used alternative, however, that would be a change in the
> specification (as Calling Libraries would then need to know to catch that
> exception as well).

Let me disagree. Why Calling Libraries have to know about it? Why Calling Libraries don't have to know about AssertionError exception then?

PHP is annoying in this part: it doesn't distinguish between checked and unchecked exceptions at language level
like Java does.
However, it doesn't mean that we have to treat all exceptions as checked ones.
Who said that \InvalidArgumentException or, let's say, \LogicException must be documented?

These exceptions are very close to runtime exceptions in Java: https://docs.oracle.com/javase/7/docs/api/java/lang/RuntimeException.html
And runtime exceptions include, for example, IllegalArgumentException.
These exceptions notify about errors, this is not public contract of an interface. 
You usually don't need to catch them directly. They just should be fixed at the code level. Or infrastructure level.

(Moreover, in my opinion, \LogicException is something that close to compile errors in Java, not even runtime exceptions, because it includes http://php.net/manual/en/class.badmethodcallexception.php).

Brent Shaffer

unread,
Jul 26, 2016, 5:36:09 PM7/26/16
to PHP Framework Interoperability Group
assert issues a warning or exception based on your "assert" configuration. If the goal is to mimic a type check, we could *actually* mimic a type check and use E_RECOVERABLE_ERROR in PHP 5 and throw TypeError in PHP 7. But we aren't doing this, so using "assert" does not mimic a type check. Since the PHP documentation says to not use "assert", and it essentially issues a warning anyway, why not just issue a WARNING without assert?

Larry Garfield

unread,
Jul 26, 2016, 7:01:17 PM7/26/16
to php...@googlegroups.com
From http://www.php-fig.org/psr/psr-6/#error-handling

" For that reason Implementing Libraries MUST NOT throw exceptions other than those defined by the interface"

The spec explicitly says not to throw exceptions except where explicitly specified.  We can't recommend throwing an extra exception then without it being a BC break in the specification, which would affect potentially all current users of it.  (There are multiple PSR-6 implementations already in the wild, and no doubt projects using them.)

Whatever the arguments for an exception may be, I don't think we can consider them without a BC break.

--Larry Garfield
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/8a897f5c-41b1-436e-95ed-38397cfccead%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Daniel Plainview

unread,
Jul 26, 2016, 7:06:31 PM7/26/16
to PHP Framework Interoperability Group
Is it bad if you break BC with non-sense implementations?

Larry Garfield

unread,
Jul 27, 2016, 1:05:52 AM7/27/16
to php...@googlegroups.com
I hardly think "nonsense [sic]" is an appropriate way to describe implementations that are following the spec as currently written.

--Larry Garfield

Matteo Beccati

unread,
Jul 27, 2016, 3:57:40 AM7/27/16
to php...@googlegroups.com
Hi,
I agree that using assert() is a fairly weird and I would vote against it.

Also I would expect to be able to pass a null value in order to reset
the expiration to (never|the default). That's in fact supported:

"If null is passed explicitly, a default value MAY be used..."


Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Takashi Matsuo

unread,
Jul 27, 2016, 11:38:48 AM7/27/16
to PHP Framework Interoperability Group

On Tuesday, July 26, 2016 at 4:01:17 PM UTC-7, Larry Garfield wrote:
From http://www.php-fig.org/psr/psr-6/#error-handling

" For that reason Implementing Libraries MUST NOT throw exceptions other than those defined by the interface"

The spec explicitly says not to throw exceptions except where explicitly specified.  We can't recommend throwing an extra exception then without it being a BC break in the specification, which would affect potentially all current users of it.  (There are multiple PSR-6 implementations already in the wild, and no doubt projects using them.)

Whatever the arguments for an exception may be, I don't think we can consider them without a BC break.

Re: BC break

Is this a really big problem? The changes in PSRs doesn't immediately change libraries in the wild. The psr-6 libraries can still provide another major version, so that the users of those libraries can keep the old behavior with the old stables.

Actually there are some libraries already throwing InivalidArgumentException:

Of course yours is using assertion:

If we decide on either of assertion or exception, there will be certainly BC breaks in either side, and you can not avoid them :(
If you can not avoid them, let's think this way; it is just a good chance to learn how to change PSRs :)

Another small benefit of throwing exception is that, it will allow libraries to notify users when it is invalid (for example past date). `CacheItemInterface::expiresAt()` returns the object, so there is no reliable way to notify such errors to users (warnings can be easily neglected, you know?).

Someone unintentionally passes a past date and the value will never be cached, then they might keep banging their heads for hours.

-- Takashi Matsuo

Daniel Hunsaker

unread,
Jul 27, 2016, 1:45:46 PM7/27/16
to PHP Framework Interoperability Group


On Wednesday, July 27, 2016 at 9:38:48 AM UTC-6, Takashi Matsuo wrote:

On Tuesday, July 26, 2016 at 4:01:17 PM UTC-7, Larry Garfield wrote:
From http://www.php-fig.org/psr/psr-6/#error-handling

" For that reason Implementing Libraries MUST NOT throw exceptions other than those defined by the interface"

The spec explicitly says not to throw exceptions except where explicitly specified.  We can't recommend throwing an extra exception then without it being a BC break in the specification, which would affect potentially all current users of it.  (There are multiple PSR-6 implementations already in the wild, and no doubt projects using them.)

Whatever the arguments for an exception may be, I don't think we can consider them without a BC break.

Re: BC break

Is this a really big problem? The changes in PSRs doesn't immediately change libraries in the wild. The psr-6 libraries can still provide another major version, so that the users of those libraries can keep the old behavior with the old stables.

According to the PSR Workflow, a major version bump would require a replacement PSR.  Unless I'm not understanding the bylaw correctly.
 
Actually there are some libraries already throwing InivalidArgumentException:

Of course yours is using assertion:

If we decide on either of assertion or exception, there will be certainly BC breaks in either side, and you can not avoid them :(
If you can not avoid them, let's think this way; it is just a good chance to learn how to change PSRs :)

Another small benefit of throwing exception is that, it will allow libraries to notify users when it is invalid (for example past date). `CacheItemInterface::expiresAt()` returns the object, so there is no reliable way to notify such errors to users (warnings can be easily neglected, you know?).

Someone unintentionally passes a past date and the value will never be cached, then they might keep banging their heads for hours.

For my own part, I would argue that InvalidArgumentException is one used internally by PHP itself, so could be considered an exception to the exception, since it could as easily come from PHP itself as from a cache implementation.  Yes, it's being thrown in a new context, but aren't all Exceptions that PHP throws itself implicitly on any list of expected Exceptions one's application might receive?

That points to an issue for the Errata to address, of course, in that PHP might throw exceptions of its own accord, outside the control of the PSR-6 lib itself, and to be aware of the built-in ones when using a PSR-6 implementation.  It also points to at least two implementations not actually following the spec as currently written, but again, for my part, their choice can be fairly easily justified, despite actually violating the spec.

So, +1 for InvalidArgumentException and an Errata note that built-in descendants of Exception are implicitly approved on any interface, if for no other reason than PHP itself might throw them before the implementation even gets control.

Takashi Matsuo

unread,
Jul 27, 2016, 2:02:54 PM7/27/16
to PHP Framework Interoperability Group
Great idea :)

FYI, the current implementations above are both throwing an implementation of `Psr\Cache\InvalidArgumentException`, but they are also descendant of builtin InvalidArgumentException.
 

Larry Garfield

unread,
Jul 27, 2016, 2:04:52 PM7/27/16
to php...@googlegroups.com
Er.  Except that's not true.  PHP itself throws exceptions only in two very specific cases:

* PDO, if configured to do so.
* random_int()/random_bytes(), if no entropy source can be found.

As noted before, a failed type check in PHP 5 is equivalent to

trigger_error('error message', E_RECOVERABLE_ERROR).

In PHP 7, it's equivalent to

throw new TypeError(...);

(TypeError implementing Throwable, but not being a descendant of \Exception.)

See:

https://3v4l.org/XL0Of

Neither of those are InvalidArgumentException.

The spec very explicitly says that cache failures must be non-fatal to the application.  Throwing uncaught exceptions is fatal to the application.  In fact, the spec explicitly says that exceptions from the underlying engine (PDO, file system, Redis, whatever) must be caught and normalized to just a normal cache failure so that it's non-fatal.

Remember, we're talking about a very narrow problem space: Passing a non-DateTimeInterface object to a method that expects one.  There are no conditions where that is anything other than the programmer screwing up, probably very very close to the call itself.  (eg, if you call expiresAt('2016-07-29'), then according to the spec you're just wrong and should fix your damned code. <g>)

If the spec were still subject to change then I'd be more open to InvalidArgumentException, as the class does throw that explicitly in a few other places (that are also "Developer, you're just wrong" cases).  As this is just an errata, however, I don' think throws are on the table.

--Larry Garfield

Brent Shaffer

unread,
Jul 27, 2016, 2:21:30 PM7/27/16
to PHP Framework Interoperability Group
Unfortunately, there is no way to trigger an E_RECOVERABLE_ERROR. You can only trigger user-level errors, so E_USER_ERROR is the closest you can get. E_USER_ERROR is catchable via set_error_handler(), so this should be approximately the same.

- Brent

Brent Shaffer

unread,
Jul 28, 2016, 12:19:50 PM7/28/16
to PHP Framework Interoperability Group
If we want to mimic a type error, we could do the following:

    $error = sprintf('Argument 1 passed to %s::expiresAt() must be an instance of DateTime, string given', get_class($this));
    if (class_exists('TypeError')) {
        throw new \TypeError($error);
    }
    trigger_error($error, E_USER_ERROR);

It's verbose but it's the closest we can get to native handling while still obeying the spec.

- Brent

Larry Garfield

unread,
Jul 28, 2016, 12:28:57 PM7/28/16
to php...@googlegroups.com
Hm.  I would be open to this.  We can include the sample code to copy-paste in the errata, and include it in the utlis package.

Does anyone object to this recommendation for the errata?  Paul and Robert especially?

--Larry Garfield
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

Daniel Hunsaker

unread,
Jul 28, 2016, 1:31:50 PM7/28/16
to PHP Framework Interoperability Group
Er.  Except that's not true.  PHP itself throws exceptions only in two very specific cases:

* PDO, if configured to do so.
* random_int()/random_bytes(), if no entropy source can be found.

Hrm.  Must've been working inside frameworks too long.  Of course PHP doesn't throw exceptions - it didn't even have them for the longest time.  Point ceded, proposal withdrawn.

As noted before, a failed type check in PHP 5 is equivalent to

trigger_error('error message', E_RECOVERABLE_ERROR).

In PHP 7, it's equivalent to

throw new TypeError(...);

(TypeError implementing Throwable, but not being a descendant of \Exception.)

See:

https://3v4l.org/XL0Of

Neither of those are InvalidArgumentException.

Yeah, errors in both cases...

The spec very explicitly says that cache failures must be non-fatal to the application.  Throwing uncaught exceptions is fatal to the application.  In fact, the spec explicitly says that exceptions from the underlying engine (PDO, file system, Redis, whatever) must be caught and normalized to just a normal cache failure so that it's non-fatal.

Remember, we're talking about a very narrow problem space: Passing a non-DateTimeInterface object to a method that expects one.  There are no conditions where that is anything other than the programmer screwing up, probably very very close to the call itself.  (eg, if you call expiresAt('2016-07-29'), then according to the spec you're just wrong and should fix your damned code. <g>)

If only PHP < 5.5 didn't need to be supported, this discussion wouldn't even be taking place, because type hinting would make PHP handle this for us instead.  (Or if the PHP devs had made DateTimeImmutable extend DateTime, even that wouldn't have been an issue - but then libs relying on getting DateTime objects could be handed DateTimeImmutable objects instead, and fail spectacularly.  Oh well.)  Exceptions do make the most sense during development to force developers to notice coding errors, but since we need to avoid issues in production, and the PSR doesn't specify that any exceptions can be thrown there, I do think our hands are rather tied.
 
If the spec were still subject to change then I'd be more open to InvalidArgumentException, as the class does throw that explicitly in a few other places (that are also "Developer, you're just wrong" cases).  As this is just an errata, however, I don' think throws are on the table.

The Errata *might* still need to be updated to clarify whether throwing \Errors is acceptable, as they aren't \Exceptions, but \Throwables of a different kind.  My guess is they aren't acceptable either, given the reasoning behind prohibiting exceptions (lower case, which also leans toward all \Throwables) in the first place.

--Larry Garfield

My only real concern is that with assertions disabled, invalid arguments will cause other issues in the lib, with no way for the lib to catch and properly handle them internally - because the lib is relying on the assertion to do that for it.  Mimicking a type error may be a better option in this case, since these invalid values "MUST be treated as an invalid syntax error"...

Paul Dragoonis

unread,
Jul 30, 2016, 12:30:04 PM7/30/16
to php...@googlegroups.com
On Thu, Jul 28, 2016 at 5:28 PM, Larry Garfield <la...@garfieldtech.com> wrote:
Hm.  I would be open to this.  We can include the sample code to copy-paste in the errata, and include it in the utlis package.

Does anyone object to this recommendation for the errata?  Paul and Robert especially?

--Larry Garfield

Hey Larry,

Can you ping me on twitter or something when you need my attention, I'm not checking every thread message. You should ping Robert manually too as I doubt he's noticed this, his input will be of value.

I agree with the rejection of non DateTime|DateTimeImmutable parameters to CacheItemInterface::expiresAt($expiration).

I'm happy with the trigger_error() approach that Brent Shaffer provided as a way to mimic PHP5 and PHP7 invalid type handling.

Many thanks,
Paul
 


On 07/28/2016 11:19 AM, Brent Shaffer wrote:
If we want to mimic a type error, we could do the following:

    $error = sprintf('Argument 1 passed to %s::expiresAt() must be an instance of DateTime, string given', get_class($this));
    if (class_exists('TypeError')) {
        throw new \TypeError($error);
    }
    trigger_error($error, E_USER_ERROR);

It's verbose but it's the closest we can get to native handling while still obeying the spec.

- Brent

On Wednesday, July 27, 2016 at 11:21:30 AM UTC-7, Brent Shaffer wrote:
Unfortunately, there is no way to trigger an E_RECOVERABLE_ERROR. You can only trigger user-level errors, so E_USER_ERROR is the closest you can get. E_USER_ERROR is catchable via set_error_handler(), so this should be approximately the same.

- Brent
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/b38673aa-3cac-4c29-bbc3-69030036dfbf%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

Larry Garfield

unread,
Jul 31, 2016, 2:29:39 PM7/31/16
to php...@googlegroups.com
On 07/30/2016 11:29 AM, Paul Dragoonis wrote:

On Thu, Jul 28, 2016 at 5:28 PM, Larry Garfield <la...@garfieldtech.com> wrote:
Hm.  I would be open to this.  We can include the sample code to copy-paste in the errata, and include it in the utlis package.

Does anyone object to this recommendation for the errata?  Paul and Robert especially?

--Larry Garfield

Hey Larry,

Can you ping me on twitter or something when you need my attention, I'm not checking every thread message. You should ping Robert manually too as I doubt he's noticed this, his input will be of value.

I agree with the rejection of non DateTime|DateTimeImmutable parameters to CacheItemInterface::expiresAt($expiration).

I'm happy with the trigger_error() approach that Brent Shaffer provided as a way to mimic PHP5 and PHP7 invalid type handling.

Many thanks,
Paul

I have pushed another commit to the PR that updates the recommended handling per Bret's suggestion.

If no one objects by mid-week or so, I will call a vote on this errata.

--Larry Garfield

Matteo Beccati

unread,
Aug 1, 2016, 2:01:29 AM8/1/16
to php...@googlegroups.com
Hi Larry,

On 31/07/2016 20:29, Larry Garfield wrote:

> I have pushed another commit to the PR that updates the recommended
> handling per Bret's suggestion.
>
> If no one objects by mid-week or so, I will call a vote on this errata.


I still haven't seen any replies to my objection re: null input. I don't
think the PR is ready for voting just yet.

Larry Garfield

unread,
Aug 1, 2016, 12:25:36 PM8/1/16
to php...@googlegroups.com
On 08/01/2016 01:01 AM, Matteo Beccati wrote:
> Hi Larry,
>
> On 31/07/2016 20:29, Larry Garfield wrote:
>
>> I have pushed another commit to the PR that updates the recommended
>> handling per Bret's suggestion.
>>
>> If no one objects by mid-week or so, I will call a vote on this errata.
>
>
> I still haven't seen any replies to my objection re: null input. I
> don't think the PR is ready for voting just yet.
>
>
> Cheers

Ah, I missed that message, sorry. Valid point. I've updated the PR
accordingly to also allow a NULL through.

--Larry Garfield

Robert Hafner

unread,
Aug 1, 2016, 1:08:20 PM8/1/16
to php...@googlegroups.com
I haven't had a chance to review this but will make sure to do so this week.

Rob
> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/15e42777-cfd1-54af-5c9f-ff22d6ec8e89%40garfieldtech.com.

Larry Garfield

unread,
Aug 7, 2016, 10:58:50 PM8/7/16
to php...@googlegroups.com
Robert, any feedback? It sounds like everyone else is on board so I
want to get this Errata to a vote ASAP, then update the util library.

--Larry Garfield

Nicolas Grekas

unread,
Aug 8, 2016, 12:27:10 PM8/8/16
to php...@googlegroups.com
Hi

sorry to come late in the story. I'm the author of the PSR-6 implementation in Symfony Cache 3.1.

There are more grey zones on PSR-6 and maybe an errata should address them also?
Below is the list I computed.

First, on the very matter of this topic, IMHO the best behavior is to trigger an InvalidArgumentException. Here is why:
- there is no BC issue: the behavior on this case was not defined in the spec; and we cannot reasonably expect that someone wrong some *working* code that expects *not* getting an exception here.
- because the spec says that InvalidArgumentException are expected in some situations, users should already be prepared to deal with them.
- and last but not least: hey! this is really an invalid argument! As far as DX is concerned, it would be inconsistent to have different argument handling strategies in the spec.

IMHO again, triggering an E_USER_ERROR would be make PSR-6 implementation very fragile. In the same way that having a lib that calls "exit()" is a really bad practice, using E_USER_ERROR would be fatal by default, unless one uses a custom that error handler that triggers... an exception (or would that break PSR-6 again?). By my own personal standards, I tend avoid using any lib that exit() of E_USER_ERROR. Please don't force me (us) to break this principle.

I've read that InvalidArgumentException is not possible per the spec. But if you read that full paragraph again, it rationale is built for *runtime errors* triggered "by an underlying data store". The situation here is not the same: providing a wrong argument to a $item->expire*() is a devtime issue. It won't happend reasonably at runtime. Thus, I thing the argument against InvalidArgumentException does not apply.

As promised, here is my list of grey zones in PSR-6, with the behavior implemented in Symfony in brackets:

- What happens when one provides an unserializable value to save? ($pool->save/commit() returns false. Note the difference with the previous issue: CacheItemInterface throws, CacheItemPoolInterface does not throw, that's the rule I inferred.)
- What happens when one of several values can't be saved? (the serializable ones are saved, the others ignored and $pool->save/commit() returns false)
- What happens when a value cannot be *un*serialized? (return a miss) Should we allow a __PHP_Incomplete_Class? (no)
- What happens when an already expired or negative lifetime item is saved? (invalidate any existing value & return true)
- Are deferred items subject to expiration? (yes)
- Is getting a deferred item a hit? (yes)
- Are deferred items commited atomically? (no)
- Should we store a `null` value for items that were a "miss" but have no value set? (yes)
- Should committed item objects have isHit() === true after being saved? (no: not when miss at the origin)
- What's the rationale for having reserved characters in keys, and why these ones specifically? (no idea)

Given that we already managed to answer all those questions while implementing the spec, I hope we didn't make any mistake in interpreting the PSR. Please tell me if we did anything wrong here.

For future implementers, it could maybe help to make some of these points less implicit and more explicit? I'll let you cherry-pick :)

Cheers,
Nicolas

Larry Garfield

unread,
Aug 8, 2016, 2:36:29 PM8/8/16
to php...@googlegroups.com
On 08/08/2016 11:26 AM, Nicolas Grekas wrote:
> Hi
>
> sorry to come late in the story. I'm the author of the PSR-6
> implementation in Symfony Cache 3.1.

Greetings!

I'm going to only reply to the second half for the moment, as it's new
stuff. I'll let someone else chime in on the DateTime error handling first.

> As promised, here is my list of grey zones in PSR-6, with the behavior
> implemented in Symfony in brackets:
>
> - What happens when one provides an unserializable value to save?
> ($pool->save/commit() returns false. Note the difference with the
> previous issue: CacheItemInterface throws, CacheItemPoolInterface does
> not throw, that's the rule I inferred.)

Return false, save nothing, and get a cache miss as a result later. I
believe that's the correct behavior per spec.

> - What happens when one of several values can't be saved? (the
> serializable ones are saved, the others ignored and
> $pool->save/commit() returns false)

That seems like a reasonable solution. Save what you can, don't save
the rest.

> - What happens when a value cannot be *un*serialized? (return a miss)
> Should we allow a __PHP_Incomplete_Class? (no)

If a complete and valid value cannot be returned in a type-safe manner
for any reason, it's a cache miss. I'd say you're correct per spec.

> - What happens when an already expired or negative lifetime item is
> saved? (invalidate any existing value & return true)

I'd say this is correct. Technically, I believe the spec would say you
can save it and thus it will be a miss when it's next read, but since
you know up front it's guaranteed to be a miss not saving it at all
seems like a valid optimization. You're correct per spec.

> - Are deferred items subject to expiration? (yes)

Yes.

> - Is getting a deferred item a hit? (yes)

Yes.

> - Are deferred items commited atomically? (no)

That is unspecified, so I guess either is legal? The intent for
deferred items was to allow for multi-set operations, or transactions on
an SQL DB. Some of those will result in a multi-value atomic operation
anyway. Since the spec doesn't specify, however, I'd say it's
unspecified. (I don't know if we can practically require one way or
another.)

> - Should we store a `null` value for items that were a "miss" but have
> no value set? (yes)

If a CacheItem represents a cache miss, $item->get() MUST return null,
per spec. How you do that internally is up to you.

> - Should committed item objects have isHit() === true after being
> saved? (no: not when miss at the origin)

Saving a cache item should probably be considered a destructive
operation on the object. Honestly I could see an argument either way here.

> - What's the rationale for having reserved characters in keys, and why
> these ones specifically? (no idea)

Reserved for usage in future extensions, like namespacing or tags. Those
seemed like the most likely ones we'd want to use.

> Given that we already managed to answer all those questions while
> implementing the spec, I hope we didn't make any mistake in
> interpreting the PSR. Please tell me if we did anything wrong here.

It looks like you got it nearly all right, except for places where the
spec doesn't specify either way. :-)

> For future implementers, it could maybe help to make some of these
> points less implicit and more explicit? I'll let you cherry-pick :)

Thoughts on which ones? Most of them seem (to me at least) fairly
straightforward reads of the spec, and since you got them all right...

--Larry Garfield

Nicolas Grekas

unread,
Aug 9, 2016, 11:22:34 AM8/9/16
to php...@googlegroups.com
Thanks for the reply Larry,

I'd like to report that there has been great collaboration with the php-cache implementers around the test suite, that is now used as a reference by both php-cache and Symfony:
https://github.com/php-cache/integration-tests/

The good thing is that all these small implications of the PSR can now be checked easily.
It would maybe be worth mentioning that somewhere.


- What happens when a value cannot be *un*serialized? (return a miss) Should we allow a __PHP_Incomplete_Class? (no)

If a complete and valid value cannot be returned in a type-safe manner for any reason, it's a cache miss.  I'd say you're correct per spec.

This has come very late to my mind and in fact its not yet merged into Symfony nor in the reference test suite. I really doubt that other implementations deal with unserialize failures today, esp. the __PHP_Incomplete_Class issue.
An explicit reminder here might be a good idea (also because it's a hard to test behavior).

 

- What happens when an already expired or negative lifetime item is saved? (invalidate any existing value & return true)

I'd say this is correct. [...]

This one could be mentioned, at least it came late to us while working on PSR-6... :)
 
 
- What's the rationale for having reserved characters in keys, and why these ones specifically? (no idea)

Reserved for usage in future extensions, like namespacing or tags. Those seemed like the most likely ones we'd want to use.

FYI, we implemented tags in Symfony, and namespacing is ready also. We didn't require using any special characters in userland/public keys, but we did use reserved chars internally, so good for me.

Nicolas
Reply all
Reply to author
Forward
0 new messages