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

Autoloader: refactor Plugins_Handler and add unit tests #15516

Merged
merged 2 commits into from
Apr 27, 2020

Conversation

kbrown9
Copy link
Member

@kbrown9 kbrown9 commented Apr 22, 2020

This branch is based on the feature/update_autoloader branch.

Changes proposed in this Pull Request:

  • Refactor Plugins_Handler by adding a few methods. This change will make the class easier to test. The new methods are:
    • get_multisite_plugins()
    • get_active_plugins()
  • Add unit tests for Plugins_Handler.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • This changes an existing part of Jetpack.

Testing instructions:

  • This branch includes a small refactor, so make sure that Jetpack still works without errors.

Proposed changelog entry for your changes:

  • n/a

Refactor a few methods in Plugins_Handler to make the class easier
to test.
@kbrown9 kbrown9 added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Pri] Normal [Focus] Jetpack DNA labels Apr 22, 2020
@kbrown9 kbrown9 self-assigned this Apr 22, 2020
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Apr 22, 2020
@jetpackbot
Copy link

jetpackbot commented Apr 22, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: May 5, 2020.
Scheduled code freeze: April 28, 2020

Generated by 🚫 dangerJS against 1ad4b36

@kbrown9 kbrown9 added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Apr 23, 2020
@kbrown9 kbrown9 requested a review from sergeymitr April 23, 2020 20:42
@kbrown9 kbrown9 merged commit 0d7a0ff into feature/update_autoloader Apr 27, 2020
@kbrown9 kbrown9 deleted the update/autoloader_tests branch April 27, 2020 19:48
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Apr 27, 2020
@kbrown9 kbrown9 removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Apr 29, 2020
kbrown9 added a commit that referenced this pull request May 14, 2020
* Autoloader: refactor a few methods in Plugins_Handler

Refactor a few methods in Plugins_Handler to make the class easier
to test.

* Autoloader: add unit tests for Plugins_Handler
kbrown9 added a commit that referenced this pull request May 22, 2020
* Autoloader: refactor a few methods in Plugins_Handler

Refactor a few methods in Plugins_Handler to make the class easier
to test.

* Autoloader: add unit tests for Plugins_Handler
kraftbj pushed a commit that referenced this pull request Jun 22, 2020
* Autoloader: refactor a few methods in Plugins_Handler

Refactor a few methods in Plugins_Handler to make the class easier
to test.

* Autoloader: add unit tests for Plugins_Handler
dereksmart pushed a commit that referenced this pull request Jun 23, 2020
* Autoloader: remove ignore list and notice (#14938)

We're planning to remove the plugins_loaded restriction, so let's
remove the ignore list and the PHP notice.

* Autoloader: Change autoloader function name to autoload (#14937)

Change the function name from 'autoloader' to 'autoload'. This change will
allow us to use a new version of the autoloader function when an old
version has been loaded before the new version.

* Moves autoloader code into a separate file and renames class/file maps. (#14944)

* Added autoloader meta file generation.

To be used to determine the package version that has generated the autoloader, and figuring out what autoloader should take precedence.

* Removed the meta file, instead moved the autoloader function to a separate file for easier development.

* Renamed the classmap files to avoid confusing them with old autoloader class and file maps.

* Removed unneeded changes.

* Renamed class and file map files to a jetpack specific name to avoid conflicts.

* Fixed an invalid rename, thank you @kbrown9!

* Removed a redundant composer.json file declaration.

* Using suffix in the namespace to allow for repeating names.

* Wrapped the code that runs on file load in a function to make unit tests pass.

* Find the latest autoloader on first load. (#14992)

* [not verified] Autoloader: add the set_up_autoloader function.

* [not verified] Autoloader: update global array names and remove spl_autoload_register

Update the global classmap and filemap array names so the old autoloaders
can't overwrite the new autoloader's version selections.

Remove the call to spl_autoload_register(), because that it now handled
in set_up_autoloader().

* [not verified] Moved the autoloader functions into a separate include.

* Autoloader: remove setup() from test_file_loader.php

The spl_autoload_register() call isn't needed in setup() and the
setup() method doesn't do anything else, so remove the entire setup()
method.

Co-authored-by: Kim Brown <50059399+kbrown9@users.noreply.github.com>

* Autoloader: use all of the map files to populate the global maps (#15002)

* Autoloader: loop through all map files in enqueue_files()

The latest autoloader needs to loop through all of the classmap and
filemap files created by the new autoloader and populate the globals.
Add a loop through the active plugins to the enqueue_files() function.

* Autoloader: improve enqueue_files

Improve enqueue_files() by:
 - checking the map files with is_readable instead of file_exists
 - checking that the local map variables are arrays before using them
 - move the file_loader() block out of the active plugins loop

* Autoloader: remove plugins_loaded action conditional around file_loader

We no longer need to wait until the plugins_loaded action to call file_loader(),
so remove the conditionals and the plugins_loaded hook.

* Autoloader: Handle an activating plugin

When a plugin is in the process of activating, it won't be in the active
plugins list yet. We need to make sure that its classmap and filemap files
are processed and the globals are updated so that the autoloader can use
them with the newly activated plugin.

 - In enqueue_files(), process the current plugin's map file and the map files
   for all active plugins.
 - In set_up_autoloader(), detect if a new autoloader has been loaded by an
   activating plugin. If it has, reset the global classmap.

* Autoloader: handle an activating plugin

When a plugin is in the process of activating, it's not in the active
plugins list. We need to add the activating plugin's packages to the
autoloader before the plugin loads.

* Added a check against bulk activation.

* Added network activated plugins to the active plugin list.

* Added a nonce 'check'.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>

* Autoloader: refactor functions.php (#15111)

* Autoloader: refactor functions.php into classes

* Autoloader: update unit tests

Update the unit tests to use the new Classes_Handler and Files_Handler classes.

* Autoloader: update is_current_plugin_active()

Update is_current_plugin_active() to check both active and activating
plugins when determining whether the current plugin is active. The
active and activating plugins can be obtained using the
Plugins_Handler::get_active_plugins() method.

* Added newly generated files to gitignore.

* Autoloader: improve Plugins_Handler

Improve the Plugins_Handler class by:
 - Giving some methods more descriptive names.
 - Adding a $jetpack_autoloader_activating_plugins global variable that
   contains the plugins that are activating via a non-request method.
 - Refactoring a few methods to improve clarity.

* Autoloader: update the autoloader chain after autoloader reset

Add a new function, Autoloader_Handler::update_autoloader_chain(), which
handles updates of the autoloader chain as follows:
 - Registers the latest autoloader function.
 - Moves the v1 autoloader function to the end of the chain.
 - Removes any other v2 autoloader functions. This is needed for situations where
   a plugin activates using a non-request method, so the activating plugin was not
   known when the autoloader first ran. If the activating plugin has an autoloader
   with a later version, that autoloader will be registered and the previously
   registered autoloader function should be removed.

Finally, use the Autoloader_Handler::update_autoloader_chain() method in
set_up_autoloader().

* Autoloader: update update_autoloader_chain()

In Autoloader_Handler::update_autoloader_chain(), check whether the autoloader
object is a string. If not, just continue. We're only looking for Jetpack
autoloader functions, and they're registered as strings.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>

* Autoloader: Skip single-file plugins (#15232)

Some plugins don't have a dedicated directory and only consist of a single file (e.g. 'Hello Dolly').
This means they also don't use Composer, so we can skip them entirely.

* Autoloader: add Version_Selector class (#15135)

Add a new class, Version_Selector, which is used by the Autoloader_Handler,
Classes_Handler, and Files_Handler classes to select package versions.

Remove Plugins_Handler::create_map_path_array() and Plugins_Handler::get_active_plugins_paths()
because they're no longer used.

* Autoloader: display an error message if a non-default vendor directory is used (#15364)

The autoloader package only supports the default composer vendor directory. If
a consuming plugin uses a non-default vendor directory, generate an error during
composer dump-autoload.

* Autoloader: remove file_exists() before loading the file (#15375)

* Autoloader: remove unneeded methods (#15480)

Remove Plugins_Handler::create_map_path_array() and
Plugins_Handler::get_active_plugins_paths() because they are
not used.

* Autoloader: refactor Plugins_Handler and add unit tests (#15516)

* Autoloader: refactor a few methods in Plugins_Handler

Refactor a few methods in Plugins_Handler to make the class easier
to test.

* Autoloader: add unit tests for Plugins_Handler

* Autoloader: add unit tests for Version_Selector (#15592)

* Autoloader: improve performance by using directories to identify plugins

Performance profiling showed that the get_plugins() call in
Plugins_Handler::get_current_plugin() is the longest running call in the
autoloader setup process.

We use get_plugins() to obtain the main file for the current plugin. Instead
of identifying plugins by their directory and main file, let's identify them
with their directory. This will allow us to eliminate the get_plugins() call
and improve autoloader performance.

* Autoloader: add a new method for converting plugin dir/file strings to dirs

* Autoloader: refactor Plugins_Handler and unit tests

Refactor Plugin_Handler so that units tests will cover the
Plugins_Handler::convert_plugins_to_dirs() method. Also fix the
Plugins_Handler unit tests.

* Autoloader: fix Plugins_Handler unit tests

PHPUnit5.7.27 doesn't support the assertEqualsCanonicalizing() method,
so just sort the output arrays before calling assertEquals().

* Autoloader: handle an updating plugin

The Autoloader generates the in-memory classmap when the first autoloader package
is loaded. When a plugin is updated, the path to a package class could have changed,
making the in-memory classmap point to an invalid file location.

Reset the autoloader when a plugin with the v2.x autoloader is updated so that the classmap
is kept up to date.

* Autoloader: remove the unnecessary 'convert_plugins_to_dirs()' calls

The plugin slugs are converted to the directory names in the get_all_active_plugins()
method. So remove the convert_plugins_to_dirs() calls from get_multisite_plugins()
and get_active_plugins().

* Autoloader: move the upgrader_post_install hook callback to Plugins_Handler

* Autoloader: fix is_directory_plugin() call

* Update test

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
Co-authored-by: Sergey Mitroshin <sergeymitr@gmail.com>
Co-authored-by: Brandon Kraft <public@brandonkraft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Jetpack DNA [Package] Autoloader [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants