Merge pull request #1822 from nextcloud/restore-overwritten-services-after-errors

Restore overwritten services after errors

5 files changed, 46 insertions, 51 deletions

diff --git a/tests/lib/AppConfigTest.php b/tests/lib/AppConfigTest.php
index 520dbf66d36..c4da7507752 100644
--- a/tests/lib/AppConfigTest.php
+++ b/tests/lib/AppConfigTest.php
@@ -40,7 +40,7 @@ class AppConfigTest extends TestCase {
 		$sql->delete('appconfig');
 		$sql->execute();
 
-		$this->registerAppConfig(new \OC\AppConfig($this->connection));
+		$this->overwriteService('AppConfig', new \OC\AppConfig($this->connection));
 
 		$sql = $this->connection->getQueryBuilder();
 		$sql->insert('appconfig')
@@ -130,21 +130,10 @@ class AppConfigTest extends TestCase {
 			$sql->execute();
 		}
 
-		$this->registerAppConfig(new \OC\AppConfig(\OC::$server->getDatabaseConnection()));
+		$this->restoreService('AppConfig');
 		parent::tearDown();
 	}
 
-	/**
-	 * Register an app config object for testing purposes.
-	 *
-	 * @param \OCP\IAppConfig $appConfig
-	 */
-	protected function registerAppConfig($appConfig) {
-		\OC::$server->registerService('AppConfig', function () use ($appConfig) {
-			return $appConfig;
-		});
-	}
-
 	public function testGetApps() {
 		$config = new \OC\AppConfig(\OC::$server->getDatabaseConnection());
 
diff --git a/tests/lib/AppTest.php b/tests/lib/AppTest.php
index ac4fb913beb..ea4f328b63a 100644
--- a/tests/lib/AppTest.php
+++ b/tests/lib/AppTest.php
@@ -493,24 +493,22 @@ class AppTest extends \Test\TestCase {
 	 * @param IAppConfig $appConfig app config mock
 	 */
 	private function registerAppConfig(IAppConfig $appConfig) {
-		\OC::$server->registerService('AppConfig', function ($c) use ($appConfig) {
-			return $appConfig;
-		});
-		\OC::$server->registerService('AppManager', function (\OC\Server $c) use ($appConfig) {
-			return new \OC\App\AppManager($c->getUserSession(), $appConfig, $c->getGroupManager(), $c->getMemCacheFactory(), $c->getEventDispatcher());
-		});
+		$this->overwriteService('AppConfig', $appConfig);
+		$this->overwriteService('AppManager', new \OC\App\AppManager(
+			\OC::$server->getUserSession(),
+			$appConfig,
+			\OC::$server->getGroupManager(),
+			\OC::$server->getMemCacheFactory(),
+			\OC::$server->getEventDispatcher()
+		));
 	}
 
 	/**
 	 * Restore the original app config service.
 	 */
 	private function restoreAppConfig() {
-		\OC::$server->registerService('AppConfig', function (\OC\Server $c) {
-			return new \OC\AppConfig($c->getDatabaseConnection());
-		});
-		\OC::$server->registerService('AppManager', function (\OC\Server $c) {
-			return new \OC\App\AppManager($c->getUserSession(), $c->getAppConfig(), $c->getGroupManager(), $c->getMemCacheFactory(), $c->getEventDispatcher());
-		});
+		$this->restoreService('AppConfig');
+		$this->restoreService('AppManager');
 
 		// Remove the cache of the mocked apps list with a forceRefresh
 		\OC_App::getEnabledApps();
diff --git a/tests/lib/Share/ShareTest.php b/tests/lib/Share/ShareTest.php
index 42adee21d0c..63289529843 100644
--- a/tests/lib/Share/ShareTest.php
+++ b/tests/lib/Share/ShareTest.php
@@ -119,12 +119,6 @@ class ShareTest extends \Test\TestCase {
 		parent::tearDown();
 	}
 
-	protected function setHttpHelper($httpHelper) {
-		\OC::$server->registerService('HTTPHelper', function () use ($httpHelper) {
-			return $httpHelper;
-		});
-	}
-
 	public function testShareInvalidShareType() {
 		$message = 'Share type foobar is not valid for test.txt';
 		try {
@@ -1046,11 +1040,10 @@ class ShareTest extends \Test\TestCase {
 	 * @param string $urlHost
 	 */
 	public function testRemoteShareUrlCalls($shareWith, $urlHost) {
-		$oldHttpHelper = \OC::$server->query('HTTPHelper');
-		$httpHelperMock = $this->getMockBuilder('OC\HttpHelper')
+		$httpHelperMock = $this->getMockBuilder('OC\HTTPHelper')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->setHttpHelper($httpHelperMock);
+		$this->overwriteService('HTTPHelper', $httpHelperMock);
 
 		$httpHelperMock->expects($this->at(0))
 			->method('post')
@@ -1075,7 +1068,7 @@ class ShareTest extends \Test\TestCase {
 			->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]);
 
 		\OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith);
-		$this->setHttpHelper($oldHttpHelper);
+		$this->restoreService('HTTPHelper');
 	}
 
 	/**
@@ -1473,11 +1466,10 @@ class ShareTest extends \Test\TestCase {
 	 * Make sure that a user cannot have multiple identical shares to remote users
 	 */
 	public function testOnlyOneRemoteShare() {
-		$oldHttpHelper = \OC::$server->query('HTTPHelper');
-		$httpHelperMock = $this->getMockBuilder('OC\HttpHelper')
+		$httpHelperMock = $this->getMockBuilder('OC\HTTPHelper')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->setHttpHelper($httpHelperMock);
+		$this->overwriteService('HTTPHelper', $httpHelperMock);
 
 		$httpHelperMock->expects($this->at(0))
 			->method('post')
@@ -1502,7 +1494,7 @@ class ShareTest extends \Test\TestCase {
 			->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]);
 
 		\OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost');
-		$this->setHttpHelper($oldHttpHelper);
+		$this->restoreService('HTTPHelper');
 	}
 
 	/**
diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php
index 7ccff382357..61372d4ae90 100644
--- a/tests/lib/TestCase.php
+++ b/tests/lib/TestCase.php
@@ -102,6 +102,16 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 		return false;
 	}
 
+	public function restoreAllServices() {
+		if (!empty($this->services)) {
+			if (!empty($this->services)) {
+				foreach ($this->services as $name => $service) {
+					$this->restoreService($name);
+				}
+			}
+		}
+	}
+
 	protected function getTestTraits() {
 		$traits = [];
 		$class = $this;
@@ -132,9 +142,7 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 
 		// overwrite the command bus with one we can run ourselves
 		$this->commandBus = new QueueBus();
-		\OC::$server->registerService('AsyncCommandBus', function () {
-			return $this->commandBus;
-		});
+		$this->overwriteService('AsyncCommandBus', $this->commandBus);
 
 		$traits = $this->getTestTraits();
 		foreach ($traits as $trait) {
@@ -145,7 +153,22 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 		}
 	}
 
+	protected function onNotSuccessfulTest($e) {
+		$this->restoreAllServices();
+
+		// restore database connection
+		if (!$this->IsDatabaseAccessAllowed()) {
+			\OC::$server->registerService('DatabaseConnection', function () {
+				return self::$realDatabase;
+			});
+		}
+
+		parent::onNotSuccessfulTest($e);
+	}
+
 	protected function tearDown() {
+		$this->restoreAllServices();
+
 		// restore database connection
 		if (!$this->IsDatabaseAccessAllowed()) {
 			\OC::$server->registerService('DatabaseConnection', function () {
diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php
index 619963a2abf..a1671191ab8 100644
--- a/tests/lib/UtilTest.php
+++ b/tests/lib/UtilTest.php
@@ -87,24 +87,17 @@ class UtilTest extends \Test\TestCase {
 	function testFormatDateWithTZFromSession($offset, $expected, $expectedTimeZone) {
 		date_default_timezone_set("UTC");
 
-		$oldDateTimeFormatter = \OC::$server->query('DateTimeFormatter');
 		\OC::$server->getSession()->set('timezone', $offset);
 
 		$selectedTimeZone = \OC::$server->getDateTimeZone()->getTimeZone(1350129205);
 		$this->assertEquals($expectedTimeZone, $selectedTimeZone->getName());
 		$newDateTimeFormatter = new \OC\DateTimeFormatter($selectedTimeZone, new \OC_L10N('lib', 'en'));
-		$this->setDateFormatter($newDateTimeFormatter);
+		$this->overwriteService('DateTimeFormatter', $newDateTimeFormatter);
 
 		$result = OC_Util::formatDate(1350129205, false);
 		$this->assertEquals($expected, $result);
 
-		$this->setDateFormatter($oldDateTimeFormatter);
-	}
-
-	protected function setDateFormatter($formatter) {
-		\OC::$server->registerService('DateTimeFormatter', function ($c) use ($formatter) {
-			return $formatter;
-		});
+		$this->restoreService('DateTimeFormatter');
 	}
 
 	function testSanitizeHTML() {