diff --git a/plugins/woocommerce/changelog/improve-di-related-unit-testing-infrastructure b/plugins/woocommerce/changelog/improve-di-related-unit-testing-infrastructure new file mode 100644 index 00000000000..5420db24ffd --- /dev/null +++ b/plugins/woocommerce/changelog/improve-di-related-unit-testing-infrastructure @@ -0,0 +1,4 @@ +Significance: minor +Type: dev + +Improvements to the DI related unit testing infrastructure diff --git a/plugins/woocommerce/src/Internal/DependencyManagement/Definition.php b/plugins/woocommerce/src/Internal/DependencyManagement/Definition.php index 4cc9651f1e7..3295c0a97bf 100644 --- a/plugins/woocommerce/src/Internal/DependencyManagement/Definition.php +++ b/plugins/woocommerce/src/Internal/DependencyManagement/Definition.php @@ -25,15 +25,46 @@ class Definition extends BaseDefinition { * @return object */ protected function resolveClass( string $concrete ) { + $instance = new $concrete(); + $this->invokeInit( $instance ); + return $instance; + } + + /** + * Invoke methods on resolved instance, including 'init'. + * + * @param object $instance The concrete to invoke methods on. + * + * @return object + */ + protected function invokeMethods( $instance ) { + $this->invokeInit( $instance ); + parent::invokeMethods( $instance ); + return $instance; + } + + /** + * Invoke the 'init' method on a resolved object. + * + * Constructor injection causes backwards compatibility problems + * so we will rely on method injection via an internal method. + * + * @param object $instance The resolved object. + * @return void + */ + private function invokeInit( $instance ) { $resolved = $this->resolveArguments( $this->arguments ); - $concrete = new $concrete(); - // Constructor injection causes backwards compatibility problems - // so we will rely on method injection via an internal method. - if ( method_exists( $concrete, static::INJECTION_METHOD ) ) { - call_user_func_array( array( $concrete, static::INJECTION_METHOD ), $resolved ); + if ( method_exists( $instance, static::INJECTION_METHOD ) ) { + call_user_func_array( array( $instance, static::INJECTION_METHOD ), $resolved ); } + } - return $concrete; + /** + * Forget the cached resolved object, so the next time it's requested + * it will be resolved again. + */ + public function forgetResolved() { + $this->resolved = null; } } diff --git a/plugins/woocommerce/src/Internal/DependencyManagement/ExtendedContainer.php b/plugins/woocommerce/src/Internal/DependencyManagement/ExtendedContainer.php index e78ebda711e..d3522c7be26 100644 --- a/plugins/woocommerce/src/Internal/DependencyManagement/ExtendedContainer.php +++ b/plugins/woocommerce/src/Internal/DependencyManagement/ExtendedContainer.php @@ -6,6 +6,8 @@ namespace Automattic\WooCommerce\Internal\DependencyManagement; use Automattic\WooCommerce\Container; +use Automattic\WooCommerce\Proxies\LegacyProxy; +use Automattic\WooCommerce\Testing\Tools\DependencyManagement\MockableLegacyProxy; use Automattic\WooCommerce\Utilities\StringUtil; use Automattic\WooCommerce\Vendor\League\Container\Container as BaseContainer; use Automattic\WooCommerce\Vendor\League\Container\Definition\DefinitionInterface; @@ -23,6 +25,13 @@ class ExtendedContainer extends BaseContainer { */ private $woocommerce_namespace = 'Automattic\\WooCommerce\\'; + /** + * Holds the original registrations so that 'reset_replacement' can work, keys are class names and values are the original concretes. + * + * @var array + */ + private $original_concretes = array(); + /** * Whitelist of classes that we can register using the container * despite not belonging to the WooCommerce root namespace. @@ -68,7 +77,7 @@ class ExtendedContainer extends BaseContainer { } /** - * Replace an existing registration with a different concrete. + * Replace an existing registration with a different concrete. See also 'reset_replacement' and 'reset_all_replacements'. * * @param string $class_name The class name whose definition will be replaced. * @param mixed $concrete The new concrete (same as "add"). @@ -86,17 +95,49 @@ class ExtendedContainer extends BaseContainer { throw new ContainerException( "You cannot use concrete '$concrete_class', only classes in the {$this->woocommerce_namespace} namespace are allowed." ); } + if ( ! array_key_exists( $class_name, $this->original_concretes ) ) { + // LegacyProxy is a special case: we replace it with MockableLegacyProxy at unit testing bootstrap time. + $original_concrete = LegacyProxy::class === $class_name ? MockableLegacyProxy::class : $this->extend( $class_name )->getConcrete( $concrete ); + $this->original_concretes[ $class_name ] = $original_concrete; + } + return $this->extend( $class_name )->setConcrete( $concrete ); } + /** + * Reset a replaced registration back to its original concrete. + * + * @param string $class_name The class name whose definition had been replaced. + * @return bool True if the registration has been reset, false if no replacement had been made for the specified class name. + */ + public function reset_replacement( string $class_name ) : bool { + if ( ! array_key_exists( $class_name, $this->original_concretes ) ) { + return false; + } + + $this->extend( $class_name )->setConcrete( $this->original_concretes[ $class_name ] ); + unset( $this->original_concretes[ $class_name ] ); + + return true; + } + + /** + * Reset all the replaced registrations back to their original concretes. + */ + public function reset_all_replacements() { + foreach ( $this->original_concretes as $class_name => $concrete ) { + $this->extend( $class_name )->setConcrete( $concrete ); + } + + $this->original_concretes = array(); + } + /** * Reset all the cached resolutions, so any further "get" for shared definitions will generate the instance again. */ public function reset_all_resolved() { foreach ( $this->definitions->getIterator() as $definition ) { - // setConcrete causes the cached resolved value to be forgotten. - $concrete = $definition->getConcrete(); - $definition->setConcrete( $concrete ); + $definition->forgetResolved(); } } diff --git a/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php b/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php index 8760170c266..cbdc9fe9e7a 100644 --- a/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php +++ b/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php @@ -217,6 +217,15 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase { wc_get_container()->reset_all_resolved(); } + /** + * Reset all the class registration replacements in the dependency injection container, + * so any further "get" will return an instance of the class originally registered. + * For this to work with shared definitions 'reset_container_resolutions' is required too. + */ + public function reset_container_replacements() { + wc_get_container()->reset_all_replacements(); + } + /** * Reset the mock legacy proxy class so that all the registered mocks are unregistered. */ diff --git a/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithDependencies.php b/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithDependencies.php index aba4752e658..82beb2e4ff7 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithDependencies.php +++ b/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithDependencies.php @@ -36,6 +36,13 @@ class ClassWithDependencies { */ public $dependency_class = null; + /** + * Creates a new instance of the class. + */ + public function __construct() { + self::$instances_count++; + } + /** * Initialize the class instance. * @@ -45,8 +52,7 @@ class ClassWithDependencies { * @param int $some_number Some number we need for some reason. */ final public function init( DependencyClass $dependency_class, int $some_number = self::SOME_NUMBER ) { - self::$instances_count++; $this->dependency_class = $dependency_class; - $this->some_number = self::SOME_NUMBER; + $this->some_number = $some_number; } } diff --git a/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/DerivedDependencyClass.php b/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/DerivedDependencyClass.php new file mode 100644 index 00000000000..18e3629b08c --- /dev/null +++ b/plugins/woocommerce/tests/php/src/Internal/DependencyManagement/ExampleClasses/DerivedDependencyClass.php @@ -0,0 +1,12 @@ +expectException( ContainerException::class ); @@ -75,7 +79,7 @@ class ExtendedContainerTest extends \WC_Unit_Test_Case { } /** - * @testdox 'replace' + * @testDox 'replace' should throw an exception when trying to use a class outside the Automattic\WooCommerce\ namespace as the replacement. */ public function test_replace_throws_if_concrete_not_in_woocommerce_root_namespace() { $instance = new DependencyClass(); @@ -90,7 +94,7 @@ class ExtendedContainerTest extends \WC_Unit_Test_Case { } /** - * @testdox 'replace' should allow to replace existing registrations. + * @testDox 'replace' should allow to replace existing registrations with object instances. */ public function test_replace_allows_replacing_existing_registrations() { $instance_1 = new DependencyClass(); @@ -104,7 +108,7 @@ class ExtendedContainerTest extends \WC_Unit_Test_Case { } /** - * @testdox 'replace' should allow to replace existing registrations with anonymous classes. + * @testDox 'replace' should allow to replace existing registrations with anonymous classes. */ public function test_replace_allows_replacing_existing_registrations_with_anonymous_classes() { $instance_1 = new DependencyClass(); @@ -113,12 +117,42 @@ class ExtendedContainerTest extends \WC_Unit_Test_Case { $this->sut->add( DependencyClass::class, $instance_1, true ); $this->assertSame( $instance_1, $this->sut->get( DependencyClass::class ) ); - $this->sut->replace( DependencyClass::class, $instance_2, true ); + $this->sut->replace( DependencyClass::class, $instance_2 ); $this->assertSame( $instance_2, $this->sut->get( DependencyClass::class ) ); } /** - * @testdox 'reset_all_resolved' should discard cached resolutions for classes registered as 'shared'. + * @testDox 'replace' should allow replacing existing registrations with other class names. + */ + public function test_replace_allows_replacing_existing_registrations_with_class_names() { + $this->sut->add( DependencyClass::class, new DependencyClass(), true ); + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( DependencyClass::class ) ); + + $this->sut->replace( DependencyClass::class, DerivedDependencyClass::class ); + $this->assertInstanceOf( DerivedDependencyClass::class, $this->sut->get( DependencyClass::class ) ); + } + + /** + * @testDox 'init' should_be executed when resolving the class in the instance passed to 'replace' + */ + public function test_init_is_executed_when_resolving_the_class_in_the_instance_passed_to_replace() { + $this->sut->add( DependencyClass::class ); + $this->sut->add( ClassWithDependencies::class )->addArgument( DependencyClass::class ); + + $this->sut->get( ClassWithDependencies::class ); + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( ClassWithDependencies::class )->dependency_class ); + + $derived_class = new class() extends ClassWithDependencies {}; + $this->sut->replace( ClassWithDependencies::class, $derived_class ); + $this->sut->replace( DependencyClass::class, DerivedDependencyClass::class ); + + $replaced_instance = $this->sut->get( ClassWithDependencies::class ); + $this->assertEquals( $derived_class, $replaced_instance ); + $this->assertInstanceOf( DerivedDependencyClass::class, $replaced_instance->dependency_class ); + } + + /** + * @testDox 'reset_all_resolved' should discard cached resolutions for classes registered as 'shared'. */ public function test_reset_all_resolved_discards_cached_shared_resolutions() { $this->sut->add( DependencyClass::class ); @@ -137,4 +171,89 @@ class ExtendedContainerTest extends \WC_Unit_Test_Case { $this->sut->get( ClassWithDependencies::class ); $this->assertEquals( 2, ClassWithDependencies::$instances_count ); } + + /** + * @testDox 'reset_replacement' should revert a replaced definition back to its original concrete. + */ + public function test_reset_replacement_returns_a_replaced_definition_back_to_its_original_concrete() { + $this->sut->add( DependencyClass::class, new DependencyClass(), false ); + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( DependencyClass::class ) ); + + $this->sut->replace( DependencyClass::class, DerivedDependencyClass::class ); + $this->assertInstanceOf( DerivedDependencyClass::class, $this->sut->get( DependencyClass::class ) ); + + $rederived_instance = new class() extends DerivedDependencyClass {}; + $this->sut->replace( DependencyClass::class, $rederived_instance ); + $this->assertSame( $rederived_instance, $this->sut->get( DependencyClass::class ) ); + + $was_reset = $this->sut->reset_replacement( DependencyClass::class ); + $this->assertTrue( $was_reset ); + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( DependencyClass::class ) ); + } + + /** + * @testDox 'reset_replacement' returns false if the given class hadn't got a replacement. + */ + public function test_reset_replacement_returns_false_if_the_given_class_hadnt_got_a_replacement() { + $this->assertFalse( $this->sut->reset_replacement( DependencyClass::class ) ); + } + + /** + * @testDox 'reset_all_replacements' should revert all the replaced definitions back to their original concretes. + */ + public function test_reset_all_replacements_reverts_all_the_replaced_definitions_back_to_their_original_concretes() { + $this->sut->add( DependencyClass::class ); + $this->sut->add( ClassWithDependencies::class )->addArgument( DependencyClass::class ); + + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( ClassWithDependencies::class )->dependency_class ); + $this->assertInstanceOf( ClassWithDependencies::class, $this->sut->get( ClassWithDependencies::class ) ); + + $this->sut->replace( DependencyClass::class, DerivedDependencyClass::class ); + $derived_class = new class() extends ClassWithDependencies {}; + $this->sut->replace( ClassWithDependencies::class, $derived_class ); + + $this->assertInstanceOf( DerivedDependencyClass::class, $this->sut->get( ClassWithDependencies::class )->dependency_class ); + $this->assertSame( $derived_class, $this->sut->get( ClassWithDependencies::class ) ); + + $this->sut->reset_all_replacements(); + + $this->assertInstanceOf( DependencyClass::class, $this->sut->get( ClassWithDependencies::class )->dependency_class ); + $this->assertInstanceOf( ClassWithDependencies::class, $this->sut->get( ClassWithDependencies::class ) ); + } + + /** + * @testDox 'reset_replacement' treats LegacyProxy as an exception: it reverts is to MockableLegacyProxy. + */ + public function test_reset_replacement_resets_legacy_proxy_to_mockable_legacy_proxy() { + // For this test we need the original container that gets initialized in tests/legacy/bootstrap.php + // (where LegacyProxy is replaced with MockableLegacyProxy). + $sut = wc_get_container(); + + $some_other_proxy = new class() extends LegacyProxy {}; + $sut->replace( LegacyProxy::class, $some_other_proxy ); + + $this->assertSame( $some_other_proxy, $sut->get( LegacyProxy::class ) ); + + $sut->reset_replacement( LegacyProxy::class ); + + $this->assertInstanceOf( MockableLegacyProxy::class, $sut->get( LegacyProxy::class ) ); + } + + /** + * @testDox 'reset_all_replacements' treats LegacyProxy as an exception: it reverts is to MockableLegacyProxy. + */ + public function test_reset_all_replacements_resets_legacy_proxy_to_mockable_legacy_proxy() { + // For this test we need the original container that gets initialized in tests/legacy/bootstrap.php + // (where LegacyProxy is replaced with MockableLegacyProxy). + $sut = wc_get_container(); + + $some_other_proxy = new class() extends LegacyProxy {}; + $sut->replace( LegacyProxy::class, $some_other_proxy ); + + $this->assertSame( $some_other_proxy, $sut->get( LegacyProxy::class ) ); + + $sut->reset_all_replacements(); + + $this->assertInstanceOf( MockableLegacyProxy::class, $sut->get( LegacyProxy::class ) ); + } }