Skip to content

Commit

Permalink
Merge pull request #21 from mindplay-dk/prevent-cycles
Browse files Browse the repository at this point in the history 
Prevent cycles
  • Loading branch information
@mindplay-dk
mindplay-dk authored Feb 11, 2019
2 parents 7ca6a82 + 11af777 commit fd7b1f2
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 36 deletions.
75 changes: 39 additions & 36 deletions src/Container.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ class Container extends Configuration implements ContainerInterface, FactoryInte
*/
protected $active = [];

/**
* @var int[] map where component name => activation depth
*
* @see get()
*/
private $activations = [];

/**
* @param Configuration $config
*/
Expand Down Expand Up @@ -47,21 +54,43 @@ public function __construct(Configuration $config)
public function get($name)
{
if (! isset($this->active[$name])) {
if (isset($this->factory[$name])) {
$factory = $this->factory[$name];
try {
if (isset($this->activations[$name])) {
$activations = array_flip($this->activations);

$reflection = new ReflectionFunction($factory);
ksort($activations, SORT_NUMERIC); // order by activation depth

$params = $this->resolve($reflection->getParameters(), $this->factory_map[$name]);
$activations = array_slice($activations, array_search($name, $activations, true));

$this->values[$name] = call_user_func_array($factory, $params);
} elseif (! array_key_exists($name, $this->values)) {
throw new NotFoundException($name);
}
$activations[] = $name;

$activation_path = implode(" -> ", $activations);

throw new ContainerException("Dependency cycle detected: " . $activation_path);
}

$this->active[$name] = true;
$this->activations[$name] = count($this->activations);

$this->initialize($name);
if (isset($this->factory[$name])) {
$this->values[$name] = $this->call($this->factory[$name], $this->factory_map[$name]);
} elseif (! array_key_exists($name, $this->values)) {
throw new NotFoundException($name);
}

if (isset($this->config[$name])) {
foreach ($this->config[$name] as $index => $config) {
$value = $this->call($config, [$this->values[$name]] + $this->config_map[$name][$index]);

if ($value !== null) {
$this->values[$name] = $value;
}
}
}

$this->active[$name] = true;
} finally {
unset($this->activations[$name]);
}
}

return $this->values[$name];
Expand Down Expand Up @@ -227,30 +256,4 @@ protected function inject($name, $value)
$this->values[$name] = $value;
$this->active[$name] = true;
}

/**
* Internally initialize an active component.
*
* @param string $name component name
*
* @return void
*/
private function initialize($name)
{
if (isset($this->config[$name])) {
foreach ($this->config[$name] as $index => $config) {
$map = $this->config_map[$name][$index];

$reflection = Reflection::createFromCallable($config);

$params = $this->resolve($reflection->getParameters(), $map);

$value = call_user_func_array($config, $params);

if ($value !== null) {
$this->values[$name] = $value;
}
}
}
}
}
159 changes: 159 additions & 0 deletions test/test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,165 @@ function () {
}
);

test(
'can trap direct dependency cycle',
function () {
$factory = new ContainerFactory();

$factory->register("a", function ($b) {
return "A{$b}";
});

$factory->register("b", function ($a) {
return "B{$a}";
});

expect(
ContainerException::class,
"should throw for dependency cycle",
function () use ($factory) {
$factory->createContainer()->get("a");
},
"{Dependency cycle detected: a -> b -> a}i"
);

expect(
ContainerException::class,
"should throw for dependency cycle",
function () use ($factory) {
$factory->createContainer()->get("b");
},
"{Dependency cycle detected: b -> a -> b}i"
);
}
);

test(
'can trap indirect dependency cycle',
function () {
$factory = new ContainerFactory();

$factory->register("a", function ($b) {
return "A{$b}";
});

$factory->register("b", function ($c) {
return "B{$c}";
});

$factory->register("c", function ($a) {
return "C{$a}";
});

expect(
ContainerException::class,
"should throw for dependency cycle (a)",
function () use ($factory) {
$factory->createContainer()->get("a");
},
"{Dependency cycle detected: a -> b -> c -> a}i"
);

expect(
ContainerException::class,
"should throw for dependency cycle (b)",
function () use ($factory) {
$factory->createContainer()->get("b");
},
"{Dependency cycle detected: b -> c -> a -> b}i"
);

expect(
ContainerException::class,
"should throw for dependency cycle (c)",
function () use ($factory) {
$factory->createContainer()->get("c");
},
"{Dependency cycle detected: c -> a -> b -> c}i"
);

expect(
ContainerException::class,
"should throw for repeated dependency cycle",
function () use ($factory) {
$container = $factory->createContainer();

try {
$container->get("c");
} catch (ContainerException $error) {
$container->get("c");
}
},
"{Dependency cycle detected: c -> a -> b -> c}i"
);
}
);

test(
'can trap indirect dependency cycle via configuration',
function () {
$factory = new ContainerFactory();

$factory->register("a", function ($b) {
return "A{$b}";
});

$factory->register("b", function () {
return "B";
});

$factory->configure("b", function ($b, $a) {
return "{$b}{$a}";
});

expect(
ContainerException::class,
"should throw for dependency cycle",
function () use ($factory) {
$factory->createContainer()->get("a");
},
"{Dependency cycle detected: a -> b -> a}i"
);

expect(
ContainerException::class,
"should throw for dependency cycle",
function () use ($factory) {
$factory->createContainer()->get("b");
},
"{Dependency cycle detected: b -> a -> b}i"
);
}
);

test(
'can trap deep dependency cycle',
function () {
$factory = new ContainerFactory();

$factory->register("a", function ($b) {
return "A{$b}";
});

$factory->register("b", function ($c) {
return "B{$c}";
});

$factory->register("c", function ($b) {
return "C{$b}";
});

expect(
ContainerException::class,
"should throw for dependency cycle",
function () use ($factory) {
$factory->createContainer()->get("a");
},
"{Dependency cycle detected: b -> c -> b}i"
);
}
);

if (version_compare(PHP_VERSION, "7", ">=")) {
require __DIR__ . "/test-php70.php";
} else {
Expand Down

0 comments on commit fd7b1f2

Please sign in to comment.