Ensure a null value is passed to setCreationOptions instead of empty array
#210
Conversation
…pty array Per zendframework#159 (comment), the updates in zendframework#205 broke a previously fixed edge case whereby `AbstractPluginManager::createServiceViaCallback()` would inject a `null` value to a factory's `setCreationOptions()` method if no instance options were provided; it instead passed an empty array, which broke a number of plugins that relied on a null value being present. This patch does the following: - Adds a regression test for the behavior. - Modifies `AbstractPluginManager::createServiceViaCallback()` to have three distinct behaviors surrounding creation options, in the following order: - if the factory is an `InvokableFactory`, it tests if the creation options are an array or non-empty; if not, it passes a `null` value to `setCreationOptions()`. - if the factory is an `MutableCreationOptionsInterface`, it tests if the creation options are an array or non-empty; if not, it passes an empty array to `setCreationOptions()`. - if the factory implements the `setCreationOptions()` and the creation options are a non-array or non-empty, it reflects the method to determine if the options argument has a default value, using that if present, or an empty array if not.
|
How complex :o |
|
One thing I don't quite get... I thought I already wrote a unit test to cover this regression. How come it didn't catch this? |
|
@amcsi Because the behavior with an Anyways, there's now three issues worth of tests capturing different edge cases, so hopefully it's fixed for the rest of the 2.7 lifetime. |
|
|
||
| public function testIfCreationOptionsAreEmptySetThemThemToNullWhenCreatingAService() | ||
| { | ||
| // @codingStandardsIgnoreEnd |
There was a problem hiding this comment.
@weierophinney Do we need this comment above here? I can't see where is start... ?
There was a problem hiding this comment.
Odd; that must be a copy-paste artifact from a test I used as a starting point.
No, not needed, but already released.
Per #159 (comment), the updates in #205 broke a previously fixed edge case whereby
AbstractPluginManager::createServiceViaCallback()would inject anullvalue to a factory'ssetCreationOptions()method if no instance options were provided; it instead passed an empty array, which broke a number of plugins that relied on a null value being present.This patch does the following:
AbstractPluginManager::createServiceViaCallback()to have three distinct behaviors surrounding creation options, in the following order:InvokableFactory, it tests if the creation options are an array or non-empty; if not, it passes anullvalue tosetCreationOptions().MutableCreationOptionsInterface, it tests if the creation options are an array or non-empty; if not, it passes an empty array tosetCreationOptions().setCreationOptions()and the creation options are a non-array or non-empty, it reflects the method to determine if the options argument has a default value, using that if present, or an empty array if not.