Merge pull request #82 from symfony-cmf/file-upload

handle file upload errors
symfony-cmf · Apr 17, 2014 · 747a311 · 747a311
2 parents 6bb1b5d + 708f86d
commit 747a311
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 67 deletions.
diff --git a/File/UploadFileHelperDoctrine.php b/File/UploadFileHelperDoctrine.php
@@ -49,7 +49,7 @@ public function __construct(
     {
         $this->managerRegistry = $registry;
         $this->managerName     = $managerName;
-        $this->class           = $class === '' ? null : $class;
+        $this->setClass($class);
         $this->rootPath        = $rootPath;
         $this->mediaManager    = $mediaManager;
     }
@@ -81,6 +81,18 @@ public function setManagerName($managerName)
      */
     public function setClass($class)
     {
+        if (empty($class)) {
+            $this->class = null;
+
+            return;
+        }
+
+        if (!is_subclass_of($class, 'Symfony\Cmf\Bundle\MediaBundle\FileInterface')) {
+            throw new \InvalidArgumentException(sprintf(
+                'The class "%s" does not implement Symfony\Cmf\Bundle\MediaBundle\FileInterface',
+                $class
+            ));
+        }
         $this->class = $class;
     }
 
@@ -131,56 +143,60 @@ public function getEditorHelper($name = null)
      *
      * @param UploadedFile $file
      *
-     * @return bool
+     * @return boolean true either returns true or throws an exception
+     *
+     * @throws UploadException if the upload failed for some reason.
      */
     protected function validateFile(UploadedFile $file)
     {
         if ($file->isValid()) {
             return true;
         }
 
-        switch ($file->getError()) {
-            case UPLOAD_ERR_INI_SIZE:
-                $message = "The uploaded file exceeds the upload_max_filesize directive in php.ini";
-                break;
-            case UPLOAD_ERR_FORM_SIZE:
-                $message = "The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form";
-                break;
-            case UPLOAD_ERR_PARTIAL:
-                $message = "The uploaded file was only partially uploaded";
-                break;
-            case UPLOAD_ERR_NO_FILE:
-                $message = "No file was uploaded";
-                break;
-            case UPLOAD_ERR_NO_TMP_DIR:
-                $message = "Missing a temporary folder";
-                break;
-            case UPLOAD_ERR_CANT_WRITE:
-                $message = "Failed to write file to disk";
-                break;
-            case UPLOAD_ERR_EXTENSION:
-                $message = "File upload stopped by extension";
-                break;
-            case UPLOAD_ERR_OK:
-                $message = "The file likely did not pass the is_uploaded_file() check";
-                break;
-            default:
-                $message = sprintf('Unknown upload error : \"%s\"', $file->getError());
-                break;
-        }
+        throw new UploadException($this->getErrorMessage($file));
+    }
 
-        throw new UploadException($message);
+    /**
+     * Returns an informative upload error message.
+     *
+     * Copied from UploadedFile because its only public since 2.4
+     *
+     * @param UploadedFile $file The file with the error.
+     *
+     * @return string The error message regarding the specified error code
+     */
+    private function getErrorMessage(UploadedFile $file)
+    {
+        $errorCode = $file->getError();
+        static $errors = array(
+            UPLOAD_ERR_INI_SIZE   => 'The file "%s" exceeds your upload_max_filesize ini directive (limit is %d kb).',
+            UPLOAD_ERR_FORM_SIZE  => 'The file "%s" exceeds the upload limit defined in your form.',
+            UPLOAD_ERR_PARTIAL    => 'The file "%s" was only partially uploaded.',
+            UPLOAD_ERR_NO_FILE    => 'No file was uploaded.',
+            UPLOAD_ERR_CANT_WRITE => 'The file "%s" could not be written on disk.',
+            UPLOAD_ERR_NO_TMP_DIR => 'File could not be uploaded: missing temporary directory.',
+            UPLOAD_ERR_EXTENSION  => 'File upload was stopped by a PHP extension.',
+        );
+
+        $maxFilesize = $errorCode === UPLOAD_ERR_INI_SIZE ? $file->getMaxFilesize() / 1024 : 0;
+        $message = isset($errors[$errorCode]) ? $errors[$errorCode] : 'The file "%s" was not uploaded due to an unknown error.';
+
+        return sprintf($message, $file->getClientOriginalName(), $maxFilesize);
     }
 
     /**
      * {@inheritDoc}
      */
-    public function handleUploadedFile(UploadedFile $uploadedFile)
+    public function handleUploadedFile(UploadedFile $uploadedFile, $class = null)
     {
         $this->validateFile($uploadedFile);
 
+        $class = $class ?: $this->class;
         /** @var $file FileInterface */
-        $file = new $this->class();
+        $file = new $class();
+        if (!$file instanceof FileInterface) {
+            throw new UploadException(sprintf('Invalid class %s specified', $class));
+        }
         $file->setName($uploadedFile->getClientOriginalName());
         $file->copyContentFromFile($uploadedFile);
 
@@ -198,7 +214,6 @@ public function handleUploadedFile(UploadedFile $uploadedFile)
      */
     public function getUploadResponse(Request $request, array $uploadedFiles = array())
     {
-        /** @var \Symfony\Cmf\Bundle\MediaBundle\Editor\EditorHelperInterface $editorHelper */
         $editorHelper = $this->getEditorHelper($request->get('editor', 'default'));
 
         if (! $editorHelper) {

diff --git a/File/UploadFileHelperInterface.php b/File/UploadFileHelperInterface.php
@@ -38,22 +38,26 @@ public function addEditorHelper($name, UploadEditorHelperInterface $helper);
     /**
      * Get helper
      *
-     * @param $name leave null to get the default helper
+     * @param string $name leave null to get the default helper
      *
      * @return UploadEditorHelperInterface|null
      */
     public function getEditorHelper($name = null);
 
     /**
-     * Handle the UploadedFile and create a FileInterface object specified by
-     * the configured class.
+     * Handle the UploadedFile and create a FileInterface object.
+     *
+     * If $class is specified, an instance of that class should be created if
+     * possible. If $class is null, the implementation chooses a suitable
+     * class, e.g. through configuration.
      *
      * @param Request      $request
      * @param UploadedFile $uploadedFile
+     * @param string       $class        Optional class name for the file class to generate.
      *
      * @return FileInterface
      */
-    public function handleUploadedFile(UploadedFile $uploadedFile);
+    public function handleUploadedFile(UploadedFile $uploadedFile, $class = null);
 
     /**
      * Process upload and get a response
@@ -65,4 +69,4 @@ public function handleUploadedFile(UploadedFile $uploadedFile);
      * @return Response
      */
     public function getUploadResponse(Request $request, array $uploadedFiles = array());
-}
+}
diff --git a/Form/DataTransformer/ModelToFileTransformer.php b/Form/DataTransformer/ModelToFileTransformer.php
@@ -12,28 +12,27 @@
 
 namespace Symfony\Cmf\Bundle\MediaBundle\Form\DataTransformer;
 
+use Symfony\Cmf\Bundle\MediaBundle\File\UploadFileHelperInterface;
 use Symfony\Cmf\Bundle\MediaBundle\FileInterface;
 use Symfony\Component\Form\DataTransformerInterface;
+use Symfony\Component\Form\Exception\TransformationFailedException;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
+use Symfony\Component\HttpFoundation\File\Exception\UploadException;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 
 class ModelToFileTransformer implements DataTransformerInterface
 {
-    private $dataClass;
+    private $helper;
+    private $class;
 
     /**
-     * @param string $class
+     * @param UploadFileHelperInterface $helper
+     * @param string                    $class  Optional class to overwrite generated file class.
      */
-    public function __construct($class)
+    public function __construct(UploadFileHelperInterface $helper, $class = null)
     {
-        if (!is_subclass_of($class, 'Symfony\Cmf\Bundle\MediaBundle\FileInterface')) {
-            throw new \InvalidArgumentException(sprintf(
-                'The class "%s" does not implement Symfony\Cmf\Bundle\MediaBundle\FileInterface',
-                $class
-            ));
-        }
-
-        $this->dataClass = $class;
+        $this->helper = $helper;
+        $this->class = $class;
     }
 
     /**
@@ -45,12 +44,11 @@ public function reverseTransform($uploadedFile)
             return $uploadedFile;
         }
 
-        /** @var $file FileInterface */
-        $file = new $this->dataClass;
-        $file->copyContentFromFile($uploadedFile);
-        $file->setName($uploadedFile->getClientOriginalName());
-
-        return $file;
+        try {
+            return $this->helper->handleUploadedFile($uploadedFile, $this->class);
+        } catch(UploadException $e) {
+            throw new TransformationFailedException($e->getMessage(), $e->getCode(), $e);
+        }
     }
 
     /**
@@ -60,4 +58,4 @@ public function transform($file)
     {
         return $file;
     }
-}
+}
diff --git a/Form/Type/ImageType.php b/Form/Type/ImageType.php
@@ -12,6 +12,8 @@
 
 namespace Symfony\Cmf\Bundle\MediaBundle\Form\Type;
 
+use Symfony\Cmf\Bundle\MediaBundle\File\UploadFileHelperDoctrine;
+use Symfony\Cmf\Bundle\MediaBundle\File\UploadFileHelperInterface;
 use Symfony\Cmf\Bundle\MediaBundle\Form\DataTransformer\ModelToFileTransformer;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -22,17 +24,20 @@
 class ImageType extends AbstractType
 {
     private $dataClass;
+    private $uploadFileHelper;
     private $useImagine;
     private $defaultFilter;
 
     /**
-     * @param string $class
-     * @param bool   $useImagine
-     * @param bool   $defaultFilter
+     * @param string                    $class
+     * @param UploadFileHelperInterface $uploadFileHelper
+     * @param bool                      $useImagine
+     * @param bool                      $defaultFilter
      */
-    public function __construct($class, $useImagine = false, $defaultFilter = false)
+    public function __construct($class, UploadFileHelperInterface $uploadFileHelper, $useImagine = false, $defaultFilter = false)
     {
         $this->dataClass = $class;
+        $this->uploadFileHelper = $uploadFileHelper;
         $this->useImagine = $useImagine;
         $this->defaultFilter = $this->useImagine ? $defaultFilter : false;
     }
@@ -49,7 +54,7 @@ public function getName()
 
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
-        $transformer = new ModelToFileTransformer($options['data_class']);
+        $transformer = new ModelToFileTransformer($this->uploadFileHelper, $options['data_class']);
         $builder->addModelTransformer($transformer);
     }
 

diff --git a/Resources/config/persistence-phpcr.xml b/Resources/config/persistence-phpcr.xml
@@ -123,6 +123,7 @@
         <service id="cmf_media.form.type.image" class="%cmf_media.form.image.class%">
             <tag name="form.type" alias="cmf_media_image" />
             <argument>%cmf_media.persistence.phpcr.image.class%</argument>
+            <argument type="service" id="cmf_media.upload_file_helper" />
             <argument>%cmf_media.use_imagine%</argument>
             <argument>%cmf_media.imagine.filter.upload_thumbnail%</argument>
         </service>

diff --git a/Resources/views/Form/fields.html.twig b/Resources/views/Form/fields.html.twig
@@ -1,8 +1,8 @@
 {% block cmf_media_image_widget %}
     {{ form_widget(form) }}
     {% block cmf_media_image_widget_preview %}
-        {% if form.vars.data and form.vars.data.id %}
+        {% if form.vars.data and form.vars.data.id is defined and form.vars.data.id %}
                 <img src="{{ cmf_media_display_url(form.vars.data, { imagine_filter : imagine_filter }) }}" alt="" {% if not imagine_filter %}width="100"{% endif %} />
         {% endif %}
     {% endblock %}
-{% endblock %}
+{% endblock %}
diff --git a/Tests/Unit/File/UploadFileHelperTest.php b/Tests/Unit/File/UploadFileHelperTest.php
@@ -97,14 +97,14 @@ public function provideHandleUploadedFile()
                 'upload_error' => UPLOAD_ERR_OK,
                 'expected_exception' => array(
                     'symfony\component\httpfoundation\file\exception\uploadexception',
-                    'The file likely did not pass the is_uploaded_file() check',
+                    'The file "test.txt" was not uploaded due to an unknown error.',
                 ),
             )),
             array(array(
                 'upload_error' => UPLOAD_ERR_INI_SIZE,
                 'expected_exception' => array(
                     'symfony\component\httpfoundation\file\exception\uploadexception',
-                    'The uploaded file exceeds the upload_max_filesize directive in php.ini',
+                    'The file "test.txt" exceeds your upload_max_filesize ini directive (limit is ',
                 ),
             )),
             array(array()),