Skip to content

Commit

Permalink
Fixes TTL deletion issue: Use the correct firestore timestamp data type
Browse files Browse the repository at this point in the history
  • Loading branch information
richan-fongdasen committed May 4, 2024
1 parent 9f6c1df commit 2d7f4d8
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
17 changes: 12 additions & 5 deletions src/Cache/FirestoreStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace RichanFongdasen\Firestore\Cache;

use DateTimeInterface;
use Google\Cloud\Core\Timestamp;
use Google\Cloud\Firestore\FieldValue;
use Google\Cloud\Firestore\FirestoreClient;
use Illuminate\Contracts\Cache\Store;
Expand Down Expand Up @@ -109,8 +110,14 @@ protected function isExpired(array $item, ?DateTimeInterface $expiration = null)
$expiration = Carbon::now();
}

return isset($item[$this->expirationAttribute]) &&
$expiration->getTimestamp() >= (int) $item[$this->expirationAttribute];
$value = data_get($item, $this->expirationAttribute);

// If the expiration attribute is not set or not an instance of Timestamp, then consider it as expired.
if (! ($value instanceof Timestamp)) {
return true;
}

return $expiration->getTimestamp() >= $value->get()->getTimestamp();
}

/**
Expand Down Expand Up @@ -283,11 +290,11 @@ public function flush()
/**
* Get the UNIX timestamp for the given number of seconds.
*/
protected function toTimestamp(int $seconds): int
protected function toTimestamp(int $seconds): Timestamp
{
return $seconds > 0
? $this->availableAt($seconds)
: $this->currentTime();
? new Timestamp(Carbon::now()->addSeconds($seconds))
: new Timestamp(Carbon::now());
}

/**
Expand Down
6 changes: 4 additions & 2 deletions tests/Feature/Cache/CacheLockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
->and($document->id())->toBe('test-lock-key')
->and($document->data()['key'])->toBe('test-lock-key')
->and($document->data()['value'])->toBe(serialize($lock->owner()))
->and($document->data()['expired_at'])->toBeGreaterThan(now()->addSeconds(9)->getTimestamp());
->and($document->data()['expired_at'])->toBeInstanceOf('Google\Cloud\Core\Timestamp')
->and($document->data()['expired_at']->get()->getTimestamp())->toBeGreaterThan(now()->addSeconds(9)->getTimestamp());

$lock->release();

Expand All @@ -43,7 +44,8 @@
->and($document->id())->toBe('test-lock-key')
->and($document->data()['key'])->toBe('test-lock-key')
->and($document->data()['value'])->toBe(serialize($lock->owner()))
->and($document->data()['expired_at'])->toBeGreaterThan(now()->addSeconds(9)->getTimestamp());
->and($document->data()['expired_at'])->toBeInstanceOf('Google\Cloud\Core\Timestamp')
->and($document->data()['expired_at']->get()->getTimestamp())->toBeGreaterThan(now()->addSeconds(9)->getTimestamp());

$lock->forceRelease();

Expand Down
5 changes: 3 additions & 2 deletions tests/Feature/FirestoreStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare(strict_types=1);

use Google\Cloud\Core\Timestamp;
use Google\Cloud\Firestore\FirestoreClient;
use RichanFongdasen\Firestore\Cache\FirestoreStore;

Expand All @@ -22,13 +23,13 @@
$this->firestore->collection('cache')->document('prefix_first-test-key')->set([
'key' => 'prefix_first-test-key',
'value' => serialize('first test value'),
'expired_at' => now()->addHours(4)->getTimestamp(),
'expired_at' => new Timestamp(now()->addHours(4)),
]);

$this->firestore->collection('cache')->document('prefix_second-test-key')->set([
'key' => 'prefix_second-test-key',
'value' => serialize('second test value'),
'expired_at' => now()->addHours(5)->getTimestamp(),
'expired_at' => new Timestamp(now()->addHours(5)),
]);
});

Expand Down

0 comments on commit 2d7f4d8

Please sign in to comment.