Skip to content

Commit 02847b8

Browse files
tchapitchapi
authored
Migration fixes (#286)
1 parent 1fd5d47 commit 02847b8

15 files changed

Lines changed: 342 additions & 13 deletions

‎.github/workflows/ci.yml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,16 @@ jobs:
200200
php bin/console doctrine:database:create --if-not-exists --env=test
201201
php bin/console doctrine:migrations:migrate --no-interaction --env=test
202202
php bin/console doctrine:schema:validate --env=test
203+
# An empty schema only proves the DDL is valid. Seed the kind of row a DAV client
204+
# creates (optional columns left NULL) so the rollback is exercised against data too.
205+
php bin/console dbal:run-sql "INSERT INTO addressbooks (principaluri, uri, synctoken) VALUES ('principals/ci', 'ci-rollback-probe', 1)" --env=test
206+
# Full chain check: roll every migration back down, then all the way up again.
207+
# Nothing else ever runs the down() methods, so a broken rollback (wrong column
208+
# name, duplicated ALTER clause, invalid cast) stays invisible until an operator
209+
# actually needs to roll back.
210+
php bin/console doctrine:migrations:migrate first --no-interaction --env=test
211+
php bin/console doctrine:migrations:migrate --no-interaction --env=test
212+
php bin/console doctrine:schema:validate --env=test
203213
204214
- name: Run migrations (PostgreSQL)
205215
if: matrix.database == 'postgresql'
@@ -209,6 +219,16 @@ jobs:
209219
php bin/console doctrine:database:create --if-not-exists --env=test
210220
php bin/console doctrine:migrations:migrate --no-interaction --env=test
211221
php bin/console doctrine:schema:validate --skip-sync --env=test
222+
# An empty schema only proves the DDL is valid. Seed the kind of row a DAV client
223+
# creates (optional columns left NULL) so the rollback is exercised against data too.
224+
php bin/console dbal:run-sql "INSERT INTO addressbooks (id, principaluri, uri, synctoken) VALUES (nextval('addressbooks_id_seq'), 'principals/ci', 'ci-rollback-probe', 1)" --env=test
225+
# Full chain check: roll every migration back down, then all the way up again.
226+
# Nothing else ever runs the down() methods, so a broken rollback (wrong column
227+
# name, duplicated ALTER clause, invalid cast) stays invisible until an operator
228+
# actually needs to roll back.
229+
php bin/console doctrine:migrations:migrate first --no-interaction --env=test
230+
php bin/console doctrine:migrations:migrate --no-interaction --env=test
231+
php bin/console doctrine:schema:validate --skip-sync --env=test
212232
213233
- name: Run migrations (SQLite)
214234
if: matrix.database == 'sqlite'
@@ -217,6 +237,16 @@ jobs:
217237
run: |
218238
php bin/console doctrine:migrations:migrate --no-interaction --env=test
219239
php bin/console doctrine:schema:validate --skip-sync --env=test
240+
# An empty schema only proves the DDL is valid. Seed the kind of row a DAV client
241+
# creates (optional columns left NULL) so the rollback is exercised against data too.
242+
php bin/console dbal:run-sql "INSERT INTO addressbooks (principaluri, uri, synctoken) VALUES ('principals/ci', 'ci-rollback-probe', 1)" --env=test
243+
# Full chain check: roll every migration back down, then all the way up again.
244+
# Nothing else ever runs the down() methods, so a broken rollback (wrong column
245+
# name, duplicated ALTER clause, invalid cast) stays invisible until an operator
246+
# actually needs to roll back.
247+
php bin/console doctrine:migrations:migrate first --no-interaction --env=test
248+
php bin/console doctrine:migrations:migrate --no-interaction --env=test
249+
php bin/console doctrine:schema:validate --skip-sync --env=test
220250
221251
smoke-test:
222252
name: Application Smoke Test

‎migrations/Version20191202091507.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ public function down(Schema $schema): void
2828
{
2929
$this->skipIf('mysql' !== $this->connection->getDatabasePlatform()->getName(), 'This migration is specific to \'mysql\'. Skipping it is fine.');
3030

31-
$this->addSql('ALTER TABLE calendarinstances CHANGE access access SMALLINT NOT NULL, CHANGE share_invitestatus share_invitestatus INT NOT NULL, CHANGE timezone timezone LONGTEXT DEFAULT NULL, CHANGE timezone timezone VARCHAR(255) CHARACTER SET utf8mb4 DEFAULT \'NULL\' COLLATE `utf8mb4_unicode_ci`');
31+
// NB: `timezone` is restored to its original VARCHAR(255); a VTIMEZONE blob longer than
32+
// that would be rejected by MySQL rather than silently truncated.
33+
$this->addSql('ALTER TABLE calendarinstances CHANGE access access SMALLINT NOT NULL, CHANGE share_invitestatus share_invitestatus INT NOT NULL, CHANGE timezone timezone VARCHAR(255) DEFAULT NULL');
3234
}
3335
}

‎migrations/Version20191203111729.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ public function down(Schema $schema): void
2828
{
2929
$this->skipIf('mysql' !== $this->connection->getDatabasePlatform()->getName(), 'This migration is specific to \'mysql\'. Skipping it is fine.');
3030

31-
$this->addSql('ALTER TABLE addressbooks CHANGE description description LONGTEXT CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci`');
31+
// Since up() made the column nullable, address books created in the meantime may have
32+
// no description at all; they would violate the restored NOT NULL.
33+
$this->addSql("UPDATE addressbooks SET description = '' WHERE description IS NULL");
34+
$this->addSql('ALTER TABLE addressbooks CHANGE description description LONGTEXT NOT NULL');
3235
}
3336
}

‎migrations/Version20231001214112.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ public function down(Schema $schema): void
3030
{
3131
$this->skipIf('postgresql' !== $this->connection->getDatabasePlatform()->getName(), 'This migration is specific to \'postgresql\'. Skipping it is fine.');
3232

33-
$this->addSql("ALTER TABLE calendarobjects ALTER COLUMN calendardata TYPE BYTEA DEFAULT NULL USING convert_from(calendardata, 'utf8')");
34-
$this->addSql("ALTER TABLE cards ALTER COLUMN carddata TYPE BYTEA DEFAULT NULL USING convert_from(carddata, 'utf8')");
35-
$this->addSql("ALTER TABLE schedulingobjects ALTER COLUMN calendardata TYPE BYTEA DEFAULT NULL USING convert_from(calendardata, 'utf8')");
33+
$this->addSql("ALTER TABLE calendarobjects ALTER COLUMN calendardata TYPE BYTEA USING convert_to(calendardata, 'utf8')");
34+
$this->addSql("ALTER TABLE cards ALTER COLUMN carddata TYPE BYTEA USING convert_to(carddata, 'utf8')");
35+
$this->addSql("ALTER TABLE schedulingobjects ALTER COLUMN calendardata TYPE BYTEA USING convert_to(calendardata, 'utf8')");
3636
}
3737
}

‎migrations/Version20260131161930.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ public function down(Schema $schema): void
4444

4545
// Revert public = true back to ACCESS_PUBLIC (10)
4646
if ('postgresql' === $engine) {
47-
$this->addSql('UPDATE calendarinstances SET access = 10 WHERE is_public = TRUE');
47+
$this->addSql('UPDATE calendarinstances SET access = 10 WHERE public = TRUE');
4848
} else {
49-
$this->addSql('UPDATE calendarinstances SET access = 10 WHERE is_public = 1');
49+
$this->addSql('UPDATE calendarinstances SET access = 10 WHERE public = 1');
5050
}
5151

