Correct behavior of Sorter when no filename and/or extension is supplied

Closes #1962, #1957

sabnzbd/constants.py

@@ -122,6 +122,7 @@ VALID_NZB_FILES = (".nzb", ".gz", ".bz2")
 CHEETAH_DIRECTIVES = {"directiveStartToken": "<!--#", "directiveEndToken": "#-->", "prioritizeSearchListOverSelf": True}
 
 IGNORED_FOLDERS = ("@eaDir", ".appleDouble")
+IGNORED_MOVIE_FOLDERS = ("video_ts", "audio_ts", "bdmv")
 
 EXCLUDED_GUESSIT_PROPERTIES = [
     "part",

sabnzbd/postproc.py

@@ -74,6 +74,7 @@ from sabnzbd.constants import (
     JOB_ADMIN,
     Status,
     VERIFIED_FILE,
+    IGNORED_MOVIE_FOLDERS,
 )
 from sabnzbd.nzbparser import process_single_nzb
 import sabnzbd.emailer as emailer
@@ -499,7 +500,7 @@ def process_job(nzo: NzbObject):
                     )
                     logging.info("Traceback: ", exc_info=True)
                     # Better disable sorting because filenames are all off now
-                    file_sorter.sort_file = None
+                    file_sorter.sorter_active = None
 
             if empty:
                 job_result = -1
@@ -510,7 +511,7 @@ def process_job(nzo: NzbObject):
                 remove_samples(workdir_complete)
 
             # TV/Movie/Date Renaming code part 2 - rename and move files to parent folder
-            if all_ok and file_sorter.sort_file:
+            if all_ok and file_sorter.sorter_active:
                 if newfiles:
                     workdir_complete, ok = file_sorter.sorter.rename(newfiles, workdir_complete)
                     if not ok:
