Merge pull request #8477 from owncloud/better-controllers

Better appframework controllers

12 files changed, 181 insertions, 34 deletions

diff --git a/lib/private/appframework/app.php b/lib/private/appframework/app.php
index 3b13194862d..baf52d02054 100644
--- a/lib/private/appframework/app.php
+++ b/lib/private/appframework/app.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/core/api.php b/lib/private/appframework/core/api.php
index e7269373bb0..c114d0be9ff 100644
--- a/lib/private/appframework/core/api.php
+++ b/lib/private/appframework/core/api.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php
index becd755bda7..97a6569a0f6 100644
--- a/lib/private/appframework/dependencyinjection/dicontainer.php
+++ b/lib/private/appframework/dependencyinjection/dicontainer.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -33,6 +33,7 @@ use OC\AppFramework\Middleware\Security\SecurityMiddleware;
 use OC\AppFramework\Middleware\Security\CORSMiddleware;
 use OC\AppFramework\Utility\SimpleContainer;
 use OC\AppFramework\Utility\TimeFactory;
+use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\AppFramework\IApi;
 use OCP\AppFramework\IAppContainer;
 use OCP\AppFramework\Middleware;
@@ -81,7 +82,12 @@ class DIContainer extends SimpleContainer implements IAppContainer{
 		});
 
 		$this['Dispatcher'] = $this->share(function($c) {
-			return new Dispatcher($c['Protocol'], $c['MiddlewareDispatcher']);
+			return new Dispatcher(
+				$c['Protocol'], 
+				$c['MiddlewareDispatcher'], 
+				$c['ControllerMethodReflector'],
+				$c['Request']
+			);
 		});
 
 
@@ -90,11 +96,18 @@ class DIContainer extends SimpleContainer implements IAppContainer{
 		 */
 		$app = $this;
 		$this['SecurityMiddleware'] = $this->share(function($c) use ($app){
-			return new SecurityMiddleware($app, $c['Request']);
+			return new SecurityMiddleware(
+				$app, 
+				$c['Request'], 
+				$c['ControllerMethodReflector']
+			);
 		});
 
 		$this['CORSMiddleware'] = $this->share(function($c) {
-			return new CORSMiddleware($c['Request']);
+			return new CORSMiddleware(
+				$c['Request'],
+				$c['ControllerMethodReflector']
+			);
 		});
 
 		$middleWares = &$this->middleWares;
@@ -118,6 +131,9 @@ class DIContainer extends SimpleContainer implements IAppContainer{
 			return new TimeFactory();
 		});
 
+		$this['ControllerMethodReflector'] = $this->share(function($c) {
+			return new ControllerMethodReflector();
+		});
 
 	}
 
diff --git a/lib/private/appframework/http.php b/lib/private/appframework/http.php
index 65d926105f1..d09e1d3ce2e 100644
--- a/lib/private/appframework/http.php
+++ b/lib/private/appframework/http.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt, Thomas Tanghus, Bart Visscher
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/http/dispatcher.php b/lib/private/appframework/http/dispatcher.php
index a2afb53f0fa..39ca3398c66 100644
--- a/lib/private/appframework/http/dispatcher.php
+++ b/lib/private/appframework/http/dispatcher.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt, Thomas Tanghus, Bart Visscher
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -26,7 +26,11 @@ namespace OC\AppFramework\Http;
 
 use \OC\AppFramework\Middleware\MiddlewareDispatcher;
 use \OC\AppFramework\Http;
+use \OC\AppFramework\Utility\ControllerMethodReflector;
+
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Response;
+use OCP\IRequest;
 
 
 /**
@@ -36,17 +40,25 @@ class Dispatcher {
 
 	private $middlewareDispatcher;
 	private $protocol;
-
+	private $reflector;
+	private $request;
 
 	/**
 	 * @param Http $protocol the http protocol with contains all status headers
 	 * @param MiddlewareDispatcher $middlewareDispatcher the dispatcher which
 	 * runs the middleware
+	 * @param ControllerMethodReflector the reflector that is used to inject
+	 * the arguments for the controller
+	 * @param IRequest $request the incoming request
 	 */
 	public function __construct(Http $protocol,
-	                            MiddlewareDispatcher $middlewareDispatcher) {
+	                            MiddlewareDispatcher $middlewareDispatcher,
+	                            ControllerMethodReflector $reflector,
+	                            IRequest $request) {
 		$this->protocol = $protocol;
 		$this->middlewareDispatcher = $middlewareDispatcher;
+		$this->reflector = $reflector;
+		$this->request = $request;
 	}
 
 
