Skip to content

Commit 76ce4cb

Browse files
tijmenbruggemantijmenbruggeman
committed
chore: simplify rotation
1 parent 9adff72 commit 76ce4cb

File tree

3 files changed

+119
-73
lines changed

3 files changed

+119
-73
lines changed

‎src/class-tiny-logger.php‎

Lines changed: 19 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ class Tiny_Logger {
3232
const LOG_LEVEL_DEBUG = 'debug';
3333

3434
const MAX_LOG_SIZE = 5242880; // 5MB
35-
const MAX_LOG_FILES = 3;
3635

3736
private static $instance = null;
3837

@@ -43,8 +42,6 @@ class Tiny_Logger {
4342
* To log on various places easily, we create a singleton
4443
* to prevent passing around the instance.
4544
*
46-
* @since 3.7.0
47-
*
4845
* @return Tiny_Logger The logger instance.
4946
*/
5047
public static function get_instance() {
@@ -60,6 +57,7 @@ public static function get_instance() {
6057
*/
6158
private function __construct() {
6259
$this->log_file_path = $this->resolve_log_file_path();
60+
$this->log_enabled = 'on' === get_option( 'tinypng_logging_enabled', false );
6361
}
6462

6563
/**
@@ -90,10 +88,6 @@ public static function reset() {
9088
* @return bool True if logging is enabled, false otherwise.
9189
*/
9290
public function get_log_enabled() {
93-
if ( null === $this->log_enabled) {
94-
$this->log_enabled = 'on' === get_option( 'tinypng_logging_enabled', false );
95-
}
96-
9791
return $this->log_enabled;
9892
}
9993

@@ -112,12 +106,12 @@ public function get_log_file_path()
112106
* - set the setting on the instance
113107
* - if turn off, clear the logs
114108
* - if turned on, check if we can create the log file
115-
*
116-
* @since 3.7.0
117109
*/
118110
public static function on_save_log_enabled( $log_enabled, $old, $option ) {
111+
$instance = self::get_instance();
112+
$instance->log_enabled = 'on' === $log_enabled;
113+
119114
if ( 'on' !== $log_enabled ) {
120-
$instance = self::get_instance();
121115
$instance->clear_logs();
122116
}
123117

@@ -129,8 +123,6 @@ public static function on_save_log_enabled( $log_enabled, $old, $option ) {
129123
* should only be used internally. Use the getter to get the
130124
* memoized function.
131125
*
132-
* @since 3.7.0
133-
*
134126
* @return string The log file path.
135127
*/
136128
private function resolve_log_file_path() {
@@ -139,22 +131,9 @@ private function resolve_log_file_path() {
139131
return trailingslashit($log_dir) . 'tiny-compress.log';
140132
}
141133

142-
/**
143-
* Checks if logging is enabled.
144-
*
145-
* @since 3.7.0
146-
*
147-
* @return bool True if logging is enabled.
148-
*/
149-
public function is_enabled() {
150-
return $this->log_enabled;
151-
}
152-
153134
/**
154135
* Logs an error message.
155136
*
156-
* @since 3.7.0
157-
*
158137
* @param string $message The message to log.
159138
* @param array $context Optional. Additional context data. Default empty array.
160139
*/
@@ -166,8 +145,6 @@ public static function error( $message, $context = array() ) {
166145
/**
167146
* Logs a debug message.
168147
*
169-
* @since 3.7.0
170-
*
171148
* @param string $message The message to log.
172149
* @param array $context Optional. Additional context data. Default empty array.
173150
*/
@@ -179,11 +156,10 @@ public static function debug( $message, $context = array() ) {
179156
/**
180157
* Logs a message.
181158
*
182-
* @since 3.7.0
183-
*
184159
* @param string $level The log level.
185160
* @param string $message The message to log.
186161
* @param array $context Optional. Additional context data. Default empty array.
162+
* @return void
187163
*/
188164
private function log( $level, $message, $context = array() ) {
189165
if ( ! $this->log_enabled ) {
@@ -192,10 +168,16 @@ private function log( $level, $message, $context = array() ) {
192168

193169
$this->rotate_logs();
194170

171+
// Ensure log directory exists.
172+
$log_dir = dirname( $this->log_file_path );
173+
if ( ! file_exists( $log_dir ) ) {
174+
wp_mkdir_p( $log_dir );
175+
}
176+
195177
$timestamp = current_time( 'Y-m-d H:i:s' );
196178
$level_str = strtoupper( $level );
197179
$context_str = ! empty( $context ) ? ' ' . wp_json_encode( $context ) : '';
198-
$log_entry = "[{$timestamp}] [{$level_str}] {$message}{$context_str}\n";
180+
$log_entry = "[{$timestamp}] [{$level_str}] {$message}{$context_str}\\n";
199181

200182
$file = fopen( $this->log_file_path, 'a' );
201183
if ( $file ) {
@@ -205,9 +187,10 @@ private function log( $level, $message, $context = array() ) {
205187
}
206188

207189
/**
208-
* Rotates log files when they exceed the max size.
190+
* Deletes log file and creates a new one when the
191+
* MAX_LOG_SIZE is met.
209192
*
210-
* @since 3.7.0
193+
* @return void
211194
*/
212195
private function rotate_logs() {
213196
if ( ! file_exists( $this->log_file_path ) ) {
@@ -219,53 +202,25 @@ private function rotate_logs() {
219202
return;
220203
}
221204

222-
for ( $i = self::MAX_LOG_FILES - 1; $i > 0; $i-- ) {
223-
$old_file = $this->log_file_path . '.' . $i;
224-
$new_file = $this->log_file_path . '.' . ($i + 1);
225-
226-
if ( file_exists( $old_file ) ) {
227-
if ( self::MAX_LOG_FILES - 1 === $i ) {
228-
unlink( $old_file );
229-
} else {
230-
rename( $old_file, $new_file );
231-
}
232-
}
233-
}
234-
235-
rename( $this->log_file_path, $this->log_file_path . '.1' );
205+
unlink( $this->log_file_path );
236206
}
237207

238208
/**
239-
* Clears all log files.
240-
*
241-
* @since 3.7.0
209+
* Clears log file
242210
*
243211
* @return bool True if logs were cleared successfully.
244212
*/
245213
public function clear_logs() {
246-
$cleared = true;
247-
248-
// Remove main log file.
249214
if ( file_exists( $this->log_file_path ) ) {
250-
$cleared = unlink( $this->log_file_path ) && $cleared;
251-
}
252-
253-
// Remove rotated log files.
254-
for ( $i = 1; $i <= self::MAX_LOG_FILES; $i++ ) {
255-
$log_file = $this->log_file_path . '.' . $i;
256-
if ( file_exists( $log_file ) ) {
257-
$cleared = unlink( $log_file ) && $cleared;
258-
}
215+
return unlink( $this->log_file_path );
259216
}
260217

261-
return $cleared;
218+
return true;
262219
}
263220

264221
/**
265222
* Gets all log file paths.
266223
*
267-
* @since 3.7.0
268-
*
269224
* @return array Array of log file paths.
270225
*/
271226
public function get_log_files() {
@@ -275,13 +230,6 @@ public function get_log_files() {
275230
$files[] = $this->log_file_path;
276231
}
277232

278-
for ( $i = 1; $i <= self::MAX_LOG_FILES; $i++ ) {
279-
$log_file = $this->log_file_path . '.' . $i;
280-
if ( file_exists( $log_file ) ) {
281-
$files[] = $log_file;
282-
}
283-
}
284-
285233
return $files;
286234
}
287235
}

‎test/helpers/wordpress.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public function __construct($vfs)
8888
$this->addMethod('is_customize_preview');
8989
$this->addMethod('is_plugin_active');
9090
$this->addMethod('trailingslashit');
91+
$this->addMethod('current_time');
92+
$this->addMethod('wp_mkdir_p');
9193
$this->defaults();
9294
$this->create_filesystem();
9395
}
@@ -356,6 +358,25 @@ public function trailingslashit($value)
356358
{
357359
return $value . '/';
358360
}
361+
362+
/**
363+
* Mocked function for https://developer.wordpress.org/reference/functions/current_time/
364+
*
365+
* @return int|string
366+
*/
367+
public function current_time() {
368+
$dt = new DateTime( 'now' );
369+
return $dt->format('Y-m-d H:i:s');
370+
}
371+
372+
/**
373+
* Mocked function for https://developer.wordpress.org/reference/functions/wp_mkdir_p/
374+
*
375+
* @return bool
376+
*/
377+
public function wp_mkdir_p( $dir ) {
378+
mkdir( $dir, 0755, true );
379+
}
359380
}
360381

361382
class WP_HTTP_Proxy

‎test/unit/TinyLoggerTest.php‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
<?php
22

33
use function PHPUnit\Framework\assertEquals;
4+
use function PHPUnit\Framework\assertFalse;
45
use function PHPUnit\Framework\assertTrue;
6+
use org\bovigo\vfs\vfsStream;
7+
use org\bovigo\vfs\content\LargeFileContent;
58

69
require_once dirname(__FILE__) . '/TinyTestCase.php';
710

@@ -16,6 +19,7 @@ public function tear_down()
1619
{
1720
// ensure we clear the logger on each test
1821
$logger = Tiny_Logger::get_instance();
22+
$logger->clear_logs();
1923
$logger->reset();
2024
}
2125

@@ -26,14 +30,87 @@ public function test_logger_always_has_one_instance()
2630
assertEquals($instance1, $instance2, 'logger should be a singleton');
2731
}
2832

29-
public function test_get_log_enabled_memoizes_log_enabled() {
33+
public function test_get_log_enabled_memoizes_log_enabled()
34+
{
3035
$this->wp->addOption('tinypng_logging_enabled', 'on');
3136
$logger = Tiny_Logger::get_instance();
3237
assertTrue($logger->get_log_enabled(), 'log should be enabled when tinypng_logging_enabled is on');
3338
}
3439

35-
public function test_sets_log_path_on_construct() {
40+
public function test_sets_log_path_on_construct()
41+
{
3642
$logger = Tiny_Logger::get_instance();
3743
assertEquals($logger->get_log_file_path(), 'vfs://root/wp-content/uploads/tiny-compress-logs/tiny-compress.log');
3844
}
45+
46+
public function test_registers_save_update_when_log_enabled()
47+
{
48+
$logger = Tiny_Logger::get_instance();
49+
$logger->init();
50+
WordPressStubs::assertHook('pre_update_option_tinypng_logging_enabled', 'Tiny_Logger::on_save_log_enabled');
51+
}
52+
53+
public function test_option_hook_updates_log_enabled()
54+
{
55+
$this->wp->addOption('tinypng_logging_enabled', false);
56+
Tiny_Logger::init();
57+
$logger = Tiny_Logger::get_instance();
58+
59+
assertFalse($logger->get_log_enabled(), 'option is not set so should be false');
60+
61+
apply_filters('pre_update_option_tinypng_logging_enabled', 'on', null, '');
62+
63+
assertTrue($logger->get_log_enabled(), 'when option is updated, should be true');
64+
}
65+
66+
public function test_will_not_log_if_disabled()
67+
{
68+
$this->wp->addOption('tinypng_logging_enabled', false);
69+
$logger = Tiny_Logger::get_instance();
70+
71+
Tiny_Logger::error('This should not be logged');
72+
Tiny_Logger::debug('This should also not be logged');
73+
74+
$log_path = $logger->get_log_file_path();
75+
$log_exists = file_exists($log_path);
76+
assertFalse($log_exists, 'log file should not exist when logging is disabled');
77+
}
78+
79+
public function test_creates_log_when_log_is_enabled()
80+
{
81+
$this->wp->addOption('tinypng_logging_enabled', 'on');
82+
83+
$logger = Tiny_Logger::get_instance();
84+
$log_path = $logger->get_log_file_path();
85+
$log_exists = file_exists($log_path);
86+
assertFalse($log_exists, 'log file should not exist initially');
87+
88+
Tiny_Logger::error('This should be logged');
89+
Tiny_Logger::debug('This should also be logged');
90+
91+
$log_path = $logger->get_log_file_path();
92+
$log_exists = file_exists($log_path);
93+
assertTrue($log_exists, 'log file is created when logging is enabled');
94+
}
95+
96+
public function test_removes_full_log_and_creates_new()
97+
{
98+
$this->wp->addOption('tinypng_logging_enabled', 'on');
99+
100+
$log_dir_path = 'wp-content/uploads/tiny-compress-logs';
101+
vfsStream::newDirectory($log_dir_path)->at($this->vfs);
102+
$log_dir = $this->vfs->getChild($log_dir_path);
103+
104+
vfsStream::newFile('tiny-compress.log')
105+
->withContent(LargeFileContent::withMegabytes(5.1))
106+
->at($log_dir);
107+
108+
$logger = Tiny_Logger::get_instance();
109+
110+
assertTrue(filesize($logger->get_log_file_path()) > 5242880, 'log file should be larger than 5MB');
111+
112+
Tiny_Logger::error('This should be logged');
113+
114+
assertTrue(filesize($logger->get_log_file_path()) < 1048576, 'log file rotated and less than 1MB');
115+
}
39116
}

0 commit comments

Comments
 (0)