diff --git a/app/Actions/CreateCommit.php b/app/Actions/CreateCommit.php index 95354559f..68b51b9ee 100644 --- a/app/Actions/CreateCommit.php +++ b/app/Actions/CreateCommit.php @@ -103,7 +103,9 @@ public function handle(User $user, string $body, ?Commit $parent = null, ?Commit } if ($commit->status === CommitStatus::Approved) { - dispatch(new CheckUserLevel($user))->afterCommit(); + // Outside the transaction, like CreateCommitStar: on Postgres, taking a unique-job lock that + // is already held fails the insert and aborts this transaction (25P02), which 500s the create. + DB::afterCommit(fn () => dispatch(new CheckUserLevel($user))); } if ($commit->status === CommitStatus::Pending && ! $hasGoodReputation) { diff --git a/tests/Feature/Controllers/CommitControllerTest.php b/tests/Feature/Controllers/CommitControllerTest.php index 5809dd842..5ce2b1c83 100644 --- a/tests/Feature/Controllers/CommitControllerTest.php +++ b/tests/Feature/Controllers/CommitControllerTest.php @@ -7,6 +7,7 @@ use App\Enums\CommitStatus; use App\Enums\Log; use App\Http\Middleware\HandleInertiaRequests; +use App\Jobs\CheckUserLevel; use App\Jobs\DetectCommitLanguage; use App\Jobs\UnfurlLink; use App\Models\Commit; @@ -14,8 +15,10 @@ use App\Models\LinkPreview; use App\Models\Notification; use App\Models\User; +use Illuminate\Database\DatabaseTransactionsManager; use Illuminate\Database\Eloquent\Factories\Sequence; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; @@ -35,6 +38,36 @@ expect($user->commits()->sole()->body)->toBe('Hello, devnull!'); }); +it('creates a commit while a unique level-check job already holds a database cache lock', function (): void { + config(['cache.default' => 'database']); + config(['app.admins' => ['nunomaduro']]); + + $user = User::factory()->create(['handle' => 'nunomaduro']); + $lock = Cache::lock('laravel_unique_job:'.CheckUserLevel::class.':'.$user->id, 3600); + + expect($lock->get())->toBeTrue(); + + // The test transaction manager runs after-commit callbacks before its wrapping transaction ends. + // Use the production manager to keep the callback pending until the outer transaction commits. + $connection = DB::connection(); + $testManager = $this->app['db.transactions']; + $manager = new DatabaseTransactionsManager; + $manager->begin($connection->getName(), $connection->transactionLevel()); + + $connection->setTransactionManager($manager); + + try { + $this->actingAs($user) + ->fromRoute('log.show', Log::Following) + ->post(route('commit.store'), ['body' => 'Second commit']) + ->assertRedirectToRoute('log.show', Log::Following); + } finally { + $connection->setTransactionManager($testManager); + } + + expect($user->commits()->sole()->body)->toBe('Second commit'); +}); + it('flashes the created commit', function (): void { $user = User::factory()->create();