Use ITimeFactory

* Inject the timefacotry so we can mock it properly in the tests.
* Extended unit tests to cover the new paths

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

4 files changed, 181 insertions, 71 deletions

diff --git a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php
index 5006ce1c2ab..8759392caea 100644
--- a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php
+++ b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php
@@ -31,6 +31,7 @@ use OCA\Files_Sharing\Tests\TestCase;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
@@ -48,33 +49,36 @@ use OCP\OCS\IDiscoveryService;
  */
 class GetSharedSecretTest extends TestCase {
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IClient */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClient */
 	private $httpClient;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IClientService */
+	/** @var  \PHPUnit_Framework_MockObject_MockObject|IClientService */
 	private $httpClientService;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IJobList */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IJobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IURLGenerator */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IURLGenerator */
 	private $urlGenerator;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers  */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers  */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
 	private $logger;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IResponse */
 	private $response;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IDiscoveryService */
 	private $discoverService;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	/** @var GetSharedSecret */
 	private $getSharedSecret;
 
@@ -92,6 +96,7 @@ class GetSharedSecretTest extends TestCase {
 		$this->logger = $this->getMockBuilder(ILogger::class)->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
 		$this->discoverService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->discoverService->expects($this->any())->method('discover')->willReturn([]);
 		$this->httpClientService->expects($this->any())->method('newClient')->willReturn($this->httpClient);
@@ -103,7 +108,8 @@ class GetSharedSecretTest extends TestCase {
 			$this->trustedServers,
 			$this->logger,
 			$this->dbHandler,
-			$this->discoverService
+			$this->discoverService,
+			$this->timeFactory
 		);
 	}
 
@@ -124,10 +130,11 @@ class GetSharedSecretTest extends TestCase {
 					$this->trustedServers,
 					$this->logger,
 					$this->dbHandler,
-					$this->discoverService
+					$this->discoverService,
+					$this->timeFactory
 				]
-			)->setMethods(['parentExecute', 'reAddJob'])->getMock();
-		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url']]);
+			)->setMethods(['parentExecute'])->getMock();
+		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
 		$this->trustedServers->expects($this->once())->method('isTrustedServer')
 			->with('url')->willReturn($isTrustedServer);
@@ -138,11 +145,22 @@ class GetSharedSecretTest extends TestCase {
 		}
 		$this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]);
 		$this->jobList->expects($this->once())->method('remove');
+
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		if ($retainBackgroundJob) {
-			$getSharedSecret->expects($this->once())->method('reAddJob');
+			$this->jobList->expects($this->once())
+				->method('add')
+				->with(
+					GetSharedSecret::class,
+					[
+						'url' => 'url',
+						'token' => 'token',
+						'created' => 42,
+					]
+				);
 		} else {
-			$getSharedSecret->expects($this->never())->method('reAddJob');
-
+			$this->jobList->expects($this->never())->method('add');
 		}
 
 		$getSharedSecret->execute($this->jobList);
@@ -163,13 +181,15 @@ class GetSharedSecretTest extends TestCase {
 	 * @param int $statusCode
 	 */
 	public function testRun($statusCode) {
-
 		$target = 'targetURL';
 		$source = 'sourceURL';
 		$token = 'token';
 
 		$argument = ['url' => $target, 'token' => $token];
 
+		$this->timeFactory->method('getTime')
+			->willReturn(42);
+
 		$this->urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('/')
 			->willReturn($source);
 		$this->httpClient->expects($this->once())->method('get')
@@ -216,7 +236,6 @@ class GetSharedSecretTest extends TestCase {
 		} else {
 			$this->assertFalse($this->invokePrivate($this->getSharedSecret, 'retainJob'));
 		}
-
 	}
 
 	public function dataTestRun() {
@@ -227,4 +246,33 @@ class GetSharedSecretTest extends TestCase {
 		];
 	}
 
+	public function testRunExpired() {
+		$target = 'targetURL';
+		$source = 'sourceURL';
+		$token = 'token';
+		$created = 42;
+
+		$argument = [
+			'url' => $target,
+			'token' => $token,
+			'created' => $created,
+		];
+
+		$this->urlGenerator->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('/')
+			->willReturn($source);
+
+		$this->timeFactory->method('getTime')
+			->willReturn($created + 2592000 + 1);
+
+		$this->trustedServers->expects($this->once())
+			->method('setServerStatus')
+			->with(
+				$target,
+				TrustedServers::STATUS_FAILURE
+			);
+
+		$this->invokePrivate($this->getSharedSecret, 'run', [$argument]);
+	}
 }
