From 178049a8daaef503cfa04099ae4f5ba703e7a311 Mon Sep 17 00:00:00 2001 From: AnnoyingTechnology Date: Mon, 20 Jul 2026 18:55:47 +0200 Subject: [PATCH] Restore DAV database invariants and sync indexes Add the collection-scoped uniqueness constraints expected by the SabreDAV PDO backends, including the path/name constraint required by property upserts. Add compound indexes matching calendar and address-book sync queries.\n\nAbort with an actionable message when legacy duplicates would make a unique index unsafe rather than deleting or merging user data. --- config/services.yaml | 5 + migrations/Version20260720103000.php | 79 +++++++++++++++ migrations/Version20260720104000.php | 102 ++++++++++++++++++++ src/Doctrine/SQLiteForeignKeyMiddleware.php | 20 ++++ src/Entity/AddressBook.php | 1 + src/Entity/AddressBookChange.php | 3 +- src/Entity/CalendarChange.php | 3 +- src/Entity/CalendarInstance.php | 5 +- src/Entity/CalendarObject.php | 3 +- src/Entity/CalendarSubscription.php | 1 + src/Entity/Card.php | 3 +- src/Entity/Principal.php | 4 +- src/Entity/PropertyStorage.php | 1 + src/Entity/SchedulingObject.php | 1 + tests/Functional/SQLiteForeignKeyTest.php | 26 +++++ 15 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 migrations/Version20260720103000.php create mode 100644 migrations/Version20260720104000.php create mode 100644 src/Doctrine/SQLiteForeignKeyMiddleware.php create mode 100644 tests/Functional/SQLiteForeignKeyTest.php diff --git a/config/services.yaml b/config/services.yaml index 25662b12..a0ba9a14 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -26,6 +26,11 @@ services: resource: '../src/*' exclude: '../src/{DependencyInjection,Entity,Migrations,Tests,Kernel.php}' + App\Doctrine\SQLiteForeignKeyMiddleware: + autoconfigure: false + tags: + - { name: doctrine.middleware, priority: 100 } + App\Services\Utils: arguments: $authRealm: "%env(AUTH_REALM)%" diff --git a/migrations/Version20260720103000.php b/migrations/Version20260720103000.php new file mode 100644 index 00000000..752027dd --- /dev/null +++ b/migrations/Version20260720103000.php @@ -0,0 +1,79 @@ + ['addressbooks', ['principaluri', 'uri'], false], + 'uniq_calendarinstances_principal_uri' => ['calendarinstances', ['principaluri', 'uri'], true], + 'uniq_calendarinstances_calendar_principal' => ['calendarinstances', ['calendarid', 'principaluri'], true], + 'uniq_calendarinstances_calendar_share' => ['calendarinstances', ['calendarid', 'share_href'], true], + 'uniq_calendarobjects_calendar_uri' => ['calendarobjects', ['calendarid', 'uri'], true], + 'uniq_calendarsubscriptions_principal_uri' => ['calendarsubscriptions', ['principaluri', 'uri'], false], + 'uniq_cards_addressbook_uri' => ['cards', ['addressbookid', 'uri'], true], + 'uniq_propertystorage_path_name' => ['propertystorage', ['path', 'name'], false], + 'uniq_schedulingobjects_principal_uri' => ['schedulingobjects', ['principaluri', 'uri'], true], + ]; + + public function getDescription(): string + { + return 'Restore DAV uniqueness constraints and add sync query indexes'; + } + + public function up(Schema $schema): void + { + foreach (self::UNIQUE_INDEXES as $name => [$table, $columns, $nullable]) { + $where = $nullable + ? ' WHERE '.implode(' AND ', array_map(static fn (string $column): string => $column.' IS NOT NULL', $columns)) + : ''; + $columnList = implode(', ', $columns); + $duplicate = $this->connection->fetchOne(sprintf( + 'SELECT 1 FROM (SELECT 1 FROM %s%s GROUP BY %s HAVING COUNT(*) > 1) duplicate_rows', + $table, + $where, + $columnList, + )); + + $this->abortIf(false !== $duplicate, sprintf( + 'Cannot create %s: duplicate (%s) values exist in %s. Resolve them and rerun the migration.', + $name, + $columnList, + $table, + )); + + $this->addSql(sprintf('CREATE UNIQUE INDEX %s ON %s (%s)', $name, $table, $columnList)); + } + + $this->addSql('CREATE INDEX idx_calendarchanges_calendar_sync ON calendarchanges (calendarid, synctoken)'); + $this->addSql('CREATE INDEX idx_addressbookchanges_book_sync ON addressbookchanges (addressbookid, synctoken)'); + } + + public function down(Schema $schema): void + { + $engine = $this->connection->getDatabasePlatform()->getName(); + + $this->dropIndex('idx_addressbookchanges_book_sync', 'addressbookchanges', $engine); + $this->dropIndex('idx_calendarchanges_calendar_sync', 'calendarchanges', $engine); + + foreach (array_reverse(self::UNIQUE_INDEXES, true) as $name => [$table]) { + $this->dropIndex($name, $table, $engine); + } + } + + private function dropIndex(string $name, string $table, string $engine): void + { + if ('mysql' === $engine) { + $this->addSql(sprintf('DROP INDEX %s ON %s', $name, $table)); + + return; + } + + $this->addSql(sprintf('DROP INDEX %s', $name)); + } +} diff --git a/migrations/Version20260720104000.php b/migrations/Version20260720104000.php new file mode 100644 index 00000000..373028b9 --- /dev/null +++ b/migrations/Version20260720104000.php @@ -0,0 +1,102 @@ + [ + 'FK_4C258FD8B26C2E9' => ['addressbookid', 'addressbooks'], + ], + 'addressbookchanges' => [ + 'FK_EB122CD58B26C2E9' => ['addressbookid', 'addressbooks'], + ], + 'calendarobjects' => [ + 'FK_E14F332CB8CB7204' => ['calendarid', 'calendars'], + ], + 'calendarinstances' => [ + 'FK_51856561B8CB7204' => ['calendarid', 'calendars'], + ], + 'calendarchanges' => [ + 'FK_737547E2B8CB7204' => ['calendarid', 'calendars'], + ], + 'groupmembers' => [ + 'FK_6F15EDAC474870EE' => ['principal_id', 'principals'], + 'FK_6F15EDAC7597D3FE' => ['member_id', 'principals'], + ], + ]; + + public function getDescription(): string + { + return 'Enforce foreign keys consistently across supported databases'; + } + + public function up(Schema $schema): void + { + foreach (self::FOREIGN_KEYS as $table => $foreignKeys) { + foreach ($foreignKeys as $name => [$column, $parentTable]) { + $orphan = $this->connection->fetchOne(sprintf( + 'SELECT 1 FROM %s child LEFT JOIN %s parent ON parent.id = child.%s WHERE parent.id IS NULL LIMIT 1', + $table, + $parentTable, + $column, + )); + + $this->abortIf(false !== $orphan, sprintf( + 'Cannot create %s: %s.%s contains values missing from %s.id. Resolve orphaned rows and rerun the migration.', + $name, + $table, + $column, + $parentTable, + )); + } + } + + $this->changeForeignKeys(true); + } + + public function down(Schema $schema): void + { + $this->changeForeignKeys(false); + } + + private function changeForeignKeys(bool $enable): void + { + $schemaManager = $this->connection->createSchemaManager(); + $currentSchema = $schemaManager->introspectSchema(); + $comparator = $schemaManager->createComparator(); + $platform = $this->connection->getDatabasePlatform(); + $remove = !$enable && 'sqlite' === $platform->getName(); + + foreach (self::FOREIGN_KEYS as $tableName => $foreignKeys) { + $currentTable = $currentSchema->getTable($tableName); + $targetTable = clone $currentTable; + + foreach ($foreignKeys as $name => [$column, $parentTable]) { + if ($targetTable->hasForeignKey($name)) { + $targetTable->removeForeignKey($name); + } + + if (!$remove) { + $targetTable->addForeignKeyConstraint( + $parentTable, + [$column], + ['id'], + $enable ? ['onDelete' => 'CASCADE'] : [], + $name, + ); + } + } + + $diff = $comparator->compareTables($currentTable, $targetTable); + foreach ($platform->getAlterTableSQL($diff) as $sql) { + $this->addSql($sql); + } + } + } +} diff --git a/src/Doctrine/SQLiteForeignKeyMiddleware.php b/src/Doctrine/SQLiteForeignKeyMiddleware.php new file mode 100644 index 00000000..a8ff96c3 --- /dev/null +++ b/src/Doctrine/SQLiteForeignKeyMiddleware.php @@ -0,0 +1,20 @@ +getDatabasePlatform() instanceof SqlitePlatform + ? (new EnableForeignKeys())->wrap($driver) + : $driver; + } +} diff --git a/src/Entity/AddressBook.php b/src/Entity/AddressBook.php index 5b1ebaa0..f71b25bc 100644 --- a/src/Entity/AddressBook.php +++ b/src/Entity/AddressBook.php @@ -10,6 +10,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'addressbooks')] +#[ORM\UniqueConstraint(name: 'uniq_addressbooks_principal_uri', columns: ['principaluri', 'uri'])] #[UniqueEntity(fields: ['principalUri', 'uri'], errorPath: 'uri', message: 'form.uri.unique')] class AddressBook { diff --git a/src/Entity/AddressBookChange.php b/src/Entity/AddressBookChange.php index 9035ce7e..c7c0effa 100644 --- a/src/Entity/AddressBookChange.php +++ b/src/Entity/AddressBookChange.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'addressbookchanges')] +#[ORM\Index(name: 'idx_addressbookchanges_book_sync', columns: ['addressbookid', 'synctoken'])] class AddressBookChange { #[ORM\Id] @@ -20,7 +21,7 @@ class AddressBookChange private $synctoken; #[ORM\ManyToOne(targetEntity: "App\Entity\AddressBook", inversedBy: 'changes')] - #[ORM\JoinColumn(name: 'addressbookid', nullable: false)] + #[ORM\JoinColumn(name: 'addressbookid', nullable: false, onDelete: 'CASCADE')] private $addressBook; #[ORM\Column(type: 'integer')] diff --git a/src/Entity/CalendarChange.php b/src/Entity/CalendarChange.php index 3036afe1..1f81eb47 100644 --- a/src/Entity/CalendarChange.php +++ b/src/Entity/CalendarChange.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'calendarchanges')] +#[ORM\Index(name: 'idx_calendarchanges_calendar_sync', columns: ['calendarid', 'synctoken'])] class CalendarChange { #[ORM\Id] @@ -20,7 +21,7 @@ class CalendarChange private $synctoken; #[ORM\ManyToOne(targetEntity: "App\Entity\Calendar", inversedBy: 'changes')] - #[ORM\JoinColumn(name: 'calendarid', nullable: false)] + #[ORM\JoinColumn(name: 'calendarid', nullable: false, onDelete: 'CASCADE')] private $calendar; #[ORM\Column(type: 'smallint')] diff --git a/src/Entity/CalendarInstance.php b/src/Entity/CalendarInstance.php index 1929272a..a4b4d368 100644 --- a/src/Entity/CalendarInstance.php +++ b/src/Entity/CalendarInstance.php @@ -10,6 +10,9 @@ #[ORM\Entity(repositoryClass: "App\Repository\CalendarInstanceRepository")] #[ORM\Table(name: 'calendarinstances')] +#[ORM\UniqueConstraint(name: 'uniq_calendarinstances_principal_uri', columns: ['principaluri', 'uri'])] +#[ORM\UniqueConstraint(name: 'uniq_calendarinstances_calendar_principal', columns: ['calendarid', 'principaluri'])] +#[ORM\UniqueConstraint(name: 'uniq_calendarinstances_calendar_share', columns: ['calendarid', 'share_href'])] #[UniqueEntity(fields: ['principalUri', 'uri'], errorPath: 'uri', message: 'form.uri.unique')] class CalendarInstance { @@ -27,7 +30,7 @@ public static function getOwnerAccesses(): array private $id; #[ORM\ManyToOne(targetEntity: "App\Entity\Calendar", cascade: ['persist'], inversedBy: 'instances')] - #[ORM\JoinColumn(name: 'calendarid', nullable: false)] + #[ORM\JoinColumn(name: 'calendarid', nullable: false, onDelete: 'CASCADE')] private $calendar; #[ORM\Column(name: 'principaluri', type: 'string', length: 255, nullable: true)] diff --git a/src/Entity/CalendarObject.php b/src/Entity/CalendarObject.php index 149d46e8..33d2c837 100644 --- a/src/Entity/CalendarObject.php +++ b/src/Entity/CalendarObject.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'calendarobjects')] +#[ORM\UniqueConstraint(name: 'uniq_calendarobjects_calendar_uri', columns: ['calendarid', 'uri'])] class CalendarObject { #[ORM\Id] @@ -23,7 +24,7 @@ class CalendarObject private $uri; #[ORM\ManyToOne(targetEntity: "App\Entity\Calendar", inversedBy: 'objects')] - #[ORM\JoinColumn(name: 'calendarid', nullable: false)] + #[ORM\JoinColumn(name: 'calendarid', nullable: false, onDelete: 'CASCADE')] private $calendar; #[ORM\Column(name: 'lastmodified', type: 'bigint', nullable: true)] diff --git a/src/Entity/CalendarSubscription.php b/src/Entity/CalendarSubscription.php index 70f5fbb1..b7068c7c 100644 --- a/src/Entity/CalendarSubscription.php +++ b/src/Entity/CalendarSubscription.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'calendarsubscriptions')] +#[ORM\UniqueConstraint(name: 'uniq_calendarsubscriptions_principal_uri', columns: ['principaluri', 'uri'])] class CalendarSubscription { #[ORM\Id] diff --git a/src/Entity/Card.php b/src/Entity/Card.php index 0b293e1f..98a566a1 100644 --- a/src/Entity/Card.php +++ b/src/Entity/Card.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'cards')] +#[ORM\UniqueConstraint(name: 'uniq_cards_addressbook_uri', columns: ['addressbookid', 'uri'])] class Card { #[ORM\Id] @@ -14,7 +15,7 @@ class Card private $id; #[ORM\ManyToOne(targetEntity: "App\Entity\AddressBook", inversedBy: 'cards')] - #[ORM\JoinColumn(name: 'addressbookid', nullable: false)] + #[ORM\JoinColumn(name: 'addressbookid', nullable: false, onDelete: 'CASCADE')] private $addressBook; /** diff --git a/src/Entity/Principal.php b/src/Entity/Principal.php index a994ab22..5b097a2e 100644 --- a/src/Entity/Principal.php +++ b/src/Entity/Principal.php @@ -46,8 +46,8 @@ class Principal #[ORM\ManyToMany(targetEntity: 'Principal')] #[ORM\JoinTable(name: 'groupmembers')] - #[ORM\JoinColumn(name: 'principal_id', referencedColumnName: 'id')] - #[ORM\InverseJoinColumn(name: 'member_id', referencedColumnName: 'id')] + #[ORM\JoinColumn(name: 'principal_id', referencedColumnName: 'id', onDelete: 'CASCADE')] + #[ORM\InverseJoinColumn(name: 'member_id', referencedColumnName: 'id', onDelete: 'CASCADE')] private $delegees; public function __construct() diff --git a/src/Entity/PropertyStorage.php b/src/Entity/PropertyStorage.php index 94744de4..80d0760d 100644 --- a/src/Entity/PropertyStorage.php +++ b/src/Entity/PropertyStorage.php @@ -6,6 +6,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'propertystorage')] +#[ORM\UniqueConstraint(name: 'uniq_propertystorage_path_name', columns: ['path', 'name'])] class PropertyStorage { #[ORM\Id] diff --git a/src/Entity/SchedulingObject.php b/src/Entity/SchedulingObject.php index b1b0fb3c..70b19dd9 100644 --- a/src/Entity/SchedulingObject.php +++ b/src/Entity/SchedulingObject.php @@ -7,6 +7,7 @@ #[ORM\Entity()] #[ORM\Table(name: 'schedulingobjects')] +#[ORM\UniqueConstraint(name: 'uniq_schedulingobjects_principal_uri', columns: ['principaluri', 'uri'])] class SchedulingObject { #[ORM\Id] diff --git a/tests/Functional/SQLiteForeignKeyTest.php b/tests/Functional/SQLiteForeignKeyTest.php new file mode 100644 index 00000000..5a9da100 --- /dev/null +++ b/tests/Functional/SQLiteForeignKeyTest.php @@ -0,0 +1,26 @@ +get(Connection::class); + if ('sqlite' !== $connection->getDatabasePlatform()->getName()) { + self::markTestSkipped('SQLite-specific connection invariant.'); + } + + self::assertSame(1, (int) $connection->fetchOne('PRAGMA foreign_keys')); + + $foreignKey = $connection->fetchAssociative("SELECT * FROM pragma_foreign_key_list('cards')"); + self::assertSame('addressbooks', $foreignKey['table']); + self::assertSame('addressbookid', $foreignKey['from']); + self::assertSame('CASCADE', $foreignKey['on_delete']); + } +}