Useless database query in hikashopPluginsClass::getByName()

  • Posts: 26
  • Thank you received: 2
2 weeks 2 days ago #369858

-- HikaShop version -- : 6.1.1
-- Joomla version -- : 5.4.1
-- PHP version -- : 8.3.0

It's better to use native Joomla plugin helper to get plugin params, it utilizes native Joomla caching of plugins.

    public function getByName(string $type, string $name): ?\stdClass
    {
        $plugin = \Joomla\CMS\Plugin\PluginHelper::getPlugin($type, $name);
        if ($plugin) {
            $this->loadParams($plugin);
        }

        return $plugin;
	}

Last edit: 2 weeks 2 days ago by Denitz.

Please Log in or Create an account to join the conversation.

  • Posts: 84941
  • Thank you received: 13835
  • MODERATOR
2 weeks 2 days ago #369863

Hi,

Thanks again for your feedback.

While you're correct, this is sometimes not possible as we need to be able to load plugins which PluginHelper::getPlugin won't find.
JPluginHelper::getPlugin won't return anything if the plugin is disabled in the Joomla plugins manager. But the $pluginClass->getByName function will be able to provide the data of the plugin in that case.
Making the change you suggest would prevent the code there to work properly.

However, I can see that over the years, we've used getByName more and more all over the place and not having a cache mechanism can lead to a lot of unnecessary MySQL queries while the cases were we specifically need to load an unpublished plugin are quite rare in the code base of HikaShop.

After some deliberation, we decided to do it like this:

	function getByName($type, $name, $fromCache = true){
		if($fromCache){
			$result = JPluginHelper::getPlugin($type, $name);
		} else {
			$query = 'SELECT * FROM '.hikashop_table('extensions',false).' WHERE folder='.$this->database->Quote($type).' AND element='.$this->database->Quote($name).' AND type=\'plugin\'';
			$this->database->setQuery($query);
			$result = $this->database->loadObject();
		}
		if(!empty($result))
			$this->loadParams($result);
		return $result;
	}
This way, the PluginHelper::getPlugin function will be used almost everywhere, and where we specifically want to be able to get the plugin data regardless of the enabled state and we do not need cache, we can set the extra parameter to false to get the previous behavior.

Note that HikaShop will still be compatible with Joomla 3 in the foreseeable future so we still want to use JPluginHelper with a class alias for the \Joomla\CMS\Plugin\PluginHelper class when necessary.

Please Log in or Create an account to join the conversation.

  • Posts: 26
  • Thank you received: 2
2 weeks 1 day ago #369868

OK, thanks for feedback!
To be honest, I don't see the cases when disabled plugin should be loaded, disabled state means that it should not be used.
And Joomla3 compatibility looks a bit strange at the end of 2025, keeping compatibility with Joomla3 and Joomla4 freezes the development process. Hope you will abandon Joomla3 and Joomla4 as soon as possible to get the benefits of new Joomla6.

Small remark: if(!empty($result)) is useless here, it's enough to have if(!$result) and omit useless function call.
And $database has quote() method but not Quote()

Please Log in or Create an account to join the conversation.

  • Posts: 84941
  • Thank you received: 13835
  • MODERATOR
2 weeks 1 day ago #369872

Hi,

Thanks again for your feedback.

To be honest, I don't see the cases when disabled plugin should be loaded, disabled state means that it should not be used.

For example, at the end of the data import from another ecommerce extension, if there is a compatible URL fallback plugin (which can redirect the URLs of the products of that other ecommerce solution to the products in HikaShop), the system will display a specific message about this plugin so that the user can enable it via the Joomla plugins manager.
To decide whether to display or not the message, the system looks for the presence of the corresponding plugin being installed. And obviously, you don't want to enable these plugins by default.

And Joomla3 compatibility looks a bit strange at the end of 2025, keeping compatibility with Joomla3 and Joomla4 freezes the development process.

Joomla version usage statistics still points at Joomla 3 being the majority:
w3techs.com/technologies/details/cm-joomla
While new websites are not made with Joomla 3 anymore, I don't see a reason to not support users stuck with Joomla 3. This allows us the possibility to easily push security updates to them if necessary in the future. Supporting it also makes for an easier migration process for users that want to migrate their HikaShop to the latest version of Joomla.

Hope you will abandon Joomla3 and Joomla4 as soon as possible to get the benefits of new Joomla6.

Supporting Joomla 3 doesn't stop us from implementing improvements made by more recent versions of Joomla.
For example, in 2025, we've added support for email templates which were added in Joomla 5.2, and we have an integration with the Smart Search component which isn't available on Joomla 3, or with the black mode of Joomla 5 and 6.

if(!empty($result)) is useless here, it's enough to have if(!$result) and omit useless function call.

That's indeed correct. Note however that it should be if($result) to keep the same code logic.

And $database has quote() method but not Quote()

Yes. I think that in the past the function was advertised with a capital letter and since the case doesn't matter in function names unless you have two functions with the same letters but different casing, we didn't bother changing it.

Please Log in or Create an account to join the conversation.

  • Posts: 26
  • Thank you received: 2
2 weeks 1 day ago #369874

OK, many thanks for explanation, Happy Holidays!

The following user(s) said Thank You: nicolas

Please Log in or Create an account to join the conversation.

Time to create page: 0.060 seconds
Powered by Kunena Forum