You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4php-dev@logging.apache.org by ch...@apache.org on 2009/10/06 22:28:47 UTC

svn commit: r822478 - in /incubator/log4php/trunk/src: main/php/appenders/LoggerAppenderPDO.php main/php/layouts/LoggerLayoutPattern.php test/php/appenders/LoggerAppenderPDOTest.php

Author: chammers
Date: Tue Oct  6 20:28:47 2009
New Revision: 822478

URL: http://svn.apache.org/viewvc?rev=822478&view=rev
Log:
LOG4PHP-46      Use PreparedStatement in PDO-Appender
LOG4PHP-47      Check if PDO Appender needs quote()
LOG4PHP-79      AppenderPDO: Some database fields seem a bit too short

Improved version of the PDO appender. Apart from increased field
sizes in the CREATE statement, this one features a new configuration
style that enables us to use prepared statements and therefore
automatically quoting.

The old style is documented as deprecated but still working to
make updates from earlier log4php versions easier.

phpunit test case attached.


Modified:
    incubator/log4php/trunk/src/main/php/appenders/LoggerAppenderPDO.php
    incubator/log4php/trunk/src/main/php/layouts/LoggerLayoutPattern.php
    incubator/log4php/trunk/src/test/php/appenders/LoggerAppenderPDOTest.php

Modified: incubator/log4php/trunk/src/main/php/appenders/LoggerAppenderPDO.php
URL: http://svn.apache.org/viewvc/incubator/log4php/trunk/src/main/php/appenders/LoggerAppenderPDO.php?rev=822478&r1=822477&r2=822478&view=diff
==============================================================================
--- incubator/log4php/trunk/src/main/php/appenders/LoggerAppenderPDO.php (original)
+++ incubator/log4php/trunk/src/main/php/appenders/LoggerAppenderPDO.php Tue Oct  6 20:28:47 2009
@@ -125,7 +125,7 @@
     	
             // test if log table exists
             try {
-                $result = $this->db->query('select * from ' . $this->table . ' where 1 = 0');
+                $result = $this->db->query('SELECT * FROM ' . $this->table . ' WHERE 1 = 0');
             } catch (PDOException $e) {
                 // It could be something else but a "no such table" is the most likely
                 $result = false;
@@ -133,14 +133,15 @@
             
             // create table if necessary
             if ($result == false and $this->createTable) {
-        	   // TODO mysql syntax?
-                $query = "CREATE TABLE {$this->table} (	 timestamp varchar(32)," .
-            										"logger varchar(32)," .
+                // The syntax should at least be compatible with MySQL, PostgreSQL, SQLite and Oracle.
+                $query = "CREATE TABLE {$this->table} (".
+                            "timestamp varchar(32)," .
+            				"logger varchar(64)," .
             										"level varchar(32)," .
-            										"message varchar(64)," .
+            				"message varchar(9999)," .
             										"thread varchar(32)," .
-            										"file varchar(64)," .
-            										"line varchar(4) );";
+            				"file varchar(255)," .
+            				"line varchar(6))";
                 $result = $this->db->query($query);
             }
         } catch (PDOException $e) {
@@ -148,37 +149,49 @@
             throw new LoggerException($e);
         }
         
-        if($this->sql == '' || $this->sql == null) {
-            $this->sql = "INSERT INTO $this->table ( timestamp, " .
-            										"logger, " .
-            										"level, " .
-            										"message, " .
-            										"thread, " .
-            										"file, " .
-            										"line" .
-						 ") VALUES ('%d','%c','%p','%m','%t','%F','%L')";
-        }
+        $this->layout = new LoggerLayoutPattern();
         
-		$this->layout = LoggerReflectionUtils::createObject('LoggerLayoutPattern');
+        //
+        // Keep compatibility to legacy option $sql which already included the format patterns!
+        //
+        if (empty($this->sql)) {
+            // new style with prepared Statment and $insertSql and $insertPattern
+            // Maybe the tablename has to be substituted.
+            $this->insertSql = preg_replace('/__TABLE__/', $this->table, $this->insertSql);
+            $this->preparedInsert = $this->db->prepare($this->insertSql);
+            $this->layout->setConversionPattern($this->insertPattern);
+        } else {
+            // Old style with format strings in the $sql query should be used.
         $this->layout->setConversionPattern($this->sql);
+        }
+
         $this->canAppend = true;
         return true;
     }
     
     /**
-     * Appends a new event to the database using the sql format.
+     * Appends a new event to the database.
+     * 
+     * @throws LoggerException      If the pattern conversion or the INSERT statement fails.
      */
-     // TODO:should work with prepared statement
     public function append(LoggerLoggingEvent $event) {
-        if ($this->canAppend) {
-            $query = $this->layout->format($event);
+        // TODO: Can't activateOptions() simply throw an Exception if it encounters problems?
+        if ( ! $this->canAppend) return;
+
             try {
+            if (empty($this->sql)) {
+                // new style with prepared statement
+                $params = $this->layout->formatToArray($event);
+                $this->preparedInsert->execute($params);
+            } else {
+                // old style
+                $query = $this->layout->format($event);
                 $this->db->exec($query);
+            }
             } catch (Exception $e) {
                 throw new LoggerException($e);
             }
         }
-    }
     
     /**
      * Closes the connection to the logging database
@@ -226,12 +239,34 @@
      * ('%d','%c','%p','%m','%t','%F','%L')
      * 
      * It's not necessary to change this except you have customized logging'
+     *
+     * @deprecated See {@link setInsertSql} and {@link setInsertPattern}.
      */
     public function setSql($sql) {
         $this->sql = $sql;    
     }
     
     /**
+     * Sets the SQL INSERT string to use with {@link $insertPattern}.
+     *
+     * @param $sql          A complete INSERT INTO query with "?" that gets replaced.
+     */
+    public function setInsertSql($sql) {
+        $this->insertSql = $sql;
+    }
+
+    /**
+     * Sets the {@link LoggerLayoutPattern} format strings for {@link $insertSql}.
+     *
+     * It's not necessary to change this except you have customized logging.
+     *
+     * @param $pattern          Comma separated format strings like "%p,%m,%C"
+     */
+    public function setInsertPattern($pattern) {
+        $this->insertPattern = $pattern;
+    }
+
+    /**
      * Sets the tablename to which this appender should log.
      * Defaults to log4php_log
      */

Modified: incubator/log4php/trunk/src/main/php/layouts/LoggerLayoutPattern.php
URL: http://svn.apache.org/viewvc/incubator/log4php/trunk/src/main/php/layouts/LoggerLayoutPattern.php?rev=822478&r1=822477&r2=822478&view=diff
==============================================================================
--- incubator/log4php/trunk/src/main/php/layouts/LoggerLayoutPattern.php (original)
+++ incubator/log4php/trunk/src/main/php/layouts/LoggerLayoutPattern.php Tue Oct  6 20:28:47 2009
@@ -191,4 +191,29 @@
 		}
 		return $sbuf;
 	}
+	
+    /**
+     * Returns an array with the formatted elements.
+     * 
+     * This method is mainly used for the prepared statements of {@see LoggerAppenderPDO}.
+     * 
+     * It requires {@link $this->pattern} to be a comma separated string of patterns like
+     * e.g. <code>%d,%c,%p,%m,%t,%F,%L</code>.
+     * 
+     * @return array(string)   An array of the converted elements i.e. timestamp, message, filename etc.
+     */
+    public function formatToArray(LoggerLoggingEvent $event) {
+        $results = array();
+        $c = $this->head;
+        while ($c !== null) {
+            if ( ! $c instanceOf LoggerLiteralPatternConverter) {
+                $sbuf = null;
+                $c->format($sbuf, $event);
+                $results[] = $sbuf;
+            }
+            $c = $c->next;
+        }
+        return $results;
+    }      
+	
 }
\ No newline at end of file

Modified: incubator/log4php/trunk/src/test/php/appenders/LoggerAppenderPDOTest.php
URL: http://svn.apache.org/viewvc/incubator/log4php/trunk/src/test/php/appenders/LoggerAppenderPDOTest.php?rev=822478&r1=822477&r2=822478&view=diff
==============================================================================
--- incubator/log4php/trunk/src/test/php/appenders/LoggerAppenderPDOTest.php (original)
+++ incubator/log4php/trunk/src/test/php/appenders/LoggerAppenderPDOTest.php Tue Oct  6 20:28:47 2009
@@ -24,62 +24,122 @@
  */
 
 class LoggerAppenderPDOTest extends PHPUnit_Framework_TestCase {
+    const dsn = 'sqlite:../../../target/pdotest.sqlite';
+    const file = '../../../target/pdotest.sqlite';
         
-	public function testSimpleLogging() {
+    /** To start with an empty database for each single test. */
+    public function setUp() {
+        if (file_exists(self::file)) unlink(self::file);
+    }
+
+    /** Clean up after the last test was run. */
+    public function tearDownAfterClass() {
+        if (file_exists(self::file)) unlink(self::file);
+    }
 		
+    /** Tests new-style logging using prepared statements and the default SQL definition. */
+    public function testSimpleWithDefaults() {
 		if(!extension_loaded('pdo_sqlite')) {
 			self::markTestSkipped("Please install 'pdo_sqlite' in order to run this test");
 		}
 		
+        // Log event
 		$event = new LoggerLoggingEvent("LoggerAppenderPDOTest", new Logger("TEST"), LoggerLevel::getLevelError(), "testmessage");
+        $appender = new LoggerAppenderPDO("myname");
+        $appender->setDSN(self::dsn);
+        $appender->activateOptions();
+        $appender->append($event);
+        $appender->close();
 
-		$dbname = '../../../target/pdotest.sqlite';
-		$dsn = 'sqlite:'.$dbname;
+        // Test the default pattern %d,%c,%p,%m,%t,%F,%L
+        $db = new PDO(self::dsn);
+        $query = "SELECT * FROM log4php_log";
+        $sth = $db->query($query);
+        $row = $sth->fetch(PDO::FETCH_NUM);
+        self::assertTrue(is_array($row), "No rows found.");
+        self::assertEquals(7, count($row));
+        self::assertEquals(1, preg_match('/^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d,\d\d\d$/', $row[0])); // %d = date
+        self::assertEquals('TEST', $row[1]); // %c = category
+        self::assertEquals('ERROR', $row[2]); // %p = priority
+        self::assertEquals('testmessage', $row[3]); // %m = message
+        self::assertEquals(posix_getpid(), $row[4]); // %t = thread
+        self::assertEquals('NA', $row[5]); // %F = file, NA due to phpunit magic
+        self::assertEquals('NA', $row[6]); // %L = line, NA due to phpunit magic
+    }
 
-		$database = new PDO($dsn);
-		$database = null;
+
+    /** Tests new style prepared statment logging with customized SQL. */
+    public function testCustomizedSql() {
+        if(!extension_loaded('pdo_sqlite')) {
+            self::markTestSkipped("Please install 'pdo_sqlite' in order to run this test");
+        }
 		
+        // Prepare appender
 		$appender = new LoggerAppenderPDO("myname");
-		$appender->setDSN($dsn);
-		$appender->setCreateTable(true);
+        $appender->setDSN(self::dsn);
+        $appender->setTable('unittest2');
+        $appender->setInsertSql("INSERT INTO unittest2 (file, line, thread, timestamp, logger, level, message) VALUES (?,?,?,?,?,?,?)");
+        $appender->setInsertPattern("%F,%L,%t,%d,%c,%p,%m");
 		$appender->activateOptions();
+
+        // Action!
+        $event = new LoggerLoggingEvent("LoggerAppenderPDOTest2", new Logger("TEST"), LoggerLevel::getLevelError(), "testmessage");
 		$appender->append($event);
 		
+        // Check
+        $db = new PDO(self::dsn);
+        $result = $db->query("SELECT * FROM unittest2");
+        $row = $result->fetch(PDO::FETCH_OBJ);
+        self::assertTrue(is_object($row));
+        self::assertEquals("NA", $row->file); // "NA" due to phpunit magic
+        self::assertEquals("NA", $row->line); // "NA" due to phpunit magic
+        self::assertEquals(posix_getpid(), $row->thread);
+        self::assertEquals(1, preg_match('/^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d$/', $row->timestamp));
+        self::assertEquals('TEST', $row->logger);
+        self::assertEquals('ERROR', $row->level);
+        self::assertEquals('testmessage', $row->message);
+    }
 		
-		$db = $appender->getDatabaseHandle();
-		$q = "select * from log4php_log";	
-		$error = "";
-		if($result = $db->query($q)) {
-			while($row = $result->fetch()) {
-    			self::assertEquals($row['1'], 'TEST');
-    			self::assertEquals($row['2'], 'ERROR');
-    			self::assertEquals($row['3'], 'testmessage');
-  			}
-		} else {
-			// todo propagate exception to phpunit
-		   self::assertTrue(false);
+    /** Tests old-style logging using the $sql variable. */
+    public function testOldStyle() {
+        if(!extension_loaded('pdo_sqlite')) {
+            self::markTestSkipped("Please install 'pdo_sqlite' in order to run this test");
 		}
-		$appender->close();
 		
+        // Create table with different column order
+        $db = new PDO(self::dsn);
+        $db->exec('CREATE TABLE unittest3 (ts timestamp, level varchar(32), msg varchar(64))');
+
+        // Prepare appender
+        $appender = new LoggerAppenderPDO("myname");
+        $appender->setDSN(self::dsn);
+        $appender->setCreateTable(false);
+        $appender->setSql("INSERT INTO unittest3 (ts, level, msg) VALUES ('%d', '%p', '%m')");
+        $appender->activateOptions();
+
+        // Action!
+        $event = new LoggerLoggingEvent("LoggerAppenderPDOTest", new Logger("TEST"), LoggerLevel::getLevelError(), "testmessage");
+        $appender->append($event);
+
+        // Check
+        $db = new PDO(self::dsn);
+        $result = $db->query("SELECT * FROM unittest3");
+        self::assertFalse($result === false);
+        $row = $result->fetch(PDO::FETCH_OBJ);
+        self::assertTrue(is_object($row));
+        self::assertEquals(1, preg_match('/^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d$/', $row->ts));
+        self::assertEquals('ERROR', $row->level);
+        self::assertEquals('testmessage', $row->msg);
     }
     
+    /** Tests if log4php throws an Exception if the appender does not work. 
+     * @expectedException LoggerException
+     */
     public function testException() {
         $dsn = 'doenotexist';
         $appender = new LoggerAppenderPDO("myname");
         $appender->setDSN($dsn);
         $appender->setCreateTable(true);
-        
-        $catchedException = null;
-        try {
             $appender->activateOptions();
-        } catch (LoggerException $e) {
-            $catchedException = $e;
         }
-        self::assertNotNull($catchedException);
-    }
-    
-    public function tearDown() {
-    	@unlink('../../../target/pdotest.sqlite');
-    }
-    
 }