Some changes on the dependency injection engine based on feedback:

- `wc_get_container` declares a return type now.
- The autoloader now runs before `autoload_packages.php` is included.
- The autloader now excludes the namespaces for Admin, Blocs, Rest API.
- `private $container` in `Container` is now declared as being
  of type `\League\Container\Container`.
- The underlying container is registered with `share` instead of `add`.
- All methods in `AbstractServiceProvider` now allow passing a concrete.
- `AbstractServiceProvider::addWithAutoArguments` now checks if an
  argument has a default value, and registers the argument so that
  that value is supplied when constructing the object.
- `Proxies` class renamed to `ProxiesServiceProvider`.
- The methods to call code in `LegacyProxy` now use `call_user_func_array`.
This commit is contained in:
Nestor Soriano 2020-06-22 11:41:11 +02:00
parent 064ae558ab
commit b53fc512bf
6 changed files with 48 additions and 25 deletions

View File

@ -7,8 +7,6 @@
namespace Automattic\WooCommerce;
use PHPUnit\Exception;
defined( 'ABSPATH' ) || exit;
/**
@ -23,6 +21,12 @@ class Autoloader {
*/
private function __construct() {}
const NON_CORE_WOO_NAMESPACES = array(
'Automattic\\WooCommerce\\Admin\\',
'Automattic\\WooCommerce\\Blocks\\',
'Automattic\\WooCommerce\\RestApi\\',
);
/**
* Require the autoloader and return the result.
*
@ -38,23 +42,32 @@ class Autoloader {
return false;
}
self::registerPsr4Autoloader();
$autoloader_result = require $autoloader;
if ( ! $autoloader_result ) {
return false;
}
self::registerPsr4Autoloader();
return $autoloader_result;
}
/**
* Define a PSR4 autoloader for the dependency injection engine to work.
* Function grabbed from https://container.thephpleague.com/3.x
*
* TODO: Assess if this is still needed after https://github.com/Automattic/jetpack/pull/15106 is merged.
* If it still is, remove this notice. If it isn't, remove the method.
*/
protected static function registerPsr4Autoloader() {
spl_autoload_register(
function ( $class ) {
foreach ( self::NON_CORE_WOO_NAMESPACES as $non_core_namespace ) {
if ( $non_core_namespace === substr( $class, 0, strlen( $non_core_namespace ) ) ) {
return;
}
}
$prefix = 'Automattic\\WooCommerce\\';
$base_dir = __DIR__ . '/';
$len = strlen( $prefix );
@ -64,9 +77,7 @@ class Autoloader {
}
$relative_class = substr( $class, $len );
$file = $base_dir . str_replace( '\\', '/', $relative_class ) . '.php';
if ( file_exists( $file ) ) {
require $file;
}
require $file;
}
);
}

View File

@ -7,7 +7,7 @@
namespace Automattic\WooCommerce;
use Automattic\WooCommerce\Tools\DependencyManagement\ServiceProviders as ServiceProviders;
use Automattic\WooCommerce\Tools\DependencyManagement\ServiceProviders\ProxiesServiceProvider;
/**
* PSR11 compliant dependency injection container for WooCommerce.
@ -34,13 +34,13 @@ final class Container implements \Psr\Container\ContainerInterface {
* @var string[]
*/
private $service_providers = array(
ServiceProviders\Proxies::class,
ProxiesServiceProvider::class,
);
/**
* The underlying container.
*
* @var \Psr\Container\ContainerInterface
* @var \League\Container\Container
*/
private $container;
@ -53,7 +53,7 @@ final class Container implements \Psr\Container\ContainerInterface {
// Add ourselves as the shared instance of ContainerInterface,
// register everything else using service providers.
$this->container->add( \Psr\Container\ContainerInterface::class, $this );
$this->container->share( \Psr\Container\ContainerInterface::class, $this );
foreach ( $this->service_providers as $service_provider_class ) {
$this->container->addServiceProvider( $service_provider_class );

View File

@ -7,6 +7,7 @@
namespace Automattic\WooCommerce\Tools\DependencyManagement;
use League\Container\Argument\RawArgument;
use League\Container\Definition\DefinitionInterface;
use League\Container\Definition\Definition;
@ -27,21 +28,24 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
/**
* Register a class in the container and use reflection to guess the constructor arguments.
*
* WARNING: this method uses reflection, so please have performance in mind when using it.
*
* @param string $class_name Class name to register.
* @param mixed $concrete The concrete to register. Can be a shared instance, a factory callback, or a class name.
* @param bool $shared Whether to register the class as shared (`get` always returns the same instance) or not.
*
* @return DefinitionInterface The generated container definition.
*
* @throws \Exception Error when reflecting the class, or class constructor is not public, or an argument has no valid type hint.
*/
public function addWithAutoArguments( string $class_name, bool $shared = false ) : DefinitionInterface {
public function addWithAutoArguments( string $class_name, $concrete = null, bool $shared = false ) : DefinitionInterface {
try {
$reflector = new \ReflectionClass( $class_name );
} catch ( \ReflectionException $ex ) {
throw new \Exception( get_class( $this ) . "::addWithAutoArguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
}
$definition = new Definition( $class_name, null );
$definition = new Definition( $class_name, $concrete );
$constructor = $reflector->getConstructor();
@ -52,12 +56,17 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
$constructor_arguments = $constructor->getParameters();
foreach ( $constructor_arguments as $argument ) {
$argument_class = $argument->getClass();
if ( is_null( $argument_class ) ) {
throw new \Exception( get_class( $this ) . "::addWithAutoArguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
}
if ( $argument->isDefaultValueAvailable() ) {
$default_value = $argument->getDefaultValue();
$definition->addArgument( new RawArgument( $default_value ) );
} else {
$argument_class = $argument->getClass();
if ( is_null( $argument_class ) ) {
throw new \Exception( get_class( $this ) . "::addWithAutoArguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
}
$definition->addArgument( $argument_class->name );
$definition->addArgument( $argument_class->name );
}
}
}
@ -72,14 +81,17 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
* 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.
*
* WARNING: this method uses reflection, so please have performance in mind when using it.
*
* @param string $class_name Class name to register.
* @param mixed $concrete The concrete to register. Can be a shared instance, a factory callback, or a class name.
*
* @return DefinitionInterface The generated container definition.
*
* @throws \Exception Error when reflecting the class, or class constructor is not public, or an argument has no valid type hint.
*/
public function shareWithAutoArguments( string $class_name ) : DefinitionInterface {
return $this->addWithAutoArguments( $class_name, true );
public function shareWithAutoArguments( string $class_name, $concrete = null ) : DefinitionInterface {
return $this->addWithAutoArguments( $class_name, $concrete, true );
}
/**
@ -104,6 +116,6 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
* @return DefinitionInterface The generated container definition.
*/
public function share( string $id, $concrete = null ) : DefinitionInterface {
return $this->getContainer()->share( $id, $concrete );
return $this->add( $id, $concrete, true );
}
}

View File

@ -13,7 +13,7 @@ use Automattic\WooCommerce\Tools\Proxies as ProxyClasses;
/**
* Service provider for the classes in the Automattic\WooCommerce\Tools\Proxies namespace.
*/
class Proxies extends AbstractServiceProvider {
class ProxiesServiceProvider extends AbstractServiceProvider {
/**
* The classes/interfaces that are serviced by this service provider.

View File

@ -96,7 +96,7 @@ class LegacyProxy {
* @return mixed The result from the function.
*/
public function callFunction( $function_name, ...$parameters ) {
return call_user_func( $function_name, ...$parameters );
return call_user_func_array( $function_name, $parameters );
}
/**
@ -110,6 +110,6 @@ class LegacyProxy {
* @return mixed The result from the method.
*/
public function callStatic( $class_name, $method_name, ...$parameters ) {
return $class_name::$method_name( ...$parameters );
return call_user_func_array( "$class_name::$method_name", $parameters );
}
}

View File

@ -53,7 +53,7 @@ function WC() { // phpcs:ignore WordPress.NamingConventions.ValidFunctionName.Fu
*
* @return \Psr\Container\ContainerInterface The WooCommerce PSR11 container.
*/
function wc_get_container() {
function wc_get_container() : \Psr\Container\ContainerInterface {
return $GLOBALS['wc_container'];
}