Adding a more meaningful message for sabre dav exception - fixes #14516

diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php
index f631e47b5f68e0c652ef7a30684c84461a507e1d..e260f85fdb045191e28365180baddc83d25ee538 100644 (file)
--- a/apps/files/appinfo/remote.php
+++ b/apps/files/appinfo/remote.php
@@ -40,7 +40,7 @@ $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName()
 $server->addPlugin(new \OC\Connector\Sabre\DummyGetResponsePlugin());
 $server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree));
 $server->addPlugin(new \OC\Connector\Sabre\MaintenancePlugin());
-$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav'));
+$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav', \OC::$server->getLogger()));
 
 // wait with registering these until auth is handled and the filesystem is setup
 $server->on('beforeMethod', function () use ($server, $objectTree) {

diff --git a/apps/files_sharing/publicwebdav.php b/apps/files_sharing/publicwebdav.php
index 150f05a862aa4d925f85664c2890737fc98711eb..c7ee950532e7c0cb72613a42ea49a8b358815987 100644 (file)
--- a/apps/files_sharing/publicwebdav.php
+++ b/apps/files_sharing/publicwebdav.php
@@ -33,7 +33,7 @@ $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName()
 $server->addPlugin(new \Sabre\DAV\Browser\Plugin(false)); // Show something in the Browser, but no upload
 $server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree));
 $server->addPlugin(new \OC\Connector\Sabre\MaintenancePlugin());
-$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav'));
+$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav', \OC::$server->getLogger()));
 
 // wait with registering these until auth is handled and the filesystem is setup
 $server->on('beforeMethod', function () use ($server, $objectTree, $authBackend) {

diff --git a/lib/private/connector/sabre/exceptionloggerplugin.php b/lib/private/connector/sabre/exceptionloggerplugin.php
index 2bd43e56f558b37f13e166119c9e6aa46f916d82..a0dc6e7c1829d81c846fa5bbd013b973d44f7025 100644 (file)
--- a/lib/private/connector/sabre/exceptionloggerplugin.php
+++ b/lib/private/connector/sabre/exceptionloggerplugin.php
@@ -10,6 +10,10 @@
 
 namespace OC\Connector\Sabre;
 
+use OCP\ILogger;
+use Sabre\DAV\Exception;
+use Sabre\HTTP\Response;
+
 class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin {
        private $nonFatalExceptions = array(
                'Sabre\DAV\Exception\NotAuthenticated' => true,
@@ -22,13 +26,19 @@ class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin {
                'Sabre\DAV\Exception\PreconditionFailed' => true,
        );
 
+       /** @var string */
        private $appName;
 
+       /** @var ILogger */
+       private $logger;
+
        /**
         * @param string $loggerAppName app name to use when logging
+        * @param ILogger $logger
         */
-       public function __construct($loggerAppName = 'webdav') {
+       public function __construct($loggerAppName, $logger) {
                $this->appName = $loggerAppName;
+               $this->logger = $logger;
        }
 
        /**
@@ -50,14 +60,30 @@ class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin {
        /**
         * Log exception
         *
-        * @internal param Exception $e exception
         */
-       public function logException($e) {
-               $exceptionClass = get_class($e);
+       public function logException(\Exception $ex) {
+               $exceptionClass = get_class($ex);
                $level = \OCP\Util::FATAL;
                if (isset($this->nonFatalExceptions[$exceptionClass])) {
                        $level = \OCP\Util::DEBUG;
                }
-               \OCP\Util::logException($this->appName, $e, $level);
+
+               $message = $ex->getMessage();
+               if ($ex instanceof Exception) {
+                       if (empty($message)) {
+                               $response = new Response($ex->getHTTPCode());
+                               $message = $response->getStatusText();
+                       }
+                       $message = "HTTP/1.1 {$ex->getHTTPCode()} $message";
+               }
+
+               $exception = [
+                       'Message' => $message,
+                       'Code' => $ex->getCode(),
+                       'Trace' => $ex->getTraceAsString(),
+                       'File' => $ex->getFile(),
+                       'Line' => $ex->getLine(),
+               ];
+               $this->logger->log($level, 'Exception: ' . json_encode($exception), ['app' => $this->appName]);
        }
 }

diff --git a/tests/lib/connector/sabre/exceptionloggerplugin.php b/tests/lib/connector/sabre/exceptionloggerplugin.php
new file mode 100644 (file)
index 0000000..0662ba0
--- /dev/null
+++ b/tests/lib/connector/sabre/exceptionloggerplugin.php
@@ -0,0 +1,71 @@
+<?php
+
+/**
+ * Copyright (c) 2015 Thomas Müller <deepdiver@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace Test\Connector\Sabre;
+
+use OC\Connector\Sabre\Exception\InvalidPath;
+use OC\Connector\Sabre\ExceptionLoggerPlugin as PluginToTest;
+use OC\Log;
+use OCP\ILogger;
+use PHPUnit_Framework_MockObject_MockObject;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\Server;
+use Test\TestCase;
+
+class TestLogger extends Log {
+       public $message;
+       public $level;
+
+       public function __construct($logger = null) {
+               //disable original constructor
+       }
+
+       public function log($level, $message, array $context = array()) {
+               $this->level = $level;
+               $this->message = $message;
+       }
+}
+
+class ExceptionLoggerPlugin extends TestCase {
+
+       /** @var Server */
+       private $server;
+
+       /** @var PluginToTest */
+       private $plugin;
+
+       /** @var TestLogger | PHPUnit_Framework_MockObject_MockObject */
+       private $logger;
+
+       private function init() {
+               $this->server = new Server();
+               $this->logger = new TestLogger();
+               $this->plugin = new PluginToTest('unit-test', $this->logger);
+               $this->plugin->initialize($this->server);
+       }
+
+       /**
+        * @dataProvider providesExceptions
+        */
+       public function testLogging($expectedLogLevel, $expectedMessage, $exception) {
+               $this->init();
+               $this->plugin->logException($exception);
+
+               $this->assertEquals($expectedLogLevel, $this->logger->level);
+               $this->assertStringStartsWith('Exception: {"Message":"' . $expectedMessage, $this->logger->message);
+       }
+
+       public function providesExceptions() {
+               return [
+                       [0, 'HTTP\/1.1 404 Not Found', new NotFound()],
+                       [4, 'HTTP\/1.1 400 This path leads to nowhere', new InvalidPath('This path leads to nowhere')]
+               ];
+       }
+
+}

diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php
index f2812e390ac740c9b8df5e7602ea1998b9d38026..74e289c17514208a44ec60cc61a61c42160da5ed 100644 (file)
--- a/tests/lib/connector/sabre/file.php
+++ b/tests/lib/connector/sabre/file.php
@@ -8,9 +8,6 @@
 
 namespace Test\Connector\Sabre;
 
-
-use OC_Connector_Sabre_File;
-
 class File extends \Test\TestCase {
 
        /**