[Architecture] Make the MVCFactory usable for custom services

While writing developer documentation I ran into a maddening problem which I feel needs to be solved sooner rather than later and possibly before 5.0. It will not only make 3PD's life easier, it will also sell them on the idea of having a DI Container / Service Locator in their components. Right now, most 3PDs perceive it as a liability than an asset and the more I work with it the more I tend to agree with them. Without further ado, let's get to the problem at hand: how in the name of Joomla can I use a custom service in a component's models?

Problem identified

I have a component, my very minimalist com_example.

I want to implement a custom service in my component which can be used by my Models. To demonstrate this I created a very simple \Acme\Example\Administrator\Service\Foobar\Foobar service.

How do I do that? Hold on to your hats, it's gonna be a wild ride.

Part I: Creating and using a custom service

Code for part I of the wild ride: https://github.com/nikosdion/com_example/commit/573110f7209b53c7e25c784b0db7153b1135f4aa

First of all, I need to register my service to the component's DI Container (which is really a service locator but let's not split hairs, I don't have many to spare anyway). So, for starters, I need to create a service provider \Acme\Example\Administrator\Provider\Foobar which implements the \Joomla\DI\ServiceProviderInterface and modify my services.php to register it. Cool.

I need to know which objects are aware of the existence of this service so let me also create a \Acme\Example\Administrator\Service\Foobar\FoobarAware interface and a corresponding Trait to implement it, \Acme\Example\Administrator\Service\Foobar\FoobarAwareTrait.

My model, \Acme\Example\Administrator\Model\FoobarModel, can now implement the aforementioned Interface and use the aforementioned Trait. Very cool. However, since nothing has set my custom service to my Model and my Model does not have access to the component's ~service locator~ DI Container my component is broken. 😞

Part II: A custom MVCFactory

You can see the code for Part II here: https://github.com/nikosdion/com_example/commit/8acf819931893265a1123155ea2c39a201613258

My Model is created by the MVCFactory object the component knows about. Therefore I need to create a custom MVCFactory which checks if the created Model object implements the \Acme\Example\Administrator\Service\Foobar\FoobarAware interface and call setFoobar to push the service. For this reason we create \Acme\Example\Administrator\Service\MVCFactory.

I have two problems now. One: I need to tell Joomla to use my custom MVCFactory instead of \Joomla\CMS\MVC\Factory\MVCFactory. Two, when the custom MVCFactory is created I need to pass it an instance of my custom service.

IDENTIFIED PROBLEM 1: AN MVCFactory CAN ONLY PUSH ALREADY INSTANTIATED SERVICES (YOU CANNOT DO LATE / ON-DEMAND INSTANTIATION).

But how can I tell Joomla to use my custom MVCFactory? I need to substitute the \Joomla\CMS\Extension\Service\Provider\MVCFactory service provider. However, trying to extend that class does not work because a. it has a private property for the namespace and b. its register method is registering a closure which is defined inline in said method. Therefore I have to copy the entire code of \Joomla\CMS\Extension\Service\Provider\MVCFactory into my custom \Acme\Example\Administrator\Provider\MVCFactory service provider even though I only really needed two changes: 1. using a different class name to instantiate the MVCFactory and 2. pass a custom service to the MVCFactory.

Here's the insanity of this approach.

