Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve containers types to use iterable objects #299

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
48 changes: 30 additions & 18 deletions src/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Psr\Container\ContainerInterface;
use Psr\Container\NotFoundExceptionInterface;
use Throwable;
use Traversable;
use Yiisoft\Definitions\ArrayDefinition;
use Yiisoft\Definitions\DefinitionStorage;
use Yiisoft\Definitions\Exception\CircularReferenceException;
Expand Down Expand Up @@ -59,7 +60,7 @@ final class Container implements ContainerInterface

/**
* @var array Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
* @psalm-var array<string, list<string>>
* @psalm-var array<string, iterable<string>>
*/
private array $tags;

Expand Down Expand Up @@ -223,13 +224,14 @@ private function addDefinition(string $id, mixed $definition): void
/**
* Sets multiple definitions at once.
*
* @param array $config Definitions indexed by their IDs.
* @param iterable $config Definitions indexed by their IDs.
*
* @throws InvalidConfigException
*/
private function addDefinitions(array $config): void
private function addDefinitions(iterable $config): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
{
/** @var mixed $definition */
/** @psalm-suppress MixedAssignment */
foreach ($config as $id => $definition) {
if ($this->validate && !is_string($id)) {
throw new InvalidConfigException(
Expand All @@ -239,8 +241,8 @@ private function addDefinitions(array $config): void
)
);
}
/** @var string $id */

$id = (string) $id;
vjik marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not need type-casting. Above check that $id is string.

$this->addDefinition($id, $definition);
}
}
Expand All @@ -251,11 +253,11 @@ private function addDefinitions(array $config): void
* Each delegate must is a callable in format "function (ContainerInterface $container): ContainerInterface".
* The container instance returned is used in case a service can not be found in primary container.
*
* @param array $delegates
* @param iterable $delegates
*
* @throws InvalidConfigException
*/
private function setDelegates(array $delegates): void
private function setDelegates(iterable $delegates): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
{
$this->delegates = new CompositeContainer();
$container = $this->get(ContainerInterface::class);
Expand Down Expand Up @@ -323,10 +325,12 @@ private function validateDefinition(mixed $definition, ?string $id = null): void
/**
* @throws InvalidConfigException
*/
private function validateMeta(array $meta): void
private function validateMeta(iterable $meta): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateMeta always receive array. Not need iterable.

{
/** @var mixed $value */
/** @psalm-suppress MixedAssignment */
foreach ($meta as $key => $value) {
$key = (string)$key;
arogachev marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$key = (string)$key;

Not need.

if (!in_array($key, self::ALLOWED_META, true)) {
throw new InvalidConfigException(
sprintf(
Expand All @@ -353,10 +357,10 @@ private function validateMeta(array $meta): void
*/
private function validateDefinitionTags(mixed $tags): void
{
if (!is_array($tags)) {
if (!is_iterable($tags)) {
throw new InvalidConfigException(
sprintf(
'Invalid definition: tags should be array of strings, %s given.',
'Invalid definition: tags should be either iterable or array of strings, %s given.',
get_debug_type($tags)
)
);
Expand Down Expand Up @@ -387,22 +391,22 @@ private function validateDefinitionReset(mixed $reset): void
/**
* @throws InvalidConfigException
*/
private function setTags(array $tags): void
private function setTags(iterable $tags): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
{
if ($this->validate) {
foreach ($tags as $tag => $services) {
if (!is_string($tag)) {
throw new InvalidConfigException(
sprintf(
'Invalid tags configuration: tag should be string, %s given.',
$tag
get_debug_type($services)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
get_debug_type($services)
get_debug_type($tag)

)
);
}
if (!is_array($services)) {
if (!is_iterable($services)) {
throw new InvalidConfigException(
sprintf(
'Invalid tags configuration: tag should contain array of service IDs, %s given.',
'Invalid tags configuration: tag should be either iterable or array of service IDs, %s given.',
get_debug_type($services)
)
);
Expand All @@ -420,18 +424,26 @@ private function setTags(array $tags): void
}
}
}
/** @psalm-var array<string, list<string>> $tags */
/** @psalm-var iterable<string, iterable<string>> $tags */

$this->tags = $tags;
$this->tags = $tags instanceof Traversable ? iterator_to_array($tags, true) : $tags ;
}

/**
* @psalm-param string[] $tags
*/
private function setDefinitionTags(string $id, array $tags): void
private function setDefinitionTags(string $id, iterable $tags): void
{
foreach ($tags as $tag) {
if (!isset($this->tags[$tag]) || !in_array($id, $this->tags[$tag], true)) {
if (!isset($this->tags[$tag])) {
$this->tags[$tag] = [$id];
continue;
}

$tags = $this->tags[$tag];
$tags = $tags instanceof Traversable ? iterator_to_array($tags, true) : $tags;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better convert iterable to array before foreach

if (!in_array($id, $tags, true)) {
/** @psalm-suppress PossiblyInvalidArrayAssignment */
$this->tags[$tag][] = $id;
}
}
Expand Down Expand Up @@ -537,7 +549,7 @@ private function buildInternal(string $id)
* @throws CircularReferenceException
* @throws InvalidConfigException
*/
private function addProviders(array $providers): void
private function addProviders(iterable $providers): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
{
$extensions = [];
/** @var mixed $provider */
Expand Down
32 changes: 16 additions & 16 deletions src/ContainerConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
*/
final class ContainerConfig implements ContainerConfigInterface
{
private array $definitions = [];
private array $providers = [];
private array $tags = [];
private iterable $definitions = [];
private iterable $providers = [];
private iterable $tags = [];
private bool $validate = true;
private array $delegates = [];
private iterable $delegates = [];
private bool $useStrictMode = false;

private function __construct()
Expand All @@ -26,46 +26,46 @@ public static function create(): self
}

/**
* @param array $definitions Definitions to put into container.
* @param iterable $definitions Definitions to put into container.
*/
public function withDefinitions(array $definitions): self
public function withDefinitions(iterable $definitions): self
{
$new = clone $this;
$new->definitions = $definitions;
return $new;
}

public function getDefinitions(): array
public function getDefinitions(): iterable
{
return $this->definitions;
}

/**
* @param array $providers Service providers to get definitions from.
* @param iterable $providers Service providers to get definitions from.
*/
public function withProviders(array $providers): self
public function withProviders(iterable $providers): self
{
$new = clone $this;
$new->providers = $providers;
return $new;
}

public function getProviders(): array
public function getProviders(): iterable
{
return $this->providers;
}

/**
* @param array $tags Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
* @param iterable $tags Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
*/
public function withTags(array $tags): self
public function withTags(iterable $tags): self
{
$new = clone $this;
$new->tags = $tags;
return $new;
}

public function getTags(): array
public function getTags(): iterable
{
return $this->tags;
}
Expand All @@ -86,18 +86,18 @@ public function shouldValidate(): bool
}

/**
* @param array $delegates Container delegates. Each delegate is a callable in format
* @param iterable $delegates Container delegates. Each delegate is a callable in format
* `function (ContainerInterface $container): ContainerInterface`. The container instance returned is used
* in case a service can not be found in primary container.
*/
public function withDelegates(array $delegates): self
public function withDelegates(iterable $delegates): self
{
$new = clone $this;
$new->delegates = $delegates;
return $new;
}

public function getDelegates(): array
public function getDelegates(): iterable
{
return $this->delegates;
}
Expand Down
16 changes: 8 additions & 8 deletions src/ContainerConfigInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,31 @@
interface ContainerConfigInterface
{
/**
* @return array Definitions to put into container.
* @return iterable Definitions to put into container.
*/
public function getDefinitions(): array;
public function getDefinitions(): iterable;

/**
* @return array Service providers to get definitions from.
* @return iterable Service providers to get definitions from.
*/
public function getProviders(): array;
public function getProviders(): iterable;

/**
* @return array Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
* @return iterable Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
*/
public function getTags(): array;
public function getTags(): iterable;

/**
* @return bool Whether definitions should be validated immediately.
*/
public function shouldValidate(): bool;

/**
* @return array Container delegates. Each delegate is a callable in format
* @return iterable Container delegates. Each delegate is a callable in format
* `function (ContainerInterface $container): ContainerInterface`. The container instance returned is used
* in case a service can not be found in primary container.
*/
public function getDelegates(): array;
public function getDelegates(): iterable;

/**
* @return bool If the automatic addition of definition when class exists and can be resolved is disabled.
Expand Down
57 changes: 51 additions & 6 deletions tests/Unit/ContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,30 @@ public function testTagsWithExternalDefinition(): void
$this->assertSame(EngineMarkTwo::class, $engines[0]::class);
}

public function testTagsIterable(): void
{
$config = ContainerConfig::create()
->withDefinitions([
EngineMarkOne::class => [
'class' => EngineMarkOne::class,
'tags' => ['engine'],
],
EngineMarkTwo::class => [
'class' => EngineMarkTwo::class,
],
])
->withTags(['engine' => new ArrayIterator([EngineMarkTwo::class])])
->withValidate(true);
$container = new Container($config);

$engines = $container->get('tag@engine');

$this->assertIsArray($engines);
$this->assertCount(2, $engines);
$this->assertSame(EngineMarkOne::class, $engines[1]::class);
$this->assertSame(EngineMarkTwo::class, $engines[0]::class);
}

public function testTagsWithExternalDefinitionMerge(): void
{
$config = ContainerConfig::create()
Expand Down Expand Up @@ -1920,7 +1944,7 @@ public function testNonArrayTags(): void

$this->expectException(InvalidConfigException::class);
$this->expectExceptionMessage(
'Invalid definition: tags should be array of strings, string given.'
'Invalid definition: tags should be either iterable or array of strings, string given.'
);
new Container($config);
}
Expand All @@ -1939,22 +1963,22 @@ public function testNonArrayArguments(): void
$this->expectExceptionMessage(
'Invalid definition: incorrect method "setNumber()" arguments. Expected array, got "int". Probably you should wrap them into square brackets.',
);
$container = new Container($config);
new Container($config);
}

public function dataInvalidTags(): array
{
return [
[
'/^Invalid tags configuration: tag should be string, 42 given\.$/',
'Invalid tags configuration: tag should be string, array given.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does not look right. The problem here is 42, right?

[42 => [EngineMarkTwo::class]],
],
[
'/^Invalid tags configuration: tag should contain array of service IDs, (integer|int) given\.$/',
'Invalid tags configuration: tag should be either iterable or array of service IDs, int given.',
['engine' => 42],
],
[
'/^Invalid tags configuration: service should be defined as class string, (integer|int) given\.$/',
'Invalid tags configuration: service should be defined as class string, int given.',
['engine' => [42]],
],
];
Expand All @@ -1969,7 +1993,28 @@ public function testInvalidTags(string $message, array $tags): void
->withTags($tags);

$this->expectException(InvalidConfigException::class);
$this->expectExceptionMessageMatches($message);
$this->expectExceptionMessage($message);
new Container($config);
}

public function testSupportIterableDefinitions(): void
{
$config = ContainerConfig::create()
->withDefinitions(
(function () {
yield from [
EngineMarkOne::class => [
'class' => EngineMarkOne::class,
'setNumber()' => [42],
],
];
})()
);

$container = new Container($config);

$this->assertTrue($container->has(EngineMarkOne::class));
$engine = $container->get(EngineMarkOne::class);
$this->assertEquals(42, $engine->getNumber());
}
}