fixed various status code related issues and refactored redirect supp…

…ort (fixes FriendsOfSymfony#64)
mwoerlein · Jun 8, 2011 · 853190f · 853190f
1 parent e279ad4
commit 853190f
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 37 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -40,6 +40,7 @@ public function getConfigTreeBuilder()
 
         $rootNode
             ->fixXmlConfig('format', 'formats')
+            ->fixXmlConfig('force_redirect', 'force_redirects')
             ->fixXmlConfig('normalizer', 'normalizers')
             ->fixXmlConfig('default_normalizer', 'default_normalizers')
             ->fixXmlConfig('class', 'classes')
@@ -54,6 +55,10 @@ public function getConfigTreeBuilder()
                     ->prototype('scalar')->end()
                 ->end()
                 ->scalarNode('fallback_normalizer')->defaultValue('fos_rest.noop_normalizer')->end()
+                ->arrayNode('force_redirects')
+                    ->useAttributeAsKey('name')
+                    ->prototype('scalar')->end()
+                ->end()
                 ->arrayNode('normalizers')
                     ->useAttributeAsKey('name')
                     ->prototype('scalar')->end()

diff --git a/DependencyInjection/FOSRestExtension.php b/DependencyInjection/FOSRestExtension.php
@@ -41,6 +41,9 @@ public function load(array $configs, ContainerBuilder $container)
             'default_normalizers' => array(
                 'fos_rest.constraint_violation_normalizer',
             ),
+            'force_redirects' => array(
+                'html'  => true,
+            ),
         ));
 
         $processor = new Processor();
@@ -72,6 +75,8 @@ public function load(array $configs, ContainerBuilder $container)
         }
         $container->setParameter($this->getAlias().'.fallback_normalizer', $config['fallback_normalizer']);
 