5252
if ('mysql' === $engine) {

‎migrations/Version20260908210000.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DoctrineMigrations;
6+
7+
use Doctrine\DBAL\Schema\Schema;
8+
use Doctrine\Migrations\AbstractMigration;
9+
10+
/**
11+
* Give calendarsubscriptions.calendarorder a default value.
12+
*/
13+
final class Version20260908210000 extends AbstractMigration
14+
{
15+
public function getDescription(): string
16+
{
17+
return 'Default calendarsubscriptions.calendarorder to 0, as sabre/dav omits the column when a client subscribes without a calendar-order';
18+
}
19+
20+
public function up(Schema $schema): void
21+
{
22+
$engine = $this->connection->getDatabasePlatform()->getName();
23+
24+
// \Sabre\CalDAV\Backend\PDO::createSubscription() only lists `calendarorder` in its
25+
// INSERT when the client sent {http://apple.com/ns/ical/}calendar-order. Without a
26+
// default, subscribing to a feed then fails with a NOT NULL violation (HTTP 500).
27+
if ('mysql' === $engine) {
28+
$this->addSql('ALTER TABLE calendarsubscriptions CHANGE calendarorder calendarorder INT DEFAULT 0 NOT NULL');
29+
} elseif ('postgresql' === $engine) {
30+
$this->addSql('ALTER TABLE calendarsubscriptions ALTER COLUMN calendarorder SET DEFAULT 0');
31+
} elseif ('sqlite' === $engine) {
32+
// SQLite cannot alter a column in place: add the replacement, copy, swap, drop.
33+
$this->addSql('ALTER TABLE calendarsubscriptions ADD COLUMN new_calendarorder INTEGER DEFAULT 0 NOT NULL');
34+
$this->addSql('UPDATE calendarsubscriptions SET new_calendarorder = calendarorder');
35+
$this->addSql('ALTER TABLE calendarsubscriptions RENAME COLUMN calendarorder TO old_calendarorder');
36+
$this->addSql('ALTER TABLE calendarsubscriptions RENAME COLUMN new_calendarorder TO calendarorder');
37+
$this->addSql('ALTER TABLE calendarsubscriptions DROP COLUMN old_calendarorder');
38+
}
39+
}
40+
41+
public function down(Schema $schema): void
42+
{
43+
$engine = $this->connection->getDatabasePlatform()->getName();
44+
45+
if ('mysql' === $engine) {
46+
$this->addSql('ALTER TABLE calendarsubscriptions CHANGE calendarorder calendarorder INT NOT NULL');
47+
} elseif ('postgresql' === $engine) {
48+
$this->addSql('ALTER TABLE calendarsubscriptions ALTER COLUMN calendarorder DROP DEFAULT');
49+
} elseif ('sqlite' === $engine) {
50+
// SQLite refuses to ADD a NOT NULL column without a default, so the only way back
51+
// is to rebuild the table with its original definition.
52+
$this->addSql('CREATE TABLE calendarsubscriptions_old (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, uri VARCHAR(255) NOT NULL, principaluri VARCHAR(255) NOT NULL, source CLOB DEFAULT NULL, displayname VARCHAR(255) DEFAULT NULL, refreshrate VARCHAR(10) DEFAULT NULL, calendarorder INTEGER NOT NULL, calendarcolor VARCHAR(10) DEFAULT NULL, striptodos SMALLINT DEFAULT NULL, stripalarms SMALLINT DEFAULT NULL, stripattachments SMALLINT DEFAULT NULL, lastmodified INTEGER DEFAULT NULL)');
53+
$this->addSql('INSERT INTO calendarsubscriptions_old (id, uri, principaluri, source, displayname, refreshrate, calendarorder, calendarcolor, striptodos, stripalarms, stripattachments, lastmodified) SELECT id, uri, principaluri, source, displayname, refreshrate, calendarorder, calendarcolor, striptodos, stripalarms, stripattachments, lastmodified FROM calendarsubscriptions');
54+
$this->addSql('DROP TABLE calendarsubscriptions');
55+
$this->addSql('ALTER TABLE calendarsubscriptions_old RENAME TO calendarsubscriptions');
56+
}
57+
}
58+
}

‎migrations/Version20260908220000.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DoctrineMigrations;
6+
7+
use Doctrine\DBAL\Schema\Schema;
8+
use Doctrine\Migrations\AbstractMigration;
9+
10+
/**
11+
* Allow addressbooks.displayname to be null.
12+
*/
13+
final class Version20260908220000 extends AbstractMigration
14+
{
15+
public function getDescription(): string
16+
{
17+
return 'Allow addressbooks.displayname to be null, as a display name is optional when a client creates an address book';
18+
}
19+
20+
public function up(Schema $schema): void
21+
{
22+
$engine = $this->connection->getDatabasePlatform()->getName();
23+
24+
// A display name is optional in CardDAV: \Sabre\CardDAV\Backend\PDO::createAddressBook()
25+
// binds NULL when the client's MKCOL carries no {DAV:}displayname, and updateAddressBook()
26+
// does the same when a PROPPATCH removes it. With a NOT NULL column both fail (HTTP 500).
27+
if ('mysql' === $engine) {
28+
$this->addSql('ALTER TABLE addressbooks CHANGE displayname displayname VARCHAR(255) DEFAULT NULL');
29+
} elseif ('postgresql' === $engine) {
30+
$this->addSql('ALTER TABLE addressbooks ALTER COLUMN displayname DROP NOT NULL');
31+
} elseif ('sqlite' === $engine) {
32+
// SQLite cannot alter a column in place: add the replacement, copy, swap, drop.
33+
$this->addSql('ALTER TABLE addressbooks ADD COLUMN new_displayname VARCHAR(255) DEFAULT NULL');
34+
$this->addSql('UPDATE addressbooks SET new_displayname = displayname');
35+
$this->addSql('ALTER TABLE addressbooks RENAME COLUMN displayname TO old_displayname');
36+
$this->addSql('ALTER TABLE addressbooks RENAME COLUMN new_displayname TO displayname');
37+
$this->addSql('ALTER TABLE addressbooks DROP COLUMN old_displayname');
38+
}
39+
}
40+
41+
public function down(Schema $schema): void
42+
{
43+
$engine = $this->connection->getDatabasePlatform()->getName();
44+
45+
// Address books created without a display name would violate the restored NOT NULL,
46+
// so fall back to their uri rather than losing the row.
47+
$this->addSql('UPDATE addressbooks SET displayname = uri WHERE displayname IS NULL');
48+
49+
if ('mysql' === $engine) {
50+
$this->addSql('ALTER TABLE addressbooks CHANGE displayname displayname VARCHAR(255) NOT NULL');
51+
} elseif ('postgresql' === $engine) {
52+
$this->addSql('ALTER TABLE addressbooks ALTER COLUMN displayname SET NOT NULL');
53+
} elseif ('sqlite' === $engine) {
54+
// SQLite refuses to ADD a NOT NULL column without a default, so rebuild the table.
55+
$this->addSql('CREATE TABLE addressbooks_old (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, principaluri VARCHAR(255) NOT NULL, displayname VARCHAR(255) NOT NULL, uri VARCHAR(255) NOT NULL, description CLOB DEFAULT NULL, synctoken VARCHAR(255) NOT NULL, included_in_birthday_calendar INTEGER DEFAULT 0)');
56+
$this->addSql('INSERT INTO addressbooks_old (id, principaluri, displayname, uri, description, synctoken, included_in_birthday_calendar) SELECT id, principaluri, displayname, uri, description, synctoken, included_in_birthday_calendar FROM addressbooks');
57+
$this->addSql('DROP TABLE addressbooks');
58+
$this->addSql('ALTER TABLE addressbooks_old RENAME TO addressbooks');
59+
}
60+
}
61+
}

‎src/Entity/AddressBook.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class AddressBook
2121
#[ORM\Column(name: 'principaluri', type: 'string', length: 255)]
2222
private $principalUri;
2323

24-
#[ORM\Column(name: 'displayname', type: 'string', length: 255)]
24+
#[ORM\Column(name: 'displayname', type: 'string', length: 255, nullable: true)]
2525
private $displayName;
2626

2727
#[ORM\Column(type: 'string', length: 255)]
@@ -73,7 +73,7 @@ public function getDisplayName(): ?string
7373
return $this->displayName;
7474
}
7575

76-
public function setDisplayName(string $displayName): self
76+
public function setDisplayName(?string $displayName): self
7777
{
7878
$this->displayName = $displayName;
7979

‎src/Entity/CalendarSubscription.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ class CalendarSubscription
2828
#[ORM\Column(name: 'refreshrate', type: 'string', length: 10, nullable: true)]
2929
private $refreshRate;
3030

31-
#[ORM\Column(name: 'calendarorder', type: 'integer')]
32-
private $calendarOrder;
31+
#[ORM\Column(name: 'calendarorder', type: 'integer', options: ['default' => 0])]
32+
private $calendarOrder = 0;
3333

3434
#[ORM\Column(name: 'calendarcolor', type: 'string', length: 10, nullable: true)]
3535
private $calendarColor;

‎src/Form/AddressBookType.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
2424
->add('displayName', TextType::class, [
2525
'label' => 'form.displayName',
2626
'help' => 'form.name.help.carddav',
27+
// Optional in CardDAV: clients may create an address book without one
28+
'required' => false,
2729
])
2830
->add('includedInBirthdayCalendar', ChoiceType::class, [
2931
'label' => 'form.includedInBirthdayCalendar',

0 commit comments

Comments
 (0)