@@ -63,10 +75,13 @@ class Dispatcher {
 		$out = array(null, array(), null);
 
 		try {
+			// prefill reflector with everything thats needed for the 
+			// middlewares
+			$this->reflector->reflect($controller, $methodName);
 
 			$this->middlewareDispatcher->beforeController($controller,
 				$methodName);
-			$response = $controller->$methodName();
+			$response = $this->executeController($controller, $methodName);
 
 			// if an exception appears, the middleware checks if it can handle the
 			// exception and creates a response. If no response is created, it is
@@ -98,4 +113,70 @@ class Dispatcher {
 	}
 
 
+	/**
+	 * Uses the reflected parameters, types and request parameters to execute
+	 * the controller
+	 * @param Controller $controller the controller to be executed
+	 * @param string $methodName the method on the controller that should be executed
+	 * @return Response
+	 */
+	private function executeController($controller, $methodName) {
+		$arguments = array();
+
+		// valid types that will be casted
+		$types = array('int', 'integer', 'bool', 'boolean', 'float');
+
+		foreach($this->reflector->getParameters() as $param) {
+
+			// try to get the parameter from the request object and cast
+			// it to the type annotated in the @param annotation
+			$value = $this->request->getParam($param);
+			$type = $this->reflector->getType($param);
+			
+			// if this is submitted using GET or a POST form, 'false' should be 
+			// converted to false
+			if(($type === 'bool' || $type === 'boolean') &&
+				$value === 'false' && 
+				(
+					$this->request->method === 'GET' ||
+					strpos($this->request->getHeader('Content-Type'), 
+						'application/x-www-form-urlencoded') !== false
+				)
+			) {
+				$value = false;
+
+			} elseif(in_array($type, $types)) {
+				settype($value, $type);
+			}
+			
+			$arguments[] = $value;
+		}
+
+		$response = call_user_func_array(array($controller, $methodName), $arguments);
+
+		// format response if not of type response
+		if(!($response instanceof Response)) {
+			
+			// get format from the url format or request format parameter
+			$format = $this->request->getParam('format');
+			
+			// if none is given try the first Accept header
+			if($format === null) {
+				$header = $this->request->getHeader('Accept');
+				$formats = explode(',', $header);
+
+				if($header !== null && count($formats) > 0) {
+					$accept = strtolower(trim($formats[0]));
+					$format = str_replace('application/', '', $accept);
+				} else {
+					$format = 'json';
+				}
+			}
+
+			$response = $controller->buildResponse($response, $format);
+		}
+
+		return $response;
+	}
+
 }
diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php
index 643fa685adc..8b68ca486ff 100644
--- a/lib/private/appframework/http/request.php
+++ b/lib/private/appframework/http/request.php
@@ -3,7 +3,9 @@
  * ownCloud - Request
  *
  * @author Thomas Tanghus
+ * @author Bernhard Posselt
  * @copyright 2013 Thomas Tanghus (thomas@tanghus.net)
+ * @copyright 2014 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/middleware/middlewaredispatcher.php b/lib/private/appframework/middleware/middlewaredispatcher.php
index 598743e523f..dcb63a8e552 100644
--- a/lib/private/appframework/middleware/middlewaredispatcher.php
+++ b/lib/private/appframework/middleware/middlewaredispatcher.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/middleware/security/corsmiddleware.php b/lib/private/appframework/middleware/security/corsmiddleware.php
index e32c5d42875..dca3996ea2e 100644
--- a/lib/private/appframework/middleware/security/corsmiddleware.php
+++ b/lib/private/appframework/middleware/security/corsmiddleware.php
@@ -11,7 +11,7 @@
 
 namespace OC\AppFramework\Middleware\Security;
 
-use OC\AppFramework\Utility\MethodAnnotationReader;
+use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\IRequest;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Middleware;
@@ -25,12 +25,16 @@ use OCP\AppFramework\Middleware;
 class CORSMiddleware extends Middleware {
 
 	private $request;
+	private $reflector;
 
 	/**
 	 * @param IRequest $request
+	 * @param ControllerMethodReflector $reflector
 	 */
-	public function __construct(IRequest $request) {
+	public function __construct(IRequest $request, 
+	                            ControllerMethodReflector $reflector) {
 		$this->request = $request;
+		$this->reflector = $reflector;
 	}
 
 
@@ -46,10 +50,9 @@ class CORSMiddleware extends Middleware {
 	 */
 	public function afterController($controller, $methodName, Response $response){
 		// only react if its a CORS request and if the request sends origin and
-		$reflector = new MethodAnnotationReader($controller, $methodName);
 
 		if(isset($this->request->server['HTTP_ORIGIN']) &&
-			$reflector->hasAnnotation('CORS')) {
+			$this->reflector->hasAnnotation('CORS')) {
 
 			// allow credentials headers must not be true or CSRF is possible 
 			// otherwise
@@ -57,7 +60,7 @@ class CORSMiddleware extends Middleware {
 				if(strtolower($header) === 'access-control-allow-credentials' &&
 				   strtolower(trim($value)) === 'true') {
 					$msg = 'Access-Control-Allow-Credentials must not be '.
-					       'set to true in order to prevent CSRF';
+						   'set to true in order to prevent CSRF';
 					throw new SecurityException($msg);
 				}
 			}
diff --git a/lib/private/appframework/middleware/security/securityexception.php b/lib/private/appframework/middleware/security/securityexception.php
index e551675acdf..df37d5e3275 100644
--- a/lib/private/appframework/middleware/security/securityexception.php
+++ b/lib/private/appframework/middleware/security/securityexception.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
diff --git a/lib/private/appframework/middleware/security/securitymiddleware.php b/lib/private/appframework/middleware/security/securitymiddleware.php
index 0f160d224ad..d7e398fe445 100644
--- a/lib/private/appframework/middleware/security/securitymiddleware.php
+++ b/lib/private/appframework/middleware/security/securitymiddleware.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -25,7 +25,7 @@
 namespace OC\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Http;
-use OC\AppFramework\Utility\MethodAnnotationReader;
+use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Http\Response;
@@ -53,12 +53,20 @@ class SecurityMiddleware extends Middleware {
 	private $request;
 
 	/**
+	 * @var OC\AppFramework\Utility\ControllerMethodReflector
+	 */
+	private $reflector;
+
+	/**
 	 * @param IAppContainer $app
 	 * @param IRequest $request
+	 * @param ControllerMethodReflector $reflector
 	 */
-	public function __construct(IAppContainer $app, IRequest $request){
+	public function __construct(IAppContainer $app, IRequest $request,
+	                            ControllerMethodReflector $reflector){
 		$this->app = $app;
 		$this->request = $request;
+		$this->reflector = $reflector;
 	}
 
 
@@ -72,28 +80,25 @@ class SecurityMiddleware extends Middleware {
 	 */
 	public function beforeController($controller, $methodName){
 
-		// get annotations from comments
-		$annotationReader = new MethodAnnotationReader($controller, $methodName);
-
 		// 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());
 
 		// security checks
-		$isPublicPage = $annotationReader->hasAnnotation('PublicPage');
+		$isPublicPage = $this->reflector->hasAnnotation('PublicPage');
 		if(!$isPublicPage) {
 			if(!$this->app->isLoggedIn()) {
 				throw new SecurityException('Current user is not logged in', Http::STATUS_UNAUTHORIZED);
 			}
 
-			if(!$annotationReader->hasAnnotation('NoAdminRequired')) {
+			if(!$this->reflector->hasAnnotation('NoAdminRequired')) {
 				if(!$this->app->isAdminUser()) {
 					throw new SecurityException('Logged in user must be an admin', Http::STATUS_FORBIDDEN);
 				}
 			}
 		}
 
-		if(!$annotationReader->hasAnnotation('NoCSRFRequired')) {
+		if(!$this->reflector->hasAnnotation('NoCSRFRequired')) {
 			if(!$this->request->passesCSRFCheck()) {
 				throw new SecurityException('CSRF check failed', Http::STATUS_PRECONDITION_FAILED);
 			}
diff --git a/lib/private/appframework/utility/methodannotationreader.php b/lib/private/appframework/utility/controllermethodreflector.php
index 42060a08529..c9cdadcca4a 100644
--- a/lib/private/appframework/utility/methodannotationreader.php
+++ b/lib/private/appframework/utility/controllermethodreflector.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2014 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -28,23 +28,63 @@ namespace OC\AppFramework\Utility;
 /**
  * Reads and parses annotations from doc comments
  */
-class MethodAnnotationReader {
+class ControllerMethodReflector {
 
 	private $annotations;
+	private $types;
+	private $parameters;
+
+	public function __construct() {
+		$this->types = array();
+		$this->parameters = array();
+		$this->annotations = array();
+	}
+
 
 	/**
 	 * @param object $object an object or classname
-	 * @param string $method the method which we want to inspect for annotations
+	 * @param string $method the method which we want to inspect
 	 */
-	public function __construct($object, $method){
-		$this->annotations = array();
-
+	public function reflect($object, $method){
 		$reflection = new \ReflectionMethod($object, $method);
 		$docs = $reflection->getDocComment();
 
 		// extract everything prefixed by @ and first letter uppercase
 		preg_match_all('/@([A-Z]\w+)/', $docs, $matches);
 		$this->annotations = $matches[1];
+
+		// extract type parameter information
+		preg_match_all('/@param (?<type>\w+) \$(?<var>\w+)/', $docs, $matches);
+		$this->types = array_combine($matches['var'], $matches['type']);
+
+		// get method parameters
+		foreach ($reflection->getParameters() as $param) {
+			$this->parameters[] = $param->name;
+		}
+	}
+
+
+	/**
+	 * Inspects the PHPDoc parameters for types
+	 * @param string $parameter the parameter whose type comments should be 
+	 * parsed
+	 * @return string|null type in the type parameters (@param int $something) 
+	 * would return int or null if not existing
+	 */
+	public function getType($parameter) {
+		if(array_key_exists($parameter, $this->types)) {
+			return $this->types[$parameter];
+		} else {
+			return null;
+		}
+	}
+
+
+	/**
+	 * @return array the arguments of the method
+	 */
+	public function getParameters() {
+		return $this->parameters;
 	}
 
 
diff --git a/lib/private/appframework/utility/timefactory.php b/lib/private/appframework/utility/timefactory.php
index 2c3dd6cf5e3..a9b07a356e3 100644
--- a/lib/private/appframework/utility/timefactory.php
+++ b/lib/private/appframework/utility/timefactory.php
@@ -4,7 +4,7 @@
  * ownCloud - App Framework
  *
  * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt nukeawhale@gmail.com
+ * @copyright 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE