From 495b5ffc3f49534f33f5e24b04e87a66c8673f34 Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Thu, 26 Feb 2026 15:48:14 +0100 Subject: [PATCH] Prevent migration to json Prior to this change, doctrine wants to change the longtext types of consent_settings and idp_discoveries to `json`. This has some implications, as at a db level, as it requires a migration. This migration is not strictly needed. Ensure no database migrations are applied by configuring doctrine to use the old legacy mappings, until we have reason to upgrade in a coordinated operation. Even with this change, doctrine wants to remove the comments. But we can safely apply this in the next release, without causing any big migrations. --- config/packages/doctrine.yaml | 1 + .../Metadata/Entity/IdentityProvider.php | 5 +- .../Doctrine/Type/LegacyJsonType.php | 90 ++++++++++ .../Doctrine/Type/MetadataCoinType.php | 6 +- .../Doctrine/Type/MetadataMduiType.php | 6 +- .../Doctrine/Type/LegacyJsonTypeTest.php | 170 ++++++++++++++++++ 6 files changed, 268 insertions(+), 10 deletions(-) create mode 100644 src/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonType.php create mode 100644 tests/unit/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonTypeTest.php diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 1425a8eea9..a2e1c5563d 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -26,6 +26,7 @@ doctrine: array: OpenConext\EngineBlockBundle\Doctrine\Type\SerializedArrayType object: OpenConext\EngineBlockBundle\Doctrine\Type\SerializedObjectType + legacy_json: OpenConext\EngineBlockBundle\Doctrine\Type\LegacyJsonType orm: auto_generate_proxy_classes: "%kernel.debug%" diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php index 92524a2f1e..504562d188 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php @@ -33,6 +33,7 @@ use OpenConext\EngineBlock\Metadata\Service; use OpenConext\EngineBlock\Metadata\ShibMdScope; use OpenConext\EngineBlock\Metadata\StepupConnections; +use OpenConext\EngineBlockBundle\Doctrine\Type\LegacyJsonType; use OpenConext\EngineBlockBundle\Doctrine\Type\SerializedArrayType; use RobRichards\XMLSecLibs\XMLSecurityKey; use SAML2\Constants; @@ -90,7 +91,7 @@ class IdentityProvider extends AbstractRole * with green/blue deployment strategies. * */ - #[ORM\Column(name: 'consent_settings', type: Types::JSON, length: 16777215)] + #[ORM\Column(name: 'consent_settings', type: LegacyJsonType::NAME)] private $consentSettings; /** @@ -102,7 +103,7 @@ class IdentityProvider extends AbstractRole /** * @var array */ - #[ORM\Column(name: 'idp_discoveries', type: Types::JSON)] + #[ORM\Column(name: 'idp_discoveries', type: LegacyJsonType::NAME)] private $discoveries; /** diff --git a/src/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonType.php b/src/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonType.php new file mode 100644 index 0000000000..d63be8065d --- /dev/null +++ b/src/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonType.php @@ -0,0 +1,90 @@ +getClobTypeDeclarationSQL( + ['length' => null] + ); + } + + public function convertToDatabaseValue(mixed $value, AbstractPlatform $platform): ?string + { + if ($value === null) { + return null; + } + + try { + return json_encode($value, JSON_THROW_ON_ERROR | JSON_PRESERVE_ZERO_FRACTION); + } catch (JsonException $e) { + throw SerializationFailed::new($value, 'json', $e->getMessage(), $e); + } + } + + public function convertToPHPValue(mixed $value, AbstractPlatform $platform): mixed + { + if ($value === null || $value === '') { + return null; + } + + if (is_resource($value)) { + $value = stream_get_contents($value); + } + + try { + return json_decode($value, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + throw ValueNotConvertible::new($value, self::NAME, $e->getMessage(), $e); + } + } + + public function getName(): string + { + return self::NAME; + } +} diff --git a/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataCoinType.php b/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataCoinType.php index e0dbbc0197..61ace806ee 100644 --- a/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataCoinType.php +++ b/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataCoinType.php @@ -30,10 +30,8 @@ class MetadataCoinType extends Type public function getSQLDeclaration(array $column, AbstractPlatform $platform): string { - // We want a `TEXT` field declaration in our column, the `LONGTEXT` default causes issues when running the - // DBMS in strict mode. - $column['length'] = 65535; - return $platform->getJsonTypeDeclarationSQL($column); + $column['length'] = null; // Ensure a longtext + return $platform->getClobTypeDeclarationSQL($column); } public function convertToDatabaseValue($value, AbstractPlatform $platform): mixed diff --git a/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataMduiType.php b/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataMduiType.php index ed460e95e5..f050910e42 100644 --- a/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataMduiType.php +++ b/src/OpenConext/EngineBlockBundle/Doctrine/Type/MetadataMduiType.php @@ -30,10 +30,8 @@ class MetadataMduiType extends Type public function getSQLDeclaration(array $column, AbstractPlatform $platform): string { - // We want a `TEXT` field declaration in our column, the `LONGTEXT` default causes issues when running the - // DBMS in strict mode. - $column['length'] = 65535; - return $platform->getJsonTypeDeclarationSQL($column); + $column['length'] = null; // Ensure a longtext + return $platform->getClobTypeDeclarationSQL($column); } public function convertToDatabaseValue($value, AbstractPlatform $platform): mixed diff --git a/tests/unit/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonTypeTest.php b/tests/unit/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonTypeTest.php new file mode 100644 index 0000000000..dfaec0e0c8 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlockBundle/Doctrine/Type/LegacyJsonTypeTest.php @@ -0,0 +1,170 @@ +platform = new MySQLPlatform(); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function null_converts_to_null_in_database(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $result = $type->convertToDatabaseValue(null, $this->platform); + + $this->assertNull($result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function a_value_round_trips_correctly(): void + { + $type = Type::getType(LegacyJsonType::NAME); + $input = ['foo' => 'bar', 'baz' => [1, 2, 3]]; + + $dbValue = $type->convertToDatabaseValue($input, $this->platform); + $phpValue = $type->convertToPHPValue($dbValue, $this->platform); + + $this->assertSame($input, $phpValue); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function a_scalar_value_is_encoded_to_json(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $result = $type->convertToDatabaseValue('hello', $this->platform); + + $this->assertSame('"hello"', $result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function a_float_with_zero_fraction_preserves_decimal_point(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + // JSON_PRESERVE_ZERO_FRACTION ensures 1.0 encodes as "1.0", not "1" + $result = $type->convertToDatabaseValue(1.0, $this->platform); + + $this->assertSame('1.0', $result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function an_unserializable_value_throws_a_conversion_exception(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + // A recursive reference cannot be JSON-encoded and triggers SerializationFailed + $recursive = []; + $recursive[] = &$recursive; + + $this->expectException(ConversionException::class); + $type->convertToDatabaseValue($recursive, $this->platform); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function null_from_database_converts_to_null(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $result = $type->convertToPHPValue(null, $this->platform); + + $this->assertNull($result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function empty_string_from_database_converts_to_null(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $result = $type->convertToPHPValue('', $this->platform); + + $this->assertNull($result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function valid_json_from_database_converts_to_php_value(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $result = $type->convertToPHPValue('{"key":"value","num":42}', $this->platform); + + $this->assertSame(['key' => 'value', 'num' => 42], $result); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function invalid_json_from_database_throws_a_conversion_exception(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $this->expectException(ConversionException::class); + $type->convertToPHPValue('this is not valid json }{', $this->platform); + } + + #[Group('EngineBlockBundle')] + #[Group('Doctrine')] + #[Test] + public function sql_declaration_uses_clob_not_native_json(): void + { + $type = Type::getType(LegacyJsonType::NAME); + + $declaration = $type->getSQLDeclaration([], $this->platform); + + // Must produce longtext, not the native MySQL JSON column type + $this->assertSame('LONGTEXT', $declaration); + $this->assertStringNotContainsStringIgnoringCase('json', $declaration); + } +}