[4.0] Revert removal of the "custom_data" field of the "extensions" database table.

[richard67]

Pull Request for Issue #19348 .

Summary of Changes

This Pull Request (PR) reverts the removal of column custom_data from the extensions table made once with PR #14750 , as requested by extension developers in the referenced issue.

Testing Instructions

Test 1: Reproduce the issue

1. Have a current 4.0-dev branch or latest nightly build, either a new installation or one updated from 3.10, whatever you just have, and if you have nothing just install one.

2. With a tool like e.g. phpMyAdmin (MySQL or MariaDB) or phpPgAdmin (PostgreSQL), check if column custom_data of the extensions table exists.

Result: See section "Actual result BEFORE applying this Pull Request" below. The column doesn't exist.

Test 2: New installation with the patch of this PR applied

This test shall verify that that the column custom_data of the extensions table is there after a new installation, and that installing extensions of any kind works.

1. Apply the patch of this PR on a current 4.0-dev branch or latest nightly build and make a new installation, or use the installation package which has been built for this PR to make a new installation.

You can find a page with a link to an installation package for download as shown in the following screenshot with the green ellipse (if necessary expand the "Show all checks" to see that):

2. After the installation has finished, login to backend ang to to "System - Information - Database. Check if there are database problems shown.

Result: If you have used the installation package which has been built for this PR, there is only one database problem about not matching versions shown, otherwise no database problem is shown.

3. With a tool like e.g. phpMyAdmin (MySQL or MariaDB) or phpPgAdmin (PostgreSQL), check if column custom_data of the extensions table exists.

Result: The column exists.

4. Install extensions of different types, whatever you can find. Possible types are:

• Component
• File
• Language
• Library
• Module
• Package
• Plugin
• Template

I'm not sure if you can find extensions of all of these types which are ready for J4, but test whatever you can, the more, the better.

Result: No SQL errors about column custom_data not having a default value. The PHP added back with this PR prevents this.

Test 3: Update a 3.10 to a 4 which has the patch of this PR applied

This test shall verify that that the column custom_data of the extensions table is still there after updating a 3.10.

1. Update a current 3.10-dev or 3.10 nightly to a 4.0-dev which contains the patch of this PR.

You can find a page with a link to a custom update URL and an update package for download at the same place as described in thep 1 or the previous test 1 for the installation package.

Result: The update suceeds without SQL errors.

2. With a tool like e.g. phpMyAdmin (MySQL or MariaDB) or phpPgAdmin (PostgreSQL), check if column custom_data of the extensions table exists.

Result: The column exists.

Test 4: Update a 4 without this patch to a 4 which has the patch of this PR applied, including a manual pre-update procedure

This test shall verify that the manual pre-update procedure to fix updating e.g. from 4.0 Beta-3 to a future Beta-4 which will contain the patch of this PR is sufficient to make the update succeed.

1. Manual pre-update step: On an installation of a current 4.0-dev branch or latest nightly build which does not contain the patch of this PR, use a tool like e.g. phpMyAdmin (MySQL or MariaDB) or phpPgAdmin (PostgreSQL) to run following SQL commands, having replaced #_ by the database prefix of your installation.

On MySQL or MariaDB databases:

ALTER TABLE `#__extensions` ADD COLUMN `custom_data` text NOT NULL;

On PostgreSQL databases:

ALTER TABLE "#__extensions" ADD COLUMN "custom_data" text NOT NULL;

2. Update this installation to a newer 4.0-dev which contains the patch of this PR.
   See step 1 of test 1 about where to find a custom update URL or update package.

Result: The update suceeds without SQL errors.

3. With a tool like e.g. phpMyAdmin (MySQL or MariaDB) or phpPgAdmin (PostgreSQL), check if column custom_data of the extensions table still exists.

Result: The column still exists.

4. Install extensions of different types, whatever you can find. See step 4 of test 2 for possible extension types.

Result: No SQL errors about column custom_data not having a default value. The PHP added back with this PR prevents this.

Actual result BEFORE applying this Pull Request

Colum custom_data of the #__extensions table is removed when updating 3.10 to 4 and not existing on a new installation of 4.

Expected result AFTER applying this Pull Request

Colum custom_data of the #__extensions table is not removed when updating 3.10 to 4, and it is existing on a installation of 4.

Updating a previous 4 Beta version to a future one which contains this PR will require a manual step for other reasons (see e.g. #30333 #30363 #30365 for details). After this manual step, colum custom_data of the #__extensions tables will be back, and after the update it will be initialized properly when installing any kind of extension.

Documentation Changes Required

We haven't mentioned the removal of this column in https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4, as far as I can see, so no need to change something there.

But in the release notes of a future J4 Beta which will include this PR we need to describe the manual step, see "Test 4" in the testing instructions. In additon it might need other manual steps for another issue not related to this PR, see e.g. #30369 .

[wilsonge]

I know you know - but so it's in this issue rather than glip - we'll also need to add back some of the PHP from #14750 that got removed too I guess

[richard67]

I see. Will check later.

[richard67]

@wilsonge But it doesn't need to add back the column to the unit tests, does it?

[wilsonge]

Depends, currently that column is marked as NOT NULL so IF those tests still exist it may need adding else you're going to get an error from SQL i guess?

[richard67]

@wilsonge

Depends, currently that column is marked as NOT NULL

Yes, and we shouldn't add a default value because it's a column of type text.

so IF those tests still exist it may need adding else you're going to get an error from SQL i guess?

Yes. But I can't find those tests in the CMS in 4.0. Could you give me a hint? Or are they coming from the framework now?

[richard67]

Wait, I just notice I have to fix something. Insert statements into the extensions table in SQL scripts running after the ones modified by this PR need tio be extended by the column and values for it because the column doesn't have and shall not have a default value. Text type columns shall not have a defauls in MySQL 8.

[richard67]

Drone failure is unrelated.

Now finally ready for testing.

[wilsonge]

Largely looks good to me

We’re still using an empty string for 3rd party extensions. I also ask because the core extensions update scripts will have the empty json string but if they are discover installed from a 3.x update then it will be an empty string. Is it not better to use empty json there too?

Also because you’ve added the property into the base table class I assume we no longer need it in the adapters.

[SharkyKZ]

Keep using empty string. The column may not necessarily be used for JSON.

Insert statements in install SQL need to be updated.

[richard67]

@SharkyKZ For the custom_data I use consistently empty string. The change to empty json I’ve made at some places for the „params“ column, which at the most places is json but at a few places empty string. Please review this again and confirm.

Regarding the insert statements in the installation SQL you are right, I will add that tomorrow.

[richard67]

@steffans Do you have a build of your YOOtheme Pro theme available which can be installed on J4 and uses the custom_data field, or can you easily prepare suchg a build? If so: Could you use that for testing this pull request here, checking that saving data in that field works, in addition to what the testing instructions say? That would be very helpful. Thanks in advance.

[richard67]

@SharkyKZ Done. I've also remove the unrelated standardisation of the params column, because when working on the base.sql I saw we have more ugly mix of '' and {}, and it made the diff for this PR hard to read. Shall someone else clean up that mess one day ;-) So now it should be fine. Thanks for your review.

[richard67]

@wilsonge

We’re still using an empty string for 3rd party extensions. I also ask because the core extensions update scripts will have the empty json string but if they are discover installed from a 3.x update then it will be an empty string. Is it not better to use empty json there too?

I've found a terrible mix of empty string or empty json in the update sqwl scripts, partly even mixed in the same one script. So I won't touch that. For the custom_data I use empty string everywhere. I had also made a standardisation of the "params" to empty json, but that was confusing here so I've reverted it.

Also because you’ve added the property into the base table class I assume we no longer need it in the adapters.

You mean revert all changes in this PR for the "*Adapter.php" files?

[richard67]

@wilsonge Assuming I understood you right I've reverted the adapter changes. Tested here and it still works. No idea thought why the MySQL system tests fail. Here installation works.

[richard67]

@wilsonge MySQL system test failure is related. No idea why I don't get that here. Please don't merge this by review as long as this is not fixed.

[richard67]

Ok, all fixed. Ready for testing.

[wilsonge]

I want this in beta 4 so merging even though it doesn't have tests. Thankyou @richard67

[richard67]

@wilsonge Please remember to add a hint in the release notes that people updating from 4.0 Beta 1, 2 or 4 to the Beta 4 have to add the custom_data database column "manually" in their SQL client (e.g. phpMyAdmin). The database fixer won't show anything because there is no "ALTER TABLE ..." statement anymore for this field in the update SQL scripts.

[wilsonge]

It will be in the notes

[hendrikbehncke]

Hi, @richard67 @wilsonge
thank you guys for handling the issue.
One last change would be super awesome, if the type could be changed from TEXT to LONGTEXT?

[wilsonge]

Going to point back to this one I'm afraid #19618 (comment) (not the same field obviously but I'm going to provide the same justification)