Steve-Dee-Designs
/
woocommerce
mirror of https://github.com/woocommerce/woocommerce.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve AbstractServiceProvider::add_with_auto_arguments 

If a class name is passed as a concrete, check that the class
constructor is public if it exists. If another type of concrete is
passed, check that it's valid (a callback or an object).

Also update the autoloader to check if the class file exists,
otherwise class_exists fails if a namespaced class doesn't exist.
This commit is contained in:
Nestor Soriano 2020-07-20 15:21:37 +02:00
parent 6fd84a0401
commit 65b5cbe692
4 changed files with 135 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Autoloader.php
View File

@ -91,7 +91,10 @@ class Autoloader {
foreach ( self::$autoloaded_namespaces as $namespace => $directory ) {
$len = strlen( $namespace );
if ( substr( $class, 0, $len ) === $namespace ) {
require $directory . str_replace( '\\', '/', substr( $class, $len ) ) . '.php';
$filename = $directory . str_replace( '\\', '/', substr( $class, $len ) ) . '.php';
if ( file_exists( $filename ) ) {
require $filename;
}
return;
}
}

42
src/Internal/DependencyManagement/AbstractServiceProvider.php
View File

@ -37,21 +37,11 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
* @throws \Exception Error when reflecting the class, or class constructor is not public, or an argument has no valid type hint.
*/
protected function add_with_auto_arguments( string $class_name, $concrete = null, bool $shared = false ) : DefinitionInterface {
try {
$reflector = new \ReflectionClass( $class_name );
} catch ( \ReflectionException $ex ) {
throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
}
$definition = new Definition( $class_name, $concrete );
$constructor = $reflector->getConstructor();
$constructor = $this->verify_class_and_concrete( $class_name, $concrete );
if ( ! is_null( $constructor ) ) {
if ( ! $constructor->isPublic() ) {
throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '$class_name' isn't public, instances can't be created." );
}
$constructor_arguments = $constructor->getParameters();
foreach ( $constructor_arguments as $argument ) {
if ( $argument->isDefaultValueAvailable() ) {
@ -75,6 +65,36 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
return $definition;
}
/**
* Check if a combination of class name and concrete is valid for registration.
* Also return the class constructor if the concrete is either a class name or null (then use the supplied class name).
*
* @param string $class_name The class name to check.
* @param mixed $concrete The concrete to check.
*
* @return \ReflectionMethod|null A ReflectionMethod representing the class constructor, if $concrete is null or a class name; null otherwise.
* @throws \Exception Class has a private constructor, or can't reflect class, or the concrete is invalid.
*/
private function verify_class_and_concrete( string $class_name, $concrete ) {
if ( ! isset( $concrete ) || is_string( $concrete ) && class_exists( $concrete ) ) {
try {
$class = $concrete ?? $class_name;
$reflector = new \ReflectionClass( $class );
$constructor = $reflector->getConstructor();
if ( isset( $constructor ) && ! $constructor->isPublic() ) {
throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '$class' isn't public, instances can't be created." );
}
return $constructor;
} catch ( \ReflectionException $ex ) {
throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class '$class': {$ex->getMessage()}" );
}
} elseif ( ! is_object( $concrete ) && ! is_callable( $concrete ) && ! function_exists( $concrete ) ) {
throw new \Exception( 'AbstractServiceProvider::add_with_auto_arguments: concrete must be a valid class name, function name, object, or callable.' );
}
return null;
}
/**
* Register a class in the container and use reflection to guess the constructor arguments.
* The class is registered as shared, so `get` on the container always returns the same instance.

10
src/Internal/DependencyManagement/ExtendedContainer.php
View File

@ -29,19 +29,19 @@ class ExtendedContainer extends \League\Container\Container {
/**
* Register a class in the container.
*
* @param string $id Class name.
* @param string $class_name Class name.
* @param mixed $concrete How to resolve the class with `get`: a factory callback, a concrete instance, onother class name, or null to just create an instance of the class.
* @param bool|null $shared Whether the resolution should be performed only once and cached.
*
* @return DefinitionInterface The generated definition for the container.
* @throws \Exception Invalid parameters.
*/
public function add( string $id, $concrete = null, bool $shared = null ) : DefinitionInterface {
if ( ! $this->class_is_in_root_namespace( $id ) && ! in_array( $id, $this->registration_whitelist, true ) ) {
throw new \Exception( "Can't use the container to register '$id', only objects in the " . Container::WOOCOMMERCE_ROOT_NAMESPACE . ' namespace are allowed for registration.' );
public function add( string $class_name, $concrete = null, bool $shared = null ) : DefinitionInterface {
if ( ! $this->class_is_in_root_namespace( $class_name ) && ! in_array( $class_name, $this->registration_whitelist, true ) ) {
throw new \Exception( "Can't use the container to register '$class_name', only objects in the " . Container::WOOCOMMERCE_ROOT_NAMESPACE . ' namespace are allowed for registration.' );
}
return parent::add( $id, $concrete, $shared );
return parent::add( $class_name, $concrete, $shared );
}
/**

100
tests/php/src/Internal/DependencyManagement/AbstractServiceProviderTest.php
View File

@ -64,25 +64,64 @@ class AbstractServiceProviderTest extends \WC_Unit_Test_Case {
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if an invalid class name is passed.
* Runs before all the tests of the class.
*/
public function test_add_with_auto_arguments_throws_on_non_class_passed() {
public static function setUpBeforeClass() {
/**
* Return a new instance of DependencyClass.
*
* @return DependencyClass The new instance.
*/
function get_new_dependency_class() {
return new DependencyClass();
};
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if an invalid class name is passed as class name.
*/
public function test_add_with_auto_arguments_throws_on_non_class_passed_as_class_name() {
$this->expectException( \Exception::class );
$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class 'foobar': Class foobar does not exist" );
$this->sut->add_with_auto_arguments( 'foobar' );
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if the passed concrete is neither an existing class name, an existing function, an object, or a callback.
*
* @testWith ["foobar"]
* [1234]
*
* @param mixed $concrete The concrete to use to register the class.
*/
public function test_add_with_auto_arguments_throws_on_non_class_passed_as_concrete( $concrete ) {
$this->expectException( \Exception::class );
$this->expectExceptionMessage( 'AbstractServiceProvider::add_with_auto_arguments: concrete must be a valid class name, function name, object, or callable.' );
$this->sut->add_with_auto_arguments( get_class( $this ), $concrete );
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if the passed class has a private constructor.
*/
public function test_add_with_auto_arguments_throws_on_private_constructor() {
public function test_add_with_auto_arguments_throws_on_class_private_constructor() {
$this->expectException( \Exception::class );
$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '" . ClassWithPrivateConstructor::class . "' isn't public, instances can't be created." );
$this->sut->add_with_auto_arguments( ClassWithPrivateConstructor::class );
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if the passed concrete is a class with a private constructor.
*/
public function test_add_with_auto_arguments_throws_on_concrete_private_constructor() {
$this->expectException( \Exception::class );
$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '" . ClassWithPrivateConstructor::class . "' isn't public, instances can't be created." );
$this->sut->add_with_auto_arguments( ClassWithDependencies::class, ClassWithPrivateConstructor::class );
}
/**
* @testdox 'add_with_auto_arguments' should throw an exception if the passed class has a constructor argument without type hint.
*/
@ -104,7 +143,7 @@ class AbstractServiceProviderTest extends \WC_Unit_Test_Case {
}
/**
* @testdox 'add_with_auto_arguments' should properly register the supplied class.
* @testdox 'add_with_auto_arguments' should properly register the supplied class when no concrete is passed.
*
* @testWith [true, 1]
* [false, 2]
@ -112,7 +151,7 @@ class AbstractServiceProviderTest extends \WC_Unit_Test_Case {
* @param bool $shared Whether to register the test class as shared or not.
* @param int $expected_constructions_count Expected number of times that the test class will have been instantiated.
*/
public function test_add_with_auto_arguments_works_as_expected( bool $shared, int $expected_constructions_count ) {
public function test_add_with_auto_arguments_works_as_expected_with_no_concrete( bool $shared, int $expected_constructions_count ) {
ClassWithDependencies::$instances_count = 0;
$this->container->share( DependencyClass::class );
@ -130,4 +169,55 @@ class AbstractServiceProviderTest extends \WC_Unit_Test_Case {
// Constructor arguments are filled as expected.
$this->assertSame( $this->container->get( DependencyClass::class ), $resolved->dependency_class );
}
/**
* @testdox 'add_with_auto_arguments' should properly register the supplied class when a concrete representing a class name is passed.
*/
public function test_add_with_auto_arguments_works_as_expected_when_concrete_is_class_name() {
$this->sut->add_with_auto_arguments( ClassWithDependencies::class, DependencyClass::class );
$resolved = $this->container->get( ClassWithDependencies::class );
$this->assertInstanceOf( DependencyClass::class, $resolved );
}
/**
* @testdox 'add_with_auto_arguments' should properly register the supplied class when a concrete that is an object is passed.
*/
public function test_add_with_auto_arguments_works_as_expected_when_concrete_is_object() {
$object = new DependencyClass();
$this->sut->add_with_auto_arguments( ClassWithDependencies::class, $object );
$resolved = $this->container->get( ClassWithDependencies::class );
$this->assertSame( $object, $resolved );
}
/**
* @testdox 'add_with_auto_arguments' should properly register the supplied class when a concrete that is a closure is passed.
*/
public function test_add_with_auto_arguments_works_as_expected_when_concrete_is_a_closure() {
$callable = function() {
return new DependencyClass();
};
$this->sut->add_with_auto_arguments( ClassWithDependencies::class, $callable );
$resolved = $this->container->get( ClassWithDependencies::class );
$this->assertInstanceOf( DependencyClass::class, $resolved );
}
/**
* @testdox 'add_with_auto_arguments' should properly register the supplied class when a concrete that is a function name is passed.
*/
public function test_add_with_auto_arguments_works_as_expected_when_concrete_is_a_function_name() {
$this->sut->add_with_auto_arguments( ClassWithDependencies::class, __NAMESPACE__ . '\get_new_dependency_class' );
$resolved = $this->container->get( ClassWithDependencies::class );
$this->assertInstanceOf( DependencyClass::class, $resolved );
}
}