PSR Cache Proposal #96

Closed
wants to merge 52 commits into
from

Conversation

Projects
None yet
Member

dragoonis commented Mar 12, 2013

Quick Link: https://github.com/dragoonis/fig-standards/blob/master/proposed/psr-cache.md

note: the examples section in psr-cache.md will not be part of the official proposal, it is just here so that everything is under the one hood and to prove that the interfaces are solid.

@evert evert referenced this pull request Mar 12, 2013

Closed

Proposal for an object cache #11

Contributor

bobthecow commented Mar 12, 2013

This could probably use a rebase?

+ *
+ * @return CacheItemInterface The newly populated CacheItem class representing the stored data in the cache
+ */
+ public function get($key);
@staabm

staabm Mar 16, 2013

What do you think about a lazy method, which takes a $key and a Closure which would be invoked when no cached value is found.
This simplifies client code to something like

$cachedValue = $cache->lazy($key, function() {
  // impl whatever should be done on cache miss
  // the returned value will be set into the CacheItem
});

This also reduces the need for the always repeated cache pattern

$item = $cache->get($key);
If(!$item->isHit()) {
  // do stuff
  $item->set($key, $value);
} else {
  $value = $item->getValue();
}
@staabm

staabm Mar 16, 2013

So the implementation of the lazy method would look more or less like the pattern which is otherwise used all over the applications

@dragoonis

dragoonis Mar 16, 2013

Member

@staab please bring up these discussions on our mailing list. https://groups.google.com/d/msg/php-fig/UkSWS48eEgo/cbchuV0mlEgJ

Contributor

AmyStephen commented Apr 13, 2013

Thinking that returning a $this for set, remove, getMultiple, removeMultiple, and clear is better than a true or false in that if the method fails, it should throw an exception and, for that reason, there will never be a false.

Contributor

ashnazg commented Apr 13, 2013

That's a really good observation... I'd +1 that suggestion.

Contributor

Crell commented Apr 14, 2013

I think Amy has been reading my blog... ;-)

Contributor

AmyStephen commented Apr 14, 2013

lol, that is true @Crell -- and I am complying, /me goes to check her temperature.

Contributor

AmyStephen commented Apr 14, 2013

(That comment was a reflection of my stubborn nature, nothing else!)

👍 for cache psr

Member

dragoonis commented May 2, 2013

But set returns a boolean to tell you if the se was successful or not.

Is this not important to people? A fluent interface is fun and shiny, but
is it more critical to have than a success boolean?

On Wed, May 1, 2013 at 9:40 PM, Matthew Ratzke notifications@github.comwrote:

[image: 👍] for cache psr


Reply to this email directly or view it on GitHubhttps://github.com/php-fig/fig-standards/pull/96#issuecomment-17305114
.

Contributor

AmyStephen commented May 2, 2013

@dragoonis Please read my comment. It's pretty short. ;-) #96 (comment)

@jfsimon jfsimon referenced this pull request in symfony/symfony May 11, 2013

Closed

[POC] Cache component #7986

14 of 18 tasks complete

jfsimon commented May 11, 2013

👍 for the CacheInterface & CacheItemInterface and 👍 for @AmyStephen's suggestion about exceptions.
What about replacing isHit() by isCached()?

Contributor

philsturgeon commented May 11, 2013

Is cached sounds vague, it implies it was cached at some point ever and not "is good".

But I'm drunk, so my feedback might not be hugely helpful at this point.

Sent from my iPhone

On May 11, 2013, at 4:42 PM, Jean-François Simon notifications@github.com wrote:

for the CacheInterface & CacheItemInterface and for @AmyStephen's suggestion about exceptions.
What about replacing isHit() by isCached()?


Reply to this email directly or view it on GitHub.

jfsimon commented May 11, 2013

@philsturgeon indeed, isHit() is maybe more understable. Have a good night ;)

Contributor

philsturgeon commented May 11, 2013

I have to get through the afternoon before I worry about the night! >.<

Sent from my iPhone

On May 11, 2013, at 4:55 PM, Jean-François Simon notifications@github.com wrote:

@philsturgeon indeed, isHit() is maybe more understable. Have a good night ;)


Reply to this email directly or view it on GitHub.

proposed/psr-cache.md
+ /**
+ * This will wipe out the entire cache's keys
+ *
+ * @return boolean The result of the clear operation
@staabm

staabm May 12, 2013

Should this be more explicit? Somethink like Returns true on success and otherwise false?

@philsturgeon

philsturgeon May 12, 2013

Contributor

Agreed.

@staabm

staabm May 12, 2013

Same for set, setMultiple, remove

proposed/psr-cache.md
+ * @param null|integer $ttl Optional. The TTL value of this item. If no value is sent and the driver supports TTL
+ * then the library may set a default value for it or let the driver take care of that.
+ *
+ * @return boolean
+ *
+ * @param array $keys A list of keys that can obtained in a single operation.
+ *
+ * @return array An array of CacheItem classes.
@staabm

staabm May 12, 2013

Type should be CacheItemInterface[]

+ /**
+ * Persisting a set of key => value pairs in the cache, with an optional TTL.
+ *
+ * @param array $items An array of key => value pairs for a multiple-set operation.
@staabm

staabm May 12, 2013

Should this also support CacheItemInterface[]?

+ * Sets a cache instance on the object
+ *
+ * @param CacheInterface $cache
+ * @return null
@staabm

staabm May 12, 2013

Should this setter support a fluid interface?

empire commented May 12, 2013

What about adding hasKey (or isKeyCached or isCached) to CacheInterfae for checking existence of key in cache?
How can we check some key in cache or not!? We must first call get and then based on result we can guess the cache exist.

  if ($item = $cache->get('some-key')) {
    $cache->remove('some-key');
  }

OR

  if ($cache->has('some-key')) {
    $cache->remove('some-key');
  }

empire commented May 12, 2013

removeMultiple can be implemented by remove, so, some implementations need to do it, adding AbstractCache will be useful for implementing some methods like removeMultiple.

+ * @param string $key The key of the item to store
+ * @param mixed $value The value of the item to store
+ * @param null|integer $ttl Optional. The TTL value of this item. If no value is sent and the driver supports TTL
+ * then the library may set a default value for it or let the driver take care of that.
@jfsimon

jfsimon May 14, 2013

Maybe you shoud specify the unit for $ttl here.

+ *
+ * @return array An array of 'key' => result, elements. Each array row has the key being deleted
+ * and the result of that operation. The result will be a boolean of true or false
+ * representing if the cache item was removed or not
@AurelC2G

AurelC2G May 14, 2013

Not really clear: what would be returned if the key was not in cache? false because it could not be removed? true because after the call to remove(), the key is still not in cache?
Same for remove().

@dragoonis

dragoonis May 24, 2013

Member

If you were setting foo, bar, baz. then the result would be something like this

array(
    'foo' => true,
    'bar' => false,
    'baz' => true
);
@AurelC2G

AurelC2G May 24, 2013

In this case it is remove, not set. What is returned if you try to remove an item that is not in cache? The spec is not clear about this.

One question I have looking at the proposal is why increment() has been omitted when a number of caching engines provide native support for incrementing a value?

If increment() isn't part of the standard, what should be the de facto way of mimicking increment functionality when consuming a library that implements this standard?

Contributor

AmyStephen commented May 18, 2013

Your adapter can add functionality, the interfaces are a minimum.

On Sat, May 18, 2013 at 11:05 AM, Noah Goodrich notifications@github.comwrote:

One question I have looking at the proposal is why increment() has been
omitted when a number of caching engines provide native support for
incrementing a value?

If increment() isn't part of the standard, what should be the de facto way
of mimicking increment functionality when consuming a library that
implements this standard?


Reply to this email directly or view it on GitHubhttps://github.com/php-fig/fig-standards/pull/96#issuecomment-18103545
.

Contributor

bobthecow commented May 18, 2013

What Amy said. And if it turns out that a lot of libraries implement increment() in addition to the PSR Cache interface, then a new Cache PSR can extend the current one by adding increment() (and whatever other commonalities emerge).

Contributor

philsturgeon commented May 18, 2013

Paul D has mentioned interest in making a basic cache proposal then making a more advanced one later on. Justin's got it right, whatever turns up in these super-set implementations would very likely become specced in the next PSR.

Phil Sturgeon

On Saturday, May 18, 2013 at 1:19 PM, Justin Hileman wrote:

What Amy said. And if it turns out that a lot of libraries implement increment() in addition to the PSR Cache interface, then a new Cache PSR can extend the current one by adding increment() (and whatever other commonalities emerge).


Reply to this email directly or view it on GitHub (#96 (comment)).

@AmyStephen and @bobthecow

I maintain a stand-alone Data Mapper that utilizes the increment() functionality in memcache. I'm looking at the standard from the viewpoint of a library that will consume PSR\Cache implementations as drop-in replacements without requiring customization.

This standard doesn't do much good if everyone who uses my Data Mapper library has to customize their chosen Cache library. As such, I wonder what the standard will be for libraries that use a cache library implementing this standard if increment() is a required function?

Contributor

AmyStephen commented May 18, 2013

Remember, this is only an interface. Meaning, a developer has to create a
handler for each type of cache that is supported. The adapter is
instantiated and the selected handler injected via the constructor. So,
it's not possible to avoid a customized handler - that's the intention.

Now, in the case of an automatic key, curious, how does the cache get
retrieved? (How does the developer know what to use for a get key?) Reason
I ask is the only issue I can see that might be a problem is if sending in
a value for the key is impossible for the set.

If you can explain a bit on how the cache is retrieved, that would help.

On Sat, May 18, 2013 at 1:53 PM, Noah Goodrich notifications@github.comwrote:

@AmyStephen https://github.com/AmyStephen and @bobthecowhttps://github.com/bobthecow

I maintain a stand-alone Data Mapper that utilizes the increment()
functionality in memcache. I'm looking at the standard from the viewpoint
of a library that will consume PSR\Cache implementations as drop-in
replacements without requiring customization.

This standard doesn't do much good if everyone who uses my Data Mapper
library has to customize their chosen Cache library. As such, I wonder what
the standard will be for libraries that use a cache library implementing
this standard if increment() is a required function?


Reply to this email directly or view it on GitHubhttps://github.com/php-fig/fig-standards/pull/96#issuecomment-18106147
.

Contributor

AmyStephen commented May 18, 2013

By the way, interested in seeing your solution. Been thinking more and more
we are heading towards Data Mappers sitting in front of a lot of this.

On Sat, May 18, 2013 at 2:41 PM, Amy Stephen amystephen@gmail.com wrote:

Remember, this is only an interface. Meaning, a developer has to create a
handler for each type of cache that is supported. The adapter is
instantiated and the selected handler injected via the constructor. So,
it's not possible to avoid a customized handler - that's the intention.

Now, in the case of an automatic key, curious, how does the cache get
retrieved? (How does the developer know what to use for a get key?) Reason
I ask is the only issue I can see that might be a problem is if sending in
a value for the key is impossible for the set.

If you can explain a bit on how the cache is retrieved, that would help.

On Sat, May 18, 2013 at 1:53 PM, Noah Goodrich notifications@github.comwrote:

@AmyStephen https://github.com/AmyStephen and @bobthecowhttps://github.com/bobthecow

I maintain a stand-alone Data Mapper that utilizes the increment()
functionality in memcache. I'm looking at the standard from the viewpoint
of a library that will consume PSR\Cache implementations as drop-in
replacements without requiring customization.

This standard doesn't do much good if everyone who uses my Data Mapper
library has to customize their chosen Cache library. As such, I wonder what
the standard will be for libraries that use a cache library implementing
this standard if increment() is a required function?


Reply to this email directly or view it on GitHubhttps://github.com/php-fig/fig-standards/pull/96#issuecomment-18106147
.

@AmyStephen

https://github.com/energylab/gacela/blob/master/library/Gacela/Gacela.php

The important bits are the enableCache(), _cache() and incrementDataCache() methods.

Originally this was all written to just use the Memcache library, but I'm looking at this PSR as a way to allow someone using Gacela as their Data Mapper to drop in any Cache library and just run with it.

Right now if you look at my docs, the requirements for enabling a cache are as follows:

Gacela will use any caching library that supports get(), set(), and increment()

With this PSR as an available standard, my requirement would become any caching library that supports the PSR-Cache standard. Make sense?

As for the keys, the singleton cache instance is used in the DataSource\Adapters to store metadata for the specific data source. Or in the Mapper\Mapper class if caching is enabled for a concrete mapper class. In both cases the keys are basically hashes implemented within the context of the framework and the developer never interacts directly with the cache.

Contributor

AmyStephen commented May 18, 2013

@noah-goodrich As an FYI, if you want to look at one, here's my Cache Adapter/Handlers with the draft PSR Interfaces https://github.com/Molajo/cache - you are right, if increment is a critical feature of your application, the PSR would not be enough.

You probably should request the addition in this thread https://groups.google.com/forum/#!topic/php-fig/lANQM7LPX_M - Although I'm not sure if that will help or not, the project is in a weird spot.

If it doesn't get included, you can work with developer(s) providing the library, ask them to add increment, I wouldn't have a problem adding that if it helps. Or you can fork their library, do it yourself and include it. We're going to have to collaborate while these PSR's get rolling.

jfsimon commented May 19, 2013

I think increment() should be part of the interface too.

If I build a library based on the cache and I use PSR/Cache, I will only rely on the interface for interoperability. If I have to increment/decrement interger value I will have to reimplment these methods with get() + set(). It's a performance issue (2 requests instead of 1). Caching is all about performance.

Of course, interface is the "minimal contract" of the backend which could implement these methods, but if I have to check if backend class supports or not incrementing, the interface does not fulfill its role.

Furthemore, calling increment() with a negative argument looks really weird to me, so I think decrement() should be part of the interface too.

Contributor

Crell commented May 19, 2013

Once again, this discussion belongs on the FIG list, not in a GitHub thread where most people won't see it. We're discussing the cache PSR there now anyway, so this is the time to bring it up.

jfsimon commented May 19, 2013

@Crell sorry.

empire commented May 19, 2013

@AmyStephen , @jfsimon @noah-goodrich I think we can have some other interfaces for making increment happen, some thing like following

inteface IncrementableItemInteface 
{
    public function increment($value = 1);
}

And then, for some cache engine, we can have CacheItem class that implements IncrementableItemInteface, we can check just by instance-of

   public function workOnSomeItem(CacheItem $cacheItem)
   {
       if ($cacheItem instanceof IncrementableItemInteface) {
             $cacheItem->increment(123);
       }
    }

jfsimon commented May 19, 2013

@empire you should publish your response on the mailing list: https://groups.google.com/forum/#!topic/php-fig/lANQM7LPX_M ;)

empire commented May 19, 2013

@jfsimon thanks, I post it.
In hope that will be useful.

+
+use Psr\Cache\ItemInterface;
+
+interface CacheInterface
@moisadoru

moisadoru Jun 2, 2013

I think a better suited name for this interface would be Psr\Cache\SingleAccessStore (or something similar), to clearly denote the fact that you can only get/set single items.

+
+```
+
+### 2.3 MultipleInterface
@moisadoru

moisadoru Jun 2, 2013

I think a better suited name for this interface would be Psr\Cache\MultipleAccessStore (or something similar), to clearly denote the fact that you can access multiple items in a single call.

+ *
+ * @return boolean The result of the multiple-set operation
+ */
+ public function setMultiple($items, $ttl = null);
@moisadoru

moisadoru Jun 2, 2013

How does one set individual TTLs for $items ?

+
+### 2.4 IncrementableInterface
+
+This interface provides the ability to increment and decrement cache entries by their specified value. Some cache backends support this natively so that you don't have to read the item and then increment it and write it back to the cache server, this can be done in a single call to the cache server since it's natively supported by many modern cache servers.
@moisadoru

moisadoru Jun 2, 2013

What is the behavior when trying to increment/decrement an entry that is not numeric?
Ex:

$cache->set('foo', 'bar');
// ... later in the code:
$cache->increment('foo', 1);
// or
$cache->decrement('foo', 1);
Member

dragoonis commented Jun 25, 2013

Closing this PR.

@dragoonis dragoonis closed this Jun 25, 2013

Just out of curiosity any reason why this was closed? I personally think this would be a useful standard.

Contributor

philsturgeon commented Jul 31, 2013

This specific PR has been closed to make way for #149

Contributor

dave1010 commented on 1453a9c Nov 6, 2013

This gist looks like APC, not Memcached. Do you have a Memcached version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment