Commit ce11715b90f for woocommerce
commit ce11715b90ff2672dbe7f21beb635e76b93aa7cf
Author: Brandon Kraft <public@brandonkraft.com>
Date: Tue Jul 21 15:37:37 2026 -0600
Fix clearing a log source leaving files behind past the first 20 (#66170)
* fix(logging): clear all of a source's log files, not just the first 20
clear() called get_files() with only the source argument, so it inherited
the default per-page cap of 20 and silently left any files beyond the first
20 behind for a source with more log files than that. Fetch and delete in
batches, mirroring the fix applied to the sibling delete_logs_before_timestamp().
* test(logging): cover clear() batch boundary and undeletable-file paths
Address code-review feedback:
- Reword the DELETE_BATCH_SIZE docblock: it no longer applies only to
expired files now that clear() reuses the constant for source deletion.
- Add a test for clearing a source with exactly one full batch (100 files),
covering the batch-boundary termination path.
- Add a test that clears a >100-file source in which files scattered across
the first full batch, the batch boundary, and the trailing partial batch
are undeletable, asserting every deletable file is still removed. This
exercises the $skipped offset-advance guard and confirms the loop never
strands a deletable file behind an undeletable one.
* fix(logging): page clear() by created for stable ordering on PHP 7.4
clear() advances its batch offset past files that could not be deleted, so
the offset is only correct if get_files() returns a strict, stable total
order across iterations. The default 'modified' sort can tie for same-source
files that share a modified timestamp, and PHP < 8.0's usort() is not stable,
so tied files could re-order between re-globs and let a deletable file slip
past the advancing offset and be stranded when an undeletable file is present
in a full batch.
Page by 'created' instead: for a single source, created timestamps plus
rotation are unique per file, giving a strict total order independent of sort
stability. Update the undeletable-file test to give every file the same
modified time (the tie condition) to document that clear() stays deterministic.
* fix(logging): make log file sort a strict total order for stable paging
The batched deletion loops in clear() and delete_logs_before_timestamp()
page past undeletable/vetoed files by advancing a numeric offset, which is
only correct if get_files() returns a strict, stable total order across
re-globs. Two defects broke that guarantee for same-source files:
- The rotation tie-breaker in every get_files() sort comparator was written
`get_rotation() || -1`, which evaluates to boolean true for every rotation
value (null, 0, 1, ...) and so discriminated nothing. Corrected to
`get_rotation() ?? -1` so base files (-1) and rotations (0, 1, ...) order
deterministically. This alone was a latent ordering bug in the logs UI.
- delete_logs_before_timestamp() paged in the default 'modified' order, which
can tie for same-source files. It now orders by 'created' like clear().
With the tie-breaker fixed, (created, source, rotation) is unique per file,
so on PHP < 8.0 (where usort() is unstable) tied files can no longer re-order
between iterations and strand a file behind an undeletable/vetoed one. Adds a
FileController test asserting same-source rotations sort by rotation number.
* fix(logging): drop obsolete PHPStan baseline entry for the rotation sort
Correcting get_rotation() || -1 to ?? -1 resolves the 8 baselined
"Right side of || is always true" errors in FileController, so remove the
now-unmatched baseline entry (CI fails on stale baseline entries).
* test(logging): make rotation-ordering test deterministic
The rotation tie-break test built files from a plain path, so
File::get_created_timestamp() fell back to the live filectime(). If the
setup straddled a one-second boundary the created values could diverge and
get_files() would sort by created instead of exercising the rotation
tie-break, making the test intermittently flaky. Build the files with
standard filenames that embed a fixed created date so the created values are
parsed (and tied) deterministically.
* fix(logging): only clear the exact source, not prefix-matched siblings
get_files() filters by source with a glob prefix, so clear('foo') also matched
files from sibling sources like 'foo-two' -- and the batched deletion made it
delete all of them. Add an opt-in 'exact_source' arg to get_files() and use it
from clear(); the default stays a prefix match because OrderLogsCleanupHelper
deliberately sweeps a family of 'place-order-debug-*' sub-sources.
Also guard clear() against an empty (or empty-after-sanitize) source, which
would otherwise match every file and, with batching, wipe out all logs.
Adds tests for exact vs prefix matching, rotated files of the exact source,
and the empty-source guard, plus docblock coverage of the new argument.
Addresses review feedback from Konamiman on the prefix-match blast radius.
* chore(logging): consolidate changelog entries into one
diff --git a/plugins/woocommerce/changelog/fix-clear-logs-20-file-cap b/plugins/woocommerce/changelog/fix-clear-logs-20-file-cap
new file mode 100644
index 00000000000..7d403c1952b
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-clear-logs-20-file-cap
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Fix clearing a log source so it reliably deletes all of that source's log files, and only that source's files (previously it stopped after 20 files and could remove files from other sources sharing the same name prefix).
diff --git a/plugins/woocommerce/phpstan-baseline.neon b/plugins/woocommerce/phpstan-baseline.neon
index 4ef0b29d9bc..36b15f4e32d 100644
--- a/plugins/woocommerce/phpstan-baseline.neon
+++ b/plugins/woocommerce/phpstan-baseline.neon
@@ -56439,12 +56439,6 @@ parameters:
count: 1
path: src/Internal/Admin/Logging/FileV2/FileController.php
- -
- message: '#^Right side of \|\| is always true\.$#'
- identifier: booleanOr.rightAlwaysTrue
- count: 8
- path: src/Internal/Admin/Logging/FileV2/FileController.php
-
-
message: '#^Variable \$sort_callback might not be defined\.$#'
identifier: variable.undefined
diff --git a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileController.php b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileController.php
index 01531f71f4a..30233a241eb 100644
--- a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileController.php
+++ b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileController.php
@@ -28,14 +28,15 @@ class FileController {
* @const array
*/
public const DEFAULTS_GET_FILES = array(
- 'date_end' => 0,
- 'date_filter' => '',
- 'date_start' => 0,
- 'offset' => 0,
- 'order' => 'desc',
- 'orderby' => 'modified',
- 'per_page' => 20,
- 'source' => '',
+ 'date_end' => 0,
+ 'date_filter' => '',
+ 'date_start' => 0,
+ 'exact_source' => false,
+ 'offset' => 0,
+ 'order' => 'desc',
+ 'orderby' => 'modified',
+ 'per_page' => 20,
+ 'source' => '',
);
/**
@@ -187,14 +188,17 @@ class FileController {
* @param array $args {
* Optional. Arguments to filter and sort the files that are returned.
*
- * @type int $date_end The end of the date range to filter by, as a Unix timestamp.
- * @type string $date_filter Filter files by one of the date props. 'created' or 'modified'.
- * @type int $date_start The beginning of the date range to filter by, as a Unix timestamp.
- * @type int $offset Omit this number of files from the beginning of the list. Works with $per_page to do pagination.
- * @type string $order The sort direction. 'asc' or 'desc'. Defaults to 'desc'.
- * @type string $orderby The property to sort the list by. 'created', 'modified', 'source', 'size'. Defaults to 'modified'.
- * @type int $per_page The number of files to include in the list. Works with $offset to do pagination.
- * @type string $source Only include files from this source.
+ * @type int $date_end The end of the date range to filter by, as a Unix timestamp.
+ * @type string $date_filter Filter files by one of the date props. 'created' or 'modified'.
+ * @type int $date_start The beginning of the date range to filter by, as a Unix timestamp.
+ * @type bool $exact_source Match $source exactly instead of as a name prefix. Defaults to false.
+ * @type int $offset Omit this number of files from the beginning of the list. Works with $per_page to do pagination.
+ * @type string $order The sort direction. 'asc' or 'desc'. Defaults to 'desc'.
+ * @type string $orderby The property to sort the list by. 'created', 'modified', 'source', 'size'. Defaults to 'modified'.
+ * @type int $per_page The number of files to include in the list. Works with $offset to do pagination.
+ * @type string $source Only include files whose source begins with this value. Matching is a prefix by
+ * default, so 'foo' also matches the 'foo-two' source; set $exact_source to true for
+ * an exact match.
* }
* @param bool $count_only Optional. True to return a total count of the files.
*
@@ -215,6 +219,23 @@ class FileController {
$files = $this->convert_paths_to_objects( $paths );
+ /*
+ * The glob pattern is only a prefix match, so a source like 'foo' also
+ * matches files belonging to 'foo-two'. When 'exact_source' is requested,
+ * filter down to the exact source so the caller only acts on the source it
+ * asked for -- this is what clear() needs to avoid deleting a sibling
+ * source's files. It is opt-in because some callers (e.g. the order-logs
+ * cleanup) deliberately rely on the prefix match to sweep a family of
+ * sub-sources. Rotated files still parse to their base source, so those
+ * remain included either way.
+ */
+ if ( $args['exact_source'] && '' !== $args['source'] ) {
+ $files = array_filter(
+ $files,
+ fn( $file ) => $args['source'] === $file->get_source()
+ );
+ }
+
if ( $args['date_filter'] && $args['date_start'] && $args['date_end'] ) {
switch ( $args['date_filter'] ) {
case 'created':
@@ -265,7 +286,7 @@ class FileController {
$sort_sets = array(
array( $a->get_created_timestamp(), $b->get_created_timestamp() ),
array( $a->get_source(), $b->get_source() ),
- array( $a->get_rotation() || -1, $b->get_rotation() || -1 ),
+ array( $a->get_rotation() ?? -1, $b->get_rotation() ?? -1 ),
);
$order_sets = array( $args['order'], 'asc', 'asc' );
return $multi_sorter( $sort_sets, $order_sets );
@@ -276,7 +297,7 @@ class FileController {
$sort_sets = array(
array( $a->get_modified_timestamp(), $b->get_modified_timestamp() ),
array( $a->get_source(), $b->get_source() ),
- array( $a->get_rotation() || -1, $b->get_rotation() || -1 ),
+ array( $a->get_rotation() ?? -1, $b->get_rotation() ?? -1 ),
);
$order_sets = array( $args['order'], 'asc', 'asc' );
return $multi_sorter( $sort_sets, $order_sets );
@@ -287,7 +308,7 @@ class FileController {
$sort_sets = array(
array( $a->get_source(), $b->get_source() ),
array( $a->get_created_timestamp(), $b->get_created_timestamp() ),
- array( $a->get_rotation() || -1, $b->get_rotation() || -1 ),
+ array( $a->get_rotation() ?? -1, $b->get_rotation() ?? -1 ),
);
$order_sets = array( $args['order'], 'desc', 'asc' );
return $multi_sorter( $sort_sets, $order_sets );
@@ -298,7 +319,7 @@ class FileController {
$sort_sets = array(
array( $a->get_file_size(), $b->get_file_size() ),
array( $a->get_source(), $b->get_source() ),
- array( $a->get_rotation() || -1, $b->get_rotation() || -1 ),
+ array( $a->get_rotation() ?? -1, $b->get_rotation() ?? -1 ),
);
$order_sets = array( $args['order'], 'asc', 'asc' );
return $multi_sorter( $sort_sets, $order_sets );
diff --git a/plugins/woocommerce/src/Internal/Admin/Logging/LogHandlerFileV2.php b/plugins/woocommerce/src/Internal/Admin/Logging/LogHandlerFileV2.php
index aaccd177bba..868f9f34109 100644
--- a/plugins/woocommerce/src/Internal/Admin/Logging/LogHandlerFileV2.php
+++ b/plugins/woocommerce/src/Internal/Admin/Logging/LogHandlerFileV2.php
@@ -11,7 +11,7 @@ use WC_Log_Handler;
*/
class LogHandlerFileV2 extends WC_Log_Handler {
/**
- * Maximum number of expired log files to delete in one loop iteration.
+ * Maximum number of log files to delete in one loop iteration.
*
* @var int
*/
@@ -175,22 +175,59 @@ class LogHandlerFileV2 extends WC_Log_Handler {
public function clear( string $source, bool $quiet = false ): int {
$source = File::sanitize_source( $source );
- $files = $this->file_controller->get_files(
- array(
- 'source' => $source,
- )
- );
-
- if ( is_wp_error( $files ) || ! is_array( $files ) || count( $files ) < 1 ) {
+ // Bail on an empty source: an empty value would match every file and,
+ // combined with the batched deletion below, wipe out all log files.
+ if ( '' === $source ) {
return 0;
}
- $file_ids = array_map(
- fn( $file ) => $file->get_file_id(),
- $files
- );
+ $deleted = 0;
+ $skipped = 0;
+
+ /*
+ * Fetch and delete in batches so that sources with more than the default
+ * per-page of log files don't leave files behind.
+ *
+ * Order by 'created' rather than the default 'modified'. Because paging
+ * advances $skipped past undeletable files, the offset is only reliable if
+ * get_files() returns a stable, strict total order across iterations. For a
+ * single source, created timestamps (plus rotation) are unique per file,
+ * whereas modified times can tie -- and on PHP < 8.0 usort() is not stable,
+ * so tied files could re-order between iterations and strand a deletable file.
+ */
+ do {
+ $files = $this->file_controller->get_files(
+ array(
+ 'source' => $source,
+ 'exact_source' => true,
+ 'orderby' => 'created',
+ 'per_page' => self::DELETE_BATCH_SIZE,
+ 'offset' => $skipped,
+ )
+ );
- $deleted = $this->file_controller->delete_files( $file_ids );
+ if ( is_wp_error( $files ) || ! is_array( $files ) ) {
+ break;
+ }
+
+ $fetched_count = count( $files );
+ if ( $fetched_count < 1 ) {
+ break;
+ }
+
+ $file_ids = array_map(
+ fn( $file ) => $file->get_file_id(),
+ $files
+ );
+
+ $deleted_in_batch = $this->file_controller->delete_files( $file_ids );
+ $deleted += $deleted_in_batch;
+
+ // Deleted files disappear from the directory, so only files that could
+ // not be deleted need to be skipped. This avoids retrying a permanently
+ // undeletable batch forever.
+ $skipped += $fetched_count - $deleted_in_batch;
+ } while ( self::DELETE_BATCH_SIZE === $fetched_count );
if ( $deleted > 0 && ! $quiet ) {
$this->handle(
@@ -237,14 +274,23 @@ class LogHandlerFileV2 extends WC_Log_Handler {
$deleted = 0;
$skipped = 0;
- // Fetch and delete in batches so that sites with more than the default
- // per-page of log files don't leave expired files behind.
+ /*
+ * Fetch and delete in batches so that sites with more than the default
+ * per-page of log files don't leave expired files behind.
+ *
+ * Order by 'created' so that paging past vetoed files stays reliable: the
+ * offset only lands correctly if get_files() returns a strict, stable total
+ * order across iterations, and (created, source, rotation) is unique per file
+ * whereas the default 'modified' order can tie -- which PHP < 8.0's unstable
+ * usort() could re-order between iterations, stranding an expired file.
+ */
do {
$files = $this->file_controller->get_files(
array(
'date_filter' => 'created',
'date_start' => 1,
'date_end' => $timestamp,
+ 'orderby' => 'created',
'per_page' => self::DELETE_BATCH_SIZE,
'offset' => $skipped,
)
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/FileV2/FileControllerTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/FileV2/FileControllerTest.php
index f4954444612..838c4351aa0 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/FileV2/FileControllerTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/FileV2/FileControllerTest.php
@@ -231,6 +231,67 @@ class FileControllerTest extends WC_Unit_Test_Case {
$this->assertEquals( 'plugin-woocommerce', $first_file->get_source() );
}
+ /**
+ * @testdox The get_files method breaks ties between same-source rotations by rotation number.
+ */
+ public function test_get_files_orders_same_source_rotations_by_rotation(): void {
+ /*
+ * Create a current file plus two rotations for one source, all sharing a
+ * created day. Standard filenames embed the created date, so
+ * get_created_timestamp() is parsed from the filename rather than falling
+ * back to the live filectime(); this keeps the created values tied -- and
+ * therefore the rotation tie-break under test -- deterministic.
+ */
+ $created = strtotime( '-1 day' );
+ foreach ( array( null, 0, 1 ) as $rotation ) {
+ $file_id = File::generate_file_id( 'rotated-source', $rotation, $created );
+ $path = Settings::get_log_directory() . $file_id . '-' . File::generate_hash( $file_id ) . '.log';
+ $file = new File( $path );
+ $file->write( 'test' );
+ }
+
+ $files = $this->sut->get_files(
+ array(
+ 'source' => 'rotated-source',
+ 'orderby' => 'created',
+ 'order' => 'desc',
+ 'per_page' => 10,
+ )
+ );
+
+ $rotations = array_map(
+ fn( $retrieved ) => $retrieved->get_rotation(),
+ $files
+ );
+ $this->assertSame(
+ array( null, 0, 1 ),
+ $rotations,
+ 'Same-source rotations must sort by rotation ascending (current file first), not in arbitrary order.'
+ );
+ }
+
+ /**
+ * @testdox The get_files method prefix-matches source by default but matches exactly when exact_source is set.
+ */
+ public function test_get_files_exact_source_excludes_prefix_siblings(): void {
+ $this->handler->handle( time(), 'debug', 'a', array( 'source' => 'foo' ) );
+ $this->handler->handle( time(), 'debug', 'b', array( 'source' => 'foo-two' ) );
+
+ // Default matching is a prefix, so 'foo' also returns the 'foo-two' source.
+ $prefix = $this->sut->get_files( array( 'source' => 'foo' ) );
+ $this->assertCount( 2, $prefix );
+
+ // Exact matching returns only the 'foo' source.
+ $exact = $this->sut->get_files(
+ array(
+ 'source' => 'foo',
+ 'exact_source' => true,
+ )
+ );
+ $this->assertCount( 1, $exact );
+ $this->assertEquals( 'foo', array_shift( $exact )->get_source() );
+ }
+
/**
* @testdox The get_files method should return a count of files that meet the filtering criteria.
*/
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/LogHandlerFileV2Test.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/LogHandlerFileV2Test.php
index 80264e8d07b..b79da93a654 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/LogHandlerFileV2Test.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/LogHandlerFileV2Test.php
@@ -5,7 +5,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Admin\Logging;
use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Internal\Admin\Logging\{ LogHandlerFileV2, Settings };
-use Automattic\WooCommerce\Internal\Admin\Logging\FileV2\File;
+use Automattic\WooCommerce\Internal\Admin\Logging\FileV2\{ File, FileController };
use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use WC_Unit_Test_Case;
@@ -124,7 +124,7 @@ class LogHandlerFileV2Test extends WC_Unit_Test_Case {
*/
public function test_handle_message_formatting() {
$time = time();
- $message = <<<MESSAGE
+ $message = <<<'MESSAGE'
How to win
1. Bake cookies
2. ???
@@ -337,7 +337,8 @@ MESSAGE;
$this->assertEquals( 3, $result );
$paths = glob( Settings::get_log_directory() . '*.log' );
- $this->assertCount( 2, $paths ); // New log gets created when old logs are deleted!
+ $this->assertCount( 2, $paths );
+ // New log gets created when old logs are deleted!
$paths = glob( Settings::get_log_directory() . 'wc_logger*.log' );
$this->assertCount( 1, $paths );
@@ -348,6 +349,229 @@ MESSAGE;
$this->assertStringContainsString( $expected_string, $actual_content );
}
+ /**
+ * @testdox Check that clear deletes more than the default per-page of log files from a source in one run.
+ */
+ public function test_clear_deletes_more_than_default_per_page() {
+ // Create more files for a single source than a single delete batch can
+ // remove (batch size is 100), using a distinct date per file.
+ $expired_count = 101;
+ foreach ( range( 1, $expired_count ) as $days_ago ) {
+ $this->sut->handle( strtotime( "-{$days_ago} days" ), 'debug', 'quack.', array( 'source' => 'duck' ) );
+ }
+
+ // Add a couple of files from a different source that must be left alone.
+ $this->sut->handle( time(), 'debug', 'honk.', array( 'source' => 'goose' ) );
+ $this->sut->handle( strtotime( '-1 day' ), 'debug', 'honk.', array( 'source' => 'goose' ) );
+
+ $paths = glob( Settings::get_log_directory() . 'duck*.log' );
+ $this->assertCount( $expired_count, $paths );
+
+ $result = $this->sut->clear( 'duck' );
+ $this->assertEquals( $expired_count, $result );
+
+ $paths = glob( Settings::get_log_directory() . 'duck*.log' );
+ $this->assertCount( 0, $paths );
+
+ // The two goose files remain untouched.
+ $paths = glob( Settings::get_log_directory() . 'goose*.log' );
+ $this->assertCount( 2, $paths );
+
+ $paths = glob( Settings::get_log_directory() . 'wc_logger*.log' );
+ $this->assertCount( 1, $paths );
+
+ // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+ $actual_content = file_get_contents( reset( $paths ) );
+ $expected_string = '101 log files from source <code>duck</code> were deleted.';
+ $this->assertStringContainsString( $expected_string, $actual_content );
+ }
+
+ /**
+ * @testdox Check that clear terminates and deletes everything when a source has exactly the batch-size number of files.
+ */
+ public function test_clear_deletes_exactly_one_full_batch() {
+ // 100 files is exactly DELETE_BATCH_SIZE: the loop fetches a full page, then
+ // must make one more (empty) fetch and break rather than miscount or hang.
+ $total = 100;
+ foreach ( range( 1, $total ) as $days_ago ) {
+ $this->sut->handle( strtotime( "-{$days_ago} days" ), 'debug', 'quack.', array( 'source' => 'duck' ) );
+ }
+
+ $this->assertCount( $total, glob( Settings::get_log_directory() . 'duck*.log' ) );
+
+ $result = $this->sut->clear( 'duck' );
+
+ $this->assertEquals( $total, $result );
+ $this->assertCount( 0, glob( Settings::get_log_directory() . 'duck*.log' ) );
+ }
+
+ /**
+ * @testdox Check that clear removes every deletable file for a source even when a full batch contains undeletable files.
+ */
+ public function test_clear_removes_all_deletable_files_when_a_full_batch_contains_undeletable_files() {
+ $total = 150;
+ foreach ( range( 1, $total ) as $days_ago ) {
+ $this->sut->handle( strtotime( "-{$days_ago} days" ), 'debug', 'quack.', array( 'source' => 'duck' ) );
+ }
+
+ /*
+ * Give every file the same modified time. This is the tie condition that
+ * makes the default 'modified' sort non-deterministic on PHP < 8.0 (unstable
+ * usort). clear() pages by 'created' instead, which is unique per file for a
+ * single source, so the batch layout stays deterministic and offset paging
+ * never strands a deletable file behind an undeletable one.
+ */
+ $filesystem = FilesystemUtil::get_wp_filesystem();
+ $paths = glob( Settings::get_log_directory() . 'duck*.log' );
+ $this->assertCount( $total, $paths );
+ $shared_mtime = time();
+ foreach ( $paths as $path ) {
+ $filesystem->touch( $path, $shared_mtime );
+ }
+ // glob() sorts oldest-date first; reverse to created-descending, the order clear() pages in.
+ $newest_first = array_reverse( $paths );
+
+ /*
+ * Lock files at adversarial positions: inside the first full batch (0, 50, 99),
+ * on the batch boundary (100), and in the trailing partial batch (149). If clear()
+ * advanced its offset past a deletable file while skipping an undeletable one, one of
+ * the non-locked files would survive and the deleted count would fall short.
+ */
+ $locked_indexes = array( 0, 50, 99, 100, 149 );
+ $locked_ids = array();
+ foreach ( $locked_indexes as $index ) {
+ $locked_ids[] = ( new File( $newest_first[ $index ] ) )->get_file_id();
+ }
+
+ $controller = new class( $locked_ids ) extends FileController {
+ /**
+ * File IDs that delete_files() must refuse to delete, simulating files that
+ * cannot be removed from disk (e.g. a permission error).
+ *
+ * @var string[]
+ */
+ private $locked_ids;
+
+ /**
+ * Constructor.
+ *
+ * @param string[] $locked_ids File IDs that must not be deleted.
+ */
+ public function __construct( array $locked_ids ) {
+ // FileController declares no constructor, so there is intentionally no parent::__construct() call.
+ $this->locked_ids = $locked_ids;
+ }
+
+ /**
+ * Delete every requested file except the locked ones.
+ *
+ * @param string[] $file_ids The file IDs to delete.
+ *
+ * @return int The number of files that were deleted.
+ */
+ public function delete_files( array $file_ids ): int {
+ return parent::delete_files( array_values( array_diff( $file_ids, $this->locked_ids ) ) );
+ }
+ };
+
+ $property = new \ReflectionProperty( LogHandlerFileV2::class, 'file_controller' );
+ $property->setAccessible( true );
+ $property->setValue( $this->sut, $controller );
+
+ $result = $this->sut->clear( 'duck' );
+
+ $this->assertEquals( $total - count( $locked_indexes ), $result, 'Every deletable file for the source should be removed.' );
+
+ $remaining = glob( Settings::get_log_directory() . 'duck*.log' );
+ sort( $remaining );
+ $expected_remaining = array();
+ foreach ( $locked_indexes as $index ) {
+ $expected_remaining[] = $newest_first[ $index ];
+ }
+ sort( $expected_remaining );
+ $this->assertSame( $expected_remaining, $remaining, 'Only the undeletable files should remain on disk.' );
+ }
+
+ /**
+ * @testdox Check that clear only deletes files for the exact source, not sources that merely share its prefix.
+ */
+ public function test_clear_only_deletes_the_exact_source() {
+ $this->sut->handle( time(), 'debug', 'a', array( 'source' => 'foo' ) );
+ $this->sut->handle( time(), 'debug', 'b', array( 'source' => 'foo-two' ) );
+ $this->sut->handle( time(), 'debug', 'c', array( 'source' => 'foobar' ) );
+
+ $this->assertCount( 3, glob( Settings::get_log_directory() . '*.log' ) );
+
+ $result = $this->sut->clear( 'foo' );
+
+ $this->assertEquals( 1, $result, 'clear() should delete only the file belonging to the exact source.' );
+ $this->assertCount( 1, glob( Settings::get_log_directory() . 'foo-two-*.log' ), 'The foo-two source must be left untouched.' );
+ $this->assertCount( 1, glob( Settings::get_log_directory() . 'foobar-*.log' ), 'The foobar source must be left untouched.' );
+ }
+
+ /**
+ * @testdox Check that clear deletes rotated files of the exact source while leaving a prefix-sibling source alone.
+ */
+ public function test_clear_deletes_rotated_files_of_the_exact_source() {
+ $file_controller = wc_get_container()->get( FileController::class );
+
+ // Log to 'foo', rotate that file, then log again so there is a current file plus
+ // a rotation of it. Both parse to source 'foo'.
+ $this->sut->handle( time(), 'debug', 'first', array( 'source' => 'foo' ) );
+ $foo = $file_controller->get_files(
+ array(
+ 'source' => 'foo',
+ 'exact_source' => true,
+ )
+ );
+ $foo[0]->rotate();
+ $this->sut->handle( time(), 'debug', 'second', array( 'source' => 'foo' ) );
+
+ // A prefix sibling that must survive.
+ $this->sut->handle( time(), 'debug', 'sibling', array( 'source' => 'foo-two' ) );
+
+ $this->assertCount(
+ 2,
+ $file_controller->get_files(
+ array(
+ 'source' => 'foo',
+ 'exact_source' => true,
+ )
+ ),
+ 'Setup: expected the current file and one rotation for source foo.'
+ );
+
+ $result = $this->sut->clear( 'foo' );
+
+ $this->assertEquals( 2, $result, 'clear() should delete the current and rotated files of the exact source.' );
+ $this->assertCount(
+ 0,
+ $file_controller->get_files(
+ array(
+ 'source' => 'foo',
+ 'exact_source' => true,
+ )
+ ),
+ 'All foo files, including rotations, should be gone.'
+ );
+ $this->assertCount( 1, glob( Settings::get_log_directory() . 'foo-two-*.log' ), 'The foo-two source must be left untouched.' );
+ }
+
+ /**
+ * @testdox Check that clear does nothing when given a source that sanitizes to an empty string.
+ */
+ public function test_clear_does_nothing_for_an_empty_source() {
+ $this->sut->handle( time(), 'debug', 'a', array( 'source' => 'keep-me' ) );
+ $this->sut->handle( time(), 'debug', 'b', array( 'source' => 'keep-me-too' ) );
+
+ $this->assertCount( 2, glob( Settings::get_log_directory() . '*.log' ) );
+
+ $result = $this->sut->clear( '' );
+
+ $this->assertEquals( 0, $result, 'clear() must not delete anything for an empty source.' );
+ $this->assertCount( 2, glob( Settings::get_log_directory() . '*.log' ), 'No log files should be deleted for an empty source.' );
+ }
+
/**
* @testdox Check that the delete_logs_before_timestamp method deletes files based on their created date.
*/
@@ -369,7 +593,8 @@ MESSAGE;
$this->assertEquals( 4, $result );
$paths = glob( Settings::get_log_directory() . '*.log' );
- $this->assertCount( 3, $paths ); // New log gets created when old logs are deleted!
+ $this->assertCount( 3, $paths );
+ // New log gets created when old logs are deleted!
$paths = glob( Settings::get_log_directory() . 'wc_logger*.log' );
$this->assertCount( 1, $paths );