Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(metadata): fix generating discriminator dependencies #115

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/AutoMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,13 @@ public static function create(
$customTransformerRegistry = new PropertyTransformerRegistry($propertyTransformers);
$metadataRegistry = new MetadataRegistry($configuration);
$providerRegistry = new ProviderRegistry($providers);
$classDiscriminatorResolver = new ClassDiscriminatorResolver($classDiscriminatorFromClassMetadata);

$metadataFactory = MetadataFactory::create(
$configuration,
$customTransformerRegistry,
$metadataRegistry,
$classDiscriminatorResolver,
$transformerFactories,
$classMetadataFactory,
$nameConverter,
Expand All @@ -168,7 +171,7 @@ public static function create(
);

$mapperGenerator = new MapperGenerator(
new ClassDiscriminatorResolver($classDiscriminatorFromClassMetadata),
$classDiscriminatorResolver,
$configuration,
$expressionLanguage,
);
Expand Down
9 changes: 2 additions & 7 deletions src/Generator/InjectMapperMethodStatementsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace AutoMapper\Generator;

use AutoMapper\Generator\Shared\DiscriminatorStatementsGenerator;
use AutoMapper\Metadata\Dependency;
use AutoMapper\Metadata\GeneratorMetadata;
use PhpParser\Node\Arg;
Expand All @@ -29,10 +28,8 @@
*/
final readonly class InjectMapperMethodStatementsGenerator
{
public function __construct(
private DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorSource,
private DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorTarget,
) {
public function __construct()
{
}

/**
Expand Down Expand Up @@ -66,8 +63,6 @@ public function getStatements(Expr\Variable $automapperRegistryVariable, Generat

return [
...$injectMapperStatements,
...$this->discriminatorStatementsGeneratorSource->injectMapperStatements($metadata),
...$this->discriminatorStatementsGeneratorTarget->injectMapperStatements($metadata),
];
}
}
9 changes: 3 additions & 6 deletions src/Generator/MapperGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,14 @@ public function __construct(
);

$this->mapMethodStatementsGenerator = new MapMethodStatementsGenerator(
$discriminatorStatementsGeneratorSource = new DiscriminatorStatementsGenerator($classDiscriminatorResolver, true),
$discriminatorStatementsGeneratorTarget = new DiscriminatorStatementsGenerator($classDiscriminatorResolver, false),
new DiscriminatorStatementsGenerator($classDiscriminatorResolver, true),
new DiscriminatorStatementsGenerator($classDiscriminatorResolver, false),
$cachedReflectionStatementsGenerator,
$expressionLanguage,
$configuration->allowReadOnlyTargetToPopulate,
);

$this->injectMapperMethodStatementsGenerator = new InjectMapperMethodStatementsGenerator(
$discriminatorStatementsGeneratorSource,
$discriminatorStatementsGeneratorTarget
);
$this->injectMapperMethodStatementsGenerator = new InjectMapperMethodStatementsGenerator();

$this->disableGeneratedMapper = !$configuration->autoRegister;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Generator/Shared/ClassDiscriminatorResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function getDiscriminatorPropertyMetadata(GeneratorMetadata $metadata, bo
}

/**
* @return array<string, string>
* @return array<class-string<object>, string>
*/
public function discriminatorMapperNames(GeneratorMetadata $metadata, bool $fromSource): array
{
Expand Down
40 changes: 0 additions & 40 deletions src/Generator/Shared/DiscriminatorStatementsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,46 +27,6 @@ public function __construct(
) {
}

/**
* @return list<Stmt>
*/
public function injectMapperStatements(GeneratorMetadata $metadata): array
{
if (!$this->supports($metadata)) {
return [];
}

$discriminatorMapperNames = $this->classDiscriminatorResolver->discriminatorMapperNames($metadata, $this->fromSource);

$injectMapperStatements = [];

foreach ($discriminatorMapperNames as $typeTarget => $discriminatorMapperName) {
/*
* We inject dependencies for all the discriminator variant
*
* ```php
* $this->mappers['Discriminator_Mapper_VariantA'] = $autoMapperRegistry->getMapper($source, VariantA::class);
* $this->mappers['Discriminator_Mapper_VariantB'] = $autoMapperRegistry->getMapper($source, VariantB::class);
* ...
* ```
*/
$injectMapperStatements[] = new Stmt\Expression(
new Expr\Assign(
new Expr\ArrayDimFetch(
new Expr\PropertyFetch(new Expr\Variable('this'), 'mappers'),
new Scalar\String_($discriminatorMapperName)
),
new Expr\MethodCall(new Expr\Variable('autoMapperRegistry'), 'getMapper', [
new Arg(new Scalar\String_($this->fromSource ? $typeTarget : $metadata->mapperMetadata->source)),
new Arg(new Scalar\String_($this->fromSource ? $metadata->mapperMetadata->target : $typeTarget)),
])
)
);
}

return $injectMapperStatements;
}

/**
* @return list<Stmt>
*
Expand Down
39 changes: 38 additions & 1 deletion src/Metadata/MetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use AutoMapper\Extractor\ReadWriteTypeExtractor;
use AutoMapper\Extractor\SourceTargetMappingExtractor;
use AutoMapper\Extractor\WriteMutator;
use AutoMapper\Generator\Shared\ClassDiscriminatorResolver;
use AutoMapper\Transformer\AllowNullValueTransformerInterface;
use AutoMapper\Transformer\ArrayTransformerFactory;
use AutoMapper\Transformer\BuiltinTransformerFactory;
Expand All @@ -30,6 +31,7 @@
use AutoMapper\Transformer\DateTimeTransformerFactory;
use AutoMapper\Transformer\DependentTransformerInterface;
use AutoMapper\Transformer\EnumTransformerFactory;
use AutoMapper\Transformer\MapperDependency;
use AutoMapper\Transformer\MultipleTransformerFactory;
use AutoMapper\Transformer\NullableTransformerFactory;
use AutoMapper\Transformer\ObjectTransformerFactory;
Expand Down Expand Up @@ -65,6 +67,7 @@ public function __construct(
private readonly TransformerFactoryInterface $transformerFactory,
private readonly EventDispatcherInterface $eventDispatcher,
public readonly MetadataRegistry $metadataRegistry,
private readonly ClassDiscriminatorResolver $classDiscriminatorResolver,
) {
}

Expand All @@ -80,7 +83,7 @@ public function getGeneratorMetadata(string $source, string $target): GeneratorM
$metadata = $this->createGeneratorMetadata($this->metadataRegistry->get($source, $target));
$this->generatorMetadata[$source][$target] = $metadata;

// Add dependencies to the mapper
// Add dependencies from transformer to the mapper
foreach ($metadata->propertiesMetadata as $propertyMapping) {
if ($propertyMapping->transformer instanceof DependentTransformerInterface) {
foreach ($propertyMapping->transformer->getDependencies() as $mapperDependency) {
Expand All @@ -90,6 +93,25 @@ public function getGeneratorMetadata(string $source, string $target): GeneratorM
}
}
}

// Add dependencies from discriminator to the mapper
if ($this->classDiscriminatorResolver->hasClassDiscriminator($metadata, true)) {
foreach ($this->classDiscriminatorResolver->discriminatorMapperNames($metadata, true) as $newSourceType => $mapperDependencyName) {
$dependencyMetadata = $this->getGeneratorMetadata($newSourceType, $metadata->mapperMetadata->target);
$mapperDependency = new MapperDependency($mapperDependencyName, $newSourceType, $metadata->mapperMetadata->target);

$metadata->addDependency(new Dependency($mapperDependency, $dependencyMetadata));
}
}

if ($this->classDiscriminatorResolver->hasClassDiscriminator($metadata, false)) {
foreach ($this->classDiscriminatorResolver->discriminatorMapperNames($metadata, false) as $newTargetType => $mapperDependencyName) {
$dependencyMetadata = $this->getGeneratorMetadata($metadata->mapperMetadata->source, $newTargetType);
$mapperDependency = new MapperDependency($mapperDependencyName, $metadata->mapperMetadata->source, $newTargetType);

$metadata->addDependency(new Dependency($mapperDependency, $dependencyMetadata));
}
}
}

return $this->generatorMetadata[$source][$target];
Expand Down Expand Up @@ -117,6 +139,19 @@ public function resolveAllMetadata(MetadataRegistry $metadataRegistry): void
}
}
}

// Add dependencies from discriminator to the mapper
if ($this->classDiscriminatorResolver->hasClassDiscriminator($generatorMetadata, true)) {
foreach ($this->classDiscriminatorResolver->discriminatorMapperNames($generatorMetadata, true) as $newSourceType => $mapperDependencyName) {
$remainingMetadata[] = $metadataRegistry->get($newSourceType, $generatorMetadata->mapperMetadata->target);
}
}

if ($this->classDiscriminatorResolver->hasClassDiscriminator($generatorMetadata, false)) {
foreach ($this->classDiscriminatorResolver->discriminatorMapperNames($generatorMetadata, false) as $newTargetType => $mapperDependencyName) {
$remainingMetadata[] = $metadataRegistry->get($generatorMetadata->mapperMetadata->source, $newTargetType);
}
}
}
}

Expand Down Expand Up @@ -286,6 +321,7 @@ public static function create(
Configuration $configuration,
PropertyTransformerRegistry $customTransformerRegistry,
MetadataRegistry $metadataRegistry,
ClassDiscriminatorResolver $classDiscriminatorResolver,
array $transformerFactories = [],
ClassMetadataFactory $classMetadataFactory = null,
AdvancedNameConverterInterface $nameConverter = null,
Expand Down Expand Up @@ -376,6 +412,7 @@ public static function create(
$transformerFactory,
$eventDispatcher,
$metadataRegistry,
$classDiscriminatorResolver,
);
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/Resources/config/metadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use AutoMapper\Extractor\FromSourceMappingExtractor;
use AutoMapper\Extractor\FromTargetMappingExtractor;
use AutoMapper\Extractor\SourceTargetMappingExtractor;
use AutoMapper\Generator\Shared\ClassDiscriminatorResolver;
use AutoMapper\Metadata\MetadataFactory;
use AutoMapper\Transformer\TransformerFactoryInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
Expand Down Expand Up @@ -47,6 +48,7 @@
service(TransformerFactoryInterface::class),
service(EventDispatcherInterface::class),
service('automapper.config_mapping_registry'),
service(ClassDiscriminatorResolver::class),
])
;
};
4 changes: 2 additions & 2 deletions src/Transformer/MapperDependency.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
final readonly class MapperDependency
{
/**
* @param class-string $source
* @param class-string $target
* @param class-string<object>|'array' $source
* @param class-string<object>|'array' $target
*/
public function __construct(
public string $name,
Expand Down
8 changes: 4 additions & 4 deletions src/Transformer/ObjectTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private function getDependencyName(): string
}

/**
* @return class-string<mixed>|'array'
* @return class-string<object>|'array'
*/
private function getSource(): string
{
Expand All @@ -105,7 +105,7 @@ private function getSource(): string
/**
* Cannot be null since we check the source type is an Object.
*
* @var class-string<mixed> $sourceTypeName
* @var class-string<object> $sourceTypeName
*/
$sourceTypeName = $this->sourceType->getClassName();
}
Expand All @@ -114,7 +114,7 @@ private function getSource(): string
}

/**
* @return class-string<mixed>|'array'
* @return class-string<object>|'array'
*/
private function getTarget(): string
{
Expand All @@ -124,7 +124,7 @@ private function getTarget(): string
/**
* Cannot be null since we check the target type is an Object.
*
* @var class-string<mixed> $targetTypeName
* @var class-string<object> $targetTypeName
*/
$targetTypeName = $this->targetType->getClassName();
}
Expand Down
4 changes: 3 additions & 1 deletion tests/Bundle/Resources/App/Api/Processor/BookProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = [])
{
if (!$data instanceof Book) {
return;
return null;
}

if ($operation instanceof HttpOperation && $operation->getMethod() === 'POST') {
$data->id = random_int(1, 1000);
}

return $data;
}
}
1 change: 1 addition & 0 deletions tests/Bundle/Resources/config/packages/automapper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ automapper:
mappers:
- { source: 'AutoMapper\Tests\Bundle\Resources\App\Entity\NestedObject', target: 'array' }
- { source: 'AutoMapper\Tests\Bundle\Resources\App\Entity\AddressDTO', target: 'array', reverse: true }
- { source: 'AutoMapper\Tests\Bundle\Resources\App\Entity\Pet', target: 'array', reverse: true }
- { source: 'AutoMapper\Tests\Bundle\Resources\App\Entity\UserDTO', target: 'array', reverse: false }
- { source: 'array', target: 'AutoMapper\Tests\Bundle\Resources\App\Entity\Order', reverse: false }
- { source: 'AutoMapper\Tests\Bundle\Resources\App\Api\Entity\Book', target: 'array', reverse: true }
3 changes: 3 additions & 0 deletions tests/Bundle/ServiceInstantiationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public function testWarmup(): void
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_NestedObject_array.php');
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_User_array.php');
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_AddressDTO_array.php');
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_Pet_array.php');
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_Dog_array.php');
self::assertFileExists(__DIR__ . '/Resources/var/cache/test/automapper/Symfony_Mapper_AutoMapper_Tests_Bundle_Resources_App_Entity_Cat_array.php');
}

public function testAutoMapper(): void
Expand Down
4 changes: 3 additions & 1 deletion tests/Metadata/MetadataFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use AutoMapper\Extractor\FromSourceMappingExtractor;
use AutoMapper\Extractor\FromTargetMappingExtractor;
use AutoMapper\Extractor\SourceTargetMappingExtractor;
use AutoMapper\Generator\Shared\ClassDiscriminatorResolver;
use AutoMapper\Metadata\GeneratorMetadata;
use AutoMapper\Metadata\MetadataFactory;
use AutoMapper\Metadata\MetadataRegistry;
Expand Down Expand Up @@ -88,7 +89,8 @@ protected function setUp(): void
$fromTargetMappingExtractor,
$transformerFactory,
new EventDispatcher(),
new MetadataRegistry($configuration)
new MetadataRegistry($configuration),
new ClassDiscriminatorResolver()
);
}

Expand Down
Loading