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

Added second argument to Container::has callas in order to make the ServiceManager to include abstract factories #156

Merged
merged 2 commits into from Oct 13, 2015

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Oct 13, 2015

Every time the Container::has method is internally called, the second argument is set to true, so that when using the Zend\ServiceManager, it includes abstract factories.
Without this, when using the ServiceManager v3, regular middleware, route middleware and any service that is internally fetched and should be created by abstract factories is instantiated instead of being pulled from the Container.

@acelaya
Copy link
Contributor Author

acelaya commented Oct 13, 2015

I have changed all the Container::has calls, not just those fetching middlewares, in case any other service (like the router or the final handler fetched in the ApplicationFactory) needs to be created by using an abstract factory.

@weierophinney
Copy link
Member

@acelaya I don't think we should do the has() checks in the factories, as those should all be considered "well-known names" (i.e., they should have dedicated factories). Middleware is a different story, as those are run-time resolutions, and very likely could benefit from a generalized construction pattern (such as an abstract factory).

One thing to note as well: Because of changes in the FactoryInterface, you can now write factories that can return different instances based on the requested name. As such, the relevance of abstract factories will likely decrease once we have the v3 release stable.

Can you please revert the changes to the various factories? Thanks in advance!

@weierophinney weierophinney added this to the 0.5.1 milestone Oct 13, 2015
@weierophinney weierophinney self-assigned this Oct 13, 2015
@acelaya
Copy link
Contributor Author

acelaya commented Oct 13, 2015

That makes sense.
I'll change it right away.

@acelaya
Copy link
Contributor Author

acelaya commented Oct 13, 2015

I think it's correct now

@weierophinney
Copy link
Member

@acelaya Perfect! Thanks!

weierophinney added a commit that referenced this pull request Oct 13, 2015
Added second argument to Container::has callas in order to make the ServiceManager to include abstract factories
weierophinney added a commit that referenced this pull request Oct 13, 2015
weierophinney added a commit that referenced this pull request Oct 13, 2015
@weierophinney weierophinney merged commit 83ff1af into zendframework:master Oct 13, 2015
weierophinney added a commit that referenced this pull request Oct 13, 2015
@acelaya acelaya deleted the feature/sm-abstract-factories-fix branch October 13, 2015 17:46
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.

None yet

2 participants