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: Skip single-file plugins #15232

Merged

Conversation

sergeymitr
Copy link
Contributor

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.

Changes proposed in this Pull Request:

  • Function to determine if the plugin is located in a dedicated directory.
  • Filter that runs that function over all active plugins.

Testing instructions:

  • Enable plugin "Hello Dolly".
  • Make sure the method Plugins_Handler::get_all_active_plugins() skips that plugin.

Proposed changelog entry for your changes:

  • n/a

@sergeymitr sergeymitr requested a review from a team as a April 1, 2020 15:55
@sergeymitr sergeymitr self-assigned this Apr 1, 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 1, 2020
@sergeymitr sergeymitr added [Pri] Normal [Focus] Jetpack DNA and removed [Status] Needs Package Release This PR made changes to a package. Let's update that package now. labels Apr 1, 2020
@jetpackbot
Copy link

jetpackbot commented Apr 1, 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: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against d42e86c

@sergeymitr sergeymitr added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label Apr 1, 2020
@sergeymitr sergeymitr requested review from zinigor and kbrown9 and removed request for a team April 1, 2020 16:13
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, seems to work fine both in singe and multisite environments, code looks great too, thanks!

@kraftbj kraftbj added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Apr 1, 2020
@kraftbj
Copy link
Contributor

kraftbj commented Apr 1, 2020

The E2E tests are failing. Can you check it out?

Copy link
Member

@kbrown9 kbrown9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I tested with a single site environment, and the single-file plugin was removed from the list as expected.

@kbrown9
Copy link
Member

kbrown9 commented Apr 1, 2020

I think that a PR that's recently been merged into master will fix the failing E2E test, so I rebased the feature/update_autoloader branch on master. It worked for a failing E2E test for PR #15135.

Sorry about the messed up commits! This PR will need to be rebased onto the updated feature/update_autoloader branch.

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.
@sergeymitr sergeymitr force-pushed the update/autoloader_skip_onefile_plugins branch from c7c9d64 to d42e86c Compare April 2, 2020 01:28
@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 2, 2020
@sergeymitr sergeymitr merged commit 2281a0d into feature/update_autoloader Apr 2, 2020
@sergeymitr sergeymitr deleted the update/autoloader_skip_onefile_plugins branch April 2, 2020 01:53
@sergeymitr sergeymitr removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Status] Needs Changelog labels Apr 2, 2020
kbrown9 pushed a commit that referenced this pull request Apr 9, 2020
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.
kbrown9 pushed a commit that referenced this pull request Apr 20, 2020
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.
kbrown9 pushed a commit that referenced this pull request May 14, 2020
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.
kbrown9 pushed a commit that referenced this pull request May 22, 2020
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.
kraftbj pushed a commit that referenced this pull request Jun 22, 2020
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.
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>
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants