Fix issue #160 (TarFS implicit directory bug) (#161)

* Add regression tests for issue #160 (TarFS missing dirs bug)

Signed-off-by: Martin Larralde <[email protected]>

* Fix issue #160

* Use stable deduplication and remove dead code in `ReadTarFS.listdir`

Author: Martin Larralde

fs/tarfs.py

@@ -16,7 +16,7 @@
 from .info import Info
 from .iotools import RawWrapper
 from .opener import open_fs
-from .path import dirname, normpath, relpath, basename
+from .path import dirname, relpath, basename, isbase, parts, frombase
 from .wrapfs import WrapFS
 from .permissions import Permissions
 
@@ -256,9 +256,14 @@ def getinfo(self, path, namespaces=None):
 
         else:
             try:
+                implicit = False
                 member = self._tar.getmember(self._encode(_path))
             except KeyError:
-                raise errors.ResourceNotFound(path)
+                if not self.isdir(_path):
+                    raise errors.ResourceNotFound(path)
+                implicit = True
+                member = tarfile.TarInfo(_path)
+                member.type = tarfile.DIRTYPE
 
             raw_info["basic"] = {
                 "name": basename(self._decode(member.name)),
@@ -268,18 +273,19 @@ def getinfo(self, path, namespaces=None):
             if "details" in namespaces:
                 raw_info["details"] = {
                     "size": member.size,
-                    "type": int(self.type_map[member.type]),
-                    "modified": member.mtime,
+                    "type": int(self.type_map[member.type])
                 }
-            if "access" in namespaces:
+                if not implicit:
+                    raw_info["details"]["modified"] = member.mtime
+            if "access" in namespaces and not implicit:
                 raw_info["access"] = {
                     "gid": member.gid,
                     "group": member.gname,
                     "permissions": Permissions(mode=member.mode).dump(),
                     "uid": member.uid,
                     "user": member.uname,
                 }
-            if "tar" in namespaces:
+            if "tar" in namespaces and not implicit:
                 raw_info["tar"] = _get_member_info(member, self.encoding)
                 raw_info["tar"].update({
                     k.replace('is', 'is_'):getattr(member, k)()
@@ -289,39 +295,46 @@ def getinfo(self, path, namespaces=None):
 
         return Info(raw_info)
 
+    def isdir(self, path):
+        _path = relpath(self.validatepath(path))
+        try:
+            return self._directory[_path].isdir()
+        except KeyError:
+            return any(isbase(_path, name) for name in self._directory)
+
+    def isfile(self, path):
+        _path = relpath(self.validatepath(path))
+        try:
+            return self._directory[_path].isfile()
+        except KeyError:
+            return False
+
     def setinfo(self, path, info):
         self.check()
         raise errors.ResourceReadOnly(path)
 
     def listdir(self, path):
-        self.check()
-        _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
+        _path = relpath(self.validatepath(path))
+
+        if not self.gettype(path) is ResourceType.directory:
+            raise errors.DirectoryExpected(path)
+
+        children = (frombase(_path, n) for n in self._directory if isbase(_path, n))
+        content = (parts(child)[1] for child in children if relpath(child))
+        return list(OrderedDict.fromkeys(content))
 
     def makedir(self, path, permissions=None, recreate=False):
         self.check()
         raise errors.ResourceReadOnly(path)
 
     def openbin(self, path, mode="r", buffering=-1, **options):
-        self.check()
-        path = relpath(normpath(path))
+        _path = relpath(self.validatepath(path))
 
         if 'w' in mode or '+' in mode or 'a' in mode:
             raise errors.ResourceReadOnly(path)
 
         try:
-            member = self._tar.getmember(self._encode(path))
+            member = self._tar.getmember(self._encode(_path))
         except KeyError:
             six.raise_from(errors.ResourceNotFound(path), None)
 

tests/test_tarfs.py

@@ -1,16 +1,20 @@
 # -*- encoding: UTF-8
 from __future__ import unicode_literals
 
+import io
 import os
 import six
 import gzip
 import tarfile
 import getpass
+import tarfile
 import tempfile
 import unittest
+import uuid
 
 from fs import tarfs
 from fs import errors
+from fs.enums import ResourceType
 from fs.compress import write_tar
 from fs.opener import open_fs
 from fs.opener.errors import NotWriteable
@@ -184,6 +188,73 @@ def test_getinfo(self):
         self.assertTrue(top.get('tar', 'is_file'))
 
 
+class TestImplicitDirectories(unittest.TestCase):
+    """Regression tests for #160.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        cls.tmpfs = open_fs("temp://")
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.tmpfs.close()
+
+    def setUp(self):
+        self.tempfile = self.tmpfs.open('test.tar', 'wb+')
+        with tarfile.open(mode="w", fileobj=self.tempfile) as tf:
+            tf.addfile(tarfile.TarInfo("foo/bar/baz/spam.txt"), io.StringIO())
+            tf.addfile(tarfile.TarInfo("foo/eggs.bin"), io.StringIO())
+            tf.addfile(tarfile.TarInfo("foo/yolk/beans.txt"), io.StringIO())
+            info = tarfile.TarInfo("foo/yolk")
+            info.type = tarfile.DIRTYPE
+            tf.addfile(info, io.BytesIO())
+        self.tempfile.seek(0)
+        self.fs = tarfs.TarFS(self.tempfile)
+
+    def tearDown(self):
+        self.fs.close()
+        self.tempfile.close()
+
+    def test_isfile(self):
+        self.assertFalse(self.fs.isfile("foo"))
+        self.assertFalse(self.fs.isfile("foo/bar"))
+        self.assertFalse(self.fs.isfile("foo/bar/baz"))
+        self.assertTrue(self.fs.isfile("foo/bar/baz/spam.txt"))
+        self.assertTrue(self.fs.isfile("foo/yolk/beans.txt"))
+        self.assertTrue(self.fs.isfile("foo/eggs.bin"))
+        self.assertFalse(self.fs.isfile("foo/eggs.bin/baz"))
+
+    def test_isdir(self):
+        self.assertTrue(self.fs.isdir("foo"))
+        self.assertTrue(self.fs.isdir("foo/yolk"))
+        self.assertTrue(self.fs.isdir("foo/bar"))
+        self.assertTrue(self.fs.isdir("foo/bar/baz"))
+        self.assertFalse(self.fs.isdir("foo/bar/baz/spam.txt"))
+        self.assertFalse(self.fs.isdir("foo/eggs.bin"))
+        self.assertFalse(self.fs.isdir("foo/eggs.bin/baz"))
+        self.assertFalse(self.fs.isdir("foo/yolk/beans.txt"))
+
+    def test_listdir(self):
+        self.assertEqual(sorted(self.fs.listdir("foo")), ["bar", "eggs.bin", "yolk"])
+        self.assertEqual(self.fs.listdir("foo/bar"), ["baz"])
+        self.assertEqual(self.fs.listdir("foo/bar/baz"), ["spam.txt"])
+        self.assertEqual(self.fs.listdir("foo/yolk"), ["beans.txt"])
+
+    def test_getinfo(self):
+        info = self.fs.getdetails("foo/bar/baz")
+        self.assertEqual(info.name, "baz")
+        self.assertEqual(info.size, 0)
+        self.assertIs(info.type, ResourceType.directory)
+
+        info = self.fs.getdetails("foo")
+        self.assertEqual(info.name, "foo")
+        self.assertEqual(info.size, 0)
+        self.assertIs(info.type, ResourceType.directory)
+
+
+
+
 class TestReadTarFSMem(TestReadTarFS):
 
     def make_source_fs(self):