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.

Sign up for GitHub

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

Modify how nested resolutions get cached #10

Merged
merged 2 commits into from
Feb 23, 2022
Merged

Modify how nested resolutions get cached #10

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3adec9
Modify how nested resolutions get cached
stevegrunwell Feb 23, 2022
a75b6cf
Update leftover comment
stevegrunwell Feb 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions src/Container.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@
*/
abstract class Container implements ContainerInterface
{
/**
* Whether or not resolutions should be cached.
*
* By default, this will be false but will be set to `true` when calling `get()`.
*
* @var bool True if resolutions should be cached, false otherwise.
*/
protected $cacheResolutions = false;

/**
* Abstracts currently being resolved.
*
Expand All @@ -41,6 +32,15 @@ abstract class Container implements ContainerInterface
*/
protected $extensions = [];

/**
* Whether or not resolutions should be cached.
*
* By default, this will be false but will be set to `true` when calling `get()`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* By default, this will be false but will be set to `true` when calling `get()`.
* By default, this will be `0` and will get incremented when calling `get()`.

*
* @var int An integer representing the depth of the dependency caching.
*/
protected $resolutionCacheDepth = 0;

/**
* A cache of all resolved dependencies.
*
Expand Down Expand Up @@ -123,10 +123,15 @@ public function forget(...$abstracts)
*/
public function get($abstract)
{
if (! array_key_exists($abstract, $this->resolved)) {
$this->cacheResolutions = true;
$this->make($abstract);
$this->cacheResolutions = false;
/*
* If we don't yet have a resolution for this abstract, increment the resolutionCacheDepth;
* if this value is > 0, calls to the make() command for sub-dependencies will also be
* cached as if they were called via get().
*/
if (! isset($this->resolved[$abstract])) {
$this->resolutionCacheDepth++;
$this->resolved[$abstract] = $this->make($abstract);
$this->resolutionCacheDepth--;
}

return $this->resolved[$abstract];
Expand Down Expand Up @@ -214,11 +219,9 @@ public function make($abstract)
} else {
throw new ContainerException(sprintf('Unhandled definition type (%s)', gettype($config[$abstract])));
}
} catch (RecursiveDependencyException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is much better. Good update.

throw $e;
} catch (\Exception $e) {
if ($e instanceof RecursiveDependencyException) {
throw $e;
}

throw new ContainerException(
sprintf('An error occured building "%s": %s', $abstract, $e->getMessage()),
$e->getCode(),
Expand All @@ -227,7 +230,7 @@ public function make($abstract)
}

// If the cache is enabled, cache this resolution.
if ($this->cacheResolutions) {
if ($this->resolutionCacheDepth > 0) {
$this->resolved[$abstract] = $resolved;
}

Expand Down
42 changes: 23 additions & 19 deletions tests/Stubs/Concrete.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,46 @@ class Concrete extends Container
/**
* References to keys within the config.
*/
const ALIAS_KEY = 'alias';
const EXCEPTION_KEY = 'exception';
const INSTANCE_KEY = 'instance';
const INVALID_KEY = 'some-other-key';
const NESTED_GET_KEY = 'nested-get-key';
const NESTED_MAKE_KEY = 'nested-make-key';
const NULL_KEY = \DateTime::class;
const RECURSIVE_KEY = 'recursion';
const UNDEFINED_KEY = 'undefined';
const VALID_KEY = 'some-key';
const ALIAS_KEY = 'alias';
const EXCEPTION_KEY = 'exception';
const INSTANCE_KEY = 'instance';
const INVALID_KEY = 'some-other-key';
const NESTED_GET_KEY = 'nested-get-key';
const NESTED_MAKE_KEY = 'nested-make-key';
const NESTED_UNDEFINED_KEY = 'nested-undefined-key';
const NULL_KEY = \DateTime::class;
const RECURSIVE_KEY = 'recursion';
const UNDEFINED_KEY = 'undefined';
const VALID_KEY = 'some-key';

/**
* {@inheritDoc}
*/
public function config()
{
return [
self::VALID_KEY => function () {
self::VALID_KEY => function () {
return new \stdClass();
},
self::NESTED_GET_KEY => function ($app) {
self::NESTED_GET_KEY => function ($app) {
return new \ArrayObject($app->get(self::VALID_KEY));
},
self::NESTED_MAKE_KEY => function ($app) {
self::NESTED_MAKE_KEY => function ($app) {
return new \ArrayObject($app->make(self::VALID_KEY));
},
self::NULL_KEY => null,
self::ALIAS_KEY => 'some-key',
self::RECURSIVE_KEY => function ($container) {
self::NESTED_UNDEFINED_KEY => function ($app) {
return new \ArrayObject($app->make(self::UNDEFINED_KEY));
},
self::NULL_KEY => null,
self::ALIAS_KEY => self::VALID_KEY,
self::RECURSIVE_KEY => function ($container) {
return $container->make(self::RECURSIVE_KEY);
},
self::EXCEPTION_KEY => function () {
self::EXCEPTION_KEY => function () {
throw new \RuntimeException('Something went wrong');
},
self::INSTANCE_KEY => new \stdClass(),
self::INVALID_KEY => [],
self::INSTANCE_KEY => new \stdClass(),
self::INVALID_KEY => [],
// self::UNDEFINED_KEY should not be in this array.
];
}
Expand Down
27 changes: 25 additions & 2 deletions tests/Unit/ContainerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,22 @@ public function get_should_be_able_to_return_a_new_instance_of_an_identifier_wit

/**
* @test
* @testdox get() should recursively cache dependencies, even if they use make()
* @testdox get() should recursively cache dependencies using get()
*/
public function get_should_recursively_cache_dependencies_even_if_they_use_make()
public function get_should_recursively_cache_dependencies_using_get()
{
$container = new Concrete();

$container->get(Concrete::NESTED_GET_KEY);
$this->assertTrue($container->hasResolved(Concrete::NESTED_GET_KEY));
$this->assertTrue($container->hasResolved(Concrete::VALID_KEY));
}

/**
* @test
* @testdox get() should recursively cache dependencies using make
*/
public function get_should_recursively_cache_dependencies_using_make()
{
$container = new Concrete();

Expand Down Expand Up @@ -289,6 +302,16 @@ public function get_should_throw_a_ContainerException_if_an_exception_occurs()
$container->get(Concrete::EXCEPTION_KEY);
}

/**
* @test
* @testdox get() should throw a ContainerException if any recursive dependencies are undefined
*/
public function get_should_throw_a_ContainerException_if_any_recursive_dependencies_are_undefined()
{
$this->expectException(ContainerException::class);
(new Concrete())->get(Concrete::NESTED_UNDEFINED_KEY);
}

/**
* @test
* @testdox Subsequent calls to get() should return the same cached instance
Expand Down