Repository: PyFilesystem/pyfilesystem2
Pull request #148 (2018-03-08 13:37:13)

Allow wrapping exceptions in `CreateFailed`
Hi Will,
I made it so `CreateFailed` accepts an additional `exc` argument, as discussed in #144.

I also added a decorator classmethod to `CreateFailed`, so that it is easier to wrap any errors occuring in a filesystem `__init__` method into a `CreateFailed` exception.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/15873924/badge)](https://coveralls.io/builds/15873924)

Coverage increased (+0.0002%) to 99.951% when pulling **a34807bfc65331da4b3713a619836726aefb50ab on althonos:master** into **3a691e0506a9f310ba00b9ea4ead0c2f78486b22 on PyFilesystem:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/15873924/badge)](https://coveralls.io/builds/15873924)

Coverage increased (+0.0002%) to 99.951% when pulling **a34807bfc65331da4b3713a619836726aefb50ab on althonos:master** into **3a691e0506a9f310ba00b9ea4ead0c2f78486b22 on PyFilesystem:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/15873924/badge)](https://coveralls.io/builds/15873924)

Coverage increased (+0.0002%) to 99.951% when pulling **a34807bfc65331da4b3713a619836726aefb50ab on althonos:master** into **3a691e0506a9f310ba00b9ea4ead0c2f78486b22 on PyFilesystem:master**.


Patch:
diff --git a/fs/errors.py b/fs/errors.py
index 3ed4fe4..47fd3ce 100644
--- a/fs/errors.py
+++ b/fs/errors.py
@@ -11,6 +11,8 @@ which may be used as a catch-all filesystem exception.
 from __future__ import unicode_literals
 from __future__ import print_function
 
+import functools
+
 import six
 from six import text_type
 
@@ -90,8 +92,24 @@ class CreateFailed(FSError):
     """Filesystem could not be created.
     """
 
-    default_message = "unable to create filesystem"
-
+    default_message = "unable to create filesystem, {details}"
+
+    @classmethod
+    def catch_all(cls, func):
+        @functools.wraps(func)
+        def new_func(*args, **kwargs):
+            try:
+                return func(*args, **kwargs)
+            except cls:
+                raise
+            except Exception as e:
+                raise cls(exc=e)
+        return new_func
+
+    def __init__(self, msg=None, exc=None):
+        self._msg = msg or self.default_message
+        self.details = '' if exc is None else text_type(exc)
+        self.exc = exc
 
 class PathError(FSError):
     """Base exception for errors to do with a path string.
diff --git a/fs/osfs.py b/fs/osfs.py
index 09cf2bf..f110eb0 100644
--- a/fs/osfs.py
+++ b/fs/osfs.py
@@ -88,13 +88,12 @@ class OSFS(FS):
                     os.makedirs(_root_path, mode=create_mode)
             except OSError as error:
                 raise errors.CreateFailed(
-                    'unable to create {} ({})'.format(root_path, error)
+                    'unable to create {} ({})'.format(root_path, error),
+                    error,
                 )
         else:
             if not os.path.isdir(_root_path):
-                raise errors.CreateFailed(
-                    'root path does not exist'
-                )
+                raise errors.CreateFailed('root path does not exist')
 
         _meta = self._meta = {
             'case_insensitive': os.path.normcase('Aa') != 'aa',
diff --git a/fs/tarfs.py b/fs/tarfs.py
index 24f85a7..543b203 100644
--- a/fs/tarfs.py
+++ b/fs/tarfs.py
@@ -4,6 +4,8 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+from collections import OrderedDict
+import os
 import tarfile
 import six
 
@@ -85,7 +87,11 @@ class TarFS(WrapFS):
                 encoding="utf-8",
                 temp_fs="temp://__tartemp__"):
 
-        filename = str(getattr(file, 'name', file))
+        if isinstance(file, (six.text_type, six.binary_type)):
+            file = os.path.expanduser(file)
+            filename = file
+        else:
+            filename = getattr(file, 'name', '')
 
         if write and compression is None:
             compression = None
@@ -99,8 +105,8 @@ class TarFS(WrapFS):
                               compression=compression,
                               encoding=encoding,
                               temp_fs=temp_fs)
-        # else:
-        return ReadTarFS(file, encoding=encoding)
+        else:
+            return ReadTarFS(file, encoding=encoding)
 
 
 @six.python_2_unicode_compatible
@@ -198,6 +204,7 @@ class ReadTarFS(FS):
         tarfile.LNKTYPE: ResourceType.symlink,
     }
 
+    @errors.CreateFailed.catch_all
     def __init__(self, file, encoding='utf-8'):
         super(ReadTarFS, self).__init__()
         self._file = file
@@ -207,12 +214,31 @@ class ReadTarFS(FS):
         else:
             self._tar = tarfile.open(file, mode='r')
 
+        self._directory = OrderedDict(
+            (relpath(self._decode(info.name)).rstrip('/'), info)
+            for info in self._tar
+        )
+
     def __repr__(self):
         return "ReadTarFS({!r})".format(self._file)
 
     def __str__(self):
         return "<TarFS '{}'>".format(self._file)
 
+    def _encode(self, s):
+        return(
+            s.encode(self.encoding)
+            if six.PY2 else
+            s
+        )
+
+    def _decode(self, s):
+        return(
+            s.decode(self.encoding)
+            if six.PY2 else
+            s
+        )
+
     def getinfo(self, path, namespaces=None):
         _path = relpath(self.validatepath(path))
         namespaces = namespaces or ()
@@ -230,12 +256,12 @@ class ReadTarFS(FS):
 
         else:
             try:
-                member = self._tar.getmember(_path)
+                member = self._tar.getmember(self._encode(_path))
             except KeyError:
                 raise errors.ResourceNotFound(path)
 
             raw_info["basic"] = {
-                "name": basename(member.name),
+                "name": basename(self._decode(member.name)),
                 "is_dir": member.isdir(),
             }
 
@@ -269,21 +295,19 @@ class ReadTarFS(FS):
 
     def listdir(self, path):
         self.check()
-        path = relpath(path)
-        if path:
-            try:
-                member = self._tar.getmember(path)
-            except KeyError:
-                six.raise_from(errors.ResourceNotFound(path), None)
-            else:
-                if not member.isdir():
-                    six.raise_from(errors.DirectoryExpected(path), None)
-
-        return [
-            basename(member.name)
-            for member in self._tar
-            if dirname(member.path) == path
+        _path = relpath(path)
+        info = self._directory.get(_path)
+        if _path:
+            if info is None:
+                raise errors.ResourceNotFound(path)
+            if not info.isdir():
+                raise errors.DirectoryExpected(path)
+        dir_list = [
+            basename(name)
+            for name in self._directory.keys()
+            if dirname(name) == _path
         ]
+        return dir_list
 
     def makedir(self, path, permissions=None, recreate=False):
         self.check()
@@ -297,7 +321,7 @@ class ReadTarFS(FS):
             raise errors.ResourceReadOnly(path)
 
         try:
-            member = self._tar.getmember(path)
+            member = self._tar.getmember(self._encode(path))
         except KeyError:
             six.raise_from(errors.ResourceNotFound(path), None)
 
diff --git a/fs/zipfs.py b/fs/zipfs.py
index 367597f..9588e5d 100644
--- a/fs/zipfs.py
+++ b/fs/zipfs.py
@@ -241,6 +241,7 @@ class ReadZipFS(FS):
         'virtual': False,
     }
 
+    @errors.CreateFailed.catch_all
     def __init__(self, file, encoding='utf-8'):
         super(ReadZipFS, self).__init__()
         self._file = file


Test patch:
diff --git a/tests/test_archives.py b/tests/test_archives.py
index 3b85b72..f435885 100644
--- a/tests/test_archives.py
+++ b/tests/test_archives.py
@@ -1,3 +1,4 @@
+# -*- encoding: UTF-8
 from __future__ import unicode_literals
 
 from six import text_type
@@ -18,6 +19,7 @@ class ArchiveTestCases(object):
     def build_source(self, fs):
         fs.makedirs('foo/bar/baz')
         fs.makedir('tmp')
+        fs.settext('Файл', 'unicode filename')
         fs.settext('top.txt', 'Hello, World')
         fs.settext('top2.txt', 'Hello, World')
         fs.settext('foo/bar/egg', 'foofoo')
@@ -141,3 +143,11 @@ class ArchiveTestCases(object):
     def test_implied_dir(self):
         self.fs.getinfo('foo/bar')
         self.fs.getinfo('foo')
+
+    def test_listdir(self):
+        for name in self.fs.listdir('/'):
+            self.assertIsInstance(name, text_type)
+        with self.assertRaises(errors.ResourceNotFound):
+            self.fs.listdir('nope')
+        with self.assertRaises(errors.DirectoryExpected):
+            self.fs.listdir('top.txt')
\ No newline at end of file
diff --git a/tests/test_errors.py b/tests/test_errors.py
index b27542f..4fad45b 100644
--- a/tests/test_errors.py
+++ b/tests/test_errors.py
@@ -5,6 +5,7 @@ import unittest
 from six import text_type
 
 from fs import errors
+from fs.errors import CreateFailed
 
 
 class TestErrors(unittest.TestCase):
@@ -24,3 +25,22 @@ class TestErrors(unittest.TestCase):
             text_type(err),
             "not supported"
         )
+
+
+class TestCreateFailed(unittest.TestCase):
+
+    def test_catch_all(self):
+
+        errors = (ZeroDivisionError, ValueError, CreateFailed)
+
+        @CreateFailed.catch_all
+        def test(x):
+            raise errors[x]
+
+        for index, exc in enumerate(errors):
+            try:
+                test(index)
+            except Exception as e:
+                self.assertIsInstance(e, CreateFailed)
+                if e.exc is not None:
+                    self.assertNotIsInstance(e.exc, CreateFailed)
diff --git a/tests/test_tarfs.py b/tests/test_tarfs.py
index bd763e5..e894f6b 100644
--- a/tests/test_tarfs.py
+++ b/tests/test_tarfs.py
@@ -1,3 +1,4 @@
+# -*- encoding: UTF-8
 from __future__ import unicode_literals
 
 import os
@@ -18,6 +19,27 @@ from fs.test import FSTestCases
 from .test_archives import ArchiveTestCases
 
 
+class TestWriteReadTarFS(unittest.TestCase):
+
+    def setUp(self):
+        fh, self._temp_path = tempfile.mkstemp()
+
+    def tearDown(self):
+        os.remove(self._temp_path)
+
+    def test_unicode_paths(self):
+        # https://github.com/PyFilesystem/pyfilesystem2/issues/135
+        with tarfs.TarFS(self._temp_path, write=True) as tar_fs:
+            tar_fs.settext("Файл", "some content")
+
+        with tarfs.TarFS(self._temp_path) as tar_fs:
+            paths = list(tar_fs.walk.files())
+            for path in paths:
+                self.assertIsInstance(path, six.text_type)
+                with tar_fs.openbin(path) as f:
+                    f.read()
+
+
 class TestWriteTarFS(FSTestCases, unittest.TestCase):
     """
     Test TarFS implementation.
diff --git a/tests/test_zipfs.py b/tests/test_zipfs.py
index 054b86d..dada39b 100644
--- a/tests/test_zipfs.py
+++ b/tests/test_zipfs.py
@@ -7,6 +7,8 @@ import tempfile
 import unittest
 import zipfile
 
+import six
+
 from fs import zipfs
 from fs.compress import write_zip
 from fs.opener import open_fs
@@ -34,6 +36,7 @@ class TestWriteReadZipFS(unittest.TestCase):
         with zipfs.ZipFS(self._temp_path) as zip_fs:
             paths = list(zip_fs.walk.files())
             for path in paths:
+                self.assertIsInstance(path, six.text_type)
                 with zip_fs.openbin(path) as f:
                     f.read()