Replace session by cookie for error message

Follow up for #296

6 files changed, 62 insertions, 68 deletions

diff --git a/tests/unit/controller/FilesApiControllerTest.php b/tests/unit/controller/FilesApiControllerTest.php
index e53d179e..1891fe1c 100644
--- a/tests/unit/controller/FilesApiControllerTest.php
+++ b/tests/unit/controller/FilesApiControllerTest.php
@@ -14,6 +14,11 @@ namespace OCA\Gallery\Controller;
 
 require_once __DIR__ . '/FilesControllerTest.php';
 
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\RedirectResponse;
+
+use OCA\Gallery\Service\NotFoundServiceException;
+
 /**
  * Class FilesApiControllerTest
  *
@@ -35,4 +40,23 @@ class FilesApiControllerTest extends FilesControllerTest {
 		);
 	}
 
+	public function testDownloadWithWrongId() {
+		$fileId = 99999;
+		$filename = null;
+		$status = Http::STATUS_NOT_FOUND;
+
+		$exception = new NotFoundServiceException('Not found');
+		$this->mockGetResourceFromIdWithBadFile($this->downloadService, $fileId, $exception);
+
+		$redirectUrl = '/index.php/app/error';
+		$this->mockUrlToErrorPage($status, $redirectUrl);
+
+		/** @type RedirectResponse $response */
+		$response = $this->controller->download($fileId, $filename);
+
+		$this->assertEquals($redirectUrl, $response->getRedirectURL());
+		$this->assertEquals(Http::STATUS_TEMPORARY_REDIRECT, $response->getStatus());
+		$this->assertFalse(array_key_exists('galleryErrorMessage',$response->getCookies()));
+	}
+
 }
diff --git a/tests/unit/controller/FilesControllerTest.php b/tests/unit/controller/FilesControllerTest.php
index 76a9c7e8..00b45734 100644
--- a/tests/unit/controller/FilesControllerTest.php
+++ b/tests/unit/controller/FilesControllerTest.php
@@ -15,7 +15,6 @@ namespace OCA\Gallery\Controller;
 use OCA\Gallery\Service\ServiceException;
 use OCP\IRequest;
 use OCP\IURLGenerator;
-use OCP\ISession;
 use OCP\ILogger;
 
 use OCP\AppFramework\IAppContainer;
@@ -125,17 +124,22 @@ class FilesControllerTest extends \Test\GalleryUnitTest {
 	public function testDownloadWithWrongId() {
 		$fileId = 99999;
 		$filename = null;
+		$status = Http::STATUS_NOT_FOUND;
 
 		$exception = new NotFoundServiceException('Not found');
 		$this->mockGetResourceFromIdWithBadFile($this->downloadService, $fileId, $exception);
 
-		$redirect = new RedirectResponse(
-			$this->urlGenerator->linkToRoute($this->appName . '.page.error_page')
-		);
+		$redirectUrl = '/index.php/app/error';
+		$this->mockUrlToErrorPage($status, $redirectUrl);
 
+		/** @type RedirectResponse $response */
 		$response = $this->controller->download($fileId, $filename);
 
-		$this->assertEquals($redirect->getRedirectURL(), $response->getRedirectURL());
+		$this->assertEquals($redirectUrl, $response->getRedirectURL());
+		$this->assertEquals(Http::STATUS_TEMPORARY_REDIRECT, $response->getStatus());
+		$this->assertEquals(
+			$exception->getMessage(), $response->getCookies()['galleryErrorMessage']['value']
+		);
 	}
 
 	public function testGetFilesWithWorkingSetup() {
@@ -238,6 +242,19 @@ class FilesControllerTest extends \Test\GalleryUnitTest {
 	}
 
 	/**
+	 * Mocks IURLGenerator->linkToRoute()
+	 *
+	 * @param int $code
+	 * @param string $url
+	 */
+	protected function mockUrlToErrorPage($code, $url) {
+		$this->urlGenerator->expects($this->once())
+						   ->method('linkToRoute')
+						   ->with($this->appName . '.page.error_page', ['code' => $code])
+						   ->willReturn($url);
+	}
+
+	/**
 	 * Mocks Files->getDownload
 	 *
 	 * @param int $fileId the ID of the file of which we need a large preview of
diff --git a/tests/unit/controller/HttpErrorTest.php b/tests/unit/controller/HttpErrorTest.php
index f3476f27..473b5e45 100644
--- a/tests/unit/controller/HttpErrorTest.php
+++ b/tests/unit/controller/HttpErrorTest.php
@@ -90,25 +90,17 @@ class HttpErrorTest extends \Test\TestCase {
 	 * @param String $status
 	 */
 	public function testHtmlError($exception, $message, $status) {
-		$session = $this->mockISession();
 		$urlGenerator = $this->mockIURLGenerator();
-		$this->mockSession($session, 'galleryErrorMessage', $message);
 		$redirectUrl = '/index.php/app/error';
 		$this->mockUrlToErrorPage($urlGenerator, $status, $redirectUrl);
 
 		$httpError = $this->getMockForTrait('\OCA\Gallery\Controller\HttpError');
 
 		/** @type RedirectResponse $response */
-		$response = $httpError->htmlError($session, $urlGenerator, $this->appName, $exception);
+		$response = $httpError->htmlError($urlGenerator, $this->appName, $exception);
 		$this->assertEquals($redirectUrl, $response->getRedirectURL());
 		$this->assertEquals(Http::STATUS_TEMPORARY_REDIRECT, $response->getStatus());
-		$this->assertEquals($message, $session->get('galleryErrorMessage'));
-	}
-
-	private function mockISession() {
-		return $this->getMockBuilder('\OCP\ISession')
-					->disableOriginalConstructor()
-					->getMock();
+		$this->assertEquals($message, $response->getCookies()['galleryErrorMessage']['value']);
 	}
 
 	private function mockIURLGenerator() {
@@ -118,25 +110,6 @@ class HttpErrorTest extends \Test\TestCase {
 	}
 
 	/**
-	 * Needs to be called at least once by testDownloadWithWrongId() or the tests will fail
-	 *
-	 * @param $session
-	 * @param $key
-	 * @param $value
-	 */
-	private function mockSession($session, $key, $value) {
-		$session->expects($this->once())
-				->method('set')
-				->with($key)
-				->willReturn($value);
-
-		$session->expects($this->once())
-				->method('get')
-				->with($key)
-				->willReturn($value);
-	}
-
-	/**
 	 * Mocks IURLGenerator->linkToRoute()
 	 *
 	 * @param int $code
diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php
index 0189dfff..5eb0e6b0 100644
--- a/tests/unit/controller/PageControllerTest.php
+++ b/tests/unit/controller/PageControllerTest.php
@@ -15,7 +15,6 @@ namespace OCA\Gallery\Controller;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
-use OCP\ISession;
 
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -38,8 +37,6 @@ class PageControllerTest extends \Test\TestCase {
 	private $urlGenerator;
 	/** @var IConfig */
 	private $appConfig;
-	/** @var ISession */
-	protected $session;
 	/** @var PageController */
 	protected $controller;
 
@@ -61,16 +58,12 @@ class PageControllerTest extends \Test\TestCase {
 		$this->appConfig = $this->getMockBuilder('\OCP\IConfig')
 								->disableOriginalConstructor()
 								->getMock();
-		$this->session = $this->getMockBuilder('\OCP\ISession')
-							  ->disableOriginalConstructor()
-							  ->getMock();
 		$this->controller = new PageController(
 			$this->appName,
 			$this->request,
 			$this->environment,
 			$this->urlGenerator,
-			$this->appConfig,
-			$this->session
+			$this->appConfig
 		);
 	}
 
@@ -165,21 +158,15 @@ class PageControllerTest extends \Test\TestCase {
 	public function testErrorPage() {
 		$message = 'Not found!';
 		$code = Http::STATUS_NOT_FOUND;
-		$params = [
-			'appName' => $this->appName,
-			'message' => $message,
-			'code'    => $code,
-		];
-		$template = new TemplateResponse($this->appName, 'index', $params, 'guest');
-		$template->setStatus($code);
-		$this->mockSessionGet('galleryErrorMessage', $message);
+
+		$this->mockCookieGet('galleryErrorMessage', $message);
 
 		$response = $this->controller->errorPage($code);
 
-		$this->assertEquals($params, $response->getParams());
 		$this->assertEquals('index', $response->getTemplateName());
 		$this->assertTrue($response instanceof TemplateResponse);
-		$this->assertEquals($template->getStatus(), $response->getStatus());
+		$this->assertEquals($code, $response->getStatus());
+		$this->assertContains($message, $response->getParams()['message']);
 	}
 
 	private function mockGetSharedNode($nodeType, $nodeId) {
@@ -245,9 +232,9 @@ class PageControllerTest extends \Test\TestCase {
 	 * @param $key
 	 * @param $value
 	 */
-	private function mockSessionGet($key, $value) {
-		$this->session->expects($this->once())
-					  ->method('get')
+	private function mockCookieGet($key, $value) {
+		$this->request->expects($this->once())
+					  ->method('getCookie')
 					  ->with($key)
 					  ->willReturn($value);
 	}
diff --git a/tests/unit/middleware/EnvCheckMiddlewareTest.php b/tests/unit/middleware/EnvCheckMiddlewareTest.php
index 7f37ac65..51e48ff7 100644
--- a/tests/unit/middleware/EnvCheckMiddlewareTest.php
+++ b/tests/unit/middleware/EnvCheckMiddlewareTest.php
@@ -463,12 +463,15 @@ class EnvCheckMiddlewareTest extends \Codeception\TestCase\Test {
 		$code = Http::STATUS_NOT_FOUND;
 		$exception = new CheckException($message, $code);
 
-		$template = $this->mockHtml404Response($code);
+		$redirectUrl = '/app/error/route';
+		$this->mockHtml404Response($redirectUrl, $code);
 
 		$response =
 			$this->middleware->afterException($this->controller, 'authenticate', $exception);
 
-		$this->assertEquals($template, $response);
+		$this->assertEquals($redirectUrl, $response->getRedirectURL());
+		$this->assertEquals(Http::STATUS_TEMPORARY_REDIRECT, $response->getStatus());
+		$this->assertEquals($message, $response->getCookies()['galleryErrorMessage']['value']);
 	}
 
 	public function testAfterExceptionWithCheckExceptionAndJsonAccept() {
@@ -544,12 +547,9 @@ class EnvCheckMiddlewareTest extends \Codeception\TestCase\Test {
 		return new TemplateResponse($this->appName, 'authenticate', [], 'guest');
 	}
 
-	private function mockHtml404Response($code) {
+	private function mockHtml404Response($redirectUrl, $code) {
 		$this->mockAcceptHeader('html');
-		$redirectUrl = 'http://newroute.com';
 		$this->mockUrlToErrorPage($code, $redirectUrl);
-
-		return new RedirectResponse($redirectUrl);
 	}
 
 	private function mockJsonResponse($message, $code) {
diff --git a/tests/unit/middleware/SharingCheckMiddlewareTest.php b/tests/unit/middleware/SharingCheckMiddlewareTest.php
index a798fe6b..ff3b31b3 100644
--- a/tests/unit/middleware/SharingCheckMiddlewareTest.php
+++ b/tests/unit/middleware/SharingCheckMiddlewareTest.php
@@ -21,7 +21,6 @@ use OCP\IConfig;
 use OCP\IRequest;
 use OCP\ILogger;
 use OCP\IURLGenerator;
-use OCP\ISession;
 
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -45,8 +44,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 	protected $reflector;
 	/** @var IURLGenerator */
 	private $urlGenerator;
-	/** @var ISession */
-	private $session;
 	/** @var ILogger */
 	protected $logger;
 	/** @var Controller */
@@ -71,9 +68,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 		$this->urlGenerator = $this->getMockBuilder('\OCP\IURLGenerator')
 								   ->disableOriginalConstructor()
 								   ->getMock();
-		$this->session = $this->getMockBuilder('\OCP\ISession')
-							  ->disableOriginalConstructor()
-							  ->getMock();
 		$this->logger = $this->getMockBuilder('\OCP\ILogger')
 							 ->disableOriginalConstructor()
 							 ->getMock();
@@ -87,7 +81,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 			$this->config,
 			$this->reflector,
 			$this->urlGenerator,
-			$this->session,
 			$this->logger
 		);
 	}