First of all, this code is WET (Write Everything Twice), not DRY (Don't Repeat Yourself) which is doubleplusungood.

Second, the code inside \Joomla\CMS\Extension\Service\Provider\MVCFactory is considered “internals” and may change anytime in any minor or patch release without it being considered a b/c break. Therefore I need to check that code in every patch release of Joomla and update my extension which will otherwise might be broken. I went through all that trouble to do things the One True Joomla Way™ and all I got for my trouble is an unstable component which randomly breaks when Joomla is updated.

IDENTIFIED PROBLEM 2: HAVING TO COPY THE ENTIRE CODE OF \Joomla\CMS\Extension\Service\Provider\MVCFactory IN THIRD PARTY COMPONENTS IS UNMAINTAINABLE.

Proposed solution

I don't (currently) have a definitive solution for the first problem. See “open questions” below.

For the second problem I propose refactoring the \Joomla\CMS\Extension\Service\Provider\MVCFactory provider.

The namespace property should be protected, not private.

The provider closure should be a protected method instead of an inline closure. Overriding it allows a 3PD to register custom services to the factory object.

The if-block which returns the factory object should be a protected method. A 3PD can override it to return their own, custom MVCFactory object.

Open questions

Regarding the first problem and considering that the component's DI Container is in fact a service locator I'd propose passing it to the MVCFactory object i.e. the \Joomla\CMS\MVC\Factory\MVCFactory should implement \Joomla\DI\ContainerAwareInterface and use the \Joomla\DI\ContainerAwareTrait.

In this case we do not need to pass a custom service to the MVCProvider object. In our example we could instead implement its getFoobar method as:

protected function getFoobar(): Foobar
{
    if (!empty($this->foobarService)) {
        return $this->foobarService;
    }

    return $this->foobarService = $this->getContainer()->get(Foobar::class); 
}

This is very important if the custom services implemented by the component require some heavy initialisation, something VERY likely to happen in 3PD code.

Further to that, this makes creating custom MVCFactory instances easier, without having to massively refactor the \Joomla\CMS\Extension\Service\Provider\MVCFactory provider. Instead, the if-block which figures out the class name would be first looking for an MVCFactory class in the component's namespace for the application side we are in, falling back to the custom backend MVCFactory for other CMSApplication types (e.g. CLI) and if all else fails fall back to Joomla's default MVCFactory and ApiMVCFactory classes.

Such a change would eliminate the boilerplate code a 3PD needs to write in the same way no boilerplate is needed for a custom component Dispatcher.

Tagging @nibra @HLeithner @laoneo and @wilsonge for feedback.

While writing developer documentation I ran into a maddening problem which I feel needs to be solved sooner rather than later and possibly before 5.0

As a self-identified person who sometimes writes docs and contributes to core but doesn't build extensions properly i have opinions too to make everyone mad :D

IDENTIFIED PROBLEM 1: AN MVCFactory CAN ONLY PUSH ALREADY INSTANTIATED SERVICES (YOU CANNOT DO LATE / ON-DEMAND INSTANTIATION).

I think this is by design? Containers effectively store a singleton and because we can have multiple models loaded on the same page (e.g. content article and archived article module) you have to have the factory in the DIC and the factory will then create it when you need it

My Model is created by the MVCFactory object the component knows about. Therefore I need to create a custom MVCFactory which checks if the created Model object implements the \Acme\Example\Administrator\Service\Foobar\FoobarAware interface and call setFoobar to push the service. For this reason we create \Acme\Example\Administrator\Service\MVCFactory.

OK I'm going to start here by playing a bit of devils advocate. If your booting something through a DI container and you REQUIRE it to available at model boot through setFoodbar - why isn't it a constructor argument (which could then be injected by the DIC) rather than going through the setter? I know @laoneo has started this for a while (e.g. with the dbaware stuff) so it's not strictly a blame on any extension when core started it. But I've said it before and I'll say it again - i'm really unsure about it in these DIC scenarios. I mean the biggest argument against is just the fact that from a b/c perspective it get's really hard the day core ever does a major revamp of it's constructor args for the MVC classes (and maybe that makes it a 5.0 thing - but doesn't mean that setters are a good bodge either to keep b/c at all costs).

I need to substitute the \Joomla\CMS\Extension\Service\Provider\MVCFactory service provider. However, trying to extend that class does not work because a. it has a private property for the namespace

OK Makes sense - a protected getter method sounds reasonable (don't really want to allow internal overrides)

b. its register method is registering a closure which is defined inline in said method

I mean we can point to a method within the class I guess - anything with an executable should work right?

Second, the code inside \Joomla\CMS\Extension\Service\Provider\MVCFactory is considered “internals” and may change anytime in any minor or patch release without it being considered a b/c break

Why would you think that. In theory everything in the library folder (therefore this) is subject to the b/c promises no? I guess the bigger problem is all the constant injections through into the factory as we add more and more setters?

and all I got for my trouble is an unstable component which randomly breaks when Joomla is updated.

given you've just overriden the entire service provider - is it? in theory you've just made it more stable surely?

Regarding the first problem and considering that the component's DI Container is in fact a service locator I'd propose passing it to the MVCFactory object i.e. the \Joomla\CMS\MVC\Factory\MVCFactory should implement \Joomla\DI\ContainerAwareInterface and use the \Joomla\DI\ContainerAwareTrait.

I don't like it (surprise surprise). If such a class needs heavy initialisation - to me it means that it should be a separate service provider that the mvcfactory provider depends on. Not that all that logic should happen in the mvcfactory (maybe I've just looked at too much java recently too :D )

The if-block which returns the factory object should be a protected method. A 3PD can override it to return their own, custom MVCFactory object.

Bearing in mind 80% of the work (once you remove deprecated code) from the factory is just initializing the mvc classes (bearing in mind it has to deal with whatever namespaces - whereas for any given component you do know the namespace and can hardcode if you wanted) - remaining 20% being the setters. I'll argue that for a custom component with tonnes of setters and a custom factory too - that potentially you should just be using your own factory and service provider and just implement MVCFactoryInterface? Like there's gotta be a line where your supposed to implement the interface and do it yourself instead of core anyhow from a practical perspective. Not 100% on exactly where it is but...

Containers effectively store a singleton and because we can have multiple models loaded on the same page (e.g. content article and archived article module) you have to have the factory in the DIC and the factory will then create it when you need it

You didn't get me at all. I am not talking about how you get "a" service from the container, I understand how DI works. I am talking about the fact that the MVCFactory needs to be pushed concrete instances of all services it will subsequently be pushing to the MVC objects.

However, this is not always a good idea. For example, I may have a service which needs to do heavy I/O or use up a lot of memory to initialise e.g. because it needs to load and parse a large JWT. Having it initialise every time the component's MVCFactory initialises is killing performance. This happens every time the component is booted which is NOT ONLY when the component is loading.

If your booting something through a DI container and you REQUIRE it to available at model boot through setFoodbar - why isn't it a constructor argument (which could then be injected by the DIC) rather than going through the setter?

That was my question too. The answer is that Joomla does not create MVC objects using the DI Container :trollface: That's exactly why I am saying that Joomla's DIC is actually a service locator, not a DIC. It's only ever used as a service locator. Walks like a duck, talks like a duck, gotta be a duck.

But I've said it before and I'll say it again - i'm really unsure about it in these DIC scenarios

PREACH, BROTHER!

I mean the biggest argument against is just the fact that from a b/c perspective it get's really hard the day core ever does a major revamp of it's constructor args for the MVC classes (and maybe that makes it a 5.0 thing - but doesn't mean that setters are a good bodge either to keep b/c at all costs).

Confession: I wrote the following three times. I was going to say it's not possible then I realised it is, then I realised it's even simpler. Thank you for being my rubber ducky :)

We only have one problem: the model has a pesky $config array we can't get rid of for the next 1 or 2 major releases. The way \Joomla\DI\Container::buildObject works it would choke on that array. We would have to create a bastardised \Joomla\CMS\DI\Container with a buildCMSObject(string $resourceName, array $config = []) signature to do that. This method would take care of passing the array in the first array argument (the config array of Joomla's MVC object) and would set the new resource as non-shared and unprotected (so it doesn't leak outside our DIC's scope and can be replaced if needed).

If we could indeed do that we can change the base MVC objects' constructor signature. For example the BaseDatabaseModel would be

public function __construct($config = array(), ?MVCFactoryInterface $factory = null, ?DatabaseInterface $db = null, ?DispatcherInterface $dispatcher = null, ?User $currentUser = null, ?CacheControllerFactoryInterface $cacheFactory = null)

then a descendant class with an overridden constructor like this

public function __construct($config = array(), ?MVCFactoryInterface $factory = null)

can still work AS LONG AS the developer calls the parent constructor with NULL arguments for all dependencies AND implements the interfaces (and uses the traits) already in place in 4.2.

The MVCFactory would create the model object by doing

// Clone because the Factory needs to return a new instance whenever it's called
$model = clone $this->getContainer()->buildCMSObject($className, $config);

instead of

$model = new $className($config, $this);

Dude, this is doable. The code changes can even be implemented as Rector rules.

I mean we can point to a method within the class I guess - anything with an executable should work right?

Yup

Why would you think that. In theory everything in the library folder (therefore this) is subject to the b/c promises no?

The code of the MVCFactory service changed drastically in 4.1 and especially 4.2 since it's being pushed more and more services. Since it cannot be extended from it has to be duplicated. Therefore anyone who did exactly that in 4.1 would have a broken component in 4.2 which, as far as I am concerned, is a b/c break.

I guess the bigger problem is all the constant injections through into the factory as we add more and more setters?

Yes, precisely.

However, if we construct the MVCFactory object through the DI Container's very handy buildObject method and add the services it needs to its constructor we have another problem. Every time we need a new service in MVCFactory there's a b/c break because its constructor signature changes.

That said, having the MVCFactory construct objects through the DI Container requires passing the DI Container / Service Locator to the MVCFactory object. Maybe it would make more sense pulling those service when needed, unless they have already been pushed to it. This way we don't need to wait for a new major version to add a service to the MVCFactory, we can maintain b/c between two subsequent major versions AND it's still testable as we can push mocks in Unit Tests (if they we push mocks they will be used instead of pulling these services from the DI Container). It's not clean but it's practical.

given you've just overriden the entire service provider - is it? in theory you've just made it more stable surely?

No, it's not. Let's say Joomla 4.3 adds a UserFactory service to the MVCFactory. My MVCFactory service code is unaware of that change. When my component tries to run on Joomla 4.3 it will break because the UserFactory service is not set. This means that I need to perpetually check if something changed in the core code and copy it over to my component, all because I wanted to use a custom service and do it the Joomla Way.

What I'd do instead is override my component's extension class to have a static member which returns the component's DIC and use it to get the DIC in my model and use the DIC to instantiate my service. Not clean, I am pulling dependencies instead of pushing them BUT my code would work on 4.3 and 5.0 no matter what changes you make to the MVCFactory. Meaning, the only way for me to use Joomla without my component breaking in every minor Joomla version update is to NOT use Joomla's MVC as Joomla intended. This is the maddening part.

I don't like it (surprise surprise). If such a class needs heavy initialisation - to me it means that it should be a separate service provider that the mvcfactory provider depends on. Not that all that logic should happen in the mvcfactory (maybe I've just looked at too much java recently too :D )

