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 @@ -7,6 +7,7 @@
use Closure;
use InvalidArgumentException;
use Psr\Container\ContainerInterface;
use Traversable;
use Yiisoft\Definitions\ArrayDefinition;
use Yiisoft\Definitions\Exception\CircularReferenceException;
use Yiisoft\Definitions\Exception\InvalidConfigException;
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 @@ -230,13 +231,14 @@ private function addDefinition(string $id, $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 @@ -246,8 +248,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 @@ -258,11 +260,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();
foreach ($delegates as $delegate) {
Expand Down Expand Up @@ -327,10 +329,12 @@ private function validateDefinition($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 Down Expand Up @@ -359,10 +363,10 @@ private function validateMeta(array $meta): void
*/
private function validateDefinitionTags($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 iterable object or array of strings, %s given.',
arogachev marked this conversation as resolved.
Show resolved Hide resolved
$this->getVariableType($tags)
)
);
Expand Down Expand Up @@ -395,22 +399,22 @@ private function validateDefinitionReset($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
$this->getVariableType($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 iterable object or array of service IDs, %s given.',
arogachev marked this conversation as resolved.
Show resolved Hide resolved
$this->getVariableType($services)
)
);
Expand All @@ -428,18 +432,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 @@ -545,7 +557,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
30 changes: 27 additions & 3 deletions tests/Unit/ContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,30 @@ public function testTagsWithExternalDefinition(): void
$this->assertSame(EngineMarkTwo::class, get_class($engines[0]));
}

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, get_class($engines[1]));
$this->assertSame(EngineMarkTwo::class, get_class($engines[0]));
}

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

$this->expectException(InvalidConfigException::class);
$this->expectExceptionMessage(
'Invalid definition: tags should be array of strings, integer given.'
'Invalid definition: tags should be iterable object or array of strings, integer given.'
);
new Container($config);
}
Expand All @@ -1780,11 +1804,11 @@ public function dataInvalidTags(): array
{
return [
[
'Invalid tags configuration: tag should be string, 42 given.',
'Invalid tags configuration: tag should be string, integer given.',
[42 => [EngineMarkTwo::class]],
],
[
'Invalid tags configuration: tag should contain array of service IDs, integer given.',
'Invalid tags configuration: tag should be iterable object or array of service IDs, integer given.',
['engine' => 42],
],
[
Expand Down