bug #814 Ensure model implementations are unique in TargetEntitiesResolver (martijnc)

This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | fixes #813
| License         | MIT



Some models (like the `Order` models) can end up twice in the resulting `$interfaces` array (once through `sylius.order` and again through `sylius.promotion_subject`). This meant these interfaces were incorrectly being filtered after the loop, and that Doctrine was unable to find these mappings for these interfaces unless the interfaces were explicitly configured (which is deprecated as of 1.6).

`TargetEntitiesResolver::resolve` constructs an array that maps interfaces to their implementations (`$interfaces`). This is a list of interfaces that are implemented by at least one resource model from `$resourcesConfiguration`. Interfaces that have multiple implementations are filtered out so that non-unique interface (like `Sylius\Component\Resource\Model\TimestampableInterface`) are not mapped to a random implementation.

The list of implementations for an interface can contain duplicates. This happens for all the interface on Order because the Order model is processed twice, once for the `sylius.order` resource and once for the `sylius.promotion_subject` resource.

So `TargetEntitiesResolver::resolve` is normally called with all Sylius resources, but lets say it only contains `sylius.order` and `sylius.promotion_subject`  for simplicity:

```php
[
    'sylius.order' => [
        'driver' => 'doctrine/orm',
        'classes' => [
            'model' => 'App\Entity\Order\Order',
            'controller' => 'Sylius\Bundle\CoreBundle\Controller\OrderController',
            'repository' => 'App\Repository\Order\OrderRepository',
            'interface' => 'Sylius\Component\Order\Model\OrderInterface',
            'factory' => 'Sylius\Component\Resource\Factory\Factory',
            'form' => 'Sylius\Bundle\OrderBundle\Form\Type\OrderType',
        ]
    ],
    'sylius.promotion_subject' => [
        'driver' => 'doctrine/orm',
        'classes' => [
                'model' => 'App\Entity\Order\Order',
            ]
    ]
]
```

This means ` App\Entity\Order\Order` is processed twice.  After the loop (line 38), the `$interfaces` array contains:

```php
[
    'Sylius\Component\Order\Model\OrderInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Resource\Model\TimestampableInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Order\Model\AdjustableInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Core\Model\OrderInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Promotion\Model\PromotionSubjectInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Channel\Model\ChannelAwareInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Customer\Model\CustomerAwareInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Promotion\Model\PromotionCouponAwarePromotionSubjectInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Promotion\Model\CountablePromotionSubjectInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'Sylius\Component\Payment\Model\PaymentsSubjectInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'App\Entity\Order\OrderInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'MyCompany\MyBundle\Model\Order\OrderInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
    'SyliusMolliePlugin\Entity\OrderInterface' => [
        0 => 'App\Entity\Order\Order',
        1 => 'App\Entity\Order\Order',
    ],
]
```

Every interface on the Order model maps to the Order implementation twice. The `array_filter` (line 38) then filters them all out, because it thinks these interfaces map to multiple implementations.
Most interface in this list would of course be filter out anyways because they are implemented by other models, but `SyliusMolliePlugin\Entity\OrderInterface`, `App\Entity\Order\OrderInterface`, `MyCompany\MyBundle\Model\Order\OrderInterface`, and `Sylius\Component\Core\Model\OrderInterface` are not.

Because these interfaces are filtered, trying to get the repository for them fails because Doctrine does not have a mapping for them:

```php
// Repository not found
$repository = $this->managerRegistry->getRepository(MyCompany\MyBundle\Model\Order\OrderInterface::class);
```

This can be fixed by configuring this interface in the resource configuration, but that is deprecated and means another interface will no longer work.

This change ensures the interfaces to models map (`$interfaces`) cannot contain the same model twice for an interface and thus that these models are no longer incorrectly filtered.



Commits
-------

2337f5b Ensure model implementations are unique in `TargetEntitiesResolver`

Some models (like the `Order` models) can end up twice in the resulting `$interfaces` array (once through `sylius.order` and again through `sylius.promotion_subject`). This meant these interfaces were incorrectly being filtered after the loop, and that Doctrine was unable to find these mappings for these interfaces unless the interfaces were explicitly configured (which is deprecated as of 1.6).

This change ensures the interfaces to models map cannot contain duplicate models for an interface and thus that these models are no longer incorrectly filtered because the code after the loop thinks these interfaces have multiple implementations.
a368efd Add test for `TargetEntitiesResolver` with duplicated model

The `Fly` model is used by both `app.fly` and `app.another_resource_with_fly_model` and its interfaces should be part of the result.

Author: lchrusciel

src/Bundle/DependencyInjection/Compiler/Helper/TargetEntitiesResolver.php

@@ -31,6 +31,10 @@ public function resolve(array $resourcesConfiguration): array
                     continue;
                 }
 
+                if (isset($interfaces[$interface]) && in_array($model, $interfaces[$interface], true)) {
+                    continue;
+                }
+
                 $interfaces[$interface][] = $model;
             }
         }

src/Bundle/spec/DependencyInjection/Compiler/Helper/TargetEntitiesResolverSpec.php

@@ -69,6 +69,18 @@ function it_autodiscovers_only_unique_interfaces_based_on_model_classes(): void
         $this->resolve($config)->shouldNotHaveKeyWithValue(AnimalInterface::class, Bear::class);
     }
 
+    function it_autodiscovers_interfaces_on_models_when_passed_multiple_times(): void
+    {
+        $config = [
+            'app.fly' => ['classes' => ['model' => Fly::class]],
+            'app.another_resource_with_fly_model' => ['classes' => ['model' => Fly::class]],
+        ];
+
+        $this->resolve($config)->shouldHaveCount(2);
+        $this->resolve($config)->shouldHaveKeyWithValue(FlyInterface::class, Fly::class);
+        $this->resolve($config)->shouldHaveKeyWithValue(AnimalInterface::class, Fly::class);
+    }
+
     function it_uses_the_interface_defined_in_the_config(): void
     {
         $config = [