Skip to content

Commit 44071a1

Browse files
committed
Improve clarity regarding migrations
Doctrine/dbal has been implementing more platform specific integrations (https://www.doctrine-project.org/2021/11/26/dbal-3.2.0.html). In DBAL 4, it will move away from db comments to infer types. Currently, we manage the schema using migrations. It is important to know that these migrations are targeted toward the configured mariadb version in the development environment. This matches the SURF version. Other vendors should not blindly apply these migrations if they use different database engines if the (major) version differs. Also clarify the situation regarding the `consent.deleted_at` sentinel value and Doctrine wanting to change the type of `identityprovider.consent_settings` and `identityprovider.idp_discoveries` Resolves: #1906
1 parent 7024aa8 commit 44071a1

8 files changed

Lines changed: 142 additions & 5 deletions

File tree

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ EngineBlock requires database settings, without it doctrine migrate will not fun
136136
the application must use the production settings (`--env=prod`), this could be replaced with `dev` should you run a
137137
development version._
138138

139+
_**Note**:
140+
The doctrine migrations shipped with engineblock are built against MariaDB. They should work on MySQL, if the version matches._
141+
139142
### Configure HTTP server
140143

141144
Configure a single virtual host, this should point to the `public` directory:

ci/qa/docheader.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ set -e
44
cd $(dirname $0)/../../
55

66
echo -e "\nDoc header check\n"
7-
./vendor/bin/docheader check src/ tests/ library/ --exclude-dir resources --exclude-dir languages
7+
./vendor/bin/docheader check src/ tests/ library/ migrations/ --exclude-dir resources --exclude-dir languages
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
3+
/**
4+
* Copyright 2026 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
declare(strict_types=1);
20+
21+
namespace OpenConext\EngineBlock\Doctrine\Migrations;
22+
23+
use Doctrine\DBAL\Platforms\MariaDBPlatform;
24+
use Doctrine\DBAL\Platforms\MySQLPlatform;
25+
use Doctrine\DBAL\Schema\Schema;
26+
use Doctrine\Migrations\AbstractMigration;
27+
28+
/**
29+
* Base class for all EngineBlock Doctrine migrations.
30+
*
31+
* All migrations in this project target MariaDB exclusively. The generated DDL SQL is platform-specific
32+
* and is not guaranteed to be compatible with MySQL or any other database engine.
33+
*
34+
*/
35+
abstract class AbstractEngineBlockMigration extends AbstractMigration
36+
{
37+
public function preUp(Schema $schema): void
38+
{
39+
$this->checkPlatform();
40+
}
41+
42+
public function preDown(Schema $schema): void
43+
{
44+
$this->checkPlatform();
45+
}
46+
47+
private function checkPlatform(): void
48+
{
49+
if ($this->platform instanceof MariaDBPlatform) {
50+
return;
51+
}
52+
53+
if ($this->platform instanceof MySQLPlatform) {
54+
$this->warnIf(
55+
true,
56+
sprintf(
57+
'This migration is built for MariaDB. The current database platform is MySQL ("%s"). '
58+
. 'EngineBlock migrations may contain MariaDB-specific DDL that may fail. '
59+
. 'Check manually to ensure the migrations run as expected.',
60+
get_class($this->platform),
61+
),
62+
);
63+
return;
64+
}
65+
66+
$this->abortIf(
67+
true,
68+
sprintf(
69+
'This migration is built for MariaDB only. The current database platform "%s" is not supported.',
70+
get_class($this->platform),
71+
),
72+
);
73+
}
74+
}

migrations/DoctrineMigrations/Version20260210000000.php

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,26 @@
11
<?php
22

3+
/**
4+
* Copyright 2026 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
319
declare(strict_types=1);
420

521
namespace OpenConext\EngineBlock\Doctrine\Migrations;
622

723
use Doctrine\DBAL\Schema\Schema;
8-
use Doctrine\Migrations\AbstractMigration;
924

1025
/**
1126
* Baseline migration - Creates initial database schema based on production state (6.18) as of feb 2026
@@ -14,7 +29,7 @@
1429
* It creates all required tables from scratch for new installations, and gracefully skips
1530
* execution on existing databases where tables are already present.
1631
*/
17-
final class Version20260210000000 extends AbstractMigration
32+
final class Version20260210000000 extends AbstractEngineBlockMigration
1833
{
1934
public function getDescription(): string
2035
{
@@ -23,6 +38,8 @@ public function getDescription(): string
2338

2439
public function preUp(Schema $schema): void
2540
{
41+
parent::preUp($schema);
42+
2643
$tables = $this->sm->listTableNames();
2744
$this->skipIf(
2845
in_array('sso_provider_roles_eb5', $tables, true),

migrations/DoctrineMigrations/Version20260224000000.php

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,34 @@
11
<?php
22

3+
/**
4+
* Copyright 2026 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
319
declare(strict_types=1);
420

521
namespace OpenConext\EngineBlock\Doctrine\Migrations;
622

723
use Doctrine\DBAL\Schema\Schema;
8-
use Doctrine\Migrations\AbstractMigration;
924

1025
/**
1126
* Patch/repair migration - Removes the deleted_at index from the consent table if present.
1227
*
1328
* On existing databases where the index is already absent this migration is marked as done
1429
* without executing any SQL. On databases where the index still exists it will be dropped.
1530
*/
16-
final class Version20260224000000 extends AbstractMigration
31+
final class Version20260224000000 extends AbstractEngineBlockMigration
1732
{
1833
public function getDescription(): string
1934
{
@@ -22,6 +37,8 @@ public function getDescription(): string
2237

2338
public function preUp(Schema $schema): void
2439
{
40+
parent::preUp($schema);
41+
2542
$indexes = $this->connection->createSchemaManager()->listTableIndexes('consent');
2643
$this->skipIf(
2744
!isset($indexes['deleted_at']),

src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,17 @@ class IdentityProvider extends AbstractRole
7878

7979
/**
8080
* @var ConsentSettings
81+
*
82+
* NOTE: The consent_settings and idp_discoveries columns are physically stored as LONGTEXT in the database.
83+
* This predates native JSON column support and is intentional. Running `doctrine:schema:update` will suggest:
84+
*
85+
* ALTER TABLE sso_provider_roles_eb5
86+
* CHANGE consent_settings consent_settings JSON DEFAULT NULL,
87+
* CHANGE idp_discoveries idp_discoveries JSON DEFAULT NULL;
88+
*
89+
* Do not apply this migration. Switching to native MySQL/MariaDB JSON columns requires a careful migration to work
90+
* with green/blue deployment strategies.
91+
*
8192
*/
8293
#[ORM\Column(name: 'consent_settings', type: Types::JSON, length: 16777215)]
8394
private $consentSettings;

src/OpenConext/EngineBlockBundle/Authentication/Entity/Consent.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ class Consent
6767

6868
/**
6969
* @var DateTime
70+
*
71+
* Soft-delete sentinel using MariaDB's special zero-date ('0000-00-00 00:00:00') as the "not deleted" value.
72+
*
73+
* Active (non-deleted) records have deleted_at = '0000-00-00 00:00:00'.
74+
* Soft-deleted records have deleted_at = NOW()
75+
*
76+
* Queries use `deleted_at IS NULL` to select active records. This works because MariaDB defines that
77+
* expressions involving a zero-date evaluate to NULL at the database level (see MariaDB DATETIME docs).
78+
*
79+
* IMPORTANT deleted_at cannot be made nullable because it is part of the composite primary key (hashed_user_id,
80+
* service_id, deleted_at). Primary key columns cannot be nullable in MySQL/MariaDB.
81+
*
7082
*/
7183
#[ORM\Id]
7284
#[ORM\Column(name: 'deleted_at', type: Types::DATETIME_MUTABLE, nullable: false, options: ['default' => '0000-00-00 00:00:00'])]

src/OpenConext/EngineBlockBundle/Authentication/Repository/DbalConsentRepository.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public function __construct(ManagerRegistry $registry, LoggerInterface $logger)
6262
*/
6363
public function findAllFor($userId)
6464
{
65+
// deleted_at IS NULL matches active records whose deleted_at is '0000-00-00 00:00:00'.
66+
// See Consent::$deletedAt for full context.
6567
$sql = '
6668
SELECT
6769
service_id
@@ -126,6 +128,7 @@ public function deleteAllFor($userId)
126128
*/
127129
public function deleteOneFor(string $userId, string $serviceProviderEntityId): bool
128130
{
131+
// deleted_at IS NULL matches active records. See Consent::$deletedAt for full context.
129132
$sql = '
130133
UPDATE
131134
consent

0 commit comments

Comments
 (0)