Update Dropbox backend for dropbox==9.3.0 client (#724)

* Update Dropbox backend for dropbox==9.3.0 client

* Fix flake8 error (import sort order)

* Update Dropbox backend for current dropbox client (9.3.0)

AUTHORS

@@ -36,6 +36,7 @@ By order of apparition, thanks:
     * Alex Watt (Google Cloud Storage patch)
     * Jumpei Yoshimura (S3 docs)
     * Jon Dufresne
+    * Rodrigo Gadea (Dropbox fixes)
 
 
 

storages/backends/dropbox.py

@@ -10,7 +10,7 @@
 
 from __future__ import absolute_import
 
-from datetime import datetime
+from io import BytesIO
 from shutil import copyfileobj
 from tempfile import SpooledTemporaryFile
 
@@ -21,12 +21,10 @@
 from django.utils.deconstruct import deconstructible
 from dropbox import Dropbox
 from dropbox.exceptions import ApiError
-from dropbox.files import CommitInfo, UploadSessionCursor
+from dropbox.files import CommitInfo, FolderMetadata, UploadSessionCursor
 
 from storages.utils import setting
 
-DATE_FORMAT = '%a, %d %b %Y %X +0000'
-
 
 class DropBoxStorageException(Exception):
     pass
@@ -36,16 +34,32 @@ class DropBoxFile(File):
     def __init__(self, name, storage):
         self.name = name
         self._storage = storage
+        self._file = None
 
-    @property
-    def file(self):
-        if not hasattr(self, '_file'):
-            response = self._storage.client.files_download(self.name)
+    def _get_file(self):
+        if self._file is None:
             self._file = SpooledTemporaryFile()
-            copyfileobj(response, self._file)
+            # As dropbox==9.3.0, the client returns a tuple
+            # (dropbox.files.FileMetadata, requests.models.Response)
+            file_metadata, response = \
+                self._storage.client.files_download(self.name)
+            if response.status_code == 200:
+                with BytesIO(response.content) as file_content:
+                    copyfileobj(file_content, self._file)
+            else:
+                # JIC the exception isn't catched by the dropbox client
+                raise DropBoxStorageException(
+                    "Dropbox server returned a {} response when accessing {}"
+                    .format(response.status_code, self.name)
+                )
             self._file.seek(0)
         return self._file
 
+    def _set_file(self, value):
+        self._file = value
+
+    file = property(_get_file, _set_file)
+
 
 @deconstructible
 class DropBoxStorage(Storage):
@@ -78,29 +92,25 @@ def exists(self, name):
     def listdir(self, path):
         directories, files = [], []
         full_path = self._full_path(path)
-        metadata = self.client.files_get_metadata(full_path)
-        for entry in metadata['contents']:
-            entry['path'] = entry['path'].replace(full_path, '', 1)
-            entry['path'] = entry['path'].replace('/', '', 1)
-            if entry['is_dir']:
-                directories.append(entry['path'])
+        metadata = self.client.files_list_folder(full_path)
+        for entry in metadata.entries:
+            if isinstance(entry, FolderMetadata):
+                directories.append(entry.name)
             else:
-                files.append(entry['path'])
+                files.append(entry.name)
         return directories, files
 
     def size(self, name):
         metadata = self.client.files_get_metadata(self._full_path(name))
-        return metadata['bytes']
+        return metadata.size
 
     def modified_time(self, name):
         metadata = self.client.files_get_metadata(self._full_path(name))
-        mod_time = datetime.strptime(metadata['modified'], DATE_FORMAT)
-        return mod_time
+        return metadata.server_modified
 
     def accessed_time(self, name):
         metadata = self.client.files_get_metadata(self._full_path(name))
-        acc_time = datetime.strptime(metadata['client_mtime'], DATE_FORMAT)
-        return acc_time
+        return metadata.client_modified
 
     def url(self, name):
         media = self.client.files_get_temporary_link(self._full_path(name))

tests/test_dropbox.py

@@ -4,8 +4,10 @@
 from django.core.exceptions import (
     ImproperlyConfigured, SuspiciousFileOperation,
 )
-from django.core.files.base import ContentFile, File
+from django.core.files.base import File
 from django.test import TestCase
+from dropbox.files import FileMetadata, FolderMetadata, GetTemporaryLinkResult
+from requests.models import Response
 
 from storages.backends import dropbox
 
@@ -15,50 +17,38 @@
     import mock
 
 
-class F(object):
-    pass
+FILE_DATE = datetime(2015, 8, 24, 15, 6, 41)
+FILE_METADATA_MOCK = mock.MagicMock(spec=FileMetadata)
+FILE_METADATA_MOCK.size = 4
+FILE_METADATA_MOCK.client_modified = FILE_DATE
+FILE_METADATA_MOCK.server_modified = FILE_DATE
+FILE_METADATA_MOCK.path_lower = '/foo.txt'
+FILE_METADATA_MOCK.path_display = '/foo.txt'
+FILE_METADATA_MOCK.name = 'foo.txt'
+FILE_METADATA_MOCK.rev = '012c0000000150c838f0'
+FILE_METADATA_MOCK.content_hash = \
+    '3865695d47c02576e8578df30d56bb3faf737c11044d804f09ffb6484453020f'
 
+FOLDER_METADATA_MOCK = mock.MagicMock(spec=FolderMetadata)
+FOLDER_METADATA_MOCK.name = 'bar'
 
-FILE_DATE = datetime(2015, 8, 24, 15, 6, 41)
-FILE_FIXTURE = {
-    'bytes': 4,
-    'client_mtime': 'Mon, 24 Aug 2015 15:06:41 +0000',
-    'icon': 'page_white_text',
-    'is_dir': False,
-    'mime_type': 'text/plain',
-    'modified': 'Mon, 24 Aug 2015 15:06:41 +0000',
-    'path': '/foo.txt',
-    'rev': '23b7cdd80',
-    'revision': 2,
-    'root': 'app_folder',
-    'size': '4 bytes',
-    'thumb_exists': False
-}
-FILES_FIXTURE = {
-    'bytes': 0,
-    'contents': [
-        FILE_FIXTURE,
-        {'bytes': 0,
-         'icon': 'folder',
-         'is_dir': True,
-         'modified': 'Mon, 6 Feb 2015 15:06:40 +0000',
-         'path': '/bar',
-         'rev': '33b7cdd80',
-         'revision': 3,
-         'root': 'app_folder',
-         'size': '0 bytes',
-         'thumb_exists': False}
-    ],
-    'hash': 'aeaa0ed65aa4f88b96dfe3d553280efc',
-    'icon': 'folder',
-    'is_dir': True,
-    'path': '/',
-    'root': 'app_folder',
-    'size': '0 bytes',
-    'thumb_exists': False
-}
-FILE_MEDIA_FIXTURE = F()
-FILE_MEDIA_FIXTURE.link = 'https://dl.dropboxusercontent.com/1/view/foo'
+FILES_MOCK = mock.MagicMock(spec=FolderMetadata)
+FILES_MOCK.entries = [
+    FILE_METADATA_MOCK, FOLDER_METADATA_MOCK
+]
+
+FILE_MEDIA_MOCK = mock.MagicMock(spec=GetTemporaryLinkResult)
+FILE_MEDIA_MOCK.link = 'https://dl.dropboxusercontent.com/1/view/foo'
+
+FILES_EMPTY_MOCK = mock.MagicMock(spec=FolderMetadata)
+FILES_EMPTY_MOCK.entries = []
+
+RESPONSE_200_MOCK = mock.MagicMock(spec=Response)
+RESPONSE_200_MOCK.status_code = 200
+RESPONSE_200_MOCK.content = b'bar'
+
+RESPONSE_500_MOCK = mock.MagicMock(spec=Response)
+RESPONSE_500_MOCK.status_code = 500
 
 
 class DropBoxTest(TestCase):
@@ -70,12 +60,12 @@ def test_no_access_token(self, *args):
             dropbox.DropBoxStorage(None)
 
     @mock.patch('dropbox.Dropbox.files_delete',
-                return_value=FILE_FIXTURE)
+                return_value=FILE_METADATA_MOCK)
     def test_delete(self, *args):
         self.storage.delete('foo')
 
     @mock.patch('dropbox.Dropbox.files_get_metadata',
-                return_value=[FILE_FIXTURE])
+                return_value=[FILE_METADATA_MOCK])
     def test_exists(self, *args):
         exists = self.storage.exists('foo')
         self.assertTrue(exists)
@@ -86,8 +76,8 @@ def test_not_exists(self, *args):
         exists = self.storage.exists('bar')
         self.assertFalse(exists)
 
-    @mock.patch('dropbox.Dropbox.files_get_metadata',
-                return_value=FILES_FIXTURE)
+    @mock.patch('dropbox.Dropbox.files_list_folder',
+                return_value=FILES_MOCK)
     def test_listdir(self, *args):
         dirs, files = self.storage.listdir('/')
         self.assertGreater(len(dirs), 0)
@@ -96,19 +86,19 @@ def test_listdir(self, *args):
         self.assertEqual(files[0], 'foo.txt')
 
     @mock.patch('dropbox.Dropbox.files_get_metadata',
-                return_value=FILE_FIXTURE)
+                return_value=FILE_METADATA_MOCK)
     def test_size(self, *args):
         size = self.storage.size('foo')
-        self.assertEqual(size, FILE_FIXTURE['bytes'])
+        self.assertEqual(size, FILE_METADATA_MOCK.size)
 
     @mock.patch('dropbox.Dropbox.files_get_metadata',
-                return_value=FILE_FIXTURE)
+                return_value=FILE_METADATA_MOCK)
     def test_modified_time(self, *args):
         mtime = self.storage.modified_time('foo')
         self.assertEqual(mtime, FILE_DATE)
 
     @mock.patch('dropbox.Dropbox.files_get_metadata',
-                return_value=FILE_FIXTURE)
+                return_value=FILE_METADATA_MOCK)
     def test_accessed_time(self, *args):
         mtime = self.storage.accessed_time('foo')
         self.assertEqual(mtime, FILE_DATE)
