Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Issue18 #20

Merged
merged 6 commits into from
Jan 22, 2019
Merged

Issue18 #20

merged 6 commits into from
Jan 22, 2019

Conversation

Wilt
Copy link

@Wilt Wilt commented Jan 15, 2019

  • Detail how the bug is invoked currently.

Pull request for test case as requested by @weierophinney here: #18 (comment)

WouterGit added 3 commits December 19, 2018 15:43
# Conflicts:
#	test/Factory/RpcControllerFactoryTest.php
…ncyLookupCondition to demonstrate an issue with a double wrapped callable
While we were correctly detecting a circular dependency condition within
the factory method itself, we needed to prevent a circular _lookup_ as
well, which occurs when the plugin manager calls `has()` on the service.

This patch adds a check to see if the service being checked is the same
as the last one being created; if so, it returns true to ensure we do
not attempt a second creation.
@@ -326,5 +330,20 @@ public function testFactoryDoesNotEnterACircularDependencyLookupCondition()

$controller = $controllerManager->get(TestAsset\Foo::class);
$this->assertInstanceOf(RpcController::class, $controller);

$wrappedCallable = Assert::readAttribute($controller, 'wrappedCallable');
Copy link
Member

Choose a reason for hiding this comment

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

$this->readAttribute and we don't need import Assert.

We need to use a different RouteMatch class based on whether zend-mvc v2
or v3 is in place.
@weierophinney weierophinney merged commit d6caace into zfcampus:master Jan 22, 2019
weierophinney added a commit that referenced this pull request Jan 22, 2019
weierophinney added a commit that referenced this pull request Jan 22, 2019
Forward port #20

Conflicts:
	CHANGELOG.md
@weierophinney
Copy link
Member

Thanks, @Wilt, for the great test case! Fixed now!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants