From ed26b251c830bd7b096cd717bc1baeb6c2d4c590 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 23 Dec 2016 15:41:41 +1300 Subject: [PATCH] ENHANCEMENT: Better output type detection for debugging --- src/Control/HTTPRequest.php | 2 +- src/Dev/CliDebugView.php | 74 ++++++++ src/Dev/Debug.php | 176 +++++++++--------- src/Dev/DebugView.php | 74 ++++++++ tests/php/Dev/CLIDebugViewTest.php | 65 +++++++ tests/php/Dev/DebugViewTest.php | 73 ++++++++ .../php/Dev/DebugViewTest/ObjectWithDebug.php | 13 ++ 7 files changed, 391 insertions(+), 86 deletions(-) create mode 100644 tests/php/Dev/CLIDebugViewTest.php create mode 100644 tests/php/Dev/DebugViewTest.php create mode 100644 tests/php/Dev/DebugViewTest/ObjectWithDebug.php diff --git a/src/Control/HTTPRequest.php b/src/Control/HTTPRequest.php index 6c54f8a2c..7912c6dc4 100644 --- a/src/Control/HTTPRequest.php +++ b/src/Control/HTTPRequest.php @@ -428,7 +428,7 @@ class HTTPRequest implements ArrayAccess { return ( $this->requestVar('ajax') || - $this->getHeader('X-Requested-With') && $this->getHeader('X-Requested-With') == "XMLHttpRequest" + $this->getHeader('X-Requested-With') === "XMLHttpRequest" ); } diff --git a/src/Dev/CliDebugView.php b/src/Dev/CliDebugView.php index 3c13b1086..fbb41b3b1 100644 --- a/src/Dev/CliDebugView.php +++ b/src/Dev/CliDebugView.php @@ -3,6 +3,8 @@ namespace SilverStripe\Dev; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Core\ClassInfo; +use SilverStripe\Core\Convert; /** * A basic HTML wrapper for stylish rendering of a developement info view. @@ -129,4 +131,76 @@ class CliDebugView extends DebugView return $output; } + + /** + * Similar to renderVariable() but respects debug() method on object if available + * + * @param mixed $val + * @param array $caller + * @param bool $showHeader + * @return string + */ + public function debugVariable($val, $caller, $showHeader = true) + { + $text = $this->debugVariableText($val); + if ($showHeader) { + $callerFormatted = $this->formatCaller($caller); + return "Debug ($callerFormatted)\n{$text}\n\n"; + } else { + return $text; + } + } + + /** + * Get debug text for this object + * + * @param mixed $val + * @return string + */ + public function debugVariableText($val) + { + // Check debug + if (ClassInfo::hasMethod($val, 'debug')) { + return $val->debug(); + } + + // Format as array + if (is_array($val)) { + $result = ''; + foreach ($val as $key => $valItem) { + $valText = $this->debugVariableText($valItem); + $result .= "$key = $valText\n"; + } + return $result; + } + + // Format object + if (is_object($val)) { + return var_export($val, true); + } + + // Format bool + if (is_bool($val)) { + return '(bool) ' . ($val ? 'true' : 'false'); + } + + // Format text + if (is_string($val)) { + return wordwrap($val, self::config()->columns); + } + + // Other + return var_export($val, true); + } + + public function renderMessage($message, $caller, $showHeader = true) + { + $header = ''; + if ($showHeader) { + $file = basename($caller['file']); + $line = $caller['line']; + $header .= "Debug (line {$line} of {$file}):\n"; + } + return $header . "{$message}\n\n"; + } } diff --git a/src/Dev/Debug.php b/src/Dev/Debug.php index 669f8f59a..069dbff88 100644 --- a/src/Dev/Debug.php +++ b/src/Dev/Debug.php @@ -3,7 +3,7 @@ namespace SilverStripe\Dev; use SilverStripe\Control\Director; -use SilverStripe\Core\Convert; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DB; use SilverStripe\Security\Permission; @@ -35,33 +35,21 @@ class Debug /** * Show the contents of val in a debug-friendly way. * Debug::show() is intended to be equivalent to dprintr() + * Does not work on live mode. * * @param mixed $val * @param bool $showHeader + * @param HTTPRequest|null $request */ - public static function show($val, $showHeader = true) + public static function show($val, $showHeader = true, HTTPRequest $request = null) { - if (!Director::isLive()) { - if ($showHeader) { - $caller = Debug::caller(); - if (Director::is_ajax() || Director::is_cli()) { - echo "Debug ($caller[class]$caller[type]$caller[function]() in " . basename($caller['file']) - . ":$caller[line])\n"; - } else { - echo "
\n
\n" - . "

Debug ($caller[class]$caller[type]$caller[function]()" - . " \nin " . basename($caller['file']) . ":$caller[line])\n

\n"; - } - } - - echo Debug::text($val); - - if (!Director::is_ajax() && !Director::is_cli()) { - echo "
"; - } else { - echo "\n\n"; - } + // Don't show on live + if (Director::isLive()) { + return; } + + echo static::create_debug_view($request) + ->debugVariable($val, static::caller(), $showHeader); } /** @@ -88,118 +76,136 @@ class Debug } /** - * Close out the show dumper + * Close out the show dumper. + * Does not work on live mode * * @param mixed $val + * @param bool $showHeader + * @param HTTPRequest $request */ - public static function endshow($val) + public static function endshow($val, $showHeader = true, HTTPRequest $request = null) { - if (!Director::isLive()) { - $caller = Debug::caller(); - echo "
\n

Debug \n($caller[class]$caller[type]$caller[function]()" - . " \nin " . basename($caller['file']) . ":$caller[line])\n

\n"; - echo Debug::text($val); - die(); + // Don't show on live + if (Director::isLive()) { + return; } + + echo static::create_debug_view($request) + ->debugVariable($val, static::caller(), $showHeader); + + die(); } /** * Quick dump of a variable. + * Note: This method will output in live! * * @param mixed $val + * @param HTTPRequest $request Current request to influence output format */ - public static function dump($val) + public static function dump($val, HTTPRequest $request = null) { - echo self::create_debug_view()->renderVariable($val, self::caller()); + echo self::create_debug_view($request) + ->renderVariable($val, self::caller()); } /** + * Get debug text for this object + * * @param mixed $val + * @param HTTPRequest $request * @return string */ - public static function text($val) + public static function text($val, HTTPRequest $request = null) { - if (is_object($val)) { - if (method_exists($val, 'hasMethod')) { - $hasDebugMethod = $val->hasMethod('debug'); - } else { - $hasDebugMethod = method_exists($val, 'debug'); - } - - if ($hasDebugMethod) { - return $val->debug(); - } - } - - if (is_array($val)) { - $result = "\n"; - } elseif (is_object($val)) { - $val = var_export($val, true); - } elseif (is_bool($val)) { - $val = $val ? 'true' : 'false'; - $val = '(bool) ' . $val; - } else { - if (!Director::is_cli() && !Director::is_ajax()) { - $val = "
" . htmlentities($val, ENT_COMPAT, 'UTF-8')
-                    . "
\n"; - } - } - - return $val; + return static::create_debug_view($request) + ->debugVariableText($val); } /** - * Show a debugging message + * Show a debugging message. + * Does not work on live mode * * @param string $message * @param bool $showHeader + * @param HTTPRequest|null $request */ - public static function message($message, $showHeader = true) + public static function message($message, $showHeader = true, HTTPRequest $request = null) { - if (!Director::isLive()) { - $caller = Debug::caller(); - $file = basename($caller['file']); - if (Director::is_cli()) { - if ($showHeader) { - echo "Debug (line $caller[line] of $file):\n "; - } - echo $message . "\n"; - } else { - echo "

\n"; - if ($showHeader) { - echo "Debug (line $caller[line] of $file):\n "; - } - echo Convert::raw2xml($message) . "

\n"; - } + // Don't show on live + if (Director::isLive()) { + return; } + + echo static::create_debug_view($request) + ->renderMessage($message, static::caller(), $showHeader); } /** * Create an instance of an appropriate DebugView object. * + * @param HTTPRequest $request Optional request to target this view for * @return DebugView */ - public static function create_debug_view() + public static function create_debug_view(HTTPRequest $request = null) { - $service = Director::is_cli() || Director::is_ajax() - ? CliDebugView::class - : DebugView::class; + $service = static::supportsHTML($request) + ? DebugView::class + : CliDebugView::class; return Injector::inst()->get($service); } + /** + * Determine if the given request supports html output + * + * @param HTTPRequest $request + * @return bool + */ + protected static function supportsHTML(HTTPRequest $request = null) + { + // No HTML output in CLI + if (Director::is_cli()) { + return false; + } + + // Get current request if registered + if (!$request && Injector::inst()->has(HTTPRequest::class)) { + $request = Injector::inst()->get(HTTPRequest::class); + } + if (!$request) { + return false; + } + // Request must include text/html + $accepted = $request->getAcceptMimetypes(false); + + // Explicit opt in + if (in_array('text/html', $accepted)) { + return true; + }; + + // Implicit opt-out + if (in_array('application/json', $accepted)) { + return false; + } + + // Fallback to wildcard comparison + if (in_array('*/*', $accepted)) { + return true; + } + return false; + } + /** * Check if the user has permissions to run URL debug tools, * else redirect them to log in. */ public static function require_developer_login() { + // Don't require login for dev mode if (Director::isDev()) { return; } + if (isset($_SESSION['loggedInAs'])) { // We have to do some raw SQL here, because this method is called in Object::defineMethods(). // This means we have to be careful about what objects we create, as we don't want Object::defineMethods() diff --git a/src/Dev/DebugView.php b/src/Dev/DebugView.php index 64bf4243a..0747eedc7 100644 --- a/src/Dev/DebugView.php +++ b/src/Dev/DebugView.php @@ -5,6 +5,7 @@ namespace SilverStripe\Dev; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injectable; @@ -378,4 +379,77 @@ class DebugView return $output; } + + public function renderMessage($message, $caller, $showHeader = true) + { + $header = ''; + if ($showHeader) { + $file = basename($caller['file']); + $line = $caller['line']; + $header .= "Debug (line {$line} of {$file}):\n"; + } + return "

\n" . $header . Convert::raw2xml($message) . "

\n"; + } + + /** + * Similar to renderVariable() but respects debug() method on object if available + * + * @param mixed $val + * @param array $caller + * @param bool $showHeader + * @return string + */ + public function debugVariable($val, $caller, $showHeader = true) + { + $text = $this->debugVariableText($val); + + if ($showHeader) { + $callerFormatted = $this->formatCaller($caller); + return "
\n
\n" + . "

Debug ($callerFormatted)\n

\n" + . $text + . "
"; + } else { + return $text; + } + } + + /** + * Get debug text for this object + * + * @param mixed $val + * @return string + */ + public function debugVariableText($val) + { + // Check debug + if (ClassInfo::hasMethod($val, 'debug')) { + return $val->debug(); + } + + // Format as array + if (is_array($val)) { + $result = ''; + foreach ($val as $key => $valueItem) { + $keyText = Convert::raw2xml($key); + $valueText = $this->debugVariableText($valueItem); + $result .= "
  • {$keyText} = {$valueText}
  • \n"; + } + return "\n"; + } + + // Format object + if (is_object($val)) { + return var_export($val, true); + } + + // Format bool + if (is_bool($val)) { + return '(bool) ' . ($val ? 'true' : 'false'); + } + + // Format text + $html = Convert::raw2xml($val); + return "
    {$html}
    \n"; + } } diff --git a/tests/php/Dev/CLIDebugViewTest.php b/tests/php/Dev/CLIDebugViewTest.php new file mode 100644 index 000000000..8729def03 --- /dev/null +++ b/tests/php/Dev/CLIDebugViewTest.php @@ -0,0 +1,65 @@ +caller = [ + 'line' => 17, + 'file' => __FILE__, + 'args' => [], + 'type' => '->', + 'class' => __CLASS__, + 'function' => __FUNCTION__, + ]; + } + + public function testDebugVariable() + { + $view = new CliDebugView(); + $this->assertEquals( + <<debugVariable('string', $this->caller) + ); + + $this->assertEquals( + <<debugVariable([ 'key' => 'value', 'another' => 'text' ], $this->caller) + ); + + $this->assertEquals( + <<debugVariable(new ObjectWithDebug(), $this->caller) + ); + } +} diff --git a/tests/php/Dev/DebugViewTest.php b/tests/php/Dev/DebugViewTest.php new file mode 100644 index 000000000..e9d296c39 --- /dev/null +++ b/tests/php/Dev/DebugViewTest.php @@ -0,0 +1,73 @@ +caller = [ + 'line' => 17, + 'file' => __FILE__, + 'args' => [], + 'type' => '->', + 'class' => __CLASS__, + 'function' => __FUNCTION__, + ]; + } + + public function testDebugVariable() + { + $view = new DebugView(); + $this->assertEquals( + << +
    +

    Debug (DebugViewTest.php:17 - SilverStripe\Dev\Tests\DebugViewTest::setUp()) +

    +
    string
    + +EOS + , + $view->debugVariable('string', $this->caller) + ); + + $this->assertEquals( + << +
    +

    Debug (DebugViewTest.php:17 - SilverStripe\Dev\Tests\DebugViewTest::setUp()) +

    +
      +
    • key =
      value
      +
    • +
    • another =
      text
      +
    • +
    + +EOS + , + $view->debugVariable([ 'key' => 'value', 'another' => 'text' ], $this->caller) + ); + + $this->assertEquals( + << +
    +

    Debug (DebugViewTest.php:17 - SilverStripe\Dev\Tests\DebugViewTest::setUp()) +

    +SilverStripe\Dev\Tests\DebugViewTest\ObjectWithDebug::debug() custom content +EOS + , + $view->debugVariable(new ObjectWithDebug(), $this->caller) + ); + } +} diff --git a/tests/php/Dev/DebugViewTest/ObjectWithDebug.php b/tests/php/Dev/DebugViewTest/ObjectWithDebug.php new file mode 100644 index 000000000..bafd17100 --- /dev/null +++ b/tests/php/Dev/DebugViewTest/ObjectWithDebug.php @@ -0,0 +1,13 @@ +