@@ -137,10 +127,10 @@ def test_chunked_upload(self, start, append, finish, upload):
         self.assertFalse(upload.called)
 
     @mock.patch('dropbox.Dropbox.files_get_temporary_link',
-                return_value=FILE_MEDIA_FIXTURE)
+                return_value=FILE_MEDIA_MOCK)
     def test_url(self, *args):
         url = self.storage.url('foo')
-        self.assertEqual(url, FILE_MEDIA_FIXTURE.link)
+        self.assertEqual(url, FILE_MEDIA_MOCK.link)
 
     def test_formats(self, *args):
         self.storage = dropbox.DropBoxStorage('foo')
@@ -157,14 +147,21 @@ def setUp(self, *args):
         self.file = dropbox.DropBoxFile('/foo.txt', self.storage)
 
     @mock.patch('dropbox.Dropbox.files_download',
-                return_value=ContentFile(b'bar'))
+                return_value=(FILE_METADATA_MOCK, RESPONSE_200_MOCK))
     def test_read(self, *args):
-        file = self.storage._open(b'foo')
+        file = self.storage._open('foo.txt')
         self.assertEqual(file.read(), b'bar')
 
+    @mock.patch('dropbox.Dropbox.files_download',
+                return_value=(FILE_METADATA_MOCK, RESPONSE_500_MOCK))
+    def test_server_bad_response(self, *args):
+        with self.assertRaises(dropbox.DropBoxStorageException):
+            file = self.storage._open('foo.txt')
+            file.read()
+
 
-@mock.patch('dropbox.Dropbox.files_get_metadata',
-            return_value={'contents': []})
+@mock.patch('dropbox.Dropbox.files_list_folder',
+            return_value=FILES_EMPTY_MOCK)
 class DropBoxRootPathTest(TestCase):
     def test_jailed(self, *args):
         self.storage = dropbox.DropBoxStorage('foo', '/bar')