fix: fix megfile ls for alias paths (#423)

* fix megfile ls

* fix unit-tests

* fix unit-tests in Python 3.12

* make lint happy

* add more tests

---------

Co-authored-by: liyang <[email protected]>

Author: bbtfr

megfile/cli.py

@@ -29,6 +29,7 @@
     smart_open,
     smart_path_join,
     smart_readlink,
+    smart_relpath,
     smart_remove,
     smart_rename,
     smart_scan_stat,
@@ -78,7 +79,7 @@ def get_echo_path(file_stat, base_path: str = "", full_path: bool = False):
     elif full_path:
         path = file_stat.path
     else:
-        path = os.path.relpath(file_stat.path, start=base_path)
+        path = smart_relpath(file_stat.path, start=base_path)
     return path
 
 

megfile/pathlike.py

@@ -636,8 +636,8 @@ def relative_to(self: Self, *other: str) -> Self:
             relative = path[len(other_path) :]
             relative = relative.lstrip("/")
             return type(self)(relative)  # pyre-ignore[19]
-        else:
-            raise ValueError("%r does not start with %r" % (path, other))
+
+        raise ValueError("%r does not start with %r" % (path, other))
 
     def with_name(self: Self, name: str) -> Self:
         """Return a new path with the name changed"""
@@ -655,6 +655,21 @@ def with_suffix(self: Self, suffix: str) -> Self:
         raw_suffix = self.suffix
         return self.from_path(path[: len(path) - len(raw_suffix)] + suffix)
 
+    def relpath(self, start=None):
+        """Return the relative path."""
+        if start is None:
+            raise TypeError("start is required")
+
+        other_path = self.from_path(start).path_with_protocol
+        path = self.path_with_protocol
+
+        if path.startswith(other_path):
+            relative = path[len(other_path) :]
+            relative = relative.lstrip("/")
+            return relative
+
+        raise ValueError("%r does not start with %r" % (path, other_path))
+
     def is_absolute(self) -> bool:
         return True
 

megfile/smart_path.py

@@ -1,11 +1,11 @@
 import os
 from configparser import ConfigParser
 from pathlib import PurePath
-from typing import Dict, Tuple, Union
+from typing import Dict, Optional, Tuple, Union
 
 from megfile.lib.compat import fspath
 from megfile.lib.url import get_url_scheme
-from megfile.utils import classproperty
+from megfile.utils import cached_classproperty
 
 from .errors import ProtocolExistsError, ProtocolNotFoundError
 from .interfaces import BasePath, BaseURIPath, PathLike
@@ -54,15 +54,15 @@ def __init__(self, path: Union[PathLike, int], *other_paths: PathLike):
             self.path = str(pathlike)
         self.pathlike = pathlike
 
-    @classproperty
+    @cached_classproperty
     def _aliases(cls) -> Dict[str, Dict[str, str]]:
         config_path = os.path.expanduser(aliases_config)
-        aliases = _load_aliases_config(config_path)
-        setattr(cls, "_aliases", aliases)
-        return aliases
+        return _load_aliases_config(config_path)
 
-    @staticmethod
-    def _extract_protocol(path: Union[PathLike, int]) -> Tuple[str, Union[str, int]]:
+    @classmethod
+    def _extract_protocol(
+        cls, path: Union[PathLike, int]
+    ) -> Tuple[str, Union[str, int]]:
         if isinstance(path, int):
             protocol = "file"
             path_without_protocol = path
@@ -74,27 +74,26 @@ def _extract_protocol(path: Union[PathLike, int]) -> Tuple[str, Union[str, int]]
             else:
                 path_without_protocol = path[len(protocol) + 3 :]
         elif isinstance(path, (BaseURIPath, SmartPath)):
-            protocol = path.protocol
-            path_without_protocol = str(path)
+            return str(path.protocol), str(path)
         elif isinstance(path, (PurePath, BasePath)):
-            protocol, path_without_protocol = SmartPath._extract_protocol(fspath(path))
+            return SmartPath._extract_protocol(fspath(path))
         else:
             raise ProtocolNotFoundError("protocol not found: %r" % path)
-        return protocol, path_without_protocol
-
-    @classmethod
-    def _create_pathlike(cls, path: Union[PathLike, int]) -> BaseURIPath:
-        protocol, path_without_protocol = cls._extract_protocol(path)
         aliases: Dict[str, Dict[str, str]] = cls._aliases  # pyre-ignore[9]
         if protocol in aliases:
             protocol = aliases[protocol]["protocol"]
-            path = protocol + "://" + str(path_without_protocol)
+            path = "%s://%s" % (protocol, path_without_protocol)
+        return protocol, path
+
+    @classmethod
+    def _create_pathlike(cls, path: Union[PathLike, int]) -> BaseURIPath:
+        protocol, unaliased_path = cls._extract_protocol(path)
         if protocol.startswith("s3+"):
             protocol = "s3"
         if protocol not in cls._registered_protocols:
             raise ProtocolNotFoundError("protocol %r not found: %r" % (protocol, path))
         path_class = cls._registered_protocols[protocol]
-        return path_class(path)
+        return path_class(unaliased_path)
 
     @classmethod
     def register(cls, path_class, override_ok: bool = False):
@@ -137,7 +136,6 @@ def register(cls, path_class, override_ok: bool = False):
     joinpath = _bind_function("joinpath")
     abspath = _bind_function("abspath")
     realpath = _bind_function("realpath")
-    relpath = _bind_function("relpath")
     is_absolute = _bind_function("is_absolute")
     is_mount = _bind_function("is_mount")
     md5 = _bind_function("md5")
@@ -150,6 +148,16 @@ def protocol(self) -> str:
     def from_uri(cls, path: str):
         return cls(path)
 
+    def relpath(self, start: Optional[str] = None) -> str:
+        """Return the relative path of given path
+
+        :param start: Given start directory
+        :returns: Relative path from start
+        """
+        if start is not None:
+            _, start = SmartPath._extract_protocol(fspath(start))
+        return self.pathlike.relpath(start=start)
+
     as_uri = _bind_function("as_uri")
     as_posix = _bind_function("as_posix")
     __lt__ = _bind_function("__lt__")
@@ -170,7 +178,6 @@ def from_uri(cls, path: str):
     is_mount = _bind_function("is_mount")
     abspath = _bind_function("abspath")
     realpath = _bind_function("realpath")
-    relpath = _bind_function("relpath")
     iterdir = _bind_function("iterdir")
     cwd = _bind_function("cwd")
     home = _bind_function("home")

megfile/utils/__init__.py

@@ -4,7 +4,7 @@
 import os
 import uuid
 from copy import copy
-from functools import wraps
+from functools import cached_property, wraps
 from io import (
     BufferedIOBase,
     BufferedRandom,
@@ -15,6 +15,7 @@
     TextIOBase,
     TextIOWrapper,
 )
+from threading import RLock
 from typing import IO, Callable, Optional
 
 from megfile.utils.mutex import ProcessLocal, ThreadLocal
@@ -298,3 +299,50 @@ def __delete__(self, cls_or_obj) -> None:
         """
         # call this method only on the class, not the instance
         super(classproperty, self).__delete__(_get_class(cls_or_obj))
+
+
+class cached_classproperty(cached_property):
+    """
+    The use this class as a decorator for your class property with cache.
+    Example:
+        @cached_classproperty
+        def prop(cls):
+            return "value"
+    """
+
+    def __init__(self, func: Callable) -> None:
+        """
+        This method initializes the cached_classproperty instance.
+        @param func: The function to be called when the property value is requested.
+        """
+        super().__init__(func)
+        # Python 3.12 removed the lock attribute from cached_property.
+        # Maybe we should remove this in the future.
+        # See also: https://github.com/python/cpython/pull/101890
+        #        https://github.com/python/cpython/issues/87634
+        if not hasattr(func, "lock"):
+            self.lock = RLock()
+
+    def __get__(  # pyre-ignore[14]
+        self,
+        _,
+        cls,  # pytype: disable=signature-mismatch
+    ) -> object:
+        """
+        This method gets called when a property value is requested.
+        @param cls: The class type of the above instance.
+        @return: The value of the property.
+        """
+        if self.attrname is None:
+            raise TypeError(
+                "Cannot use cached_classproperty instance without calling "
+                "__set_name__ on it."
+            )
+        with self.lock:
+            # check if another thread filled cache while we awaited lock
+            # cannot use getattr since it will cause RecursionError
+            val = cls.__dict__[self.attrname]
+            if val is self:
+                val = self.func(cls)
+                setattr(cls, self.attrname, val)
+        return val

tests/test_smart.py

@@ -931,6 +931,11 @@ def s3_path():
     yield "s3://bucket/test"
 
 
+@pytest.fixture
+def s3_bucket():
+    yield "s3://bucket"
+
+
 @pytest.fixture
 def abs_path(fs):
     fs.create_file(os.path.join(os.path.dirname(__file__), "test"))
@@ -979,11 +984,14 @@ def test_smart_realpath(s3_path, abs_path, link_path):
     assert smart.smart_realpath(link_path) == abs_path
 
 
-def test_smart_relpath(mocker, s3_path, abs_path, rel_path):
+def test_smart_relpath(mocker, s3_path, s3_bucket, abs_path, rel_path):
     mocker.patch("os.getcwd", return_value=os.path.dirname(__file__))
-    with pytest.raises(NotImplementedError):
+    with pytest.raises(TypeError):
         assert smart.smart_relpath(s3_path) == s3_path
+    with pytest.raises(ValueError):
+        smart.smart_relpath(s3_path, abs_path)
     assert smart.smart_relpath(abs_path, os.path.dirname(__file__)) == rel_path
+    assert smart.smart_relpath(s3_path, s3_bucket) == rel_path
 
 
 def test_smart_open_stdin(mocker):

tests/test_smart_path.py

@@ -113,31 +113,31 @@ def test_extract_protocol():
     )
     assert SmartPath._extract_protocol(FS_TEST_ABSOLUTE_PATH_WITH_PROTOCOL) == (
         FSPath.protocol,
-        FS_TEST_ABSOLUTE_PATH,
+        FS_TEST_ABSOLUTE_PATH_WITH_PROTOCOL,
     )
     assert SmartPath._extract_protocol(FS_TEST_RELATIVE_PATH) == (
         FSPath.protocol,
         FS_TEST_RELATIVE_PATH,
     )
     assert SmartPath._extract_protocol(FS_TEST_RELATIVE_PATH_WITH_PROTOCOL) == (
         FSPath.protocol,
-        FS_TEST_RELATIVE_PATH,
+        FS_TEST_RELATIVE_PATH_WITH_PROTOCOL,
     )
     assert SmartPath._extract_protocol(S3_TEST_PATH) == (
         S3Path.protocol,
-        S3_TEST_PATH_WITHOUT_PROTOCOL,
+        S3_TEST_PATH,
     )
     assert SmartPath._extract_protocol(HTTP_TEST_PATH) == (
         HttpPath.protocol,
-        HTTP_TEST_PATH_WITHOUT_PROTOCOL,
+        HTTP_TEST_PATH,
     )
     assert SmartPath._extract_protocol(HTTPS_TEST_PATH) == (
         HttpsPath.protocol,
-        HTTPS_TEST_PATH_WITHOUT_PROTOCOL,
+        HTTPS_TEST_PATH,
     )
     assert SmartPath._extract_protocol(STDIO_TEST_PATH) == (
         StdioPath.protocol,
-        STDIO_TEST_PATH_WITHOUT_PROTOCOL,
+        STDIO_TEST_PATH,
     )
 
     fs_path = FSPath(FS_TEST_ABSOLUTE_PATH)

tests/utils/test_init.py

@@ -6,6 +6,7 @@
     _get_class,
     _is_pickle,
     binary_open,
+    cached_classproperty,
     combine,
     get_human_size,
     necessary_params,
@@ -55,6 +56,34 @@ class Test:
     assert _get_class(Test) == _get_class(Test())
 
 
+def test_cached_classproperty():
+    class Test1:
+        count = 0
+
+        @cached_classproperty
+        def test(cls):
+            cls.count += 1
+            return cls.count
+
+    assert Test1().test == 1
+    assert Test1().test == 1
+    assert Test1.test == 1
+    assert Test1.test == 1
+
+    class Test2:
+        count = 0
+
+        @cached_classproperty
+        def test(cls):
+            cls.count += 1
+            return cls.count
+
+    assert Test2.test == 1
+    assert Test2.test == 1
+    assert Test2().test == 1
+    assert Test2().test == 1
+
+
 def test__is_pickle():
     data = "test"
     fileObj = BytesIO(pickle.dumps(data))