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.