symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.75k stars 9.46k forks source link

[Serializer] Mapping loaders can differ in production and dev due to independent loader constructor arguments. #43346

Closed silverbackdan closed 2 years ago

silverbackdan commented 3 years ago

Description The services 'serializer.mapping.chain_loader' and 'serializer.mapping.cache_warmer' both call the ClassMetadataFactory::getMetadataFor method with their own loaders in the constructor arguments. It'd be preferable for the chain_loader constructor arguments to be used in the cache_warmer, or for the ClassMetadata not to be generated again.

Example I have a compiler pass that looks like this

<?php

/*
 * This file is part of the Silverback API Components Bundle Project
 *
 * (c) Daniel West <daniel@silverback.is>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

declare(strict_types=1);

namespace Silverback\ApiComponentsBundle\DependencyInjection\CompilerPass;

use Silverback\ApiComponentsBundle\Serializer\MappingLoader\CwaResourceLoader;
use Silverback\ApiComponentsBundle\Serializer\MappingLoader\PublishableLoader;
use Silverback\ApiComponentsBundle\Serializer\MappingLoader\TimestampedLoader;
use Silverback\ApiComponentsBundle\Serializer\MappingLoader\UploadableLoader;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;

/**
 * @author Vincent Chalamon <vincent@les-tilleuls.coop>
 */
class SerializerCompilerPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $definitons = ['serializer.mapping.chain_loader', 'serializer.mapping.cache_warmer'];
        foreach ($definitons as $definitonId) {
            $definition = $container->getDefinition($definitonId);
            $definition->replaceArgument(
                0,
                array_merge(
                    $definition->getArgument(0),
                    [
                        new Reference(PublishableLoader::class),
                        new Reference(TimestampedLoader::class),
                        new Reference(CwaResourceLoader::class),
                        new Reference(UploadableLoader::class),
                    ]
                )
            );
        }
    }
}

I have to populate my additional loaders into the constructor argument of both services. Otherwise the cache_warmer will override the metadata in production only. So the behaviour in dev and production differs.

Do you think it'd be worth passing the chain loader as a service to the cache warmer perhaps to share the metadata that's already been generated?

I only raise this because it took a little while for me to work out why this was happening. In production, if I did not add the loaders to the cache warmer, then if I specified any annotation or yaml configuration for a PHP class, it'd override the metadata instead of merging it.

carsonbot commented 2 years ago

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

silverbackdan commented 2 years ago

I can't remember, but I did have this solution in place so I have not felt the need to revisit it.

xabbuh commented 2 years ago

Let's close here for now then and we can consider to reopen once some comes back with the same problem. Thank you for the feedback @silverbackdan!