Merge pull request #1126 from schmittjoh/i596-implicit-expose-property

Avoid implicit expose of a property instead of virtual-property
schmittjoh · Sep 20, 2019 · 7344ef9 · 7344ef9
2 parents ff89778 + 78092dc
commit 7344ef9
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 19 deletions.
diff --git a/src/Metadata/Driver/XmlDriver.php b/src/Metadata/Driver/XmlDriver.php
@@ -158,8 +158,8 @@ protected function loadMetadataFromFile(\ReflectionClass $class, string $path):
                 if ($property->class !== $name || (isset($property->info) && $property->info['class'] !== $name)) {
                     continue;
                 }
-
-                $propertiesMetadata[] = new PropertyMetadata($name, $pName = $property->getName());
+                $pName = $property->getName();
+                $propertiesMetadata[] = new PropertyMetadata($name, $pName);
                 $pElems = $elem->xpath("./property[@name = '" . $pName . "']");
 
                 $propertiesNodes[] = $pElems ? reset($pElems) : null;

diff --git a/src/Metadata/Driver/YamlDriver.php b/src/Metadata/Driver/YamlDriver.php
@@ -114,6 +114,7 @@ protected function loadMetadataFromFile(ReflectionClass $class, string $file): ?
         $this->addClassProperties($metadata, $config);
 
         $propertiesMetadata = [];
+        $propertiesData = [];
         if (array_key_exists('virtual_properties', $config)) {
             foreach ($config['virtual_properties'] as $methodName => $propertySettings) {
                 if (isset($propertySettings['exp'])) {
@@ -132,10 +133,8 @@ protected function loadMetadataFromFile(ReflectionClass $class, string $file): ?
                     $virtualPropertyMetadata = new VirtualPropertyMetadata($name, $methodName);
                 }
 
-                $pName = !empty($propertySettings['name']) ? $propertySettings['name'] : $virtualPropertyMetadata->name;
-
-                $propertiesMetadata[$pName] = $virtualPropertyMetadata;
-                $config['properties'][$pName] = $propertySettings;
+                $propertiesMetadata[] = $virtualPropertyMetadata;
+                $propertiesData[] = $propertySettings;
             }
         }
 
@@ -146,18 +145,20 @@ protected function loadMetadataFromFile(ReflectionClass $class, string $file): ?
                 }
 
                 $pName = $property->getName();
-                $propertiesMetadata[$pName] = new PropertyMetadata($name, $pName);
+                $propertiesMetadata[] = new PropertyMetadata($name, $pName);
+                $propertiesData[] =  !empty($config['properties']) && true === array_key_exists($pName, $config['properties'])
+                    ? (array) $config['properties'][$pName]
+                    : null;
             }
 