+        $container->setParameter($this->getAlias().'.force_redirects', $config['force_redirects']);
+
         foreach ($config['exception']['codes'] as $exception => $code) {
             if (is_string($code)) {
                 $config['exception']['codes'][$exception] = constant("\FOS\RestBundle\Response\Codes::$code");

diff --git a/README.md b/README.md
@@ -84,7 +84,10 @@ Registering a custom encoder requires modifying your configuration options.
 Following is an example adding support for a custom RSS encoder while removing
 support for xml.
 
-Also the default JSON encoder class is modified and a custom serializer service
+When using View::setResourceRoute() the default behavior of forcing
+a redirect to the route for html is disabled.
+
+The default JSON encoder class is modified and a custom serializer service
 is configured.
 
 The a normalizer is registered for the class ``Acme\HelloBundle\Document\Article``
@@ -101,6 +104,8 @@ Finally the HTTP response status code for failed validation is set to ``400``:
         formats:
             rss: my.encoder.rss
             xml: false
+        force_redirects:
+            html: false
         normalizers:
             "Acme\HelloBundle\Document\Article": "my.article_normalizer"
         default_normalizers:

diff --git a/Resources/config/view.xml b/Resources/config/view.xml
@@ -43,6 +43,8 @@
         <service id="fos_rest.view" class="%fos_rest.view.class%">
             <argument>%fos_rest.formats%</argument>
             <argument>%fos_rest.failed_validation%</argument>
+            <argument>%fos_rest.default_form_key</argument>
+            <argument>%fos_rest.force_redirects%</argument>
             <call method="setContainer">
                 <argument type="service" id="service_container" />
             </call>

diff --git a/View/View.php b/View/View.php
@@ -61,9 +61,14 @@ class View implements ContainerAwareInterface
     protected $failedValidation;
 
     /**
-     * @var array redirect configuration
+     * @var array target uri
      */
-    protected $redirect;
+    protected $location;
+
+    /**
+     * @var Boolean if to force a redirect for the given format (key) for an Codes::HTTP_CREATED
+     */
+    protected $forceRedirects;
 
     /**
      * @var string|TemplateReference template
@@ -106,12 +111,14 @@ class View implements ContainerAwareInterface
      * @param array $formats The supported formats
      * @param int $failedValidation The HTTP response status code for a failed validation
      * @param string $defaultFormKey The default parameter form key
+     * @param array $forceRedirects For which formats to force redirection even for targets without a 3xx status code
      */
     public function __construct(array $formats = null, $failedValidation = Codes::HTTP_BAD_REQUEST, $defaultFormKey = 'form')
     {
         $this->formats = (array)$formats;
         $this->failedValidation = $failedValidation;
         $this->defaultFormKey = $defaultFormKey;
+        $this->forceRedirects = (array)$forceRedirects;
     }
 
     /**
@@ -124,7 +131,7 @@ public function reset()
         $this->format = null;
         $this->engine = 'twig';
         $this->parameters = array();
-        $this->code = null;
+        $this->code = Codes::HTTP_OK;
         $this->formKey = $this->defaultFormKey;
     }
 
@@ -172,13 +179,23 @@ public function registerHandler($format, $callback)
      * @param array $parameters route parameters
      * @param int $code HTTP status code
      */
-    public function setRouteRedirect($route, array $parameters = array(), $code = Codes::HTTP_FOUND)
+    public function setResourceRoute($route, array $parameters = array(), $code = Codes::HTTP_CREATED)
     {
-        $this->redirect = array(
-            'route' => $route,
-            'parameters' => $parameters,
-            'status_code' => $code,
-        );
+        $this->setLocation($this->container->get('router')->generate($route, $parameters, true));
+        $this->setStatusCode($code);
+    }
+
+    /**
+     * Sets a redirect using a route and parameters
+     *
+     * @param string $route route name
+     * @param array $parameters route parameters
+     * @param int $code HTTP status code
+     */
+    public function setRedirectRoute($route, array $parameters = array(), $code = Codes::HTTP_FOUND)
+    {
+        $this->setLocation($this->container->get('router')->generate($route, $parameters, true));
+        $this->setStatusCode($code);
     }
 
     /**
@@ -187,9 +204,36 @@ public function setRouteRedirect($route, array $parameters = array(), $code = Co
      * @param string $uri URI
      * @param int $code HTTP status code
      */
-    public function setUriRedirect($uri, $code = Codes::HTTP_FOUND)
+    public function setRedirectUri($uri, $code = Codes::HTTP_FOUND)
+    {
+        $this->setLocation($uri);
+        $this->setStatusCode($code);
+    }
+
+    /**
+     * Sets target location to use when recreating a response
+     *
+     * @param string $location target uri
+     *
+     * @throws \InvalidArgumentException if the location is empty
+     */
+    public function setLocation($location)
+    {
+        if (empty($location)) {
+            throw new \InvalidArgumentException('Cannot redirect to an empty URL.');
+        }
+
+        $this->location = $location;
+    }
+
+    /**
+     * Gets target to use for creating the response
+     *
+     * @return string target uri
+     */
+    public function getLocation()
     {
-        $this->redirect = array('location' => $uri, 'status_code' => $code);
+        return $this->location;
     }
 
     /**
@@ -203,7 +247,7 @@ public function setStatusCode($code)
     }
 
     /**
-     * Sets a response HTTP status code for a failed validation
+     * Sets the response HTTP status code for a failed validation
      */
     public function setFailedValidationStatusCode()
     {
@@ -248,7 +292,7 @@ private function getStatusCodeFromParameters()
             if (null === $this->formKey){
                 foreach ($parameters as $key => $parameter) {
                     if ($parameter instanceof FormInterface) {
-                        $this->formKey = $key;
+                        $this->setFormKey($key);
                         $form = $parameter;
                         break;
                     }
@@ -262,22 +306,12 @@ private function getStatusCodeFromParameters()
             if (isset($form)) {
                 // Check if the form is valid, return an appropriate response code
                 if ($form->isBound() && !$form->isValid()) {
-                    return $this->failedValidation;
+                    $this->setFailedValidationStatusCode();
                 }
             }
         }
 
-        return Codes::HTTP_OK;
-    }
-
-    /**
-     * Gets a redirect
-     *
-     * @return array redirect location and status code
-     */
-    public function getRedirect()
-    {
-        return $this->redirect;
+        return $this->getStatusCode();
     }
 
     /**
@@ -421,12 +455,14 @@ public function handle(Request $request = null, Response $response = null)
             $request = $this->container->get('request');
         }
 
+        $code = $this->getStatusCode();
         if (null === $response) {
-            $code = $this->getStatusCode();
             if (null === $code) {
                 $code = $this->getStatusCodeFromParameters();
             }
             $response = new Response('' , $code);
+        } else {
+            $response->setStatusCode($code);
         }
 
         $format = $this->getFormat();
@@ -456,7 +492,7 @@ public function handle(Request $request = null, Response $response = null)
     /**
      * Generic transformer
      *
-     * Handles redirects, or transforms the parameters into a response content
+     * Handles target and parameter transformation into a response
      *
      * @param Request $request
      * @param Response $response
@@ -466,19 +502,24 @@ public function handle(Request $request = null, Response $response = null)
      */
     protected function transform(Request $request, Response $response, $format)
     {
-        if ($this->redirect) {
-            // TODO add support to optionally return the target url
-            if (empty($this->redirect['location'])) {
-                // TODO add support to optionally forward to the route
-                $this->redirect['location'] = $this->container->get('router')->generate($this->redirect['route'], $this->redirect['parameters']);
+        $parameters = $this->getParameters();
+
+        $location = $this->getLocation();
+        if ($location) {
+            if (!empty($this->forceRedirects[$format]) && !$response->isRedirect()) {
+                $response->setStatusCode(Codes::HTTP_FOUND);
             }
-            $redirect = new RedirectResponse($this->redirect['location'], $this->redirect['status_code']);
-            $response->setContent($redirect->getContent());
-            $response->headers->set('Location', $redirect->headers->get('Location'));
+
+            if ('html' === $format && $response->isRedirect()) {
+                $redirect = new RedirectResponse($location, $response->getStatusCode());
+                $response->setContent($redirect->getContent());
+            }
+
+            $response->headers->set('Location', $location);
+
             return $response;
         }
 
-        $parameters = $this->getParameters();
         $serializer = $this->getSerializer();
         $encoder = $serializer->getEncoder($format);