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

AnnotationOrAttributeDriver problem with Autowire attribute #944

Closed
themark147 opened this issue Dec 12, 2023 · 8 comments
Closed

AnnotationOrAttributeDriver problem with Autowire attribute #944

themark147 opened this issue Dec 12, 2023 · 8 comments

Comments

@themark147
Copy link

Q A
Bug report? yes
Feature request? no
BC Break report? yes
RFC? no

Hi :) , after upgrade to 5.4 tag I'm getting error during cache:warmup command
I used 5.3.1 version and it worked well it can be related to that bundle? thanks

symfony version: 6.4
PHP: 8.2

In AnnotationOrAttributeDriver.php line 356:
Attribute "Symfony\Component\DependencyInjection\Attribute\Autowire" cannot target property (allowed targets: parameter)  

Steps required to reproduce the problem

  1. Using Autowire attribute to autowire service / env

Expected Result

Actual Result

@mbabker
Copy link
Contributor

mbabker commented Dec 12, 2023

It looks like you have an older version of the jms/serializer library installed, try upgrading that as well.

The error message is right in that the #[Autowire] attribute is only declared to support parameters. So the fact the error is saying it can't target a property leads me to believe you've got that attribute on a constructor with property promotion. Are you trying to serialize a class with services injected into it?

@themark147
Copy link
Author

I have 3.29 version of jms/serialzer & 2.8.0 jms/metadata ... but its still weird why it is happening right after upgrade that package from 5.3 to 5.4 ... No, I dont think so

@mbabker
Copy link
Contributor

mbabker commented Dec 13, 2023

Without knowing what class it's trying to read metadata from (and if it's project internal it sounds like at least the constructor would be needed), it'd be hard to identify what's happening here.

To my recollection, the bundle and library aren't automagically reading every PHP file in the project, so knowing what's causing a class with the #[Autowire] attribute to be picked up would be good to figure out, too (because in my mind if it has that attribute then it probably shouldn't be serialized).

@themark147
Copy link
Author

File is included in warmup paths even if I tried that simple class it still fails during warmup
with 5.3.1 bundle version it works well that class above

use Symfony\Component\DependencyInjection\Attribute\Autowire;

class DummyFile
{
    public function __construct(
        #[Autowire('%locale%')]
        private readonly string $locale
    )
    {
    }
}

@mbabker
Copy link
Contributor

mbabker commented Dec 13, 2023

The reason why it worked with the attribute reader that relied on the annotations interface is because of this workaround to limit it to classes in the JMS\Serializer\Annotation namespace. The standalone driver doesn't have that limitation.

To me, the quickest fix is to adjust your app so that the metadata reader isn't picking up classes that shouldn't be considered for serialization. Long term, either that namespace-based filter would need to be added to the standalone driver (IMO unless the serializer is saying that its metadata is closed from extension, I wouldn't suggest that; yes I'm also aware that if an app were adding their own metadata they'd need their own driver anyway, but I'm still not a fan of the namespace check) or some form of marker interface would need to be added so the Reflection getAttributes() calls could be updated to pass a name and the ReflectionAttribute::IS_INSTANCEOF flag instead of grabbing all attributes (which also would've solved schmittjoh/serializer#1525 in a somewhat nicer way).

@mbabker
Copy link
Contributor

mbabker commented Dec 13, 2023

schmittjoh/serializer#1529 should fix this.

@scyzoryck
Copy link
Collaborator

Thanks @mbabker for contribution! :) I released it under 3.29.1 tag.
@themark147 - Could you check if it works fine for you, please?

Best.

@themark147
Copy link
Author

@mbabker @scyzoryck yea. tested and it works pretty well.

thanks to you guys for such a fast respond and quick fix

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

3 participants