From ffd0d380eea1a12dc9ffaa9970744fb97469d649 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Sun, 11 Dec 2016 12:02:40 +0100 Subject: [PATCH] Remove threshold logic from handlers. Email handler implements threshold logic internally. Other handlers handle all logs received. Handler constructors have changed. --- .../abstracts/abstract-wc-log-handler.php | 44 ------------ .../log-handlers/class-wc-log-handler-db.php | 4 -- .../class-wc-log-handler-email.php | 70 ++++++++++++------- .../class-wc-log-handler-file.php | 22 ++---- tests/unit-tests/log/log-handler-email.php | 6 +- tests/unit-tests/log/log-handler-file.php | 27 ++++--- 6 files changed, 72 insertions(+), 101 deletions(-) diff --git a/includes/abstracts/abstract-wc-log-handler.php b/includes/abstracts/abstract-wc-log-handler.php index b1e244b24d3..7df9cf4b871 100644 --- a/includes/abstracts/abstract-wc-log-handler.php +++ b/includes/abstracts/abstract-wc-log-handler.php @@ -13,30 +13,6 @@ if ( ! defined( 'ABSPATH' ) ) { */ abstract class WC_Log_Handler { - /** - * Minimum log level this handler will process. - * - * @var int 0-7 minimum level severity for handling log entry. - * @access private - */ - protected $threshold; - - /** - * Constructor for log handler. - * - * @param array $args { - * @type string $threshold Optional. Default 'emergency'. Sets the log severity threshold. - * emergency|alert|critical|error|warning|notice|info|debug - * } - */ - public function __construct( $args = array() ) { - - $args = wp_parse_args( $args, array( - 'threshold' => WC_Log_Levels::DEBUG, - ) ); - - $this->set_threshold( $args['threshold'] ); - } /** * Handle a log entry. @@ -50,26 +26,6 @@ abstract class WC_Log_Handler { */ abstract public function handle( $timestamp, $level, $message, $context ); - /** - * Set handler severity threshold. - * - * @param string $level emergency|alert|critical|error|warning|notice|info|debug - */ - public function set_threshold( $level ) { - $level = apply_filters( 'woocommerce_log_handler_set_threshold', $level, get_class( $this ) ); - $this->threshold = WC_Log_Levels::get_level_severity( $level ); - } - - /** - * Determine whether handler should handle log. - * - * @param string $level emergency|alert|critical|error|warning|notice|info|debug - * @return bool True if the log should be handled. - */ - public function should_handle( $level ) { - return $this->threshold <= WC_Log_Levels::get_level_severity( $level ); - } - /** * Formats a timestamp for use in log messages. * diff --git a/includes/log-handlers/class-wc-log-handler-db.php b/includes/log-handlers/class-wc-log-handler-db.php index 2aeb3652465..6d7207bce0e 100644 --- a/includes/log-handlers/class-wc-log-handler-db.php +++ b/includes/log-handlers/class-wc-log-handler-db.php @@ -34,10 +34,6 @@ class WC_Log_Handler_DB extends WC_Log_Handler { */ public function handle( $timestamp, $level, $message, $context ) { - if ( ! $this->should_handle( $level ) ) { - return true; - } - if ( isset( $context['tag'] ) && $context['tag'] ) { $tag = $context['tag']; } else { diff --git a/includes/log-handlers/class-wc-log-handler-email.php b/includes/log-handlers/class-wc-log-handler-email.php index 4fc54ae611a..25fd1e77916 100644 --- a/includes/log-handlers/class-wc-log-handler-email.php +++ b/includes/log-handlers/class-wc-log-handler-email.php @@ -18,40 +18,62 @@ if ( ! class_exists( 'WC_Log_Handler' ) ) { */ class WC_Log_Handler_Email extends WC_Log_Handler { + /** + * Minimum log level this handler will process. + * + * @var int Integer representation of minimum log level to handle. + * @access private + */ + protected $threshold; + /** * Stores email recipients. * * @var array * @access private */ - private $to = array(); + private $recipients = array(); /** - * Constructor for the logger. + * Constructor for log handler. * - * @param $args additional args. { - * Optional. @see WC_Log_Handler::__construct. - * - * @type string|array $to Optional. Email or emails to recieve log messages. Default site admin. - * } + * @param string|array $recipients Optional. Email(s) to receive log messages. Defaults to site admin email. + * @param string $threshold Optional. Minimum level that should receive log messages. + * Default 'alert'. One of: emergency|alert|critical|error|warning|notice|info|debug */ - public function __construct( $args = array() ) { - - $args = wp_parse_args( $args, array( - 'threshold' => 'critical', - 'to' => get_option( 'admin_email' ), - ) ); - - parent::__construct( $args ); - - if ( is_array( $args['to'] ) ) { - foreach ( $args['to'] as $to ) { - $this->add_email( $to ); - } - } else { - $this->add_email( $args['to'] ); + public function __construct( $recipients = null, $threshold = 'alert' ) { + if ( null === $recipients ) { + $recipients = get_option( 'admin_email' ); } + if ( is_array( $recipients ) ) { + foreach ( $recipients as $recipient ) { + $this->add_email( $recipient ); + } + } else { + $this->add_email( $recipients ); + } + + $this->set_threshold( $threshold ); + } + + /** + * Set handler severity threshold. + * + * @param string $level emergency|alert|critical|error|warning|notice|info|debug + */ + public function set_threshold( $level ) { + $this->threshold = WC_Log_Levels::get_level_severity( $level ); + } + + /** + * Determine whether handler should handle log. + * + * @param string $level emergency|alert|critical|error|warning|notice|info|debug + * @return bool True if the log should be handled. + */ + public function should_handle( $level ) { + return $this->threshold <= WC_Log_Levels::get_level_severity( $level ); } /** @@ -69,7 +91,7 @@ class WC_Log_Handler_Email extends WC_Log_Handler { if ( $this->should_handle( $level ) ) { $subject = $this->get_subject( $timestamp, $level, $message, $context ); $body = $this->get_body( $timestamp, $level, $message, $context ); - return wp_mail( $this->to, $subject, $body ); + return wp_mail( $this->recipients, $subject, $body ); } return true; @@ -112,6 +134,6 @@ class WC_Log_Handler_Email extends WC_Log_Handler { * @param string email Email address to add */ public function add_email( $email ) { - array_push( $this->to, $email ); + array_push( $this->recipients, $email ); } } diff --git a/includes/log-handlers/class-wc-log-handler-file.php b/includes/log-handlers/class-wc-log-handler-file.php index 18c51e48f5e..f6ae7ff0ffb 100644 --- a/includes/log-handlers/class-wc-log-handler-file.php +++ b/includes/log-handlers/class-wc-log-handler-file.php @@ -37,21 +37,15 @@ class WC_Log_Handler_File extends WC_Log_Handler { /** * Constructor for the logger. * - * @param $args additional args. { - * Optional. @see WC_Log_Handler::__construct. - * - * @type int $log_size_limit Optional. Size limit for log files. Default 5mb. - * } + * @param int $log_size_limit Optional. Size limit for log files. Default 5mb. */ - public function __construct( $args = array() ) { + public function __construct( $log_size_limit = null ) { - $args = wp_parse_args( $args, array( - 'log_size_limit' => 5 * 1024 * 1024, - ) ); + if ( null === $log_size_limit ) { + $log_size_limit = 5 * 1024 * 1024; + } - parent::__construct( $args ); - - $this->log_size_limit = $args['log_size_limit']; + $this->log_size_limit = $log_size_limit; } /** @@ -83,10 +77,6 @@ class WC_Log_Handler_File extends WC_Log_Handler { */ public function handle( $timestamp, $level, $message, $context ) { - if ( ! $this->should_handle( $level ) ) { - return true; - } - if ( isset( $context['tag'] ) && $context['tag'] ) { $handle = $context['tag']; } else { diff --git a/tests/unit-tests/log/log-handler-email.php b/tests/unit-tests/log/log-handler-email.php index c665ef9f098..120b245748f 100644 --- a/tests/unit-tests/log/log-handler-email.php +++ b/tests/unit-tests/log/log-handler-email.php @@ -23,15 +23,15 @@ class WC_Tests_Log_Handler_Email extends WC_Unit_Test_Case { * @since 2.8 */ public function test_handle() { - $handler = new WC_Log_Handler_Email( array( 'threshold' => 'debug' ) ); + $handler = new WC_Log_Handler_Email(); $time = time(); - $handler->handle( $time, 'debug', 'msg_debug', array() ); + $handler->handle( $time, 'emergency', 'msg_emergency', array() ); $mailer = tests_retrieve_phpmailer_instance(); $this->assertEquals( - 'You have recieved the following WooCommerce log message:' . PHP_EOL . PHP_EOL . date( 'c', $time ) . ' DEBUG msg_debug' . PHP_EOL, + 'You have recieved the following WooCommerce log message:' . PHP_EOL . PHP_EOL . date( 'c', $time ) . ' EMERGENCY msg_emergency' . PHP_EOL, $mailer->get_sent()->body ); } diff --git a/tests/unit-tests/log/log-handler-file.php b/tests/unit-tests/log/log-handler-file.php index e8c5d9599dd..d5b7d4c0632 100644 --- a/tests/unit-tests/log/log-handler-file.php +++ b/tests/unit-tests/log/log-handler-file.php @@ -59,7 +59,7 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { * @since 2.8 */ public function test_clear() { - $handler = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + $handler = new WC_Log_Handler_File(); $log_name = '_test_clear'; $handler->handle( time(), 'debug', 'debug', array( 'tag' => $log_name ) ); $handler->clear( $log_name ); @@ -72,7 +72,7 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { * @since 2.8 */ public function test_remove() { - $handler = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + $handler = new WC_Log_Handler_File(); $log_name = '_test_remove'; $handler->handle( time(), 'debug', 'debug', array( 'tag' => $log_name ) ); $handler->remove( $log_name ); @@ -85,7 +85,7 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { * @since 2.8 */ public function test_writes_file() { - $handler = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + $handler = new WC_Log_Handler_File(); $time = time(); $handler->handle( $time, 'debug', 'debug', array() ); @@ -107,7 +107,7 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { * @since 2.8 */ public function test_log_file_tag() { - $handler = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + $handler = new WC_Log_Handler_File(); $time = time(); $context_tag = array( 'tag' => 'unit-tests' ); @@ -130,8 +130,8 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { * @since 2.8 */ public function test_multiple_handlers() { - $handler_a = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); - $handler_b = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + $handler_a = new WC_Log_Handler_File(); + $handler_b = new WC_Log_Handler_File(); $time = time(); $context_tag = array( 'tag' => 'unit-tests' ); @@ -152,31 +152,38 @@ class WC_Tests_Log_Handler_File extends WC_Unit_Test_Case { /** * Test log_rotate() * + * Ensure logs are rotated correctly when limit is surpassed. + * * @since 2.8 */ public function test_log_rotate() { - $handler = new WC_Log_Handler_File( array( 'threshold' => 'debug' ) ); + + // Handler with log size limit of 5mb + $handler = new WC_Log_Handler_File( 5 * 1024 * 1024 ); $time = time(); $log_name = '_test_log_rotate'; $base_log_file = wc_get_log_file_path( $log_name ); - // Create log file larger than limit (5mb) to ensure log is rotated + // Create log file larger than 5mb to ensure log is rotated $handle = fopen( $base_log_file, 'w' ); - fseek( $handle, 6 * 1024 * 1024 ); + fseek( $handle, 5 * 1024 * 1024 ); fwrite( $handle, '_base_log_file_contents_' ); fclose( $handle ); + // Write some files to ensure they've rotated correctly for ( $i = 0; $i < 10; $i++ ) { file_put_contents( wc_get_log_file_path( $log_name . ".{$i}" ), $i ); } $context_tag = array( 'tag' => $log_name ); - $handler->handle( $time, 'emergency', 'emergency', $context_tag ); $this->assertFileExists( wc_get_log_file_path( $log_name ) ); + + // Ensure the handled log is correct $this->assertStringEndsWith( 'EMERGENCY emergency' . PHP_EOL, $this->read_content( $log_name ) ); + // Ensure other logs have rotated correctly $this->assertEquals( '_base_log_file_contents_', trim( $this->read_content( $log_name . '.0' ) ) ); for ( $i = 1; $i < 10; $i++ ) { $this->assertEquals( $i - 1, $this->read_content( $log_name . ".{$i}" ) );