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

FormElementManager: Only initialize a shared element once #6132

Merged
merged 7 commits into from
Apr 14, 2014

Conversation

Danielss89
Copy link
Contributor

If you call $formElementManager->get('My\Element'); several times, My\Element->init() is called each time.
This is correct if the element is not shared, but if the element is shared it should only be called once.

@Danielss89 Danielss89 changed the title FormElementManager: Only initialize a shared elementonce FormElementManager: Only initialize a shared element once Apr 13, 2014
@Ocramius
Copy link
Member

@Danielss89 I know it is a rare case, but what if the service is unset/replaced?

@Ocramius
Copy link
Member

An alternate approach to fix this is to move the init call to an initializer, no?

@Danielss89
Copy link
Contributor Author

@Ocramius Hm, what about using spl_object_hash in the $initializedPlugins?

@Ocramius
Copy link
Member

As far as I can see, the init() method is used in validatePlugin to ensure that all dependencies are injected before init() is called.

I think moving it to an initializer is fine as long as we ensure that it is at the bottom of the initializer stack

* Keep track of which plugins have been intizalied
* @var array
*/
protected $initializedPlugins = array();
Copy link
Member

Choose a reason for hiding this comment

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

Not needed anymore

@Danielss89
Copy link
Contributor Author

You are right, i need to sleep.
Will fix tomorrow ;)

grizzm0 added a commit to grizzm0/ZfcUser that referenced this pull request Apr 14, 2014
@Danielss89
Copy link
Contributor Author

@Ocramius Ok, cleaned up.

@weierophinney
Copy link
Member

@Danielss89 I'd like to see some unit tests. While I'm pretty sure it does what you want it to, we really need to verify it.

@Ocramius
Copy link
Member

@Danielss89 yes, a test that requires the same (shared) plugin twice should do the trick.

@Ocramius Ocramius self-assigned this Apr 14, 2014
@Ocramius Ocramius added this to the 2.3.1 milestone Apr 14, 2014
Ocramius added a commit to Ocramius/zf2 that referenced this pull request Apr 14, 2014
…t()` on shared form elements multiple times
@Danielss89
Copy link
Contributor Author

@weierophinney Tests added, with thankts to @Ocramius

Ocramius added a commit that referenced this pull request Apr 14, 2014
…d-elements-once' into develop

Close #6132
Forward Port #6132
@Ocramius Ocramius merged commit 253e31f into zendframework:master Apr 14, 2014
Ocramius added a commit that referenced this pull request Apr 14, 2014
grizzm0 added a commit to grizzm0/ZfcUser that referenced this pull request Oct 29, 2014
PHP54 for short array/echo support
Zend\Form due to zendframework/zendframework#6132
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants