Skip to content

[5.8] Fix PSR-16 TTL conformity #27217

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

Merged
merged 2 commits into from
Jan 19, 2019
Merged
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
6 changes: 5 additions & 1 deletion src/Illuminate/Cache/RedisTaggedCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ class RedisTaggedCache extends TaggedCache
*
* @param string $key
* @param mixed $value
* @param \DateTime|float|int|null $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @return bool
*/
public function put($key, $value, $minutes = null)
{
if ($minutes === null) {
return $this->forever($key, $value);
}

$this->pushStandardKeys($this->tags->getNamespace(), $key);

return parent::put($key, $value, $minutes);
Expand Down
114 changes: 75 additions & 39 deletions src/Illuminate/Cache/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Repository implements CacheContract, ArrayAccess
/**
* The default number of minutes to store items.
*
* @var float|int
* @var float|int|null
*/
protected $default = 60;

Expand Down Expand Up @@ -199,22 +199,26 @@ public function pull($key, $default = null)
public function put($key, $value, $minutes = null)
{
if (is_array($key)) {
$result = $this->putMany($key, $value);
return $this->putMany($key, $value);
}

return $result;
if ($minutes === null) {
return $this->forever($key, $value);
}

if (! is_null($minutes = $this->getMinutes($minutes))) {
$result = $this->store->put($this->itemKey($key), $value, $minutes);
$minutes = $this->getMinutes($minutes);

if ($result) {
$this->event(new KeyWritten($key, $value, $minutes));
}
if ($minutes <= 0) {
return $this->delete($key);
}

$result = $this->store->put($this->itemKey($key), $value, $minutes);

return $result;
if ($result) {
$this->event(new KeyWritten($key, $value, $minutes));
}

return false;
return $result;
}

/**
Expand All @@ -229,24 +233,52 @@ public function set($key, $value, $ttl = null)
* Store multiple items in the cache for a given number of minutes.
*
* @param array $values
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @return bool
*/
public function putMany(array $values, $minutes)
public function putMany(array $values, $minutes = null)
{
if (! is_null($minutes = $this->getMinutes($minutes))) {
$result = $this->store->putMany($values, $minutes);
if ($minutes === null) {
return $this->putManyForever($values);
}

if ($result) {
foreach ($values as $key => $value) {
$this->event(new KeyWritten($key, $value, $minutes));
}
$minutes = $this->getMinutes($minutes);

if ($minutes <= 0) {
return $this->deleteMultiple(array_keys($values));
}

$result = $this->store->putMany($values, $minutes);

if ($result) {
foreach ($values as $key => $value) {
$this->event(new KeyWritten($key, $value, $minutes));
}
}

return $result;
}

return $result;
/**
* Store multiple items in the cache indefinitely.
*
* @param array $values
* @return bool
*/
protected function putManyForever(array $values)
{
$result = true;

// We'll loop over every item and attempt to store it indefinitely.
// If we notice that one of the items can't be stored forever we
// will return false. Otherwise we'll return a success result.
foreach ($values as $key => $value) {
if (! $this->forever($key, $value)) {
$result = false;
}
}

return false;
return $result;
}

/**
Expand All @@ -262,22 +294,26 @@ public function setMultiple($values, $ttl = null)
*
* @param string $key
* @param mixed $value
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @return bool
*/
public function add($key, $value, $minutes)
public function add($key, $value, $minutes = null)
{
if (is_null($minutes = $this->getMinutes($minutes))) {
return false;
}
if ($minutes !== null) {
if ($this->getMinutes($minutes) <= 0) {
return false;
}

// If the store has an "add" method we will call the method on the store so it
// has a chance to override this logic. Some drivers better support the way
// this operation should work with a total "atomic" implementation of it.
if (method_exists($this->store, 'add')) {
$minutes = $this->getMinutes($minutes);

// If the store has an "add" method we will call the method on the store so it
// has a chance to override this logic. Some drivers better support the way
// this operation should work with a total "atomic" implementation of it.
if (method_exists($this->store, 'add')) {
return $this->store->add(
$this->itemKey($key), $value, $minutes
);
return $this->store->add(
$this->itemKey($key), $value, $minutes
);
}
}

// If the value did not exist in the cache, we will put the value in the cache
Expand Down Expand Up @@ -336,7 +372,7 @@ public function forever($key, $value)
* Get an item from the cache, or execute the given Closure and store the result.
*
* @param string $key
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @param \Closure $callback
* @return mixed
*/
Expand Down Expand Up @@ -479,7 +515,7 @@ public function getDefaultCacheTime()
/**
* Set the default cache time in minutes.
*
* @param float|int $minutes
* @param float|int|null $minutes
* @return $this
*/
public function setDefaultCacheTime($minutes)
Expand Down Expand Up @@ -571,18 +607,18 @@ public function offsetUnset($key)
/**
* Calculate the number of minutes with the given duration.
*
* @param \DateTimeInterface|\DateInterval|float|int $duration
* @return float|int|null
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @return float|int
*/
protected function getMinutes($duration)
protected function getMinutes($minutes)
{
$duration = $this->parseDateInterval($duration);
$duration = $this->parseDateInterval($minutes);

if ($duration instanceof DateTimeInterface) {
$duration = Carbon::now()->diffInRealSeconds($duration, false) / 60;
}

return (int) ($duration * 60) > 0 ? $duration : null;
return (int) ($duration * 60) > 0 ? $duration : 0;
}

/**
Expand Down
20 changes: 19 additions & 1 deletion src/Illuminate/Cache/TaggedCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

class TaggedCache extends Repository
{
use RetrievesMultipleKeys;
use RetrievesMultipleKeys {
putMany as putManyAlias;
}

/**
* The tag set instance.
Expand All @@ -29,6 +31,22 @@ public function __construct(Store $store, TagSet $tags)
$this->tags = $tags;
}

/**
* Store multiple items in the cache for a given number of minutes.
*
* @param array $values
* @param float|int|null $minutes
* @return bool
*/
public function putMany(array $values, $minutes = null)
{
if ($minutes === null) {
return $this->putManyForever($values);
}

return $this->putManyAlias($values, $minutes);
}

/**
* Increment the value of an item in the cache.
*
Expand Down
8 changes: 4 additions & 4 deletions src/Illuminate/Contracts/Cache/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function pull($key, $default = null);
*
* @param string $key
* @param mixed $value
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @return bool
*/
public function put($key, $value, $minutes);
Expand All @@ -31,10 +31,10 @@ public function put($key, $value, $minutes);
*
* @param string $key
* @param mixed $value
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @return bool
*/
public function add($key, $value, $minutes);
public function add($key, $value, $minutes = null);

/**
* Increment the value of an item in the cache.
Expand Down Expand Up @@ -67,7 +67,7 @@ public function forever($key, $value);
* Get an item from the cache, or execute the given Closure and store the result.
*
* @param string $key
* @param \DateTimeInterface|\DateInterval|float|int $minutes
* @param \DateTimeInterface|\DateInterval|float|int|null $minutes
* @param \Closure $callback
* @return mixed
*/
Expand Down
6 changes: 1 addition & 5 deletions src/Illuminate/Filesystem/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ public function save()
{
$contents = $this->getForStorage();

if (! is_null($this->expire)) {
$this->repository->put($this->key, $contents, $this->expire);
} else {
$this->repository->forever($this->key, $contents);
}
$this->repository->put($this->key, $contents, $this->expire);
}
}
42 changes: 33 additions & 9 deletions tests/Cache/CacheRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,30 @@ public function testSettingMultipleItemsInCache()
$this->assertTrue($result);
}

public function testPutWithDatetimeInPastOrZeroSecondsDoesntSaveItem()
public function testPutWithNullTTLRemembersItemForever()
{
$repo = $this->getRepository();
$repo->getStore()->shouldReceive('forever')->once()->with('foo', 'bar')->andReturn(true);
$this->assertTrue($repo->put('foo', 'bar'));
}

public function testPutWithDatetimeInPastOrZeroSecondsRemovesOldItem()
{
$repo = $this->getRepository();
$repo->getStore()->shouldReceive('put')->never();
$repo->getStore()->shouldReceive('forget')->twice()->andReturn(true);
$result = $repo->put('foo', 'bar', Carbon::now()->subMinutes(10));
$this->assertFalse($result);
$this->assertTrue($result);
$result = $repo->put('foo', 'bar', Carbon::now());
$this->assertFalse($result);
$this->assertTrue($result);
}

public function testAddWithDatetimeInPastOrZeroSecondsReturnsImmediately()
public function testPutManyWithNullTTLRemembersItemsForever()
{
$repo = $this->getRepository();
$repo->getStore()->shouldReceive('add', 'get', 'put')->never();
$result = $repo->add('foo', 'bar', Carbon::now()->subMinutes(10));
$this->assertFalse($result);
$result = $repo->add('foo', 'bar', Carbon::now());
$this->assertFalse($result);
$repo->getStore()->shouldReceive('forever')->with('foo', 'bar')->andReturn(true);
$repo->getStore()->shouldReceive('forever')->with('bar', 'baz')->andReturn(true);
$this->assertTrue($repo->putMany(['foo' => 'bar', 'bar' => 'baz']));
}

public function testAddWithStoreFailureReturnsFalse()
Expand All @@ -174,6 +180,24 @@ public function testCacheAddCallsRedisStoreAdd()
$this->assertTrue($repository->add('k', 'v', 60));
}

public function testAddWithNullTTLRemembersItemForever()
{
$repo = $this->getRepository();
$repo->getStore()->shouldReceive('get')->once()->with('foo')->andReturn(null);
$repo->getStore()->shouldReceive('forever')->once()->with('foo', 'bar')->andReturn(true);
$this->assertTrue($repo->add('foo', 'bar'));
}

public function testAddWithDatetimeInPastOrZeroSecondsReturnsImmediately()
{
$repo = $this->getRepository();
$repo->getStore()->shouldReceive('add', 'get', 'put')->never();
$result = $repo->add('foo', 'bar', Carbon::now()->subMinutes(10));
$this->assertFalse($result);
$result = $repo->add('foo', 'bar', Carbon::now());
$this->assertFalse($result);
}

public function dataProviderTestGetMinutes()
{
Carbon::setTestNow(Carbon::parse($this->getTestDate()));
Expand Down
17 changes: 17 additions & 0 deletions tests/Cache/CacheTaggedCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,23 @@ public function testRedisCacheTagsPushStandardKeysCorrectly()
$conn->shouldReceive('sadd')->once()->with('prefix:foo:standard_ref', 'prefix:'.sha1('foo|bar').':key1');
$conn->shouldReceive('sadd')->once()->with('prefix:bar:standard_ref', 'prefix:'.sha1('foo|bar').':key1');
$store->shouldReceive('push')->with(sha1('foo|bar').':key1', 'key1:value');
$store->shouldReceive('put')->andReturn(true);

$redis->put('key1', 'key1:value', 60);
}

public function testRedisCacheTagsPushForeverKeysCorrectlyWithNullTTL()
{
$store = m::mock(Store::class);
$tagSet = m::mock(TagSet::class, [$store, ['foo', 'bar']]);
$tagSet->shouldReceive('getNamespace')->andReturn('foo|bar');
$tagSet->shouldReceive('getNames')->andReturn(['foo', 'bar']);
$redis = new RedisTaggedCache($store, $tagSet);
$store->shouldReceive('getPrefix')->andReturn('prefix:');
$store->shouldReceive('connection')->andReturn($conn = m::mock(stdClass::class));
$conn->shouldReceive('sadd')->once()->with('prefix:foo:forever_ref', 'prefix:'.sha1('foo|bar').':key1');
$conn->shouldReceive('sadd')->once()->with('prefix:bar:forever_ref', 'prefix:'.sha1('foo|bar').':key1');
$store->shouldReceive('forever')->with(sha1('foo|bar').':key1', 'key1:value');

$redis->put('key1', 'key1:value');
}
Expand Down