From 83a72651db6db47667f2ec7d0cdc94c23f1a1552 Mon Sep 17 00:00:00 2001 From: Zaruike Date: Thu, 5 Dec 2024 14:44:41 +0100 Subject: [PATCH 1/2] feat(chore): refactor code to prevent breaking changes in ORM3 while maintaining ORM2 compatibility --- Entity/LibrariesHubCache.php | 2 +- Entity/LibrariesLanguagesRepository.php | 14 +++++++-- Entity/Library.php | 2 +- Entity/LibraryRepository.php | 15 +++++----- Service/DoctrineParser.php | 38 +++++++++++++++++++++++++ composer.json | 3 +- 6 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 Service/DoctrineParser.php diff --git a/Entity/LibrariesHubCache.php b/Entity/LibrariesHubCache.php index cc45f85..03093a4 100644 --- a/Entity/LibrariesHubCache.php +++ b/Entity/LibrariesHubCache.php @@ -4,7 +4,7 @@ use Doctrine\ORM\Mapping as ORM; -#[ORM\Entity()] +#[ORM\Entity] #[ORM\Table('h5p_libraries_hub_cache')] class LibrariesHubCache { diff --git a/Entity/LibrariesLanguagesRepository.php b/Entity/LibrariesLanguagesRepository.php index 0831fa6..799ed96 100644 --- a/Entity/LibrariesLanguagesRepository.php +++ b/Entity/LibrariesLanguagesRepository.php @@ -5,6 +5,7 @@ use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; +use Studit\H5PBundle\Service\DoctrineParser; /** * LibrariesLanguagesRepository @@ -22,7 +23,12 @@ public function findForLibrary($machineName, $majorVersion, $minorVersion, $lang ->select('ll.languageJson') ->join('ll.library', 'l', 'WITH', 'l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') ->where('ll.languageCode = :languageCode') - ->setParameters(['majorVersion' => $majorVersion, 'machineName' => $machineName, 'minorVersion' => $minorVersion, 'languageCode' => $languageCode]); + ->setParameters(DoctrineParser::buildParams([ + 'majorVersion' => $majorVersion, + 'machineName' => $machineName, + 'minorVersion' => $minorVersion, + 'languageCode' => $languageCode + ])); try { $result = $qb->getQuery()->getSingleResult(); } catch (NoResultException $e) { @@ -35,7 +41,11 @@ public function findForLibraryAllLanguages($machineName, $majorVersion, $minorVe $qb = $this->createQueryBuilder('ll') ->select('ll.languageCode') ->join('ll.library', 'l', 'WITH', 'l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') - ->setParameters(['majorVersion' => $majorVersion, 'machineName' => $machineName, 'minorVersion' => $minorVersion]); + ->setParameters(DoctrineParser::buildParams([ + 'majorVersion' => $majorVersion, + 'machineName' => $machineName, + 'minorVersion' => $minorVersion + ])); try { $results = $qb->getQuery()->getArrayResult(); } catch (NoResultException $e) { diff --git a/Entity/Library.php b/Entity/Library.php index 902d2b3..0556bac 100644 --- a/Entity/Library.php +++ b/Entity/Library.php @@ -116,7 +116,7 @@ class Library #[ORM\OneToMany(targetEntity: ContentLibraries::class, mappedBy: "library")] /** - * @var ArrayCollection|Collection + * @var ArrayCollection|Collection $contentLibraries */ private ArrayCollection|Collection $contentLibraries; /** diff --git a/Entity/LibraryRepository.php b/Entity/LibraryRepository.php index 212f266..d12ce43 100644 --- a/Entity/LibraryRepository.php +++ b/Entity/LibraryRepository.php @@ -7,6 +7,7 @@ use Doctrine\ORM\AbstractQuery; use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; +use Studit\H5PBundle\Service\DoctrineParser; /** * LibraryRepository @@ -95,11 +96,11 @@ public function findHasSemantics($machineName, $majorVersion, $minorVersion) $qb = $this->createQueryBuilder('l') ->select('l') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.semantics is not null') - ->setParameters([ + ->setParameters(DoctrineParser::buildParams([ 'machineName' => $machineName, 'majorVersion' => $majorVersion, 'minorVersion' => $minorVersion - ]); + ])); try { $library = $qb->getQuery()->getSingleResult(); } catch (NoResultException $e) { @@ -123,7 +124,7 @@ public function findOneArrayBy($parameters) { $qb = $this->createQueryBuilder('l') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') - ->setParameters($parameters); + ->setParameters(DoctrineParser::buildParams($parameters)); return $qb->getQuery()->getOneOrNullResult(AbstractQuery::HYDRATE_ARRAY); } public function findIdBy($machineName, $majorVersion, $minorVersion) @@ -131,11 +132,11 @@ public function findIdBy($machineName, $majorVersion, $minorVersion) $qb = $this->createQueryBuilder('l') ->select('l.id') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.semantics is not null') - ->setParameters([ + ->setParameters(DoctrineParser::buildParams([ 'machineName' => $machineName, 'majorVersion' => $majorVersion, 'minorVersion' => $minorVersion - ]); + ])); try { return $qb->getQuery()->getSingleScalarResult(); } catch (NoResultException $e) { @@ -147,12 +148,12 @@ public function isPatched($library): bool $qb = $this->createQueryBuilder('l') ->select('COUNT(l)') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.patchVersion < :patchVersion') - ->setParameters([ + ->setParameters(DoctrineParser::buildParams([ 'machineName' => $library['machineName'], 'majorVersion' => $library['majorVersion'], 'minorVersion' => $library['minorVersion'], 'patchVersion' => $library['patchVersion'] - ]); + ])); return $qb->getQuery()->getSingleScalarResult() > 0; } } diff --git a/Service/DoctrineParser.php b/Service/DoctrineParser.php new file mode 100644 index 0000000..0518cdc --- /dev/null +++ b/Service/DoctrineParser.php @@ -0,0 +1,38 @@ + $val){ + $paramsCollection[] = new \Doctrine\ORM\Query\Parameter($k, $val); + } + + return new ArrayCollection($paramsCollection); + } + // For Doctrine ORM v2, return the parameters as is + return $params; + } +} \ No newline at end of file diff --git a/composer.json b/composer.json index e5b72b5..56b9c8b 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "jorisdugue/h5p-bundle", - "version": "3.0.1", + "version": "3.0.2", "type": "symfony-bundle", "description": "H5P Bundle for Symfony 5, 6 and Symfony 7", "keywords": [ @@ -29,6 +29,7 @@ ], "require": { "php": ">= 8.1", + "composer-runtime-api": "^2", "doctrine/orm": "~2.0|~3.0", "guzzlehttp/guzzle": "^7.9", "h5p/h5p-core": "1.27", From fe9004773e7d3d9e6e676e4223060be653b0e6a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joris=20Dugu=C3=A9?= Date: Thu, 5 Dec 2024 15:49:59 +0100 Subject: [PATCH 2/2] feat(chore): refactor some stuff for better output and implement test --- Entity/LibrariesLanguagesRepository.php | 10 ++-- Entity/LibraryRepository.php | 16 +++-- Service/DoctrineParser.php | 16 +++-- Tests/Service/DoctrineParserTest.php | 78 +++++++++++++++++++++++++ Utils/VersionORM.php | 25 ++++++++ 5 files changed, 130 insertions(+), 15 deletions(-) create mode 100644 Tests/Service/DoctrineParserTest.php create mode 100644 Utils/VersionORM.php diff --git a/Entity/LibrariesLanguagesRepository.php b/Entity/LibrariesLanguagesRepository.php index 799ed96..a5ea268 100644 --- a/Entity/LibrariesLanguagesRepository.php +++ b/Entity/LibrariesLanguagesRepository.php @@ -12,7 +12,7 @@ */ class LibrariesLanguagesRepository extends ServiceEntityRepository { - public function __construct(ManagerRegistry $registry) + public function __construct(ManagerRegistry $registry, private readonly DoctrineParser $parser) { parent::__construct($registry, LibrariesLanguages::class); } @@ -23,7 +23,7 @@ public function findForLibrary($machineName, $majorVersion, $minorVersion, $lang ->select('ll.languageJson') ->join('ll.library', 'l', 'WITH', 'l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') ->where('ll.languageCode = :languageCode') - ->setParameters(DoctrineParser::buildParams([ + ->setParameters($this->parser->buildParams([ 'majorVersion' => $majorVersion, 'machineName' => $machineName, 'minorVersion' => $minorVersion, @@ -31,7 +31,7 @@ public function findForLibrary($machineName, $majorVersion, $minorVersion, $lang ])); try { $result = $qb->getQuery()->getSingleResult(); - } catch (NoResultException $e) { + } catch (NoResultException) { return null; } return $result['languageJson'] ? $result['languageJson'] : null; @@ -41,14 +41,14 @@ public function findForLibraryAllLanguages($machineName, $majorVersion, $minorVe $qb = $this->createQueryBuilder('ll') ->select('ll.languageCode') ->join('ll.library', 'l', 'WITH', 'l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') - ->setParameters(DoctrineParser::buildParams([ + ->setParameters($this->parser->buildParams([ 'majorVersion' => $majorVersion, 'machineName' => $machineName, 'minorVersion' => $minorVersion ])); try { $results = $qb->getQuery()->getArrayResult(); - } catch (NoResultException $e) { + } catch (NoResultException) { return null; } $codes = array('en'); // Semantics is 'en' by default. diff --git a/Entity/LibraryRepository.php b/Entity/LibraryRepository.php index d12ce43..44b8094 100644 --- a/Entity/LibraryRepository.php +++ b/Entity/LibraryRepository.php @@ -15,10 +15,9 @@ * This class was generated by the PhpStorm "Php Annotations" Plugin. Add your own custom * repository methods below. */ - class LibraryRepository extends ServiceEntityRepository { - public function __construct(ManagerRegistry $registry) + public function __construct(ManagerRegistry $registry, private readonly DoctrineParser $parser) { parent::__construct($registry, Library::class); } @@ -91,12 +90,13 @@ public function findLatestLibraryVersions(): array } return $libraryVersions; } + public function findHasSemantics($machineName, $majorVersion, $minorVersion) { $qb = $this->createQueryBuilder('l') ->select('l') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.semantics is not null') - ->setParameters(DoctrineParser::buildParams([ + ->setParameters($this->parser->buildParams([ 'machineName' => $machineName, 'majorVersion' => $majorVersion, 'minorVersion' => $minorVersion @@ -108,6 +108,7 @@ public function findHasSemantics($machineName, $majorVersion, $minorVersion) } return (object)$library; } + public function findAllRunnableWithSemantics() { $qb = $this->createQueryBuilder('l') @@ -120,19 +121,21 @@ public function findAllRunnableWithSemantics() } return $libraries; } + public function findOneArrayBy($parameters) { $qb = $this->createQueryBuilder('l') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion') - ->setParameters(DoctrineParser::buildParams($parameters)); + ->setParameters($this->parser->buildParams($parameters)); return $qb->getQuery()->getOneOrNullResult(AbstractQuery::HYDRATE_ARRAY); } + public function findIdBy($machineName, $majorVersion, $minorVersion) { $qb = $this->createQueryBuilder('l') ->select('l.id') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.semantics is not null') - ->setParameters(DoctrineParser::buildParams([ + ->setParameters($this->parser->buildParams([ 'machineName' => $machineName, 'majorVersion' => $majorVersion, 'minorVersion' => $minorVersion @@ -143,12 +146,13 @@ public function findIdBy($machineName, $majorVersion, $minorVersion) return null; } } + public function isPatched($library): bool { $qb = $this->createQueryBuilder('l') ->select('COUNT(l)') ->where('l.machineName = :machineName and l.majorVersion = :majorVersion and l.minorVersion = :minorVersion and l.patchVersion < :patchVersion') - ->setParameters(DoctrineParser::buildParams([ + ->setParameters($this->parser->buildParams([ 'machineName' => $library['machineName'], 'majorVersion' => $library['majorVersion'], 'minorVersion' => $library['minorVersion'], diff --git a/Service/DoctrineParser.php b/Service/DoctrineParser.php index 0518cdc..bbeaf5b 100644 --- a/Service/DoctrineParser.php +++ b/Service/DoctrineParser.php @@ -4,6 +4,7 @@ use Composer\InstalledVersions; use Doctrine\Common\Collections\ArrayCollection; +use Studit\H5PBundle\Utils\VersionORM; /** * This class exists to prevent breaking changes when working with different versions of Doctrine ORM. @@ -12,6 +13,13 @@ */ class DoctrineParser { + private VersionORM $versionORM; + + public function __construct(VersionORM $versionORM) + { + $this->versionORM = $versionORM; + } + /** * This method converts parameters to an ArrayCollection for ORM v3. * If using ORM v2, it simply returns the received parameters as is. @@ -19,14 +27,14 @@ class DoctrineParser * @param array $params The input parameters to process. * @return ArrayCollection|array Returns an ArrayCollection for ORM v3 or the original parameters for ORM v2. */ - public static function buildParams(array $params): ArrayCollection|array + public function buildParams(array $params): ArrayCollection|array { - $doctrineVersion = InstalledVersions::getVersion('doctrine/orm'); + $doctrineVersion = $this->versionORM->getDoctrineVersion(); if ($doctrineVersion !== null && str_starts_with($doctrineVersion, '3')) { // For Doctrine ORM v3, ensure the parameters are returned as an ArrayCollection $paramsCollection = []; - foreach ($params as $k => $val){ + foreach ($params as $k => $val) { $paramsCollection[] = new \Doctrine\ORM\Query\Parameter($k, $val); } @@ -35,4 +43,4 @@ public static function buildParams(array $params): ArrayCollection|array // For Doctrine ORM v2, return the parameters as is return $params; } -} \ No newline at end of file +} diff --git a/Tests/Service/DoctrineParserTest.php b/Tests/Service/DoctrineParserTest.php new file mode 100644 index 0000000..4825c74 --- /dev/null +++ b/Tests/Service/DoctrineParserTest.php @@ -0,0 +1,78 @@ +createMock(VersionORM::class); + $mockVersionORM->method('getDoctrineVersion') + ->willReturn('3.1.0'); // Simulate Doctrine v2 version + // Inject the mocked version provider into DoctrineParser + $doctrineParser = new DoctrineParser($mockVersionORM); + + // Define test parameters + $params = [ + 'param1' => 'value1', + 'param2' => 'value2', + ]; + + // Call the method under test + $result = $doctrineParser->buildParams($params); + + // Assert that the result is an instance of ArrayCollection + $this->assertInstanceOf(ArrayCollection::class, $result); + + // Assert that the ArrayCollection contains Parameter objects + foreach ($result as $param) { + $this->assertInstanceOf(Parameter::class, $param); + } + + // Assert that the parameters inside the Parameter objects match the input parameters + $this->assertEquals('value1', $result[0]->getValue()); + $this->assertEquals('value2', $result[1]->getValue()); + } + + /** + * Test that buildParams returns the original parameters as an array for Doctrine ORM v2. + * @throws Exception + */ + public function testBuildParamsForDoctrineV2() + { + // Mock VersionProvider to simulate Doctrine v2 version + $mockVersionORM = $this->createMock(VersionORM::class); + $mockVersionORM->method('getDoctrineVersion') + ->willReturn('2.9.3'); // Simulate Doctrine v2 version + + // Inject the mocked version provider into DoctrineParser + $doctrineParser = new DoctrineParser($mockVersionORM); + + // Define test parameters + $params = [ + 'param1' => 'value1', + 'param2' => 'value2', + ]; + + // Call the method under test + $result = $doctrineParser->buildParams($params); + + // Assert that the result is an array (not an ArrayCollection) + $this->assertIsArray($result); + + // Assert that the returned array contains the same values as the input parameters + $this->assertSame($params, $result); + } +} diff --git a/Utils/VersionORM.php b/Utils/VersionORM.php new file mode 100644 index 0000000..a85d672 --- /dev/null +++ b/Utils/VersionORM.php @@ -0,0 +1,25 @@ +