Skip to content

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Jul 30, 2020

Pull Request for Issue #30223 .

Summary of Changes

This pull request (PR) fixes an error from PR #17537 which has introduced database schema checker and fixer for extensions in Joomla 4.

The double error is that an arry index and not the value of an array which holds extension IDs is compared with the extension element value 'com_admin'.

This PR fixes it by using the element property of the extensions element of the previously obtained changeset for the given extension ID.

Testing Instructions

Preparation

Have a clean, current 4.0-dev branch or a 4.0 nightly or a Beta 3 with at least one extension installed, e.g. the patchtester.

Have error reporting set to "Maximum" in Global Configuration in server settings.

In your PHP settings, have display errors on or log errors on and log into a file.

Procedure

  1. Open file administrator/components/com_installer/src/Model/DatabaseModel.php in an editor and go to following line:
    https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_installer/src/Model/DatabaseModel.php#L264.

  2. Before that line, add a line with debug output into the PHP log as follows:
    error_log('Hello, I am com_admin');
    or whatever else comes into your mind, so it finally looks like:

if ($i === 'com_admin')
{
	error_log('Hello, I am com_admin');
	$installer = new \JoomlaInstallerScript;

	...
}
  1. Go to "System -> Information -> Database".

  2. Use the "Update Structure" button with each of the following combinations of selected check boxes and then check your PHP error log if there is the debug log you have added before with the preparation for the test.
    a) Only some extension's check box selected, but not the one for "Joomla CMS" = the core.
    b Only "Joomla CMS" = the core's check box selected, but not any other check box for any extension.
    c) The check box for "Joomla CMS" and also one of some extension selected.
    Result: See section "Actual result BEFORE applying this Pull Request" below.

  3. Apply the patch of this PR.
    Note: When using patchtester you might have to remove the debug log which you have added before in step 2, then apply the patch and after that add back the debug log, so it looks like:

if ($changeSet['extension']->element === 'com_admin')
{
	error_log('Hello, I am com_admin');
	$installer = new \JoomlaInstallerScript;

	...
}
  1. Repeat steps 3 and 4.
    Result: See section "Expected result AFTER applying this Pull Request" below.

Actual result BEFORE applying this Pull Request

In all 3 cases a) to c) no debug log.

Expected result AFTER applying this Pull Request

a) Only some extension's check box selected, but not the one for "Joomla CMS" = the core => No debug log.
b Only "Joomla CMS" = the core's check box selected, but not any other check box for any extension => Debug log.
c) The check box for "Joomla CMS" and also one of some extension selected => Debug log.

Documentation Changes Required

None.

@Denitz
Copy link
Contributor

Denitz commented Jul 30, 2020

I deleted a column in #__content_frontpage table, run the schema checker and see the problem:

Table 'jos_content_frontpage' does not have column 'featured_down'. (From file 4.0.0-2019-08-20.sql.)

But it's displayed for "Joomla CMS" extension (eid=208) where element is joomla. Is it correct to compare element as com_admin ?

Surprisingly, clicking "Update Structure" helped even on the current beta3 code without patch applied (the column was re-created).

@richard67
Copy link
Member Author

richard67 commented Jul 30, 2020

But it's displayed for "Joomla CMS" extension (eid=208) where element is joomla. Is it correct to compare element as com_admin ?

Here it is extension ID 213, which has element = 'com_admin. Maybe the difference is due to your update history? I have used a brand nex installation of current 4.0-dev branch. But you can check it yourself by adding some debug output like in my instructions, extended a bit, e.g. error_log('$i=' . $i . ', $cid=' . $cid . ', $changeSet[\'extension\']->element=' .$changeSet['extension']->element);

deleted a column ....
Surprisingly, clicking "Update Structure" helped even on the current beta3 code without patch applied (the column was re-created).

Well, that is how it shall work and is absolutely unrelated to this PR. The schema changes are done anway, with and without this PR. The only thing what the check for ' com_admin' is good for is to run additional utf8mb4 stuff for the core.

@richard67
Copy link
Member Author

@Denitz You are right, it's weird that in database, the extension record has element = 'joomla'. But could you test as described in the testing instructions, and if in doubt add the additional debug output which I've mentioned in my previous comment? Then you should see it my PR works or not. I think the reason is that the changeset of the core belongs to com_admin and not to joomla files which is used for display.

@richard67
Copy link
Member Author

Just corrected testing instructions, now they are ok.

@richard67
Copy link
Member Author

richard67 commented Jul 30, 2020

@Denitz If you add the error_log('$i=' . $i . ', $cid=' . $cid . ', $changeSet[\'extension\']->element=' . $changeSet['extension']->element); just before the if statement changed by this PR, you will see that it works.

@Denitz
Copy link
Contributor

Denitz commented Jul 30, 2020

@richard67 I performed clean beta3 installation and performed the same trick with column removal.

eid is 213 and element is really 'joomla' but not com_admin.

Schemas are checked via

SELECT `extensions`.`client_id`,`extensions`.`element`,`extensions`.`extension_id`,`extensions`.`folder`,`extensions`.`manifest_cache`,`extensions`.`name`,`extensions`.`type`,`schemas`.`version_id`
FROM `jos_schemas` AS `schemas`
INNER JOIN `jos_extensions` AS `extensions` ON `schemas`.`extension_id` = `extensions`.`extension_id`

#__schemas has a single record with extension_id = 213 which is files_joomla. Hence, #__schemas are unrelated to com_admin at all.

But the load changeset really has element=com_admin even though extension_id is 213 which is not com_admin:

object(stdClass)[723]
  public 'client_id' => int 0
  public 'element' => string 'com_admin' (length=9)
  public 'extension_id' => int 213
  public 'folder' => string '' (length=0)
  public 'manifest_cache' => string '{"name":"files_joomla","type":"file","creationDate":"July 2020","author":"Joomla! Project","copyright":"(C) 2005 - 2020 Open Source Matters. All rights reserved","authorEmail":"[email protected]","authorUrl":"www.joomla.org","version":"4.0.0-beta3","description":"FILES_JOOMLA_XML_DESCRIPTION","group":""}' (length=304)
  public 'name' => string 'Joomla CMS' (length=10)
  public 'type' => string 'file' (length=4)
  public 'version_id' => string '4.0.0-2020-05-29' (length=16)
  public 'creationDate' => string 'July 2020' (length=9)
  public 'author' => string 'Joomla! Project' (length=15)
  public 'copyright' => string '(C) 2005 - 2020 Open Source Matters. All rights reserved' (length=56)
  public 'authorEmail' => string '[email protected]' (length=16)
  public 'authorUrl' => string 'www.joomla.org' (length=14)
  public 'version' => string '4.0.0-beta3' (length=11)
  public 'description' => string 'Joomla! 4 Content Management System.' (length=36)
  public 'group' => string '' (length=0)
  public 'author_info' => string '[email protected]<br>www.joomla.org' (length=34)
  public 'client' => string 'Site' (length=4)
  public 'client_translated' => string 'Site' (length=4)
  public 'type_translated' => string 'File' (length=4)
  public 'folder_translated' => string 'N/A' (length=3)

It's because of model code which auto-replaces element from joomla to com_admin to load the schema SQLs:

			if (strcmp($result->element, 'joomla') === 0)
			{
				$result->element = 'com_admin';

				if (!$this->getDefaultTextFilters())
				{
					$errorMessages[] = Text::_('COM_INSTALLER_MSG_DATABASE_FILTER_ERROR');
					$errorCount++;
				}
			}

So, checking for element=com_admin is correct.

And: the patch works.

But: the utf8mb4 conversion is still executed if #__utf8_conversion table is found and the conversion will be never finished due to new #__history table.

@richard67
Copy link
Member Author

But: the utf8mb4 conversion is still executed if #__utf8_conversion table is found and the conversion will be never finished due to new #__history table.

For fixing this you need my other PR #30227 .

So please test both as described and then mark the test results on the issue tracker by using the "Test this" button, selecting the appropriate test result and then submitting.

Issue tracker item for this PR: https://issues.joomla.org/tracker/joomla-cms/30233

Issue tracker item for the other PR: https://issues.joomla.org/tracker/joomla-cms/30227

Thanks in advance.

@Denitz
Copy link
Contributor

Denitz commented Jul 30, 2020

I have tested this item ✅ successfully on ffd0ee2


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jul 30, 2020

I have tested this item ✅ successfully on ffd0ee2


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

@Quy Quy removed the PR-4.0-dev label Jul 30, 2020
@Quy
Copy link
Contributor

Quy commented Jul 30, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 30, 2020
@Quy Quy added the PR-4.0-dev label Jul 30, 2020
@richard67
Copy link
Member Author

Thanks guys for testing.

@Quy Quy added this to the Joomla 4.0 milestone Jul 31, 2020
@Quy Quy merged commit 51f4619 into joomla:4.0-dev Aug 1, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 1, 2020
@richard67 richard67 deleted the 4.0-dev-fix-dbchecker-for-extensions branch August 2, 2020 12:40
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants