Skip to content

Commit e106f16

Browse files
committed
Merge branch '5.x' of https://github.com/craftcms/cms into 6.x
# Conflicts: # CHANGELOG.md # src/elements/Entry.php # tests/unit/services/EntriesTest.php
2 parents f5431b7 + 06292c9 commit e106f16

2 files changed

Lines changed: 90 additions & 3 deletions

File tree

src/Entry/Elements/Entry.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,9 +2518,13 @@ private function _saveAuthors(): void
25182518
->all();
25192519
}
25202520

2521-
DB::table(Table::ENTRIES_AUTHORS)
2522-
->where('entryId', $this->id)
2523-
->delete();
2521+
// Only issue the delete if there’s something to delete: an unconditional delete for a brand-new
2522+
// entry ID can take a gap lock on the primary index and deadlock against other transactions
2523+
// inserting authors for their own new entries.
2524+
$authorsQuery = DB::table(Table::ENTRIES_AUTHORS)->where('entryId', $this->id);
2525+
if ($authorsQuery->exists()) {
2526+
$authorsQuery->delete();
2527+
}
25242528

25252529
if (! empty($this->_authorIds)) {
25262530
$data = [];

tests/Feature/Entry/EntriesTest.php

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,40 @@
1313
use CraftCms\Cms\Section\Enums\SectionType;
1414
use CraftCms\Cms\Section\Models\Section;
1515
use CraftCms\Cms\Site\Models\Site;
16+
use CraftCms\Cms\Support\Facades\Elements;
1617
use CraftCms\Cms\Support\Facades\Sections;
1718
use CraftCms\Cms\User\Models\User;
19+
use Illuminate\Database\Events\QueryExecuted;
1820
use Illuminate\Support\Facades\DB;
1921
use Illuminate\Support\Facades\Event;
2022

23+
/**
24+
* Records which `entries_authors` write statements (delete/insert) run during $callback.
25+
*
26+
* @return string[]
27+
*/
28+
function captureEntriesAuthorsCommands(Closure $callback): array
29+
{
30+
$commands = [];
31+
32+
DB::listen(function (QueryExecuted $query) use (&$commands) {
33+
$sql = strtolower(ltrim($query->sql));
34+
if (! str_contains($sql, 'entries_authors')) {
35+
return;
36+
}
37+
38+
if (str_starts_with($sql, 'delete')) {
39+
$commands[] = 'delete';
40+
} elseif (str_starts_with($sql, 'insert')) {
41+
$commands[] = 'insert';
42+
}
43+
});
44+
45+
$callback();
46+
47+
return $commands;
48+
}
49+
2150
beforeEach(function () {
2251
$this->entries = app(Entries::class);
2352
});
@@ -205,3 +234,57 @@
205234
->pluck('authorId')
206235
->all())->toBe([$newAuthor->id, $newAuthor->id]);
207236
});
237+
238+
it('does not delete missing author rows when saving an entry’s authors for the first time', function () {
239+
$entryType = EntryType::factory()->create();
240+
$section = Section::factory()->withEntryTypes($entryType)->create([
241+
'type' => SectionType::Channel,
242+
]);
243+
$author = User::factory()->create();
244+
245+
$entry = Entry::factory()
246+
->forSection($section)
247+
->forEntryType($entryType)
248+
->createElement();
249+
$entry->setAuthorIds([$author->id]);
250+
251+
$commands = captureEntriesAuthorsCommands(fn () => Elements::saveElement($entry));
252+
253+
// No existing rows to delete yet, so the save shouldn’t issue a delete (which would otherwise
254+
// take a needless gap lock on the primary index and risk deadlocking against other transactions
255+
// inserting authors for their own new entries).
256+
expect($commands)->not->toContain('delete');
257+
expect($commands)->toContain('insert');
258+
expect(DB::table(Table::ENTRIES_AUTHORS)
259+
->where('entryId', $entry->id)
260+
->pluck('authorId')
261+
->all())->toBe([$author->id]);
262+
});
263+
264+
it('deletes existing author rows when an entry’s authors change', function () {
265+
$entryType = EntryType::factory()->create();
266+
$section = Section::factory()->withEntryTypes($entryType)->create([
267+
'type' => SectionType::Channel,
268+
]);
269+
$oldAuthor = User::factory()->create();
270+
$newAuthor = User::factory()->create();
271+
272+
$entry = Entry::factory()
273+
->forSection($section)
274+
->forEntryType($entryType)
275+
->createElement();
276+
$entry->setAuthorIds([$oldAuthor->id]);
277+
Elements::saveElement($entry);
278+
279+
$entry->setAuthorIds([$newAuthor->id]);
280+
281+
$commands = captureEntriesAuthorsCommands(fn () => Elements::saveElement($entry));
282+
283+
// The old author row does exist this time, so the existing rows must still be removed.
284+
expect($commands)->toContain('delete');
285+
expect($commands)->toContain('insert');
286+
expect(DB::table(Table::ENTRIES_AUTHORS)
287+
->where('entryId', $entry->id)
288+
->pluck('authorId')
289+
->all())->toBe([$newAuthor->id]);
290+
});

0 commit comments

Comments
 (0)