Handle Direct Unpack sets before proceeding to unpack the rest

sabnzbd/directunpacker.py

@@ -25,12 +25,12 @@ import subprocess
 import time
 import threading
 import logging
-from typing import Optional
+from typing import Optional, Dict, Tuple, List
 
 import sabnzbd
 import sabnzbd.cfg as cfg
-from sabnzbd.misc import int_conv, format_time_string, build_and_run_command
+from sabnzbd.misc import format_time_string, build_and_run_command
-from sabnzbd.filesystem import long_path, remove_all, real_path, remove_file
+from sabnzbd.filesystem import long_path, remove_all, real_path, remove_file, analyze_rar_filename
 from sabnzbd.nzbstuff import NzbObject, NzbFile
 from sabnzbd.encoding import platform_btou
 from sabnzbd.decorators import synchronized
@@ -45,8 +45,6 @@ START_STOP_LOCK = threading.RLock()
 
 ACTIVE_UNPACKERS = []
 
-RAR_NR = re.compile(r"(.*?)(\.part(\d*).rar|\.r(\d*))$", re.IGNORECASE)
-
 
 class DirectUnpacker(threading.Thread):
     def __init__(self, nzo: NzbObject):
@@ -64,7 +62,7 @@ class DirectUnpacker(threading.Thread):
         self.total_volumes = {}
         self.unpack_time = 0.0
 
-        self.success_sets = {}
+        self.success_sets: Dict[str, Tuple[List[str], List[str]]] = {}
         self.next_sets = []
 
         self.duplicate_lines = 0
@@ -503,23 +501,6 @@ class DirectUnpacker(threading.Thread):
         return self.cur_volume
 
 
-def analyze_rar_filename(filename):
-    """Extract volume number and setname from rar-filenames
-    Both ".part01.rar" and ".r01"
-    """
-    m = RAR_NR.search(filename)
-    if m:
-        if m.group(4):
-            # Special since starts with ".rar", ".r00"
-            return m.group(1), int_conv(m.group(4)) + 2
-        return m.group(1), int_conv(m.group(3))
-    else:
-        # Detect if first of "rxx" set
-        if filename.endswith(".rar"):
-            return os.path.splitext(filename)[0], 1
-    return None, None
-
-
 def abort_all():
     """ Abort all running DirectUnpackers """
     logging.info("Aborting all DirectUnpackers")

sabnzbd/filesystem.py

@@ -64,6 +64,27 @@ def setname_from_path(path: str) -> str:
     return os.path.splitext(os.path.basename(path))[0]
 
 
+RAR_NR = re.compile(r"(.*?)(\.part(\d*).rar|\.r(\d*))$", re.IGNORECASE)
+
+
+def analyze_rar_filename(filename_or_path: str) -> Tuple[Optional[str], Optional[int]]:
+    """Extract volume number and setname from rar-filenames or paths
+    Both ".part01.rar" and ".r01" work
+    """
+    filename = os.path.basename(filename_or_path)
+    m = RAR_NR.search(filename)
+    if m:
+        if m.group(4):
+            # Special since starts with ".rar", ".r00"
+            return m.group(1), sabnzbd.misc.int_conv(m.group(4)) + 2
+        return m.group(1), sabnzbd.misc.int_conv(m.group(3))
+    else:
+        # Detect if first of "rxx" set
+        if filename.endswith(".rar"):
+            return os.path.splitext(filename)[0], 1
+    return None, None
+
+
 def is_writable(path: str) -> bool:
     """ Return True is file is writable (also when non-existent) """
     if os.path.isfile(path):

sabnzbd/newsunpack.py

@@ -28,6 +28,7 @@ import time
 import zlib
 import shutil
 import functools
+from typing import List
 
 import sabnzbd
 from sabnzbd.encoding import platform_btou, correct_unknown_encoding, ubtou
@@ -55,6 +56,7 @@ from sabnzbd.filesystem import (
     setname_from_path,
     get_ext,
     get_filename,
+    analyze_rar_filename,
 )
 from sabnzbd.nzbstuff import NzbObject, NzbFile
 from sabnzbd.sorting import SeriesSorter
@@ -63,7 +65,6 @@ from sabnzbd.constants import Status
 
 # Regex globals
 RAR_RE = re.compile(r"\.(?P<ext>part\d*\.rar|rar|r\d\d|s\d\d|t\d\d|u\d\d|v\d\d|\d\d\d?\d)$", re.I)
-RAR_RE_V3 = re.compile(r"\.(?P<ext>part\d*)$", re.I)
 
 LOADING_RE = re.compile(r'^Loading "(.+)"')
 TARGET_RE = re.compile(r'^(?:File|Target): "(.+)" -')
@@ -467,34 +468,13 @@ def file_join(nzo: NzbObject, workdir, workdir_complete, delete, joinables):
 ##############################################################################
 # (Un)Rar Functions
 ##############################################################################
-def rar_unpack(nzo: NzbObject, workdir, workdir_complete, delete, one_folder, rars):
+def rar_unpack(nzo: NzbObject, workdir: str, workdir_complete: str, delete: bool, one_folder: bool, rars: List[str]):
     """Unpack multiple sets 'rars' of RAR files from 'workdir' to 'workdir_complete.
     When 'delete' is set, originals will be deleted.
     When 'one_folder' is set, all files will be in a single folder
     """
+    fail = False
     newfiles = extracted_files = []
-    rar_sets = {}
-    for rar in rars:
-        rar_set = setname_from_path(rar)
-        if RAR_RE_V3.search(rar_set):
-            # Remove the ".partXX" part
-            rar_set = os.path.splitext(rar_set)[0]
-        if rar_set not in rar_sets:
-            rar_sets[rar_set] = []
-        rar_sets[rar_set].append(rar)
-
-    logging.debug("Rar_sets: %s", rar_sets)
-
-    for rar_set in rar_sets:
-        # Run the RAR extractor
-        rar_sets[rar_set].sort(key=functools.cmp_to_key(rar_sort))
-
-        rarpath = rar_sets[rar_set][0]
-
-        if workdir_complete and rarpath.startswith(workdir):
-            extraction_path = workdir_complete
-        else:
-            extraction_path = os.path.split(rarpath)[0]
 
     # Is the direct-unpacker still running? We wait for it
     if nzo.direct_unpacker:
@@ -518,21 +498,54 @@ def rar_unpack(nzo: NzbObject, workdir, workdir_complete, delete, one_folder, ra
                 wait_count = 0
             last_stats = nzo.direct_unpacker.get_formatted_stats()
 
-        # Did we already direct-unpack it? Not when recursive-unpacking
+        # Process everything already extracted by Direct Unpack
-        if nzo.direct_unpacker and rar_set in nzo.direct_unpacker.success_sets:
+        for rar_set in nzo.direct_unpacker.success_sets:
             logging.info("Set %s completed by DirectUnpack", rar_set)
-            fail = False
+            unpacked_rars, newfiles = nzo.direct_unpacker.success_sets[rar_set]
-            success = True
+            logging.debug("Rars: %s", unpacked_rars)
-            rars, newfiles = nzo.direct_unpacker.success_sets.pop(rar_set)
+            logging.debug("Newfiles: %s", newfiles)
+            extracted_files.extend(newfiles)
+
+            # Remove all source files from the list and from the disk (if requested)
+            # so they don't get parsed by the regular unpack
+            for rar in unpacked_rars:
+                if rar in rars:
+                    rars.remove(rar)
+                if delete:
+                    remove_file(rar)
+
+        # Clear all sets
+        nzo.direct_unpacker.success_sets = []
+
+    # See which sets are left
+    rar_sets = {}
+    for rar in rars:
+        # Skip any files that were already removed
+        if not os.path.exists(rar):
+            continue
+        rar_set, _ = analyze_rar_filename(rar)
+        if rar_set not in rar_sets:
+            rar_sets[rar_set] = []
+        rar_sets[rar_set].append(rar)
+
+    logging.debug("Remaining rar sets: %s", rar_sets)
+
+    for rar_set in rar_sets:
+        # Run the RAR extractor
+        rar_sets[rar_set].sort(key=functools.cmp_to_key(rar_sort))
+        rarpath = rar_sets[rar_set][0]
+
+        if workdir_complete and rarpath.startswith(workdir):
+            extraction_path = workdir_complete
         else:
+            extraction_path = os.path.split(rarpath)[0]
+
         logging.info("Extracting rarfile %s (belonging to %s) to %s", rarpath, rar_set, extraction_path)
         try:
             fail, newfiles, rars = rar_extract(
                 rarpath, len(rar_sets[rar_set]), one_folder, nzo, rar_set, extraction_path
             )
-                success = not fail
         except:
-                success = False
             fail = True
             msg = sys.exc_info()[1]
             nzo.fail_msg = T("Unpacking failed, %s") % msg
@@ -542,17 +555,16 @@ def rar_unpack(nzo: NzbObject, workdir, workdir_complete, delete, one_folder, ra
             logging.error(T('Error "%s" while running rar_unpack on %s'), msg, setname)
             logging.debug("Traceback: ", exc_info=True)
 
-        if success:
+        if not fail:
-            logging.debug("rar_unpack(): Rars: %s", rars)
+            logging.debug("Rars: %s", rars)
-            logging.debug("rar_unpack(): Newfiles: %s", newfiles)
+            logging.debug("Newfiles: %s", newfiles)
             extracted_files.extend(newfiles)
 
         # Do not fail if this was a recursive unpack
         if fail and rarpath.startswith(workdir_complete):
             # Do not delete the files, leave it to user!
             logging.info("Ignoring failure to do recursive unpack of %s", rarpath)
-            fail = 0
+            fail = False
-            success = True
             newfiles = []
 
         # Do not fail if this was maybe just some duplicate fileset
@@ -560,12 +572,11 @@ def rar_unpack(nzo: NzbObject, workdir, workdir_complete, delete, one_folder, ra
         if fail and rar_set.endswith((".1", ".2")):
             # Just in case, we leave the raw files
             logging.info("Ignoring failure of unpack for possible duplicate file %s", rarpath)
-            fail = 0
+            fail = False
-            success = True
             newfiles = []
 
         # Delete the old files if we have to
-        if success and delete and newfiles:
+        if not fail and delete and newfiles:
             for rar in rars:
                 try:
                     remove_file(rar)

tests/test_functional_downloads.py

@@ -127,7 +127,6 @@ class TestDownloadFlow(SABnzbdBaseTest):
     def test_download_passworded(self):
         self.download_nzb("test_passworded{{secret}}", "testfile.bin")
 
-    @pytest.mark.xfail(not sys.platform.startswith("win"), reason="Probably #1633")
     def test_download_unicode_made_on_windows(self):
         self.download_nzb("test_win_unicode", "frènch_german_demö.bin")