Regression since 2.4.1

[bpolaszek]

Hello there,

I'd like to report a bug related to #657 merged in 2.4.1.
I've got a Symfony 4.1 app running on PHP 7.1+ which was using this bundle on version ^2.3.

A few months ago I have overridden the ArrayCollectionHandler the following way:

use Doctrine\Common\Collections\Collection;
use JMS\Serializer\Context;
use JMS\Serializer\Handler\ArrayCollectionHandler;
use JMS\Serializer\VisitorInterface;

class CollectionHandler extends ArrayCollectionHandler
{
    /**
     * @param VisitorInterface $visitor
     * @param Collection       $collection
     * @param array            $type
     * @param Context          $context
     * @return array
     */
    public function serializeCollection(VisitorInterface $visitor, Collection $collection, array $type, Context $context)
    {
        $array = parent::serializeCollection($visitor, $collection, $type, $context);
        return array_values($array); // Prevent serializing an associative array id => model
    }
}

I did this because my ArrayCollection are generated so as to use the entity Id as key and the entity as value into my ArrayCollections. But when these collections are serialized, I need them to be processed as a sequential array to be correctly stored and queried in ElasticSearch.

Since I upgraded the bundle to 2.4.1 my subscribing handler is not called anymore, despite its jms_serializer.subscribing_handler tag. The original jms_serializer.array_collection_handler is called instead.

When diving into the bundle's code, It looks like the $sorter closure is someway incorrect in JMS\SerializerBundle\DependencyInjection\Compiler\CustomHandlersPass::sortAndFlattenHandlersList:

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;
    }

When I force the use of self::stable_uasort(), (thus simulating a PHP5+ version), everything works correctly.

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 (true) {
            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;
    }

Thank you,
Ben

[goetas]

can you tell the exact php version you are running?

[bpolaszek]

@goetas PHP 7.2.4 on my iMac, PHP 7.1.12 on my FreeBSD production server (version constraint enforced in my composer.json).

From what I see on this bug a comment from CMB caught my attention:

[2015-08-13 13:31 UTC] cmb@php.net
AFAIK PHP 7 uses a stable sort algorithm for small arrays (< 16),
but for larger arrays the algorithm is still not stable.
Furthermore PHP makes no guarantee whether sorting with *sort() is
stable or not.

I did not find this information anywhere, but it seems that CMB is member of the PHP core team so this info might be considered true. In which case we should enforce the self::stable_uasort() method. In my case count($allHandlers) === 106.

What do you think?

[bpolaszek]

Just in case of, #664 & #665.

[goetas]

AFAIK PHP 7 uses a stable sort algorithm for small arrays (< 16),
but for larger arrays the algorithm is still not stable.

😱 😱 😱 😱 😱

[goetas]

fixed by #664

[bpolaszek]

Thanks @goetas !