This repository has been archived by the owner on Jan 30, 2020. It is now read-only.
Comprehensive fix for BC breaks introduced since 2.7.1 #67
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
- Created `FormElementManager` subcomponent:
- `FormElementManagerTrait` contains most common logic between v2 and v3, such
as determining factory based on name, hydrator based on name, overriding
get(), etc.
- Distinct `FormElementManager` implementations for zend-servicemanager v2 and
v3, providing definitions for all services (which cannot be done in a trait,
as traits cannot override properties from inherited classes),
version-specific constructors, version-specific initializers, and
version-specific overrides for `setInvokableClass()`
- Added polyfill via an autoloader script, which aliases `FormElementManager`
to the servicemanager version-specific implementation.
- Added tests to ensure behavior works as expected with regards to invokable
class initialization.
- `AnnotationBuilderFactory` was using v3 signature for `injectFactory()`
initializer, which causes it to fail under v2. Inlined that logic into the
factory, which changes some mocking expectations, but not the behavior.
This patch removes the change introduced in 2.7.1's `Zend\Form\Factory::create()` method that aggregated options and passed them to the `FormElementManager::get()` call. This usage was incorrect, as it creates an order-of-operations problem: most instances call their own `setOptions()` method from the constructor if options are present, and many of the actions this executes assume things like a populated `FormElementManager`, using a default instance if none is found! As such, reverting those changes to pre-2.7.1 usage fixes the issues reported in zendframework#66. Since the changes were done under an erroneous assumption, a test related to the incorrect functionality was also removed.
weierophinney
added a commit
that referenced
this pull request
May 1, 2016
weierophinney
added a commit
that referenced
this pull request
May 1, 2016
weierophinney
added a commit
to weierophinney/zend-form
that referenced
this pull request
May 1, 2016
Test is passing, indicating that the fix presented in zendframework#67 fixed this behavior as well.
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.
This patch:
Essentially, two principal issues were identified:
ElementFactoryto ensure they are instantiated correctly. Unfortunately, due to the fact that the "auto add invokables" flag is enabled, and that users can callsetInvokableClass()or defineinvokablesconfiguration, any such mappings were leading to the classes getting the base zend-servicemanager behavior instead. This patch solves that by creating polyfill solutions for zend-servicemanager v2 and v3 releases, and overriding thesetInvokableClass()implementations under each to ensure they map invokables to the specialized factory and set any needed aliases. (These polyfills also ensure that the version-specific instantiation rules are present.)Zend\Form\Factorywhen using collections, which fetch their target "element" when it's set via options. The break was due to a change inZend\Form\Factory::create(), which was passing discovered options toFormElementManager::get(). This led to an order-of-operations issue, as the options were then processed during instantiation, when previously they were processed afterwards, ensuring all setter injections were already complete. The erroneous code was removed.