From 9a54522cc9cc566d8a6d071708d3d85930132356 Mon Sep 17 00:00:00 2001
From: Julian Mann <julian.mann@gmail.com>
Date: Thu, 19 Mar 2020 15:52:41 -0500
Subject: [PATCH 1/4] Rename upload_helpers to fixtures

---
 tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/adsf   | 0
 .../a_subdir/another_subdir/even_another_subdir/ppp             | 0
 .../many_subdirs/a_subdir/another_subdir/oimsdf                 | 0
 tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/three  | 0
 tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/two    | 0
 tests/{upload_helpers => fixtures}/many_subdirs/bar             | 0
 tests/{upload_helpers => fixtures}/many_subdirs/one             | 0
 tests/{upload_helpers => fixtures}/one_subdir/a_subdir/three    | 0
 tests/{upload_helpers => fixtures}/one_subdir/a_subdir/two      | 0
 tests/{upload_helpers => fixtures}/one_subdir/one               | 0
 tests/{upload_helpers => fixtures}/one_symlink/many_subdirs     | 0
 tests/{upload_helpers => fixtures}/one_symlink/nnn              | 0
 tests/{upload_helpers => fixtures}/single_file/foo              | 0
 tests/{upload_helpers => fixtures}/two_files/one                | 0
 tests/{upload_helpers => fixtures}/two_files/two                | 0
 tests/fixtures/upload_file1                                     | 1 +
 tests/fixtures/upload_file1asdfasdf                             | 1 +
 tests/fixtures/upload_file2                                     | 1 +
 tests/{upload_helpers => fixtures}/upload_file3                 | 2 +-
 tests/test_uploads.py                                           | 2 +-
 tests/upload_helpers/upload_file1                               | 1 -
 tests/upload_helpers/upload_file1asdfasdf                       | 1 -
 tests/upload_helpers/upload_file2                               | 1 -
 23 files changed, 5 insertions(+), 5 deletions(-)
 rename tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/adsf (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/another_subdir/even_another_subdir/ppp (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/another_subdir/oimsdf (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/three (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/a_subdir/two (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/bar (100%)
 rename tests/{upload_helpers => fixtures}/many_subdirs/one (100%)
 rename tests/{upload_helpers => fixtures}/one_subdir/a_subdir/three (100%)
 rename tests/{upload_helpers => fixtures}/one_subdir/a_subdir/two (100%)
 rename tests/{upload_helpers => fixtures}/one_subdir/one (100%)
 rename tests/{upload_helpers => fixtures}/one_symlink/many_subdirs (100%)
 rename tests/{upload_helpers => fixtures}/one_symlink/nnn (100%)
 rename tests/{upload_helpers => fixtures}/single_file/foo (100%)
 rename tests/{upload_helpers => fixtures}/two_files/one (100%)
 rename tests/{upload_helpers => fixtures}/two_files/two (100%)
 create mode 100644 tests/fixtures/upload_file1
 create mode 100644 tests/fixtures/upload_file1asdfasdf
 create mode 100644 tests/fixtures/upload_file2
 rename tests/{upload_helpers => fixtures}/upload_file3 (65%)
 delete mode 100644 tests/upload_helpers/upload_file1
 delete mode 100644 tests/upload_helpers/upload_file1asdfasdf
 delete mode 100644 tests/upload_helpers/upload_file2

diff --git a/tests/upload_helpers/many_subdirs/a_subdir/adsf b/tests/fixtures/many_subdirs/a_subdir/adsf
similarity index 100%
rename from tests/upload_helpers/many_subdirs/a_subdir/adsf
rename to tests/fixtures/many_subdirs/a_subdir/adsf
diff --git a/tests/upload_helpers/many_subdirs/a_subdir/another_subdir/even_another_subdir/ppp b/tests/fixtures/many_subdirs/a_subdir/another_subdir/even_another_subdir/ppp
similarity index 100%
rename from tests/upload_helpers/many_subdirs/a_subdir/another_subdir/even_another_subdir/ppp
rename to tests/fixtures/many_subdirs/a_subdir/another_subdir/even_another_subdir/ppp
diff --git a/tests/upload_helpers/many_subdirs/a_subdir/another_subdir/oimsdf b/tests/fixtures/many_subdirs/a_subdir/another_subdir/oimsdf
similarity index 100%
rename from tests/upload_helpers/many_subdirs/a_subdir/another_subdir/oimsdf
rename to tests/fixtures/many_subdirs/a_subdir/another_subdir/oimsdf
diff --git a/tests/upload_helpers/many_subdirs/a_subdir/three b/tests/fixtures/many_subdirs/a_subdir/three
similarity index 100%
rename from tests/upload_helpers/many_subdirs/a_subdir/three
rename to tests/fixtures/many_subdirs/a_subdir/three
diff --git a/tests/upload_helpers/many_subdirs/a_subdir/two b/tests/fixtures/many_subdirs/a_subdir/two
similarity index 100%
rename from tests/upload_helpers/many_subdirs/a_subdir/two
rename to tests/fixtures/many_subdirs/a_subdir/two
diff --git a/tests/upload_helpers/many_subdirs/bar b/tests/fixtures/many_subdirs/bar
similarity index 100%
rename from tests/upload_helpers/many_subdirs/bar
rename to tests/fixtures/many_subdirs/bar
diff --git a/tests/upload_helpers/many_subdirs/one b/tests/fixtures/many_subdirs/one
similarity index 100%
rename from tests/upload_helpers/many_subdirs/one
rename to tests/fixtures/many_subdirs/one
diff --git a/tests/upload_helpers/one_subdir/a_subdir/three b/tests/fixtures/one_subdir/a_subdir/three
similarity index 100%
rename from tests/upload_helpers/one_subdir/a_subdir/three
rename to tests/fixtures/one_subdir/a_subdir/three
diff --git a/tests/upload_helpers/one_subdir/a_subdir/two b/tests/fixtures/one_subdir/a_subdir/two
similarity index 100%
rename from tests/upload_helpers/one_subdir/a_subdir/two
rename to tests/fixtures/one_subdir/a_subdir/two
diff --git a/tests/upload_helpers/one_subdir/one b/tests/fixtures/one_subdir/one
similarity index 100%
rename from tests/upload_helpers/one_subdir/one
rename to tests/fixtures/one_subdir/one
diff --git a/tests/upload_helpers/one_symlink/many_subdirs b/tests/fixtures/one_symlink/many_subdirs
similarity index 100%
rename from tests/upload_helpers/one_symlink/many_subdirs
rename to tests/fixtures/one_symlink/many_subdirs
diff --git a/tests/upload_helpers/one_symlink/nnn b/tests/fixtures/one_symlink/nnn
similarity index 100%
rename from tests/upload_helpers/one_symlink/nnn
rename to tests/fixtures/one_symlink/nnn
diff --git a/tests/upload_helpers/single_file/foo b/tests/fixtures/single_file/foo
similarity index 100%
rename from tests/upload_helpers/single_file/foo
rename to tests/fixtures/single_file/foo
diff --git a/tests/upload_helpers/two_files/one b/tests/fixtures/two_files/one
similarity index 100%
rename from tests/upload_helpers/two_files/one
rename to tests/fixtures/two_files/one
diff --git a/tests/upload_helpers/two_files/two b/tests/fixtures/two_files/two
similarity index 100%
rename from tests/upload_helpers/two_files/two
rename to tests/fixtures/two_files/two
diff --git a/tests/fixtures/upload_file1 b/tests/fixtures/upload_file1
new file mode 100644
index 00000000..9e8df757
--- /dev/null
+++ b/tests/fixtures/upload_file1
@@ -0,0 +1 @@
+fixtures/single_file
diff --git a/tests/fixtures/upload_file1asdfasdf b/tests/fixtures/upload_file1asdfasdf
new file mode 100644
index 00000000..9e8df757
--- /dev/null
+++ b/tests/fixtures/upload_file1asdfasdf
@@ -0,0 +1 @@
+fixtures/single_file
diff --git a/tests/fixtures/upload_file2 b/tests/fixtures/upload_file2
new file mode 100644
index 00000000..c3cdd030
--- /dev/null
+++ b/tests/fixtures/upload_file2
@@ -0,0 +1 @@
+fixtures/single_file,fixtures/one_symlink
diff --git a/tests/upload_helpers/upload_file3 b/tests/fixtures/upload_file3
similarity index 65%
rename from tests/upload_helpers/upload_file3
rename to tests/fixtures/upload_file3
index 145acadc..0d72a711 100644
--- a/tests/upload_helpers/upload_file3
+++ b/tests/fixtures/upload_file3
@@ -1 +1 @@
-/Users/martin/Dropbox/conductor/src/conductor_ae/src/client/tests/upload_helpers/single_file/foo
+/Users/martin/Dropbox/conductor/src/conductor_ae/src/client/tests/fixtures/single_file/foo
diff --git a/tests/test_uploads.py b/tests/test_uploads.py
index c1dbc54c..96201e22 100644
--- a/tests/test_uploads.py
+++ b/tests/test_uploads.py
@@ -19,7 +19,7 @@
 base_dir = os.path.dirname(test_dir)
 sys.path.append(base_dir)
 
-upload_test_helpers = os.path.join(test_dir,'upload_helpers')
+upload_test_helpers = os.path.join(test_dir,'fixtures')
 
 os.environ['FLASK_CONF'] = 'TEST' # disable retries n' stuff
 # os.environ['CONDUCTOR_DEVELOPMENT'] = '1' # uncomment this line to get debug messages
diff --git a/tests/upload_helpers/upload_file1 b/tests/upload_helpers/upload_file1
deleted file mode 100644
index 52d23d3d..00000000
--- a/tests/upload_helpers/upload_file1
+++ /dev/null
@@ -1 +0,0 @@
-upload_helpers/single_file
diff --git a/tests/upload_helpers/upload_file1asdfasdf b/tests/upload_helpers/upload_file1asdfasdf
deleted file mode 100644
index 52d23d3d..00000000
--- a/tests/upload_helpers/upload_file1asdfasdf
+++ /dev/null
@@ -1 +0,0 @@
-upload_helpers/single_file
diff --git a/tests/upload_helpers/upload_file2 b/tests/upload_helpers/upload_file2
deleted file mode 100644
index 27c3c774..00000000
--- a/tests/upload_helpers/upload_file2
+++ /dev/null
@@ -1 +0,0 @@
-upload_helpers/single_file,upload_helpers/one_symlink

From 6813df69642532baa5c2845ef404865d47136f7c Mon Sep 17 00:00:00 2001
From: Julian Mann <julian.mann@gmail.com>
Date: Thu, 19 Mar 2020 16:09:30 -0500
Subject: [PATCH 2/4] MD5Worker can print msgs containing unicode fn

---
 conductor/lib/uploader.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/conductor/lib/uploader.py b/conductor/lib/uploader.py
index 7eae929f..1a11c93c 100644
--- a/conductor/lib/uploader.py
+++ b/conductor/lib/uploader.py
@@ -33,13 +33,13 @@ def do_work(self, job, thread_int):
         logger.debug('job is %s', job)
         filename, submission_time_md5 = job
         assert isinstance(filename, (str, unicode)), "Filepath not of expected type. Got %s" % type(filename)
-        filename = str(filename)
+        encoded_filename = filename.encode("utf8")
         current_md5 = self.get_md5(filename)
         # if a submission time md5 was provided then check against it
         if submission_time_md5:
-            logger.info("Enforcing md5 match: %s for: %s", submission_time_md5, filename)
+            logger.info("Enforcing md5 match: %s for: %s", submission_time_md5, encoded_filename)
             if current_md5 != submission_time_md5:
-                message = 'MD5 of %s has changed since submission\n' % filename
+                message = 'MD5 of %s has changed since submission\n' % encoded_filename
                 message += 'submitted md5: %s\n' % submission_time_md5
                 message += 'current md5:   %s\n' % current_md5
                 message += 'This is likely due to the file being written to after the user submitted the job but before it got uploaded to conductor'

From 2e3c4f74131829f363cb100ace8ef936e4557133 Mon Sep 17 00:00:00 2001
From: Julian Mann <julian.mann@gmail.com>
Date: Thu, 19 Mar 2020 17:25:37 -0500
Subject: [PATCH 3/4] adds sheild for unicode in dependency filenames

---
 conductor/lib/file_utils.py                   |  7 +++++
 .../unicode_files/unicode_\304\243.jpg"       |  1 +
 tests/test_file_utils.py                      | 29 ++++++++++++++-----
 3 files changed, 29 insertions(+), 8 deletions(-)
 create mode 100644 "tests/fixtures/unicode_files/unicode_\304\243.jpg"

diff --git a/conductor/lib/file_utils.py b/conductor/lib/file_utils.py
index bef8de20..556e6554 100644
--- a/conductor/lib/file_utils.py
+++ b/conductor/lib/file_utils.py
@@ -157,6 +157,13 @@ def process_upload_filepath(path, strict=True):
     paths = []
 
     if path:
+        try:
+            str(path)
+        except UnicodeEncodeError:
+            message = "Unicode filenames are not supported: %s" % path
+            if strict:
+                raise exceptions.InvalidPathException(message)
+            logger.warning(message)
 
         # If the path is a file (and it exits)
         if os.path.isfile(path):
diff --git "a/tests/fixtures/unicode_files/unicode_\304\243.jpg" "b/tests/fixtures/unicode_files/unicode_\304\243.jpg"
new file mode 100644
index 00000000..092bfb9b
--- /dev/null
+++ "b/tests/fixtures/unicode_files/unicode_\304\243.jpg"
@@ -0,0 +1 @@
+yo
diff --git a/tests/test_file_utils.py b/tests/test_file_utils.py
index b64a164b..11000039 100644
--- a/tests/test_file_utils.py
+++ b/tests/test_file_utils.py
@@ -2,29 +2,42 @@
 
    isort:skip_file
 """
+import os
 
 import unittest
 import conductor.lib.file_utils as futil
 import logging
 
 
+fixtures_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "fixtures")
+
+
 class ProcessDependenciesTest(unittest.TestCase):
+    def setUp(self):
+        logger = logging.getLogger("conductor")
+        logger.setLevel("DEBUG")
+
     def test_regular_filename(self):
         paths = ["/path/to/filename.txt"]
         deps = futil.process_dependencies(paths)
-        self.assertTrue("/path/to/filename.txt" in deps)
+        self.assertIn("/path/to/filename.txt", deps)
 
     def test_it_encodes_unicode_chars_in_error_message(self):
         # make sure logging is triggered as it should also encode unicode.
-        logger = logging.getLogger("conductor")
-        logger.setLevel("DEBUG")
-
         paths = [u"/path/to/\u0123/name.txt"]
         deps = futil.process_dependencies(paths)
-        self.assertTrue(u"/path/to/\u0123/name.txt" in deps)
-        self.assertTrue(
-            "/path/to/\xc4\xa3/name.txt" in deps[u"/path/to/\u0123/name.txt"]
-        )
+        self.assertIn(u"/path/to/\u0123/name.txt", deps)
+        self.assertIn("/path/to/\xc4\xa3/name.txt", deps[u"/path/to/\u0123/name.txt"])
+
+    def test_catches_existing_unicode_files(self):
+        unicode_dir = unicode(os.path.join(fixtures_dir, "unicode_files"), "utf8")
+        filenames = os.listdir(unicode_dir)
+        paths = [
+            os.path.join(unicode_dir, f) for f in filenames if not f.startswith(".")
+        ]
+        deps = futil.process_dependencies(paths)
+        for key in deps:
+            self.assertIn("Unicode filenames are not supported", deps[key])
 
 
 if __name__ == "__main__":

From f77d85efcc51eec45441b350be5b9a625b1e37c8 Mon Sep 17 00:00:00 2001
From: Julian Mann <julian.mann@gmail.com>
Date: Fri, 20 Mar 2020 17:23:37 -0500
Subject: [PATCH 4/4] repairs docstrings

---
 conductor/lib/file_utils.py | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/conductor/lib/file_utils.py b/conductor/lib/file_utils.py
index 556e6554..a0bfc51d 100644
--- a/conductor/lib/file_utils.py
+++ b/conductor/lib/file_utils.py
@@ -116,16 +116,17 @@ def process_upload_filepaths(paths):
 
 def process_upload_filepath(path, strict=True):
     '''
-    Process the given path to ensure that the path is valid (exists on disk),
-    and return any/all files which the path may represent.
-    For example, if the path is a directory or an image sequence, then explicitly
-    list and return all files that that path represents/contains.
+    Process the given path to ensure that the path is valid: (Exists on disk AND
+    does not contain Unicode characters). Return any/all files the path may
+    represent. For example, if the path is a directory or image sequence, then
+    explicitly list and return all files that that path represents/contains.
 
 
-    strict: bool. When True and the give path does not exist on disk, raise an
+    strict: bool. When True and the path is not valid, raise an
                   exception.
                   Note that when this function is given a directory path, and
                   and it finds any broken symlinks within the directory, the
+                  recursion is not strict. See comment below.
 
 
     This function should be able to handle various types of paths:
@@ -136,19 +137,19 @@ def process_upload_filepath(path, strict=True):
 
     Process the path by doing the following:
 
-    1. If the path is  an image sequence notation, "explode" it and return
-        each frame's filepath.  This relies on the file
-        actually being on disk, as the underlying call is to glob.glob(regex).
-        Validate that there is at least one frame on disk for the image sequence.
-        There is no 100% reliable way to know how many frames should actually be
-        part of the image sequence, but we can at least validate that there is
-        a single frame.
+    1. If the path is an image sequence notation, "explode" it and return each
+        frame's filepath.  This relies on the file actually being on disk, as
+        the underlying call is to glob.glob(regex). Validate that there is at
+        least one frame on disk for the image sequence. There is no 100%
+        reliable way to know how many frames should actually be part of the
+        image sequence, but we can at least validate that there is a single
+        frame.
 
     2. If the path is a directory then recursively add all file/dir paths
         contained within it
 
-    3. If the path is a file then ensure that it exists on disk and that it conforms
-       to Conductor's expectations.
+    3. If the path is a file then ensure that it exists on disk and that it
+       conforms to Conductor's expectations.
 
 
 
@@ -161,6 +162,8 @@ def process_upload_filepath(path, strict=True):
             str(path)
         except UnicodeEncodeError:
             message = "Unicode filenames are not supported: %s" % path
+            # TODO: Figure out if this exception should always raise (if so,
+            # remove if strict:)
             if strict:
                 raise exceptions.InvalidPathException(message)
             logger.warning(message)