Skip to content

Commit 8237047

Browse files
johanibjohanib
authored
Merge pull request #1917 from OpenConext/feature/ensure_no_schema_changes
Tame doctrine migrations / determine which we want to apply.
2 parents 3889a6f + 495b5ff commit 8237047

File tree

6 files changed

+268
-10
lines changed

6 files changed

+268
-10
lines changed

‎config/packages/doctrine.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ doctrine:
2626

2727
array: OpenConext\EngineBlockBundle\Doctrine\Type\SerializedArrayType
2828
object: OpenConext\EngineBlockBundle\Doctrine\Type\SerializedObjectType
29+
legacy_json: OpenConext\EngineBlockBundle\Doctrine\Type\LegacyJsonType
2930

3031
orm:
3132
auto_generate_proxy_classes: "%kernel.debug%"

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use OpenConext\EngineBlock\Metadata\Service;
3434
use OpenConext\EngineBlock\Metadata\ShibMdScope;
3535
use OpenConext\EngineBlock\Metadata\StepupConnections;
36+
use OpenConext\EngineBlockBundle\Doctrine\Type\LegacyJsonType;
3637
use OpenConext\EngineBlockBundle\Doctrine\Type\SerializedArrayType;
3738
use RobRichards\XMLSecLibs\XMLSecurityKey;
3839
use SAML2\Constants;
@@ -90,7 +91,7 @@ class IdentityProvider extends AbstractRole
9091
* with green/blue deployment strategies.
9192
*
9293
*/
93-
#[ORM\Column(name: 'consent_settings', type: Types::JSON, length: 16777215)]
94+
#[ORM\Column(name: 'consent_settings', type: LegacyJsonType::NAME)]
9495
private $consentSettings;
9596

9697
/**
@@ -102,7 +103,7 @@ class IdentityProvider extends AbstractRole
102103
/**
103104
* @var array<int, Discovery>
104105
*/
105-
#[ORM\Column(name: 'idp_discoveries', type: Types::JSON)]
106+
#[ORM\Column(name: 'idp_discoveries', type: LegacyJsonType::NAME)]
106107
private $discoveries;
107108

108109
/**

‎src/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonType.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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+
namespace OpenConext\EngineBlockBundle\Doctrine\Type;
20+
21+
use Doctrine\DBAL\Platforms\AbstractPlatform;
22+
use Doctrine\DBAL\Types\Exception\SerializationFailed;
23+
use Doctrine\DBAL\Types\Exception\ValueNotConvertible;
24+
use Doctrine\DBAL\Types\Type;
25+
use JsonException;
26+
27+
use function is_resource;
28+
use function json_decode;
29+
use function json_encode;
30+
use function stream_get_contents;
31+
32+
use const JSON_PRESERVE_ZERO_FRACTION;
33+
use const JSON_THROW_ON_ERROR;
34+
35+
/**
36+
* This type stores JSON data in a longtext column, deliberately avoiding migration to a native MySQL JSON column type.
37+
* The native JSON column type (declared via getJsonTypeDeclarationSQL) was introduced when upgrading to doctrine/dbal:^4,
38+
* but migrating existing longtext columns to JSON columns is not desirable at this time.
39+
*
40+
* Use this type instead of Doctrine\DBAL\Types\Types::JSON for columns that should remain as longtext.
41+
*
42+
* Note: Doctrine column definition data passed to the SQL declaration are not taken into account to
43+
* ensure a consistent schema.
44+
*/
45+
class LegacyJsonType extends Type
46+
{
47+
public const NAME = 'legacy_json';
48+
49+
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
50+
{
51+
return $platform->getClobTypeDeclarationSQL(
52+
['length' => null]
53+
);
54+
}
55+
56+
public function convertToDatabaseValue(mixed $value, AbstractPlatform $platform): ?string
57+
{
58+
if ($value === null) {
59+
return null;
60+
}
61+
62+
try {
63+
return json_encode($value, JSON_THROW_ON_ERROR | JSON_PRESERVE_ZERO_FRACTION);
64+
} catch (JsonException $e) {
65+
throw SerializationFailed::new($value, 'json', $e->getMessage(), $e);
66+
}
67+
}
68+
69+
public function convertToPHPValue(mixed $value, AbstractPlatform $platform): mixed
70+
{
71+
if ($value === null || $value === '') {
72+
return null;
73+
}
74+
75+
if (is_resource($value)) {
76+
$value = stream_get_contents($value);
77+
}
78+
79+
try {
80+
return json_decode($value, true, 512, JSON_THROW_ON_ERROR);
81+
} catch (JsonException $e) {
82+
throw ValueNotConvertible::new($value, self::NAME, $e->getMessage(), $e);
83+
}
84+
}
85+
86+
public function getName(): string
87+
{
88+
return self::NAME;
89+
}
90+
}

