From 887afa9389d443102b856ecda9e5e74bee2ceac0 Mon Sep 17 00:00:00 2001 From: bendikro Date: Fri, 19 Feb 2016 15:54:00 +0100 Subject: [PATCH] [GTKUI] Fix bugs in files_tab and added tests After renaming files/directories in GTKUI, the file list wasn't properly updated, requiring to choose another torrent to get a file list update. --- deluge/tests/test_files_tab.py | 134 +++++++++++++++++++++++++++++++++ deluge/ui/gtkui/files_tab.py | 131 +++++++++++++------------------- 2 files changed, 186 insertions(+), 79 deletions(-) create mode 100644 deluge/tests/test_files_tab.py diff --git a/deluge/tests/test_files_tab.py b/deluge/tests/test_files_tab.py new file mode 100644 index 000000000..a068e2ba0 --- /dev/null +++ b/deluge/tests/test_files_tab.py @@ -0,0 +1,134 @@ +import pytest +from twisted.trial import unittest + +import deluge.common +import deluge.component as component +from deluge.configmanager import ConfigManager + +from . import common +from .basetest import BaseTestCase + +libs_available = True +# Allow running other tests without GTKUI dependencies available +try: + from gobject import TYPE_UINT64 + from deluge.ui.gtkui.mainwindow import MainWindow + from deluge.ui.gtkui.gtkui import DEFAULT_PREFS + from deluge.ui.gtkui.files_tab import FilesTab + +except ImportError as err: + libs_available = False + TYPE_UINT64 = "Whatever" + import traceback + traceback.print_exc() + +deluge.common.setup_translations() + + +@pytest.mark.gtkui +class FilesTabTestCase(BaseTestCase): + + def set_up(self): + if libs_available is False: + raise unittest.SkipTest("GTKUI dependencies not available") + + common.set_tmp_config_dir() + ConfigManager("gtkui.conf", defaults=DEFAULT_PREFS) + self.mainwindow = MainWindow() + self.filestab = FilesTab() + self.t_id = "1" + self.filestab.torrent_id = self.t_id + self.index = 1 + + def tear_down(self): + return component.shutdown() + + def print_treestore(self, title, treestore): + root = treestore.get_iter_root() + level = 1 + + def p_level(s, l): + print "%s%s" % (" " * l, s) + + def _print_treestore_children(i, lvl): + while i: + p_level(treestore[i][0], lvl) + if treestore.iter_children(i): + _print_treestore_children(treestore.iter_children(i), lvl + 2) + i = treestore.iter_next(i) + + print "\n%s" % title + _print_treestore_children(root, level) + print "" + + def verify_treestore(self, treestore, tree): + + def _verify_treestore(itr, tree_values): + i = 0 + while itr: + values = tree_values[i] + if treestore[itr][0] != values[0]: + return False + if treestore.iter_children(itr): + if not _verify_treestore(treestore.iter_children(itr), values[1]): + return False + itr = treestore.iter_next(itr) + i += 1 + return True + return _verify_treestore(treestore.get_iter_root(), tree) + + def test_files_tab(self): + self.filestab.files_list[self.t_id] = ({u'index': 0, u'path': u'1/test_10.txt', u'offset': 0, u'size': 13}, + {u'index': 1, u'path': u'test_100.txt', u'offset': 13, u'size': 14}) + self.filestab.update_files() + self.filestab._on_torrentfilerenamed_event(self.t_id, self.index, "2/test_100.txt") + + ret = self.verify_treestore(self.filestab.treestore, [["1/", [["test_10.txt"]]], ["2/", [["test_100.txt"]]]]) + if not ret: + self.print_treestore("Treestore not expected:", self.filestab.treestore) + self.assertFalse(True) + + def test_files_tab2(self): + self.filestab.files_list[self.t_id] = ({u'index': 0, u'path': u'1/1/test_10.txt', u'offset': 0, u'size': 13}, + {u'index': 1, u'path': u'test_100.txt', u'offset': 13, u'size': 14}) + self.filestab.update_files() + self.filestab._on_torrentfilerenamed_event(self.t_id, self.index, "1/1/test_100.txt") + + ret = self.verify_treestore(self.filestab.treestore, [["1/", [["1/", [["test_100.txt"], ["test_10.txt"]]]]]]) + if not ret: + self.print_treestore("Treestore not expected:", self.filestab.treestore) + self.assertFalse(True) + + def test_files_tab3(self): + self.filestab.files_list[self.t_id] = ({u'index': 0, u'path': u'1/test_10.txt', u'offset': 0, u'size': 13}, + {u'index': 1, u'path': u'test_100.txt', u'offset': 13, u'size': 14}) + self.filestab.update_files() + self.filestab._on_torrentfilerenamed_event(self.t_id, self.index, "1/test_100.txt") + + ret = self.verify_treestore(self.filestab.treestore, [["1/", [["test_100.txt"], ["test_10.txt"]]]]) + if not ret: + self.print_treestore("Treestore not expected:", self.filestab.treestore) + self.assertFalse(True) + + def test_files_tab4(self): + self.filestab.files_list[self.t_id] = ({u'index': 0, u'path': u'1/test_10.txt', u'offset': 0, u'size': 13}, + {u'index': 1, u'path': u'1/test_100.txt', u'offset': 13, u'size': 14}) + self.filestab.update_files() + self.filestab._on_torrentfilerenamed_event(self.t_id, self.index, "1/2/test_100.txt") + + ret = self.verify_treestore(self.filestab.treestore, [["1/", [["2/", [["test_100.txt"]]], + ["test_10.txt"]]]]) + if not ret: + self.print_treestore("Treestore not expected:", self.filestab.treestore) + self.assertFalse(True) + + def test_files_tab5(self): + self.filestab.files_list[self.t_id] = ({u'index': 0, u'path': u'1/test_10.txt', u'offset': 0, u'size': 13}, + {u'index': 1, u'path': u'2/test_100.txt', u'offset': 13, u'size': 14}) + self.filestab.update_files() + self.filestab._on_torrentfilerenamed_event(self.t_id, self.index, "1/test_100.txt") + + ret = self.verify_treestore(self.filestab.treestore, [["1/", [["test_100.txt"], ["test_10.txt"]]]]) + if not ret: + self.print_treestore("Treestore not expected:", self.filestab.treestore) + self.assertFalse(True) diff --git a/deluge/ui/gtkui/files_tab.py b/deluge/ui/gtkui/files_tab.py index 4822de683..439c5b734 100644 --- a/deluge/ui/gtkui/files_tab.py +++ b/deluge/ui/gtkui/files_tab.py @@ -633,79 +633,47 @@ class FilesTab(Tab): # We need to update the filename displayed if we're currently viewing # this torrents files. - if torrent_id == self.torrent_id: - old_name_len = len(old_name.split("/")) - name_len = len(name.split("/")) - if old_name_len != name_len: - # The parent path list changes depending on which way the file - # is moving in the tree - if old_name_len < name_len: - parent_path = [o for o in old_name.split("/")[:-1]] - else: - parent_path = [o for o in name.split("/")[:-1]] - # Find the iter to the parent folder we need to add a new folder - # to. + if torrent_id != self.torrent_id: + return - def find_parent(model, path, itr, user_data): - if model[itr][0] == parent_path[0] + "/": - if len(parent_path) == 1: - # This is the parent iter - to_create = name.split("/")[len(old_name.split("/")[:-1]):-1] - parent_iter = itr + old_name_parent = old_name.split("/")[:-1] + parent_path = name.split("/")[:-1] - for tc in to_create: - # We need to check if these folders need to be created - child_iter = self.treestore.iter_children(parent_iter) - create = True - while child_iter: - if self.treestore[child_iter][0] == tc + "/": - create = False - parent_iter = child_iter - break - child_iter = self.treestore.iter_next(child_iter) - if create: - parent_iter = self.treestore.append( - parent_iter, [tc + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) - - # Find the iter for the file that needs to be moved - def get_file_iter(model, path, itr, user_data): - if model[itr][5] == index: - model[itr][0] = name.split("/")[-1] - # t = self.treestore.append( - # parent_iter, - # self.treestore.get(itr, *xrange(self.treestore.get_n_columns()))) - itr_parent = self.treestore.iter_parent(itr) - self.treestore.remove(itr) - self.remove_childless_folders(itr_parent) - return True - - self.treestore.foreach(get_file_iter, None) - return True - else: - log.debug("parent_path: %s remove: %s", parent_path, model[itr][0]) - parent_path.remove(model[itr][0][:-1]) - - if parent_path: - self.treestore.foreach(find_parent, None) - else: - new_folders = name.split("/")[:-1] - parent_iter = None - for f in new_folders: - parent_iter = self.treestore.append( - parent_iter, [f + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) - child = self.get_iter_at_path(old_name) - self.treestore.append( - parent_iter, - self.treestore.get(child, *xrange(self.treestore.get_n_columns()))) - self.treestore.remove(child) + if old_name_parent != parent_path: + if parent_path: + for i, p in enumerate(parent_path): + p_itr = self.get_iter_at_path("/".join(parent_path[:i + 1]) + "/") + if not p_itr: + p_itr = self.get_iter_at_path("/".join(parent_path[:i]) + "/") + p_itr = self.treestore.append( + p_itr, [parent_path[i] + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) + p_itr = self.get_iter_at_path("/".join(parent_path) + "/") + old_name_itr = self.get_iter_at_path(old_name) + self.treestore.append(p_itr, + self.treestore.get(old_name_itr, *xrange(self.treestore.get_n_columns()))) + self.treestore.remove(old_name_itr) + # Remove old parent path + p_itr = self.get_iter_at_path("/".join(old_name_parent) + "/") + self.remove_childless_folders(p_itr) else: - # This is just changing a filename without any folder changes - def set_file_name(model, path, itr, user_data): - if model[itr][5] == index: - model[itr][0] = os.path.split(name)[-1] - return True - self.treestore.foreach(set_file_name, None) + new_folders = name.split("/")[:-1] + parent_iter = None + for f in new_folders: + parent_iter = self.treestore.append( + parent_iter, [f + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) + child = self.get_iter_at_path(old_name) + self.treestore.append(parent_iter, + self.treestore.get(child, *xrange(self.treestore.get_n_columns()))) + self.treestore.remove(child) + + else: + # This is just changing a filename without any folder changes + def set_file_name(model, path, itr, user_data): + if model[itr][5] == index: + model[itr][0] = os.path.split(name)[-1] + return True + self.treestore.foreach(set_file_name, None) def get_iter_at_path(self, filepath): """Returns the gtkTreeIter for filepath.""" @@ -728,14 +696,13 @@ class FilesTab(Tab): itr = self.treestore.iter_children(itr) level += 1 continue - elif (level + 1) == len(filepath) and ipath == filepath[level] + "/" if is_dir else filepath[level]: + elif (level + 1) == len(filepath) and ipath == (filepath[level] + "/" if is_dir else filepath[level]): # This is the iter we've been searching for path_iter = itr break else: itr = self.treestore.iter_next(itr) continue - return path_iter def remove_childless_folders(self, itr): @@ -754,7 +721,8 @@ class FilesTab(Tab): if old_folder[-1] != "/": old_folder += "/" - if new_folder[-1] != "/": + + if len(new_folder) > 0 and new_folder[-1] != "/": new_folder += "/" for fd in self.files_list[torrent_id]: @@ -778,7 +746,8 @@ class FilesTab(Tab): old_folder_iter = self.get_iter_at_path(old_folder) old_folder_iter_parent = self.treestore.iter_parent(old_folder_iter) - new_folder_iter = self.get_iter_at_path(new_folder) + new_folder_iter = self.get_iter_at_path(new_folder) if new_folder else None + if len(new_split) == len(old_split): # These are at the same tree depth, so it's a simple rename self.treestore[old_folder_iter][0] = new_split[-1] + "/" @@ -788,15 +757,19 @@ class FilesTab(Tab): reparent_iter(self.treestore, self.treestore.iter_children(old_folder_iter), new_folder_iter) else: parent = old_folder_iter_parent - for ns in new_split[:-1]: - parent = self.treestore.append(parent, [ns + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) + if new_split: + for ns in new_split[:-1]: + parent = self.treestore.append(parent, [ns + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY]) - self.treestore[old_folder_iter][0] = new_split[-1] + "/" - reparent_iter(self.treestore, old_folder_iter, parent) + self.treestore[old_folder_iter][0] = new_split[-1] + "/" + reparent_iter(self.treestore, old_folder_iter, parent) + else: + child_itr = self.treestore.iter_children(old_folder_iter) + reparent_iter(self.treestore, child_itr, old_folder_iter_parent, move_siblings=True) - # We need to check if the old_folder_iter_parent no longer has children + # We need to check if the old_folder_iter no longer has children # and if so, we delete it - self.remove_childless_folders(old_folder_iter_parent) + self.remove_childless_folders(old_folder_iter) def _on_torrentremoved_event(self, torrent_id): if torrent_id in self.files_list: