diff --git a/config/git.php b/config/git.php index 7c569b0435f..6edadac32da 100644 --- a/config/git.php +++ b/config/git.php @@ -62,6 +62,20 @@ 'dispatch_delay' => env('STATAMIC_GIT_DISPATCH_DELAY', 0), + /* + |-------------------------------------------------------------------------- + | Unique Lock Expiry + |-------------------------------------------------------------------------- + | + | When commits are queued, a unique lock prevents multiple jobs from + | running concurrently against the same repository. This value (in + | seconds) controls how long that lock is held as a crash-safety + | net. It should exceed your queue worker's configured timeout. + | + */ + + 'unique_lock_expiry' => env('STATAMIC_GIT_UNIQUE_LOCK_EXPIRY', 120), + /* |-------------------------------------------------------------------------- | Git User diff --git a/src/Git/CommitJob.php b/src/Git/CommitJob.php index 726443d76b4..31941328387 100644 --- a/src/Git/CommitJob.php +++ b/src/Git/CommitJob.php @@ -3,20 +3,27 @@ namespace Statamic\Git; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; +use Illuminate\Support\Facades\Cache; use Statamic\Facades\Git; -class CommitJob implements ShouldQueue +use function Statamic\trans as __; + +class CommitJob implements ShouldBeUnique, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable; + public int $uniqueFor; + /** * Create a new job instance. */ public function __construct(public $message = null, public $committer = null) { + $this->uniqueFor = config('statamic.git.unique_lock_expiry', 120); } /** @@ -24,6 +31,17 @@ public function __construct(public $message = null, public $committer = null) */ public function handle() { - Git::as($this->committer)->commit($this->message); + $saves = Cache::pull('statamic-git-pending-saves', []); + $users = collect($saves)->unique('email')->values(); + $coalesced = $users->count() > 1; + + $message = $this->message; + + if ($coalesced) { + $trailers = $users->map(fn ($u) => "Co-Authored-By: {$u['name']} <{$u['email']}>")->implode("\n"); + $message = ($message ?? __('Content saved'))."\n\n".$trailers; + } + + Git::as($coalesced ? null : $this->committer)->commit($message); } } diff --git a/src/Git/Git.php b/src/Git/Git.php index 473ce1e6ea0..994c0124a98 100644 --- a/src/Git/Git.php +++ b/src/Git/Git.php @@ -4,6 +4,7 @@ use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Statamic\Console\Processes\Git as GitProcess; use Statamic\Contracts\Auth\User as UserContract; use Statamic\Facades\Antlers; @@ -93,6 +94,10 @@ public function dispatchCommit($message = null) $message = null; } + $saves = Cache::get('statamic-git-pending-saves', []); + $saves[] = ['name' => $this->gitUserName(), 'email' => $this->gitUserEmail()]; + Cache::put('statamic-git-pending-saves', $saves); + CommitJob::dispatch($message, $this->authenticatedUser()) ->onConnection(config('statamic.git.queue_connection')) ->delay($delayInMinutes ?? null); @@ -283,8 +288,11 @@ protected function shellEscape(string $string) { $string = str_replace('"', '', $string); $string = str_replace("'", '', $string); + $string = str_replace('\\', '\\\\', $string); + $string = str_replace('$', '\\$', $string); + $string = str_replace('`', '\\`', $string); - return escapeshellcmd($string); + return $string; } /** diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index 38c1a15b3f9..5facba3e872 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -3,6 +3,7 @@ namespace Tests\Git; use Illuminate\Filesystem\Filesystem; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Queue; use PHPUnit\Framework\Attributes\Test; use Statamic\Console\Processes\Git as GitProcess; @@ -252,13 +253,8 @@ public function it_shell_escapes_git_user_name_and_email() Git::commit('Message"; echo "deleting all your files now"; #'); - $expectedUser = 'Jimmy\; echo deleting all your files now\; \# '; - $expectedMessage = 'Message\; echo deleting all your files now\; \#'; - - if (static::isRunningWindows()) { - $expectedUser = str_replace('\\', '^', $expectedUser); - $expectedMessage = str_replace('\\', '^', $expectedMessage); - } + $expectedUser = 'Jimmy; echo deleting all your files now; # '; + $expectedMessage = 'Message; echo deleting all your files now; #'; $lastCommit = $this->showLastCommit(base_path('content')); @@ -400,6 +396,115 @@ public function it_dispatches_commit_job() Queue::assertPushed(\Statamic\Git\CommitJob::class, 1); } + #[Test] + public function it_only_dispatches_one_commit_job_at_a_time() + { + // ShouldBeUnique acquires its cache lock in Bus\Dispatcher before the job + // reaches the queue driver, so Queue::fake() still enforces uniqueness here. + Queue::fake(); + + Git::dispatchCommit(); + Git::dispatchCommit(); + Git::dispatchCommit(); + + Queue::assertPushed(\Statamic\Git\CommitJob::class, 1); + } + + #[Test] + public function it_attributes_coalesced_commits_to_the_configured_user() + { + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Bob', 'email' => 'bob@example.com'], + ]); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with(null)->andReturnSelf()->once(); + Git::shouldReceive('commit')->withArgs(function ($message) { + return str_contains($message, 'Entry saved') + && str_contains($message, 'Co-Authored-By: Alice ') + && str_contains($message, 'Co-Authored-By: Bob '); + })->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + + #[Test] + public function it_attributes_non_coalesced_commits_to_the_authenticated_user() + { + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ]); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with($user)->andReturnSelf()->once(); + Git::shouldReceive('commit')->with('Entry saved')->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + + #[Test] + public function it_adds_co_authored_by_trailers_for_coalesced_saves() + { + $this->files->put(base_path('content/collections/pages.yaml'), 'title: Pages Title Changed'); + + $message = "Content saved\n\nCo-Authored-By: Alice \nCo-Authored-By: Bob "; + + Git::commit($message); + + $commit = $this->showLastCommit(base_path('content')); + + $this->assertStringContainsString('Co-Authored-By: Alice ', $commit); + $this->assertStringContainsString('Co-Authored-By: Bob ', $commit); + } + + #[Test] + public function it_deduplicates_co_authors_for_repeated_saves_by_the_same_user() + { + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Bob', 'email' => 'bob@example.com'], + ]); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with(null)->andReturnSelf()->once(); + Git::shouldReceive('commit')->withArgs(function ($message) { + return substr_count($message, 'Co-Authored-By:') === 2 + && str_contains($message, 'Co-Authored-By: Alice ') + && str_contains($message, 'Co-Authored-By: Bob '); + })->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + + #[Test] + public function it_wires_dispatch_to_co_authored_by_trailers_end_to_end() + { + Queue::fake(); + + $this->files->put(base_path('content/collections/pages.yaml'), 'title: Pages Title Changed'); + + $alice = User::make()->email('alice@example.com')->data(['name' => 'Alice'])->makeSuper(); + $bob = User::make()->email('bob@example.com')->data(['name' => 'Bob'])->makeSuper(); + + $this->actingAs($alice); + Git::dispatchCommit(); + + $this->actingAs($bob); + Git::dispatchCommit(); // dropped by ShouldBeUnique, but Bob's save is still recorded in cache + + (new \Statamic\Git\CommitJob(null, $alice))->handle(); + + $commit = $this->showLastCommit(base_path('content')); + + $this->assertStringContainsString('Co-Authored-By: Alice ', $commit); + $this->assertStringContainsString('Co-Authored-By: Bob ', $commit); + } + #[Test] public function it_doesnt_push_by_default() {