[pitivi] medialibrary: Perform a single call to addUris when importing

[Alexandru Băluț <alexbalut src gnome org>]

commit b1a7278c26721b7bb57e977b517b5e4ccccc5587
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Wed Dec 14 16:38:55 2016 +0100

    medialibrary: Perform a single call to addUris when importing
    
    Before two calls were being performed when paths were dragged onto the
    media library: once for the files, second for the files in the
    directories scanned recursively in the background.
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1558

 pitivi/medialibrary.py |   63 ++++++++++++++--------------------------------
 pitivi/utils/misc.py   |   48 +++++++++++++++++++++++-----------
 pre-commit.hook        |    1 -
 tests/common.py        |    5 +--
 tests/test_misc.py     |   66 ++++++++++++++++++++++++++++++++++++++++-------
 5 files changed, 109 insertions(+), 74 deletions(-)
---
diff --git a/pitivi/medialibrary.py b/pitivi/medialibrary.py
index 7abe239..d59374e 100644
--- a/pitivi/medialibrary.py
+++ b/pitivi/medialibrary.py
@@ -520,7 +520,7 @@ class MediaLibraryWidget(Gtk.Box, Loggable):
                            [URI_TARGET_ENTRY, FILE_TARGET_ENTRY],
                            Gdk.DragAction.COPY)
         self.drag_dest_add_uri_targets()
-        self.connect("drag_data_received", self._dndDataReceivedCb)
+        self.connect("drag_data_received", self._drag_data_received_cb)
 
         self._setupViewAsDragAndDropSource(self.treeview)
         self._setupViewAsDragAndDropSource(self.iconview)
@@ -1362,54 +1362,29 @@ class MediaLibraryWidget(Gtk.Box, Loggable):
         self.storemodel.clear()
         self._project = None
 
-    def _addUris(self, uris):
-        if self._project:
-            self._project.addUris(uris)
+    def __paths_walked_cb(self, uris):
+        """Handles the end of the path walking when importing files and dirs."""
+        if not uris:
+            return
+        if not self._project:
+            self.warning("Cannot add URIs, project missing")
+        self._last_imported_uris = set(uris)
+        assets = self._project.assetsForUris(uris)
+        if assets:
+            # All the files have already been added.
+            self._selectLastImportedUris()
         else:
-            self.warning(
-                "Adding uris to project, but the project has changed in the meantime")
-        return False
+            self._project.addUris(uris)
 
-    # Drag and Drop
-    def _dndDataReceivedCb(self, unused_widget, unused_context, unused_x,
-                           unused_y, selection, targettype, unused_time):
+    def _drag_data_received_cb(self, unused_widget, unused_context, unused_x,
+                               unused_y, selection, targettype, unused_time):
+        """Handles data being dragged onto self."""
         self.debug("targettype: %d, selection.data: %r",
                    targettype, selection.get_data())
-
-        directories = []
-        filenames = []
-
         uris = selection.get_uris()
-        # Filter out the empty uris.
-        uris = [x for x in uris if x]
-        for raw_uri in uris:
-            # Strip out NULL chars first.
-            raw_uri = raw_uri.strip('\x00')
-            uri = urlparse(raw_uri)
-            if uri.scheme == 'file':
-                path = unquote(uri.path)
-                if os.path.isfile(path):
-                    filenames.append(raw_uri)
-                elif os.path.isdir(path):
-                    directories.append(raw_uri)
-                else:
-                    self.warning("Unusable file: %s, %s", raw_uri, path)
-            else:
-                self.fixme(
-                    "Importing remote files is not implemented: %s", raw_uri)
-
-        if directories:
-            # Recursively import from folders that were dragged into the
-            # library
-            self.app.threads.addThread(PathWalker, directories, self._addUris)
-        if filenames:
-            self._last_imported_uris.update(filenames)
-            assets = self._project.assetsForUris(list(self._last_imported_uris))
-            if assets:
-                # All the files have already been added.
-                self._selectLastImportedUris()
-            else:
-                self._project.addUris(filenames)
+        # Scan in the background what was dragged and
+        # import whatever can be imported.
+        self.app.threads.addThread(PathWalker, uris, self.__paths_walked_cb)
 
     # Used with TreeView and IconView
     def _dndDragDataGetCb(self, unused_view, unused_context, data, unused_info, unused_timestamp):
diff --git a/pitivi/utils/misc.py b/pitivi/utils/misc.py
index af240ad..59b8662 100644
--- a/pitivi/utils/misc.py
+++ b/pitivi/utils/misc.py
@@ -164,27 +164,43 @@ def quote_uri(uri):
 class PathWalker(Thread):
     """Thread for recursively searching in a list of directories."""
 
-    def __init__(self, paths, callback):
+    def __init__(self, uris, callback):
         Thread.__init__(self)
-        self.log("New PathWalker for %s", paths)
-        self.paths = paths
+        self.log("New PathWalker for %s", uris)
+        self.uris = uris
         self.callback = callback
         self.stopme = threading.Event()
 
+    def _scan(self, uris):
+        """Scans the URIs and yields the file URIs."""
+        for uri in uris:
+            if self.stopme.is_set():
+                return
+            url = urlparse(uri)
+            if not url.scheme == 'file':
+                self.fixme("Unsupported URI: %s", uri)
+                continue
+            path = unquote(url.path)
+            if os.path.isfile(path):
+                yield uri
+            elif os.path.isdir(path):
+                yield from self._scan_dir(path)
+            else:
+                self.warning("Unusable, not a file nor a dir: %s, %s", uri, path)
+
+    def _scan_dir(self, folder):
+        """Scans the folder recursively and yields the URIs of the files."""
+        self.log("Scanning folder %s", folder)
+        for path, dirs, files in os.walk(folder):
+            if self.stopme.is_set():
+                return
+            for afile in files:
+                yield Gst.filename_to_uri(os.path.join(path, afile))
+
     def process(self):
-        for folder in self.paths:
-            self.log("folder %s" % folder)
-            if folder.startswith("file://"):
-                folder = unquote(folder[len("file://"):])
-            for path, dirs, files in os.walk(folder):
-                if self.stopme.isSet():
-                    return
-                uris = []
-                for afile in files:
-                    uris.append(quote_uri("file://%s" %
-                                          os.path.join(path, afile)))
-                if uris:
-                    GLib.idle_add(self.callback, uris)
+        uris = list(self._scan(self.uris))
+        if uris:
+            GLib.idle_add(self.callback, uris)
 
     def abort(self):
         self.stopme.set()
diff --git a/pre-commit.hook b/pre-commit.hook
index 8904151..11076e9 100755
--- a/pre-commit.hook
+++ b/pre-commit.hook
@@ -53,7 +53,6 @@ tests/test_clipproperties.py
 tests/test_common.py
 tests/test_log.py
 tests/test_media_library.py
-tests/test_misc.py
 tests/test_prefs.py
 tests/test_preset.py
 tests/test_previewers.py
diff --git a/tests/common.py b/tests/common.py
index 2e13788..cccc31d 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -248,9 +248,8 @@ def created_project_file(asset_uri="file:///icantpossiblyexist.png"):
 
 
 def get_sample_uri(sample):
-    assets_dir = os.path.dirname(os.path.abspath(__file__))
-
-    return "file://%s" % os.path.join(assets_dir, "samples", sample)
+    tests_dir = os.path.dirname(os.path.abspath(__file__))
+    return Gst.filename_to_uri(os.path.join(tests_dir, "samples", sample))
 
 
 def clean_proxy_samples():
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 4229e58..9627095 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -16,27 +16,73 @@
 # License along with this program; if not, write to the
 # Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
 # Boston, MA 02110-1301, USA.
+"""Tests for the utils.misc module."""
+# pylint: disable=protected-access,no-self-use
+import os
 import unittest
 
+from gi.repository import Gst
+
 from pitivi.utils.misc import binary_search
+from pitivi.utils.misc import PathWalker
+from tests.common import create_main_loop
+from tests.common import get_sample_uri
 
 
 class BinarySearchTest(unittest.TestCase):
+    """Tests for the `binary_search` method."""
 
-    def testEmptyList(self):
+    def test_empty_list(self):
+        """Checks the result when the list is empty."""
         self.assertEqual(binary_search([], 10), -1)
 
-    def testExisting(self):
-        A = [10, 20, 30]
-        for index, element in enumerate(A):
-            self.assertEqual(binary_search([10, 20, 30], element), index)
+    def test_existing(self):
+        """Checks the result when the element is present."""
+        self.assertEqual(binary_search([10, 20, 30], 10), 0)
+        self.assertEqual(binary_search([10, 20, 30], 20), 1)
+        self.assertEqual(binary_search([10, 20, 30], 30), 2)
 
-    def testMissingLeft(self):
+    def test_missing(self):
+        """Checks the result when the element is missing."""
         self.assertEqual(binary_search([10, 20, 30], 1), 0)
-        self.assertEqual(binary_search([10, 20, 30], 16), 1)
-        self.assertEqual(binary_search([10, 20, 30], 29), 2)
-
-    def testMissingRight(self):
         self.assertEqual(binary_search([10, 20, 30], 11), 0)
+        self.assertEqual(binary_search([10, 20, 30], 16), 1)
         self.assertEqual(binary_search([10, 20, 30], 24), 1)
+        self.assertEqual(binary_search([10, 20, 30], 29), 2)
         self.assertEqual(binary_search([10, 20, 30], 40), 2)
+
+
+class PathWalkerTest(unittest.TestCase):
+    """Tests for the `PathWalker` class."""
+
+    def _scan(self, uris):
+        """Uses the PathWalker to scan URIs."""
+        mainloop = create_main_loop()
+        received_uris = []
+
+        def done_cb(uris):  # pylint: disable=missing-docstring
+            received_uris.extend(uris)
+            mainloop.quit()
+        walker = PathWalker(uris, done_cb)
+        walker.run()
+        mainloop.run()
+        return received_uris
+
+    def test_scanning(self):
+        """Checks the scanning of the URIs."""
+        valid_uri = get_sample_uri("tears_of_steel.webm")
+        uris = self._scan([valid_uri,
+                           get_sample_uri("missing.webm"),
+                           "http://pitivi.org/very_real.webm";])
+        self.assertEqual(len(uris), 1, uris)
+        self.assertIn(valid_uri, uris)
+
+    def test_scanning_dir(self):
+        """Checks the scanning of the directory URIs."""
+        assets_dir = os.path.dirname(os.path.abspath(__file__))
+        valid_dir_uri = Gst.filename_to_uri(os.path.join(assets_dir, "samples"))
+        uris = [valid_dir_uri]
+        received_uris = self._scan(uris)
+        self.assertGreater(len(received_uris), 1, received_uris)
+        valid_uri = get_sample_uri("tears_of_steel.webm")
+        self.assertIn(valid_uri, received_uris)