Merge pull request #235 from jhonabreul/update-project-name-rules

Fix project validation regex

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():

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:

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")
 

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])
 

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

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)

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)

lean/container.py

@@ -14,17 +14,18 @@
 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
 from lean.components.cloud.module_manager import ModuleManager
 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)
 

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"
 

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"])

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,

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()

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

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)

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"])