phpro / zf-doctrine-hydration-module

Configurable Doctrine hydrators for ZF2
18 stars 33 forks source link

This seems not to work with ZF 2.6 #20

Closed prolic closed 8 years ago

prolic commented 8 years ago

any ideas why?

prolic commented 8 years ago

ping @aderuwe

aderuwe commented 8 years ago

No idea why you'd ping me, I've not been involved with this project.

That said, we'll need some more information. How does it not work?

prolic commented 8 years ago

@aderuwe sorry, you are mentioned as maintainer here: https://github.com/orgs/phpro/people

The problem is, that I upgraded my application to zf 2.6 and suddenly all hydrator strategies don't work anymore. Reverting the composer json to its old state solved all problems. The change might be related to the new Zend\Hydrator component. But all old Hydrators from Zend\Stdlib\Hydrator* are still existing, extending the new ones from Zend\Hydrator.

Don't know if this helps.

aderuwe commented 8 years ago

I guess we should explicitly state this in composer.json, or fix the code.

Ping @veewee.

veewee commented 8 years ago

Hello @prolic,

As far as I know, ZF 2.6 is not released yet.

In ZF 2.5 the hydrators are indeed moved from the stdlib package to a separate hydrators package. As you state, the (deprecated) hydrators are still mapped in the stdlib namespace. So normally that shoud not be a problem.

I discussed this move yesterday with @TomHAnderson. At the moment it is not clear yet how we will integrate the new package in this project.

Also the doctrine module dependency did not change. So I do not have a clue on your error. Can you provide me some additional context? These might be usefull:

Thanks!

prolic commented 8 years ago

I am using ODM currently. For doctrine-odm-module i use a patched version, see https://github.com/doctrine/DoctrineMongoODMModule/pull/149. Without this obviously doctrine odm module cannot handle the new hydrator component.

With the following composer settings (extraction):

"require": {
    "zendframework/zend-log": "~2.6.0",
    "doctrine/doctrine-mongo-odm-module": "dev-patch-1 as dev-master",
},
"repositories": [
    {
        "type": "git",
        "url": "https://github.com/prolic/DoctrineMongoODMModule"
    },

I get this exception:

Catchable fatal error: Argument 1 passed to Document\Address::setCountry() must be an instance of Enum\Country, string given, called in vendor/doctrine/doctrine-module/src/DoctrineModule/Stdlib/Hydrator/DoctrineObject.php on line 282 and defined in module/src/Document/Address.php on line 106

Reverting changes to (extraction again, removed repository of course):

"require": {
    "zendframework/zend-log": "2.5.2",
    "zendframework/zend-mvc": "2.5.2",
    "zendframework/zend-stdlib": "2.5.2",
    "doctrine/doctrine-mongo-odm-module": "dev-master"
}

The exception is gone again.

This is definitely the only change I made, just tested it again. The hydrators are all configured the same.

Btw: Some ZF2 components are already higher than 2.6, see: https://github.com/zendframework/zend-stdlib/tree/release-2.7.3

veewee commented 8 years ago

Hi @prolic,

This seems to be the problem: https://github.com/phpro/zf-doctrine-hydration-module/blob/master/src/Service/DoctrineHydratorFactory.php#L283

In the DoctrineHydratorFactory there are lots of instanceof checks. These point to the old interface location in the stdlib package. This won't be easy to fix: we want to stay compatible with the LTS version of ZF2 (2.4.X). In this version the stdlib is still the place where the hydrators live. This means we can't use zend-hydrator package yet.

We are discussing the same issue in another thread: https://github.com/zfcampus/zf-apigility-doctrine/issues/234

Hopefully we find a solution for this problem soon, but in the meantime this library won't be compatible with the new hydrator package.

prolic commented 8 years ago

The StrategyEnabledInterface from Stdlib also extends from Zend\Hydrator. Seems like Zend\Stdlib is not fully backwards-compatible. Open an issue at ZF2-repo?

Any way, thanks for your support.

veewee commented 8 years ago

Yes the interfaces in the new zend-stdlib extend the the zend-hydrator but the zend-hydrator ones do not extend the zend-stdlib once. So if you use the zend-hydrator once, you won't be compatible with ZF 2.4 (LTS)

Maybe it's a good idea to address this issue in the official ZF repo.

prolic commented 8 years ago

You cannot extend circular :)

veewee commented 8 years ago

Yes obviously :-)

veewee commented 8 years ago

