diff --git a/lean/click.py b/lean/click.py index 554477ff..2d0e67f6 100644 --- a/lean/click.py +++ b/lean/click.py @@ -188,7 +188,7 @@ def __init__(self, exists: bool = False, file_okay: bool = True, dir_okay: bool def convert(self, value: str, param: Parameter, ctx: Context) -> Path: path = Path(value).expanduser().resolve() - if not container.path_manager.is_path_valid(path): + if not container.path_manager.is_cli_path_valid(path): self.fail(f"{self._path_type} '{value}' is not a valid path.", param, ctx) if self._exists and not path.exists(): diff --git a/lean/commands/cloud/backtest.py b/lean/commands/cloud/backtest.py index 11d3066e..415d435b 100644 --- a/lean/commands/cloud/backtest.py +++ b/lean/commands/cloud/backtest.py @@ -44,8 +44,7 @@ def backtest(project: str, name: Optional[str], push: bool, open_browser: bool) try: cloud_project = cloud_project_manager.get_cloud_project(project, push) except RuntimeError as e: - if cloud_project_manager._project_config_manager.try_get_project_config(Path.cwd() / project, - cloud_project_manager._path_manager): + if cloud_project_manager._project_config_manager.try_get_project_config(Path.cwd() / project): error_message = f'No project with the given name or id "{project}" found in your cloud projects.' error_message += f" Please use `lean cloud backtest --push {project}` to backtest in cloud." else: diff --git a/lean/commands/create_project.py b/lean/commands/create_project.py index a91ca1ac..ab6839fa 100644 --- a/lean/commands/create_project.py +++ b/lean/commands/create_project.py @@ -292,8 +292,13 @@ def create_project(name: str, language: str) -> None: "https://www.lean.io/docs/v2/lean-cli/projects/project-management") full_path = Path.cwd() / name + try: + cli_root_dir = container.lean_config_manager.get_cli_root_directory() + relative_path = full_path.relative_to(cli_root_dir).as_posix() + except MoreInfoError: + relative_path = name - if not container.path_manager.is_path_valid(full_path) or not container.path_manager.is_name_valid(name): + if not container.path_manager.is_cli_path_valid(full_path) or not container.path_manager.is_name_valid(relative_path): raise MoreInfoError(f"Invalid project name. Can only contain letters, numbers & spaces. Can not start with empty char ' ' or be a reserved name [ {', '.join(reserved_names)} ]", "https://www.lean.io/docs/v2/lean-cli/key-concepts/troubleshooting#02-Common-Errors") diff --git a/lean/components/cloud/cloud_project_manager.py b/lean/components/cloud/cloud_project_manager.py index 528804b6..b151a48c 100644 --- a/lean/components/cloud/cloud_project_manager.py +++ b/lean/components/cloud/cloud_project_manager.py @@ -66,7 +66,7 @@ def get_cloud_project(self, input: str, push: bool) -> QCProject: # If the given input is a valid project directory, we try to use that project local_path = Path.cwd() / input - if self._project_config_manager.try_get_project_config(local_path, self._path_manager): + if self._project_config_manager.try_get_project_config(local_path): if push: self._push_manager.push_projects([local_path]) diff --git a/lean/components/config/project_config_manager.py b/lean/components/config/project_config_manager.py index 57227e06..1a9aeb80 100644 --- a/lean/components/config/project_config_manager.py +++ b/lean/components/config/project_config_manager.py @@ -16,7 +16,6 @@ from lean.components.config.storage import Storage from lean.components.util.xml_manager import XMLManager -from lean.components.util.path_manager import PathManager from lean.constants import PROJECT_CONFIG_FILE_NAME from lean.models.utils import CSharpLibrary @@ -31,15 +30,13 @@ def __init__(self, xml_manager: XMLManager) -> None: """ self._xml_manager = xml_manager - def try_get_project_config(self, project_directory: Path, path_manager: PathManager) -> Storage: + def try_get_project_config(self, project_directory: Path) -> Storage: """Returns a Storage instance to get/set the configuration for a project. :param project_directory: the path to the project to retrieve the configuration of :return: the Storage instance containing the project-specific configuration of the given project """ - if path_manager.is_path_valid(project_directory) \ - and project_directory.is_dir() \ - and self.get_project_config(project_directory).file.exists(): + if self.get_project_config(project_directory).file.exists(): return Storage(str(project_directory / PROJECT_CONFIG_FILE_NAME)) else: return False diff --git a/lean/components/util/library_manager.py b/lean/components/util/library_manager.py index b58382c2..7464b869 100644 --- a/lean/components/util/library_manager.py +++ b/lean/components/util/library_manager.py @@ -54,9 +54,6 @@ def is_lean_library(self, path: Path) -> bool: :param path: path to check whether it is a Lean library :return: true if the path is a Lean library path """ - if not self._path_manager.is_path_valid(path): - return False - relative_path = self._path_manager.get_relative_path(path, self._lean_config_manager.get_cli_root_directory()) path_parts = relative_path.parts library_config = self._project_config_manager.get_project_config(path) diff --git a/lean/components/util/path_manager.py b/lean/components/util/path_manager.py index dc1f1962..4731c8c8 100644 --- a/lean/components/util/path_manager.py +++ b/lean/components/util/path_manager.py @@ -12,17 +12,21 @@ # limitations under the License. from pathlib import Path + from lean.components import reserved_names, forbidden_characters +from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.util.platform_manager import PlatformManager + class PathManager: """The PathManager class provides utilities for working with paths.""" - def __init__(self, platform_manager: PlatformManager) -> None: + def __init__(self, lean_config_manager: LeanConfigManager, platform_manager: PlatformManager) -> None: """Creates a new PathManager instance. :param platform_manager: the PlatformManager used when checking which operating system is in use """ + self._lean_config_manager = lean_config_manager self._platform_manager = platform_manager def get_relative_path(self, destination: Path, source: Path = Path.cwd()) -> Path: @@ -44,15 +48,18 @@ def is_name_valid(self, name: str) -> bool: :return: True if the name is valid on Windows operating system, False if not """ import re - return re.match('^[-_a-zA-Z0-9\\s/]*$', name) is not None + return re.match(r'^[-_a-zA-Z0-9/\s]*$', name) is not None def is_path_valid(self, path: Path) -> bool: - """Returns whether a path is valid on the current operating system. + """Returns whether the given path is a valid project path in the current operating system. + + This method should only be used to check paths relative to the current lean init folder. + Passing an absolute path might result in false being returned since especial cases for root directories + for each operating system (like devices in Windows) are not validated. :param path: the path to validate :return: True if the path is valid on the current operating system, False if not """ - from platform import system try: # This call fails if the path contains invalid characters path.exists() @@ -63,9 +70,6 @@ def is_path_valid(self, path: Path) -> bool: # Trying to create them does raise errors, so we manually validate path components # We follow the rules of windows for every OS components = path.as_posix().split("/") - if system() == "Windows": - # Skip the first component, which contains the drive name - components = components[1:] for component in components: if component.startswith(" ") or component.endswith(" ") or component.endswith("."): return False @@ -78,3 +82,23 @@ def is_path_valid(self, path: Path) -> bool: if forbidden_character in component: return False return True + + def is_cli_path_valid(self, path: Path) -> bool: + """Returns whether the given path is a valid project path in the current operating system. + + :param path: the path to validate + :return: True if the path is valid on the current operating system, False if not + """ + from lean.models.errors import MoreInfoError + + try: + cli_root_dir = self._lean_config_manager.get_cli_root_directory() + relative_path = path.relative_to(cli_root_dir) + except MoreInfoError: + from platform import system + if system() == "Windows": + # Skip the first component, which contains the drive name + posix_path = path.as_posix() + relative_path = Path(posix_path[posix_path.index('/'):]) + + return relative_path == Path(".") or self.is_path_valid(relative_path) diff --git a/lean/container.py b/lean/container.py index 4efd2f05..226f8ebb 100644 --- a/lean/container.py +++ b/lean/container.py @@ -14,6 +14,9 @@ from typing import Union, Any from lean.components.api.api_client import APIClient +from lean.components.config.lean_config_manager import LeanConfigManager +from lean.components.config.project_config_manager import ProjectConfigManager +from lean.components.util.path_manager import PathManager from lean.components.cloud.cloud_project_manager import CloudProjectManager from lean.components.cloud.cloud_runner import CloudRunner from lean.components.cloud.data_downloader import DataDownloader @@ -21,10 +24,8 @@ from lean.components.cloud.pull_manager import PullManager from lean.components.cloud.push_manager import PushManager from lean.components.config.cli_config_manager import CLIConfigManager -from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.config.optimizer_config_manager import OptimizerConfigManager from lean.components.config.output_config_manager import OutputConfigManager -from lean.components.config.project_config_manager import ProjectConfigManager from lean.components.config.storage import Storage from lean.components.docker.docker_manager import DockerManager from lean.components.docker.lean_runner import LeanRunner @@ -34,7 +35,6 @@ from lean.components.util.market_hours_database import MarketHoursDatabase from lean.components.util.name_generator import NameGenerator from lean.components.util.organization_manager import OrganizationManager -from lean.components.util.path_manager import PathManager from lean.components.util.platform_manager import PlatformManager from lean.components.util.project_manager import ProjectManager from lean.components.util.task_manager import TaskManager @@ -63,7 +63,6 @@ def initialize(self, self.platform_manager = PlatformManager() self.task_manager = TaskManager(self.logger) self.name_generator = NameGenerator() - self.path_manager = PathManager(self.platform_manager) self.temp_manager = TempManager() self.xml_manager = XMLManager() self.http_client = HTTPClient(self.logger) @@ -92,6 +91,7 @@ def initialize(self, self.project_config_manager, self.module_manager, self.cache_storage) + self.path_manager = PathManager(self.lean_config_manager, self.platform_manager) self.output_config_manager = OutputConfigManager(self.lean_config_manager) self.optimizer_config_manager = OptimizerConfigManager(self.logger) diff --git a/tests/commands/test_build.py b/tests/commands/test_build.py index 21d36f3d..60d5cd79 100644 --- a/tests/commands/test_build.py +++ b/tests/commands/test_build.py @@ -22,12 +22,18 @@ from lean.models.docker import DockerImage from lean.constants import CUSTOM_FOUNDATION, CUSTOM_RESEARCH, CUSTOM_ENGINE from tests.conftest import initialize_container +from tests.test_helpers import create_fake_lean_cli_directory CUSTOM_FOUNDATION_IMAGE = DockerImage(name=CUSTOM_FOUNDATION, tag="latest") CUSTOM_ENGINE_IMAGE = DockerImage(name=CUSTOM_ENGINE, tag="latest") CUSTOM_RESEARCH_IMAGE = DockerImage(name=CUSTOM_RESEARCH, tag="latest") +@pytest.fixture(autouse=True) +def create_fake_cli_directory() -> None: + create_fake_lean_cli_directory() + + def create_fake_repositories() -> None: lean_dir = Path.cwd() / "Lean" diff --git a/tests/commands/test_create_project.py b/tests/commands/test_create_project.py index 034ba3cf..c50c7940 100644 --- a/tests/commands/test_create_project.py +++ b/tests/commands/test_create_project.py @@ -153,7 +153,7 @@ def test_create_project_aborts_when_path_invalid() -> None: create_fake_lean_cli_directory() path_manager = mock.Mock() - path_manager.is_path_valid.return_value = False + path_manager.is_cli_path_valid.return_value = False container.path_manager= path_manager result = CliRunner().invoke(lean, ["create-project", "--language", "python", "My First Project"]) diff --git a/tests/components/docker/test_lean_runner.py b/tests/components/docker/test_lean_runner.py index 54c97f15..479e0542 100644 --- a/tests/components/docker/test_lean_runner.py +++ b/tests/components/docker/test_lean_runner.py @@ -57,7 +57,7 @@ def create_lean_runner(docker_manager: mock.Mock) -> LeanRunner: xml_manager = XMLManager() platform_manager = PlatformManager() - path_manager = PathManager(platform_manager) + path_manager = PathManager(lean_config_manager, platform_manager) project_manager = ProjectManager(logger, project_config_manager, lean_config_manager, diff --git a/tests/components/util/test_library_manager.py b/tests/components/util/test_library_manager.py index a0272d8e..f0263e33 100644 --- a/tests/components/util/test_library_manager.py +++ b/tests/components/util/test_library_manager.py @@ -37,7 +37,7 @@ def _create_library_manager() -> LibraryManager: mock.Mock(), cache_storage) platform_manager = PlatformManager() - path_manager = PathManager(platform_manager) + path_manager = PathManager(lean_config_manager, platform_manager) xml_manager = XMLManager() project_config_manager = ProjectConfigManager(xml_manager) logger = mock.Mock() diff --git a/tests/components/util/test_path_manager.py b/tests/components/util/test_path_manager.py index 364b5675..5fcfa188 100644 --- a/tests/components/util/test_path_manager.py +++ b/tests/components/util/test_path_manager.py @@ -13,9 +13,11 @@ import platform from pathlib import Path +from unittest import mock import pytest +from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.util.path_manager import PathManager from lean.components.util.platform_manager import PlatformManager @@ -26,8 +28,15 @@ def fake_filesystem() -> None: return None +def _get_path_manager() -> PathManager: + lean_config_manager = mock.Mock() + lean_config_manager.get_cli_root_directory = mock.MagicMock(return_value=Path.cwd()) + + return PathManager(lean_config_manager, PlatformManager()) + + def test_get_relative_path_returns_relative_path_when_destination_is_relative_to_source() -> None: - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() source = Path.cwd() destination = Path.cwd() / "path" / "to" / "file.txt" @@ -36,7 +45,7 @@ def test_get_relative_path_returns_relative_path_when_destination_is_relative_to def test_get_relative_path_returns_full_destination_path_when_destination_is_not_relative_to_source() -> None: - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() source = Path.cwd() destination = Path.cwd().parent @@ -45,15 +54,15 @@ def test_get_relative_path_returns_full_destination_path_when_destination_is_not def test_get_relative_path_uses_cwd_as_source_when_not_given() -> None: - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() assert path_manager.get_relative_path(Path.cwd() / "path" / "to" / "file.txt") == Path("path/to/file.txt") def test_is_path_valid_returns_true_for_valid_path() -> None: - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() - assert path_manager.is_path_valid(Path.cwd() / "My Path/file.txt") + assert path_manager.is_cli_path_valid(Path.cwd() / "My Path/file.txt") @pytest.mark.parametrize("path,valid", [("My Path/file.txt", True), @@ -95,21 +104,21 @@ def test_is_path_valid_windows(path: str, valid: bool) -> None: if platform.system() != "Windows": pytest.skip("This test requires Windows") - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() - assert path_manager.is_path_valid(Path.cwd() / path) == valid + assert path_manager.is_cli_path_valid(Path.cwd() / path) == valid @pytest.mark.parametrize("name, valid", [("123", True), ("abc", True), ("1a2b3c", True), ("a-1_b-2", True), - ("1-a 2_b 3-c", True), + ("/1-a/2_b 3 c/xyz", True), ("1 a/2_b/3-c", True), ("1a2b3c$", False), ("abc:123", False), ("abc.def", False)]) def test_is_name_valid(name: str, valid: bool) -> None: - path_manager = PathManager(PlatformManager()) + path_manager = _get_path_manager() assert path_manager.is_name_valid(name) == valid diff --git a/tests/components/util/test_project_manager.py b/tests/components/util/test_project_manager.py index 5e34ccc0..1990c0be 100644 --- a/tests/components/util/test_project_manager.py +++ b/tests/components/util/test_project_manager.py @@ -38,15 +38,16 @@ def _create_project_manager() -> ProjectManager: project_config_manager = ProjectConfigManager(xml_manager) cache_storage = Storage(str(Path("~/.lean/cache").expanduser())) platform_manager = PlatformManager() - path_manager = PathManager(platform_manager) - - return ProjectManager(logger, - project_config_manager, - LeanConfigManager(mock.Mock(), + lean_config_manager = LeanConfigManager(mock.Mock(), mock.Mock(), project_config_manager, mock.Mock(), - cache_storage), + cache_storage) + path_manager = PathManager(lean_config_manager, platform_manager) + + return ProjectManager(logger, + project_config_manager, + lean_config_manager, path_manager, xml_manager, platform_manager) diff --git a/tests/test_click.py b/tests/test_click.py index 70180eae..e6531f1a 100644 --- a/tests/test_click.py +++ b/tests/test_click.py @@ -23,6 +23,7 @@ from lean.click import DateParameter, LeanCommand, PathParameter from lean.container import container +from tests.test_helpers import create_fake_lean_cli_directory def test_lean_command_enables_verbose_logging_when_verbose_option_given() -> None: @@ -41,11 +42,14 @@ def command() -> None: def test_lean_command_sets_default_lean_config_path_when_lean_config_option_given() -> None: + create_fake_lean_cli_directory() + @click.command(cls=LeanCommand, requires_lean_config=True) def command() -> None: pass lean_config_manager = mock.Mock() + lean_config_manager.get_cli_root_directory = mock.MagicMock(return_value=Path.cwd()) container.lean_config_manager = lean_config_manager with (Path.cwd() / "custom-config.json").open("w+", encoding="utf-8") as file: @@ -128,7 +132,7 @@ def command(arg: Path) -> None: pass path_manager = mock.Mock() - path_manager.is_path_valid.return_value = False + path_manager.is_cli_path_valid.return_value = False container.path_manager = path_manager result = CliRunner().invoke(command, ["invalid-path.txt"])