‎src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataCoinType.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ class MetadataCoinType extends Type
3030

3131
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
3232
{
33-
// We want a `TEXT` field declaration in our column, the `LONGTEXT` default causes issues when running the
34-
// DBMS in strict mode.
35-
$column['length'] = 65535;
36-
return $platform->getJsonTypeDeclarationSQL($column);
33+
$column['length'] = null; // Ensure a longtext
34+
return $platform->getClobTypeDeclarationSQL($column);
3735
}
3836

3937
public function convertToDatabaseValue($value, AbstractPlatform $platform): mixed

‎src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataMduiType.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ class MetadataMduiType extends Type
3030

3131
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
3232
{
33-
// We want a `TEXT` field declaration in our column, the `LONGTEXT` default causes issues when running the
34-
// DBMS in strict mode.
35-
$column['length'] = 65535;
36-
return $platform->getJsonTypeDeclarationSQL($column);
33+
$column['length'] = null; // Ensure a longtext
34+
return $platform->getClobTypeDeclarationSQL($column);
3735
}
3836

3937
public function convertToDatabaseValue($value, AbstractPlatform $platform): mixed

‎tests/unit/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonTypeTest.php‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
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+
namespace OpenConext\EngineBlockBundle\Doctrine\Type;
20+
21+
use Doctrine\DBAL\Platforms\MySQLPlatform;
22+
use Doctrine\DBAL\Types\ConversionException;
23+
use Doctrine\DBAL\Types\Type;
24+
use PHPUnit\Framework\Attributes\Group;
25+
use PHPUnit\Framework\Attributes\Test;
26+
use PHPUnit\Framework\TestCase;
27+
28+
class LegacyJsonTypeTest extends TestCase
29+
{
30+
private MySQLPlatform $platform;
31+
32+
public static function setUpBeforeClass(): void
33+
{
34+
if (!Type::hasType(LegacyJsonType::NAME)) {
35+
Type::addType(LegacyJsonType::NAME, LegacyJsonType::class);
36+
}
37+
}
38+
39+
public function setUp(): void
40+
{
41+
$this->platform = new MySQLPlatform();
42+
}
43+
44+
#[Group('EngineBlockBundle')]
45+
#[Group('Doctrine')]
46+
#[Test]
47+
public function null_converts_to_null_in_database(): void
48+
{
49+
$type = Type::getType(LegacyJsonType::NAME);
50+
51+
$result = $type->convertToDatabaseValue(null, $this->platform);
52+
53+
$this->assertNull($result);
54+
}
55+
56+
#[Group('EngineBlockBundle')]
57+
#[Group('Doctrine')]
58+
#[Test]
59+
public function a_value_round_trips_correctly(): void
60+
{
61+
$type = Type::getType(LegacyJsonType::NAME);
62+
$input = ['foo' => 'bar', 'baz' => [1, 2, 3]];
63+
64+
$dbValue = $type->convertToDatabaseValue($input, $this->platform);
65+
$phpValue = $type->convertToPHPValue($dbValue, $this->platform);
66+
67+
$this->assertSame($input, $phpValue);
68+
}
69+
70+
#[Group('EngineBlockBundle')]
71+
#[Group('Doctrine')]
72+
#[Test]
73+
public function a_scalar_value_is_encoded_to_json(): void
74+
{
75+
$type = Type::getType(LegacyJsonType::NAME);
76+
77+
$result = $type->convertToDatabaseValue('hello', $this->platform);
78+
79+
$this->assertSame('"hello"', $result);
80+
}
81+
82+
#[Group('EngineBlockBundle')]
83+
#[Group('Doctrine')]
84+
#[Test]
85+
public function a_float_with_zero_fraction_preserves_decimal_point(): void
86+
{
87+
$type = Type::getType(LegacyJsonType::NAME);
88+
89+
// JSON_PRESERVE_ZERO_FRACTION ensures 1.0 encodes as "1.0", not "1"
90+
$result = $type->convertToDatabaseValue(1.0, $this->platform);
91+
92+
$this->assertSame('1.0', $result);
93+
}
94+
95+
#[Group('EngineBlockBundle')]
96+
#[Group('Doctrine')]
97+
#[Test]
98+
public function an_unserializable_value_throws_a_conversion_exception(): void
99+
{
100+
$type = Type::getType(LegacyJsonType::NAME);
101+
102+
// A recursive reference cannot be JSON-encoded and triggers SerializationFailed
103+
$recursive = [];
104+
$recursive[] = &$recursive;
105+
106+
$this->expectException(ConversionException::class);
107+
$type->convertToDatabaseValue($recursive, $this->platform);
108+
}
109+
110+
#[Group('EngineBlockBundle')]
111+
#[Group('Doctrine')]
112+
#[Test]
113+
public function null_from_database_converts_to_null(): void
114+
{
115+
$type = Type::getType(LegacyJsonType::NAME);
116+
117+
$result = $type->convertToPHPValue(null, $this->platform);
118+
119+
$this->assertNull($result);
120+
}
121+
122+
#[Group('EngineBlockBundle')]
123+
#[Group('Doctrine')]
124+
#[Test]
125+
public function empty_string_from_database_converts_to_null(): void
126+
{
127+
$type = Type::getType(LegacyJsonType::NAME);
128+
129+
$result = $type->convertToPHPValue('', $this->platform);
130+
131+
$this->assertNull($result);
132+
}
133+
134+
#[Group('EngineBlockBundle')]
135+
#[Group('Doctrine')]
136+
#[Test]
137+
public function valid_json_from_database_converts_to_php_value(): void
138+
{
139+
$type = Type::getType(LegacyJsonType::NAME);
140+
141+
$result = $type->convertToPHPValue('{"key":"value","num":42}', $this->platform);
142+
143+
$this->assertSame(['key' => 'value', 'num' => 42], $result);
144+
}
145+
146+
#[Group('EngineBlockBundle')]
147+
#[Group('Doctrine')]
148+
#[Test]
149+
public function invalid_json_from_database_throws_a_conversion_exception(): void
150+
{
151+
$type = Type::getType(LegacyJsonType::NAME);
152+
153+
$this->expectException(ConversionException::class);
154+
$type->convertToPHPValue('this is not valid json }{', $this->platform);
155+
}
156+
157+
#[Group('EngineBlockBundle')]
158+
#[Group('Doctrine')]
159+
#[Test]
160+
public function sql_declaration_uses_clob_not_native_json(): void
161+
{
162+
$type = Type::getType(LegacyJsonType::NAME);
163+
164+
$declaration = $type->getSQLDeclaration([], $this->platform);
165+
166+
// Must produce longtext, not the native MySQL JSON column type
167+
$this->assertSame('LONGTEXT', $declaration);
168+
$this->assertStringNotContainsStringIgnoringCase('json', $declaration);
169+
}
170+
}

0 commit comments

Comments
 (0)