@TomHAnderson Today I put a lot of effort in upgrading the doctrine-hydrators package to a new version which works with zend-hydrator.

The problem is that the DoctrineModule still uses the StdLib. This enforces functions like addStrategy() to use the old stdlib instances instead of the new hydrator instances:

https://github.com/doctrine/DoctrineModule/blob/0.10.0/src/DoctrineModule/Stdlib/Hydrator/DoctrineObject.php#L30-L31

As long as the Doctrine Module is not compatible with zend-hydrator, I won't be able to make our package compatible. I prepared a v2/zend-hydrator branch which won't work because of the Doctrine Module issue. From the moment it is supported, I can create and maintain a new version pretty quick.

So, currently no progress... I Learned a lot though :)

One more question: I added https://github.com/phpro/zf-doctrine-hydration-module/blob/master/composer.json#L21 This is to make sure that all features work, but it will block installing higher versions of zf. Do you think it's okay to block at this version?

A little more context: When using higher versions of zend-stdlib, the advanced configuration like strategies, filters, naming strategy, ... won't work. This is because the factory is checking for a stdlib interface like Stdlib\Hydrator\StrategyEnabledInterface, but because the StdLib\Hydrator\AbstractHydrator extends the Hydrator\AbstractHydrator it will never implement the Stdlib\Hydrator\StrategyEnabledInterface.

TomHAnderson commented 8 years ago

It's all bas-akwards isn't it? I agree with everyting you've discovered here and found the same conclusions. We're bound because of the Doctrine module and can only move forward when that moves forward. However it's currently at 0.10.x and changing the hydrators would require a BC break so I don't know how the version numbers would work out.

I do think it's OK to block at 2.7. Although ZF is moving forward and towards PSR-7 compatibility, Apigility is ready to use today and gives the developer a more stable and broadly supported platform. I see Apigility staying where it is in relation to ZF2 for the future and a new development initiative to create a PSR-7 version of Apigility.

veewee commented 8 years ago

Thanks for the feedback! Looks like we'll have to wait than :)

stavarengo commented 8 years ago

+1

TomHAnderson commented 8 years ago

@veewee I have a fix. https://github.com/API-Skeletons/zf-doctrine-module-zend-hydrator

I override the problem classes and in this composer.json the above repository overrides the DoctrineModule allowing zend-hydrator to work together.

https://github.com/API-Skeletons/zf-doctrine-zend-hydrator/blob/master/zend-hydrator.composer.json

There are three modified custom-included modules to pull this off and we'll need to stage them all together to pull this off, provided you're comfortable with this rather drastic solution.

Tom

veewee commented 8 years ago

Hi @TomHAnderson,

This Doctrine Hydrator is becoming a real struggle. I agree in using your zf-doctrine-module-zend-hydrator untill the official DoctrineModule is supporting the new zend-hydrator.

I am not sure what you want to do with the zend-hydrator.composer.json file? I assume that I could just use the zf-doctrine-module-zend-hydrator hydrators in this module instead of the module ones?

The v2 branch is updated with your repository and all tests pass. I bumped the PHP versions to >=5.6 since the doctrine ODM module is also enforcing this and the code is PHP7 compatible now. I also dropped a comment on the new hydrator package. Can you take a look at that one before I tag the new branch?

Great job on the package!

TomHAnderson commented 8 years ago

I'm a GO for your new branch.

The zend-hydrator.composer.json file is a working example of the new library overriding DoctrineModule. It was just a demo and will be removed from the patch library.

Speaking of which, what do we call this? A "Patch Repository"?

TomHAnderson commented 8 years ago

For your new branch use for futureproofing

zendframework/zend-hydrator ~1.0 || ^2.0

veewee commented 8 years ago

Allright! I'll try to make some time to tag the releases somewhere this week.

The hydrator is indeed set to that version: https://github.com/phpro/zf-doctrine-hydration-module/blob/v2/zend-hydrator/composer.json#L21

I will tag version 1.0.0 of this package as the old version and a new named 2.0.0 with the new code. This package has been in 0.X for long enough I guess :)

veewee commented 8 years ago

The branch got merged and v1 and v2 of this project are tagged!

PowerKiKi commented 8 years ago

Thanks for your work. I look forward to upgrading my projects.

On a sidenote, I noticed the README now mentions that both branches will be kept in sync as much as possible. I appreciate the intent, but if I were you I would only focus on the latest branch and not raise to much expectations for users of branch 1.x.