@wilsonge Thanks for helping :).

We need to find a way to pass it from controller to models/views (could it be property of Model/View class?)

No. In proper MVC architectures the model and view should be unaware of the input. It's the controller that should be aware of this and be pushing the required information into the model and view.

I'm not seeing why you have a table_prefix property on the options array. This is controlled by the database driver. And as such, now that the DI package is getting more use in 4.0 in general, a component should be hooking in more with that to have its required dependencies injected in and with the right configuration.

(Note this comment is coming based solely on the PR summary, haven't clicked to code view yet)

No. In proper MVC architectures the model and view should be unaware of the input. It's the controller that should be aware of this and be pushing the required information into the model and view.

I see this. However, in scope of a component, it's the controller create the model and view object. So controller can easily pass it's input to model/view. I don't know if a view need to access to input object directly, however, a model needs it:

1. Model needs to populate states base on input data. That can be sorted by calling populateState method directly in the getModel class in controller.

2. The other case is when model needs to access to Files Upload. At the moment, in current legacy MVC, we only pass validated data (which is an array) to model in save method, so when a model needs to access to files upload, it usually calls JFactory::getApplication()->input to access to input directly, and that's preventing us from having a proper HMVC implement

I'm not seeing why you have a table_prefix property on the options array

It is not just the database table prefix. It is common preifx of all tables using in the extension, like #_edocman , #_osmembership... I am not sure if it is needed yet, the purpose is that we might use it to guess the table name base on model name and generate the list query in model list automatically....

Model needs to populate states base on input data. That can be sorted by calling populateState method directly in the getModel class in controller.

Models shouldn't be building their own state. That's part of the problem with our current architecture, and in some ways it hurts their reuse. State should be injected from the controller (or if you use the model disconnected from MVC architecture like in models that source should be pushing in the state).

I am not sure if it is needed yet, the purpose is that we might use it to guess the table name base on model name and generate the list query in model list automatically.

For now I'd leave it out of this proposal, even if it has no active use at the moment. Seems like scope creep, let's focus more on the dispatching concept for now.

No the state should be populated in a controller and injected into the model. Populate state as a concept needs to be deprecated in Joomla 4. There is work in progress on this for all our core components.

The backend frontend namespaces shouldn't be config values. We have a standard for them which is defined in the "Use of namespacing" section here https://volunteers.joomla.org/teams/joomla-4-architecture/reports/115-joomla-4-architecture-sprint-odense-dk

In terms of injecting the input that does need to be added but that needs to not be a config value but an explicit parameter

All the other config values I don't see as useful. I think having a application and optional input object should be enough for this class

The backend frontend namespaces shouldn't be config values.

How about third party extensions? We need to specify the namespace which we want to use for our extensions. Or there is a better way?

So there are two main questions here:

1. What's the correct way for model to populate state from input?

2. How model access to files upload (for example, in save method). Our current code only pass the validated data https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/controller/form.php#L744 . Will we need to pass an extra $input parameter?

1. The model should NEVER populate state from the input. That is the controllers job

2. Controllers should pass models the information as part of the mode state....

If you're using the Joomla dispatcher information you use the Joomla namespacing default. If you want your own, you use your own dispatcher implementation

The answer to both of your questions means redefining the MVC relationships. Models should be request unaware, controllers (or whatever is calling models) should be injecting the required data, whether that be as part of the model's state or as function parameters. When you stop looking at it as the model should be reading direct from the request (https://github.com/joomla/help.joomla.org or https://github.com/joomla/framework.joomla.org might better demonstrate this, unfortunately we repeated some of these mistakes with the issue tracker), you'll see some of the flaws in the current structure and why we need to revisit the approach.

If you're using the Joomla dispatcher information you use the Joomla namespacing default. If you want your own, you use your own dispatcher implementation

To me, having config option to help saving developers of having to write code would be better. But if you decide that, I can make the change

The model should NEVER populate state from the input. That is the controllers job
Models should be request unaware, controllers (or whatever is calling models) should be injecting the required data

Do you have example code which I can look at to understand exactly how to do that?

You can pass this array into JModelLegacy::getInstance():

$config = [
    'state' => new JObject,
    'ignore_request' => true,
];

$model = JModelLegacy::getInstance('Article', 'ContentModel', $config);`

That give me to next question. Look at https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/models/articles.php#L98 , we have many state variables (not sure what's the correct term for it) like filter.access, fiter.search...., how do controller know about these variables to populate data for all these variables?

The same way the model knows about them now. You basically move the bulk of that populateState method into the controller.

Thanks. Look like it is clear now.

OK. So base on suggestions, I made some changes to the original implementation:

1. Add two new optional arguments $input, $app to getInstance method so that application and input object can be passed from the code getting dispatcher. Application object now accessed via class property $app instead of calling JFactory::getApplication() directly

2. Remove normalizeConfig method.

3. Look at Controller class again, I see that we need to use Application object in that class, so passing $this->app as a new argument to create controller object code https://github.com/joomla/joomla-cms/pull/14258/files#diff-34e77048ffc88d27c50b168a7c3eb744R248

When you have a moment, could you review it again?

Since there's little communication or planning in the project's development overall, I still have no general idea what the overall goal is with this component dispatching concept as a whole (as the original PR was merged or this PR in all honesty). Without knowing what that intent is, I can't really say whether this PR is an improvement or a redefinition of the feature.

So someone make it clear to me. What is the purpose of the original dispatcher feature as merged? What is its design goal? With those answers given, how is this PR either an improvement on or a replacement for the original design?

I'm not buying into the "this can eventually replace JControllerLegacy::getInstance()" argument right now. I see two different architectural concepts here; a dispatcher (with what purpose I'm still not 100% clear) and a controller and one is not a suitable replacement for the other in terms of instantiation logic or general use.

@wilsonge and @yvesh would give us the answer about the goad and purpose of the original dispatcher merged PR

For this PR, there are some purposes:

1. Prepare (first step) for supporting namespace, PSR-4 components.

2. Provide a esier way to support HMVC support for extensions. Image in a module of an extension, we want to display a view of the component of that extension, what would we have to do if we use JControllerLegacy::getInstance() approach:

• Loading component language files
• Require any libraries required by the extension
• Calling JController::getInstance()->setInput($input)->execute();

With this dispatcher concept, I would leave the first two steps above to the dispatcher of the component (I don't have to worry or know what libraries might be using), just call the code below:
\Joomla\Cms\Dispatcher\Dispatcher::getInstance('com_mycom', $config, $input)
->dispatch()

Will respond this evening. Super busy at work today :)

Part of the issue with the language file loading is that it's already handled by a separate helper (JComponentHelper is handling all of that extra stuff before loading a component's entry file). Honestly I'm not so sure moving that logic to the dispatcher is a real fix for anything, it just moves that logic into another place but the MVC triad is still unaware if any of its support stuff has been loaded.

The dispatchers can't be ACL aware the same way as the current entry files are. Imagine trying to proxy a frontend request to a backend controller using the dispatcher or having an admin module trying to proxy to a component that isn't the active one, the dispatcher would be a roadblock to making that work if you one-for-one copy the existing logic. So we've already got a logic flaw in this new code (talking what's merged now).

I get HMVC is a goal. But the plan for this all really needs to be laid out (hmm, wasn't one of the first things I groaned to the production department leadership about related to a lack of a cohesive plan?) so we're all on the same page with how we're getting there. I still don't see how the HMVC goal is tied into this dispatcher concept or how the dispatcher concept addresses the point that there is logic being performed automagically outside of the component that really should exist somewhere closer to the component. And if that means I have to come across as challenging or asking a lot of questions to make sure that everything is crystal clear on why we're adding a new feature or a new approach to X or making an architectural change, then so be it; hopefully that means everything is well explained and everyone who has a potential to be impacted understands the 5 W's about things.

OK. So for this, I will wait to hear from @wilsonge to see whether this is something we can use, if not, I am closing the PR.

OK So the dispatcher is a two purpose machine. It has a simple and a slightly more complex purpose:

1. It allows full dependency injection for components. With a mock JApplicationCms (or JApplicationSite/JApplicationAdministrator if you're using those more specific functions), then you probably can unit test the end to end component without the presence of the CMS

2. I want to imagine a light HMVC in the CMS, I wanted this dispatcher to be a light version of the dispatcher in FOF. This means that dispatchers can be used to call components from anywhere with the following uses:

a. Modules loading component views
b. System plugins for the 'loading shit everywhere' behaviour that most extensions love at the moment).
c. Half an eye on a future webservices implementation

This wasn't supposed to be an end product. The next step was to suck up some of the default logic in the component controllers responsible for things like default views, so people don't need to create default component controllers for just setting default views. In terms of specific issues:

The ACL logic is basically a copy of what we have right now in our components. Doesn't make it right. But I wanted something simple based on our existing entry points that we could develop further.

We were unsure in the sprint how to load components (hence the current JControllerLegacy implementation remained). We were divided between the factory based controller loading and @laoneo 's preferred method of registering controllers into the DIC. But the intention is definitely to move beyond that single getInstance method.

OK, so a few thoughts shooting from the hip.

1. I don't see the need to keep this class abstract. Even as merged, BannersDispatcher is usable with the base implementation without any additional configuration. So I think for the absolute simplest base use case it should be appropriate to serve an instance of Joomla\Cms\Dispatcher\Dispatcher if it hasn't been subclassed.

2. The FOF dispatcher doesn't use singleton instances (its container does). So I think we need to weigh pros and cons of that. If we are going to go singleton, I don't know if locking it based on whomever is the first caller to the dispatcher wins the configuration battle is the preferred choice. Things like the class namespace would stay static, but if our recommended use case is to always pull a dispatcher through the singleton factory versus instantiating a new instance directly, you wouldn't be able to inject a separate app or input object into the dispatcher for separate use cases. Maybe this is a non-issue, but it does need to be pointed out.

3. Filesystem paths. This might be an appropriate time to introduce JPATH_COMPONENTS_SITE and JPATH_COMPONENTS_ADMIN. I know we're still a long way from it, but part of the logic behind the JPATH_* constants is to one day be able to move your filesystem around on the server (i.e. move the components directory outside the webspace). As the component directories are "critical" paths, the shortcut might make sense here. Probably outside scope of this PR, but something to consider too.

To answer point 1 - the reason for the abstract class is that a component dispatcher must be optional so that we call back to the legacy way of including a component (via the component_name.php). To keep it optional I elected to have the default implementation as abstract, then when a component 'opts in' to the dispatcher it creates the concrete (empty) wrapping class

To answer point 2 - I don't think we do need singletons. I agree with the idea here of having an optional input to inject. But I disagree with having a static instance of a dispatcher - I think there needs to be multiple dispatchers for a single component

To answer point 3 - I agree but I don't think it's in scope but would welcome a separate PR to the J4 repo (or maybe even 3.8 repo even if it's unused)

Also just an additional point. The intention here was also to add the autoloading as @joomdonation did - but we thought it probably made more sense to store component namespaces in the database (or component xml files) than to try and build them on the fly and allow overrides via config (which ends up with modules/plugins potentially overriding the psr-4 paths) - there should only be a single psr-4 path to load the namespaces.

1. I also don't see it need to be abstract class, too. There is no need for components have to implement a blank dispatcher class like that. We can still keep a the component entry file as how it is at the moment, and in that file, just call Joomla\Cms\Dispatcher\Dispatcher. That said, the change to JComponentHelper before to support dispatcher class is unnecessary and can be reverted

2. Singleton could still be useful here (although I don't know how much it is). A page for example, can load several modules of a same component. Each module then can get dispatcher object, set input and dispatch it to display the module https://github.com/akeeba/akeebasubs/blob/development/modules/site/aksubslist/mod_aksubslist.php#L44 (with HMVC concept, we would not have real modules, everything is actually inside component).

The real usage of HMVC as I can see at the moment, is only in scope of a single application (site or administrator). I don't know how, for example, calling a view from admin app from site app yet

However, as I said, I don't know how much it is useful (saving the creation of several dispatcher objects in a page load), so if you think that it is needed, let's remove it

3. About the additional point @wilsonge mentioned, we can deal with it later.

OK. So I made some changes today:

1. Remove singleton

2. Remove the dispatcher exists check implemented before at https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/cms/component/helper.php#L370 . If something like this is accepted, we can still have component entry like how it is at the moment. No need for that kind of check

3. Make sure the component is enabled before dispatching

To be more clear, if we decide to go with this direction, we will end up of having to re-write the current legacy MVC layer to support this concept. That mean, in Joomla 4, we would have two MVCs shipped, the current legacy MVC and the new MVC which will be working on.

@mbabker @wilsonge Would be great if you could take another look at this

I disagree with the approach you took to JComponentHelper - I'd much rather have that, move the majority of code in Dispatcher::getInstance into the __construct method (and maybe even remove the getInstance method altogether)

This is getting better tho :) don't be discouraged!

I disagree with the approach you took to JComponentHelper

The code in JComponentHelper checking for the file dispatcher.php in the root folder of the component which is not available with new implementation (the file now is Dispatcher.php, and will be placed inside component root folder or inside Dispatcher folder under component root (depends on our decision). That's why I removed it for now. Also, I still think still calling component entry file like how it was before still be OK with this approach

I'd much rather have that, move the majority of code in Dispatcher::getInstance into the __construct method (and maybe even remove the getInstance method altogether)

We can only do that if you want to force a blank Dispatcher class for every component. To me, I hate having to add a blank class for nothing like this

namespace Joomla\Component\Content\Dispatcher;
class Dispatcher extends \Joomla\Cms\Dispatcher
{
}

So I still think that getInstance() is useful in this case

Modules are not going to necessarily know of the component namespace. Hence why I prefered a compulsory non-namespaced dispatcher file over an arbitrary namespaced dispatcher file that might not even exist where the module has the ability to overwrite the namespace

The component manifest is probably going to have to be expanded to include some stuff like a fof.xml file would define, including namespaces. So the data is extracted from the manifest (by way of either live reading the XML or the manifest cache from the database).

Please ignore my last comment about Dispatcher::getInstance. Will think to see whether we can remove it as you suggested or not.

In London we considered maybe even a new mysql column (default null) for ultra-optimisation in the extensions table (as it's not just components but plugins and to a lesser extent modules that could also be namespaced). But something like that

Even as a separate field, ultimately something in the manifest will probably have to define the new data. Even the branch Allon shared a little bit ago where he's doing a POC on namespacing com_content has some stuff that would arguably be better suited in the dispatcher, it's just a matter of getting that data into it somehow.

When working on the com_content migration I got the feeling that it is not really necessary to have the namespace in a manifest file or even database. An approach can be to have that support in JLoader to load an extension namespace like this example. If an extension needs something from a different extension, then it will be a one liner to load a different extensions namespace, kind of lazy loading.

For the dispatcher, I'm not sure if it is even needed to have that class namespaced as it is the entry point of the component.

OK. So I made another changes to the code as suggested to remove getInstance() method. Now, to create dispatcher object:

1. If one component doesn't have Dispatcher class implemented, it can use Joomla\Cms\Dispatcher\Dispatcher directly

(new Joomla\Cms\Dispatcher\Dispatcher())->dispatch();

2. If component has custom dispatcher class implemented, it needs to call that dispatcher class directly

(new Joomla\Content\Admin\Dispatcher\Dispatcher())->dispatch();

For now, namespace can simply be set in $cNamespace property of Dispatcher class, if none is set, Joomla\Component\Comname will be used. In case a component wants to use custom namespace, just create a dispatcher class and set $cNamespace to the namespace it wants to use. At the moment, it doesn't need to be defined in manifest file or in database yet.

Right now, I still have two questions and looking for advice:

1. Should we introduce a new method initialize() method which will be called at the end of __contructor method ? I have the feeling that the code which registering autoloader or load language file (if needed) should be placed inside that separate method. Plus, it gives component specific dispatcher class init the dispatcher further if needed

2. Since we removed the singleton, I see that the method setInput is less useful now. I could not image a case which we get dispatcher object, dispatch it, then set a new input and dispatch again. So should we remove it?

3. Should we allow override Controller/Model/View classes in our core code if it is needed (same as override template). See http://eventbookingdoc.joomservices.com/developer-documentation/code-customization-override#override-method-in-a-controller-class

Due to my support life as extensions developer, I usually have to customize code in controllers, models, views class to meet customers need, and I see that supporting override is useful. Not sure if it is the same for Joomla core.

1.) Hmm other extensions can override the constructor and call the parent one, but it could be nice to split the tasks done in the constructor into various methods, like one (protected) loadLanguage(). Not sure if a initialize method is needed, but maybe others prefer it this way ¯_(ツ)_/¯

2.) Personally i prefer getters and setters for everything, you never know what ppl come up with..

3.) This is probably out of scope for this PR. Let's first define the basics, we can than extend it.

P.S.: Please call it componentNamespace and not cNamespace ;) The reason having it in the component params is that you can than get the name space from other places, like a second component or module.

Thanks @yvesh for your feedback. I will wait for feedback from others before making the changes. If this is the right direction, I can try to start working on base controller class so that we can have a clear picture about how it will work as as a whole

@wilsonge @mbabker Please help looking at this again when you have time

If an extension needs something from a different extension, then it will be a one liner to load a different extensions namespace, kind of lazy loading

The extension then has to know internals about the extension it is trying to lookup. That seems like a bad design pattern.

Should we allow override Controller/Model/View classes in our core code if it is needed

The only supported override method should be mangling the autoloader to put a path for a given namespace before the "default". Nothing more, nothing less. NEVER should we be designing an API which calls for class overloading as the favored approach, that is what a service driven architecture is for.

Thinking more about this Dispatcher class together with MVC layer as a whole, I wonder what would be the right option: Passing an Application object or passing a Container object as property of this class. In case passing Container object, then Container would also be a property of Controller, Model and View classes, too.

The reason I ask this question is because I see that in Model and View classes code, we have to trigger event like this https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Cms/Model/Admin.php#L757. So in case we don't use Container, we would have to call \JFactory::getApplication() to get application object (which as I understand should be avoided?). Thought ?

The DI container should NEVER be injected as a class property ANYWHERE. That is the biggest anti-pattern when it comes to dependency injection (and IIRC Symfony is actually making it harder to even do so with their DependencyInjection component).

Yes, it is available right now pretty easily (JFactory::getApplication()->getContainer(), JFactory::getContainer(), and whatever else we have implementing ContainerAwareInterface), but the places where we are explicitly reaching into the container are areas where we have factory methods trying to create service classes.

So in case of trigger event in model for example, we will still use JFactory::getApplication()->triggerEvent directly?

Actually we should have JModelLegacy (and anything that's dispatching events) implement DispatcherAwareInterface and the dispatcher be a class property of the class for a proper system. But that's probably not going to happen anytime soon without adding more keys to our options arrays that for some reason we make massive use of as constructors (and the constructors would probably still have to have the call to either the container or application to fetch required dependencies if not pushed in).

These concepts are new to me, so to be honest, I don't understand. Thanks for your advice anyway.