get_uri, resolve_uri - refactor #6605

1) Use pathlib for consistency
2) Ensure consistent output - always use posix format and resolve paths (but keep the symlinks)

src/bonsai/bonsai/tool/ifc.py

@@ -263,31 +263,46 @@ class Ifc(bonsai.core.tool.Ifc):
         IfcStore.history_edit_object(obj, finish_editing=True)
 
     @classmethod
-    def resolve_uri(cls, uri: str) -> str:
-        """Get absolute path based on the active IFC file."""
-        if os.path.isabs(uri):
-            return uri
-        ifc_path = cls.get_path()
-        if os.path.isfile(ifc_path):
-            ifc_path = os.path.dirname(ifc_path)
-        return (uri if not uri else os.path.join(ifc_path, uri)).replace("\\", "/")
+    def normalize_path(cls, path: Union[Path, str]) -> str:
+        # Do not use `Path.resolve` as it will resolve symlinks too.
+        return Path(os.path.normpath(path)).as_posix()
 
     @classmethod
-    def get_uri(cls, uri: str | Path, use_relative_path: bool = False) -> str:
-        """Get path relative to the active IFC file, if `use_relative_path` is `True`.
-
-        If `use_relative_path` is `False` - get absolute filepath from uri.
-        """
-        if not use_relative_path:
-            return Path(uri).absolute().resolve().as_posix()
+    def resolve_uri(cls, uri: str | Path) -> str:
+        """Get absolute path based on the active IFC file."""
         uri = Path(uri)
-        if not os.path.isabs(uri) or not (ifc_path := cls.get_path()):
-            return uri.as_posix().replace("\\", "/")
-        if Path(uri).drive != Path(ifc_path).drive:
-            return uri.as_posix().replace("\\", "/")
-        if os.path.isfile(ifc_path):
-            ifc_path = os.path.dirname(ifc_path)
-        return Path(os.path.relpath(uri, ifc_path)).as_posix().replace("\\", "/")
+        if uri.is_absolute():
+            return cls.normalize_path(uri)
+        ifc_path = Path(cls.get_path())
+        if ifc_path.is_file():
+            ifc_path = ifc_path.parent
+        if not str(uri):
+            # TODO: When does it occur and why we return empty path in this case?
+            return str(uri)
+        return cls.normalize_path(ifc_path / uri)
+
+    @classmethod
+    def get_uri(cls, uri: str | Path, use_relative_path: bool) -> str:
+        """Get path relative to the active IFC file, if ``use_relative_path`` is ``True``.
+
+        ``use_relative_path`` is ``False``:
+        - return ``uri`` as-is if it's absolute
+        - raise ``ValueError`` if ``uri`` is relative (this condition indicates deeper code error)
+        """
+        uri = Path(uri)
+        if not use_relative_path:
+            if not uri.is_absolute():
+                raise ValueError(f"Unexpected relative path in get_uri: '{uri}'.")
+            return cls.normalize_path(uri)
+        if not uri.is_absolute() or not (ifc_path := cls.get_path()):
+            return cls.normalize_path(uri)
+        ifc_path = Path(ifc_path)
+        if uri.drive != ifc_path.drive:
+            return cls.normalize_path(uri)
+        if ifc_path.is_file():
+            ifc_path = ifc_path.parent
+        # Use `os.path.relpath` as it does support creating '..' paths.
+        return cls.normalize_path(Path(os.path.relpath(uri, ifc_path)))
 
     @classmethod
     def unlink(

src/bonsai/test/tool/test_ifc.py

@@ -22,6 +22,8 @@ import test.bim.bootstrap
 import bonsai.core.tool
 import bonsai.tool as tool
 import pytest
+import tempfile
+from pathlib import Path
 from bonsai.tool.ifc import Ifc as subject
 
 
@@ -237,3 +239,102 @@ class TestUnlink(test.bim.bootstrap.NewFile):
             subject.unlink(element=element, obj=obj)
         assert subject.get_entity(obj) == element
         assert subject.get_object(element) == obj
+
+
+class TestUri(test.bim.bootstrap.NewFile):
+    def test_get_uri(self):
+        ifc = ifcopenshell.file()
+        subject.set(ifc)
+
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_path = Path(tmp_dir)
+            project_dir = base_path / "project"
+            project_dir.mkdir()
+            path = project_dir / "project.ifc"
+            test_name = "test.xml"
+            test_filepath = project_dir / test_name
+            test_filepath.touch()
+            path.touch()
+            subject.set_path(subject.normalize_path(path))
+
+            # Test absolute filepaths.
+            abs_filepath = str(test_filepath)
+            assert subject.get_uri(abs_filepath, False) == subject.normalize_path(abs_filepath)
+            assert subject.get_uri(abs_filepath, True) == test_name
+
+            # Test relative filepaths.
+            rel_filepath = test_name
+            with pytest.raises(ValueError):
+                subject.get_uri(rel_filepath, False)
+
+            assert subject.get_uri(rel_filepath, True) == test_name
+
+            # Test that paths are resolved, but keeping symlinks.
+            link_name = "link.xml"
+            link_path = base_path / link_name
+            link_path.symlink_to(test_filepath)
+
+            # Test absolute paths.
+            abs_filepath = test_filepath.parent / ".." / link_name
+            assert subject.get_uri(abs_filepath, False) == subject.normalize_path(abs_filepath)
+            assert subject.get_uri(abs_filepath, True) == "../" + link_name
+
+            # Test relative paths.
+            rel_filepath = "../" + link_name
+            with pytest.raises(ValueError):
+                subject.get_uri(rel_filepath, False)
+            assert subject.get_uri(rel_filepath, True) == rel_filepath
+
+    def test_resolve_uri(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_path = Path(tmp_dir)
+            project_dir = base_path / "project"
+            project_dir.mkdir()
+            path = project_dir / "project.ifc"
+            path.touch()
+            subject.set_path(subject.normalize_path(path))
+            test_name = "test.xml"
+            test_filepath = project_dir / test_name
+            test_filepath.touch()
+
+            # Test absolute filepaths.
+            abs_filepath = str(test_filepath)
+            assert subject.resolve_uri(abs_filepath) == subject.normalize_path(abs_filepath)
+            rel_filepath = test_name
+            assert subject.resolve_uri(rel_filepath) == subject.normalize_path(abs_filepath)
+
+            # Test that paths are resolved, but keeping symlinks.
+            link_name = "link.xml"
+            link_path = base_path / link_name
+            link_path.symlink_to(test_filepath)
+
+            # Test absolute path.
+            abs_filepath = test_filepath.parent / ".." / link_name
+            assert subject.resolve_uri(abs_filepath) == subject.normalize_path(abs_filepath)
+
+            # Test relative path.
+            rel_filepath = "../" + link_name
+            assert subject.resolve_uri(rel_filepath) == subject.normalize_path(abs_filepath)
+
+    def test_normalize_path(self):
+        # Posix format.
+        path = r"test\test.txt"
+        assert subject.normalize_path(path) == "test/test.txt"
+
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_path = Path(tmp_dir)
+            project_path = base_path / "project"
+            project_path.mkdir()
+            original_file = project_path / "test.xml"
+            original_file.touch()
+
+            link_name = "link.xml"
+            link_path = project_path / link_name
+            link_path.symlink_to(original_file)
+
+            # Absolute path.
+            abs_path = project_path / ".." / link_name
+            assert subject.normalize_path(abs_path) == (base_path / link_name).as_posix()
+
+            rel_path = "../" + link_name
+            assert subject.normalize_path(rel_path) == rel_path