User tests: Successful: Unsuccessful:
Moves the addIncludePath() method back to Joomla\CMS\MVC\Model\LegacyModelLoaderTrait.
Install Weblinks https://github.com/joomla-extensions/weblinks/releases/tag/3.7.0
Run this code somewhere:
\JModelLegacy::addIncludePath(JPATH_SITE . '/components/com_weblinks/models');
$model = \JModelLegacy::getInstance('Weblink', 'WeblinksModel', ['ignore_request' => true]);
Check the value of $model.
It's an instance of WeblinksModelWeblink.
No.
| Status | New | ⇒ | Pending |
| Category | ⇒ | Libraries |
And I think what your saying was exactly the issue. At a specific point we added paths with BaseModel and then when we retrieved with BaseDatabaseModel (i.e. JModelLegacy) we hit all the issues
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-03-12 13:57:06 |
| Closed_By | ⇒ | wilsonge | |
| Labels |
Added:
?
|
||
It works either way, as far as I can tell. But maybe I'm missing something.
If you're calling it as JModelLegacy::addIncludePath() then yes, it'll work 100% of the time, every time.
However, it's a public method. And as we all know, PHP has class inheritance. So you can also call it as JModelList::addIncludePath(), or WeblinksModelWeblink::addIncludePath() from the outside. And inside models, there are undoubtedly static::addIncludePath() calls. This is where it all falls apart.
Review the 3v4l I posted and you'll see why having the property in the trait starts to fall apart at a complete system level (end to end integration, not a specific isolated case where things will work no matter the parent structure). When PHP composes the classes, it essentially inlines the property separately to each class. So FirstStaticPaths::$paths and SecondStaticPaths::$paths are two totally separate arrays. Whereas, a static property in a class is the same in that class and all descendants of that class. So HasPaths::$paths, ThirdStaticPaths::$paths, and FourthStaticPaths::$paths all reference the same array. Because of the static nature of this array, it's the latter behavior that you actually want.
As for a practical demonstration of how this can be an issue, apply this patch on a CMS install and then add this as an onAfterInitialise listener:
public function onAfterInitialise()
{
\JModelLegacy::addIncludePath(JPATH_ROOT . '/includes');
}Then, in any request add this snippet inside a model constructor (shouldn't matter the component)
var_dump(static::$paths);
// This should work without an issue because you're inside the model class so the protected property should be in scope and accessible
var_dump(\JModelLegacy::$paths);
// But in the off chance it doesn't, Reflection will let us prove the point
$property = (new \ReflectionClass(\JModelLegacy::class))->getProperty('paths');
$property->setAccessible(true);
var_dump($property->getValue());The end result is you'll have two arrays with two different sets of paths, the one inside the model class not including the path you added with a plugin at the beginning of the request.
I’d leave things alone as they are now. The issue with a static property on a trait is that it isn’t going to have the same semantics as a static property on a class. See https://3v4l.org/D9KU0 for exactly why being on the trait is not reliable.