Skip to content

Commit c6aeffd

Browse files
xdevorcrynobone
andauthored
[10.x] Fix Postgres cache store failed to put exist cache in transaction (#48968)
* [10.x] Fix Postgres cache store failed to put exist cache in transaction * Test PostgresCacheStore integration test * [10.x] Improve put operation of database cache store - Prevent non-rollback error query occur in transaction - For existing keys they will have to execute two queries instead of just one with upserts * Add database cache store integration test * Chore: ad db cache store integration test to databases workflows * Fix SqlServerCacheStoreTest * Refactor db cache store integration test * Refactor db cache store integration test * Refactor style * wip --------- Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
1 parent e5a7515 commit c6aeffd

File tree

3 files changed

+53
-29
lines changed

3 files changed

+53
-29
lines changed

src/Illuminate/Cache/DatabaseStore.php

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace Illuminate\Cache;
44

55
use Closure;
6-
use Exception;
76
use Illuminate\Contracts\Cache\LockProvider;
87
use Illuminate\Contracts\Cache\Store;
98
use Illuminate\Database\ConnectionInterface;
@@ -137,13 +136,7 @@ public function put($key, $value, $seconds)
137136
$value = $this->serialize($value);
138137
$expiration = $this->getTime() + $seconds;
139138

140-
try {
141-
return $this->table()->insert(compact('key', 'value', 'expiration'));
142-
} catch (Exception) {
143-
$result = $this->table()->where('key', $key)->update(compact('value', 'expiration'));
144-
145-
return $result > 0;
146-
}
139+
return $this->table()->upsert(compact('key', 'value', 'expiration'), 'key') > 0;
147140
}
148141

149142
/**

tests/Cache/CacheDatabaseStoreTest.php

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace Illuminate\Tests\Cache;
44

55
use Closure;
6-
use Exception;
76
use Illuminate\Cache\DatabaseStore;
87
use Illuminate\Database\Connection;
98
use Illuminate\Database\PostgresConnection;
@@ -63,41 +62,25 @@ public function testValueIsReturnedOnPostgres()
6362
$this->assertSame('bar', $store->get('foo'));
6463
}
6564

66-
public function testValueIsInsertedWhenNoExceptionsAreThrown()
65+
public function testValueIsUpserted()
6766
{
6867
$store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getMocks())->getMock();
6968
$table = m::mock(stdClass::class);
7069
$store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
7170
$store->expects($this->once())->method('getTime')->willReturn(1);
72-
$table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61])->andReturnTrue();
71+
$table->shouldReceive('upsert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61], 'key')->andReturnTrue();
7372

7473
$result = $store->put('foo', 'bar', 60);
7574
$this->assertTrue($result);
7675
}
7776

78-
public function testValueIsUpdatedWhenInsertThrowsException()
79-
{
80-
$store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getMocks())->getMock();
81-
$table = m::mock(stdClass::class);
82-
$store->getConnection()->shouldReceive('table')->with('table')->andReturn($table);
83-
$store->expects($this->once())->method('getTime')->willReturn(1);
84-
$table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61])->andReturnUsing(function () {
85-
throw new Exception;
86-
});
87-
$table->shouldReceive('where')->once()->with('key', 'prefixfoo')->andReturn($table);
88-
$table->shouldReceive('update')->once()->with(['value' => serialize('bar'), 'expiration' => 61])->andReturnTrue();
89-
90-
$result = $store->put('foo', 'bar', 60);
91-
$this->assertTrue($result);
92-
}
93-
94-
public function testValueIsInsertedOnPostgres()
77+
public function testValueIsUpsertedOnPostgres()
9578
{
9679
$store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getPostgresMocks())->getMock();
9780
$table = m::mock(stdClass::class);
9881
$store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
9982
$store->expects($this->once())->method('getTime')->willReturn(1);
100-
$table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => base64_encode(serialize("\0")), 'expiration' => 61])->andReturnTrue();
83+
$table->shouldReceive('upsert')->once()->with(['key' => 'prefixfoo', 'value' => base64_encode(serialize("\0")), 'expiration' => 61], 'key')->andReturn(1);
10184

10285
$result = $store->put('foo', "\0", 60);
10386
$this->assertTrue($result);
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
namespace Illuminate\Tests\Integration\Database;
4+
5+
use Illuminate\Support\Facades\Cache;
6+
use Illuminate\Support\Facades\DB;
7+
use Orchestra\Testbench\Attributes\WithMigration;
8+
9+
#[WithMigration('cache')]
10+
class DatabaseCacheStoreTest extends DatabaseTestCase
11+
{
12+
public function testValueCanStoreNewCache()
13+
{
14+
$store = $this->getStore();
15+
16+
$store->put('foo', 'bar', 60);
17+
18+
$this->assertSame('bar', $store->get('foo'));
19+
}
20+
21+
public function testValueCanUpdateExistCache()
22+
{
23+
$store = $this->getStore();
24+
25+
$store->put('foo', 'bar', 60);
26+
$store->put('foo', 'new-bar', 60);
27+
28+
$this->assertSame('new-bar', $store->get('foo'));
29+
}
30+
31+
public function testValueCanUpdateExistCacheInTransaction()
32+
{
33+
$store = $this->getStore();
34+
35+
$store->put('foo', 'bar', 60);
36+
37+
DB::beginTransaction();
38+
$store->put('foo', 'new-bar', 60);
39+
DB::commit();
40+
41+
$this->assertSame('new-bar', $store->get('foo'));
42+
}
43+
44+
protected function getStore()
45+
{
46+
return Cache::store('database');
47+
}
48+
}

0 commit comments

Comments
 (0)