This repository has been archived by the owner on Jan 8, 2020. It is now read-only.
Fix infinite loop when chaining peering service managers #7296
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
||
| /** | ||
| * @covers Zend\ServiceManager\ServiceManager::loopPeeringServiceManagers | ||
| * @covers Zend\ServiceManager\ServiceManager::setServiceManagerCaller |
There was a problem hiding this comment.
These two statements are causing PHPUnit errors to occur; it looks like PHPUnit does not like @covers for non-public methods: https://travis-ci.org/zendframework/zf2/jobs/53314224#L486
I'll remove the statements during merge.
There was a problem hiding this comment.
Its ok to @cover non-public methods, but the given one didnt exists any more.
| if ($caller !== $peeringServiceManager) { | ||
| $peeringServiceManager->serviceManagerCaller = $this; | ||
| if ($peeringServiceManager->has($name)) { | ||
| return true; |
There was a problem hiding this comment.
Shouldn't $peeringServiceManager->serviceManagerCaller be reset to null before returning?
weierophinney
added a commit
that referenced
this pull request
Mar 10, 2015
Fix infinite loop when chaining peering service managers
weierophinney
added a commit
that referenced
this pull request
Mar 10, 2015
- Continue early in loops. - Ensure `$peeringServiceManager->serviceManagerCaller` is reset before returning from a loop. - Remove `@covers` and `@uses` annotations which were raising errors.
weierophinney
added a commit
that referenced
this pull request
Mar 10, 2015
|
Merged to develop for release with 2.4. |
|
thank you, 👍 |
weierophinney
added a commit
to zendframework/zend-servicemanager
that referenced
this pull request
May 15, 2015
…nite-loop-sm-peering Fix infinite loop when chaining peering service managers
weierophinney
added a commit
to zendframework/zend-servicemanager
that referenced
this pull request
May 15, 2015
- Continue early in loops. - Ensure `$peeringServiceManager->serviceManagerCaller` is reset before returning from a loop. - Remove `@covers` and `@uses` annotations which were raising errors.
weierophinney
added a commit
to zendframework/zend-servicemanager
that referenced
this pull request
May 15, 2015
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
we have a use case where we add a peering service manager (SM) to the SM of an ZF2 application. Also the peering SM becomes the SM of the application added as peering SM, which allows us to call services from the applications SM.
For services which can't be found this runs into a infinite loop. The applications SM calls the peering SM, which calls back the application SM and so on.
This PR tries to fix that by passing the current SM instance to the peering SM so they skip the caller SM when looping over
peeringServiceManagers.