Fixed regression that would crash trash-restore on non parseable tras…

…hinfo, with an error like: "TypeError: not enough arguments for format string"

The regression was introduced in commit 006774e (Improved error message when TrashInfo is not parsable in trash-restore Andrea Francia 04/05/23, 18:23)

tests/support/put/fake_fs/fake_fs.py

@@ -1,6 +1,8 @@
 import errno
 import os
 
+from typing import cast
+
 from tests.support.fakes.fake_volume_of import FakeVolumeOf
 from tests.support.put.fake_fs.directory import Directory
 from tests.support.put.fake_fs.directory import make_inode_dir
@@ -58,6 +60,9 @@ def mkdir(self, path):
         directory = self.get_entity_at(dirname)
         directory.add_dir(basename, 0o755, path)
 
+    def mkdir_p(self, path):
+        self.makedirs(path, 0o755)
+
     def get_entity_at(self, path):  # type: (str) -> Ent
         inode = check_cast(INode, self.get_entry_at(path))
         return inode.entity
@@ -104,8 +109,10 @@ def read(self, path):
         if isinstance(entry, SymLink):
             link_target = self.readlink(path)
             return self.read(os.path.join(dirname, link_target))
+        elif isinstance(as_inode(entry).entity, File):
+            return cast(File, as_inode(entry).entity).content
         else:
-            return as_inode(entry).reg_file().content
+            raise IOError("Unable to read: %s" % path)
 
     def readlink(self, path):
         path = self._join_cwd(path)

tests/support/put/fake_fs/inode.py

@@ -28,7 +28,3 @@ def __repr__(self):
     def directory(self):
         from tests.support.put.fake_fs.directory import Directory
         return check_cast(Directory, self.entity)
-
-    def reg_file(self):
-        from tests.support.put.fake_fs.file import File
-        return check_cast(File, self.entity)

tests/test_restore/components/trashed_files/test_trashed_files.py

@@ -1,34 +1,95 @@
 import datetime
 import unittest
 
-from mock import Mock
+from six import StringIO
 
-from trashcli.restore.file_system import FakeFileReader
-from trashcli.restore.info_dir_searcher import InfoDirSearcher, FileFound
+from tests.support.put.fake_fs.fake_fs import FakeFs
+from tests.test_restore.support.fake_logger import FakeLogger
+from tests.test_restore.support.restore_fake_fs import RestoreFakeFs
+from trashcli.restore.info_dir_searcher import InfoDirSearcher
+from trashcli.restore.info_files import InfoFiles
+from trashcli.restore.trash_directories import TrashDirectories
+from trashcli.restore.trashed_file import TrashedFile
 from trashcli.restore.trashed_files import TrashedFiles
 
 
 class TestTrashedFiles(unittest.TestCase):
     def setUp(self):
-        self.file_reader = FakeFileReader()
-        self.logger = Mock(spec=[])
-        self.searcher = Mock(spec=InfoDirSearcher)
+        self.fs = FakeFs()
+        self.out = StringIO()
+        self.logger = FakeLogger(self.out)
+
+        class FakeTrashDirectories(TrashDirectories):
+            def list_trash_dirs(self, trash_dir_from_cli):
+                return [('/trash-dir', '/')]
+
+        self.searcher = InfoDirSearcher(FakeTrashDirectories(),
+                                        InfoFiles(RestoreFakeFs(self.fs)))
         self.trashed_files = TrashedFiles(self.logger,
-                                          self.file_reader,
+                                          RestoreFakeFs(self.fs),
                                           self.searcher)
+        self.fs.mkdir_p("/trash-dir/info")
 
     def test(self):
-        self.searcher.all_file_in_info_dir.return_value = [
-            FileFound('trashinfo', 'info/info_path.trashinfo', '/volume')
-        ]
-        self.file_reader.set_content(
-            'Path=name\nDeletionDate=2001-01-01T10:10:10')
+        self.fs.write_file('/trash-dir/info/info_path.trashinfo',
+                           'Path=name\n'
+                           'DeletionDate=2001-01-01T10:10:10')
+
+        trashed_files = list(self.trashed_files.all_trashed_files(None))
+
+        assert {
+                   'trashed_files': trashed_files,
+                   'out': self.out.getvalue()} == {
+                   'trashed_files': [
+                       TrashedFile('/name',
+                                   datetime.datetime(2001, 1, 1,
+                                                     10, 10,
+                                                     10),
+                                   '/trash-dir/info/info_path.trashinfo',
+                                   '/trash-dir/files/info_path'),
+                   ],
+                   'out': ''
+               }
+
+    def test_on_non_trashinfo(self):
+        self.fs.touch('/trash-dir/info/info_path.non-trashinfo')
+
+        trashed_files = list(self.trashed_files.all_trashed_files(None))
+
+        assert {
+                   'trashed_files': trashed_files,
+                   'out': self.out.getvalue()} == {
+                   'trashed_files': [],
+                   'out': 'WARN: Non .trashinfo file in info dir\n'
+               }
+
+    def test_on_non_parsable_trashinfo(self):
+        self.fs.write_file('/trash-dir/info/info_path.trashinfo',
+                           '')
+
+        trashed_files = list(self.trashed_files.all_trashed_files(None))
+
+        assert {
+                   'trashed_files': trashed_files,
+                   'out': self.out.getvalue()} == {
+                   'trashed_files': [],
+                   'out': 'WARN: Non parsable trashinfo file: '
+                          '/trash-dir/info/info_path.trashinfo, because '
+                          'Unable to parse Path\n'
+               }
+
+    def test_on_io_error(self):
+        self.fs.mkdir_p('/trash-dir/info/info_path.trashinfo')
 
         trashed_files = list(self.trashed_files.all_trashed_files(None))
 
