[PSR-6] Return values

384 views
Skip to first unread message

Larry Garfield

unread,
Aug 11, 2015, 1:34:30 AM8/11/15
to PHP Framework Interoperability Group
In a sidebar conversation, Robert and I noticed the following issue:

    /**
     * Returns the expiration time of a not-yet-expired cache item.
     *
     * If this cache item is a Cache Miss, this method MAY return the time at
     * which the item expired or the current time if that is not available.
     *
     * @return \DateTime|boolean
     *   The timestamp at which this cache item will expire or false if it does not
     *   expire.
     */
    public function getExpiration();



This poses a problem, as it violates these return value guidelines:

http://www.garfieldtech.com/blog/empty-return-values

Specifically:

"FALSE is a boolean. It is a value,
specifically the opposite of<
span class="Apple-converted-space"> TRUE. It is
not the absence of a value of some other type. If a function can
return FALSE, then the only other
thing it should be able to return is TRUE (or NULL, in weird cases). If you can return a non-boolean, then you must never return a boolean."

And:

"In
short: Write good contracts. Obey your contracts. You get one, and only
one, return type. Don't make me do extra work because yo
u wrote a dumb contract."

However, returning false there is a dumb contract, because it means you always need to type check the return value, which is stupid.  (I haven't checked git blame, but I probably wrote the line in question. So, yeah... :-) )

Here's the problem space:

- If getExpiration() is called on a cache hit, and the item has a known expiration time, that is the value that should be returned and never anything else.
- If getExpiration() is called on a cache miss, an implementing library MAY still know when the item expired, or it may not. If it does, return that. If not, return now().  That way, a check for if ($item is expired or is soon to expire) will still work as expected, and (say) a pre-regeneration script can still behave as expected.
- If getExpiration() is called on a cache hit, and the item has no expiration time (ie, cache in the future)... then what?  Returning false is wrong, because it's a type mismatch, but what do we do instead?

I don't believe DateTime has an equivalent of PHP_MAX_INT, as it uses 64 bit time so its highest possible value is somewhere around when the Earth get engulfed by the sun.  The best option we could come up with, which none of us are particularly fond of, is to establish something like "now() +5 years" as the canonical definition of "forever".  It's technically not eternal, but far enough in the future that it may as well be.  We've heard that Laravel uses a similar definition of forever currently.  (Taylor, can you verify?)

How do people feel about that?  Is that an acceptable way to define "no expiration" (1 year from now, 5 years, something very large), or is there some better way that is still type-safe?

I believe this is a Review-period-safe tweak, and if Beau disagrees he hasn't said so. :-)

--Larry Garfield

Larry Garfield

unread,
Aug 11, 2015, 1:36:39 AM8/11/15
to php...@googlegroups.com
Ugh. OK, that formatting was totally fubar. I hate formatted
emails... Let me try this again...

-----

In a sidebar conversation, Robert and I noticed the following issue:

/**
* Returns the expiration time of a not-yet-expired cache item.
*
* If this cache item is a Cache Miss, this method MAY return the time at
* which the item expired or the current time if that is not available.
*
* @return \DateTime|boolean
* The timestamp at which this cache item will expire or false if it does not
* expire.
*/
public function getExpiration();



This poses a problem, as it violates these return value guidelines:

http://www.garfieldtech.com/blog/empty-return-values

Specifically:

"FALSE is a boolean. It is a value, specifically the opposite of TRUE. It is not the absence of a value of some other type. If a function can return FALSE, then the only other thing it should be able to return is TRUE (or NULL, in weird cases). If you can return a non-boolean, then you must never return a boolean."

And:

"In short: Write good contracts. Obey your contracts. You get one, and only one, return type. Don't make me do extra work because you wrote a dumb contract."

Matteo Beccati

unread,
Aug 11, 2015, 2:23:22 AM8/11/15
to php...@googlegroups.com
Hi Larry,

On 11/08/2015 07:36, Larry Garfield wrote:
> - If getExpiration() is called on a cache hit, and the item has a known
> expiration time, that is the value that should be returned and never
> anything else.
> - If getExpiration() is called on a cache miss, an implementing library
> MAY still know when the item expired, or it may not. If it does, return
> that. If not, return now(). That way, a check for if ($item is expired
> or is soon to expire) will still work as expected, and (say) a
> pre-regeneration script can still behave as expected.

Can't we just drop this and return null, just like get() does?

> - If getExpiration() is called on a cache hit, and the item has no
> expiration time (ie, cache in the future)... then what? Returning false
> is wrong, because it's a type mismatch, but what do we do instead?

Once again, I would return null.

> How do people feel about that? Is that an acceptable way to define "no
> expiration" (1 year from now, 5 years, something very large), or is
> there some better way that is still type-safe?

I personally don't like it. The fact that "no expiration" (e.g.
now+5yrs) might be lesser than an actual expiration (now+6yrs) feels
wrong. Capping expirations might solve that particular issue, but then
again I'd find null more appropriate.


Cheers
--
Matteo Beccati

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

Larry Garfield

unread,
Aug 11, 2015, 12:10:08 PM8/11/15
to php...@googlegroups.com
Returning NULL is less-wrong than returning false, I agree. However, it
still means every single call to getExpiration() ever needs to get
wrapped in an if statement, because the it's not type safe. Especially
with PHP 7 coming, single return types is going to be a big deal and
greatly improves code quality. I'd rather establish a clear
domain-specific empty value (equivalent of an empty string for strings
or 0 for ints, etc.) than go with nulls or an inconsistent type. The
latter is a great place for bugs.

--
--Larry Garfield

Matteo Beccati

unread,
Aug 11, 2015, 1:07:40 PM8/11/15
to php...@googlegroups.com
Hi Larry,

On 11/08/2015 18:09, Larry Garfield wrote:
> Returning NULL is less-wrong than returning false, I agree. However, it
> still means every single call to getExpiration() ever needs to get
> wrapped in an if statement, because the it's not type safe. Especially
> with PHP 7 coming, single return types is going to be a big deal and
> greatly improves code quality. I'd rather establish a clear
> domain-specific empty value (equivalent of an empty string for strings
> or 0 for ints, etc.) than go with nulls or an inconsistent type. The
> latter is a great place for bugs.

Alas, there is no actual empty value for date/time and especially not
one that is in the future and could mean "never" when used for
comparisons. In fact it's not even an empty value you're looking for,
but the equivalent of +inf.

I agree that it's going to be inconvenient, but how many use cases do
actually require a call to getExpiration? I wouldn't sacrifice the
correctness of the interface for what seems a little convenience to me.

In case, if you really want to fabricate a +inf you could try:

\DateTime::createFromFormat("U", PHP_INT_MAX)

but then again it would be a bit weird to see:

"292277026596-12-04 15:30:07"

Beau Simensen

unread,
Aug 11, 2015, 3:09:11 PM8/11/15
to PHP Framework Interoperability Group
I think we should ask what practical purposes do we have for "getting the expiration."

If it is to see if something will expire in the near future *and no other reason* then I think coming up with a reasonably "far into the future" expiration that won't collide with "when we might be taking action" is a decent solution. If this is primarily to be used for stampede protection type stuff where "hey, if you are going to expire in the next 5 minutes, maybe I'll invalidate you," then I doubt expiration dates years out will cause any problems.

I mean, seriously, if you get something from a cache and get its expiration and see it is 5 years into the future, and you act on it somehow... how relevant will that *actually be* when those five years are up if you then go back to the cache again (finally!) only to find out, hey! ...it expires another 5 years into the future....

I'm probably being too naive here so if anyone has good reasons for using expiration where +inf is meaningful to the point that we have to return null instead of a time X years into the future, I think it will be helpful in figuring out what we need to do here.

That said, I'd be equally happy to have the specification be, "for items with no expiration time, return DateTime::createFromFormat("U", PHP_INT_MAX)" as that is the best we can do (quite literally) and avoid the, "why did you pick X years arbitrarily" question even though I think we can defend whatever "X years" we pick if we go that route.

I'm not worried about people seeing that time; that would probably only come up in debugging and I'd like to think it would be obvious that it is a very big date sometime far into the future and people (developers) would understand.

Matteo Beccati

unread,
Aug 11, 2015, 5:48:45 PM8/11/15
to php...@googlegroups.com
Hi Beau,

On 11/08/2015 21:09, Beau Simensen wrote:
> I think we should ask what practical purposes do we have for "getting
> the expiration."
>
> If it is to see if something will expire in the near future *and no
> other reason* then I think coming up with a reasonably "far into the
> future" expiration that won't collide with "when we might be taking
> action" is a decent solution. If this is primarily to be used for
> stampede protection type stuff where "hey, if you are going to expire in
> the next 5 minutes, maybe I'll invalidate you," then I doubt expiration
> dates years out will cause any problems.

True, I don't know exactly what's the expected purpose of the method.
Maybe someone could shed a light.

I certainly hope that stampede protection can be offered by the PSR-6
compliant libraries (albeit with some limitations we've discussed at
length) and not something that the end users should worry about directly.


> I mean, seriously, if you get something from a cache and get its
> expiration and see it is 5 years into the future, and you act on it
> somehow... how relevant will that *actually be* when those five years
> are up if you then go back to the cache again (finally!) only to find
> out, hey! ...it expires another 5 years into the future....
>
> I'm probably being too naive here so if anyone has good reasons for
> using expiration where +inf is meaningful to the point that we have to
> return null instead of a time X years into the future, I think it will
> be helpful in figuring out what we need to do here.

I guess so. I just feel it's awkward to define a standard interface
where where a getter method returns something different from what is
used in what could be considered its setter method, e.g.

$item->expireAt(null);

$item->getExpiration() === new DateTime("Stardate 46379.1")

Niels Sonnich Poulsen

unread,
Aug 12, 2015, 12:09:48 PM8/12/15
to PHP Framework Interoperability Group
Hi all,

I felt the need to share my opinion, so here goes...

If a cache item doesn't expire it doesn't have an expiration date. In that case getExpiration() should obviously not return a date, that's what NULL values are for. Returning some magical DateTime-object, using either PHP_INT_MAX or some arbitrary point in the future, just strikes me as bad design. You would essentially be using a value to represent a non-value (again that's what NULL is for).
If you would really want a single non-NULL return type, then I think a much more correct way of doing it would be to return some sort of "Option" object (or a "Maybe" object), but without parametric polymorphism (e.g. if the return type could be specified as \Option<\DateTime>) I don't think PHP is the right language for that.
To recap, I think that the only correct return type (in PHP) for a method, that sometimes does't return a date, is \DateTime|null.

That being said, I don't quite understand why the getExpiration() method is part of the interface in the first place.

Dracony

unread,
Aug 13, 2015, 4:16:37 PM8/13/15
to PHP Framework Interoperability Group
NULL seems a perfectly valid choice for this sort of thing, well at least until DataTime supports 'Apocalypse' as a valid date =)

Robert Deutz

unread,
Aug 14, 2015, 6:42:50 AM8/14/15
to PHP Framework Interoperability Group
Support for null, seems a good choice to me.

Robert Deutz 

Larry Garfield

unread,
Aug 14, 2015, 11:27:14 AM8/14/15
to php...@googlegroups.com
A quick recap of the current status and proposals:

CacheItem::getExpiration() currently returns DateTime|FALSE. This is
wrong in many ways. I don't think anyone is defending this status quo.
The three cases to cover are "expired at some unknown point in the
past", "will expire at some known point in the future", and "will expire
with the heat-death of the universe."

It sounds like everyone is OK with "expired at some unknown point in the
past" being DateTime(now()), which I'm fine with. (If someone has a
compelling argument against that, please speak up now with a good argument.)

The problem is how to return "will expire with the heat-death of the
universe". Proposed options include:

1) Change it to DateTime|NULL, with NULL indicating "never". Pro: There
is no expiration, so there's no value. Con: Inconsistent return types
mean that every single person who ever calls getExpiration() must,
without fail, always wrap it in an if() check or risk a fatal error.

Personally I reject this option as untenable. Forcing everyone who uses
this interface to wrap calls in an if-check is rude, error prone, and
will undoubtedly lead to fatal bugs.

2) remove CacheItem::getExpiration() entirely. Pro: What purpose does
it actually serve? It was added more out of habit than to address a
specific need, so eliminate the source of the problem. Con: That doesn't
mean it's not useful, such as for cache refresh processes, etc.

I'm open to this approach if we can verify we're not cutting off some
important use case this way.

3) Define some arbitrary date in the future to mean "forever"; now+5
years, DateTime(PHP_MAX_INT), or similar. Pros: This gives us a
consistent and type-safe return value, and conditionals against
getExpiration() should always "work as expected". Con: Arbitrary magic
values can come back to bite you, and are not obvious to developers
what's magic about them. We can documented it but really, who reads docs?

I'm open to this approach if we're comfortable defining what "forever"
means chronologically. I don't have a strong preference as to what we
call it.

Any other strong feelings on 2 or 3, and specifically the caveats on both?

--Larry Garfield

Korvin Szanto

unread,
Aug 14, 2015, 4:24:17 PM8/14/15
to php...@googlegroups.com
I'd like to express concern with using the equivalent of DateTime(now()) to express an expired cache item. It occurs to me that many end users will do the equivalent of:

$now = time();
$expiration = $item->getExpiration();

if ($expiration->getTimestamp() < $now) {
    ...
}

and may spend a lot of time trying to determine why their cache items don't expire. In my mind something that is "expired" is something with an expiration date in the past.

I think we should make getExpiration a DateTime|null that simply returns the value of the expiration, and make a new isExpired method that will manage the discrepancies. 

Thoughts?

--
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/55CE08CB.6040901%40garfieldtech.com.
For more options, visit https://groups.google.com/d/optout.

Woody Gilk

unread,
Aug 14, 2015, 5:09:22 PM8/14/15
to php...@googlegroups.com
On Fri, Aug 14, 2015 at 3:24 PM, Korvin Szanto <korvin...@gmail.com> wrote:
I think we should make getExpiration a DateTime|null that simply returns the value of the expiration, and make a new isExpired method that will manage the discrepancies. 

This is the most logical suggestion so far. +1

Dracony

unread,
Aug 15, 2015, 10:17:58 AM8/15/15
to PHP Framework Interoperability Group
I'd like to express concern with using the equivalent of DateTime(now()) to express an expired cache item. It occurs to me that many end users will do the equivalent of:
if ($expiration->getTimestamp() < $now) {
    ...
}

Users should not rely on this check themselves, the meta document states that they really should only care about the isHit() method.

On Tuesday, August 11, 2015 at 7:34:30 AM UTC+2, Larry Garfield wrote:

Robert Hafner

unread,
Aug 16, 2015, 4:24:06 AM8/16/15
to php...@googlegroups.com
Can we bring this to a vote, or at least have an informal poll? I am of the mind that returning null would be just fine. Regardless, we have some options now and it would be good to see what people favored.

With regards to the “current time’ issue, just set it to “any time in the past” and we’re good to go on that.

Rob

Matteo Beccati

unread,
Aug 16, 2015, 4:42:36 AM8/16/15
to php...@googlegroups.com
Hi Robert,

On 16/08/2015 10:23, Robert Hafner wrote:
> Can we bring this to a vote, or at least have an informal poll? I am of the mind that returning null would be just fine. Regardless, we have some options now and it would be good to see what people favored.

before starting a poll, it would be good to understand what are the
actual use cases to have a getExpiration method at all. Can you come up
with any?

Robert Hafner

unread,
Aug 16, 2015, 4:48:29 AM8/16/15
to php...@googlegroups.com
It was added to Stash when someone requested it for use inside of a session handling framework. That is literally the only time it has ever been requested in stash, and I added it in because it was trivial. Stash returns false for items without an expiration.

Keep in mind I barely care about this function’s existence, so if you’re looking for someone to justify it I am not that person. I am merely trying to finish up a conversation so we can continue with the review. From my understanding there are these options-

* Return null,

* Return false,

* Return a far future date,

* Remove getExpiration.


We’ve had several years to argue about this and I don’t think much new is going to come up. I officially support returning null or false over a far future date, but as I said I don’t think this matters nearly enough and honestly I assumed when I pointed out the flaw that we’d simply add the additional return value and move on.


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/55D04CEC.8060306%40beccati.com.

Beau Simensen

unread,
Aug 16, 2015, 5:55:04 PM8/16/15
to PHP Framework Interoperability Group
On Sunday, August 16, 2015 at 3:48:29 AM UTC-5, Robert Hafner wrote:
It was added to Stash when someone requested it for use inside of a session handling framework. That is literally the only time it has ever been requested in stash, and I added it in because it was trivial. Stash returns false for items without an expiration.

Keep in mind I barely care about this function’s existence, so if you’re looking for someone to justify it I am not that person. I am merely trying to finish up a conversation so we can continue with the review. From my understanding there are these options-

* Return null,
* Return false,
* Return a far future date,
 
I believe that all of these things would probably be safe to handle during review as it doesn't change the interface but clarifies the return value under certain conditions. This isn't entirely true as the interface would actually have |false removed as a possible return value (and possibly have |null added instead) but I think that I'd be fine with that in review unless I hear major objections from people.

* Remove getExpiration.

I believe this option would require us to leave review as it is an actual change to the interface.

I think Matteo's case that expireAt(null) -> is_null(getExpiration()) is probably the most compelling case for using null over all of these options as far as consistency goes. Do Larry, Paul, or Rob see any holes in this logic? I know that Larry does not like multiple return types/null but as this method already seems to be in edge-case territory I'm inclined to say that we should just roll w/ null. If we can't get agreement between the three of you I think we should then put it up for an informal poll and give it a week to see how people feel about it.

In my reading of expiresAt, it makes the null case actually sound somewhat vague. It says that if null is passed (the typehint for the param doesn't include |null, btw) a default value MAY be used. Which I guess means that null may or may not be an expected return value for getExpiration. What does null mean on expiresAt if no default value exists? Is calling expiresAt(null) the same as not calling it at all, in which case it should be cached permanently? ...or?

If we end up agreeing that null means forever, and getExpiration can reasonably be assumed to return null and that null should mean forever (I think we should be explicit about that) then we are good to go. If not, I'll put a poll up to see how the member projects thing we should roll on this one given the options listed here.

Robert Hafner

unread,
Aug 16, 2015, 6:47:00 PM8/16/15
to php...@googlegroups.com
I agree with everything below, and think we should move forward with the null return.


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.

André R.

unread,
Aug 17, 2015, 9:26:28 AM8/17/15
to PHP Framework Interoperability Group
Hi,


On Sunday, August 16, 2015 at 10:48:29 AM UTC+2, Robert Hafner wrote:
It was added to Stash when someone requested it for use inside of a session handling framework. That is literally the only time it has ever been requested in stash, and I added it in because it was trivial. Stash returns false for items without an expiration.


It seem to me that this method should not be part of the interface as it is not within the main cache use case to know the exact date, and something we can deal with in a future spec for bridging expiry info to session cookies.
So better to remove it then ending up returning by design wrong values just because we want to cleanup return types.

If there are use cases for knowing if it has expired, then a isExpired()  is much more clear from api, usability and implementation perspective.


That said:
 

Keep in mind I barely care about this function’s existence, so if you’re looking for someone to justify it I am not that person. I am merely trying to finish up a conversation so we can continue with the review. From my understanding there are these options-

* Return null,


If you all really want to rush this now, this is the second best imho. null is universally understood as "no value" in php, which is what we are discussing here.

 

* Return false,

* Return a far future date,


This is rather sub optimal solution, agree with the wish to have only one return type, but then the correct means to fix that is either removal or change to isExpired where we don't need to have this discussion. It's straight forward, or? :)

Robert Hafner

unread,
Aug 17, 2015, 12:48:58 PM8/17/15
to php...@googlegroups.com

So better to remove it then ending up returning by design wrong values just because we want to cleanup return types.

I disagree with your assessment that we’d be returning the “wrong” value by design. If there is no expiration then null is a complete valid response.


If you all really want to rush this now, 

Seriously? If you think four years is rushing then I’d really hate to see what you’d consider a more relaxed approach. 


Rob




Dracony

unread,
Aug 17, 2015, 5:36:46 PM8/17/15
to PHP Framework Interoperability Group
I sugges we use Occam's razor on the idea and leave the method out if there is no common use case for it. As was already noted only a single person ever requested that feature in Stash, so to me that states it's just needless fat.

If the method turns out required it can be easily added with a subsequent PSR. 


On Tuesday, August 11, 2015 at 7:34:30 AM UTC+2, Larry Garfield wrote:

Paul Dragoonis

unread,
Aug 18, 2015, 12:51:42 AM8/18/15
to php...@googlegroups.com
+1 for removal

--
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.

Matteo Beccati

unread,
Aug 18, 2015, 2:01:30 AM8/18/15
to php...@googlegroups.com
On 18/08/2015 06:51, Paul Dragoonis wrote:
> +1 for removal

+1 for removing getExpiration

Korvin Szanto

unread,
Aug 18, 2015, 1:25:30 PM8/18/15
to php...@googlegroups.com
I'm with you guys, +1 removal 

+0.5 add `isExpired`.

--
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.

Beau Simensen

unread,
Aug 18, 2015, 1:46:07 PM8/18/15
to PHP Framework Interoperability Group
Larry, where are you at on this one?

Robert, if we lean in the direction of removing getExpiration entirely would you be fine with that?

Right now I'm getting a sense that the majority of people in this thread are interested in removing getExpiration entirely. I'm fine with that or with returning null. Would like to get one more round of feedback from Larry & Robert before making a decision on this.

guilher...@gmail.com

unread,
Aug 18, 2015, 2:08:40 PM8/18/15
to php...@googlegroups.com
+1 for removing

--
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.

For more options, visit https://groups.google.com/d/optout.



--
Guilherme Blanco
MSN: guilher...@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada

Larry Garfield

unread,
Aug 18, 2015, 2:20:02 PM8/18/15
to php...@googlegroups.com
On 8/18/15 12:46 PM, Beau Simensen wrote:
> Larry, where are you at on this one?
>
> Robert, if we lean in the direction of removing getExpiration entirely
> would you be fine with that?
>
> Right now I'm getting a sense that the majority of people /in this
> thread/ are interested in removing getExpiration entirely. I'm fine
> with that or with returning null. Would like to get one more round of
> feedback from Larry & Robert before making a decision on this.

My only concern is making sure that there's some other way to implement
rewarm-in-advance. (Eg, refresh anything that's going to expire within
the next X time period.) As long as we're reasonably confident that
removing getExpiration() won't block or hinder that use case, I'm OK
with it if that's the consensus.

--
--Larry Garfield

Robert Hafner

unread,
Aug 18, 2015, 2:30:53 PM8/18/15
to php...@googlegroups.com

I am 100% okay with this. As I mentioned, it was years before Stash ever received a request for this so I don’t think it’s critical for a caching library to have (and thus to be a requirement). If it gets added to a future optional interface that would be cool, but this is not something I care strongly about.

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.

Larry Garfield

unread,
Aug 18, 2015, 2:41:18 PM8/18/15
to php...@googlegroups.com
You know, thinking on this a bit more, getExpiration() on its own isn't
enough for that anyway. You would still need to either iterate all items
in the pool (which we don't support anyway) in order to check each one
for an upcoming expiration, or you'd have to have some kind of
query-by-expiration feature (SELECT * WHERE expiration < now() + 5
minutes), which we also don't support. So getExpiration() is already
not useful for that use case.

+1 to just removing it outright.

--
--Larry Garfield

Beau Simensen

unread,
Aug 18, 2015, 2:47:44 PM8/18/15
to PHP Framework Interoperability Group
We have agreement between Larry, Paul, and Rob, so my recommendation is to formulate a PR. Once it has been reviewed and agreed upon by the interested parties, I'll cancel the review, merge the PR, and then put it back up for review again. 

Larry Garfield

unread,
Aug 18, 2015, 5:46:13 PM8/18/15
to php...@googlegroups.com
Is this change large enough to warrant resetting the Review period?

--
--Larry Garfield

Robert Hafner

unread,
Aug 18, 2015, 6:19:07 PM8/18/15
to php...@googlegroups.com
I would say so.

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/55D3A79E.2040004%40garfieldtech.com.

Beau Simensen

unread,
Aug 18, 2015, 7:04:31 PM8/18/15
to PHP Framework Interoperability Group
Is this change large enough to warrant resetting the Review period?

Any of the other options I was willing to consider doing in Review as they were maybe considered clarifications even though the interface was technically changing. In this case, though, we are removing a method entirely. That seems like a substantial change that should require going back to draft.

I don't think there are any other major discussions going on around cache (if there are, let me know as they've slipped under the radar for me) so if this is all we have to deal with then I think we're actually sitting in a pretty good place.

Larry Garfield

unread,
Aug 18, 2015, 7:47:55 PM8/18/15
to php...@googlegroups.com
On 8/18/15 6:04 PM, Beau Simensen wrote:
>
> Is this change large enough to warrant resetting the Review period?
>
>
> Any of the other options I was willing to consider doing in Review as
> they were maybe considered clarifications even though the interface
> was technically changing. In this case, though, we are removing a
> method entirely. That seems like a substantial change that should
> require going back to draft.
>
> I don't /think/ there are any other major discussions going on around
> cache (if there are, let me know as they've slipped under the radar
> for me) so if this is all we have to deal with then I think we're
> actually sitting in a pretty good place.

I believe the only other outstanding concerns were Guilherme's, which
would be far more invasive changes. It didn't sound like there was much
support for them.

If there was anything else, I missed it, too.

--
--Larry Garfield

André R.

unread,
Aug 19, 2015, 2:12:17 AM8/19/15
to PHP Framework Interoperability Group


On Monday, August 17, 2015 at 6:48:58 PM UTC+2, Robert Hafner wrote:

So better to remove it then ending up returning by design wrong values just because we want to cleanup return types.

I disagree with your assessment that we’d be returning the “wrong” value by design. If there is no expiration then null is a complete valid response.



Context: Was referring to returning fake dates, as I said further down null is what is commonly used for missing values, but as I also see Larry's point I argued primarily for removal/renaming to isExpiry.
 



If you all really want to rush this now, 

Seriously? If you think four years is rushing then I'd really hate to see what you'd consider a more relaxed approach. 


Context: the difference between getting this accepted in september or october does not matter imo.


Larry Garfield

unread,
Aug 20, 2015, 9:22:44 PM8/20/15
to php...@googlegroups.com
Two pull requests have been filed:

https://github.com/php-fig/fig-standards/pull/604 (getExpiration)
https://github.com/php-fig/fig-standards/pull/605 (exceptions)

--Larry Garfield

Larry Garfield

unread,
Aug 26, 2015, 3:01:56 PM8/26/15
to php...@googlegroups.com
Robert and Paul, I believe Beau is waiting on your feedback on these PRs
before continuing.

--Larry Garfield

Nicholas Ruunu

unread,
Feb 2, 2016, 8:18:18 PM2/2/16
to PHP Framework Interoperability Group
Since you removed getExpiration() in the PR, how do you figure you'd save(CacheItemInterface) with an expiration?

Andrew Carter

unread,
Feb 4, 2016, 4:49:15 AM2/4/16
to PHP Framework Interoperability Group
Hi Nicholas,

There is a solution that prevents you from having to supplement the interface. It isn't elegant, but I believe it's cleaner than adding a public method that you don't actually want people to be using!

The solution is using a key value store that the pool shares with the item upon construction. PSR-6 states that cache items can only be saved to the cache pool that created them, so this is acceptable.

The important thing is to make sure that when the item falls out of scope (__destruct) any entries in this key value store are erased, otherwise you will have a memory leak. In the proof of concept, I have used item cloning to test that the entry in the key value store is erased, but in reality you would want to disable item cloning.

http://pastebin.com/JnG1dxKy
https://3v4l.org/XWtYg

Regards,

Andrew Carter
Reply all
Reply to author
Forward
0 new messages