[5.0] Decouple models from CMSObject

[wilsonge]

Removes the dependency of \Joomla\CMS\MVC\BaseModel on \Joomla\CMS\Object\CMSObject. We polyfill in support for setError/getError/getErrors for compatibility with how most models across our codebase do error handling and replace it with a method to have exceptions (note the controller change is an example of how I see the support working - if this PoC is accepted I'll roll it out to all controller methods).

[nikosdion]

Before doing that, please check the code base. I believe that we have CMSObject type hints which need to go before we refactor out CMSObject itself.

Also note that since at least Joomla 1.5 (maybe 1.0 but I can't remember that far back) the MVC objects extended JObject / CMSObject. A lot of code written over the past 15 years assumes that getProperties, get and set are available; it's not just the legacy error handling. Moreover, some code does check if an object descends from CMSObject.

This is the reason why in my PR I did NOT remove CMSObject, I had it extend from a b/c shim. The idea was that 5.0 would introduce the shim and mark it deprecated (removed in 7.0), 6.0 would remove the legacy error handling and 7.0 would get rid of it completely.

The way you are trying to do it you make CMSObject disappear in 5.x —a major b/c break— when we have promised this won't happen. I don't see this as manageable from a backwards compatibility promise point of view. The code is fine, the time it comes is not. Moreover, it does not provide the promised bridge from one major version to the next. No plugin can work around base Joomla MVC classes having a completely different (or no) class to extend from.

[wilsonge]

Before doing that, please check the code base. I believe that we have CMSObject type hints which need to go before we refactor out CMSObject itself.

I did run this locally last night and it seemed to work fine (I haven't tried every single view - but did content/contact/newsfeeds in the frontend and article edit/article list/plugin list + dashboard views in the backend). Everything there seemed to work ok with this patch so I don't believe that is the case.

I'm not claiming it's perfectly B/C - it's obviously not - but trying to draw a line around what's likely to be used and what's not at least in core (and this isn't the result of any changes that I've made in the Model for 4.x btw - they were pretty much untouched from 3.x - most changes went to controller + views in 4.0)

[nikosdion]

Ah, I misremembered. It's the Serialiser which is looking for a CMSObject. Well, that and the Form. This means that you can't actually refactor Tables out of CMSObject unless you make sure they convert cleanly to arrays — which might be too much b/c break.

Still, I think that your code needs to go into Joomla 6, not 5. There was a whole debacle in August about not breaking J4 extensions written with the J3 API in J5. I don't see how we can remove CMSObject and not break some of these extensions.

I think the best solution would be a fusion of what you and I did. Instead of having my CMSDynamicObject replace everything in CMSObject with Traits. The Traits themselves can have a deprecation target of 7.0. In Joomla 5 we change core code to no longer rely on CMSObject. In Joomla 6 CMSObject is removed completely. In Joomla 7 the Traits go away.

This way:

• Code written for J3/J4 expecting CMSObject works on J5 but gets deprecation notices
• Code written for J5, expecting the Traits to exist (but NOT CMSObject itself) will continue working on J6 with deprecation notices.
• Code written for J6 (no CMSObject, no Traits) will continue working on J7

This implements the Major+2 obsolescence clarification given in August.

Yes, yes, I know CMSObject was deprecated since 3 but there was no replacement for it in 3 and 4. Therefore we can't pull the rug under the feet of developers. In the end of the day, we will still need to publish code which works on Joomla 4.3 and 5.0, 5.1, 5.2 and 5.3. Otherwise our users are left without an upgrade path. Rest assured that more than half of the sites out there will go from 4.3 to 5.3 because their template or some such holds them back. I am already seeing it with Joomla 4. There was an initial wave of early adopters who went from 3.9 to 3.10 and immediately to 4.0, nothing for a year, now there's a big wave going from 3.10 to 4.2 (they finally got the budget approved) but by the time they are ready to launch 4.3 will be out. Effectively, they are going from 3.10 to 4.3. LTS to LTS.

[wilsonge]

Still, I think that your code needs to go into Joomla 6, not 5. There was a whole debacle in August about not breaking J4 extensions written with the J3 API in J5. I don't see how we can remove CMSObject and not break some of these extensions.

I think it needs to be staggered. I think the earlier we introduce the interface for exception catching the better (and I mean J4) - we can choose when we implement the exceptions - but having the interface and try/catch logic around the controllers as an example will allow people to see clearly what the path looks like.

Fully accept moving inheritance around might need to come later - but if we can agree on a path I'm happy to figure out the right versions for things.

Ah, I misremembered. It's the Serialiser which is looking for a CMSObject. Well, that and the Form. This means that you can't actually refactor Tables out of CMSObject unless you make sure they convert cleanly to arrays — which might be too much b/c break.

Yeah I have no clue how to fix JTable :) I mean for the serializer I'm pretty sure we can add the support into that serializer once it becomes available so I'm not so fussed about that. But Table uses the full get/set/getProperties/setError/getError construct which fills me with fear.

I think the best solution would be a fusion of what you and I did. Instead of having my CMSDynamicObject replace everything in CMSObject with Traits. The Traits themselves can have a deprecation target of 7.0. In Joomla 5 we change core code to no longer rely on CMSObject. In Joomla 6 CMSObject is removed completely. In Joomla 7 the Traits go away.

I'm ok with this - but depends how hard an eventual php 9 breaks CMSObject - because joomla 7 is effectively be 7'ish years away assuming the major every 2 years thing.

[nikosdion]

I think the earlier we introduce the interface for exception catching the better (and I mean J4) - we can choose when we implement the exceptions - but having the interface and try/catch logic around the controllers as an example will allow people to see clearly what the path looks like.

I agree, as long as we realise that this is a b/c break which needs to be announced, not just buried in the internals.

As soon as setError starts throwing an exception instead of stacking errors, all calls to getError need to be replaced by try/catch blocks. There is no way to make it b/c per se. This means we can't do that by default in either J4 or J5. Clarification: we can't have either J4 or J5 throw exceptions by default. The default behaviour will still have to be stacked messages.

We can introduce a new flag which makes getError throw exceptions instead of stacking errors AND update the core MVC to expect both stacked errors and exceptions. Something like:

try {
  // Our regular call which might be throwing exceptions or using setError.
  $foo->doSomething();

  // Upgrade stacked errors to exceptions.
  if ($foo->getError()) {
    throw new RuntimeException($foo->getError);
  }
} catch (\RuntimeException $e) {
  // Do error handling here.
}

We can have that as optional in J4/J5 and turn it on by default in J6. Still, code written for J5 (catching exceptions) will work on J6 just fine, with just an if-block to no longer try to set the flag as it will be removed in 6.

Yeah I have no clue how to fix JTable :) I mean for the serializer I'm pretty sure we can add the support into that serializer once it becomes available so I'm not so fussed about that. But Table uses the full get/set/getProperties/setError/getError construct which fills me with fear.

Ah, see, this is why I said that we should be using Traits.

The get/set methods simply declare properties dynamically into the table. We need a three-pronged approach to get rid of set/get:

• Add the #[\AllowDynamicProperties] attribute to Table so things don't break in 8.2
• Use a Trait with (deprecated) get/set methods which create and read properties; calling these should also log a deprecated notice
• Move all core tables to having concrete public properties, indicating that this is the best way to move forward (from a performance POV this would be spanking great as well!)

For getProperties() we need Table to have a deprecated, concrete method which proxies it to a new concrete method getRecordData. The deprecated method would log a deprecated notice when called. The new concrete method will KINDA do what getProperties does: return all the public properties of the table object. Look at my PR for inspiration. I do that without reflection and it's really fast!

The getError / setError would be implemented by the Trait we already discussed above.

I'm ok with this - but depends how hard an eventual php 9 breaks CMSObject - because joomla 7 is effectively be 7'ish years away assuming the major every 2 years thing.

So, the PHP RFC didn't say it but Stitcher explained it: https://stitcher.io/blog/new-in-php-82#deprecate-dynamic-properties-rfc and https://stitcher.io/blog/deprecated-dynamic-properties-in-php-82

Basically, as long as we either extend from stdClass OR use the #[\AllowDynamicProperties] attribute we are golden. I did some tests on PHP 8.2-rc3 and I can confirm that descendants of these classes do not complain either; the “can have dynamic properties” magic flows to the descendants. See https://3v4l.org/fp9h8#v8.2rc3

If PHP 10 removes dynamic attributes we will see what we'll do. I don't know if that would even happen, honestly. It would be a hell of a lot of trouble with negative performance impact. Then again, maybe PHP gives us something like an SplDynamicProperties class we can extend from, though I can't see how that would work better than an attribute (in PHP 9 the default PHP object won't allow dynamic properties, therefore the PHP VM can already optimise property access for most objects which do not have the attribute or extends from stdClass as far as I can understand the logic behind this).

[laoneo]

I would love to see the CMSObject being split up into traits in 4.3, so we will give developers enough time to start the transition already in 4.x. I'm fine removing the CMSObject inheritance from the Model in 5.0 as it is unlikely somebody checks if the model extends from CMSObject in an extension. At least I see no use case for it.

[laoneo]

Made a pr with the traits in 4.3 #39020.

[wilsonge]

I've created a PR in #41809 to add the interface. The trait's been added and implemented as @laoneo linked to. So effectively this has been fully implemented (we need to create exceptions and start using them in the J5 branch but that can be done slowly over time - I guess J5.1 and upwards). Closing this as an RFC