diff --git a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
index 807ee2c000a..276180e5137 100644
--- a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
+++ b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
@@ -29,6 +29,7 @@ use OCA\Federation\BackgroundJob\RequestSharedSecret;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
@@ -40,33 +41,36 @@ use Test\TestCase;
 
 class RequestSharedSecretTest extends TestCase {
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IClientService */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClientService */
 	private $httpClientService;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IClient */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClient */
 	private $httpClient;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IJobList */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IJobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IURLGenerator */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IURLGenerator */
 	private $urlGenerator;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IResponse */
 	private $response;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IDiscoveryService */
 	private $discoveryService;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | ILogger */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
 	private $logger;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	/** @var  RequestSharedSecret */
 	private $requestSharedSecret;
 
@@ -84,6 +88,7 @@ class RequestSharedSecretTest extends TestCase {
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
 		$this->discoveryService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
 		$this->logger = $this->createMock(ILogger::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
 		$this->httpClientService->expects($this->any())->method('newClient')->willReturn($this->httpClient);
@@ -95,7 +100,8 @@ class RequestSharedSecretTest extends TestCase {
 			$this->trustedServers,
 			$this->dbHandler,
 			$this->discoveryService,
-			$this->logger
+			$this->logger,
+			$this->timeFactory
 		);
 	}
 
@@ -116,10 +122,11 @@ class RequestSharedSecretTest extends TestCase {
 					$this->trustedServers,
 					$this->dbHandler,
 					$this->discoveryService,
-					$this->logger
+					$this->logger,
+					$this->timeFactory
 				]
-			)->setMethods(['parentExecute', 'reAddJob'])->getMock();
-		$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url']]);
+			)->setMethods(['parentExecute'])->getMock();
+		$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
 		$this->trustedServers->expects($this->once())->method('isTrustedServer')
 			->with('url')->willReturn($isTrustedServer);
@@ -130,10 +137,22 @@ class RequestSharedSecretTest extends TestCase {
 		}
 		$this->invokePrivate($requestSharedSecret, 'retainJob', [$retainBackgroundJob]);
 		$this->jobList->expects($this->once())->method('remove');
+
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		if ($retainBackgroundJob) {
-			$requestSharedSecret->expects($this->once())->method('reAddJob');
+			$this->jobList->expects($this->once())
+				->method('add')
+				->with(
+					RequestSharedSecret::class,
+					[
+						'url' => 'url',
+						'token' => 'token',
+						'created' => 42,
+					]
+				);
 		} else {
-			$requestSharedSecret->expects($this->never())->method('reAddJob');
+			$this->jobList->expects($this->never())->method('add');
 		}
 
 		$requestSharedSecret->execute($this->jobList);
@@ -161,6 +180,8 @@ class RequestSharedSecretTest extends TestCase {
 
 		$argument = ['url' => $target, 'token' => $token];
 
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		$this->urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('/')
 			->willReturn($source);
 		$this->httpClient->expects($this->once())->method('post')
@@ -209,4 +230,34 @@ class RequestSharedSecretTest extends TestCase {
 			[Http::STATUS_CONFLICT],
 		];
 	}
+
+	public function testRunExpired() {
+		$target = 'targetURL';
+		$source = 'sourceURL';
+		$token = 'token';
+		$created = 42;
+
+		$argument = [
+			'url' => $target,
+			'token' => $token,
+			'created' => $created,
+		];
+
+		$this->urlGenerator->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('/')
+			->willReturn($source);
+
+		$this->timeFactory->method('getTime')
+			->willReturn($created + 2592000 + 1);
+
+		$this->trustedServers->expects($this->once())
+			->method('setServerStatus')
+			->with(
+				$target,
+				TrustedServers::STATUS_FAILURE
+			);
+
+		$this->invokePrivate($this->requestSharedSecret, 'run', [$argument]);
+	}
 }
diff --git a/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
index d0529583309..ef6c7c80bfc 100644
--- a/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
+++ b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
@@ -29,8 +29,8 @@ use OC\BackgroundJob\JobList;
 use OCA\Federation\Controller\OCSAuthAPIController;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
-use OCP\AppFramework\Http;
 use OCP\AppFramework\OCS\OCSForbiddenException;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
@@ -38,24 +38,28 @@ use Test\TestCase;
 
 class OCSAuthAPIControllerTest extends TestCase {
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IRequest */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IRequest */
 	private $request;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ISecureRandom  */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ISecureRandom  */
 	private $secureRandom;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | JobList */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|JobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
 	private $logger;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
+
 	/** @var  OCSAuthAPIController */
 	private $ocsAuthApi;
 
@@ -65,31 +69,28 @@ class OCSAuthAPIControllerTest extends TestCase {
 	public function setUp() {
 		parent::setUp();
 
-		$this->request = $this->getMockBuilder('OCP\IRequest')->getMock();
-		$this->secureRandom = $this->getMockBuilder('OCP\Security\ISecureRandom')->getMock();
-		$this->trustedServers = $this->getMockBuilder('OCA\Federation\TrustedServers')
-			->disableOriginalConstructor()->getMock();
-		$this->dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')
-			->disableOriginalConstructor()->getMock();
-		$this->jobList = $this->getMockBuilder('OC\BackgroundJob\JobList')
-			->disableOriginalConstructor()->getMock();
-		$this->logger = $this->getMockBuilder('OCP\ILogger')
-			->disableOriginalConstructor()->getMock();
-
-		$this->ocsAuthApi = $this->getMockBuilder(OCSAuthAPIController::class)
-			->setConstructorArgs(
-				[
-					'federation',
-					$this->request,
-					$this->secureRandom,
-					$this->jobList,
-					$this->trustedServers,
-					$this->dbHandler,
-					$this->logger
-				]
-			)->setMethods(['getTimestamp'])->getMock();
-
-		$this->ocsAuthApi->expects($this->any())->method('getTimestamp')->willReturn($this->currentTime);
+		$this->request = $this->createMock(IRequest::class);
+		$this->secureRandom = $this->createMock(ISecureRandom::class);
+		$this->trustedServers = $this->createMock(TrustedServers::class);
+		$this->dbHandler = $this->createMock(DbHandler::class);
+		$this->jobList = $this->createMock(JobList::class);
+		$this->logger = $this->createMock(ILogger::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
+
+
+		$this->ocsAuthApi = new OCSAuthAPIController(
+			'federation',
+			$this->request,
+			$this->secureRandom,
+			$this->jobList,
+			$this->trustedServers,
+			$this->dbHandler,
+			$this->logger,
+			$this->timeFactory
+		);
+
+		$this->timeFactory->method('getTime')
+			->willReturn($this->currentTime);
 
 	}
 
@@ -157,7 +158,8 @@ class OCSAuthAPIControllerTest extends TestCase {
 					$this->jobList,
 					$this->trustedServers,
 					$this->dbHandler,
-					$this->logger
+					$this->logger,
+					$this->timeFactory
 				]
 			)->setMethods(['isValidToken'])->getMock();
 
diff --git a/apps/federation/tests/TrustedServersTest.php b/apps/federation/tests/TrustedServersTest.php
index ee63faa3ba5..5995c5e4462 100644
--- a/apps/federation/tests/TrustedServersTest.php
+++ b/apps/federation/tests/TrustedServersTest.php
@@ -29,6 +29,7 @@ namespace OCA\Federation\Tests;
 
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
@@ -71,6 +72,9 @@ class TrustedServersTest extends TestCase {
 	/** @var  \PHPUnit_Framework_MockObject_MockObject | EventDispatcherInterface */
 	private $dispatcher;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	public function setUp() {
 		parent::setUp();
 
@@ -85,6 +89,7 @@ class TrustedServersTest extends TestCase {
 		$this->jobList = $this->getMockBuilder(IJobList::class)->getMock();
 		$this->secureRandom = $this->getMockBuilder(ISecureRandom::class)->getMock();
 		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->trustedServers = new TrustedServers(
 			$this->dbHandler,
@@ -93,7 +98,8 @@ class TrustedServersTest extends TestCase {
 			$this->jobList,
 			$this->secureRandom,
 			$this->config,
-			$this->dispatcher
+			$this->dispatcher,
+			$this->timeFactory
 		);
 
 	}
@@ -114,14 +120,16 @@ class TrustedServersTest extends TestCase {
 					$this->jobList,
 					$this->secureRandom,
 					$this->config,
-					$this->dispatcher
+					$this->dispatcher,
+					$this->timeFactory
 				]
 			)
-			->setMethods(['normalizeUrl', 'updateProtocol', 'getTimestamp'])
+			->setMethods(['normalizeUrl', 'updateProtocol'])
 			->getMock();
 		$trustedServers->expects($this->once())->method('updateProtocol')
 				->with('url')->willReturn('https://url');
-		$trustedServers->expects($this->any())->method('getTimestamp')->willReturn(1234567);
+		$this->timeFactory->method('getTime')
+			->willReturn(1234567);
 		$this->dbHandler->expects($this->once())->method('addServer')->with('https://url')
 			->willReturn($success);
 
@@ -273,7 +281,8 @@ class TrustedServersTest extends TestCase {
 					$this->jobList,
 					$this->secureRandom,
 					$this->config,
-					$this->dispatcher
+					$this->dispatcher,
+					$this->timeFactory
 				]
 			)
 			->setMethods(['checkOwnCloudVersion'])