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

Ensure console (and controller!) usage works for v2 applications #81

Merged

Conversation

weierophinney
Copy link
Member

This patch is a result of testing the develop branch against Apigility. The only major show-stopper issue I ran across was with console usage, and this patch provides fixes as follows:

  • Aliases console to ConsoleAdapter, to ensure retrieval can occur under that name.
  • Updates the RouterFactory::createService() method to test for Console::isConsole() and, if true, to set the $requestedName passed to __invoke() to ConsoleRouter. This ensures that the console router can be selected and used at runtime!
  • Re-implements the ServiceLocatorAwareInterface implementation in AbstractController. It was removed originally when we were targeting a v3 release on the develop branch; now that we're targeting v2.7, it needs to be re-introduced.
  • Related, re-implemented a ServiceLocatorAwareInterface initializer in the ControllerManager to ensure the application container is injected.

At this point all testing has been integration testing; I'll add tests for this behavior shortly.

v2 tests for `Console::isConsole()` to determine if the console router
should be created; added code to the `RouterFactory::createService()`
implementation to do that and overwrite the `$requestedName` when
detected.
We originally removed the ServiceLocatorAwareInterface implementation
from AbstractController when we were targeting a 3.0 release, but it
needs to be present for a 2.7 release.
This functionality was removed when we were targeting a v3 release, but
needs to be re-added when targeting v2.7.
- to allow retrieval with either `Console` or `console` as the service name.
@weierophinney weierophinney added this to the 2.7.0 milestone Feb 29, 2016
Now that controllers re-implement ServiceLocatorAwareInterface, we get
deprecation errors. By adding `PHPUnit_Framework_Error_Deprecated::$enabled = false`
to the setup on these tests, we can eliminate having those cast to exceptions,
and thus prevent them from marking tests as errored or failed.
Since zend-servicemanager v3 does not define
`ServiceLocatorAwareInterface`, then we cannot extend it and still support
that release series. As such, `AbstractController` no longer explicitly
implements it, but does so implicitly by defining the methods.

The initializers in both `ServiceManagerConfig` and `ControllerManager`
were updated to *also* look for the combination:

- interface `ServiceLocatorAwareInterface` does not exist AND
- method `setServiceLocator()` is present

If that combination is present, the same behavior is retained, including
the deprecation notices.
After duck-typing the `ServiceLocatorAwareInterface` usage, two more
tests showed failures due to deprecation notices; these have now been
marked to exclude them.
Adds tests for both `ServiceManagerConfig` and `ControllerManager`,
verifying that each can do duck-typed `ServiceLocatorAwareInterface`
injection via the initializers they register.
The test cases removed are tested implicitly when retrieving aliases;
however, they can never be retrieved directly, as they will not resolve
to fully qualified class names. (When retrieving by alias, the FQCN is
passed as the "requested name", which is why the factory works ever.)
Do not check if ServiceLocatorAwareInterface exists, as that will skip
the initializer when it does, but the instance does not implement it and
*does* fit duck typing rules.
@weierophinney weierophinney removed the WIP label Feb 29, 2016
@weierophinney weierophinney merged commit 99e4212 into zendframework:develop Feb 29, 2016
weierophinney added a commit that referenced this pull request Feb 29, 2016
weierophinney added a commit that referenced this pull request Feb 29, 2016
@weierophinney weierophinney deleted the hotfix/console-usage branch February 29, 2016 19:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants