Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

php notice on ExtensionAdapter.php #34959

Closed
jschmi102 opened this issue Jul 28, 2021 · 9 comments
Closed

php notice on ExtensionAdapter.php #34959

jschmi102 opened this issue Jul 28, 2021 · 9 comments

Comments

@jschmi102
Copy link

If I click "check for updates" on extensions I'm seeing 7 php-notices:

PHP Notice: Trying to get property 'id' of non-object in C:\\myjoomla4\\libraries\\src\\Updater\\Adapter\\ExtensionAdapter.php on line 333, referer: http://localhost/myjoomla4/administrator/index.php?option=com_installer&view=update

Could it be that check on ExtensionAdapter.php line 331 is wrong? ( e.g. notice is gone if check is changed to
if (isset($this->latest->client) && !\strlen($this->latest->client))

Or what else is the problem?

@jschmi102
Copy link
Author

hi,
forgot to mention situation happens with Joomla 4.0 RC5

@richard67
Copy link
Member

The proposed change is not right in my opinion from reading the original code here https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Updater/Adapter/ExtensionAdapter.php#L331 . But I can’t dig deeper right now.

@jschmi102
Copy link
Author

jschmi102 commented Jul 29, 2021

hi,
what I've found (what I get) sofar is (in my case):

$this->latest = Joomla\CMS\Table\Update
$this->latest->client = 0
so calling "getClientInfo($this->latest->client, true)" will query client by name ("true") and no id will be returned for client name ="0"

Is my conclusion correct?
so if I change line 331 to
if (isset($this->latest->client) && !is_numeric($this->latest->client))
php notices are gone...

@joomdonation
Copy link
Contributor

@jschmi102 In Joomla4, the client in update server definition needs to be client name (site/administrator) instead of client id like in the past. I don't know when the decision was made but that makes sense because the tag is client, not client_id like in the past

So you should check the update sites of the extensions use on your site, if there is any update server use ID (0, 1) for client, please contact them and tell them to change it to site/administrator to have it compatible with Joomla 4

(I need to change it in my extensions, too, I still use client ID at the moment :( )

Take a look at Akeeba backup update server for example https://cdn.akeeba.com/updates/pkgakeebacore.xml , you will see that they use site instead of <client>0</client)

For more information, we have deprecated using Client ID for client in Joomla 3 already, see https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Updater/Adapter/ExtensionAdapter.php#L356-L365

@jschmi102
Copy link
Author

hi,
thanks for your Info - I dd not know how strict this requirement has to be implemented in Joomla4 update server's xml..

Nevertheless i think the code in ExtensionAdapter.php has to be changed. E..g. check that client is not numeric.
(php code should be clean and should hot produce notices or warnings)

@joomdonation
Copy link
Contributor

@jschmi102 Agree that the code should be updated. Maybe show a warning if ID (0/1) is used instead of name (site/administrator)

@jschmi102
Copy link
Author

How about this (like in joomla3):

  1. define namespace for logs: use Joomla\CMS\Log\Log;
  2. add/change code at line 331
	if (isset($this->latest->client) && strlen($this->latest->client))
			{
				if (!is_numeric($this->latest->client))
				  $this->latest->client_id = ApplicationHelper::getClientInfo($this->latest->client, true)->id;
			        else
				  Log::add('Using numeric values for <client> in the updater xml is deprecated. Use \'administrator\' or \'site\' instead.',
						Log::WARNING, 'deprecated');	

				unset($this->latest->client);
			} 

@joomdonation
Copy link
Contributor

@jschmi102 No, the purpose of deprecated in J3 and removed it in J4 is to force developer to use correct value for client tag on update server

So if it still use numeric value, we should show a warning show that they know about it and update. That's how I think/understand how it should work.

@Quy
Copy link
Contributor

Quy commented Aug 28, 2021

Closing as duplicate of #29778 #35376. Thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants