schmittjoh / serializer

Library for (de-)serializing data of any complexity (supports JSON, and XML)
http://jmsyst.com/libs/serializer
MIT License
2.32k stars 589 forks source link

feat(driver): TypedPropertiesDriver support virtual property getter #1487

Closed mpoiriert closed 1 year ago

mpoiriert commented 1 year ago
Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT

This allow to extract return type of virtual property when they are a getter on the class. Same logic as a type properties but also supporting virtual property getter.

Also support type from php doc for virtual property getter.

mpoiriert commented 1 year ago

I am using it on a project, I have a issue while removing all Type definition. I'll check if it's a mistake from my side or an issue with my patch.

I'll put this back to ready to review after.

mpoiriert commented 1 year ago

@scyzoryck It seems that the issue I have is not related to my change.

I am in symfony context, I have use the jms_serializer.metadata.infer_types_from_doc_block = true which is new (didn't use it before).

I get an error from the DocBlockTypeResolver that is trying to parse Doctrine\\Common\\Collections\\AbstractLazyCollection:collection

    "code": 500,
    "message": "Can't use non-array generic type Collection for collection in Doctrine\\Common\\Collections\\AbstractLazyCollection:collection",
    "detail": {
        "class": "InvalidArgumentException",
        "message": "Can't use non-array generic type Collection for collection in Doctrine\\Common\\Collections\\AbstractLazyCollection:collection",
        "code": 0,
        "file": "/home/wwwroot/vendor/jms/serializer/src/Metadata/Driver/DocBlockDriver/DocBlockTypeResolver.php",
        "line": 108,
        "stack": [
            "#0 /home/wwwroot/vendor/jms/serializer/src/Metadata/Driver/DocBlockDriver.php(64): JMS\\Serializer\\Metadata\\Driver\\DocBlockDriver\\DocBlockTypeResolver->getPropertyDocblockTypeHint(Object(ReflectionProperty))",
            "#1 /home/wwwroot/vendor/jms/metadata/src/Driver/LazyLoadingDriver.php(38): JMS\\Serializer\\Metadata\\Driver\\DocBlockDriver->loadMetadataForClass(Object(ReflectionClass))",
            "#2 /home/wwwroot/vendor/jms/metadata/src/MetadataFactory.php(111): Metadata\\Driver\\LazyLoadingDriver->loadMetadataForClass(Object(ReflectionClass))",
            "#3 /home/wwwroot/vendor/jms/serializer-bundle/Debug/TraceableMetadataFactory.php(37): Metadata\\MetadataFactory->getMetadataForClass('Doctrine\\\\ORM\\\\Pe...')",
            "#4 /home/wwwroot/vendor/jms/serializer/src/Handler/ArrayCollectionHandler.php(89): JMS\\SerializerBundle\\Debug\\TraceableMetadataFactory->getMetadataForClass('Doctrine\\\\ORM\\\\Pe...')",
        Called from the ArrayCollecitonHandler exclusion strategy. 
            if (false === $this->initializeExcluded) {
            $exclusionStrategy = $context->getExclusionStrategy();
            if (null !== $exclusionStrategy && $exclusionStrategy->shouldSkipClass($context->getMetadataFactory()->getMetadataForClass(\get_class($collection)), $context)) {
                $context->startVisiting($collection);

                return $visitor->visitArray([], $type);
            }
        }

I don't see exactly when we would check 'should skip class' on the collection itself ? Since we are not returning the collection but rather the object inside of it as an array.

I have check all the related code and everything is old, I have the feeling that not a lot (AKA no one) is using the DocBlockDriver with Doctrine. If I put the 'initialize_excluded' = true (which default to false) everything works.

Anyway, this is not related to the my pr so everything is fine.

goetas commented 1 year ago

Thank you