refactor clearing HDD cache to remove empty directories (pluginkollek…

…tiv#289) We opt out early, if a directory was already empty before and delete emptied directories only in recursive mode. Remove the early return and the additional condition, so we do not leave behind empty directories.
timse201 · Mar 29, 2024 · 429f4e3 · 429f4e3
1 parent c06283b
commit 429f4e3
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. This projec
 Requires PHP 5.6 and WordPress 4.7 or above
 
 * Fix: invalidate cache when permalink changes (#285, #286, props raffaelj)
+* Fix: remove empty directories when pruning the HDD cache (#289)
 * Enhance: adjust styling for setup instructions (#215, props timse201)
 * Enhance: update hooks for Multisite initialization in WordPress 5.1 and above (#246, props ouun)
 * Enhance: rework flush hooks and add some third-party triggers for Autoptimize and WooCommerce (#225, props timse201)

diff --git a/inc/class-cachify-hdd.php b/inc/class-cachify-hdd.php
@@ -229,44 +229,45 @@ private static function _create_file( $file, $data ) {
 	 * @since 2.0
 	 */
 	private static function _clear_dir( $dir, $recursive = false ) {
-		/* Remote training slash */
+		// Remove trailing slash.
 		$dir = untrailingslashit( $dir );
 
-		/* Is directory? */
+		// Is directory?
 		if ( ! is_dir( $dir ) ) {
 			return;
 		}
 
-		/* Read */
+		// List directory contents.
 		$objects = array_diff(
 			scandir( $dir ),
 			array( '..', '.' )
 		);
 
-		/* Empty? */
-		if ( empty( $objects ) ) {
-			return;
-		}
-
-		/* Loop over items */
+		// Loop over items.
 		foreach ( $objects as $object ) {
-			/* Expand path */
+			// Expand path.
 			$object = $dir . DIRECTORY_SEPARATOR . $object;
 
-			/* Directory or file */
-			if ( is_dir( $object ) && $recursive ) {
-				self::_clear_dir( $object, $recursive );
+			if ( is_dir( $object ) ) {
+				if ( $recursive ) {
+					// Recursively clear the directory.
+					self::_clear_dir( $object, $recursive );
+				} elseif ( self::_user_can_delete( $object ) && 0 === count( glob( trailingslashit( $object ) . '*' ) ) ) {
+					// Delete the directory, if empty.
+					@rmdir( $object );
+				}
 			} elseif ( self::_user_can_delete( $object ) ) {
+				// Delete the file.
 				unlink( $object );
 			}
 		}
 
-		/* Remove directory */
-		if ( $recursive && self::_user_can_delete( $dir ) && 0 === count( glob( trailingslashit( $dir ) . '*' ) ) ) {
+		// Remove directory, if empty.
+		if ( self::_user_can_delete( $dir ) && 0 === count( glob( trailingslashit( $dir ) . '*' ) ) ) {
 			@rmdir( $dir );
 		}
 
-		/* Clean up */
+		// Clean up.
 		clearstatcache();
 	}
 

diff --git a/tests/test-cachify-hdd.php b/tests/test-cachify-hdd.php
@@ -73,6 +73,17 @@ public function test_caching() {
 		);
 		self::assertStringEndsWith( ' -->', $cached );
 
+		// A subpage
+		self::go_to( '/testme/sub' );
+		Cachify_HDD::store_item(
+			'965b4abf2414e45036ab90c9d3f8dbc7', // Ignored.
+			'<html><head><title>Test Me</title></head><body><p>This is a subpage.</p></body></html>',
+			3600, // Ignored.
+			false
+		);
+		self::assertTrue( Cachify_HDD::get_item() );
+		self::assertTrue( is_file( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme/sub/index.html' ) );
+
 		// Another item.
 		self::go_to( '/test2/' );
 		Cachify_HDD::store_item(
@@ -96,10 +107,17 @@ public function test_caching() {
 		Cachify_HDD::delete_item( '965b4abf2414e45036ab90c9d3f8dbc7', 'http://example.org/testme/' );
 		self::assertFalse( is_file( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme/index.html' ), 'first item was not deleted' );
 		self::assertTrue( is_file( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/test2/index.html' ), 'second item should still be present' );
+		self::assertTrue( is_file( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme/sub/index.html' ), 'subpage should now have been deleted' );
+
+		// Delete the subpage.
+		Cachify_HDD::delete_item( '965b4abf2414e45036ab90c9d3f8dbc7', 'http://example.org/testme/sub' );
+		self::assertFalse( is_dir( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme/sub/index.html' ), 'subpage item was not deleted' );
+		self::assertFalse( is_dir( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme/sub' ), 'empty directory was not deleted' );
 
 		// Clear the cache.
 		Cachify_HDD::clear_cache();
-		self::assertFalse( is_dir( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/test2' ) );
+		self::assertFalse( is_dir( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/testme' ), 'empty directory was not deleted' );
+		self::assertFalse( is_dir( CACHIFY_CACHE_DIR . DIRECTORY_SEPARATOR . 'example.org/test2' ), 'second test page was not deleted' );
 		self::assertFalse( Cachify_HDD::get_item() );
 	}
 }