It's WAY TOO LATE to change that now. You'd be telling all 3PDs like me who did the right thing and used the Joomla 4 MVC that they need to rewrite their components from scratch to instantiate MVC classes through the DI Container instead of Joomla's MVCFactory. Sorry, I just spent a year of 10 hour days during which I could not implement new features. If I have to do that all over again I might just as well go out of business!

Also, if I am to just use the DI Container to instantiate MVC classes, does it really make all that much of a difference if I had a helper called MVCFactory to do that for me?

First of all I want to clarify, that Joomla 4.0 is the first iteration of using a DI container to build extensions, so I wanted to make it as simple as possible by not replacing one magic system with another one. Sure you write every time similar code in the provider.php file, but it is clear as hell now and the dev knows what he is doing when writing the service provider. It was a huge effort to even come to that point, now it is time for the next step and optimize things. So I'm grateful we are discussing it now.

YOU CANNOT DO LATE / ON-DEMAND INSTANTIATION

If this is really needed, as I would rather fix the class which does the heavy instantiation, then we probably need a concept similar to symfonies proxy approach. But I would rather prefer to make constructors as lightweight as possible.

Therefore I have to copy the entire code of \Joomla\CMS\Extension\Service\Provider\MVCFactory

You can use the extend function of the Container class and then return your own factory which wrapps the already created one as there are only four functions to handle.

First of all, this code is WET (Write Everything Twice), not DRY (Don't Repeat Yourself)

Go ahead and create some composition service providers. You are not the first one who brings this on the table.

The MVCFactory class itself was built at that time that way, to be as much BC as possible with the legacy approach. So it would be worth a try, how it would look like with a reference to a DI container.

What bothers me most on this discussion is that you guys want to shift to constructor injection. Having setters makes adding new service much easier in regards of BC. Good example was the MVCFactory in the controller class which brought up some troubles. Symfony has implemented autowiring for setters, so this is possible as well, when we want to go the way to auto build objects.

@laoneo

If this is really needed, as I would rather fix the class which does the heavy instantiation, then we probably need a concept similar to symfonies proxy approach. But I would rather prefer to make constructors as lightweight as possible.

No matter how lightweight they are, at some point we are instantiating too many objects. remember that we are booting each component in the frontend to do routing. Beyond performance measured in milliseconds there's also memory performance. I am worried that moving our components to services would end up bloating the memory usage of the Joomla site.

You can use the extend function of the Container class and then return your own factory which wrapps the already created one as there are only four functions to handle.

So, you're saying that we should basically be using decorators for the custom MVCFactory instead? I am mostly concerned because the MVCFactory is constructed twice: once implicitly by the ComponentDispatcherFactory and once explicitly when setting it to the component's extension object in the component's service provider. I guess extending the service should happen right after registering the MVCFactory service provider to avoid a “lame duck” factory?

Go ahead and create some composition service providers. You are not the first one who brings this on the table.

Fair enough. I will give it a go.

What bothers me most on this discussion is that you guys want to shift to constructor injection. Having setters makes adding new service much easier in regards of BC.

Look, I can see pros and cons in both approaches. I do think that using setters makes it easier for BC. I do not think it's the easier approach for creating objects when we already have a DI Container. I would be okay with it and shut up if the decision was “Joomla is using its DI Container as a glorified Service Locator and nothing more”. If that's the case, sure, setters are the only reasonable way to construct objects and good for managing b/c, so no contest from me.

@laoneo Unfortunately, extending the service does not work at all.

Here's the thing. Our decorated MVCFactory can only operate on objects constructed by the \Joomla\CMS\MVC\Factory\MVCFactory object we are decorating, right?

The problem lies with instantiating the Controller and the fact that the Controller object itself is a proxy to the MVCFactory's createModel and createView methods.

Our decorated class' createController HAS TO call the createController of the undecorated \Joomla\CMS\MVC\Factory\MVCFactory object. That method creates the Controller instance by passing it a bunch of constructor arguments, including the MVCFactory object in the $factory argument. However, the \Joomla\CMS\MVC\Factory\MVCFactory neither has access to the DI Container nor does it know it's decorated. It simply passes $this as the Controller's $factory argument — meaning, our Controller now uses the undecorated MVCFactory!

When the Controller calls its getModel or getView method it uses that undecorated MVCFactory object which, of course, is blissfully unaware of our custom service. As you might remember, this was the problem we were trying to solve all along.

As a result, our component breaks 🤦🏽‍♂️

The controller does NOT have a setMVCFactory method or any other sensible way to set the MVCFactory object without ~practicing the forbidden Dark Arts~ using Reflection. Therefore the decorated MVCFactory cannot, in fact, fix the problem above.

You can see this problem in https://github.com/nikosdion/com_example/commit/06f43fde850b79673500697e41c4945602fa3415

What if I created a custom subclass of \Joomla\CMS\MVC\Factory\MVCFactory and abused the DIC's extend() to use this instead of the decorated MVCFactory instance? Well, we are back to the beginning: I cannot create an instance of my custom MVCFactory class unless I copy the code of Joomla's MVCFactory service. On top of that, I have also abused extend() for a double whammy of bad coding practices :trollface:

True, would the mentioned setMVCFactory function solve it?

@laoneo Yes, it would.

I would also propose a base MVCFactoryWrapper class developers can extend from so that:

They don't have to write a lot of unnecessary boilerplate code (see my source tree...).
They don't forget to set the MVCFactory when creating a Controller or Model, the latter is necessary for example in BaseDatabaseModel descendants which create tables by proxying the MVCFactory.

Would you be open to that idea?

Optional, further idea. If we ever write that auto–magic service provider we were talking about today we could prescribe the normative location of an MVCFactory decorator in the component's source tree so that this extended service is registered automatically, without the developer having to write a lot of boilerplate code in provider.php. Convention over configuration is a pretty solid idea considering who our developers are and what are their strengths and business priorities.

Sure go ahead. As already said, the current implementation is the simplest possible solution. The architecture allows us now to improve it into a direction which makes the life of us 3rd party devs more convenient.

joomla / joomla-cms