Skip to content

[Cache] Implement PSR-16 SimpleCache #20636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"twig/twig": "~1.28|~2.0",
"psr/cache": "~1.0",
"psr/log": "~1.0",
"psr/simple-cache": "^0.2",
"symfony/polyfill-intl-icu": "~1.0",
"symfony/polyfill-mbstring": "~1.0",
"symfony/polyfill-php56": "~1.0",
Expand Down Expand Up @@ -97,7 +98,8 @@
"phpdocumentor/type-resolver": "<0.2.0"
},
"provide": {
"psr/cache-implementation": "1.0"
"psr/cache-implementation": "1.0",
"psr/simple-cache-implementation": "1.0"
},
"autoload": {
"psr-4": {
Expand Down
60 changes: 59 additions & 1 deletion src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
use Psr\SimpleCache\CounterInterface;
use Symfony\Component\Cache\CacheItem;
use Symfony\Component\Cache\Exception\InvalidArgumentException;

/**
* @author Nicolas Grekas <p@tchwork.com>
*/
abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface
abstract class AbstractAdapter implements AdapterInterface, CounterInterface, LoggerAwareInterface
{
use LoggerAwareTrait;

Expand Down Expand Up @@ -372,6 +373,40 @@ public function commit()
return $ok;
}

/**
* {@inheritdoc}
*
* This implementation is not atomic unless the "doIncrement()" method provided by the concrete adapter is.
*/
public function increment($key, $step = 1)
{
if (!is_numeric($step)) {
return false;
}
$id = $this->getId($key);

try {
if (is_numeric($result = $this->doIncrement($id, (int) $step))) {
return $result;
}
CacheItem::log($this->logger, 'Failed to increment key "{key}"', array('key' => $key));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you call this code also from decrement, this message may be misleading in some cases

} catch (\Exception $e) {
CacheItem::log($this->logger, 'Failed to increment key "{key}"', array('key' => $key, 'exception' => $e));
}

return false;
}

/**
* {@inheritdoc}
*
* This implementation is not atomic unless the "doIncrement()" method provided by the concrete adapter is.
*/
public function decrement($key, $step = 1)
{
return is_numeric($step) ? $this->increment($key, -$step) : false;
}

public function __destruct()
{
if ($this->deferred) {
Expand Down Expand Up @@ -406,6 +441,29 @@ protected static function unserialize($value)
}
}

/**
* Increments a value in the cache by the given step value and returns the new value.
*
* If the key does not exist, it is initialized to the value of $step.
*
* @param string $key The cache item key
* @param int $step The value to increment by
*
* @return int|bool The new value on success and false on failure
*/
protected function doIncrement($id, $step)
{
foreach ($this->doFetch(array($id)) as $value) {
if (is_numeric($value)) {
$step += $value;
}
}

$e = $this->doSave(array($id => $step), 0);
Copy link
Member

Choose a reason for hiding this comment

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

this implementation in the base adapter is suspicious. The spec asks for atomicity, but atomicity requires to overwrite this method in each child adapter. An adapter not overwriting it would not provide atomicity, thus breaking the spec.

Copy link
Member Author

Choose a reason for hiding this comment

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


return true === $e || array() === $e ? $step : false;
}

private function getId($key)
{
CacheItem::validateKey($key);
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Cache/Adapter/ApcuAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,12 @@ protected function doSave(array $values, $lifetime)

throw $e;
}

/**
* {@inheritdoc}
*/
protected function doIncrement($id, $step)
{
return apcu_inc($id, $step);
}
}
30 changes: 29 additions & 1 deletion src/Symfony/Component/Cache/Adapter/ArrayAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
use Psr\Cache\CacheItemInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\SimpleCache\CounterInterface;
use Symfony\Component\Cache\CacheItem;

/**
* @author Nicolas Grekas <p@tchwork.com>
*/
class ArrayAdapter implements AdapterInterface, LoggerAwareInterface
class ArrayAdapter implements AdapterInterface, CounterInterface, LoggerAwareInterface
{
use LoggerAwareTrait;

Expand Down Expand Up @@ -197,6 +198,33 @@ public function commit()
return true;
}

/**
* {@inheritdoc}
*/
public function increment($key, $step = 1)
{
if (!is_numeric($step)) {
return false;
}

if ($this->hasItem($key) && is_numeric($this->values[$key])) {
$this->values[$key] += (int) $step;
} else {
$this->values[$key] = (int) $step;
$this->expiries[$key] = PHP_INT_MAX;
}

return $this->values[$key];
}

/**
* {@inheritdoc}
*/
public function decrement($key, $step = 1)
{
return is_numeric($step) ? $this->increment($key, -$step) : false;
}

private function generateItems(array $keys, $now)
{
$f = $this->createCacheItem;
Expand Down
19 changes: 19 additions & 0 deletions src/Symfony/Component/Cache/Adapter/FilesystemAdapterTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,25 @@ protected function doDelete(array $ids)
return $ok;
}

/**
* {@inheritdoc}
*/
protected function doIncrement($id, $step)
{
if (!$lock = @fopen($this->getFile($id, true), 'cb')) {
return false;
}
if (!flock($lock, LOCK_EX)) {
return false;
}
$result = parent::doIncrement($id, $step);

flock($lock, LOCK_UN);
fclose($lock);

return $result;
}

private function write($file, $data, $expiresAt = null)
{
if (false === @file_put_contents($this->tmp, $data)) {
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Cache/Adapter/PdoAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,19 @@ protected function doSave(array $values, $lifetime)
return $failed;
}

/**
* {@inheritdoc}
*/
protected function doIncrement($id, $step)
{
$conn = $this->getConnection();
$conn->beginTransaction();
$result = parent::doIncrement($id, $step);
$conn->commit();

return $result;
}

/**
* @return \PDO|Connection
*/
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Cache/Adapter/RedisAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ protected function doSave(array $values, $lifetime)
return $failed;
}

/**
* {@inheritdoc}
*/
protected function doIncrement($id, $step)
{
return $this->redis->incrBy($id, $step);
}

private function execute($command, $id, array $args, $redis = null)
{
array_unshift($args, $id);
Expand Down
Loading