Skip to content

Commit

Permalink
Merge pull request #3706 from kingamajick/update_sftp_list_and_downlo…
Browse files Browse the repository at this point in the history 
…ad_symlink_handling

Update sftp list and download symlink handling
  • Loading branch information
@ianstalk
ianstalk committed Apr 4, 2023
2 parents edb11bd + 2e41181 commit 5bd6247
Show file tree
Hide file tree
Showing 4 changed files with 235 additions and 110 deletions.
40 changes: 32 additions & 8 deletions flexget/components/ftp/sftp_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from dataclasses import dataclass
from functools import partial
from pathlib import Path, PurePath, PurePosixPath
from stat import S_ISLNK
from typing import Callable, List, Optional
from urllib.parse import quote, urljoin

Expand Down Expand Up @@ -143,8 +144,15 @@ def list_directories(

for directory in directories:
try:
# Always normalize the root path so it's not necessary to normalised
# nodes as there are discovered, which means that symlinks will appear
# in the entry paths raw rather than been resolved to their target.
self._sftp.walktree(
directory, file_handler, dir_handler, unknown_handler, recursive
self._sftp.normalize(directory),
file_handler,
dir_handler,
unknown_handler,
recursive,
)
except OSError as e:
logger.warning('Failed to open {} ({})', directory, str(e))
Expand All @@ -158,7 +166,8 @@ def download(self, source: str, to: str, recursive: bool, delete_origin: bool) -
:param source: path of the resource to download
:param to: path of the directory to download to
:param recursive: indicates whether to download the contents of "source" recursively
:param delete_origin: indicates whether to delete the source resource upon download
:param delete_origin: indicates whether to delete the source resource upon download, is the source
is a symlink, only the symlink will be removed rather than it's target.
"""

dir_handler: NodeHandler = self._handler_builder.get_null_handler()
Expand All @@ -169,21 +178,25 @@ def download(self, source: str, to: str, recursive: bool, delete_origin: bool) -
if not self.path_exists(source):
raise SftpError(f'Remote path does not exist: {source}')

is_symlink: bool = self.is_link(source)
if self.is_file(source):
source_file: str = parsed_path.name
source_dir: str = parsed_path.parent.as_posix()
try:
self._sftp.cwd(source_dir)
self._download_file(to, delete_origin, source_file)
self._download_file(to, delete_origin and not is_symlink, source_file)
except Exception as e:
raise SftpError(f'Failed to download file {source} ({str(e)})')

if delete_origin:
self.remove_dir(source_dir)
if delete_origin and is_symlink:
self.remove_file(source)

elif self.is_dir(source):
base_path: str = parsed_path.joinpath('..').as_posix()
dir_name: str = parsed_path.name
handle_file: NodeHandler = partial(self._download_file, to, delete_origin)
handle_file: NodeHandler = partial(
self._download_file, to, delete_origin and not is_symlink
)

try:
self._sftp.cwd(base_path)
Expand All @@ -192,7 +205,10 @@ def download(self, source: str, to: str, recursive: bool, delete_origin: bool) -
raise SftpError(f'Failed to download directory {source} ({str(e)})')

if delete_origin:
self.remove_dir(source)
if self.is_link(source):
self.remove_file(source)
else:
self.remove_dir(source)
else:
logger.warning('Skipping unknown file: {}', source)

Expand Down Expand Up @@ -247,6 +263,14 @@ def is_dir(self, path: str) -> bool:
"""
return self._sftp.isdir(path)

def is_link(self, path: str) -> bool:
"""
Check if the node at a given path is a directory
:param path: path to check
:return: boolean indicating if the path is a directory
"""
return S_ISLNK(self._sftp.sftp_client.lstat(path).st_mode)

def path_exists(self, path: str) -> bool:
"""
Check of a path exists
Expand Down Expand Up @@ -609,7 +633,7 @@ def _get_entry(
private_key_pass: Optional[str],
host_key: Optional[HostKey],
) -> Entry:
url = urljoin(prefix, quote(sftp.normalize(path)))
url = urljoin(prefix, quote(path))
title = PurePosixPath(path).name

entry = Entry(title, url)
Expand Down
82 changes: 43 additions & 39 deletions flexget/tests/test_sftp_download.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,63 +14,57 @@ class TestSftpDownload:
_config = """
templates:
anchors:
_sftp_list: &base_sftp_download
to: {{ download_path }}
socket_timeout_sec: 2
connection_tries: 1
_sftp_download: &base_sftp_download
to: {{ download_path }}
socket_timeout_sec: 2
connection_tries: 1
_mock_file:
- &mock_file
{'title': 'file.mkv', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/file.mkv', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
_mock_dir:
- &mock_dir
{'title': 'dir', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/dir', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
tasks:
sftp_download_file:
mock:
- {'title': 'file.mkv', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/file.mkv', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
mock:
- *mock_file
accept_all: True
sftp_download:
<<: *base_sftp_download
delete_origin: False
sftp_download_dir:
mock:
- {'title': 'dir', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/dir', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
mock:
- *mock_dir
accept_all: True
sftp_download:
to: {{ download_path }}
delete_origin: False
sftp_download_dir_recusive:
mock:
- {'title': 'dir', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/dir', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
sftp_download_dir_recusive_true:
mock:
- *mock_dir
accept_all: True
sftp_download:
to: {{ download_path }}
delete_origin: False
recursive: True
sftp_download_file_delete_origin:
mock:
- {'title': 'file.mkv', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/file.mkv', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
sftp_download_file_delete_origin_true:
mock:
- *mock_file
accept_all: True
sftp_download:
<<: *base_sftp_download
delete_origin: True
sftp_download_dir_delete_origin:
mock:
- {'title': 'dir', 'url': 'sftp://test_user:test_pass@127.0.0.1:40022/home/test_user/dir', 'host_key': {
'key_type': 'ssh-rsa',
'public_key': 'AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hn9BizDY6wI1oNYUBoVHAVioXzOJkZDPB+QsUHDBOqVIcdL/glfMtgIO1E5khoBYql8DSSI+EyrxaC+mfeJ7Ax5qZnimOFvZsJvwvO5h7LI4W1KkoJrYUfMLFfHkDy5EbPIuXeAQGdF/JzOXoIqMcCmKQDS56WRDnga91CGQeXAuzINiviZ63R55b8ynN2JFqKW5V6WZiYZBSmTia68s2ZefkFMiv7E6gmD4WYj6hitz8FGPUoyFAGIR+NVqZ5i9l/8CDuNcZ8E8G7AmNFQhChAeQdEOPO0f2vdH6aRb8Cn0EAy6zpBllxQO8EuLjiEfH01n4/VlGeQEiXlyCLqj'
}}
sftp_download_dir_delete_origin_true_recursive_true:
mock:
- *mock_dir
accept_all: True
sftp_download:
<<: *base_sftp_download
Expand Down Expand Up @@ -117,17 +111,27 @@ def test_sftp_download_dir_recusive(
sftp_fs.create_file('dir/file.mkv', 100)
sftp_fs.create_file('dir/nested/file.mkv', 100)

execute_task('sftp_download_dir_recusive')
execute_task('sftp_download_dir_recusive_true')
assert filecmp.dircmp(remote_dir, download_path / 'dir')

def test_sftp_download_file_and_delete(
self, execute_task, download_path: Path, sftp_fs: TestSFTPFileSystem
):
remote_file: Path = sftp_fs.create_file('file.mkv', 100)

execute_task('sftp_download_file_delete_origin')
execute_task('sftp_download_file_delete_origin_true')
assert not remote_file.exists()

def test_sftp_download_file_and_delete_when_symlink_deletes_symlink_only(
self, execute_task, download_path: Path, sftp_fs: TestSFTPFileSystem
):
remote_file: Path = sftp_fs.create_file('/target.mkv', 100)
remote_link: Path = sftp_fs.create_symlink('file.mkv', remote_file)

execute_task('sftp_download_file_delete_origin_true')
assert remote_file.exists(), '/target.mkv should not have been deleted.'
assert not remote_link.exists(), 'file.mkv should have been deleted.'

@pytest.mark.skip(
reason='No attempt is made by the sftp_download plugin to remove the directories)'
)
Expand All @@ -138,5 +142,5 @@ def test_sftp_download_dir_and_delete(
sftp_fs.create_file('dir/file.mkv', 100)
sftp_fs.create_file('dir/nested/file.mkv', 100)

execute_task('sftp_download_dir_delete_origin')
execute_task('sftp_download_dir_delete_origin_true_recursive_true')
assert not remote_dir.exists()

0 comments on commit 5bd6247

Please sign in to comment.