From 5872edc052e74f8eb95d2b68be814b5fe7b4fece Mon Sep 17 00:00:00 2001 From: Andrew Ozz Date: Fri, 3 Feb 2023 01:48:36 +0000 Subject: [PATCH] Filesystem API: Add directory support to `WP_Filesystem_Direct::move()`. Introduces: - New function: `wp_opcache_invalidate_directory()`, to recursively call `wp_opcache_invalidate()` after overwriting .php files. - New function: `move_dir()`, similar to `copy_dir()` that uses `WP_Filesystem::move()` followed by `wp_opcache_invalidate_directory()`, and has a fallback to `copy_dir()`. Props: costdev, afragen, peterwilsoncc, sergeybiryukov, ironprogrammer, flixos90, bronsonquick, mukesh27, azaozz. Fixes #57375. git-svn-id: https://develop.svn.wordpress.org/trunk@55204 602fd350-edb4-49c9-b593-d223f7449a82 --- .../includes/class-wp-filesystem-direct.php | 17 +- src/wp-admin/includes/file.php | 131 ++++++++ tests/phpunit/tests/filesystem/moveDir.php | 279 ++++++++++++++++++ .../wpOpcacheInvalidateDirectory.php | 101 +++++++ 4 files changed, 526 insertions(+), 2 deletions(-) create mode 100644 tests/phpunit/tests/filesystem/moveDir.php create mode 100644 tests/phpunit/tests/filesystem/wpOpcacheInvalidateDirectory.php diff --git a/src/wp-admin/includes/class-wp-filesystem-direct.php b/src/wp-admin/includes/class-wp-filesystem-direct.php index e84dfed39f..e8be5506d2 100644 --- a/src/wp-admin/includes/class-wp-filesystem-direct.php +++ b/src/wp-admin/includes/class-wp-filesystem-direct.php @@ -316,7 +316,14 @@ class WP_Filesystem_Direct extends WP_Filesystem_Base { } /** - * Moves a file. + * Moves a file or directory. + * + * After moving files or directories, OPcache will need to be invalidated. + * + * If moving a directory fails, `copy_dir()` can be used for a recursive copy. + * + * Use `move_dir()` for moving directories with OPcache invalidation and a + * fallback to `copy_dir()`. * * @since 2.5.0 * @@ -331,12 +338,18 @@ class WP_Filesystem_Direct extends WP_Filesystem_Base { return false; } + if ( $overwrite && $this->exists( $destination ) && ! $this->delete( $destination, true ) ) { + // Can't overwrite if the destination couldn't be deleted. + return false; + } + // Try using rename first. if that fails (for example, source is read only) try copy. if ( @rename( $source, $destination ) ) { return true; } - if ( $this->copy( $source, $destination, $overwrite ) && $this->exists( $destination ) ) { + // Backward compatibility: Only fall back to `::copy()` for single files. + if ( $this->is_file( $source ) && $this->copy( $source, $destination, $overwrite ) && $this->exists( $destination ) ) { $this->delete( $source ); return true; diff --git a/src/wp-admin/includes/file.php b/src/wp-admin/includes/file.php index f609277826..62e9c0cb91 100644 --- a/src/wp-admin/includes/file.php +++ b/src/wp-admin/includes/file.php @@ -1946,6 +1946,75 @@ function copy_dir( $from, $to, $skip_list = array() ) { return true; } +/** + * Moves a directory from one location to another. + * + * Recursively invalidates OPcache on success. + * + * If the renaming failed, falls back to copy_dir(). + * + * Assumes that WP_Filesystem() has already been called and setup. + * + * @since 6.2.0 + * + * @global WP_Filesystem_Base $wp_filesystem WordPress filesystem subclass. + * + * @param string $from Source directory. + * @param string $to Destination directory. + * @param bool $overwrite Optional. Whether to overwrite the destination directory if it exists. + * Default false. + * @return true|WP_Error True on success, WP_Error on failure. + */ +function move_dir( $from, $to, $overwrite = false ) { + global $wp_filesystem; + + if ( trailingslashit( strtolower( $from ) ) === trailingslashit( strtolower( $to ) ) ) { + return new WP_Error( + 'source_destination_same_move_dir', + __( 'The source and destination are the same.' ) + ); + } + + if ( ! $overwrite && $wp_filesystem->exists( $to ) ) { + return new WP_Error( 'destination_already_exists_move_dir', __( 'The destination folder already exists.' ), $to ); + } + + if ( $wp_filesystem->move( $from, $to, $overwrite ) ) { + /* + * When using an environment with shared folders, + * there is a delay in updating the filesystem's cache. + * + * This is a known issue in environments with a VirtualBox provider. + * + * A 200ms delay gives time for the filesystem to update its cache, + * prevents "Operation not permitted", and "No such file or directory" warnings. + * + * This delay is used in other projects, including Composer. + * @link https://github.com/composer/composer/blob/2.5.1/src/Composer/Util/Platform.php#L228-L233 + */ + usleep( 200000 ); + wp_opcache_invalidate_directory( $to ); + + return true; + } + + // Fall back to a recursive copy. + if ( ! $wp_filesystem->is_dir( $to ) ) { + if ( ! $wp_filesystem->mkdir( $to, FS_CHMOD_DIR ) ) { + return new WP_Error( 'mkdir_failed_move_dir', __( 'Could not create directory.' ), $to ); + } + } + + $result = copy_dir( $from, $to, array( basename( $to ) ) ); + + // Clear the source directory. + if ( true === $result ) { + $wp_filesystem->delete( $from, true ); + } + + return $result; +} + /** * Initializes and connects the WordPress Filesystem Abstraction classes. * @@ -2557,3 +2626,65 @@ function wp_opcache_invalidate( $filepath, $force = false ) { return false; } + +/** + * Attempts to clear the opcode cache for a directory of files. + * + * @since 6.2.0 + * + * @see wp_opcache_invalidate() + * @link https://www.php.net/manual/en/function.opcache-invalidate.php + * + * @global WP_Filesystem_Base $wp_filesystem WordPress filesystem subclass. + * + * @param string $dir The path to the directory for which the opcode cache is to be cleared. + */ +function wp_opcache_invalidate_directory( $dir ) { + global $wp_filesystem; + + if ( ! is_string( $dir ) || '' === trim( $dir ) ) { + if ( WP_DEBUG ) { + $error_message = sprintf( + /* translators: %s: The function name. */ + __( '%s expects a non-empty string.' ), + 'wp_opcache_invalidate_directory()' + ); + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error + trigger_error( $error_message ); + } + return; + } + + $dirlist = $wp_filesystem->dirlist( $dir, false, true ); + + if ( empty( $dirlist ) ) { + return; + } + + /* + * Recursively invalidate opcache of files in a directory. + * + * WP_Filesystem_*::dirlist() returns an array of file and directory information. + * + * This does not include a path to the file or directory. + * To invalidate files within sub-directories, recursion is needed + * to prepend an absolute path containing the sub-directory's name. + * + * @param array $dirlist Array of file/directory information from WP_Filesystem_Base::dirlist(), + * with sub-directories represented as nested arrays. + * @param string $path Absolute path to the directory. + */ + $invalidate_directory = function( $dirlist, $path ) use ( &$invalidate_directory ) { + $path = trailingslashit( $path ); + + foreach ( $dirlist as $name => $details ) { + if ( 'f' === $details['type'] ) { + wp_opcache_invalidate( $path . $name, true ); + } elseif ( is_array( $details['files'] ) && ! empty( $details['files'] ) ) { + $invalidate_directory( $details['files'], $path . $name ); + } + } + }; + + $invalidate_directory( $dirlist, $dir ); +} diff --git a/tests/phpunit/tests/filesystem/moveDir.php b/tests/phpunit/tests/filesystem/moveDir.php new file mode 100644 index 0000000000..fb487c7338 --- /dev/null +++ b/tests/phpunit/tests/filesystem/moveDir.php @@ -0,0 +1,279 @@ +mkdir( self::$test_dir ); + + // Create the "from" directory structure. + $wp_filesystem->mkdir( self::$existing_from ); + $wp_filesystem->touch( self::$existing_from_file ); + $wp_filesystem->mkdir( self::$existing_from_subdir ); + $wp_filesystem->touch( self::$existing_from_subdir_file ); + + // Create the "to" directory structure. + $wp_filesystem->mkdir( self::$existing_to ); + $wp_filesystem->touch( self::$existing_to_file ); + } + + /** + * Removes the test directory structure after each test. + */ + public function tear_down() { + global $wp_filesystem; + + // Delete the root directory and its contents. + $wp_filesystem->delete( self::$test_dir, true ); + + parent::tear_down(); + } + + /** + * Tests that move_dir() returns a WP_Error object. + * + * @ticket 57375 + * + * @dataProvider data_should_return_wp_error + * + * @param string $from The source directory path. + * @param string $to The destination directory path. + * @param bool $overwrite Whether to overwrite the destination directory. + * @param string $expected The expected WP_Error code. + */ + public function test_should_return_wp_error( $from, $to, $overwrite, $expected ) { + global $wp_filesystem; + + $from = self::$test_dir . $from; + $to = self::$test_dir . $to; + $result = move_dir( $from, $to, $overwrite ); + + $this->assertWPError( + $result, + 'move_dir() did not return a WP_Error object.' + ); + + $this->assertSame( + $expected, + $result->get_error_code(), + 'The expected error code was not returned.' + ); + + if ( 'source_destination_same_move_dir' !== $expected ) { + $this->assertTrue( + $wp_filesystem->exists( $from ), + 'The $from directory does not exist anymore.' + ); + + if ( false === $overwrite && 'existing_to' === untrailingslashit( $to ) ) { + $this->assertTrue( + $wp_filesystem->exists( $to ), + 'The $to directory does not exist anymore.' + ); + } + } + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_return_wp_error() { + return array( + '$overwrite is false and $to exists' => array( + 'from' => 'existing_from', + 'to' => 'existing_to', + 'overwrite' => false, + 'expected' => 'destination_already_exists_move_dir', + ), + 'same source and destination, source has trailing slash' => array( + 'from' => 'existing_from/', + 'to' => 'existing_from', + 'overwrite' => false, + 'expected' => 'source_destination_same_move_dir', + ), + 'same source and destination, destination has trailing slash' => array( + 'from' => 'existing_from', + 'to' => 'existing_from/', + 'overwrite' => false, + 'expected' => 'source_destination_same_move_dir', + ), + 'same source and destination, source lowercase, destination uppercase' => array( + 'from' => 'existing_from', + 'to' => 'EXISTING_FROM', + 'overwrite' => false, + 'expected' => 'source_destination_same_move_dir', + ), + 'same source and destination, source uppercase, destination lowercase' => array( + 'from' => 'EXISTING_FROM', + 'to' => 'existing_from', + 'overwrite' => false, + 'expected' => 'source_destination_same_move_dir', + ), + 'same source and destination, source and destination in inverted case' => array( + 'from' => 'ExIsTiNg_FrOm', + 'to' => 'eXiStInG_fRoM', + 'overwrite' => false, + 'expected' => 'source_destination_same_move_dir', + ), + ); + } + + /** + * Tests that move_dir() successfully moves a directory. + * + * @ticket 57375 + * + * @dataProvider data_should_move_directory + * + * @param string $from The source directory path. + * @param string $to The destination directory path. + * @param bool $overwrite Whether to overwrite the destination directory. + */ + public function test_should_move_directory( $from, $to, $overwrite ) { + global $wp_filesystem; + + $from = self::$test_dir . $from; + $to = self::$test_dir . $to; + $result = move_dir( $from, $to, $overwrite ); + + $this->assertTrue( + $result, + 'The directory was not moved.' + ); + + $this->assertFalse( + $wp_filesystem->exists( $from ), + 'The source directory still exists.' + ); + + $this->assertTrue( + $wp_filesystem->exists( $to ), + 'The destination directory does not exist.' + ); + + $dirlist = $wp_filesystem->dirlist( $to, true, true ); + + // Prevent PHP array sorting bugs from breaking tests. + $to_contents = array_keys( $dirlist ); + + $this->assertSameSets( + array( + 'existing_from_file.txt', + 'existing_from_subdir', + ), + $to_contents, + 'The expected files were not moved.' + ); + + $this->assertSame( + array( 'existing_from_subdir_file.txt' ), + array_keys( $dirlist['existing_from_subdir']['files'] ), + 'Sub-directory files failed to move.' + ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_move_directory() { + return array( + '$overwrite is false and $to does not exist' => array( + 'from' => 'existing_from', + 'to' => 'non_existing_to', + 'overwrite' => false, + ), + '$overwrite is true and $to exists' => array( + 'from' => 'existing_from', + 'to' => 'existing_to', + 'overwrite' => true, + ), + ); + } + +} diff --git a/tests/phpunit/tests/filesystem/wpOpcacheInvalidateDirectory.php b/tests/phpunit/tests/filesystem/wpOpcacheInvalidateDirectory.php new file mode 100644 index 0000000000..54c71c7ab0 --- /dev/null +++ b/tests/phpunit/tests/filesystem/wpOpcacheInvalidateDirectory.php @@ -0,0 +1,101 @@ +expectError(); + $this->expectErrorMessage( + 'wp_opcache_invalidate_directory()', + 'The expected error was not triggered.' + ); + + wp_opcache_invalidate_directory( $dir ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_trigger_error_with_invalid_dir() { + return array( + 'an empty string' => array( '' ), + 'a string with spaces' => array( ' ' ), + 'a string with tabs' => array( "\t" ), + 'a string with new lines' => array( "\n" ), + 'a string with carriage returns' => array( "\r" ), + 'int -1' => array( -1 ), + 'int 0' => array( 0 ), + 'int 1' => array( 1 ), + 'float -1.0' => array( -1.0 ), + 'float 0.0' => array( 0.0 ), + 'float 1.0' => array( 1.0 ), + 'false' => array( false ), + 'true' => array( true ), + 'null' => array( null ), + 'an empty array' => array( array() ), + 'a non-empty array' => array( array( 'directory_path' ) ), + 'an empty object' => array( new stdClass() ), + 'a non-empty object' => array( (object) array( 'directory_path' ) ), + 'INF' => array( INF ), + 'NAN' => array( NAN ), + ); + } + + /** + * Tests that wp_opcache_invalidate_directory() does not trigger an error + * with a valid directory. + * + * @ticket 57375 + * + * @dataProvider data_should_not_trigger_error_wp_opcache_valid_directory + * + * @param string $dir A directory path. + */ + public function test_should_not_trigger_error_wp_opcache_valid_directory( $dir ) { + $this->assertNull( wp_opcache_invalidate_directory( $dir ) ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_not_trigger_error_wp_opcache_valid_directory() { + return array( + 'an existing directory' => array( DIR_TESTDATA ), + 'a non-existent directory' => array( 'non_existent_directory' ), + ); + } +}