Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Metadata PhpDriver always beats AnnotationDriver in DriverChain #567

Closed
akomm opened this issue May 3, 2017 · 9 comments
Closed

Comments

@akomm
Copy link

akomm commented May 3, 2017

Problem:

Given classes C1 and C2, while C1 extends C2:
When I deserialize (fromArray) C1 I get an error "can not redeclare class C2".

Found cause:

The DriverChain (jms_serializer.metadata.chain_driver) has following drivers by default (order is important):

  • YamlDriver
  • XmlDriver
  • PhpDriver
  • AnnotationDriver

Both PhpDriver and AnnotationDriver accept an annotated .php file. But because PhpDriver is first in order, AnnotationDriver never comes into play. The PhpDriver requires the config resource and that triggers the explained error.

Overriding the service in the following way fixes the error:

  jms_serializer.metadata.chain_driver:
    class: "%jms_serializer.metadata.chain_driver.class%"
    arguments:
      -
        - '@jms_serializer.metadata.yaml_driver'
        - '@jms_serializer.metadata.xml_driver'
        - '@jms_serializer.metadata.annotation_driver' # moved up
        - '@jms_serializer.metadata.php_driver'

Not sure who really is responsible for the issue, the serializer bundle or library. Possibilities:

  1. jms/serializer: The missing acceptance check API in Metadata\Driver\DriverInterface and thus missing checks using the API. Currently the use-cases I found always implicate that null returned from loadMetadataForClass call means "not accepted", but its not phpdoc-annotated and explained in DriverInterface, so I'm not sure.

  2. jms/serializer: The bold require in PhpDriver without checking acceptance and returning null, before requiring. I see the problem not requiring here thought...

  3. jms/serializer-bundle: Actually the root problem seems to be one of the above. When null return in 1. is meant to be "not accepted", then 2. is the problem. But I see some complexity when testing acceptance in case 2. before "require". So maybe rearranging the driver chain is in this package is the smarter solution.

@goetas
Copy link
Collaborator

goetas commented May 3, 2017

Are you using annotations or php metadata files?

@akomm
Copy link
Author

akomm commented May 3, 2017

Annotation.

@goetas
Copy link
Collaborator

goetas commented May 3, 2017

How are configured your paths? ( they should be not configured at all; and the php driver should never be invoked )

@akomm
Copy link
Author

akomm commented May 3, 2017

  metadata:
    auto_detection: false
    directories:
      AppMutationInput:
        namespace_prefix: "AppBundle\\GraphQL\\Resolver\\Mutation\\Input"
        path: "%kernel.root_dir%/../src/AppBundle/GraphQL/Resolver/Mutation/Input"

I use it to hydrate (fromArray) data transfer objects from graphql mutation queries. With default config it only checks AppBundle\Entities and throws, when I try to hydrate something else.

@goetas
Copy link
Collaborator

goetas commented May 3, 2017

if you use annotations, you do not need to define any metadata path (annotations are extracted via reflection) from the classes.

if you are you using a mixed approach php + annotation, make sure to not have namespace collisions... (that is probably what activates the php driver...)

@akomm
Copy link
Author

akomm commented May 3, 2017

I started with zero configuration and got this exception. That is when I started check configuration and added the directory configuration for the desired namespace. Now I removed the configuration again and no exception. But something must have changed, because there is no magic.

@goetas
Copy link
Collaborator

goetas commented May 3, 2017

replaceMetadataDir is being called. can you post your configs somewhere?

@akomm
Copy link
Author

akomm commented May 3, 2017

Its almost default. Added dunglas_action and overblog_graphql plus security config. None of them have even close something to do jms serializer. And none of them were changed a single time since I added the jms serializer bundle.

As you said it works with ZERO config - now. NO magic. I must have done something bloody stupid and did not notice it head => wall. If you want get sure that there is indeed no edge-case scenario which caused the method's invocation I can provide you the config, but I think it was just me confused by only devil knows what (I still can not reproduce the case).

@goetas
Copy link
Collaborator

goetas commented May 3, 2017

it was just me confused by only devil knows what

👍

@goetas goetas closed this as completed May 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants