Move WP_Upgrader and WP_Upgrader_Skin subclasses into separate files

[ocean90]

Related: #34432, #36335

trunk/src/wp-admin/includes/class-wp-upgrader-skins.php​ includes WP_Upgrader_Skin, Plugin_Upgrader_Skin, Bulk_Upgrader_Skin, Plugin_Installer_Skin, Theme_Installer_Skin, Theme_Upgrader_Skin, Language_Pack_Upgrader_Skin, and Automatic_Upgrader_Skin.

trunk/src/wp-admin/includes/class-wp-upgrader.php​ includes WP_Upgrader, Plugin_Upgrader, Theme_Upgrader, Language_Pack_Upgrader, and Core_Upgrader.
The same file includes also File_Upload_Upgrader and WP_Automatic_Updater which are not a subclass of WP_Upgrader and the file includes class-wp-upgrader-skins.php.

Sadly, the naming of the classes isn't quite good because they don't have a unique prefix.

Based on the current naming schema we'd have the following new and old files:

wp-admin/includes/class-wp-upgrader.php // Includes WP_Upgrader and requires files below except fo class-wp-upgrader-skins.php.
wp-admin/includes/class-plugin-upgrader.php
wp-admin/includes/class-theme-upgrader.php
wp-admin/includes/class-language-pack-upgrader.php
wp-admin/includes/class-core-upgrader.php
wp-admin/includes/class-file-upload-upgrader.php
wp-admin/includes/class-wp-automatic-updater.php
wp-admin/includes/class-wp-upgrader-skins.php // Only for back-compat, requires files below.
wp-admin/includes/class-automatic-upgrader-skin.php
wp-admin/includes/class-wp-upgrader-skin.php
wp-admin/includes/class-bulk-upgrader-skin.php
wp-admin/includes/class-bulk-plugin-upgrader-skin.php
wp-admin/includes/class-bulk-theme-upgrader-skin.php
wp-admin/includes/class-language-pack-upgrader-skin.php
wp-admin/includes/class-plugin-upgrader-skin.php
wp-admin/includes/class-theme-upgrader-skin.php
wp-admin/includes/class-plugin-installer-skin.php
wp-admin/includes/class-theme-installer-skin.php

Thoughts?

[rmccue]

+100. The prefix doesn't matter too much, as they have a common suffix. (I have a autoloader written that already handles that. :) )

Splitting it would be pretty useful, although has the potential to break currently if someone is loading the file directly. That could be fixed by using an autoloader (#36335) to ensure the class is available regardless of what files are loaded manually.

[ocean90]

Replying to rmccue:

Splitting it would be pretty useful, although has the potential to break currently if someone is loading the file directly.

Can you elaborate on this? In my approach class-wp-upgrader.php will continue loading all the files.

[rmccue]

Just a general comment about splitting, not to this specific ticket. :)

[ocean90]

Noting here that this needs some extensive testing on how it could affect a core upgrade.

@dd32 Any thoughts on this?

[dd32]

I have no objections, to either the main file just including everything else, or using a autoloader (probably in the future - Sorry there wasn't a common prefix!)

The only potential issue would be an incomplete failed update which then couldn't be completed from within wp-admin. As it is, the chances of a failed *major* update leaving WordPress in a state that WordPress can update itself again is rather slim.

[jrf]

Related: https://core.trac.wordpress.org/ticket/34676, especially the second patch which essentially already addressed the splitting of the class to separate files: https://core.trac.wordpress.org/attachment/ticket/34676/34676.2.class-seperation-and-docs.diff

[ocean90]

Let's do this in stages. Each stage is one commit.

1) Create wp-admin/includes/class-automatic-upgrader-skin.php, wp-admin/includes/class-bulk-plugin-upgrader-skin.php, wp-admin/includes/class-bulk-theme-upgrader-skin.php, wp-admin/includes/class-bulk-upgrader-skin.php, wp-admin/includes/class-language-pack-upgrader-skin.php, wp-admin/includes/class-plugin-installer-skin.php, wp-admin/includes/class-plugin-upgrader-skin.php, wp-admin/includes/class-theme-installer-skin.php, wp-admin/includes/class-theme-upgrader-skin.php, and wp-admin/includes/class-wp-upgrader-skin.php as a copy of wp-admin/includes/class-wp-upgrader-skins.php.
2) Update file headers and DocBlocks for each new file.
3) Change wp-admin/includes/class-wp-upgrader-skins.php to require_once all the new files and update file header.

4) Create wp-admin/includes/class-plugin-upgrader.php, wp-admin/includes/class-theme-upgrader.php, wp-admin/includes/class-language-pack-upgrader.php, wp-admin/includes/class-core-upgrader.php, wp-admin/includes/class-file-upload-upgrader.php, and wp-admin/includes/class-wp-automatic-updater.php as a copy of wp-admin/includes/class-wp-upgrader.php.
5) Update file headers and DocBlocks for each new file.
6) Change wp-admin/includes/class-wp-upgrader.php to only include WP_Upgrader and add require_once for all the new files (upgrader and skins). Update file header.

7) If necessary, update file paths for hook docs.
8) Check references to include/require wp-admin/includes/class-wp-upgrader.php and if they can be changed to include/require only necessary files. File_Upload_Upgrader might to be one of those.

[ocean90]

In 37406:

Upgrader: Copy WP_Upgrader_Skin and its subclasses into one file per class.

Part 1/8.
See #36618.

[ocean90]

In 37407:

Upgrader: Update file headers and class DocBlocks for new files added in [37406].

Part 2/8.
See #36618.

[ocean90]

In 37408:

Upgrader: Update wp-admin/includes/class-wp-upgrader-skins.php to require_once the new files added in [37406].

Part 3/8.
See #36618.

[ocean90]

In 37409:

Upgrader: Copy WP_Upgrader subclasses into one file per class.

Part 4/8.
See #36618.

[ocean90]

In 37410:

Upgrader: Update file headers for new files added in [37409].

Part 5/8.
See #36618.

[ocean90]

In 37411:

Upgrader: Update wp-admin/includes/class-wp-upgrader.php to require_once the new files added in [37406] and [37409].

Part 6/8.
See #36618.

[ocean90]

In 37412:

Upgrader: After [37409] move the hook docs for upgrader_process_complete to WP_Upgrader::run().

Add changelog entry for [23912].

Part 7/8.
See #36618.

[ocean90]

Replying to ocean90:

8) Check references to include/require wp-admin/includes/class-wp-upgrader.php and if they can be changed to include/require only necessary files. File_Upload_Upgrader might to be one of those.

File_Upload_Upgrader is only used in /wp-admin/update.php which has an include_once for class-wp-upgrader.php on top. WP_Automatic_Updater is another class which is not a subclass of WP_Upgrader but it's using skins and other upgraders.
For now, we have to continue loading wp-admin/includes/class-wp-upgrader.php until we have an autoloader, see #36335.

[swissspidy]

#36804 was marked as a duplicate.

[swissspidy]

@ocean90 Attention! [37411] broke update functionality in core.

There's a $package = ''; line in WP_Upgrader::download_package(), probably a leftover from debugging.

[ocean90]

In 37413:

Upgrader: Remove debug cruft.

See #36618.

[ocean90]

Replying to swissspidy:

@ocean90 Attention! [37411] broke update functionality in core.

There's a $package = ''; line in WP_Upgrader::download_package(), probably a leftover from debugging.

Thanks!

[ocean90]

In 37432:

Upgrader: Add changelog entries for when the classes were moved to its own file.

See #36618.

[DrewAPicture]

In 38023:

Docs: Cross-reference parent classes in DocBlocks for upgrader classes moved to their own files in 4.6

See #36618. See #37318.