Take into account @weaverryan's comments

symfony · dunglas · Oct 7, 2015 · Oct 7, 2015 · Oct 10, 2015 · Oct 10, 2015
commit 53e2b263178474a07611a51ab2a9956e0b79ab1c
diff --git a/src/Symfony/Component/Serializer/Normalizer/DataUriNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/DataUriNormalizer.php
@@ -44,19 +44,13 @@ public function __construct(MimeTypeGuesserInterface $mimeTypeGuesser = null)
     */
    public function normalize($object, $format = null, array $context = array())
    {
-        if ($object instanceof File) {
-            $mimeType = $object->getMimeType();
-        } elseif ($this->mimeTypeGuesser) {
-            $mimeType = $this->mimeTypeGuesser->guess($object->getPathname());
-        } else {
-            $mimeType = 'application/octet-stream';
+        if (!$object instanceof \SplFileInfo) {
+            throw new InvalidArgumentException('The object must be an instance of "\SplFileInfo".');
        }

+        $mimeType = $this->getMimeType($object);
        list($typeName) = explode('/', $mimeType, 2);
-
-        if (!$object instanceof \SplFileObject) {
-            $object = $object->openFile();
-        }
+        $object = $this->extractSplFileObject($object);

        $data = '';

@@ -125,4 +119,40 @@ public function supportsDenormalization($data, $type, $format = null)

        return isset($supportedTypes[$type]);
    }
+
+    /**
+     * Gets the mime type of the object. Defaults to application/octet-stream.
+     *
+     * @param \SplFileInfo $object
+     *
+     * @return string
+     */
+    private function getMimeType(\SplFileInfo $object)
+    {
+        if ($object instanceof File) {
+            return $object->getMimeType();
+        }
+
+        if ($this->mimeTypeGuesser && $mimeType = $this->mimeTypeGuesser->guess($object->getPathname())) {
+            return $mimeType;
+        }
+
+        return 'application/octet-stream';
+    }
+
+    /**
+     * Returns the \SplFileObject instance associated with the given \SplFileInfo instance.
+     *
+     * @param \SplFileInfo $object
+     *
+     * @return \SplFileObject
+     */
+    private function extractSplFileObject(\SplFileInfo $object)
+    {
+        if ($object instanceof \SplFileObject) {
+            return $object;
+        }
+
+        return $object->openFile();
+    }
 }
-Original file line number
+Diff line change
@@ -44,19 +44,13 @@ public function __construct(MimeTypeGuesserInterface $mimeTypeGuesser = null)
          */
         public function normalize($object, $format = null, array $context = array())
         {
-            if ($object instanceof File) {
-                $mimeType = $object->getMimeType();
-            } elseif ($this->mimeTypeGuesser) {
-                $mimeType = $this->mimeTypeGuesser->guess($object->getPathname());
-            } else {
-                $mimeType = 'application/octet-stream';
+            if (!$object instanceof \SplFileInfo) {
+                throw new InvalidArgumentException('The object must be an instance of "\SplFileInfo".');
             }
+            $mimeType = $this->getMimeType($object);
             list($typeName) = explode('/', $mimeType, 2);
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            stloyd
  

      

      

      


        Oct 8, 2015


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Why you limit explode() too 2 & use just one variable?
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            dunglas
  

      

      

      


        Oct 10, 2015


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Because it won't work:
php > var_dump(explode('/', 'text/plain', 1));
array(1) {
  [0] =>
  string(10) "text/plain"
}

    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    






    

  





  


    
    
      
  
    
    
              
          

          
        

                


  
    
        
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            weaverryan
  

      

      

      


        Jan 17, 2016


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      minutiae detail, but I'd like this line moved right before the if statement that actually uses $typeName - it's not needed up here.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
-            if (!$object instanceof \SplFileObject) {
-                $object = $object->openFile();
-            }
+            $object = $this->extractSplFileObject($object);
             Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            weaverryan
  

      

      

      


        Jan 17, 2016


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Maybe call this $splFileObject for clarity and so we're not overriding the previous variable (even though we don't need it)
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
             $data = '';
@@ -125,4 +119,40 @@ public function supportsDenormalization($data, $type, $format = null)
             return isset($supportedTypes[$type]);
         }
+        /**
+         * Gets the mime type of the object. Defaults to application/octet-stream.
+         *
+         * @param \SplFileInfo $object
+         *
+         * @return string
+         */
+        private function getMimeType(\SplFileInfo $object)
+        {
+            if ($object instanceof File) {
+                return $object->getMimeType();
+            }
+            if ($this->mimeTypeGuesser && $mimeType = $this->mimeTypeGuesser->guess($object->getPathname())) {
+                return $mimeType;
+            }
+            return 'application/octet-stream';
+        }
+        /**
+         * Returns the \SplFileObject instance associated with the given \SplFileInfo instance.
+         *
+         * @param \SplFileInfo $object
+         *
+         * @return \SplFileObject
+         */
+        private function extractSplFileObject(\SplFileInfo $object)
+        {
+            if ($object instanceof \SplFileObject) {
+                return $object;
+            }
+            return $object->openFile();
+        }
     }