-            foreach ($propertiesMetadata as $pName => $pMetadata) {
+            foreach ($propertiesMetadata as $propertyKey => $pMetadata) {
                 $isExclude = false;
                 $isExpose = $pMetadata instanceof VirtualPropertyMetadata
                     || $pMetadata instanceof ExpressionPropertyMetadata
-                    || (isset($config['properties']) && array_key_exists($pName, $config['properties']));
-
-                if (isset($config['properties'][$pName])) {
-                    $pConfig = $config['properties'][$pName];
+                    || isset($propertiesData[$propertyKey]);
 
+                $pConfig = $propertiesData[$propertyKey];
+                if (!empty($pConfig)) {
                     if (isset($pConfig['exclude'])) {
                         $isExclude = (bool) $pConfig['exclude'];
                     }
@@ -304,12 +305,8 @@ protected function loadMetadataFromFile(ReflectionClass $class, string $file): ?
                     $metadata->isMap = $metadata->isMap || PropertyMetadata::isCollectionMap($pMetadata->type);
                 }
 
-                if (isset($config['properties'][$pName])) {
-                    $pConfig = $config['properties'][$pName];
-
-                    if (isset($pConfig['name'])) {
-                        $pMetadata->name = (string) $pConfig['name'];
-                    }
+                if (!empty($pConfig) && !empty($pConfig['name'])) {
+                    $pMetadata->name = (string) $pConfig['name'];
                 }
 
                 if ((ExclusionPolicy::NONE === $exclusionPolicy && !$isExclude)

diff --git a/tests/Fixtures/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.php b/tests/Fixtures/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace JMS\Serializer\Tests\Fixtures;
+
+use JMS\Serializer\Annotation as Serializer;
+use JMS\Serializer\Annotation\VirtualProperty;
+
+/**
+ * @Serializer\ExclusionPolicy("ALL")
+ */
+class ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll
+{
+    protected $name;
+
+    /**
+     * @Serializer\SerializedName("mood")
+     *
+     * @VirtualProperty()
+     */
+    public function getName()
+    {
+        return 'value';
+    }
+}
diff --git a/tests/Metadata/Driver/BaseDriverTest.php b/tests/Metadata/Driver/BaseDriverTest.php
@@ -21,6 +21,7 @@
 use JMS\Serializer\Tests\Fixtures\ObjectWithExpressionVirtualPropertiesAndExcludeAll;
 use JMS\Serializer\Tests\Fixtures\ObjectWithInvalidExpression;
 use JMS\Serializer\Tests\Fixtures\ObjectWithVirtualPropertiesAndDuplicatePropName;
+use JMS\Serializer\Tests\Fixtures\ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll;
 use JMS\Serializer\Tests\Fixtures\ObjectWithVirtualPropertiesAndExcludeAll;
 use JMS\Serializer\Tests\Fixtures\ParentSkipWithEmptyChild;
 use Metadata\Driver\DriverInterface;
@@ -566,6 +567,18 @@ public function testExclusionIf()
         self::assertEquals($p, $m->propertyMetadata['age']);
     }
 
+    public function testObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll()
+    {
+        $class = ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll::class;
+        $m = $this->getDriver()->loadMetadataForClass(new \ReflectionClass($class));
+
+        $p = new VirtualPropertyMetadata($class, 'name');
+        $p->serializedName = 'mood';
+        $p->getter = 'getName';
+
+        self::assertEquals($p, $m->propertyMetadata['name']);
+    }
+
     public function testObjectWithVirtualPropertiesAndDuplicatePropName()
     {
         $class = ObjectWithVirtualPropertiesAndDuplicatePropName::class;

diff --git a/tests/Metadata/Driver/xml/ObjectWithExpressionVirtualPropertiesAndExcludeAll.xml b/tests/Metadata/Driver/xml/ObjectWithExpressionVirtualPropertiesAndExcludeAll.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <serializer>
-    <class name="JMS\Serializer\Tests\Fixtures\ObjectWithExpressionVirtualPropertiesAndExcludeAll"  exclusion-policy="ALL">
+    <class name="JMS\Serializer\Tests\Fixtures\ObjectWithExpressionVirtualPropertiesAndExcludeAll" exclusion-policy="ALL">
         <virtual-property name="virtualValue" expression="object.getVirtualValue()"/>
     </class>
 </serializer>
diff --git a/tests/Metadata/Driver/xml/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.xml b/tests/Metadata/Driver/xml/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<serializer>
+    <class name="JMS\Serializer\Tests\Fixtures\ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll" exclusion-policy="ALL">
+        <virtual-property method="getName" serialized-name="mood"/>
+    </class>
+</serializer>
diff --git a/tests/Metadata/Driver/yml/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.yml b/tests/Metadata/Driver/yml/ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll.yml
@@ -0,0 +1,5 @@
+JMS\Serializer\Tests\Fixtures\ObjectWithVirtualPropertiesAndDuplicatePropNameExcludeAll:
+  exclusion_policy:   all
+  virtual_properties:
+    getName:
+      serialized_name: mood