Merge pull request #658 from schmittjoh/discordier-hotfix/issue-656

Fix serialization handler registration by priority
schmittjoh · May 25, 2018 · 8f57c4a · 8f57c4a
2 parents 507d235 + 0088f04
commit 8f57c4a
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 36 deletions.
diff --git a/DependencyInjection/Compiler/CustomHandlersPass.php b/DependencyInjection/Compiler/CustomHandlersPass.php
@@ -15,11 +15,8 @@ public function process(ContainerBuilder $container)
     {
         $handlers = array();
         $handlerServices = array();
-        foreach ($this->findAndSortTaggedServices('jms_serializer.handler', $container) as $reference) {
-            $id = (string)$reference;
-            $definition = $container->getDefinition($id);
-            foreach ($definition->getTags() as $serviceTags) {
-                $attrs = $serviceTags[0];
+        foreach ($container->findTaggedServiceIds('jms_serializer.handler') as $id => $tags) {
+            foreach ($tags as $attrs) {
                 if (!isset($attrs['type'], $attrs['format'])) {
                     throw new \RuntimeException(sprintf('Each tag named "jms_serializer.handler" of service "%s" must have at least two attributes: "type" and "format".', $id));
                 }
@@ -35,20 +32,23 @@ public function process(ContainerBuilder $container)
 
                 foreach ($directions as $direction) {
                     $method = isset($attrs['method']) ? $attrs['method'] : HandlerRegistry::getDefaultMethod($direction, $attrs['type'], $attrs['format']);
-                    if (class_exists(ServiceLocatorTagPass::class) || $definition->isPublic()) {
-                        $handlerServices[$id] = $reference;
-                        $handlers[$direction][$attrs['type']][$attrs['format']] = array($id, $method);
+                    $priority = isset($attrs['priority']) ? intval($attrs['priority']) : 0;
+                    $ref = new Reference($id);
+                    if (class_exists(ServiceLocatorTagPass::class) || $container->getDefinition($id)->isPublic()) {
+                        $handlerServices[$id] = $ref;
+                        $handlers[] = array($direction, $attrs['type'], $attrs['format'], $priority, $id, $method);
                     } else {
-                        $handlers[$direction][$attrs['type']][$attrs['format']] = array($reference, $method);
+                        $handlers[] = array($direction, $attrs['type'], $attrs['format'], $priority, $ref, $method);
                     }
                 }
             }
         }
 
-        foreach ($this->findAndSortTaggedServices('jms_serializer.subscribing_handler', $container) as $reference) {
-            $id = (string)$reference;
-            $definition = $container->getDefinition($id);
-            $class = $definition->getClass();
+        foreach ($container->findTaggedServiceIds('jms_serializer.subscribing_handler') as $id => $tags) {
+
+            $def = $container->getDefinition($id);
+            $class = $def->getClass();
+
             $ref = new \ReflectionClass($class);
             if (!$ref->implementsInterface('JMS\Serializer\Handler\SubscribingHandlerInterface')) {
                 throw new \RuntimeException(sprintf('The service "%s" must implement the SubscribingHandlerInterface.', $id));
@@ -65,48 +65,71 @@ public function process(ContainerBuilder $container)
                 }
 
                 foreach ($directions as $direction) {
+                    $priority = isset($methodData['priority']) ? intval($methodData['priority']) : 0;
                     $method = isset($methodData['method']) ? $methodData['method'] : HandlerRegistry::getDefaultMethod($direction, $methodData['type'], $methodData['format']);
-                    if (class_exists(ServiceLocatorTagPass::class) || $definition->isPublic()) {
-                        $handlerServices[$id] = $reference;
-                        $handlers[$direction][$methodData['type']][$methodData['format']] = array($id, $method);
+
+                    $ref = new Reference($id);
+                    if (class_exists(ServiceLocatorTagPass::class) || $def->isPublic()) {
+                        $handlerServices[$id] = $ref;
+                        $handlers[] = array($direction, $methodData['type'], $methodData['format'], $priority, $id, $method);
                     } else {
-                        $handlers[$direction][$methodData['type']][$methodData['format']] = array($reference, $method);
+                        $handlers[] = array($direction, $methodData['type'], $methodData['format'], $priority, $ref, $method);
                     }
                 }
             }
         }
 
-        $container->findDefinition('jms_serializer.handler_registry')->addArgument($handlers);
+        $handlers = $this->sortAndFlattenHandlersList($handlers);
+
+        $container->findDefinition('jms_serializer.handler_registry')
+            ->addArgument($handlers);
 
         if (class_exists(ServiceLocatorTagPass::class)) {
             $serviceLocator = ServiceLocatorTagPass::register($container, $handlerServices);
             $container->findDefinition('jms_serializer.handler_registry')->replaceArgument(0, $serviceLocator);
         }
     }
 
+    private function sortAndFlattenHandlersList(array $allHandlers)
+    {
+        $sorter = function ($a, $b) {
+            return $b[3] == $a[3] ? 0 : ($b[3] > $a[3] ? 1 : -1);
+        };
+        // php 7 sorting is stable, while php 5 is not, and we need it stable to have consistent tests
+        if (PHP_MAJOR_VERSION < 7) {
+            self::stable_uasort($allHandlers, $sorter);
+        } else {
+            uasort($allHandlers, $sorter);
+        }
+        $handlers = [];
+        foreach ($allHandlers as $handler) {
+            list ($direction, $type, $format, $priority, $service, $method) = $handler;
+            $handlers[$direction][$type][$format] = [$service, $method];
+        }
+
+        return $handlers;
+    }
+
     /**
-     * Finds all services with the given tag name and order them by their priority.
+     * Performs stable sorting. Copied from http://php.net/manual/en/function.uasort.php#121283
      *
-     * @param string           $tagName
-     * @param ContainerBuilder $container
-     *
-     * @return Reference[]
+     * @param array $array
+     * @param $value_compare_func
+     * @return bool
      */
-    private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
+    private static function stable_uasort(array &$array, $value_compare_func)
     {
-        $services = array();
-
-        foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
-            $priority = isset($attributes[0]['priority']) ? intval($attributes[0]['priority']) : 0;
-
-            $services[$priority][] = new Reference($serviceId);
+        $index = 0;
+        foreach ($array as &$item) {
+            $item = array($index++, $item);
         }
-
-        if ($services) {
-            krsort($services);
-            $services = call_user_func_array('array_merge', $services);
+        $result = uasort($array, function ($a, $b) use ($value_compare_func) {
+            $result = call_user_func($value_compare_func, $a[1], $b[1]);
+            return $result == 0 ? $a[0] - $b[0] : $result;
+        });
+        foreach ($array as &$item) {
+            $item = $item[1];
         }
-
-        return $services;
+        return $result;
     }
 }
diff --git a/Tests/DependencyInjection/CustomHandlerPassTest.php b/Tests/DependencyInjection/CustomHandlerPassTest.php
@@ -224,6 +224,54 @@ public function testHandlerMustRespectPriorities()
         ], $args[1]);
     }
 
+    public function testHandlerCanBeRegisteredForMultipleTypesOrDirections()
+    {
+        $container = $this->getContainer();
+
+        $def = new Definition('JMS\Serializer\Foo');
+        $def->addTag('jms_serializer.handler', [
+            'type' => 'Custom',
+            'direction' => 'serialization',
+            'format' => 'json',
+            'method' => 'serialize',
+        ]);
+        $def->addTag('jms_serializer.handler', [
+            'type' => 'Custom',
+            'direction' => 'deserialization',
+            'format' => 'json',
+            'method' => 'deserialize',
+        ]);
+        $def->addTag('jms_serializer.handler', [
+            'type' => 'Custom<?>',
+            'direction' => 'serialization',
+            'format' => 'json',
+            'method' => 'serialize',
+        ]);
+        $def->addTag('jms_serializer.handler', [
+            'type' => 'Custom<?>',
+            'direction' => 'deserialization',
+            'format' => 'json',
+            'method' => 'deserialize',
+        ]);
+        $container->setDefinition('my_service', $def);
+
+        $pass = new CustomHandlersPass();
+        $pass->process($container);
+
+        $args = $container->getDefinition('jms_serializer.handler_registry')->getArguments();
+
+        $this->assertSame([
+            1 => [
+                'Custom' => ['json' => ['my_service', 'serialize']],
+                'Custom<?>' => ['json' => ['my_service', 'serialize']],
+            ],
+            2 => [
+                'Custom' => ['json' => ['my_service', 'deserialize']],
+                'Custom<?>' => ['json' => ['my_service', 'deserialize']],
+            ]
+        ], $args[1]);
+    }
+
     public function testSubscribingHandler()
     {
         $container = $this->getContainer();