[Twig Hooks] Fix hookables overriding

src/TwigHooks/config/services.php

@@ -29,6 +29,7 @@
     $services->set('twig_hooks.registry.hookables', HookablesRegistry::class)
         ->args([
             tagged_iterator('twig_hooks.hookable'),
+            service('twig_hooks.merger.hookable'),
         ])
     ;
 

src/TwigHooks/config/services/hookable_merger.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Loader\Configurator;
+
+use Sylius\TwigHooks\Hookable\Merger\HookableMerger;
+use Sylius\TwigHooks\Hookable\Merger\HookableMergerInterface;
+
+return static function (ContainerConfigurator $configurator): void {
+    $services = $configurator->services();
+
+    $services->set('twig_hooks.merger.hookable', HookableMerger::class)
+        ->alias(HookableMergerInterface::class, 'twig_hooks.renderer.hookable')
+    ;
+};

src/TwigHooks/src/Hookable/AbstractHookable.php

@@ -41,4 +41,9 @@ public function isEnabled(): bool
     abstract public function overwriteWith(self $hookable): self;
 
     abstract public function getType(): string;
+
+    /**
+     * @return array<string, mixed>
+     */
+    abstract public function toArray(): array;
 }

src/TwigHooks/src/Hookable/HookableComponent.php

@@ -59,4 +59,18 @@ public function overwriteWith(AbstractHookable $hookable): AbstractHookable
             $hookable->isEnabled(),
         );
     }
+
+    public function toArray(): array
+    {
+        return [
+            'hookName' => $this->hookName,
+            'name' => $this->name,
+            'target' => $this->target,
+            'props' => $this->props,
+            'context' => $this->context,
+            'configuration' => $this->configuration,
+            'priority' => $this->getPriority(),
+            'enabled' => $this->isEnabled(),
+        ];
+    }
 }

src/TwigHooks/src/Hookable/HookableTemplate.php

@@ -33,4 +33,17 @@ public function overwriteWith(AbstractHookable $hookable): AbstractHookable
             $hookable->isEnabled(),
         );
     }
+
+    public function toArray(): array
+    {
+        return [
+            'hookName' => $this->hookName,
+            'name' => $this->name,
+            'target' => $this->target,
+            'context' => $this->context,
+            'configuration' => $this->configuration,
+            'priority' => $this->getPriority(),
+            'enabled' => $this->isEnabled(),
+        ];
+    }
 }

src/TwigHooks/src/Hookable/Merger/HookableMerger.php

@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\TwigHooks\Hookable\Merger;
+
+use Sylius\TwigHooks\Hookable\AbstractHookable;
+
+final class HookableMerger implements HookableMergerInterface
+{
+    /**
+     * @throws \ReflectionException
+     */
+    public function merge(AbstractHookable ...$hookables): AbstractHookable
+    {
+        if ([] === $hookables) {
+            throw new \InvalidArgumentException('At least one hookable must be passed to merge.');
+        }
+
+        /** @var class-string<AbstractHookable> $class */
+        $class = get_class(end($hookables));
+
+        $serializedHookables = array_map(
+            static fn (AbstractHookable $hookable): array => $hookable->toArray(),
+            $hookables,
+        );
+
+        $inputs = array_merge(...$serializedHookables);
+        $arguments = $this->createConstructorArguments($class, $inputs);
+
+        return new $class(...$arguments);
+    }
+
+
+    /**
+     * @param class-string $class
+     * @param array<string, mixed> $inputs
+     *
+     * @return array<string>
+     * @throws \ReflectionException
+     */
+    private function createConstructorArguments(string $class, array $inputs): array
+    {
+        $reflection = new \ReflectionClass($class);
+        /** @var \ReflectionMethod $constructor */
+        $constructor = $reflection->getConstructor();
+        $parameters = array_map(
+            static fn (\ReflectionParameter $parameter): string => $parameter->getName(),
+            $constructor->getParameters(),
+        );
+
+        $arguments = [];
+
+        foreach ($inputs as $inputName => $input) {
+            if (!in_array($inputName, $parameters, true)) {
+                continue;
+            }
+
+            $arguments[$inputName] = $input;
+        }
+
+        return $arguments;
+    }
+}

src/TwigHooks/src/Hookable/Merger/HookableMergerInterface.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\TwigHooks\Hookable\Merger;
+
+use Sylius\TwigHooks\Hookable\AbstractHookable;
+
+interface HookableMergerInterface
+{
+    public function merge(AbstractHookable ...$hookables): AbstractHookable;
+}

src/TwigHooks/src/Registry/HookablesRegistry.php

@@ -6,6 +6,7 @@
 
 use Laminas\Stdlib\SplPriorityQueue;
 use Sylius\TwigHooks\Hookable\AbstractHookable;
+use Sylius\TwigHooks\Hookable\Merger\HookableMergerInterface;
 
 /** @internal */
 class HookablesRegistry
@@ -16,8 +17,10 @@ class HookablesRegistry
     /**
      * @param iterable<AbstractHookable> $hookables
      */
-    public function __construct(iterable $hookables)
-    {
+    public function __construct(
+        iterable $hookables,
+        private readonly HookableMergerInterface $hookableMerger,
+    ) {
         /** @var AbstractHookable $hookable */
         foreach ($hookables as $hookable) {
             if (!$hookable instanceof AbstractHookable) {
@@ -68,7 +71,7 @@ private function mergeHookables(array $hooksNames): array
 
             foreach ($hookables as $hookableName => $hookable) {
                 if (array_key_exists($hookableName, $mergedHookables)) {
-                    $hookable = $mergedHookables[$hookableName]->overwriteWith($hookable);
+                    $hookable = $this->hookableMerger->merge($mergedHookables[$hookableName], $hookable);
                 }
 
                 $mergedHookables[$hookableName] = $hookable;

src/TwigHooks/tests/Unit/Hookable/Merger/HookableMergerTest.php

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Sylius\TwigHooks\Unit\Hookable\Merger;
+
+use PHPUnit\Framework\TestCase;
+use Sylius\TwigHooks\Hookable\HookableComponent;
+use Sylius\TwigHooks\Hookable\HookableTemplate;
+use Sylius\TwigHooks\Hookable\Merger\HookableMerger;
+use Sylius\TwigHooks\Hookable\Merger\HookableMergerInterface;
+use Tests\Sylius\TwigHooks\Utils\MotherObject\HookableComponentMotherObject;
+use Tests\Sylius\TwigHooks\Utils\MotherObject\HookableTemplateMotherObject;
+
+final class HookableMergerTest extends TestCase
+{
+    public function testItReturnsUnchangedHookableWhenOnlyOneHookableIsPassed(): void
+    {
+        $hookable = HookableTemplateMotherObject::with([
+            'hookName' => 'some_hook',
+            'name' => 'some_template_hookable',
+            'target' => 'index.html.twig',
+        ]);
+
+        $result = $this->getTestSubject()->merge($hookable);
+
+        $this->assertInstanceOf(HookableTemplate::class, $result);
+        $this->assertSame('some_hook', $result->hookName);
+        $this->assertSame('some_template_hookable', $result->name);
+        $this->assertSame('index.html.twig', $result->target);
+    }
+
+    public function testItOverridesEarlierHookableWithLaterHookableWithSameTypes(): void
+    {
+        $earlierHookable = HookableTemplateMotherObject::with([
+            'hookName' => 'common.some_hook',
+            'name' => 'some_template',
+            'target' => 'index.html.twig',
+        ]);
+        $laterHookable = HookableTemplateMotherObject::with([
+            'hookName' => 'app.some_hook',
+            'name' => 'some_template',
+            'target' => 'base.html.twig',
+        ]);
+
+        $result = $this->getTestSubject()->merge($earlierHookable, $laterHookable);
+
+        $this->assertInstanceOf(HookableTemplate::class, $result);
+        $this->assertSame('app.some_hook', $result->hookName);
+        $this->assertSame('some_template', $result->name);
+        $this->assertSame('base.html.twig', $result->target);
+    }
+
+    public function testItOverridesEarlierHookableWithLaterHookableWithDifferentTypesAndTheComponentIsTheLastOne(): void
+    {
+        $earlierHookable = HookableTemplateMotherObject::with([
+            'hookName' => 'common.some_hook',
+            'name' => 'some_template',
+            'target' => 'index.html.twig',
+        ]);
+        $laterHookable = HookableComponentMotherObject::with([
+            'hookName' => 'app.some_hook',
+            'name' => 'some_template',
+            'target' => 'app:form',
+        ]);
+
+        $result = $this->getTestSubject()->merge($earlierHookable, $laterHookable);
+
+        $this->assertInstanceOf(HookableComponent::class, $result);
+        $this->assertSame('app.some_hook', $result->hookName);
+        $this->assertSame('some_template', $result->name);
+        $this->assertSame('app:form', $result->target);
+    }
+
+    public function testItOverridesEarlierHookableWithLaterHookableWithDifferentTypesAndTheTemplateIsTheLastOne(): void
+    {
+        $earlierHookable = HookableComponentMotherObject::with([
+            'hookName' => 'common.some_hook',
+            'name' => 'some_template',
+            'target' => 'app:form',
+        ]);
+        $laterHookable = HookableTemplateMotherObject::with([
+            'hookName' => 'app.some_hook',
+            'name' => 'some_template',
+            'target' => 'index.html.twig',
+        ]);
+
+        $result = $this->getTestSubject()->merge($earlierHookable, $laterHookable);
+
+        $this->assertInstanceOf(HookableTemplate::class, $result);
+        $this->assertSame('app.some_hook', $result->hookName);
+        $this->assertSame('some_template', $result->name);
+        $this->assertSame('index.html.twig', $result->target);
+    }
+
+    private function getTestSubject(): HookableMergerInterface
+    {
+        return new HookableMerger();
+    }
+}

src/TwigHooks/tests/Unit/Registry/HookablesRegistryTest.php

@@ -7,11 +7,22 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Sylius\TwigHooks\Hookable\AbstractHookable;
+use Sylius\TwigHooks\Hookable\Merger\HookableMergerInterface;
 use Sylius\TwigHooks\Registry\HookablesRegistry;
 use Tests\Sylius\TwigHooks\Utils\MotherObject\HookableTemplateMotherObject;
 
 final class HookablesRegistryTest extends TestCase
 {
+    /** @var HookableMergerInterface&MockObject */
+    private HookableMergerInterface $hookableMerger;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->hookableMerger = $this->createMock(HookableMergerInterface::class);
+    }
+
     public function testItThrowsAnExceptionWhenAtLeastOneElementIsNotAnAbstractHookable(): void
     {
         $this->expectException(\InvalidArgumentException::class);
@@ -42,6 +53,14 @@ public function testItMergesHookablesWithTheSameName(): void
         $hookableTwo = $this->createHookable('another_hook', 'hookable_one');
         $hookableThree = $this->createHookable('yet_another_hook', 'hookable_one', false);
 
+        $this->hookableMerger
+            ->expects($this->exactly(2))
+            ->method('merge')
+            ->willReturnCallback(static function (AbstractHookable $first, AbstractHookable $second): AbstractHookable {
+                return $second;
+            })
+        ;
+
         $hookable = $this->getTestSubject([$hookableOne, $hookableTwo, $hookableThree])
             ->getEnabledFor(['some_hook', 'another_hook', 'yet_another_hook'])
         ;
@@ -61,7 +80,7 @@ public function testItMergesHookablesWithTheSameName(): void
      */
     private function getTestSubject(iterable $hookables): HookablesRegistry
     {
-        return new HookablesRegistry($hookables);
+        return new HookablesRegistry($hookables, $this->hookableMerger);
     }
 
     /**

src/TwigHooks/tests/Utils/MotherObject/HookableComponentMotherObject.php

@@ -13,6 +13,23 @@ public static function some(): HookableComponent
         return new HookableComponent('some_hook', 'some_name', 'some_target');
     }
 
+    public static function with(array $parameters):HookableComponent
+    {
+        if (!array_key_exists('hookName', $parameters)) {
+            $parameters['hookName'] = 'some_hook';
+        }
+
+        if (!array_key_exists('name', $parameters)) {
+            $parameters['name'] = 'some_name';
+        }
+
+        if (!array_key_exists('target', $parameters)) {
+            $parameters['target'] = 'some_target';
+        }
+
+        return new HookableComponent(...$parameters);
+    }
+
     public static function withHookName(string $hookName): HookableComponent
     {
         return new HookableComponent($hookName, 'some_name', 'some_target');