-        trashed_file = trashed_files[0]
-        assert '/volume/name' == trashed_file.original_location
-        assert (datetime.datetime(2001, 1, 1, 10, 10, 10) ==
-                trashed_file.deletion_date)
-        assert 'info/info_path.trashinfo' == trashed_file.info_file
-        assert 'files/info_path' == trashed_file.original_file
+        assert {
+                   'trashed_files': trashed_files,
+                   'out': self.out.getvalue()
+               } == {
+                   'trashed_files': [],
+                   'out': "WARN: IOErrorReadingTrashInfo("
+                          "path='/trash-dir/info/info_path.trashinfo', "
+                          "error='Unable to read: "
+                          "/trash-dir/info/info_path.trashinfo')\n"
+               }

tests/test_restore/support/fake_logger.py

@@ -0,0 +1,10 @@
+from __future__ import print_function
+from trashcli.restore.trashed_files import RestoreLogger
+
+
+class FakeLogger(RestoreLogger):
+    def __init__(self, out):
+        self.out = out
+
+    def warning(self, message):
+        print("WARN: %s" % message, file=self.out)

tests/test_restore/support/restore_fake_fs.py

@@ -0,0 +1,21 @@
+import os
+
+from typing import Iterable
+
+from trashcli.restore.file_system import FileReader
+from trashcli.restore.file_system import ListingFileSystem
+
+
+class RestoreFakeFs(FileReader, ListingFileSystem):
+    def __init__(self,
+                 fs,  # type FakeFs
+                 ):
+        self.fs = fs
+
+    def contents_of(self, path):
+        return self.fs.read(path)
+
+    def list_files_in_dir(self, path):  # type: (str) -> Iterable[str]
+        for entry in self.fs.listdir(path):
+            result = os.path.join(path, entry)
+            yield result

trashcli/restore/main.py

@@ -8,6 +8,7 @@
     RealListingFileSystem
 from .info_dir_searcher import InfoDirSearcher
 from .info_files import InfoFiles
+from .real_restore_logger import RealRestoreLogger
 from .restore_cmd import RestoreCmd
 from .trash_directories import TrashDirectoriesImpl
 from .trashed_files import TrashedFiles
@@ -23,7 +24,7 @@ def main():
                                              os.getuid(),
                                              os.environ)
     searcher = InfoDirSearcher(trash_directories, info_files)
-    trashed_files = TrashedFiles(my_logger,
+    trashed_files = TrashedFiles(RealRestoreLogger(my_logger),
                                  RealFileReader(),
                                  searcher)
     RestoreCmd.make(

trashcli/restore/real_restore_logger.py

@@ -0,0 +1,13 @@
+from logging import Logger
+
+from trashcli.restore.restore_logger import RestoreLogger
+
+
+class RealRestoreLogger(RestoreLogger):
+    def __init__(self,
+                 logger,  # type: Logger
+                 ):
+        self._logger = logger
+
+    def warning(self, message):
+        self._logger.warning(message)

trashcli/restore/restore_logger.py

@@ -0,0 +1,6 @@
+from trashcli.compat import Protocol
+
+
+class RestoreLogger(Protocol):
+    def warning(self, message):
+        raise NotImplementedError

trashcli/restore/trashed_files.py

@@ -1,18 +1,21 @@
-from logging import Logger
-from typing import Optional, Iterable, NamedTuple, Union
+from typing import Iterable
+from typing import NamedTuple
+from typing import Optional
+from typing import Union
 
 from trashcli.lib.path_of_backup_copy import path_of_backup_copy
 from trashcli.parse_trashinfo.parse_deletion_date import parse_deletion_date
 from trashcli.parse_trashinfo.parse_original_location import \
     parse_original_location
 from trashcli.restore.file_system import FileReader
 from trashcli.restore.info_dir_searcher import InfoDirSearcher
+from trashcli.restore.restore_logger import RestoreLogger
 from trashcli.restore.trashed_file import TrashedFile
 
 
 class TrashedFiles:
     def __init__(self,
-                 logger,  # type: Logger
+                 logger,  # type: RestoreLogger
                  file_reader,  # type: FileReader
                  searcher,  # type: InfoDirSearcher
                  ):
@@ -29,7 +32,7 @@ def all_trashed_files(self,
             elif type(event) is NonParsableTrashInfo:
                 self.logger.warning(
                     "Non parsable trashinfo file: %s, because %s" %
-                    event.path, event.reason)
+                    (event.path, event.reason))
             elif type(event) is IOErrorReadingTrashInfo:
                 self.logger.warning(str(event))
             elif type(event) is TrashedFileFound: