prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
108 stars 82 forks source link

PSR-6 ? #117

Closed nanocom closed 1 month ago

nanocom commented 8 years ago

Are there any plans to replace default APC cache with PSR-6 soon?

erwan commented 8 years ago

I didn't know about PSR-6 but it looks like a good idea indeed!

I'll take a look at it.

phillipsnick commented 6 years ago

Shame this is missing, I quickly threw together this to use PSR6 caching.

class Cache implements CacheInterface
{
    /** @var CacheItemPoolInterface */
    private $cache;

    public function __construct(CacheItemPoolInterface $cache)
    {
        $this->cache = $cache;
    }

    public function has($key): bool
    {
        return $this->cache->getItem($key)->isHit();
    }

    public function get($key)
    {
        return $this->cache->getItem($key)->get();
    }

    public function set($key, $value, $ttl = 0): void
    {
        $item = $this->cache->getItem($key);
        $item
            ->set($value)
            ->expiresAfter($ttl);

        $this->cache->save($item);
    }

    public function delete($key): void
    {
        $this->cache->deleteItem($key);
    }

    public function clear(): void
    {
        // lets not do that
        return;
    }
}
c0nst4ntin commented 1 year ago

@nanocom @erwan @phillipsnick Are you still interested in pursuing this topic? I can see it's been some time, but I am slowly trying to get this Repository back on track!

If you are still interested, we could throw together another Driver implementing the CacheInterface. This could then be used instead of APCu by passing it to the constructor of the API Class.

We could also think about adding it as a fallback into the defaultCache() function

What do you guys think?

gsteel commented 1 year ago

It would be better to remove the CacheInterface all together, replacing it with Psr\Cache\CacheItemPoolInterface - at least then, users would be able to pass in whatever cache implementation their app already has hanging around.

c0nst4ntin commented 1 year ago

@gsteel I get where you are coming from, but with the implementation of @phillipsnick it would be possible to keep the old legacy code and still pass a Psr\Cache\CacheItemPoolInterface trough the Constructor. By keeping the Packages CacheInterface you could theoretically pass any custom implementation into this package.

We could also mark the APCu cache as deprecated and remove it in a major version 🤔

gsteel commented 1 year ago

You could also remove the entire custom cache implementation stuff in a major, obviously, you should release a minor with @\deprecated in the relevant places. IMO it's a lot easier for users to "bring their own cache" and the whole point is that PSR-6 makes that trivial… Users can choose Symfony, Laminas and other well known cache implementations on packagist - it's likely that their project already has a cache dependency installed:

https://packagist.org/providers/psr/cache-implementation

It's not like you'd be adding a dependency on a vendor package - the standard is fine and well adopted as far as I can tell.

It's how I did it here but I went the extra mile using PSRs for the underlying HTTP client and HTTP related factories - doing that extra stuff for HTTP means you don't have to keep up with vendor package versions as the PSRs stay stable for much longer

c0nst4ntin commented 1 year ago

I get your point. And this will definitely be something I'll consider, once I move towards thinking about a new major version.

But with the Implementation mentioned above, would allow everybody to pass their own PSR6 CacheItemPoolInterface implementation, or am I reading this wrong?

gsteel commented 1 year ago

@c0nst4ntin - I don't think you're reading the authors suggestion wrong - it's a straight forward feature addition; Create as many cache adapters as you like that satisfy the shipped interface.

The reason I talk about making PSR-6 the dependency, is because why is a straight forward http api client like this reinventing a cache adapter implementation when PSR-6 does this extremely well and gives consumers huge flexibility?

I personally think your focus should be on dropping PrismicCache so that you don't have to maintain it or any of the adapters.

Testing cache integration suddenly becomes really easy - all you need is an in-memory implementation in your dev-deps like Symfony's ArrayAdapter or something.