Use filenames for the media MIME fallback

panoptes_client/subject.py

@@ -213,17 +213,21 @@ def _upload_media(self, url, media_data, media_type):
         upload_response.raise_for_status()
         return upload_response
 
-    def _detect_media_type(self, media_data=None, manual_mimetype=None):
+    def _detect_media_type(self, media_data=None, manual_mimetype=None,
+                           filename=None):
         if manual_mimetype is not None:
             return manual_mimetype
 
         if MEDIA_TYPE_DETECTION == 'magic':
             return magic.from_buffer(media_data, mime=True)
 
-        media_type = mimetypes.guess_type(media_data)[0]
+        media_type = None
+        if isinstance(filename, _OLD_STR_TYPES):
+            media_type = mimetypes.guess_type(filename)[0]
         if not media_type:
             raise UnknownMediaException(
-                'Could not detect file type. Please try installing '
+                'Could not detect file type. Please supply manual_mimetype '
+                'or try installing '
                 'libmagic: https://panoptes-python-client.readthedocs.'
                 'io/en/latest/user_guide.html#uploading-non-image-'
                 'media-types'
@@ -308,7 +312,11 @@ def add_location(self, location, manual_mimetype=None):
 
         try:
             media_data = f.read()
-            media_type = self._detect_media_type(media_data, manual_mimetype)
+            filename = (location if isinstance(location, _OLD_STR_TYPES)
+                        else getattr(f, 'name', None))
+            media_type = self._detect_media_type(
+                media_data, manual_mimetype, filename=filename,
+            )
 
             self._validate_media_type(media_type)
 
@@ -369,7 +377,12 @@ def _save_attached_image(self, attached_media, manual_mimetype=None, metadata=No
             media_type = None
             try:
                 media_data = f.read()
-                media_type = self._detect_media_type(media_data, manual_mimetype)
+                filename = (attached_media
+                            if isinstance(attached_media, _OLD_STR_TYPES)
+                            else getattr(f, 'name', None))
+                media_type = self._detect_media_type(
+                    media_data, manual_mimetype, filename=filename,
+                )
                 self._validate_media_type(media_type)
             finally:
                 f.close()

panoptes_client/tests/test_media_type_fallback.py

@@ -0,0 +1,91 @@
+import io
+import os
+import tempfile
+import unittest
+from contextlib import nullcontext
+from unittest.mock import Mock, patch
+
+from panoptes_client.subject import Subject, UnknownMediaException
+
+
+class TestMediaTypeFallback(unittest.TestCase):
+    def setUp(self):
+        self.detection = patch(
+            'panoptes_client.subject.MEDIA_TYPE_DETECTION', 'mimetypes',
+        )
+        self.detection.start()
+        self.addCleanup(self.detection.stop)
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.data = b'\x89PNG\r\n\x1a\n'
+        self.subject = Subject()
+
+    def make_file(self, name):
+        path = os.path.join(self.directory.name, name)
+        with open(path, 'wb') as file:
+            file.write(self.data)
+        return path
+
+    def test_local_paths_use_extension_instead_of_binary_content(self):
+        for name, media_type in (
+            ('image.png', 'image/png'),
+            ('image.jpg', 'image/jpeg'),
+            ('video.mp4', 'video/mp4'),
+        ):
+            with self.subTest(name=name):
+                self.subject.add_location(self.make_file(name))
+                self.assertEqual(self.subject.locations[-1], media_type)
+                self.assertEqual(self.subject._media_files[-1], self.data)
+        self.assertIn('locations', self.subject.modified_attributes)
+
+    def test_open_file_uses_its_name(self):
+        file = open(self.make_file('image.png'), 'rb')
+        self.subject.add_location(file)
+        self.assertEqual(self.subject.locations, ['image/png'])
+        self.assertTrue(file.closed)
+
+    def test_named_buffer_uses_its_name(self):
+        file = io.BytesIO(self.data)
+        file.name = 'image.png'
+        self.subject.add_location(file)
+        self.assertEqual(self.subject.locations, ['image/png'])
+
+    def test_unnamed_buffer_has_actionable_error(self):
+        file = io.BytesIO(self.data)
+        with self.assertRaisesRegex(UnknownMediaException, 'manual_mimetype'):
+            self.subject.add_location(file)
+        self.assertTrue(file.closed)
+        self.assertEqual(self.subject.locations, [])
+
+    def test_unnamed_buffer_with_override_still_works(self):
+        self.subject.add_location(
+            io.BytesIO(self.data), manual_mimetype='image/png',
+        )
+        self.assertEqual(self.subject.locations, ['image/png'])
+
+    def test_unknown_extension_has_actionable_error(self):
+        with self.assertRaisesRegex(UnknownMediaException, 'manual_mimetype'):
+            self.subject.add_location(self.make_file('image.unknownextension'))
+
+    def test_magic_still_uses_content(self):
+        magic = Mock()
+        magic.from_buffer.return_value = 'image/png'
+        with patch('panoptes_client.subject.MEDIA_TYPE_DETECTION', 'magic'):
+            with patch('panoptes_client.subject.magic', magic, create=True):
+                self.subject.add_location(io.BytesIO(self.data))
+        magic.from_buffer.assert_called_once_with(self.data, mime=True)
+        self.assertEqual(self.subject.locations, ['image/png'])
+
+    def test_attached_media_uses_filename(self):
+        path = self.make_file('image.png')
+        with patch.object(self.subject, '_add_attached_image') as add_image:
+            with patch.object(self.subject, '_upload_media') as upload:
+                add_image.return_value = 'https://example.com/upload'
+                self.subject._save_attached_image(path, client=nullcontext())
+        add_image.assert_called_once_with(
+            src=None, content_type='image/png', metadata={},
+            external_link=False,
+        )
+        upload.assert_called_once_with(
+            'https://example.com/upload', self.data, 'image/png',
+        )

panoptes_client/tests/test_subject.py

@@ -28,7 +28,8 @@ def test_add_location_manual_mimetype_file_like(self):
         self.assertEqual(self.subject._media_files[-1], data)
         self.assertIn("locations", self.subject.modified_attributes)
 
-    @patch("panoptes_client.subject.magic")
+    @patch("panoptes_client.subject.MEDIA_TYPE_DETECTION", "magic")
+    @patch("panoptes_client.subject.magic", create=True)
     def test_add_location_magic_detection(self, mock_magic):
         mock_magic.from_buffer.return_value = "image/jpeg"
         data = b"fake image data"
@@ -41,12 +42,10 @@ def test_add_location_magic_detection(self, mock_magic):
 
     @patch.object(mimetypes, 'guess_type', return_value=("image/jpeg", None))
     def test_add_location_mimetypes_detection(self, mock_guess_type):
-        import panoptes_client.subject as subject_module
-        subject_module.MEDIA_TYPE_DETECTION = 'mimetypes'
-
         m = mock_open(read_data=b"fake image data")
-        with patch("panoptes_client.subject.open", m, create=True):
-            self.subject.add_location("dummy.jpg")
+        with patch("panoptes_client.subject.MEDIA_TYPE_DETECTION", "mimetypes"):
+            with patch("panoptes_client.subject.open", m, create=True):
+                self.subject.add_location("dummy.jpg")
 
         self.assertEqual(self.subject.locations[-1], "image/jpeg")
         self.assertEqual(self.subject._media_files[-1], b"fake image data")
@@ -56,4 +55,4 @@ def test_add_location_invalid_manual_mimetype(self):
         data = b"fake data"
         fake_file = io.BytesIO(data)
         with self.assertRaises(UnknownMediaException):
-            self.subject.add_location(fake_file, manual_mimetype="application/javascript")
+            self.subject.add_location(fake_file, manual_mimetype="application/javascript")