Merge pull request #7795 from drew2a/fix/7794

Fix OSError: [WinError 123] for the `_check_watch_folder`

src/tribler/core/components/watch_folder/tests/test_watch_folder.py

@@ -1,6 +1,6 @@
 import asyncio
 import shutil
-from unittest.mock import MagicMock, Mock, patch, AsyncMock
+from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
 import pytest
 
@@ -36,7 +36,7 @@ async def watch_folder(tmp_path):
 
 async def test_watch_folder_no_files(watch_folder):
     # Test that in the case of an empty folder, downloads are not started
-    await watch_folder._check_watch_folder()
+    assert await watch_folder._check_watch_folder()
 
     assert not watch_folder.download_manager.start_download.called
 
@@ -46,7 +46,7 @@ async def test_watch_folder_no_torrent_file(watch_folder: WatchFolder):
     directory = watch_folder.settings.get_path_as_absolute('directory', watch_folder.state_dir)
     shutil.copyfile(TORRENT_UBUNTU_FILE, directory / "test.txt")
 
-    await watch_folder._check_watch_folder()
+    assert await watch_folder._check_watch_folder()
 
     assert not watch_folder.download_manager.start_download.called
 
@@ -58,7 +58,7 @@ async def test_watch_folder_utf8_dir(watch_folder, tmp_path):
     unicode_folder.mkdir()
     shutil.copyfile(TORRENT_UBUNTU_FILE, unicode_folder / "\xe2\x82\xac.torrent")
 
-    await watch_folder._check_watch_folder()
+    assert await watch_folder._check_watch_folder()
 
     assert watch_folder.download_manager.start_download.called
 
@@ -80,7 +80,7 @@ async def test_watch_folder_torrent_file_no_metainfo(watch_folder: WatchFolder):
     watch_folder.download_manager.download_exists = Mock(return_value=False)
     shutil.copyfile(TORRENT_UBUNTU_FILE, watch_folder.state_dir / "test.torrent")
 
-    await watch_folder._check_watch_folder()
+    assert await watch_folder._check_watch_folder()
 
     assert not watch_folder.download_manager.start_download.called
 
@@ -90,7 +90,7 @@ async def test_watch_folder_torrent_file_start_download(watch_folder: WatchFolde
     watch_folder.download_manager.download_exists = Mock(return_value=False)
     shutil.copyfile(TORRENT_VIDEO_FILE, watch_folder.state_dir / "test.torrent")
 
-    await watch_folder._check_watch_folder()
+    assert await watch_folder._check_watch_folder()
 
     assert watch_folder.download_manager.start_download.call_count == 1
 
@@ -120,3 +120,34 @@ async def test_watch_folder_no_crash_exception(watch_folder: WatchFolder):
     # Test that errors raised in `_check_watch_folder` reraise as `NoCrashException`
     with pytest.raises(NoCrashException):
         await watch_folder._check_watch_folder_handle_exceptions()
+
+
+async def test_watch_folder_invalid_dir(watch_folder: WatchFolder, tmp_path):
+    """ Test that the watch folder is disabled when the directory is invalid"""
+    watch_folder.settings.put_path_as_relative(
+        property_name='directory',
+        value=Path('path that does not exist'),
+        state_dir=Path(tmp_path)
+    )
+
+    assert not await watch_folder._check_watch_folder()
+
+
+async def test_watch_folder_not_dir(watch_folder: WatchFolder, tmp_path):
+    """ Test that the watch folder is disabled when the "directory" setting is not a directory"""
+    any_file = tmp_path / 'any.file'
+    any_file.touch()
+
+    watch_folder.settings.put_path_as_relative(
+        property_name='directory',
+        value=any_file,
+        state_dir=Path(tmp_path)
+    )
+
+    assert not await watch_folder._check_watch_folder()
+
+
+async def test_watch_folder_disabled(watch_folder: WatchFolder):
+    """ Test that the watch folder is disabled when the "enabled" setting is False"""
+    watch_folder.settings.enabled = False
+    assert not await watch_folder._check_watch_folder()

src/tribler/core/components/watch_folder/watch_folder.py

@@ -45,24 +45,31 @@ async def _check_watch_folder_handle_exceptions(self):
             self._logger.exception(f'Failed download attempt: {e}')
             raise NoCrashException from e
 
-    async def _check_watch_folder(self):
+    async def _check_watch_folder(self) -> bool:
+        """ Check the watch folder for new torrents and start downloading them."""
+
         self._logger.debug('Checking watch folder...')
         if not self.settings.enabled or not self.state_dir:
             self._logger.debug(f'Cancelled. Enabled: {self.settings.enabled}. State dir: {self.state_dir}.')
-            return
+            return False
 
         directory = self.settings.get_path_as_absolute('directory', self.state_dir)
-        self._logger.debug(f'Watch dir: {directory}')
+        self._logger.info(f'Checking watch folder: {directory}')
+        if not directory.is_valid():
+            self._logger.warning(f'Cancelled. Directory is not valid: {directory}.')
+            return False
+
         if not directory.is_dir():
-            self._logger.debug(f'Cancelled. Is not directory: {directory}.')
-            return
+            self._logger.warning(f'Cancelled. Is not directory: {directory}.')
+            return False
 
         for root, _, files in os.walk(str(directory)):
             for name in files:
                 path = Path(root) / name
                 await self._process_torrent_file(path)
 
         self._logger.debug('Checking watch folder completed.')
+        return True
 
     async def _process_torrent_file(self, path: Path):
         if not path.name.endswith(".torrent"):

src/tribler/core/config/tribler_config_section.py

@@ -1,10 +1,11 @@
 from __future__ import annotations
 
-from pathlib import Path
 from typing import Optional
 
 from pydantic import BaseSettings, Extra, root_validator
 
+from tribler.core.utilities.path_util import Path
+
 
 class TriblerConfigSection(BaseSettings):
     """Base Class that defines Tribler Config Section
@@ -38,7 +39,7 @@ def get_path_as_absolute(self, property_name: str, state_dir: Path = None) -> Op
         value = self.__getattribute__(property_name)
         if value is None:
             return None
-        return state_dir / value
+        return Path(state_dir / value)
 
     @root_validator(pre=True)
     def convert_from_none_string_to_none_type(cls, values):  # pylint: disable=no-self-argument

src/tribler/core/utilities/path_util.py

@@ -72,6 +72,16 @@ def startswith(self, text: str) -> bool:
     def endswith(self, text: str) -> bool:
         return self.match(f"*{text}")
 
+    def is_valid(self):
+        """ Check if the path is valid.
+
+        Returns: True if the path is valid, False otherwise.
+        """
+        try:
+            return self.is_file() or self.is_dir()
+        except OSError:
+            return False
+
 
 class PosixPath(Path, pathlib.PurePosixPath):
     __slots__ = ()

src/tribler/core/utilities/rest_utils.py

@@ -1,9 +1,10 @@
 import os
-from pathlib import Path
 from typing import Any, Union
 
 from yarl import URL
 
+from tribler.core.utilities.path_util import Path
+
 MAGNET_SCHEME = 'magnet'
 FILE_SCHEME = 'file'
 HTTP_SCHEME = 'http'
@@ -55,7 +56,4 @@ def scheme_from_url(url: str) -> str:
 
 def url_is_valid_file(file_url: str) -> bool:
     file_path = url_to_path(file_url)
-    try:
-        return Path(file_path).is_file()
-    except OSError:
-        return False
+    return Path(file_path).is_valid()

src/tribler/core/utilities/tests/test_path_utils.py

@@ -1,5 +1,5 @@
 import sys
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
 import pytest
 
@@ -118,3 +118,27 @@ def test_fix_win_long_file_linux():
     """ Test that fix_win_long_file works correct on Linux"""
     path = Path('/home/user/.Tribler/7.7')
     assert Path.fix_win_long_file(path) == str(path)
+
+
+def test_is_valid(tmp_path):
+    """ Test that is_valid returns True for valid path"""
+    assert Path(tmp_path).is_valid()
+
+
+def test_is_invalid(tmp_path):
+    """ Test that is_valid returns False for invalid path"""
+    invalid_path = Path(str(tmp_path) * 2)
+    assert not Path(invalid_path).is_valid()
+
+
+@patch.object(Path, 'is_file', Mock(side_effect=OSError))
+def test_is_invalid_by_os_exception(tmp_path):
+    """ Test that is_valid returns False if OSError exception was raised"""
+    assert not Path(tmp_path).is_valid()
+
+
+@patch.object(Path, 'is_file', Mock(side_effect=ValueError))
+def test_is_valid_any_exception(tmp_path):
+    """ Test that is_valid reraise exception if it is not OSError"""
+    with pytest.raises(ValueError):
+        Path(tmp_path).is_valid()