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

[4.0] Adapt utf8mb4 conversion to latest changes in the 3.10-dev branch #29247

Conversation

richard67
Copy link
Member

@richard67 richard67 commented May 28, 2020

Pull Request for Issue # .

Summary of Changes

When latest changes of the staging branch, which include PR #29117 , will be merged up into 3.10-dev, the utf8mb4 conversion in J4 has to be adapted to these changes so it doesn't run if not needed.

This is done with this Pull Request (PR) here.

As long as the upmerge has not happened yet, I'll keep this PR in draft status on GitHub and with "[WiP]" (work in progress) in the title.

Ideally both the upmerge from staging to 3.10-dev and the merge of this PR here happen before J4 Beta 1 is built, so that people can test updates from 3.10 to 4 using the latest 3.10 nightly as starting point.

Testing Instructions

Code review with respect to the changes in PR #29117 is sufficient.

But if someone insists on a real test ....

Preconditions

  1. This PR is only relevant for MySQL and MariaDB databases but not for PostgreSQL.
  2. It needs to test updating of current 3.10-dev with the latest upmerged from staging from today, i.e. nigthly build will be usable beginning with the next one.
  3. Because most testers don't have versions of MySQL older than 5.5.3 available which don't support utf8mb4 for testing a real database migration or server update scenario with utf8mb4 conversion, it needs to patch the database driver of the 3.10-dev before update for the test with utf8mb4 conversion, so the driver detects no utf8mb4 support. For the MySQLi driver this can be done by adding a line return false; before the following line:
    https://github.com/joomla/joomla-cms/blob/3.10-dev/libraries/joomla/database/driver/mysqli.php#L961
    so that funtion serverClaimsUtf8mb4Supportalways returns false.
    For the MySQL (PDO) driver it would be more complicated, therefore please test with the MySQLi driver.
  4. For verifying if the conversions runs or not, I have build a modified update package based on the package built for this PR by drone plus some debug output. It can be downloaded from here: https://test5.richard-fath.de/Joomla_4.0.0-beta1-dev+pr.29247-Development-Update_Package_debug.zip.
  5. To be able to see the debug output mentioned in 4. above, you have to switch on error loging into a log file in your PHP settings in a php.ini or .user.ini file:
    log_errors = On;
    error_log = "/full-path-to-log-folder/php-errors.log";
    Alternatively, if PHP errors are logged already into your webserver's log file, just watch that log file during the tests.

Test 1: New installation

This test shall show that new installation is not broken by this PR.

  1. Apply the patch of this PR to a clean 4.0-dev staging branch or a 4.0-beta1-dev nightly build.

  2. Make a new installation into an empty database.

  3. Log in to backend and go to "System - Information - Database".

Result: No database error is shown.

  1. Check with a tool like e.g. PhpMyAdmin in your database if a table named #__utf8_conversion exists (replace #__ by your database prefix).

Result: The table doesn't exist.

  1. Check with the same tool the collation of all core tables.

Result: All database tables have collation utf8mb4_unicode_ci.

Test 2: Update without utf8mb4 conversion

  1. Update a clean 3.0-dev from right now or the next nightly build from tonight to 4.0 using the modified update package mentioned in item 4 of the preconditions section above.

Result: No SQL errors, all database tables have utf8mb4_unicode_ci collation.

  1. Check your PHP log if you can find one of the following logs 'DEBUG: Running utf8mb4-conversion.sql.' or 'DEBUG: Running utf8mb4-conversion_optional.sql.'.

Result: No such logs.

Test 3: Update with utf8mb4 conversion

  1. On a clean 3.0-dev from right now, or the next nightly build from tonight, patch the database driver as described in item 3 of the preconditions section above.

  2. Make a new installation.

Result: All database tables have utf8_unicode_ci or utf8_general_ci collation.

  1. Update to 4.0 using the modified update package mentioned in item 4 of the preconditions section above.

Result: No SQL errors, all database tables have utf8mb4_unicode collation.

  1. Check your PHP log if you can find one of the following logs 'DEBUG: Running utf8mb4-conversion.sql.' or 'DEBUG: Running utf8mb4-conversion_optional.sql.'.

Result: Both logs are shown if table #__core_log_searches existed before the update, otherwise only the first log is shown.

Expected result

When updating a 3.10-dev which is not converted yet to utf8mb4, the conversion runs as well as before when updating to 4.

When updating a 3.10-dev which is already on utf8mb4, the conversion doesn't run when updating to 4.

Actual result

The utf8mb4 conversion runs also if a 3.10-dev which is already on utf8mb4 is updated to 4.

Documentation Changes Required

None.

@wilsonge
Copy link
Contributor

@richard67 can you get this one ready please

@richard67 richard67 marked this pull request as ready for review May 29, 2020 22:15
@richard67
Copy link
Member Author

wait there is missing something

@richard67
Copy link
Member Author

Ah, no, all ok.

@richard67
Copy link
Member Author

@wilsonge Ready.

@richard67 richard67 changed the title [4.0] [WiP] Adapt utf8mb4 conversion to latest changes in the 3.10-dev branch [4.0] Adapt utf8mb4 conversion to latest changes in the 3.10-dev branch May 30, 2020
@wilsonge wilsonge merged commit ac6acd8 into joomla:4.0-dev May 30, 2020
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone May 30, 2020
@richard67 richard67 deleted the 4.0-dev-fix-utf8mb4-conversion-after-pr-29117-upmerge branch May 30, 2020 09:56
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.

None yet

3 participants