From b99084ab4c4972aa1300b2810e89b6dfd5a73ee2 Mon Sep 17 00:00:00 2001 From: Josh Di Fabio Date: Thu, 7 Apr 2016 12:49:34 +0100 Subject: [PATCH 1/3] Make ComponentRegistrar::register() more graceful Some workflows can result in register() being called multiple times for the same component. In order to avoid messy boilerplate in client code, it makes sense for the register() method to be null-op in cases where the component has already been registered with the same path. --- .../Magento/Framework/Component/ComponentRegistrar.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/Component/ComponentRegistrar.php b/lib/internal/Magento/Framework/Component/ComponentRegistrar.php index ba7ebdf49dc43..2d4d81aabdda3 100644 --- a/lib/internal/Magento/Framework/Component/ComponentRegistrar.php +++ b/lib/internal/Magento/Framework/Component/ComponentRegistrar.php @@ -45,13 +45,13 @@ class ComponentRegistrar implements ComponentRegistrarInterface public static function register($type, $componentName, $path) { self::validateType($type); - if (isset(self::$paths[$type][$componentName])) { + if (!isset(self::$paths[$type][$componentName])) { + self::$paths[$type][$componentName] = str_replace('\\', '/', $path); + } elseif (str_replace('\\', '/', $path) !== self::$paths[$type][$componentName]) { throw new \LogicException( ucfirst($type) . ' \'' . $componentName . '\' from \'' . $path . '\' ' . 'has been already defined in \'' . self::$paths[$type][$componentName] . '\'.' ); - } else { - self::$paths[$type][$componentName] = str_replace('\\', '/', $path); } } From cb79ebd1f41d82e27dfc7023d58c52ecdfd841e2 Mon Sep 17 00:00:00 2001 From: Josh Di Fabio Date: Thu, 7 Apr 2016 12:50:30 +0100 Subject: [PATCH 2/3] Micro-optimization If the component has already been registered then $type must be valid --- lib/internal/Magento/Framework/Component/ComponentRegistrar.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Component/ComponentRegistrar.php b/lib/internal/Magento/Framework/Component/ComponentRegistrar.php index 2d4d81aabdda3..032b67c89a7a3 100644 --- a/lib/internal/Magento/Framework/Component/ComponentRegistrar.php +++ b/lib/internal/Magento/Framework/Component/ComponentRegistrar.php @@ -44,8 +44,8 @@ class ComponentRegistrar implements ComponentRegistrarInterface */ public static function register($type, $componentName, $path) { - self::validateType($type); if (!isset(self::$paths[$type][$componentName])) { + self::validateType($type); self::$paths[$type][$componentName] = str_replace('\\', '/', $path); } elseif (str_replace('\\', '/', $path) !== self::$paths[$type][$componentName]) { throw new \LogicException( From 7f6cc5ccfce4855470458052540754155eb3d443 Mon Sep 17 00:00:00 2001 From: Josh Di Fabio Date: Thu, 7 Apr 2016 13:13:12 +0100 Subject: [PATCH 3/3] Add test for register() null-op functionality --- .../Component/Test/Unit/ComponentRegistrarTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/internal/Magento/Framework/Component/Test/Unit/ComponentRegistrarTest.php b/lib/internal/Magento/Framework/Component/Test/Unit/ComponentRegistrarTest.php index fe68f9cbd18bd..49f065b0918c7 100644 --- a/lib/internal/Magento/Framework/Component/Test/Unit/ComponentRegistrarTest.php +++ b/lib/internal/Magento/Framework/Component/Test/Unit/ComponentRegistrarTest.php @@ -42,6 +42,12 @@ public function testGetPathsForModule() $this->assertContains($expected['test_module_one'], $this->object->getPaths(ComponentRegistrar::MODULE)); $this->assertContains($expected['test_module_two'], $this->object->getPaths(ComponentRegistrar::MODULE)); } + + public function testDuplicateRegistrationIsNullOp() + { + ComponentRegistrar::register(ComponentRegistrar::MODULE, "test_module_three", "some/path/name/three"); + ComponentRegistrar::register(ComponentRegistrar::MODULE, "test_module_three", "some/path/name/three"); + } /** * @expectedException \LogicException