From 968237bec09719ae2e84f0c8bf451e898a8f26db Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:02:15 +0000 Subject: [PATCH] Ruff checks --- .../utilities.py | 47 +++++++++--- tests/unit_tests/test_utilities.py | 71 ++++++++++++------- 2 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d16d494b..10215af8 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -111,8 +111,7 @@ _WINDOWS_RESERVED_NAMES = { def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: - """Check if a file path has any unsafe elements in it, such as - unsafe characters or reserved names for its OS. + """Check if a file path has any unsafe elements in it. The path is not coerced into a safe form because if we ask for a file to be written to a location we shouldn't be changing @@ -124,6 +123,7 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: any unsafe features were found. """ # Split by separators first to sanitise components independently + components = re.split(r"([/\\])", unsafe_path_string) unsafe_character_pattern = ( @@ -146,10 +146,16 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: # 1. Check for relative paths in the wrong place if component in (".", "..") and not unsafe_relative_path: - is_first = (i == 0) - is_second = (i == 2 and components[i-1] in ("/", "\\") and components[i-2] == "..") + is_first = i == 0 + is_second = ( + i == 2 + and components[i - 1] in ("/", "\\") + and components[i - 2] == ".." + ) if not (is_first or is_second): - LOGGER.warning(f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation.") + LOGGER.warning( + f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation." + ) unsafe_relative_path = True continue @@ -157,26 +163,45 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: if "." in component and i != len(components) - 1 and not trailing_dots: # Check for trailing dots in the file path before the file name # e.g.: foo/bar./file is bad, but foo/bar.file.py is fine - LOGGER.warning(f'File path {unsafe_path_string} may be unsafe due to trailing dots.') + LOGGER.warning( + f"File path {unsafe_path_string} may be unsafe due to trailing dots." + ) trailing_dots = True # Check for trailing spaces - Windows specific - if sys.platform.startswith("win") and component.endswith(" ") and not trailing_whitespace: + if ( + sys.platform.startswith("win") + and component.endswith(" ") + and not trailing_whitespace + ): LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.") trailing_whitespace = True # 3. Check for unsafe characters (Regex) if unsafe_character_pattern.search(component) and not unsafe_characters: - LOGGER.warning(f"{unsafe_path_string} contains characters that may be unsafe on this platform.") + LOGGER.warning( + f"{unsafe_path_string} contains characters that may be unsafe on this platform." + ) unsafe_characters = True # 4. Check for Reserved Names (e.g., CON, PRN, LPT1) # Assuming _sanitise_reserved returns a different string if it's a reserved name - if _sanitise_reserved(component, is_filename=False) != component and not reserved_word: - LOGGER.warning(f"{unsafe_path_string} contains a reserved system name and may cause issues.") + if ( + _sanitise_reserved(component, is_filename=False) != component + and not reserved_word + ): + LOGGER.warning( + f"{unsafe_path_string} contains a reserved system name and may cause issues." + ) reserved_word = True - warning_raised = unsafe_relative_path or trailing_dots or trailing_whitespace or unsafe_characters or reserved_word + warning_raised = ( + unsafe_relative_path + or trailing_dots + or trailing_whitespace + or unsafe_characters + or reserved_word + ) return unsafe_path_string, warning_raised diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index c6d32431..6df07dec 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -1,26 +1,19 @@ """Unit tests for utility functions.""" -import sys import logging +import sys + import pytest from openflexure_microscope_server.utilities import ( _WINDOWS_RESERVED_NAMES, make_name_safe, - make_path_safe + make_path_safe, ) -PATHS_WITH_DOTS = { - "a./b/c.", - "path./to/file.", - "./path./to/file." -} +PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."} -RESERVED_NAME_PATHS = { - "path/CON/file", - "path/NUL.txt/file", - "C:/AUX/test" -} +RESERVED_NAME_PATHS = {"path/CON/file", "path/NUL.txt/file", "C:/AUX/test"} if sys.platform.startswith("win"): UNSAFE_CHARACTER_PATHS = { @@ -34,7 +27,7 @@ else: "/var/log/app:backup.log", "/etc/configs/network-settings\\v1", "/tmp/file_with_@_symbol.txt", - "/home/user/script(1).py" + "/home/user/script(1).py", } @@ -89,10 +82,16 @@ def test_make_path_safe_basic(caplog): with caplog.at_level(logging.WARNING): if sys.platform.startswith("win"): assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file" - assert make_path_safe("C:\\a very long path\\to/file")[0] == "C:\\a very long path\\to/file" + assert ( + make_path_safe("C:\\a very long path\\to/file")[0] + == "C:\\a very long path\\to/file" + ) else: assert make_path_safe("path/to/file")[0] == "path/to/file" - assert make_path_safe("a very long path/to/file")[0] == "a very long path/to/file" + assert ( + make_path_safe("a very long path/to/file")[0] + == "a very long path/to/file" + ) # No errors should be raised. These paths are safe. assert len(caplog.records) == 0 @@ -103,7 +102,10 @@ def test_make_path_safe_unsafe_characters_in_components(caplog, path): """Test unsafe characters within path components.""" with caplog.at_level(logging.WARNING): assert make_path_safe(path)[0] == path - assert f"{path} contains characters that may be unsafe on this platform." + assert ( + f"{path} contains characters that may be unsafe on this platform." + in caplog.messages + ) @pytest.mark.parametrize("path", RESERVED_NAME_PATHS) @@ -111,29 +113,40 @@ def test_make_path_safe_reserved_in_components(caplog, path): """Test reserved names within path components.""" with caplog.at_level(logging.WARNING): assert make_path_safe(path)[0] == path - assert f"{path} contains a reserved system name and may cause issues." + assert ( + f"{path} contains a reserved system name and may cause issues." + in caplog.messages + ) def test_make_path_safe_trailing_space_in_components(caplog): """Test trailing spaces in path components.""" with caplog.at_level(logging.WARNING): - # Test Windows - cannot have trailingspaces + # Test Windows - cannot have trailingspaces if sys.platform.startswith("win"): assert make_path_safe("path /to /file ")[0] == "path /to /file " assert make_path_safe("path/to /file")[0] == "path/to /file" # Check directories can have spaces in them but not at the end - # TODO - dont want underscores, want to keep interim spaces - assert make_path_safe("path/a long path name /file")[0] == "path/a long path name /file" + assert ( + make_path_safe("path/a long path name /file")[0] + == "path/a long path name /file" + ) assert len(caplog.records) == 3 - assert f"path /to /file contains unsafe trailing spaces." in caplog.messages - assert f"path/to /file contains unsafe trailing spaces." in caplog.messages - assert f"path/a long path name /file contains unsafe trailing spaces." in caplog.messages + assert "path /to /file contains unsafe trailing spaces." in caplog.messages + assert "path/to /file contains unsafe trailing spaces." in caplog.messages + assert ( + "path/a long path name /file contains unsafe trailing spaces." + in caplog.messages + ) else: # Trailing spaces allows in POSIX systems assert make_path_safe("path /to /file ")[0] == "path /to /file " assert make_path_safe("path/to /file")[0] == "path/to /file" # Check directories can have spaces in them but not at the end - assert make_path_safe("path/a long path name /file")[0] == "path/a long path name file" + assert ( + make_path_safe("path/a long path name /file")[0] + == "path/a long path name file" + ) # No warnings should be raised assert len(caplog.records) == 0 @@ -152,7 +165,10 @@ def test_make_path_safe_relative(caplog): assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path" assert len(caplog.records) == 1 - assert(f"File path a_bad/../relative_path may be unsafe due to unexpected relative navigation.") + assert ( + "File path a_bad/../relative_path may be unsafe due to unexpected relative navigation." + in caplog.messages + ) @pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) @@ -160,4 +176,7 @@ def test_make_path_safe_dots_warning(caplog, filepath): """Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged.""" with caplog.at_level(logging.WARNING): assert make_path_safe(filepath)[0] == filepath - assert f"File path {filepath} may be unsafe due to trailing dots." in caplog.messages + assert ( + f"File path {filepath} may be unsafe due to trailing dots." + in caplog.messages + )