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

[RFC] Treat some ArrayAccess classes like array or stdClass #144

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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: 3 additions & 2 deletions src/AutoMapper.php
Original file line number Diff line number Diff line change
@@ -108,8 +108,9 @@ public function map(array|object $source, string|array|object $target, array $co
$targetType = $target;
}

if ('array' === $sourceType && 'array' === $targetType) {
throw new InvalidMappingException('Cannot map this value, both source and target are array.');
if (('array' === $sourceType || $this->metadataRegistry->isArrayAccess($sourceType))
&& ('array' === $targetType || $this->metadataRegistry->isArrayAccess($targetType))) {
throw new InvalidMappingException('Cannot map this value, both source and target are array or ArrayAccess.');
}

return $this->getMapper($sourceType, $targetType)->map($source, $context);
21 changes: 21 additions & 0 deletions src/Configuration.php
Original file line number Diff line number Diff line change
@@ -4,8 +4,19 @@

namespace AutoMapper;

use MongoDB\BSON\Document;
use MongoDB\Model\BSONDocument;

final readonly class Configuration
{
/**
* @var list<class-string<\ArrayAccess<string, mixed>>>
*/
public array $arrayAccessClasses;

/**
* @param list<class-string<\ArrayAccess<string, mixed>>> $arrayAccessClasses classes with unknown properties, implemeting ArrayAccess
*/
public function __construct(
/**
* Class prefix used to prefix the class name of the generated mappers.
@@ -36,6 +47,16 @@ public function __construct(
* Does the mapper should throw an exception if the target is read-only.
*/
public bool $allowReadOnlyTargetToPopulate = false,
array $arrayAccessClasses = [],
) {
$arrayAccessClasses[] = \ArrayObject::class;

// Classes provided by the mongodb extension and mongodb/mongodb package
if (class_exists(Document::class, false)) {
$arrayAccessClasses[] = Document::class;
$arrayAccessClasses[] = BSONDocument::class;
}

$this->arrayAccessClasses = $arrayAccessClasses;
}
}
6 changes: 3 additions & 3 deletions src/Extractor/MappingExtractor.php
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ public function __construct(
*/
public function getProperties(string $class): iterable
{
if ($class === 'array' || $class === \stdClass::class) {
if ($class === 'array' || $class === \stdClass::class || \in_array($class, $this->configuration->arrayAccessClasses, true)) {
return [];
}

@@ -40,7 +40,7 @@ public function getProperties(string $class): iterable

public function getReadAccessor(string $class, string $property): ?ReadAccessor
{
if ('array' === $class) {
if ('array' === $class || \in_array($class, $this->configuration->arrayAccessClasses, true)) {
return new ReadAccessor(ReadAccessor::TYPE_ARRAY_DIMENSION, $property);
}

@@ -107,7 +107,7 @@ public function getWriteMutator(string $source, string $target, string $property

public function getCheckExists(string $class, string $property): bool
{
if ('array' === $class || \stdClass::class === $class) {
if ('array' === $class || \stdClass::class === $class || \in_array($class, $this->configuration->arrayAccessClasses, true)) {
return true;
}

4 changes: 0 additions & 4 deletions src/Generator/CreateTargetStatementsGenerator.php
Original file line number Diff line number Diff line change
@@ -109,10 +109,6 @@ private function targetAsStdClass(GeneratorMetadata $metadata): ?Stmt
*/
private function constructorArguments(GeneratorMetadata $metadata): array
{
if (!$metadata->isTargetUserDefined()) {
return [];
}

Comment on lines -112 to -115
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why there is it condition. It was introduced by 1486a85#diff-538038bd35959095e72795c2a102193df6a1ee0c17b27dd7f129e23d8f2464ecR111-R113
This prevent instanciating new ArrayObject.

$targetConstructor = $metadata->mapperMetadata->targetReflectionClass?->getConstructor();

if (!$targetConstructor || !$metadata->hasConstructor()) {
4 changes: 3 additions & 1 deletion src/Generator/MapMethodStatementsGenerator.php
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

namespace AutoMapper\Generator;

use AutoMapper\Configuration;
use AutoMapper\Exception\ReadOnlyTargetException;
use AutoMapper\Generator\Shared\CachedReflectionStatementsGenerator;
use AutoMapper\Generator\Shared\DiscriminatorStatementsGenerator;
@@ -27,6 +28,7 @@
private PropertyStatementsGenerator $propertyStatementsGenerator;

public function __construct(
Configuration $configuration,
DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorSource,
DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorTarget,
CachedReflectionStatementsGenerator $cachedReflectionStatementsGenerator,
@@ -37,7 +39,7 @@ public function __construct(
$discriminatorStatementsGeneratorTarget,
$cachedReflectionStatementsGenerator,
);
$this->propertyStatementsGenerator = new PropertyStatementsGenerator($expressionLanguage);
$this->propertyStatementsGenerator = new PropertyStatementsGenerator($configuration, $expressionLanguage);
}

/**
1 change: 1 addition & 0 deletions src/Generator/MapperGenerator.php
Original file line number Diff line number Diff line change
@@ -44,6 +44,7 @@ public function __construct(
);

$this->mapMethodStatementsGenerator = new MapMethodStatementsGenerator(
$configuration,
new DiscriminatorStatementsGenerator($classDiscriminatorResolver, true),
new DiscriminatorStatementsGenerator($classDiscriminatorResolver, false),
$cachedReflectionStatementsGenerator,
23 changes: 23 additions & 0 deletions src/Generator/PropertyConditionsGenerator.php
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

namespace AutoMapper\Generator;

use AutoMapper\Configuration;
use AutoMapper\Exception\CompileException;
use AutoMapper\MapperContext;
use AutoMapper\Metadata\GeneratorMetadata;
@@ -30,6 +31,7 @@
private Parser $parser;

public function __construct(
private Configuration $configuration,
private ExpressionLanguage $expressionLanguage,
Parser $parser = null,
) {
@@ -42,6 +44,7 @@ public function generate(GeneratorMetadata $metadata, PropertyMetadata $property

$conditions[] = $this->propertyExistsForStdClass($metadata, $propertyMetadata);
$conditions[] = $this->propertyExistsForArray($metadata, $propertyMetadata);
$conditions[] = $this->propertyExistsForArrayAccess($metadata, $propertyMetadata);
$conditions[] = $this->isAllowedAttribute($metadata, $propertyMetadata);

if (!$propertyMetadata->disableGroupsCheck) {
@@ -120,6 +123,26 @@ private function propertyExistsForArray(GeneratorMetadata $metadata, PropertyMet
]);
}

/**
* In case of source is an ArrayAccess implementation listed in the config.
*
* ```php
* $source->offsetExists('propertyName').
* ```
*/
private function propertyExistsForArrayAccess(GeneratorMetadata $metadata, PropertyMetadata $propertyMetadata): ?Expr
{
if (!$propertyMetadata->source->checkExists || !\in_array($metadata->mapperMetadata->source, $this->configuration->arrayAccessClasses, true)) {
return null;
}

return new Expr\MethodCall(
$metadata->variableRegistry->getSourceInput(),
'offsetExists',
[new Arg(new Scalar\String_($propertyMetadata->source->property))],
);
}

/**
* In case of supporting attributes checking, we check if the property is allowed to be mapped.
*
4 changes: 3 additions & 1 deletion src/Generator/PropertyStatementsGenerator.php
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

namespace AutoMapper\Generator;

use AutoMapper\Configuration;
use AutoMapper\Extractor\WriteMutator;
use AutoMapper\Metadata\GeneratorMetadata;
use AutoMapper\Metadata\PropertyMetadata;
@@ -22,9 +23,10 @@
private PropertyConditionsGenerator $propertyConditionsGenerator;

public function __construct(
Configuration $configuration,
ExpressionLanguage $expressionLanguage
) {
$this->propertyConditionsGenerator = new PropertyConditionsGenerator($expressionLanguage);
$this->propertyConditionsGenerator = new PropertyConditionsGenerator($configuration, $expressionLanguage);
}

/**
5 changes: 3 additions & 2 deletions src/Metadata/MetadataFactory.php
Original file line number Diff line number Diff line change
@@ -172,11 +172,12 @@ private function createGeneratorMetadata(MapperMetadata $mapperMetadata): Genera
{
$extractor = $this->sourceTargetPropertiesMappingExtractor;

if ('array' === $mapperMetadata->source || 'stdClass' === $mapperMetadata->source) {
$classes = array_merge(['array', \stdClass::class], $this->configuration->arrayAccessClasses);
if (\in_array($mapperMetadata->source, $classes, true)) {
$extractor = $this->fromTargetPropertiesMappingExtractor;
}

if ('array' === $mapperMetadata->target || 'stdClass' === $mapperMetadata->target) {
if (\in_array($mapperMetadata->target, $classes, true)) {
$extractor = $this->fromSourcePropertiesMappingExtractor;
}

5 changes: 5 additions & 0 deletions src/Metadata/MetadataRegistry.php
Original file line number Diff line number Diff line change
@@ -110,4 +110,9 @@ public function count(): int
{
return \count($this->registry, \COUNT_RECURSIVE);
}

public function isArrayAccess(string $type): bool
{
return \in_array($type, $this->configuration->arrayAccessClasses, true);
}
}
Original file line number Diff line number Diff line change
@@ -65,6 +65,7 @@ public function load(array $configs, ContainerBuilder $container): void
->setArgument('$autoRegister', $config['auto_register'])
->setArgument('$mapPrivateProperties', $config['map_private_properties'])
->setArgument('$allowReadOnlyTargetToPopulate', $config['allow_readonly_target_to_populate'])
->setArgument('$arrayAccessClasses', $config['array_access_classes'])
;

$container->registerForAutoconfiguration(PropertyTransformerInterface::class)->addTag('automapper.property_transformer');
8 changes: 8 additions & 0 deletions src/Symfony/Bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
@@ -31,6 +31,14 @@ public function getConfigTreeBuilder(): TreeBuilder
->booleanNode('check_attributes')->defaultTrue()->end()
->booleanNode('auto_register')->defaultTrue()->end()
->booleanNode('map_private_properties')->defaultTrue()->end()
->arrayNode('array_access_classes')
->scalarPrototype()
->validate()
->ifTrue(fn ($className) => !is_subclass_of($className, \ArrayAccess::class))
->thenInvalid('Class %s does not implement "ArrayAccess"')
->end()
->end()
->end()
->booleanNode('allow_readonly_target_to_populate')->defaultFalse()->end()
->arrayNode('normalizer')
->children()
41 changes: 41 additions & 0 deletions tests/AutoMapperTest.php
Original file line number Diff line number Diff line change
@@ -53,6 +53,7 @@
use AutoMapper\Tests\Fixtures\Transformer\MoneyTransformerFactory;
use AutoMapper\Tests\Fixtures\Uninitialized;
use AutoMapper\Tests\Fixtures\UserPromoted;
use MongoDB\BSON\Document;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Serializer\Attribute\Groups;
@@ -210,6 +211,46 @@ public function testAutoMapperStdObjectToStdObject(): void
self::assertEquals($user, $userStd);
}

public function testAutoMapperFromArrayObject(): void
{
$this->buildAutoMapper(mapPrivatePropertiesAndMethod: true);

$user = new \ArrayObject(['id' => 1]);

/** @var Fixtures\UserDTO $userDto */
$userDto = $this->autoMapper->map($user, Fixtures\UserDTO::class);

self::assertInstanceOf(Fixtures\UserDTO::class, $userDto);
self::assertEquals(1, $userDto->id);
}

public function testAutoMapperToArrayObject(): void
{
$userDto = new Fixtures\UserDTO();
$userDto->id = 1;

$user = $this->autoMapper->map($userDto, \ArrayObject::class);

self::assertInstanceOf(\ArrayObject::class, $user);
self::assertEquals(1, $user['id']);
}

/**
* @requires extension mongodb
*/
public function testAutoMapperFromMongoDBDocument(): void
{
$this->buildAutoMapper(mapPrivatePropertiesAndMethod: true);

$user = Document::fromPHP(['id' => 1]);

/** @var Fixtures\UserDTO $userDto */
$userDto = $this->autoMapper->map($user, Fixtures\UserDTO::class);

self::assertInstanceOf(Fixtures\UserDTO::class, $userDto);
self::assertEquals(1, $userDto->id);
}

public function testNotReadable(): void
{
$this->buildAutoMapper(classPrefix: 'CustomDateTime_');
6 changes: 4 additions & 2 deletions tools/phpstan/phpstan.neon
Original file line number Diff line number Diff line change
@@ -2,9 +2,11 @@ parameters:
level: max
paths:
- ../../src/
scanFiles:
- stubs/mongodb.php

tmpDir: cache

ignoreErrors:
- "#Instantiated class fromIteratorToArray not found#"
- "#Instantiated class toArray not found#"
- "#Instantiated class fromIteratorToArray not found#"
- "#Instantiated class toArray not found#"
9 changes: 9 additions & 0 deletions tools/phpstan/stubs/mongodb.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace MongoDB\BSON {
abstract class Document implements \ArrayAccess {}
}

namespace MongoDB\Model {
abstract class BSONDocument implements \ArrayAccess {}
}
Loading