J4 Issue ?
avatar mbabker
mbabker
14 Apr 2019

There are a lot of cases where interfaces come across as very incomplete. With a static analysis tool, there would be loads of errors related to calling methods that do not exist on the documented return type. When interfaces are in use, it is a best practice to only rely on methods that actually exist as part of the interface as anything else typically points to a dependency on a class implementing that interface.

Examples of current uses in core include:

  • Factory::getApplication()->getMenu() - The getMenu() method is not defined in CMSApplicationInterface
  • Factory::getApplication()->get() - The get() method is not defined in CMSApplicationInterface
  • Factory::getApplication()->triggerEvent() - The triggerEvent() method is not defined in CMSApplicationInterface
  • Factory::getApplication()->bootComponent()->getMVCFactory() - The getMVCFactory() method is not defined in ComponentInterface
  • CategoryServiceInterface containing a method related to counting tagged items

Valid fixes include changing doc blocks to accurate return types (i.e. classes instead of interfaces), ensuring methods are defined on interfaces as appropriate, adding inline docblocks indicating the return type for that use case, or adding method_exists() checks throughout the codebase because "unsafe" use of the interfaces are in use ("unsafe" in this context meaning calling methods not declared on a documented return type, generally an interface).

EDITED: STRIKETHROUGH'S BY @wilsonge AS WE MERGE FIXES

avatar mbabker mbabker - open - 14 Apr 2019
avatar joomla-cms-bot joomla-cms-bot - change - 14 Apr 2019
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 14 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 14 Apr 2019
Labels Added: J4 Issue
avatar franz-wohlkoenig franz-wohlkoenig - labeled - 14 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 15 Apr 2019
Status New Discussion
avatar wilsonge
wilsonge - comment - 15 Apr 2019

Factory::getApplication()->get() - The get() method is not defined in CMSApplicationInterface

We should still probably reintroduce the interfaces we revoked here joomla-framework/application@49d2f19 for the get method and extend our interface from this

Factory::getApplication()->triggerEvent() - The triggerEvent() method is not defined in CMSApplicationInterface

This one gets weird. We probably want to define a EventInterface . But we also probably want to make some of the applications own exceptions run through the AbstractApplication's dispatch event rather than the triggerEvent method

Factory::getApplication()->bootComponent()->getMVCFactory() - The getMVCFactory() method is not defined in ComponentInterface

We should check the component interface also implements the https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/MVC/Factory/MVCFactoryServiceInterface.php MVCFactoryServiceInterface interface where required

Factory::getApplication()->getMenu() - The getMenu() method is not defined in CMSApplicationInterface

We should add this in - but we'll need to add add an exception it can throw for applications that don't have menus (cli and webservices). I guess we need the same exception/method for getRouter too

avatar wilsonge wilsonge - change - 7 May 2019
The description was changed
avatar wilsonge wilsonge - edited - 7 May 2019
avatar wilsonge wilsonge - change - 7 May 2019
The description was changed
avatar wilsonge wilsonge - edited - 7 May 2019
avatar infograf768 infograf768 - change - 9 Feb 2020
Labels Added: ?
avatar infograf768 infograf768 - labeled - 9 Feb 2020
avatar richard67 richard67 - change - 9 Feb 2020
Labels Added: ?
Removed: ?
avatar richard67 richard67 - unlabeled - 9 Feb 2020
avatar richard67 richard67 - labeled - 9 Feb 2020
avatar richard67
richard67 - comment - 9 Feb 2020

API has to be complete with beta, so changed label from release blocker to beta blocker.

avatar richard67
richard67 - comment - 9 Feb 2020

@wilsonge Who do you think could be assigned to this? Is above my knowledge of the API.

avatar jwaisner jwaisner - change - 17 Feb 2020
Priority Medium Urgent
avatar joomla-cms-bot joomla-cms-bot - change - 17 Feb 2020
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - unlabeled - 17 Feb 2020
avatar jwaisner
jwaisner - comment - 17 Feb 2020

Increasing priority as per guidelines for beta-blockers.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24587.

avatar jwaisner jwaisner - change - 17 Feb 2020
Labels Added: ?
avatar jwaisner jwaisner - labeled - 17 Feb 2020
avatar wilsonge wilsonge - change - 10 Mar 2020
The description was changed
avatar wilsonge wilsonge - edited - 10 Mar 2020
avatar wilsonge
wilsonge - comment - 12 Mar 2020

Merging #24486 into here. We need to move countTagItems out of the CategoryServiceInterface it obviously makes no sense there.

avatar wilsonge wilsonge - change - 12 Mar 2020
The description was changed
avatar wilsonge wilsonge - edited - 12 Mar 2020
avatar wilsonge wilsonge - change - 18 Mar 2020
Labels Added: ? ?
Removed: ?
avatar wilsonge wilsonge - labeled - 18 Mar 2020
avatar wilsonge wilsonge - change - 18 Mar 2020
Labels Removed: ?
avatar wilsonge wilsonge - unlabeled - 18 Mar 2020
avatar wilsonge
wilsonge - comment - 6 Apr 2020

As far as I can see this is now complete. @mbabker do you have any other places you can find? If not I'm going to mark this as closed

avatar jwaisner
jwaisner - comment - 6 May 2020

@mbabker following up on if this has been taken care of and if we can close.

avatar jwaisner
jwaisner - comment - 26 May 2020

@mbabker following up on this. Has this been resolved?

avatar richard67
richard67 - comment - 29 May 2020

@wilsonge There hasn't been any reply by @mbabker , so I would assume there are no more open items. Can this be closed?

avatar wilsonge wilsonge - change - 30 May 2020
Status Discussion Closed
Closed_Date 0000-00-00 00:00:00 2020-05-30 17:58:47
Closed_By wilsonge
avatar wilsonge wilsonge - close - 30 May 2020
avatar wilsonge wilsonge - change - 30 May 2020
Labels Removed: ?
avatar wilsonge wilsonge - unlabeled - 30 May 2020

Add a Comment

Login with GitHub to post a comment