[10.x] Fix Postgres cache store failed to put exist cache in transact…

…ion (#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>

src/Illuminate/Cache/DatabaseStore.php

@@ -3,7 +3,6 @@
 namespace Illuminate\Cache;
 
 use Closure;
-use Exception;
 use Illuminate\Contracts\Cache\LockProvider;
 use Illuminate\Contracts\Cache\Store;
 use Illuminate\Database\ConnectionInterface;
@@ -137,13 +136,7 @@ public function put($key, $value, $seconds)
         $value = $this->serialize($value);
         $expiration = $this->getTime() + $seconds;
 
-        try {
-            return $this->table()->insert(compact('key', 'value', 'expiration'));
-        } catch (Exception) {
-            $result = $this->table()->where('key', $key)->update(compact('value', 'expiration'));
-
-            return $result > 0;
-        }
+        return $this->table()->upsert(compact('key', 'value', 'expiration'), 'key') > 0;
     }
 
     /**

tests/Cache/CacheDatabaseStoreTest.php

@@ -3,7 +3,6 @@
 namespace Illuminate\Tests\Cache;
 
 use Closure;
-use Exception;
 use Illuminate\Cache\DatabaseStore;
 use Illuminate\Database\Connection;
 use Illuminate\Database\PostgresConnection;
@@ -63,41 +62,25 @@ public function testValueIsReturnedOnPostgres()
         $this->assertSame('bar', $store->get('foo'));
     }
 
-    public function testValueIsInsertedWhenNoExceptionsAreThrown()
+    public function testValueIsUpserted()
     {
         $store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getMocks())->getMock();
         $table = m::mock(stdClass::class);
         $store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
         $store->expects($this->once())->method('getTime')->willReturn(1);
-        $table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61])->andReturnTrue();
+        $table->shouldReceive('upsert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61], 'key')->andReturnTrue();
 
         $result = $store->put('foo', 'bar', 60);
         $this->assertTrue($result);
     }
 
-    public function testValueIsUpdatedWhenInsertThrowsException()
-    {
-        $store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getMocks())->getMock();
-        $table = m::mock(stdClass::class);
-        $store->getConnection()->shouldReceive('table')->with('table')->andReturn($table);
-        $store->expects($this->once())->method('getTime')->willReturn(1);
-        $table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => serialize('bar'), 'expiration' => 61])->andReturnUsing(function () {
-            throw new Exception;
-        });
-        $table->shouldReceive('where')->once()->with('key', 'prefixfoo')->andReturn($table);
-        $table->shouldReceive('update')->once()->with(['value' => serialize('bar'), 'expiration' => 61])->andReturnTrue();
-
-        $result = $store->put('foo', 'bar', 60);
-        $this->assertTrue($result);
-    }
-
-    public function testValueIsInsertedOnPostgres()
+    public function testValueIsUpsertedOnPostgres()
     {
         $store = $this->getMockBuilder(DatabaseStore::class)->onlyMethods(['getTime'])->setConstructorArgs($this->getPostgresMocks())->getMock();
         $table = m::mock(stdClass::class);
         $store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
         $store->expects($this->once())->method('getTime')->willReturn(1);
-        $table->shouldReceive('insert')->once()->with(['key' => 'prefixfoo', 'value' => base64_encode(serialize("\0")), 'expiration' => 61])->andReturnTrue();
+        $table->shouldReceive('upsert')->once()->with(['key' => 'prefixfoo', 'value' => base64_encode(serialize("\0")), 'expiration' => 61], 'key')->andReturn(1);
 
         $result = $store->put('foo', "\0", 60);
         $this->assertTrue($result);

tests/Integration/Database/DatabaseCacheStoreTest.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\DB;
+use Orchestra\Testbench\Attributes\WithMigration;
+
+#[WithMigration('cache')]
+class DatabaseCacheStoreTest extends DatabaseTestCase
+{
+    public function testValueCanStoreNewCache()
+    {
+        $store = $this->getStore();
+
+        $store->put('foo', 'bar', 60);
+
+        $this->assertSame('bar', $store->get('foo'));
+    }
+
+    public function testValueCanUpdateExistCache()
+    {
+        $store = $this->getStore();
+
+        $store->put('foo', 'bar', 60);
+        $store->put('foo', 'new-bar', 60);
+
+        $this->assertSame('new-bar', $store->get('foo'));
+    }
+
+    public function testValueCanUpdateExistCacheInTransaction()
+    {
+        $store = $this->getStore();
+
+        $store->put('foo', 'bar', 60);
+
+        DB::beginTransaction();
+        $store->put('foo', 'new-bar', 60);
+        DB::commit();
+
+        $this->assertSame('new-bar', $store->get('foo'));
+    }
+
+    protected function getStore()
+    {
+        return Cache::store('database');
+    }
+}