Closed JVMartin closed 9 years ago
Correct me if I'm wrong, but isn't it undesirable to bind models to an IoC/DI container?
Definitely something I'd love to have cleared up.
Atrauzzi, thanks for your comment. I would argue that it's quite desirable.
We've made dozens of production websites now, binding all models (and, indeed, all classes) to the IoC so that models can be changed or functionality can be added to any model on any site, from any package, without having to change how all of the models and packages that depend on it (or are related to it in any way) define their relationships. This works extremely well for the sites and the groups of developers that have used it. The models are in modules, and the modules are in packages. You have to extend and re-bind the models to the IoC when you extend their functionality so that you still get all updates when updating the package without overwriting your customized additions to each model.
For instance, say we have a Change
model which represents each single change in the CMS. Every other model in the entire CMS has a relationship to this model through $this->hasMany(App::make('Change'))
, bear that in mind (spread across multiple, updateable packages). Any time you make a change to a Page
, or a BlogPost
, etc., this relationship is used. Someone might want to add extra functionality to each change (say, for instance, a comment system for each change), where that feature doesn't exist in the core CMS. If the Change
model were not bound to the IoC, they'd need to change every single instance of $this->hasMany('\Angel\Core\Change')
, to $this->hasMany('\MyModels\Change')
in order to get the extended functionality working, as all other models in the framework have a relationship to the this model. Not cool. And furthermore, that's not even possible without either editing the /vendor folder (YUCK) or creating their own version of every. single. package. in the framework. But currently, all they have to do is bind over the Change
model in the IoC with their new, customized model and everything works beautifully, since all the relationships are already defined as $this->hasMany(App::make('Change'))
in all the models that depend upon it.
To be clear: Using the IoC in this way is already working fabulously for us. The only place it doesn't work is when eager-loading polymorphic relationships.
I was still able to get eager-loading polymorphism to work well with the IoC in my own way. This is how we're achieving query-optimized IoC-based eager-loading in our menu system until this bug is fixed:
Here is how we compile each MenuItem's related models into their groups so we can do a single query on each one (Page
, BlogPost
, Product
, etc.)
This is where we resolve each group once and query it once, putting it back into the menu system in the proper order.
Being able to explain how you're using it doesn't necessarily justify it though. Regardless, I've had the advice told to me in the past to not put models in any DI/IoC. This mainly had to do with the lifecycle of objects being managed by an ORM (think of it like a 2nd container).
It's quite possible to create a more static object model that is extensible and leveraging it in a more clear and directed way. I do it all the time without involving the DI container.
There's also the other angle which is that it starts to take you back in the direction of global mutable state. So while, yes you can bind any type to a container, and obtain that type, it doesn't mean you aren't creating antipatterns by doing so.
If I had to hazard a guess, it would be along the lines of global mutable state, flattening your scope and losing control of your model lifecycle. Mainly because you've now turned your models into infrastructure and the DI container is now being manipulated at "runtime/request time".
Here is a StackOverflow question that details the problem.
I develop packages for Laravel, so all of my models (and their relationships) are always bound and resolved from the IoC so that their bindings can be easily overwritten from any individual deployment.
This is simple for non-polymorphic relationships from either side, and from one side (the inverse side) of polymorphic relationships:
However, the other (far more important) side of polymorphic relationships currently cannot do this:
The relationship will only load properly if the fully namespaced model name is in the
linkable_type
column in the database. That is,linkable_type
must be hard-coded to\Angel\Core\Page
if the MenuLink is linking to a Page model. The behavior we would want is for simplyPage
to be stored there and\Angel\Core\Page
to be resolved from the IoC (or\MyCustomModels\Page
, or whatever the binding actually is).This is, unfortunately, the case even if the properly bound class, such as
\Angel\Core\Page
, has aprotected $morphClass = 'Page';
property - the class still will not be found without its full namespace being in thelinkable_type
column. (Perhaps this is a bug?)To fix this for lazy-loading the
morphTo
relationship, one can simply change a single line from Eloquent's morphTo() function. Click here to see that change. I'm not suggesting this be implemented, I'm merely showing that it's a workaround for those of us that use the IoC.Then, you can easily lazy-load models using the correct IoC bindings from the models that have this modified morphTo(). But it is still impossible to do it for eager-loading as far as I can see. Perhaps someone could give me some guidance?
Thanks so much in advance for any advice or guidance you might have.