Merge pull request #8759 from owncloud/fix-phpunit

fix 8757, get rid of service locator antipattern

2 files changed, 45 insertions, 28 deletions

diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php
index ee492b8a9e5..61a2333ecee 100644
--- a/lib/private/appframework/dependencyinjection/dicontainer.php
+++ b/lib/private/appframework/dependencyinjection/dicontainer.php
@@ -83,8 +83,8 @@ class DIContainer extends SimpleContainer implements IAppContainer{
 
 		$this['Dispatcher'] = $this->share(function($c) {
 			return new Dispatcher(
-				$c['Protocol'], 
-				$c['MiddlewareDispatcher'], 
+				$c['Protocol'],
+				$c['MiddlewareDispatcher'],
 				$c['ControllerMethodReflector'],
 				$c['Request']
 			);
@@ -97,9 +97,14 @@ class DIContainer extends SimpleContainer implements IAppContainer{
 		$app = $this;
 		$this['SecurityMiddleware'] = $this->share(function($c) use ($app){
 			return new SecurityMiddleware(
-				$app, 
-				$c['Request'], 
-				$c['ControllerMethodReflector']
+				$c['Request'],
+				$c['ControllerMethodReflector'],
+				$app->getServer()->getNavigationManager(),
+				$app->getServer()->getURLGenerator(),
+				$app->getServer()->getLogger(),
+				$c['AppName'],
+				$app->isLoggedIn(),
+				$app->isAdminUser()
 			);
 		});
 
diff --git a/lib/private/appframework/middleware/security/securitymiddleware.php b/lib/private/appframework/middleware/security/securitymiddleware.php
index d7e398fe445..5b56210024d 100644
--- a/lib/private/appframework/middleware/security/securitymiddleware.php
+++ b/lib/private/appframework/middleware/security/securitymiddleware.php
@@ -30,8 +30,10 @@ use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\JSONResponse;
-use OCP\AppFramework\IAppContainer;
+use OCP\INavigationManager;
+use OCP\IURLGenerator;
 use OCP\IRequest;
+use OCP\ILogger;
 
 
 /**
@@ -42,31 +44,41 @@ use OCP\IRequest;
  */
 class SecurityMiddleware extends Middleware {
 
-	/**
-	 * @var \OCP\AppFramework\IAppContainer
-	 */
-	private $app;
-
-	/**
-	 * @var \OCP\IRequest
-	 */
+	private $navigationManager;
 	private $request;
-
-	/**
-	 * @var OC\AppFramework\Utility\ControllerMethodReflector
-	 */
 	private $reflector;
+	private $appName;
+	private $urlGenerator;
+	private $logger;
+	private $isLoggedIn;
+	private $isAdminUser;
 
 	/**
-	 * @param IAppContainer $app
 	 * @param IRequest $request
 	 * @param ControllerMethodReflector $reflector
+	 * @param INavigationManager $navigationManager
+	 * @param IURLGenerator $urlGenerator
+	 * @param ILogger $logger
+	 * @param string $appName
+	 * @param bool $isLoggedIn
+	 * @param bool $isAdminUser
 	 */
-	public function __construct(IAppContainer $app, IRequest $request,
-	                            ControllerMethodReflector $reflector){
-		$this->app = $app;
+	public function __construct(IRequest $request,
+	                            ControllerMethodReflector $reflector,
+	                            INavigationManager $navigationManager,
+	                            IURLGenerator $urlGenerator,
+	                            ILogger $logger,
+	                            $appName,
+	                            $isLoggedIn,
+	                            $isAdminUser){
+		$this->navigationManager = $navigationManager;
 		$this->request = $request;
 		$this->reflector = $reflector;
+		$this->appName = $appName;
+		$this->urlGenerator = $urlGenerator;
+		$this->logger = $logger;
+		$this->isLoggedIn = $isLoggedIn;
+		$this->isAdminUser = $isAdminUser;
 	}
 
 
@@ -82,17 +94,17 @@ class SecurityMiddleware extends Middleware {
 
 		// this will set the current navigation entry of the app, use this only
 		// for normal HTML requests and not for AJAX requests
-		$this->app->getServer()->getNavigationManager()->setActiveEntry($this->app->getAppName());
+		$this->navigationManager->setActiveEntry($this->appName);
 
 		// security checks
 		$isPublicPage = $this->reflector->hasAnnotation('PublicPage');
 		if(!$isPublicPage) {
-			if(!$this->app->isLoggedIn()) {
+			if(!$this->isLoggedIn) {
 				throw new SecurityException('Current user is not logged in', Http::STATUS_UNAUTHORIZED);
 			}
 
 			if(!$this->reflector->hasAnnotation('NoAdminRequired')) {
-				if(!$this->app->isAdminUser()) {
+				if(!$this->isAdminUser) {
 					throw new SecurityException('Logged in user must be an admin', Http::STATUS_FORBIDDEN);
 				}
 			}
@@ -126,13 +138,13 @@ class SecurityMiddleware extends Middleware {
 					array('message' => $exception->getMessage()),
 					$exception->getCode()
 				);
-				$this->app->log($exception->getMessage(), 'debug');
+				$this->logger->debug($exception->getMessage());
 			} else {
 
 				// TODO: replace with link to route
-				$url = $this->app->getServer()->getURLGenerator()->getAbsoluteURL('index.php');
+				$url = $this->urlGenerator->getAbsoluteURL('index.php');
 				$response = new RedirectResponse($url);
-				$this->app->log($exception->getMessage(), 'debug');
+				$this->logger->debug($exception->getMessage());
 			}
 
 			return $response;