diff --git a/src/calibre/devices/mtp/filesystem_cache.py b/src/calibre/devices/mtp/filesystem_cache.py index fd5e5cd8e9..a52a20dfa3 100644 --- a/src/calibre/devices/mtp/filesystem_cache.py +++ b/src/calibre/devices/mtp/filesystem_cache.py @@ -17,23 +17,30 @@ from calibre.utils.icu import sort_key, lower class FileOrFolder(object): - def __init__(self, entry, fs_cache, all_storage_ids=()): + def __init__(self, entry, fs_cache): self.object_id = entry['id'] self.is_folder = entry['is_folder'] - self.name = force_unicode(entry.get('name', '___'), 'utf-8') + n = entry.get('name', None) + if not n: n = '___' + self.name = force_unicode(n, 'utf-8') self.storage_id = entry.get('storage_id', None) self.persistent_id = entry.get('persistent_id', self.object_id) self.size = entry.get('size', 0) # self.parent_id is None for storage objects self.parent_id = entry.get('parent_id', None) - if self.parent_id == 0: - sid = self.storage_id - if all_storage_ids and sid not in all_storage_ids: - sid = all_storage_ids[0] - self.parent_id = sid + self.all_storage_ids = fs_cache.all_storage_ids + if self.parent_id is None and self.storage_id is None: # A storage object self.storage_id = self.object_id + + if self.storage_id not in self.all_storage_ids: + raise ValueError('Storage id %s not valid for %s'%(self.storage_id, + entry)) + + if self.parent_id == 0: + self.parent_id = self.storage_id + self.is_hidden = entry.get('is_hidden', False) self.is_system = entry.get('is_system', False) self.can_delete = entry.get('can_delete', True) @@ -42,6 +49,7 @@ class FileOrFolder(object): self.folders = [] fs_cache.id_map[self.object_id] = self self.fs_cache = weakref.ref(fs_cache) + self.deleted = False @property def id_map(self): @@ -80,6 +88,7 @@ class FileOrFolder(object): except ValueError: pass self.id_map.pop(entry.object_id, None) + entry.deleted = True def dump(self, prefix='', out=sys.stdout): c = '+' if self.is_folder else '-' @@ -110,16 +119,18 @@ class FilesystemCache(object): def __init__(self, all_storage, entries): self.entries = [] self.id_map = {} + self.all_storage_ids = tuple(x['id'] for x in all_storage) for storage in all_storage: - e = FileOrFolder(storage, self, []) + e = FileOrFolder(storage, self) self.entries.append(e) self.entries.sort(key=attrgetter('object_id')) all_storage_ids = [x.object_id for x in self.entries] + self.all_storage_ids = tuple(all_storage_ids) for entry in entries: - FileOrFolder(entry, self, all_storage_ids) + FileOrFolder(entry, self) for item in self.id_map.itervalues(): try: diff --git a/src/calibre/devices/mtp/test.py b/src/calibre/devices/mtp/test.py index ca8dd55176..07c944929b 100644 --- a/src/calibre/devices/mtp/test.py +++ b/src/calibre/devices/mtp/test.py @@ -7,35 +7,49 @@ __license__ = 'GPL v3' __copyright__ = '2012, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import unittest, gc +import unittest, gc, io from calibre.constants import iswindows, islinux from calibre.utils.icu import lower from calibre.devices.mtp.driver import MTP_DEVICE from calibre.devices.scanner import DeviceScanner +class ProgressCallback(object): + + def __init__(self): + self.count = 0 + self.end_called = False + + def __call__(self, pos, total): + if pos == total: + self.end_called = True + self.count += 1 + class TestDeviceInteraction(unittest.TestCase): @classmethod def setUpClass(cls): + cls.dev = cls.storage = None cls.dev = MTP_DEVICE(None) cls.dev.startup() cls.scanner = DeviceScanner() cls.scanner.scan() cd = cls.dev.detect_managed_devices(cls.scanner.devices) if cd is None: - raise ValueError('No MTP device found') + cls.dev.shutdown() + cls.dev = None + return cls.dev.open(cd, 'test_library') if cls.dev.free_space()[0] < 10*(1024**2): - raise ValueError('The connected device %s does not have enough free' - ' space in its main memory to do the tests'%cd) + return cls.dev.filesystem_cache cls.storage = cls.dev.filesystem_cache.entries[0] @classmethod def tearDownClass(cls): - cls.dev.shutdown() - cls.dev = None + if cls.dev is not None: + cls.dev.shutdown() + cls.dev = None def setUp(self): self.cleanup = [] @@ -44,8 +58,15 @@ class TestDeviceInteraction(unittest.TestCase): for obj in reversed(self.cleanup): self.dev.delete_file_or_folder(obj) + def check_setup(self): + if self.dev is None: + self.skipTest('No MTP device detected') + if self.storage is None: + self.skipTest('The connected device does not have enough free space') + def test_folder_operations(self): ''' Test the creation of folders, duplicate folders and sub folders ''' + self.check_setup() # Create a folder name = 'zzz-test-folder' @@ -78,6 +99,72 @@ class TestDeviceInteraction(unittest.TestCase): with self.assertRaises(ValueError): self.dev.create_folder(root_file[0], 'sub-folder') + def test_file_transfer(self): + ''' Test transferring files to and from the device ''' + self.check_setup() + # Create a folder + name = 'zzz-test-folder' + folder = self.dev.create_folder(self.storage, name) + self.cleanup.append(folder) + self.assertTrue(folder.is_folder) + self.assertEqual(folder.parent_id, self.storage.object_id) + + # Check simple file put/get + size = 1024**2 + raw = io.BytesIO(b'a'*size) + raw.seek(0) + name = 'test-file.txt' + pc = ProgressCallback() + f = self.dev.put_file(folder, name, raw, size, callback=pc) + self.cleanup.append(f) + self.assertEqual(f.name, name) + self.assertEqual(f.size, size) + self.assertEqual(f.parent_id, folder.object_id) + self.assertEqual(f.storage_id, folder.storage_id) + self.assertTrue(pc.end_called, + msg='Progress callback not called with equal values (put_file)') + self.assertTrue(pc.count > 1, + msg='Progress callback only called once (put_file)') + + raw2 = io.BytesIO() + pc = ProgressCallback() + self.dev.get_file(f, raw2, callback=pc) + self.assertEqual(raw.getvalue(), raw2.getvalue()) + self.assertTrue(pc.end_called, + msg='Progress callback not called with equal values (get_file)') + self.assertTrue(pc.count > 1, + msg='Progress callback only called once (get_file)') + + # Check file replacement + raw = io.BytesIO(b'abcd') + raw.seek(0) + size = 4 + f = self.dev.put_file(folder, name, raw, size) + self.cleanup.append(f) + self.assertEqual(f.name, name) + self.assertEqual(f.size, size) + self.assertEqual(f.parent_id, folder.object_id) + self.assertEqual(f.storage_id, folder.storage_id) + + # Check that we get an error with replace=False + raw.seek(0) + with self.assertRaises(ValueError): + self.dev.put_file(folder, name, raw, size, replace=False) + + # Check that we can put a file into the root + raw.seek(0) + name = 'zzz-test-file.txt' + f = self.dev.put_file(self.storage, name, raw, size) + self.cleanup.append(f) + self.assertEqual(f.name, name) + self.assertEqual(f.size, size) + self.assertEqual(f.parent_id, self.storage.object_id) + self.assertEqual(f.storage_id, self.storage.storage_id) + + raw2 = io.BytesIO() + self.dev.get_file(f, raw2) + self.assertEqual(raw.getvalue(), raw2.getvalue()) + def measure_memory_usage(self, repetitions, func, *args, **kwargs): from calibre.utils.mem import memory gc.disable() @@ -91,10 +178,10 @@ class TestDeviceInteraction(unittest.TestCase): gc.enable() return end_mem - start_mem + @unittest.skipUnless(iswindows or islinux, 'Can only test for leaks on windows and linux') def test_memory_leaks(self): ''' Test for memory leaks in the C modules ''' - if not (iswindows or islinux): - self.skipTest('Can only test for leaks on windows and linux') + self.check_setup() # Test device scanning used_by_one = self.measure_memory_usage(1, @@ -122,7 +209,7 @@ class TestDeviceInteraction(unittest.TestCase): def tests(): tl = unittest.TestLoader() - return tl.loadTestsFromName('test.TestDeviceInteraction.test_memory_leaks') + # return tl.loadTestsFromName('test.TestDeviceInteraction.test_file_transfer') return tl.loadTestsFromTestCase(TestDeviceInteraction) def run(): diff --git a/src/calibre/devices/mtp/unix/driver.py b/src/calibre/devices/mtp/unix/driver.py index 1ff87eb974..ff00f79ad4 100644 --- a/src/calibre/devices/mtp/unix/driver.py +++ b/src/calibre/devices/mtp/unix/driver.py @@ -238,11 +238,44 @@ class MTP_DEVICE(MTPDeviceBase): raise DeviceError( 'Failed to create folder named %s in %s with error: %s'% (name, parent.full_path, self.format_errorstack(errs))) - ans['storage_id'] = sid return parent.add_child(ans) + @synchronous + def put_file(self, parent, name, stream, size, callback=None, replace=True): + e = parent.folder_named(name) + if e is not None: + raise ValueError('Cannot upload file, %s already has a folder named: %s'%( + parent.full_path, e.name)) + e = parent.file_named(name) + if e is not None: + if not replace: + raise ValueError('Cannot upload file %s, it already exists'%( + e.full_path,)) + self.delete_file_or_folder(e) + ename = name.encode('utf-8') if isinstance(name, unicode) else name + sid, pid = parent.storage_id, parent.object_id + if pid == sid: + pid = 0 + + ans, errs = self.dev.put_file(sid, pid, ename, stream, size, callback) + if ans is None: + raise DeviceError('Failed to upload file named: %s to %s: %s' + %(name, parent.full_path, self.format_errorstack(errs))) + return parent.add_child(ans) + + @synchronous + def get_file(self, f, stream, callback=None): + if f.is_folder: + raise ValueError('%s if a folder'%(f.full_path,)) + ok, errs = self.dev.get_file(f.object_id, stream, callback) + if not ok: + raise DeviceError('Failed to get file: %s with errors: %s'%( + f.full_path, self.format_errorstack(errs))) + @synchronous def delete_file_or_folder(self, obj): + if obj.deleted: + return if not obj.can_delete: raise ValueError('Cannot delete %s as deletion not allowed'% (obj.full_path,)) @@ -255,7 +288,7 @@ class MTP_DEVICE(MTPDeviceBase): parent = obj.parent ok, errs = self.dev.delete_object(obj.object_id) if not ok: - raise DeviceError('Failed to delete %s with error: '% + raise DeviceError('Failed to delete %s with error: %s'% (obj.full_path, self.format_errorstack(errs))) parent.remove_child(obj) diff --git a/src/calibre/devices/mtp/unix/libmtp.c b/src/calibre/devices/mtp/unix/libmtp.c index 982d0faf97..7f80e5bcb3 100644 --- a/src/calibre/devices/mtp/unix/libmtp.c +++ b/src/calibre/devices/mtp/unix/libmtp.c @@ -67,7 +67,7 @@ static void dump_errorstack(LIBMTP_mtpdevice_t *dev, PyObject *list) { PyObject *err; for(stack = LIBMTP_Get_Errorstack(dev); stack != NULL; stack=stack->next) { - err = Py_BuildValue("Is", stack->errornumber, stack->error_text); + err = Py_BuildValue("is", stack->errornumber, stack->error_text); if (err == NULL) break; PyList_Append(list, err); Py_DECREF(err); @@ -119,17 +119,43 @@ static uint16_t data_from_python(void *params, void *priv, uint32_t wantlen, uns } static PyObject* build_file_metadata(LIBMTP_file_t *nf, uint32_t storage_id) { - char *filename = nf->filename; - if (filename == NULL) filename = ""; + PyObject *ans = NULL, *l = NULL; - return Py_BuildValue("{s:k,s:k,s:k,s:s,s:K,s:O}", - "id", nf->item_id, - "parent_id", nf->parent_id, - "storage_id", storage_id, - "name", filename, - "size", nf->filesize, - "is_folder", (nf->filetype == LIBMTP_FILETYPE_FOLDER) ? Py_True : Py_False - ); + ans = Py_BuildValue("{s:s}", "name", nf->filename); + if (ans == NULL) return PyErr_NoMemory(); + + // We explicitly populate the dictionary instead of using Py_BuildValue to + // handle the numeric variables properly. Without this, for some reason the + // dict sometimes has incorrect values + l = PyLong_FromUnsignedLong(nf->item_id); + if (l == NULL) goto error; + if (PyDict_SetItemString(ans, "id", l) != 0) goto error; + Py_DECREF(l); l = NULL; + + l = PyLong_FromUnsignedLong(nf->parent_id); + if (l == NULL) goto error; + if (PyDict_SetItemString(ans, "parent_id", l) != 0) goto error; + Py_DECREF(l); l = NULL; + + l = PyLong_FromUnsignedLong(storage_id); + if (l == NULL) goto error; + if (PyDict_SetItemString(ans, "storage_id", l) != 0) goto error; + Py_DECREF(l); l = NULL; + + l = PyLong_FromUnsignedLongLong(nf->filesize); + if (l == NULL) goto error; + if (PyDict_SetItemString(ans, "size", l) != 0) goto error; + Py_DECREF(l); l = NULL; + + if (PyDict_SetItemString(ans, "is_folder", + (nf->filetype == LIBMTP_FILETYPE_FOLDER) ? Py_True : Py_False) != 0) + goto error; + + return ans; + +error: + Py_XDECREF(ans); Py_XDECREF(l); + return PyErr_NoMemory(); } static PyObject* file_metadata(LIBMTP_mtpdevice_t *device, PyObject *errs, uint32_t item_id, uint32_t storage_id) { @@ -507,9 +533,8 @@ libmtp_Device_delete_object(libmtp_Device *self, PyObject *args, PyObject *kwarg static PyObject * libmtp_Device_create_folder(libmtp_Device *self, PyObject *args, PyObject *kwargs) { PyObject *errs, *fo = NULL; - uint32_t parent_id, storage_id; + uint32_t storage_id, parent_id, folder_id; char *name; - uint32_t folder_id; ENSURE_DEV(NULL); ENSURE_STORAGE(NULL); diff --git a/src/calibre/devices/mtp/windows/content_enumeration.cpp b/src/calibre/devices/mtp/windows/content_enumeration.cpp index 65831768a7..e1f439926c 100644 --- a/src/calibre/devices/mtp/windows/content_enumeration.cpp +++ b/src/calibre/devices/mtp/windows/content_enumeration.cpp @@ -73,7 +73,7 @@ static void set_size_property(PyObject *dict, REFPROPERTYKEY key, const char *py hr = properties->GetUnsignedLargeIntegerValue(key, &val); if (SUCCEEDED(hr)) { - pval = PyInt_FromSsize_t((Py_ssize_t)val); + pval = PyLong_FromUnsignedLongLong(val); if (pval != NULL) { PyDict_SetItemString(dict, pykey, pval); Py_DECREF(pval); diff --git a/src/calibre/devices/mtp/windows/driver.py b/src/calibre/devices/mtp/windows/driver.py index 91abe228ae..03e3c65ad1 100644 --- a/src/calibre/devices/mtp/windows/driver.py +++ b/src/calibre/devices/mtp/windows/driver.py @@ -275,6 +275,8 @@ class MTP_DEVICE(MTPDeviceBase): @same_thread def delete_file_or_folder(self, obj): + if obj.deleted: + return if not obj.can_delete: raise ValueError('Cannot delete %s as deletion not allowed'% (obj.full_path,))