@@ -693,7 +694,7 @@ def prepare_extraction_path(nzo: NzbObject) -> Tuple[str, str, Sorter, bool, Opt
     else:
         file_sorter = Sorter(None, nzo.cat)
     complete_dir = file_sorter.detect(nzo.final_name, complete_dir)
-    if file_sorter.sort_file:
+    if file_sorter.sorter_active:
         one_folder = False
 
     complete_dir = sanitize_and_trim_path(complete_dir)
@@ -1177,7 +1178,7 @@ def rename_and_collapse_folder(oldpath, newpath, files):
     if len(items) == 1:
         folder = items[0]
         folder_path = os.path.join(oldpath, folder)
-        if os.path.isdir(folder_path) and folder not in ("VIDEO_TS", "AUDIO_TS"):
+        if os.path.isdir(folder_path) and folder.lower() not in IGNORED_MOVIE_FOLDERS:
             logging.info("Collapsing %s", os.path.join(newpath, folder))
             oldpath = folder_path
 

sabnzbd/sorting.py

@@ -38,7 +38,7 @@ from sabnzbd.filesystem import (
     clip_path,
 )
 import sabnzbd.cfg as cfg
-from sabnzbd.constants import EXCLUDED_GUESSIT_PROPERTIES
+from sabnzbd.constants import EXCLUDED_GUESSIT_PROPERTIES, IGNORED_MOVIE_FOLDERS
 from sabnzbd.nzbstuff import NzbObject, scan_password
 
 # Do not rename .vob files as they are usually DVD's
@@ -76,7 +76,7 @@ class BaseSorter:
         self.cat = cat
         self.filename_set = ""
         self.fname = ""  # Value for %fn substitution in folders
-        self.do_rename = False
+        self.rename_files = False
         self.info = {}
         self.type = None
         self.guess = guess
@@ -259,7 +259,7 @@ class BaseSorter:
         # Split the last part of the path up for the renamer
         if extension:
             path, self.filename_set = os.path.split(path)
-            self.do_rename = True
+            self.rename_files = True
 
         # The normpath function translates "" to "." which results in an incorrect path
         return os.path.normpath(path) if path else path
@@ -317,7 +317,7 @@ class Sorter:
 
     def __init__(self, nzo: Optional[NzbObject], cat: str):
         self.sorter: Optional[BaseSorter] = None
-        self.sort_file = False
+        self.sorter_active = False
         self.nzo = nzo
         self.cat = cat
 
@@ -334,9 +334,9 @@ class Sorter:
             self.sorter = MovieSorter(self.nzo, job_name, complete_dir, self.cat, guess)
 
         if self.sorter and self.sorter.matched:
-            self.sort_file = True
+            self.sorter_active = True
 
-        return self.sorter.get_final_path() if self.sort_file else complete_dir
+        return self.sorter.get_final_path() if self.sorter_active else complete_dir
 
 
 class SeriesSorter(BaseSorter):
@@ -398,8 +398,8 @@ class SeriesSorter(BaseSorter):
         """Rename for Series"""
         if min_size < 0:
             min_size = cfg.episode_rename_limit.get_int()
-        if not self.do_rename:
+        if not self.rename_files:
-            return current_path, True
+            return move_to_parent_directory(current_path)
         else:
             logging.debug("Renaming series file(s)")
             return super().rename(files, current_path, min_size)
@@ -445,8 +445,8 @@ class MovieSorter(BaseSorter):
         if min_size < 0:
             min_size = cfg.movie_rename_limit.get_int()
 
-        if not self.do_rename:
+        if not self.rename_files:
-            return current_path, True
+            return move_to_parent_directory(current_path)
 
         logging.debug("Renaming movie file(s)")
 
@@ -539,8 +539,8 @@ class DateSorter(BaseSorter):
         """Renaming Date file"""
         if min_size < 0:
             min_size = cfg.episode_rename_limit.get_int()
-        if not self.do_rename:
+        if not self.rename_files:
-            return current_path, True
+            return move_to_parent_directory(current_path)
         else:
             logging.debug("Renaming date file(s)")
             return super().rename(files, current_path, min_size)
@@ -559,9 +559,11 @@ def move_to_parent_directory(workdir: str) -> Tuple[str, bool]:
     workdir = os.path.abspath(os.path.normpath(workdir))
     dest = os.path.abspath(os.path.normpath(os.path.join(workdir, "..")))
 
+    logging.debug("Moving all files from %s to %s", workdir, dest)
+
     # Check for DVD folders and bail out if found
     for item in os.listdir(workdir):
-        if item.lower() in ("video_ts", "audio_ts", "bdmv"):
+        if item.lower() in IGNORED_MOVIE_FOLDERS:
             return workdir, True
 
     for root, dirs, files in os.walk(workdir):
@@ -874,7 +876,7 @@ def eval_sort(sort_type: str, expression: str, name: str = None, multipart: str
     if "%fn" in path:
         path = path.replace("%fn", fname + ".ext")
     else:
-        if sorter.do_rename:
+        if sorter.rename_files:
             path = fpath + ".ext"
         else:
             path += "\\" if sabnzbd.WIN32 else "/"

tests/test_sorting.py

@@ -25,6 +25,7 @@ import sys
 from random import choice
 
 from sabnzbd import sorting
+from sabnzbd.constants import IGNORED_MOVIE_FOLDERS
 from tests.testhelper import *
 
 
@@ -315,7 +316,7 @@ class TestSortingFunctions:
             pyfakefs.fake_filesystem_unittest.set_uid(0)
             # Create a fake filesystem in a random base directory, and included a typical DVD directory
             base_dir = "/" + os.urandom(4).hex() + "/" + os.urandom(2).hex()
-            dvd = choice(("video_ts", "audio_ts", "bdmv"))
+            dvd = choice(IGNORED_MOVIE_FOLDERS)
             for test_dir in ["dir/2", "TEST/DIR2"]:
                 ffs.fs.create_dir(base_dir + "/" + test_dir, perm_bits=755)
                 assert os.path.exists(base_dir + "/" + test_dir) is True
@@ -373,7 +374,7 @@ class TestSortingFunctions:
             pyfakefs.fake_filesystem_unittest.set_uid(0)
             # Create a fake filesystem in a random base directory, and included a typical DVD directory
             base_dir = "D:\\" + os.urandom(4).hex() + "\\" + os.urandom(2).hex()
-            dvd = choice(("video_ts", "audio_ts", "bdmv"))
+            dvd = choice(IGNORED_MOVIE_FOLDERS)
             for test_dir in ["dir\\2", "TEST\\DIR2"]:
                 ffs.fs.create_dir(base_dir + "\\" + test_dir, perm_bits=755)
                 assert os.path.exists(base_dir + "\\" + test_dir) is True
@@ -553,11 +554,14 @@ class TestSortingSorters:
         _func()
 
     @pytest.mark.parametrize(
-        "s_class, job_tag, sort_string, sort_result",  # sort_result without extension
+        "s_class, job_tag, sort_string, sort_filename_result",  # Supply sort_filename_result without extension
         [
             (sorting.SeriesSorter, "S01E02", "%r/%sn s%0se%0e.%ext", "Simulated Job s01e02"),
+            (sorting.SeriesSorter, "S01E02", "%r/%sn s%0se%0e", ""),
             (sorting.MovieSorter, "2021", "%y_%.title.%r.%ext", "2021_Simulated.Job.2160p"),
-            (sorting.DateSorter, "2020-02-29", "%y/%0m/%0d/%.t-%GI<release_group>", "Simulated.Job-SAB"),
+            (sorting.MovieSorter, "2021", "%y_%.title.%r", ""),
+            (sorting.DateSorter, "2020-02-29", "%y/%0m/%0d/%.t-%GI<release_group>.%ext", "Simulated.Job-SAB"),
+            (sorting.DateSorter, "2020-02-29", "%y/%0m/%0d/%.t-%GI<release_group>", ""),
         ],
     )
     @pytest.mark.parametrize("size_limit, file_size", [(512, 1024), (1024, 512)])
@@ -569,7 +573,7 @@ class TestSortingSorters:
         s_class,
         job_tag,
         sort_string,
-        sort_result,
+        sort_filename_result,
         size_limit,
         file_size,
         extension,
@@ -631,8 +635,10 @@ class TestSortingSorters:
 
             # Check the result
             try:
+                # If there's no "%ext" in the sort_string, no filenames should be changed
                 if (
                     is_ok
+                    and sort_filename_result
                     and file_size > size_limit
                     and extension not in sorting.EXCLUDED_FILE_EXTS
                     and not (sorter.type == "movie" and number_of_files > 1 and not generate_sequential_filenames)
@@ -642,10 +648,10 @@ class TestSortingSorters:
                     if number_of_files > 1 and generate_sequential_filenames and sorter.type == "movie":
                         # Movie sequential file handling
                         for n in range(1, number_of_files + 1):
-                            expected = os.path.join(sort_dest, sort_result + " CD" + str(n) + extension)
+                            expected = os.path.join(sort_dest, sort_filename_result + " CD" + str(n) + extension)
                             assert os.path.exists(expected)
                     else:
-                        expected = os.path.join(sort_dest, sort_result + extension)
+                        expected = os.path.join(sort_dest, sort_filename_result + extension)
                         assert os.path.exists(expected)
                 else:
                     # No renaming should happen
@@ -699,7 +705,7 @@ class TestSortingSorters:
             generic = sorting.Sorter(None, "test_cat")
             generic.detect(job_name, SAB_CACHE_DIR)
 
-            assert generic.sort_file is result_sort_file
+            assert generic.sorter_active is result_sort_file
             if result_sort_file:
                 assert generic.sorter
                 assert generic.sorter.__class__ is result_class