FEATURE Formatting MySQL error messages with newlines through new SQLFormatter class (used in MySQLDatabase)

ENHANCEMENT Using CliDebugView to report errors on ajax requests (with plaintext output)
ENHANCEMENT Removed "ERROR:" prefix hack for ajax error responses - clientside evaluation should inspect HTTP status codes instead

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@62467 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/model/Database.php

@@ -423,7 +423,7 @@ abstract class Database extends Object {
 	 * @param int $errorLevel The level of the error to throw.
 	 */
 	function databaseError($msg, $errorLevel = E_USER_ERROR) {
-		user_error("DATABASE ERROR: $msg", $errorLevel);
+		user_error($msg, $errorLevel);
 	}
 	
 	/**

core/model/MySQLDatabase.php

@@ -385,6 +385,16 @@ class MySQLDatabase extends Database {
 	public function affectedRows() {
 		return mysql_affected_rows($this->dbConn);
 	}
+	
+	function databaseError($msg, $errorLevel = E_USER_ERROR) {
+		// try to extract and format query
+		if(preg_match('/Couldn\'t run query: ([^\|]*)\|\s*(.*)/', $msg, $matches)) {
+			$formatter = new SQLFormatter();
+			$msg = "Couldn't run query: \n" . $formatter->formatPlain($matches[1]) . "\n\n" . $matches[2];
+		}
+		
+		user_error($msg, $errorLevel);
+	}
 }
 
 /**

dev/Debug.php

@@ -284,7 +284,7 @@ class Debug {
 	 * Create an instance of an appropriate DebugView object.
 	 */
 	static function create_debug_view() {
-		if(Director::is_cli()) return new CliDebugView();
+		if(Director::is_cli() || Director::is_ajax()) return new CliDebugView();
 		else return new DebugView();
 	}
 
@@ -304,10 +304,10 @@ class Debug {
 			$errText = str_replace(array("\n","\r")," ",$errText);
 			header("HTTP/1.0 500 $errText");
 		}
-		if(Director::is_ajax()) {
+		
-			echo "ERROR:Error $errno: $errstr\n At l$errline in $errfile\n";
+		// Legacy error handling for customized prototype.js Ajax.Base.responseIsSuccess()
-			Debug::backtrace();
+		// if(Director::is_ajax()) echo "ERROR:\n";
-		} else {
+		
 		$reporter = self::create_debug_view();
 		
 		// Coupling alert: This relies on knowledge of how the director gets its URL, it could be improved.
@@ -331,7 +331,6 @@ class Debug {
 		$reporter->writeFooter();
 		exit(1);
 	}
-	}
 	
 	/**
 	 * Utility method to render a snippet of PHP source code, from selected file

parsers/SQLFormatter.php

@@ -0,0 +1,58 @@
+<?php
+/**
+ * Format a SQL Query for better readable output in HTML or Plaintext.
+ * Its a simple string parser, not a full tokenizer - so formatting
+ * is not aware of the SQL syntax. This means we have to be conservative
+ * with modifying the SQL string.
+ *
+ * @package sapphire
+ * @subpackage parsers
+ * @author Ingo Schommer, Silverstripe Ltd. (<firstname>@silverstripe.com)
+ * @usedby Database->databaseError()
+ */
+class SQLFormatter extends Object {
+	
+	protected static $newline_before_tokens = array(
+		'SELECT',
+		'UPDATE',
+		'INSERT',
+		'DELETE',
+		'FROM',
+		'INNER JOIN',
+		'FULL JOIN',
+		'LEFT JOIN',
+		'RIGHT JOIN',
+		'WHERE',
+		'ORDER BY',
+		'GROUP BY',
+		'LIMIT',
+	);
+	
+	public function formatPlain($sql) {
+		$sql = $this->addNewlines($sql, false);
+
+		return $sql;
+	}
+	
+	public function formatHTML($sql) {
+		$sql = $this->addNewlines($sql, true);
+
+		return $sql;
+	}
+	
+	/**
+	 * Newlines for tokens defined in $newline_before_tokens.
+	 * Case-sensitive, only applies to uppercase SQL to avoid
+	 * messing with possible content fragments in the query.
+	 */
+	protected function addNewlines($sql, $useHtmlFormatting = false) {
+		foreach(self::$newline_before_tokens as $token) {
+			$breakToken = ($useHtmlFormatting) ? "<br />\n" : "\n";
+			$sql = preg_replace('/[^\n](' . $token . ')/', $breakToken . '$1', $sql);
+		}
+		
+		return $sql;
+	}
+	
+}
+?>

tests/SQLFormatterTest.php

@@ -0,0 +1,41 @@
+<?php
+/**
+ * @package sapphire
+ * @subpackage tests
+ */
+class SQLFormatterTest extends SapphireTest {
+	
+	function testNewlineHanding() {
+		$formatter = new SQLFormatter();
+		
+		$sqlBefore = <<<SQL
+SELECT Test.Foo, Test.Bar FROM Test WHERE 'From' = "Where"
+SQL;
+		$sqlAfter = <<<SQL
+SELECT Test.Foo, Test.Bar
+FROM Test
+WHERE 'From' = "Where"
+SQL;
+		$this->assertEquals($formatter->formatPlain($sqlBefore), $sqlAfter,
+			'correct replacement of newlines and don\'t replace non-uppercase tokens'
+		);
+		
+		$sqlBefore = <<<SQL
+SELECT Test.Foo, Test.Bar
+FROM Test
+WHERE
+  'From' = "Where"
+SQL;
+		$sqlAfter = <<<SQL
+SELECT Test.Foo, Test.Bar
+FROM Test
+WHERE
+  'From' = "Where"
+SQL;
+		$this->assertEquals($formatter->formatPlain($sqlBefore), $sqlAfter,
+			'Leave existing newlines and indentation in place'
+		);
+	}
+	
+}
+?>