Reformat the logging classes. Extract the LoggingHandler to its own file. Add proper documentation. Fix typos.

4 files changed, 172 insertions, 97 deletions

diff --git a/lib/SimpleSAML/Logger/LoggingHandler.php b/lib/SimpleSAML/Logger/LoggingHandler.php
new file mode 100644
index 0000000..d4d07cd
--- /dev/null
+++ b/lib/SimpleSAML/Logger/LoggingHandler.php
@@ -0,0 +1,27 @@
+<?php
+/**
+ * The interface that must be implemented by any log handler.
+ *
+ * @author Jaime Perez Crespo, UNINETT AS.
+ * @package simpleSAMLphp
+ * @version $ID$
+ */
+
+interface SimpleSAML_Logger_LoggingHandler
+{
+    /**
+     * Log a message to its destination.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string);
+
+
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format);
+}
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php b/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
index cd9404d..7dcfc7b 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
@@ -9,38 +9,53 @@
  * @package simpleSAMLphp
  * @version $ID$
  */
-class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_LoggingHandler {
-
-	/**
-	 * This array contains the mappings from syslog loglevel to names.
-	 */
-	private static $levelNames = array(
-		SimpleSAML_Logger::EMERG => 'EMERG',
-		SimpleSAML_Logger::ALERT => 'ALERT',
-		SimpleSAML_Logger::CRIT => 'CRIT',
-		SimpleSAML_Logger::ERR => 'ERR',
-		SimpleSAML_Logger::WARNING => 'WARNING',
-		SimpleSAML_Logger::NOTICE => 'NOTICE',
-		SimpleSAML_Logger::INFO => 'INFO',
-		SimpleSAML_Logger::DEBUG => 'DEBUG',
-	);
-
-
-    function setLogFormat($format) {
+class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_LoggingHandler
+{
+
+    /**
+     * This array contains the mappings from syslog loglevel to names.
+     */
+    private static $levelNames = array(
+        SimpleSAML_Logger::EMERG   => 'EMERG',
+        SimpleSAML_Logger::ALERT   => 'ALERT',
+        SimpleSAML_Logger::CRIT    => 'CRIT',
+        SimpleSAML_Logger::ERR     => 'ERR',
+        SimpleSAML_Logger::WARNING => 'WARNING',
+        SimpleSAML_Logger::NOTICE  => 'NOTICE',
+        SimpleSAML_Logger::INFO    => 'INFO',
+        SimpleSAML_Logger::DEBUG   => 'DEBUG',
+    );
+    private $format;
+
+
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-	function log_internal($level, $string) {
-		$config = SimpleSAML_Configuration::getInstance();
-		assert($config instanceof SimpleSAML_Configuration);
-		$processname = $config->getString('logging.processname','simpleSAMLphp');
-		
-		if(array_key_exists($level, self::$levelNames)) {
-			$levelName = self::$levelNames[$level];
-		} else {
-			$levelName = sprintf('UNKNOWN%d', $level);
-		}
+    /**
+     * Log a message to syslog.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        $config = SimpleSAML_Configuration::getInstance();
+        assert($config instanceof SimpleSAML_Configuration);
+        $processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
+        if (array_key_exists($level, self::$levelNames)) {
+            $levelName = self::$levelNames[$level];
+        } else {
+            $levelName = sprintf('UNKNOWN%d', $level);
+        }
 
         $formats = array('%process', '%level');
         $replacements = array($processname, $levelName);
@@ -48,8 +63,6 @@ class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_Logg
         $string = preg_replace('/%\w+(\{[^\}]+\})?/', '', $string);
         $string = trim($string);
 
-		error_log($string);
-	}
+        error_log($string);
+    }
 }
-
-?>
\ No newline at end of file
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerFile.php b/lib/SimpleSAML/Logger/LoggingHandlerFile.php
index cb5e2b1..9ed795e 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerFile.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerFile.php
@@ -9,60 +9,82 @@
  * @version $ID$
  */
 
-class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingHandler {
-    
-	/**
-	 * This array contains the mappings from syslog loglevel to names. Copied
-	 * more or less directly from SimpleSAML_Logger_LoggingHandlerErrorLog.
-	 */
-	private static $levelNames = array(
-		SimpleSAML_Logger::EMERG => 'EMERGENCY',
-		SimpleSAML_Logger::ALERT => 'ALERT',
-		SimpleSAML_Logger::CRIT => 'CRITICAL',
-		SimpleSAML_Logger::ERR => 'ERROR',
-		SimpleSAML_Logger::WARNING => 'WARNING',
-		SimpleSAML_Logger::NOTICE => 'NOTICE',
-		SimpleSAML_Logger::INFO => 'INFO',
-		SimpleSAML_Logger::DEBUG => 'DEBUG',
-	);
-
-    private $logFile = null;
-    private $processname = null;
+class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingHandler
+{
+    /**
+     * This array contains the mappings from syslog loglevel to names. Copied
+     * more or less directly from SimpleSAML_Logger_LoggingHandlerErrorLog.
+     */
+    private static $levelNames = array(
+        SimpleSAML_Logger::EMERG   => 'EMERGENCY',
+        SimpleSAML_Logger::ALERT   => 'ALERT',
+        SimpleSAML_Logger::CRIT    => 'CRITICAL',
+        SimpleSAML_Logger::ERR     => 'ERROR',
+        SimpleSAML_Logger::WARNING => 'WARNING',
+        SimpleSAML_Logger::NOTICE  => 'NOTICE',
+        SimpleSAML_Logger::INFO    => 'INFO',
+        SimpleSAML_Logger::DEBUG   => 'DEBUG',
+    );
+    private $logFile = NULL;
+    private $processname = NULL;
     private $format;
 
-    function __construct() {
+
+    /**
+     * Build a new logging handler based on files.
+     */
+    public function __construct()
+    {
         $config = SimpleSAML_Configuration::getInstance();
         assert($config instanceof SimpleSAML_Configuration);
 
-        /* Get the metadata handler option from the configuration. */
-        $this->logFile = $config->getPathValue('loggingdir', 'log/').$config->getString('logging.logfile', 'simplesamlphp.log');
-		$this->processname = $config->getString('logging.processname','simpleSAMLphp');
-		
+        // get the metadata handler option from the configuration
+        $this->logFile = $config->getPathValue('loggingdir', 'log/') .
+            $config->getString('logging.logfile', 'simplesamlphp.log');
+        $this->processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
         if (@file_exists($this->logFile)) {
-            if (!@is_writeable($this->logFile)) throw new Exception("Could not write to logfile: ".$this->logFile);
-        }
-        else
-        {
-            if (!@touch($this->logFile))  throw new Exception("Could not create logfile: ".$this->logFile." Loggingdir is not writeable for the webserver user.");
+            if (!@is_writeable($this->logFile)) {
+                throw new Exception("Could not write to logfile: " . $this->logFile);
+            }
+        } else {
+            if (!@touch($this->logFile)) {
+                throw new Exception(
+                    "Could not create logfile: " . $this->logFile .
+                    " Loggingdir is not writeable for the webserver user."
+                );
+            }
         }
 
-	SimpleSAML_Utilities::initTimezone();
+        SimpleSAML_Utilities::initTimezone();
     }
 
 
-    function setLogFormat($format) {
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-    function log_internal($level, $string) {
-        if ($this->logFile != null) {
-            
-            // Set human-readable log level. Copied from SimpleSAML_Logger_LoggingHandlerErrorLog.
-        	if(array_key_exists($level, self::$levelNames))
-			    $levelName = self::$levelNames[$level];
-		    else
-			    $levelName = sprintf('UNKNOWN%d', $level);
+    /**
+     * Log a message to the log file.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        if ($this->logFile != NULL) {
+            // set human-readable log level. Copied from SimpleSAML_Logger_LoggingHandlerErrorLog.
+            $levelName = sprintf('UNKNOWN%d', $level);
+            if (array_key_exists($level, self::$levelNames)) {
+                $levelName = self::$levelNames[$level];
+            }
 
             $formats = array('%process', '%level');
             $replacements = array($this->processname, $levelName);
@@ -79,9 +101,7 @@ class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingH
             }
 
             $string = str_replace($formats, $replacements, $string);
-            file_put_contents($this->logFile, $string . PHP_EOL, FILE_APPEND);
+            file_put_contents($this->logFile, $string.PHP_EOL, FILE_APPEND);
         }
     }
 }
-
-?>
\ No newline at end of file
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php b/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
index 58fc183..f6d58b1 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
@@ -9,44 +9,60 @@
  * @version $ID$
  */
 
-class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_LoggingHandler {
+class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_LoggingHandler
+{
+    private $isWindows = FALSE;
+    private $format;
 
-	private $isWindows = false;
 
-    function __construct() {
+    /**
+     * Build a new logging handler based on syslog.
+     */
+    public function __construct()
+    {
         $config = SimpleSAML_Configuration::getInstance();
         assert($config instanceof SimpleSAML_Configuration);
         $facility = $config->getInteger('logging.facility', defined('LOG_LOCAL5') ? constant('LOG_LOCAL5') : LOG_USER);
 
-        $processname = $config->getString('logging.processname','simpleSAMLphp');
-        /*
-         * OS Check 
-         * Setting facility to LOG_USER (only valid in Windows), enable log level rewrite on windows systems.
-         */
+        $processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
+        // Setting facility to LOG_USER (only valid in Windows), enable log level rewrite on windows systems.
         if (SimpleSAML_Utilities::isWindowsOS()) {
-        	$this->isWindows = true;
-        	$facility = LOG_USER;
+            $this->isWindows = TRUE;
+            $facility = LOG_USER;
         }
-        	
+
         openlog($processname, LOG_PID, $facility);
     }
 
 
-    function setLogFormat($format) {
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-    function log_internal($level,$string) {
-    	/*
-    	 * Changing log level to supported levels if OS is Windows
-    	 */
-    	if ($this->isWindows) {
-    		if ($level <= 4)
-				$level = LOG_ERR;
-			else
-				$level = LOG_INFO;			
-    	}
+    /**
+     * Log a message to syslog.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        // changing log level to supported levels if OS is Windows
+        if ($this->isWindows) {
+            if ($level <= 4) {
+                $level = LOG_ERR;
+            } else {
+                $level = LOG_INFO;
+            }
+        }
 
         $formats = array('%process', '%level');
         $replacements = array('', $level);
@@ -57,4 +73,3 @@ class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_Loggin
         syslog($level, $string);
     }
 }
-?>
\ No newline at end of file