Skip to content

Commit

Permalink
[Cache] Symfony PSR-6 implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Jan 19, 2016
1 parent a4f3baa commit 91e482a
Show file tree
Hide file tree
Showing 21 changed files with 1,138 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .travis.php
Expand Up @@ -39,7 +39,7 @@

$packages[$package->name][$package->version] = $package;

$versions = file_get_contents('https://packagist.org/packages/'.$package->name.'.json');
$versions = @file_get_contents('https://packagist.org/packages/'.$package->name.'.json') ?: '{"package":{"versions":[]}}';

This comment has been minimized.

Copy link
@k00ni

k00ni Jan 25, 2016

Isn't using @ with a function problematic, in case there is a critical error, which you suppress with using @?

This comment has been minimized.

Copy link
@wouterj

wouterj Jan 26, 2016

Member

There can only be errors when the URL isn't accessible, in which case the right part of the ?: operator will be executed.

$versions = json_decode($versions);

foreach ($versions->package->versions as $version => $package) {
Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -51,7 +51,7 @@ install:
- if [[ $deps != skip ]]; then COMPONENTS=$(find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n'); fi;
- if [[ $deps != skip && $deps ]]; then php .travis.php $TRAVIS_COMMIT_RANGE $TRAVIS_BRANCH $COMPONENTS; fi;
- if [[ $deps = high && $TRAVIS_BRANCH = master ]]; then SYMFONY_VERSION=$(git ls-remote --heads | grep -o '/[1-9].*' | tail -n 1 | sed s/.//); else SYMFONY_VERSION=$(cat composer.json | grep '^ *"dev-master". *"[1-9]' | grep -o '[0-9.]*'); fi;
- if [[ $deps = high && $TRAVIS_BRANCH = master ]]; then git fetch origin $SYMFONY_VERSION; git checkout -m FETCH_HEAD; fi;
- if [[ $deps = high && $TRAVIS_BRANCH = master ]]; then git fetch origin $SYMFONY_VERSION; git checkout -m FETCH_HEAD; COMPONENTS=$(find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n'); fi;
- if [[ $deps = high && ${SYMFONY_VERSION%.*} != $(git show $(git ls-remote --heads | grep -FA1 /$SYMFONY_VERSION | tail -n 1):composer.json | grep '^ *"dev-master". *"[1-9]' | grep -o '[0-9]*' | head -n 1) ]]; then LEGACY=,legacy; fi;
- export COMPOSER_ROOT_VERSION=$SYMFONY_VERSION.x-dev;
- if [[ ! $deps ]]; then composer update --prefer-dist; else export SYMFONY_DEPRECATIONS_HELPER=weak; fi;
Expand Down
4 changes: 4 additions & 0 deletions composer.json
Expand Up @@ -19,6 +19,7 @@
"php": ">=5.5.9",
"doctrine/common": "~2.4",
"twig/twig": "~1.23|~2.0",
"psr/cache": "~1.0",
"psr/log": "~1.0",
"symfony/polyfill-apcu": "~1.0,>=1.0.2",
"symfony/polyfill-intl-icu": "~1.0",
Expand All @@ -30,6 +31,7 @@
"replace": {
"symfony/asset": "self.version",
"symfony/browser-kit": "self.version",
"symfony/cache": "self.version",
"symfony/class-loader": "self.version",
"symfony/config": "self.version",
"symfony/console": "self.version",
Expand Down Expand Up @@ -74,6 +76,8 @@
"symfony/yaml": "self.version"
},
"require-dev": {
"cache/integration-tests": "^0.6",
"doctrine/cache": "~1.6",
"doctrine/data-fixtures": "1.0.*",
"doctrine/dbal": "~2.4",
"doctrine/orm": "~2.4,>=2.4.5",
Expand Down
2 changes: 2 additions & 0 deletions phpunit.xml.dist
Expand Up @@ -54,6 +54,8 @@
<listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener">
<arguments>
<array>
<element><string>Cache\IntegrationTests</string></element>
<element><string>Doctrine\Common\Cache</string></element>
<element><string>Symfony\Component\HttpFoundation</string></element>
</array>
</arguments>
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Cache/.gitignore
@@ -0,0 +1,3 @@
composer.lock
phpunit.xml
vendor/
292 changes: 292 additions & 0 deletions src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
@@ -0,0 +1,292 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Cache\Adapter;

use Psr\Cache\CacheItemInterface;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\CacheItem;
use Symfony\Component\Cache\Exception\InvalidArgumentException;

/**
* @author Nicolas Grekas <p@tchwork.com>
*/
abstract class AbstractAdapter implements CacheItemPoolInterface
{
private $namespace;
private $deferred = array();
private $createCacheItem;
private $mergeByLifetime;

protected function __construct($namespace = '', $defaultLifetime = 0)
{
$this->namespace = $namespace;
$this->createCacheItem = \Closure::bind(

This comment has been minimized.

Copy link
@lastzero

lastzero Feb 17, 2017

I'm trying to understand your code. Is this closure just to get access to the protected methods of CacheItem? Why not passing all values to the constructor to make them immutable and avoid setters, if that's the point? And since the cache item class is final, the class properties are in fact private, right?

/**
 * @author Nicolas Grekas <p@tchwork.com>
 */
final class CacheItem implements CacheItemInterface
{
    protected $key;
    protected $value;
    protected $isHit;
    protected $expiry;
    protected $defaultLifetime;
    protected $tags = array();
    protected $innerItem;
    protected $poolHash;

This comment has been minimized.

Copy link
@stof

stof Feb 17, 2017

Member

@lastzero we don't want to expose the initialization of these properties outside the cache pool. Instantiating a CacheItem yourselves is not a supported usage of PSR-6, and we want to enforce that.

Anyway, commenting on old commits is probably the worse way to get an answer: if we miss the email notification, we have no way to find your question in another way (we never go back in the github history to check all commits to see whether they have new comments).
for such question, asking on Slack is much better.

And using protected rather than private is to simplify some internal implementation details due to PHP features being used. It started as private.

This comment has been minimized.

Copy link
@lastzero

lastzero Feb 17, 2017

See, commenting on GitHub works :) I always read my GitHub mails. Thanks for sharing the insights!

PS: PHPStorm is getting confused when parsing your code. It marks it as error, this is how it got my attention.

function ($key, $value, $isHit) use ($defaultLifetime) {
$item = new CacheItem();
$item->key = $key;
$item->value = $value;
$item->isHit = $isHit;
$item->defaultLifetime = $defaultLifetime;

return $item;
},
$this,
CacheItem::class
);
$this->mergeByLifetime = \Closure::bind(
function ($deferred, $namespace) {
$byLifetime = array();

foreach ($deferred as $key => $item) {
if (0 <= $item->lifetime) {
$byLifetime[(int) $item->lifetime][$namespace.$key] = $item->value;
}
}

return $byLifetime;
},
$this,
CacheItem::class
);
}

/**
* Fetches several cache items.
*
* @param array $ids The cache identifiers to fetch.
*
* @return array The corresponding values found in the cache.
*/
abstract protected function doFetch(array $ids);

/**
* Confirms if the cache contains specified cache item.
*
* @param string $id The identifier for which to check existence.
*
* @return bool True if item exists in the cache, false otherwise.
*/
abstract protected function doHave($id);

/**
* Deletes all items in the pool.
*
* @return bool True if the pool was successfully cleared, false otherwise.
*/
abstract protected function doClear();

/**
* Removes multiple items from the pool.
*
* @param array $ids An array of identifiers that should be removed from the pool.
*
* @return bool True if the items were successfully removed, false otherwise.
*/
abstract protected function doDelete(array $ids);

/**
* Persists several cache items immediately.
*
* @param array $values The values to cache, indexed by their cache identifier.
* @param int $lifetime The lifetime of the cached values, 0 for persisting until manual cleaning.
*
* @return array|bool The identifiers that failed to be cached or a boolean stating if caching succeeded or not.
*/
abstract protected function doSave(array $values, $lifetime);

/**
* {@inheritdoc}
*/
public function getItem($key)
{
$id = $this->getId($key);

if ($this->deferred) {
$this->commit();
}
if (isset($this->deferred[$key])) {
return $this->deferred[$key];
}

$f = $this->createCacheItem;
$isHit = false;
$value = null;

foreach ($this->doFetch(array($id)) as $value) {
$isHit = true;
}

return $f($key, $value, $isHit);
}

/**
* {@inheritdoc}
*/
public function getItems(array $keys = array())
{
if ($this->deferred) {
$this->commit();
}
$f = $this->createCacheItem;
$ids = array();
$items = array();

foreach ($keys as $key) {
$id = $this->getId($key);

if (isset($this->deferred[$key])) {
$items[$key] = $this->deferred[$key];
} else {
$ids[$key] = $id;
}
}

$values = $this->doFetch($ids);

foreach ($ids as $key => $id) {
$isHit = isset($values[$id]);
$items[$key] = $f($key, $isHit ? $values[$id] : null, $isHit);
}

return $items;
}

/**
* {@inheritdoc}
*/
public function hasItem($key)
{
if ($this->deferred) {
$this->commit();
}

return $this->doHave($this->getId($key));
}

/**
* {@inheritdoc}
*/
public function clear()
{
$this->deferred = array();

return $this->doClear();
}

/**
* {@inheritdoc}
*/
public function deleteItem($key)
{
return $this->deleteItems(array($key));
}

/**
* {@inheritdoc}
*/
public function deleteItems(array $keys)
{
$ids = array();

foreach ($keys as $key) {
$ids[] = $this->getId($key);
unset($this->deferred[$key]);
}

return $this->doDelete($ids);
}

/**
* {@inheritdoc}
*/
public function save(CacheItemInterface $item)
{
if (!$item instanceof CacheItem) {
return false;
}
$key = $item->getKey();
$this->deferred[$key] = $item;
$this->commit();

return !isset($this->deferred[$key]);
}

/**
* {@inheritdoc}
*/
public function saveDeferred(CacheItemInterface $item)
{
if (!$item instanceof CacheItem) {
return false;
}
try {
$item = clone $item;
} catch (\Error $e) {
} catch (\Exception $e) {
}
if (isset($e)) {
@trigger_error($e->__toString());

return false;
}
$this->deferred[$item->getKey()] = $item;

return true;
}

/**
* {@inheritdoc}
*/
public function commit()
{
$f = $this->mergeByLifetime;
$ko = array();
$namespaceLen = strlen($this->namespace);

foreach ($f($this->deferred, $this->namespace) as $lifetime => $values) {
if (true === $ok = $this->doSave($values, $lifetime)) {
continue;
}
if (false === $ok) {
$ok = array_keys($values);
}
foreach ($ok as $failedId) {
$key = substr($failedId, $namespaceLen);
$ko[$key] = $this->deferred[$key];
}
}

return !$this->deferred = $ko;
}

public function __destruct()
{
if ($this->deferred) {
$this->commit();
}
}

private function getId($key)
{
if (!is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given', is_object($key) ? get_class($key) : gettype($key)));
}
if (!isset($key[0])) {
throw new InvalidArgumentException('Cache key length must be greater than zero');
}
if (isset($key[strcspn($key, '{}()/\@:')])) {
throw new InvalidArgumentException('Cache key contains reserved characters {}()/\@:');
}

return $this->namespace.$key;
}
}

3 comments on commit 91e482a

@sebastianblum
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a plan to merge this feature into 2.8 and 3.0 or will it come with 3.1?

@wouterj
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebastianblum both 2.8 and 3.0 are already released, so this new feature will make it into 3.1.

@wouterj
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it's a component and it doesn't rely on any other components, yo ucan use it without a problem in 2.8/3.0 apps btw.

Please sign in to comment.