From eed0ceab7ebf5d83bb161346c0065f29ea507685 Mon Sep 17 00:00:00 2001 From: Thorsten Reiter <34274688+reithor@users.noreply.github.com> Date: Mon, 25 Nov 2024 12:17:30 +0100 Subject: [PATCH] IBX-8566: Fixed postgres language limit (#454) For more details see https://issues.ibexa.co/browse/IBX-8566 and https://github.com/ibexa/core/pull/454 Key changes: * Implemented PostgresqlGateway * Implemented SharedGateway\AbstractGateway to avoid code duplication * [Tests] Added integration test coverage for the use case * [Tests] Aligned tests with the changes * Fixed PHP deprecated error for $exp overflow for the last language --- .../Content/Gateway/DoctrineDatabase.php | 11 +-- .../Legacy/Content/Language/MaskGenerator.php | 3 +- src/lib/Persistence/Legacy/Content/Mapper.php | 3 +- .../DatabasePlatform/AbstractGateway.php | 60 ++++++++++++++++ .../DatabasePlatform/FallbackGateway.php | 25 +------ .../DatabasePlatform/PostgresqlGateway.php | 34 +++++++++ .../DatabasePlatform/SqliteGateway.php | 5 +- .../Legacy/SharedGateway/Gateway.php | 5 ++ .../legacy/shared_gateway.yaml | 6 ++ .../MaxLanguagesContentServiceTest.php | 69 +++++++++++++++++++ .../Repository/_fixtures/max_languages.yaml | 62 +++++++++++++++++ .../SharedGateway/GatewayFactoryTest.php | 2 +- tests/lib/Persistence/Legacy/TestCase.php | 2 +- 13 files changed, 246 insertions(+), 41 deletions(-) create mode 100644 src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/AbstractGateway.php create mode 100644 src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/PostgresqlGateway.php create mode 100644 tests/integration/Core/Repository/ContentService/MaxLanguagesContentServiceTest.php create mode 100644 tests/integration/Core/Repository/_fixtures/max_languages.yaml diff --git a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php index 29d1569b52..3931637596 100644 --- a/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php @@ -1357,16 +1357,7 @@ public function setName(int $contentId, int $version, string $name, string $lang */ private function getSetNameLanguageMaskSubQuery(): string { - return << 0 ) - THEN (:language_id | 1) - ELSE :language_id - END - FROM ezcontentobject - WHERE id = :content_id) - SQL; + return $this->sharedGateway->getSetNameLanguageMaskSubQuery(); } public function deleteContent(int $contentId): void diff --git a/src/lib/Persistence/Legacy/Content/Language/MaskGenerator.php b/src/lib/Persistence/Legacy/Content/Language/MaskGenerator.php index c51c84f85f..d82714cda5 100644 --- a/src/lib/Persistence/Legacy/Content/Language/MaskGenerator.php +++ b/src/lib/Persistence/Legacy/Content/Language/MaskGenerator.php @@ -150,7 +150,8 @@ public function extractLanguageIdsFromMask($languageMask): array $result = []; // Decomposition of $languageMask into its binary components. - while ($exp <= $languageMask) { + // check if $exp has not overflown and became float (happens for the last possible language in the mask) + while (is_int($exp) && $exp <= $languageMask) { if ($languageMask & $exp) { $result[] = $exp; } diff --git a/src/lib/Persistence/Legacy/Content/Mapper.php b/src/lib/Persistence/Legacy/Content/Mapper.php index aa6a670a66..64456bd400 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper.php +++ b/src/lib/Persistence/Legacy/Content/Mapper.php @@ -535,7 +535,8 @@ private function extractLanguageCodesFromMask(int $languageMask, array $allLangu $result = []; // Decomposition of $languageMask into its binary components to extract language codes - while ($exp <= $languageMask) { + // check if $exp has not overflown and became float (happens for the last possible language in the mask) + while (is_int($exp) && $exp <= $languageMask) { if ($languageMask & $exp) { if (isset($allLanguages[$exp])) { $result[] = $allLanguages[$exp]->languageCode; diff --git a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/AbstractGateway.php b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/AbstractGateway.php new file mode 100644 index 0000000000..9989c7ecac --- /dev/null +++ b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/AbstractGateway.php @@ -0,0 +1,60 @@ +connection = $connection; + } + + public function getColumnNextIntegerValue( + string $tableName, + string $columnName, + string $sequenceName + ): ?int { + return null; + } + + /** + * Return a language sub select query for setName. + * + * The query generates the proper language mask at the runtime of the INSERT/UPDATE query + * generated by setName. + * + * @see setName + */ + public function getSetNameLanguageMaskSubQuery(): string + { + return << 0 ) + THEN (:language_id | 1) + ELSE :language_id + END + FROM ezcontentobject + WHERE id = :content_id) + SQL; + } + + public function getLastInsertedId(string $sequenceName): int + { + return (int)$this->connection->lastInsertId($sequenceName); + } +} diff --git a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/FallbackGateway.php b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/FallbackGateway.php index 7a7a483c54..13330f23db 100644 --- a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/FallbackGateway.php +++ b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/FallbackGateway.php @@ -8,31 +8,8 @@ namespace Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform; -use Doctrine\DBAL\Connection; -use Ibexa\Core\Persistence\Legacy\SharedGateway\Gateway; - -final class FallbackGateway implements Gateway +final class FallbackGateway extends AbstractGateway { - /** @var \Doctrine\DBAL\Connection */ - private $connection; - - public function __construct(Connection $connection) - { - $this->connection = $connection; - } - - public function getColumnNextIntegerValue( - string $tableName, - string $columnName, - string $sequenceName - ): ?int { - return null; - } - - public function getLastInsertedId(string $sequenceName): int - { - return (int)$this->connection->lastInsertId($sequenceName); - } } class_alias(FallbackGateway::class, 'eZ\Publish\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\FallbackGateway'); diff --git a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/PostgresqlGateway.php b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/PostgresqlGateway.php new file mode 100644 index 0000000000..d7889d127f --- /dev/null +++ b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/PostgresqlGateway.php @@ -0,0 +1,34 @@ + 0 ) + THEN (cast(:language_id as BIGINT) | 1) + ELSE :language_id + END + FROM ezcontentobject + WHERE id = :content_id) + SQL; + } +} diff --git a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php index b92fa7ea03..0825754e1b 100644 --- a/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php +++ b/src/lib/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php @@ -9,9 +9,8 @@ namespace Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform; use Ibexa\Core\Base\Exceptions\DatabaseException; -use Ibexa\Core\Persistence\Legacy\SharedGateway\Gateway; -final class SqliteGateway implements Gateway +final class SqliteGateway extends AbstractGateway { /** * Error code 7 for a fatal error - taken from an existing driver implementation. @@ -21,7 +20,7 @@ final class SqliteGateway implements Gateway private const DB_INT_MAX = 2147483647; /** @var array */ - private $lastInsertedIds = []; + private array $lastInsertedIds = []; public function getColumnNextIntegerValue( string $tableName, diff --git a/src/lib/Persistence/Legacy/SharedGateway/Gateway.php b/src/lib/Persistence/Legacy/SharedGateway/Gateway.php index 9b947b146c..eb674d3de3 100644 --- a/src/lib/Persistence/Legacy/SharedGateway/Gateway.php +++ b/src/lib/Persistence/Legacy/SharedGateway/Gateway.php @@ -41,6 +41,11 @@ public function getColumnNextIntegerValue( * It returns integer as all the IDs in the Ibexa Legacy Storage are (big)integers */ public function getLastInsertedId(string $sequenceName): int; + + /** + * Return a language sub select query for setName. + */ + public function getSetNameLanguageMaskSubQuery(): string; } class_alias(Gateway::class, 'eZ\Publish\Core\Persistence\Legacy\SharedGateway\Gateway'); diff --git a/src/lib/Resources/settings/storage_engines/legacy/shared_gateway.yaml b/src/lib/Resources/settings/storage_engines/legacy/shared_gateway.yaml index 5fdbe8c133..e72873d48a 100644 --- a/src/lib/Resources/settings/storage_engines/legacy/shared_gateway.yaml +++ b/src/lib/Resources/settings/storage_engines/legacy/shared_gateway.yaml @@ -6,12 +6,18 @@ services: bind: $connection: '@ibexa.persistence.connection' + Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\AbstractGateway: ~ + Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\FallbackGateway: ~ Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\SqliteGateway: tags: - { name: ibexa.storage.legacy.gateway.shared, platform: sqlite } + Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\PostgresqlGateway: + tags: + - { name: ibexa.storage.legacy.gateway.shared, platform: postgresql } + Ibexa\Core\Persistence\Legacy\SharedGateway\GatewayFactory: arguments: $fallbackGateway: '@Ibexa\Core\Persistence\Legacy\SharedGateway\DatabasePlatform\FallbackGateway' diff --git a/tests/integration/Core/Repository/ContentService/MaxLanguagesContentServiceTest.php b/tests/integration/Core/Repository/ContentService/MaxLanguagesContentServiceTest.php new file mode 100644 index 0000000000..1885328cce --- /dev/null +++ b/tests/integration/Core/Repository/ContentService/MaxLanguagesContentServiceTest.php @@ -0,0 +1,69 @@ + */ + private static array $languagesRawList = []; + + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + self::$languagesRawList = Yaml::parseFile(dirname(__DIR__) . '/_fixtures/max_languages.yaml'); + } + + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException + */ + protected function setUp(): void + { + parent::setUp(); + + $this->prepareMaxLanguages(); + } + + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\Exception + */ + public function testCreateContent(): void + { + $names = array_merge(...array_map( + static fn (array $languageData): array => [ + $languageData['languageCode'] => $languageData['name'] . ' name', + ], + self::$languagesRawList + )); + $this->createFolder($names); + } + + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException + */ + private function prepareMaxLanguages(): void + { + $languageService = self::getLanguageService(); + + foreach (self::$languagesRawList as $languageData) { + $languageCreateStruct = $languageService->newLanguageCreateStruct(); + $languageCreateStruct->languageCode = $languageData['languageCode']; + $languageCreateStruct->name = $languageData['name']; + $languageService->createLanguage($languageCreateStruct); + } + } +} diff --git a/tests/integration/Core/Repository/_fixtures/max_languages.yaml b/tests/integration/Core/Repository/_fixtures/max_languages.yaml new file mode 100644 index 0000000000..72b8cc8fef --- /dev/null +++ b/tests/integration/Core/Repository/_fixtures/max_languages.yaml @@ -0,0 +1,62 @@ +- { languageCode: alb-SQ, name: Albanian } +- { languageCode: ara-AR, name: Arabic } +- { languageCode: aze-AZ, name: Azerbaijani } +- { languageCode: bos-BS, name: Bosnian } +- { languageCode: cha-CH, name: Chamorro } +- { languageCode: chi-ZH, name: Chinese } +- { languageCode: cze-CS, name: Czech } +- { languageCode: dan-DA, name: Danish } +- { languageCode: dut-NL, name: Dutch (Flemish) } +#- { languageCode: eng-GB, name: English (United Kingdom) } # Pre-exists in the initial db fixture +#- { languageCode: eng-US, name: English (United States) } # Pre-exists in the initial db fixture +- { languageCode: eng-AU, name: English (Australia) } +- { languageCode: epo-EO, name: Esperanto } +- { languageCode: esp-ES, name: Spanish } +- { languageCode: esp-MX, name: Spanish (Mexico) } +- { languageCode: est-ET, name: Estonian } +- { languageCode: fas-FA, name: Persian (Farsi) } +- { languageCode: fin-FI, name: Finnish } +- { languageCode: fre-FR, name: French (France) } +- { languageCode: fre-BE, name: French (Belgium) } +- { languageCode: fre-CA, name: French (Canada) } +- { languageCode: fre-CH, name: French (Switzerland) } +- { languageCode: fre-LU, name: French (Luxembourg) } +- { languageCode: geo-KA, name: Georgian } +#- { languageCode: ger-DE, name: German (Germany) } # Pre-exists in the initial db fixture +- { languageCode: ger-AT, name: German (Austria) } +- { languageCode: ger-CH, name: German (Switzerland) } +- { languageCode: ger-LI, name: German (Liechtenstein) } +- { languageCode: ger-LU, name: German (Luxembourg) } +- { languageCode: gle-GA, name: Irish } +- { languageCode: gla-GD, name: Scottish (Gaelic) } +- { languageCode: gre-EL, name: Greek } +- { languageCode: hin-HI, name: Hebrew } +- { languageCode: heb-HE, name: Hebrew } +- { languageCode: hrv-HR, name: Croatian } +- { languageCode: hun-HU, name: Hungarian } +- { languageCode: ind-ID, name: Indonesian } +- { languageCode: isl-IS, name: Icelandic } +- { languageCode: ita-IT, name: Italian } +- { languageCode: jpn-JA, name: Japanese } +- { languageCode: kor-KO, name: Korean } +- { languageCode: lat-LA, name: Latin } +- { languageCode: lav-LV, name: Latvian } +- { languageCode: lit-LT, name: Lithuanian } +- { languageCode: mao-MI, name: Maori (New Zealand) } +- { languageCode: may-MS, name: Malay } +- { languageCode: nor-NO, name: Norwegian } +- { languageCode: pol-PL, name: Polish } +- { languageCode: por-PT, name: Portuguese (Portugal) } +- { languageCode: por-BR, name: Portuguese (Brazil) } +- { languageCode: rum-RO, name: Romanian } +- { languageCode: slo-SK, name: Slovak } +- { languageCode: swe-SV, name: Swedish } +- { languageCode: bul-BG, name: Bulgarian } +- { languageCode: swa-SW, name: Swahili (Swahili) } +- { languageCode: tha-TH, name: Thai } +- { languageCode: tib-BO, name: Tibetan } +- { languageCode: tlh-TL, name: Klingon } +- { languageCode: tur-TR, name: Turkish } +- { languageCode: ukr-UK, name: Ukrainian } +- { languageCode: wel-CY, name: Welsh (Swahili) } +- { languageCode: yid-YI, name: Yiddish } diff --git a/tests/lib/Persistence/Legacy/SharedGateway/GatewayFactoryTest.php b/tests/lib/Persistence/Legacy/SharedGateway/GatewayFactoryTest.php index 090c366bf9..a75aab39c8 100644 --- a/tests/lib/Persistence/Legacy/SharedGateway/GatewayFactoryTest.php +++ b/tests/lib/Persistence/Legacy/SharedGateway/GatewayFactoryTest.php @@ -30,7 +30,7 @@ final class GatewayFactoryTest extends TestCase public function setUp(): void { $gateways = [ - 'sqlite' => new SqliteGateway(), + 'sqlite' => new SqliteGateway($this->createMock(Connection::class)), ]; $this->factory = new GatewayFactory( diff --git a/tests/lib/Persistence/Legacy/TestCase.php b/tests/lib/Persistence/Legacy/TestCase.php index 887861cfe7..72b9593222 100644 --- a/tests/lib/Persistence/Legacy/TestCase.php +++ b/tests/lib/Persistence/Legacy/TestCase.php @@ -114,7 +114,7 @@ final public function getSharedGateway(): SharedGateway\Gateway $factory = new SharedGateway\GatewayFactory( new SharedGateway\DatabasePlatform\FallbackGateway($connection), [ - 'sqlite' => new SharedGateway\DatabasePlatform\SqliteGateway(), + 'sqlite' => new SharedGateway\DatabasePlatform\SqliteGateway($connection), ] );