From commits-return-1089-apmail-logging-commits-archive=logging.apache.org@logging.apache.org Sun Aug 19 16:15:15 2012 Return-Path: X-Original-To: apmail-logging-commits-archive@minotaur.apache.org Delivered-To: apmail-logging-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1E5A89F98 for ; Sun, 19 Aug 2012 16:15:15 +0000 (UTC) Received: (qmail 45848 invoked by uid 500); 19 Aug 2012 16:15:15 -0000 Delivered-To: apmail-logging-commits-archive@logging.apache.org Received: (qmail 45820 invoked by uid 500); 19 Aug 2012 16:15:15 -0000 Mailing-List: contact commits-help@logging.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@logging.apache.org Delivered-To: mailing list commits@logging.apache.org Received: (qmail 45813 invoked by uid 99); 19 Aug 2012 16:15:15 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 19 Aug 2012 16:15:15 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 19 Aug 2012 16:15:10 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 8DE4923888FD; Sun, 19 Aug 2012 16:14:25 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1374786 - in /logging/log4php/trunk: ./ src/main/php/appenders/ src/test/php/appenders/ Date: Sun, 19 Aug 2012 16:14:25 -0000 To: commits@logging.apache.org From: ihabunek@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120819161425.8DE4923888FD@eris.apache.org> Author: ihabunek Date: Sun Aug 19 16:14:24 2012 New Revision: 1374786 URL: http://svn.apache.org/viewvc?rev=1374786&view=rev Log: A substantial rewrite of the three file appenders: - a file will not be created on activation, but on first append (LOG4PHP-130) - compression in rolling appender is done in chunks (LOG4PHP-164) - general improvements in writing logic, better error handling Modified: logging/log4php/trunk/pom.xml logging/log4php/trunk/src/main/php/appenders/LoggerAppenderDailyFile.php logging/log4php/trunk/src/main/php/appenders/LoggerAppenderFile.php logging/log4php/trunk/src/main/php/appenders/LoggerAppenderRollingFile.php logging/log4php/trunk/src/test/php/appenders/LoggerAppenderFileTest.php logging/log4php/trunk/src/test/php/appenders/LoggerAppenderRollingFileTest.php Modified: logging/log4php/trunk/pom.xml URL: http://svn.apache.org/viewvc/logging/log4php/trunk/pom.xml?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/pom.xml (original) +++ logging/log4php/trunk/pom.xml Sun Aug 19 16:14:24 2012 @@ -166,71 +166,7 @@ - - pear-package - package - - - - - - - run - - - - - pre-site - pre-site - - - - - - - - run - - - - - unit-testing - test - - - - - - - run - - - - - post-site - post-site - - - - - - - run - - - - - site-deploy - site-deploy - - - - - - - run - - + @@ -267,7 +203,7 @@ org.apache.maven.plugins maven-release-plugin - 2.2.2 + 2.3.2 site assembly:assembly @@ -276,7 +212,7 @@ org.apache.maven.plugins maven-site-plugin - 3.0 + 3.1 @@ -284,59 +220,6 @@ - - - - org.apache.maven.plugins - maven-project-info-reports-plugin - 2.4 - - - - cim - scm - dependencies - license - project-team - issue-tracking - mailing-list - - - - - - - - org.apache.maven.plugins - maven-changes-plugin - 2.6 - - - - changes-report - jira-report - - - - - Resolved, Closed - Type,Key,Summary,Assignee,Status,Resolution,Fix Version - - - - - - org.apache.maven.plugins - maven-surefire-report-plugin - 2.12 - - - - - org.apache.rat - apache-rat-plugin - 0.8 - Modified: logging/log4php/trunk/src/main/php/appenders/LoggerAppenderDailyFile.php URL: http://svn.apache.org/viewvc/logging/log4php/trunk/src/main/php/appenders/LoggerAppenderDailyFile.php?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/src/main/php/appenders/LoggerAppenderDailyFile.php (original) +++ logging/log4php/trunk/src/main/php/appenders/LoggerAppenderDailyFile.php Sun Aug 19 16:14:24 2012 @@ -48,42 +48,27 @@ class LoggerAppenderDailyFile extends Lo * @var string */ protected $datePattern = "Ymd"; - - /** - * Similar to parent method, but but replaces "%s" in the file name with - * the current date in format specified by the 'datePattern' parameter. - */ + + /** Additional validation for the date pattern. */ public function activateOptions() { - $fileName = $this->getFile(); - $date = date($this->getDatePattern()); - $fileName = sprintf($fileName, $date); - - if(!is_file($fileName)) { - $dir = dirname($fileName); - if(!is_dir($dir)) { - mkdir($dir, 0777, true); - } - } + parent::activateOptions(); - $this->fp = fopen($fileName, ($this->getAppend()? 'a':'w')); - if($this->fp) { - if(flock($this->fp, LOCK_EX)) { - if($this->getAppend()) { - fseek($this->fp, 0, SEEK_END); - } - fwrite($this->fp, $this->layout->getHeader()); - flock($this->fp, LOCK_UN); - $this->closed = false; - } else { - // TODO: should we take some action in this case? - $this->closed = true; - } - } else { + if (empty($this->datePattern)) { + $this->warn("Required parameter 'datePattern' not set. Closing appender."); $this->closed = true; + return; } } /** + * Determines target file. Replaces %s in file path with a date. + */ + protected function getTargetFile() { + $date = date($this->datePattern); + return str_replace('%s', $date, $this->file); + } + + /** * Sets the 'datePattern' parameter. * @param string $datePattern */ Modified: logging/log4php/trunk/src/main/php/appenders/LoggerAppenderFile.php URL: http://svn.apache.org/viewvc/logging/log4php/trunk/src/main/php/appenders/LoggerAppenderFile.php?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/src/main/php/appenders/LoggerAppenderFile.php (original) +++ logging/log4php/trunk/src/main/php/appenders/LoggerAppenderFile.php Sun Aug 19 16:14:24 2012 @@ -37,76 +37,134 @@ class LoggerAppenderFile extends LoggerAppender { /** - * @var boolean if {@link $file} exists, appends events. + * If set to true, the file is locked before appending. This allows + * concurrent access. However, appending without locking is faster so + * it should be used where appropriate. + * + * TODO: make this a configurable parameter + * + * @var boolean + */ + protected $locking = true; + + /** + * If set to true, appends to file. Otherwise overwrites it. + * @var boolean */ protected $append = true; /** - * @var string the file name used to append events + * Path to the target file. + * @var string */ protected $file; /** - * @var mixed file resource + * The file resource. + * @var resource */ - protected $fp = false; + protected $fp; - public function activateOptions() { - $fileName = $this->getFile(); + /** + * Helper function which can be easily overriden by daily file appender. + */ + protected function getTargetFile() { + return $this->file; + } + + /** + * Acquires the target file resource, creates the destination folder if + * necessary. Writes layout header to file. + */ + protected function openFile() { + $file = $this->getTargetFile(); + + // Create the target folder if needed + if(!is_file($file)) { + $dir = dirname($file); - if (empty($fileName)) { - $this->warn("Required parameter 'fileName' not set. Closing appender."); + if(!is_dir($dir)) { + $success = mkdir($dir, 0777, true); + if ($success === false) { + $this->warn("Failed creating target directory [$dir]. Closing appender."); + $this->closed = true; + return; + } + } + } + + $mode = $this->append ? 'a' : 'w'; + $this->fp = fopen($file, $mode); + if ($this->fp === false) { + $this->warn("Failed opening target file. Closing appender."); $this->closed = true; return; } - if(!is_file($fileName)) { - $dir = dirname($fileName); - if(!is_dir($dir)) { - mkdir($dir, 0777, true); - } + // Required when appending with concurrent access + if($this->append) { + fseek($this->fp, 0, SEEK_END); } - - $this->fp = fopen($fileName, ($this->getAppend()? 'a':'w')); - if($this->fp) { - if(flock($this->fp, LOCK_EX)) { - if($this->getAppend()) { - fseek($this->fp, 0, SEEK_END); - } - fwrite($this->fp, $this->layout->getHeader()); - flock($this->fp, LOCK_UN); - $this->closed = false; - } else { - // TODO: should we take some action in this case? - $this->closed = true; - } + + // Write the header + $this->write($this->layout->getHeader()); + } + + /** + * Writes a string to the target file. Opens file if not already open. + * @param string $string Data to write. + */ + protected function write($string) { + // Lazy file open + if(!isset($this->fp)) { + $this->openFile(); + } + + if ($this->locking) { + $this->writeWithLocking($string); } else { - $this->closed = true; + $this->writeWithoutLocking($string); } } - public function close() { - if($this->closed != true) { - if($this->fp and $this->layout !== null) { - if(flock($this->fp, LOCK_EX)) { - fwrite($this->fp, $this->layout->getFooter()); - flock($this->fp, LOCK_UN); - } - fclose($this->fp); + protected function writeWithLocking($string) { + if(flock($this->fp, LOCK_EX)) { + if(fwrite($this->fp, $string) === false) { + $this->warn("Failed writing to file. Closing appender."); + $this->closed = true; } + flock($this->fp, LOCK_UN); + } else { + $this->warn("Failed locking file for writing. Closing appender."); + $this->closed = true; + } + } + + protected function writeWithoutLocking($string) { + if(fwrite($this->fp, $string) === false) { + $this->warn("Failed writing to file. Closing appender."); + $this->closed = true; + } + } + + public function activateOptions() { + if (empty($this->file)) { + $this->warn("Required parameter 'file' not set. Closing appender."); $this->closed = true; + return; } } + + public function close() { + if (is_resource($this->fp)) { + $this->write($this->layout->getFooter()); + fclose($this->fp); + } + $this->closed = true; + } public function append(LoggerLoggingEvent $event) { - if($this->fp and $this->layout !== null) { - if(flock($this->fp, LOCK_EX)) { - fwrite($this->fp, $this->layout->format($event)); - flock($this->fp, LOCK_UN); - } else { - $this->closed = true; - } - } + $this->write($this->layout->format($event)); } /** Modified: logging/log4php/trunk/src/main/php/appenders/LoggerAppenderRollingFile.php URL: http://svn.apache.org/viewvc/logging/log4php/trunk/src/main/php/appenders/LoggerAppenderRollingFile.php?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/src/main/php/appenders/LoggerAppenderRollingFile.php (original) +++ logging/log4php/trunk/src/main/php/appenders/LoggerAppenderRollingFile.php Sun Aug 19 16:14:24 2012 @@ -44,19 +44,15 @@ */ class LoggerAppenderRollingFile extends LoggerAppenderFile { + /** Compressing backup files is done in chunks, this determines how large. */ + const COMPRESS_CHUNK_SIZE = 102400; // 100KB + /** - * Set the maximum size that the output file is allowed to reach + * The maximum size (in bytes) that the output file is allowed to reach * before being rolled over to backup files. * - *

In configuration files, the MaxFileSize option takes a - * long integer in the range 0 - 2^63. You can specify the value - * with the suffixes "KB", "MB" or "GB" so that the integer is - * interpreted being expressed respectively in kilobytes, megabytes - * or gigabytes. For example, the value "10KB" will be interpreted - * as 10240.

- *

The default maximum file size is 10MB.

- * - *

Note that MaxFileSize cannot exceed 2 GB.

+ * The default maximum file size is 10MB (10485760 bytes). Maximum value + * for this option may depend on the file system. * * @var integer */ @@ -65,36 +61,23 @@ class LoggerAppenderRollingFile extends /** * Set the maximum number of backup files to keep around. * - *

The MaxBackupIndex option determines how many backup - * files are kept before the oldest is erased. This option takes - * a positive integer value. If set to zero, then there will be no - * backup files and the log file will be truncated when it reaches - * MaxFileSize.

- *

There is one backup file by default.

+ * Determines how many backup files are kept before the oldest is erased. + * This option takes a positive integer value. If set to zero, then there + * will be no backup files and the log file will be truncated when it + * reaches maxFileSize. + * + * There is one backup file by default. * * @var integer */ protected $maxBackupIndex = 1; /** - * @var string the filename expanded - */ - private $expandedFileName = null; - - /** * The compress parameter determindes the compression with zlib. * If set to true, the rollover files are compressed and saved with the .gz extension. * @var boolean */ protected $compress = false; - - /** - * Returns the value of the MaxBackupIndex option. - * @return integer - */ - private function getExpandedFileName() { - return $this->expandedFileName; - } /** * Get the maximum size that the output file is allowed to reach @@ -108,33 +91,30 @@ class LoggerAppenderRollingFile extends /** * Implements the usual roll over behaviour. * - *

If MaxBackupIndex is positive, then files File.1, ..., File.MaxBackupIndex -1 are renamed to File.2, ..., File.MaxBackupIndex. + * If MaxBackupIndex is positive, then files File.1, ..., File.MaxBackupIndex -1 are renamed to File.2, ..., File.MaxBackupIndex. * Moreover, File is renamed File.1 and closed. A new File is created to receive further log output. * - *

If MaxBackupIndex is equal to zero, then the File is truncated with no backup files created. + * If MaxBackupIndex is equal to zero, then the File is truncated with no backup files created. * * Rollover must be called while the file is locked so that it is safe for concurrent access. + * + * @throws LoggerException If any part of the rollover procedure fails. */ private function rollOver() { // If maxBackups <= 0, then there is no file renaming to be done. if($this->maxBackupIndex > 0) { - $fileName = $this->getExpandedFileName(); - // Delete the oldest file, to keep Windows happy. - $file = $fileName . '.' . $this->maxBackupIndex; - if(is_writable($file)) { - unlink($file); + $file = $this->file . '.' . $this->maxBackupIndex; + + if (file_exists($file) && !unlink($file)) { + throw new LoggerException("Unable to delete oldest backup file from [$file]."); } // Map {(maxBackupIndex - 1), ..., 2, 1} to {maxBackupIndex, ..., 3, 2} - $this->renameArchievedLogs($fileName); + $this->renameArchievedLogs($this->file); - if (true === $this->compress) { - file_put_contents('compress.zlib://'.$fileName.'.1.gz', file_get_contents($fileName)); - } else { - // Backup the active file - copy($fileName, "$fileName.1"); - } + // Backup the active file + $this->moveToBackup($this->file); } // Truncate the active file @@ -142,115 +122,129 @@ class LoggerAppenderRollingFile extends rewind($this->fp); } + private function moveToBackup($source) { + if ($this->compress) { + $target = $source . '.1.gz'; + $this->compressFile($source, $target); + } else { + $target = $source . '.1'; + copy($source, $target); + } + } + + private function compressFile($source, $target) { + $target = 'compress.zlib://' . $target; + + $fin = fopen($source, 'rb'); + if ($fin === false) { + throw new LoggerException("Unable to open file for reading: [$source]."); + } + + $fout = fopen($target, 'wb'); + if ($fout === false) { + throw new LoggerException("Unable to open file for writing: [$target]."); + } + + while (!feof($fin)) { + $chunk = fread($fin, self::COMPRESS_CHUNK_SIZE); + if (false === fwrite($fout, $chunk)) { + throw new LoggerException("Failed writing to compressed file."); + } + } + + fclose($fin); + fclose($fout); + } + private function renameArchievedLogs($fileName) { for($i = $this->maxBackupIndex - 1; $i >= 1; $i--) { - $file = $fileName . "." . $i; - if (true === $this->compress) { - $file = $fileName . "." . $i .'.gz'; + $source = $fileName . "." . $i; + if ($this->compress) { + $source .= '.gz'; } - if(is_readable($file)) { + if(file_exists($source)) { $target = $fileName . '.' . ($i + 1); - if (true === $this->compress) { - $target = $fileName . '.' . ($i + 1) . '.gz'; + if ($this->compress) { + $target .= '.gz'; } - rename($file, $target); + rename($source, $target); } - } + } } - public function setFile($fileName) { - $this->file = $fileName; - // As LoggerAppenderFile does not create the directory, it has to exist. - // realpath() fails if the argument does not exist so the filename is separated. - $this->expandedFileName = realpath(dirname($fileName)); - if ($this->expandedFileName === false) throw new Exception("Directory of $fileName does not exist!"); - $this->expandedFileName .= DIRECTORY_SEPARATOR . basename($fileName); - } - - /** - * Set the maximum number of backup files to keep around. - * - *

The MaxBackupIndex option determines how many backup - * files are kept before the oldest is erased. This option takes - * a positive integer value. If set to zero, then there will be no - * backup files and the log file will be truncated when it reaches - * MaxFileSize. - * - * @param mixed $maxBackups + * Writes a string to the target file. Opens file if not already open. + * @param string $string Data to write. */ - public function setMaxBackupIndex($maxBackups) { - $this->setPositiveInteger('maxBackupIndex', $maxBackups); - } - - - public function append(LoggerLoggingEvent $event) { - if($this->fp and $this->layout !== null) { - if(flock($this->fp, LOCK_EX)) { - fwrite($this->fp, $this->layout->format($event)); - - // Stats cache must be cleared, otherwise filesize() returns cached results - clearstatcache(); - - // Rollover if needed - if (filesize($this->expandedFileName) > $this->maxFileSize) { + protected function write($string) { + // Lazy file open + if(!isset($this->fp)) { + $this->openFile(); + } + + // Lock the file while writing and possible rolling over + if(flock($this->fp, LOCK_EX)) { + + // Write to locked file + if(fwrite($this->fp, $string) === false) { + $this->warn("Failed writing to file. Closing appender."); + $this->closed = true; + } + + // Stats cache must be cleared, otherwise filesize() returns cached results + clearstatcache(); + + // Rollover if needed + if (filesize($this->file) > $this->maxFileSize) { + try { $this->rollOver(); + } catch (LoggerException $ex) { + $this->warn("Rollover failed: " . $ex->getMessage() . " Closing appender."); + $this->closed = true; } - - flock($this->fp, LOCK_UN); - } else { - $this->closed = true; } - } + + flock($this->fp, LOCK_UN); + } else { + $this->warn("Failed locking file for writing. Closing appender."); + $this->closed = true; + } } public function activateOptions() { parent::activateOptions(); - if ($this->compress == true && !extension_loaded('zlib')) { - $this->warn('The zlib extension is required for file-compression'); - $this->closed = true; + if ($this->compress && !extension_loaded('zlib')) { + $this->warn("The 'zlib' extension is required for file compression. Disabling compression."); + $this->compression = false; } } /** - * Returns the 'maxBackupIndex' parameter. - * @return integer + * Set the 'maxBackupIndex' parameter. + * @param integer $maxBackupIndex */ - public function getMaxBackupIndex() { - return $this->maxBackupIndex; + public function setMaxBackupIndex($maxBackupIndex) { + $this->setPositiveInteger('maxBackupIndex', $maxBackupIndex); } /** - * Set the maximum size that the output file is allowed to reach - * before being rolled over to backup files. - *

In configuration files, the maxFileSize option takes an - * long integer in the range 0 - 2^63. You can specify the value - * with the suffixes "KB", "MB" or "GB" so that the integer is - * interpreted being expressed respectively in kilobytes, megabytes - * or gigabytes. For example, the value "10KB" will be interpreted - * as 10240. - * - * @param mixed $value - * @return the actual file size set + * Returns the 'maxBackupIndex' parameter. + * @return integer */ - public function setMaxFileSize($value) { - $this->setFileSize('maxFileSize', $value); + public function getMaxBackupIndex() { + return $this->maxBackupIndex; } /** - * Set the maximum size that the output file is allowed to reach - * before being rolled over to backup files. - * + * Set the 'maxFileSize' parameter. * @param mixed $maxFileSize - * @see setMaxFileSize() - * @deprecated */ - public function setMaximumFileSize($maxFileSize) { - return $this->setMaxFileSize($maxFileSize); + public function setMaxFileSize($maxFileSize) { + $this->setFileSize('maxFileSize', $maxFileSize); } /** @@ -261,7 +255,29 @@ class LoggerAppenderRollingFile extends return $this->maxFileSize; } + /** + * Set the 'maxFileSize' parameter (kept for backward compatibility). + * @param mixed $maxFileSize + * @deprecated Use setMaxFileSize() instead. + */ + public function setMaximumFileSize($maxFileSize) { + $this->warn("The 'maximumFileSize' parameter is deprecated. Use 'maxFileSize' instead."); + return $this->setMaxFileSize($maxFileSize); + } + + /** + * Sets the 'compress' parameter. + * @param boolean $compress + */ public function setCompress($compress) { $this->setBoolean('compress', $compress); } + + /** + * Returns the 'compress' parameter. + * @param boolean + */ + public function getCompress() { + return $this->compress; + } } Modified: logging/log4php/trunk/src/test/php/appenders/LoggerAppenderFileTest.php URL: http://svn.apache.org/viewvc/logging/log4php/trunk/src/test/php/appenders/LoggerAppenderFileTest.php?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/src/test/php/appenders/LoggerAppenderFileTest.php (original) +++ logging/log4php/trunk/src/test/php/appenders/LoggerAppenderFileTest.php Sun Aug 19 16:14:24 2012 @@ -68,6 +68,21 @@ class LoggerAppenderFileTest extends PHP self::assertTrue($appender->requiresLayout()); } + public function testActivationDoesNotCreateTheFile() { + $path = PHPUNIT_TEMP_DIR . "/doesnotexisthopefully.log"; + @unlink($path); + $appender = new LoggerAppenderFile(); + $appender->setFile($path); + $appender->activateOptions(); + + self::assertFalse(file_exists($path)); + + $event = LoggerTestHelper::getInfoEvent('bla'); + $appender->append($event); + + self::assertTrue(file_exists($path)); + } + public function testSimpleLogging() { $config = $this->config1; $config['appenders']['default']['params']['file'] = $this->testPath; Modified: logging/log4php/trunk/src/test/php/appenders/LoggerAppenderRollingFileTest.php URL: http://svn.apache.org/viewvc/logging/log4php/trunk/src/test/php/appenders/LoggerAppenderRollingFileTest.php?rev=1374786&r1=1374785&r2=1374786&view=diff ============================================================================== --- logging/log4php/trunk/src/test/php/appenders/LoggerAppenderRollingFileTest.php (original) +++ logging/log4php/trunk/src/test/php/appenders/LoggerAppenderRollingFileTest.php Sun Aug 19 16:14:24 2012 @@ -113,7 +113,8 @@ class LoggerAppenderRollingFileTest exte $file = PHPUNIT_TEMP_DIR . '/TEST-rolling.txt.2'; $this->checkFileContent($file); - $this->assertFalse(file_exists(PHPUNIT_TEMP_DIR.'/TEST-rolling.txt.3'), 'should not roll over three times'); + // Should not roll over three times + $this->assertFalse(file_exists(PHPUNIT_TEMP_DIR.'/TEST-rolling.txt.3')); } public function testLoggingViaLogger() { @@ -202,5 +203,4 @@ class LoggerAppenderRollingFileTest exte @unlink(PHPUNIT_TEMP_DIR.'/TEST-rolling.txt.1.gz'); @unlink(PHPUNIT_TEMP_DIR.'/TEST-rolling.txt.2.gz'); } - }