fix crashes on --sort=none and missing deletion date

tests/test_restore/cmd/test_restore_sort_and_missing_date.py

@@ -0,0 +1,55 @@
+from six import StringIO
+
+from tests.support.py2mock import Mock
+from trashcli.lib.my_input import HardCodedInput
+from trashcli.restore.file_system import FakeReadCwd
+from trashcli.restore.restore_cmd import RestoreCmd
+from trashcli.restore.trashed_file import TrashedFile
+from trashcli.restore.trashed_files import TrashedFiles
+
+
+class TestRestoreSortAndMissingDate:
+    def setup_method(self):
+        self.stdout = StringIO()
+        self.trashed_files = Mock(spec=TrashedFiles)
+        self.cmd = RestoreCmd.make(
+            stdout=self.stdout, stderr=StringIO(), exit=lambda _: None,
+            input=HardCodedInput(''), version="0.0.0",
+            trashed_files=self.trashed_files, read_fs=Mock(spec=[]),
+            write_fs=Mock(spec=[]), read_cwd=FakeReadCwd("/home/u"))
+
+    def given_trashed_files(self, *files):
+        # a generator, like the real collaborator, to catch re-iteration bugs
+        self.trashed_files.all_trashed_files = \
+            lambda _path=None: (f for f in files)
+
+    def run_restore(self, *args):
+        self.cmd.run(['trash-restore'] + list(args))
+        return self.stdout.getvalue()
+
+    def test_sort_none_lists_without_crashing(self):
+        self.given_trashed_files(a_trashed_file("/home/u/a", dated('2020-01-01')))
+
+        assert '/home/u/a' in self.run_restore('--sort=none')
+
+    def test_a_missing_date_does_not_crash_the_default_sort(self):
+        self.given_trashed_files(a_trashed_file("/home/u/a", None),
+                                 a_trashed_file("/home/u/b", dated('2020-01-01')))
+
+        assert '/home/u/a' in self.run_restore()
+
+    def test_a_missing_date_is_listed_with_a_placeholder(self):
+        self.given_trashed_files(a_trashed_file("/home/u/a", None))
+
+        assert '????-??-?? ??:??:?? /home/u/a' in self.run_restore()
+
+
+def dated(text):
+    import datetime
+    return datetime.datetime.strptime(text, '%Y-%m-%d')
+
+
+def a_trashed_file(original_location, deletion_date):
+    return TrashedFile(original_location=original_location,
+                       deletion_date=deletion_date,
+                       info_file="/info", original_file="/orig")

tests/test_restore/components/test_sort_method.py

@@ -0,0 +1,55 @@
+import datetime
+
+from trashcli.restore.args import Sort
+from trashcli.restore.sort_method import sort_files
+from trashcli.restore.trashed_file import TrashedFile
+
+
+def a_file(location, date):
+    return TrashedFile(location, date, '/info', '/original')
+
+
+jan = datetime.datetime(2020, 1, 1)
+feb = datetime.datetime(2020, 2, 1)
+
+
+class TestSortMethod:
+    def test_do_not_sort_keeps_the_original_order(self):
+        files = [a_file('/b', feb), a_file('/a', jan)]
+
+        result = list(sort_files(Sort.DoNot, files))
+
+        assert [f.original_location for f in result] == ['/b', '/a']
+
+    def test_do_not_sort_returns_a_reusable_sequence(self):
+        # the real caller passes a generator, then len()s and re-iterates it
+        def gen():
+            yield a_file('/b', feb)
+            yield a_file('/a', jan)
+
+        result = sort_files(Sort.DoNot, gen())
+
+        assert len(result) == 2
+        assert [f.original_location for f in result] == ['/b', '/a']
+
+    def test_sort_by_date(self):
+        files = [a_file('/b', feb), a_file('/a', jan)]
+
+        result = list(sort_files(Sort.ByDate, files))
+
+        assert [f.original_location for f in result] == ['/a', '/b']
+
+    def test_sort_by_date_tolerates_a_missing_date(self):
+        # a None (unparsable) date must not blow up the comparison; sorts first
+        files = [a_file('/dated', jan), a_file('/undated', None)]
+
+        result = list(sort_files(Sort.ByDate, files))
+
+        assert [f.original_location for f in result] == ['/undated', '/dated']
+
+    def test_sort_by_path_tolerates_a_missing_date(self):
+        files = [a_file('/b', None), a_file('/a', None)]
+
+        result = list(sort_files(Sort.ByPath, files))
+
+        assert [f.original_location for f in result] == ['/a', '/b']

trashcli/restore/handler.py

@@ -4,6 +4,7 @@
 from typing import List
 
 from trashcli.lib.my_input import Input
+from trashcli.parse_trashinfo.maybe_parse_deletion_date import unknown_date
 from trashcli.restore.file_system import ReadCwd
 from trashcli.restore.output import Output
 from trashcli.restore.output_recorder import OutputRecorder
@@ -33,8 +34,11 @@ def handle_trashed_files(self,
             self.report_no_files_found(self.cwd.getcwd_as_realpath())
         else:
             for i, trashed_file in enumerate(trashed_files):
+                deletion_date = trashed_file.deletion_date
+                if deletion_date is None:
+                    deletion_date = unknown_date  # avoid printing literal "None"
                 self.output.println("%4d %s %s" % (i,
-                                                   trashed_file.deletion_date,
+                                                   deletion_date,
                                                    trashed_file.original_location))
             self.restore_asking_the_user(trashed_files, overwrite)
 

trashcli/restore/sort_method.py

@@ -1,3 +1,4 @@
+import datetime
 from abc import abstractmethod
 from typing import Callable, Any, Iterable
 
@@ -22,7 +23,8 @@ def sort_files(self, trashed_files,  # type: Iterable[TrashedFile]
 class NoSorter(Sorter):
     def sort_files(self, trashed_files,  # type: Iterable[TrashedFile]
                    ):  # type: (...) -> Iterable[TrashedFile]
-        return trashed_files
+        # materialise, like SortFunction: the caller len()s and re-iterates it
+        return list(trashed_files)
 
 
 class SortFunction(Sorter):
@@ -38,10 +40,13 @@ def sort_files(self, trashed_files,  # type: Iterable[TrashedFile]
 def sorter_for(sort,  # type: Sort
                ):  # type (...) -> Sorter
 
-    path_ranking = lambda x: x.original_location + str(x.deletion_date)
-    date_rankking = lambda x: x.deletion_date
+    def date_ranking(x):
+        # a missing/unparsable date is None; sort it first instead of crashing
+        return x.deletion_date or datetime.datetime.min
+
+    path_ranking = lambda x: (x.original_location, date_ranking(x))
     return {
         Sort.ByPath: SortFunction(path_ranking),
-        Sort.ByDate: SortFunction(date_rankking),
-        Sort.DoNot: NoSorter,
+        Sort.ByDate: SortFunction(date_ranking),
+        Sort.DoNot: NoSorter(),
     }[sort]