CollectionLinkHydrator should extend from different AbstractCollectionStrategy

[zluiten]

Fixes #13.

Class Laminas\ApiTools\Doctrine\QueryBuilder\Hydrator\Strategy\CollectionLinkHydratorV3 should extend class AbstractCollectionStrategy of doctrine/doctrine-laminas-hydrator when DoctrineModule >v3 is installed instead of the then removed AbstractCollectionStrategy of DoctrineModule v2.

[zluiten]

Can someone have a look this please?

[froschdesign]

@Netiul

…when DoctrineModule >v3 is installed instead of the then removed AbstractCollectionStrategy of DoctrineModule v2.

Your pull request does not fix the problem at all because this package still allow the installation of DoctrineModule with version 2.

api-tools-doctrine-querybuilder/composer.json

Line 33 in 002038f

"doctrine/doctrine-module": "^2.1.8 || ^3.0.1 || ^4.1.0",

https://github.com/doctrine/DoctrineModule/blob/c8ce08e9fa67de928bad306b3fe798fb6683bfea/src/DoctrineModule/Stdlib/Hydrator/Strategy/AbstractCollectionStrategy.php#L17

[zluiten]

@froschdesign

This package support both v2 and v3 of DoctrineModule.

The class DoctrineModule\Stdlib\Hydrator\Strategy\AbstractCollectionStrategy you are linking to only exists in v2. V3 of DoctrineModule uses the Laminas\Hydrator package. To support both there is a V2 and V3 version of the CollectionLinkHydrator. In src/_autoload.php the appropiate version is aliased to Hydrator\Strategy\CollectionLink based on the existence of the Laminas\Hydrator\HydratorPluginManagerInterface (which is only the case when v3 of DoctrineModule is installed).

But here is the problem, v3 of the CollectionLinkHydrator still extends the AbstractCollectonStrategy from DoctrineModule v2! This PR fixes that.

[froschdesign]

@Netiul

To support both there is a V2 and V3 in src/_autoload.php the appropiate versions is aliased to Hydrator\Strategy\CollectionLink based on the existence of the Laminas\Hydrator\HydratorPluginManagerInterface (which is only the case when v3 of DoctrineModule is installed).

Thanks, I missed this hint in the bug report and in the pull request description.

[froschdesign]

@Netiul
Can you also check the related test class because name looks strange:

api-tools-doctrine-querybuilder/test/Hydrator/Strategy/CollectionLinkTest.php

Line 11 in 002038f

use laminas\apitools\doctrine\querybuilder\hydrator\strategy\collectionlink;

Thanks in advance! 👍

[zluiten]

@froschdesign No problem! I guess I could have been more elaborative initially.

Can you also check the related test class because name looks strange:

api-tools-doctrine-querybuilder/test/Hydrator/Strategy/CollectionLinkTest.php

Line 11 in 002038f

use laminas\apitools\doctrine\querybuilder\hydrator\strategy\collectionlink;

Thanks in advance! +1

That looks strange indeed. I can fix the casing in the test but I see now that the word Collectionlink in Hydrator\Strategy\Collectionlink in _autoload.php is aliased with lowercased letter l of link. That might be a breaking change though. PHP's namespacing is case insensitive, but I think composer's autoloading is not. Can you advise on that @froschdesign ?