From a1f18c9a96d9c9af24dc88c4a02c1ca7d9f35ee3 Mon Sep 17 00:00:00 2001
From: Filippo Luca Ferretti <102977828+flferretti@users.noreply.github.com>
Date: Mon, 11 Nov 2024 15:34:18 +0100
Subject: [PATCH 1/2] Refactor SDF handling logic for non-existing string paths

---
 src/rod/sdf/sdf.py         | 45 ++++++++++++++++----------------------
 tests/test_urdf_parsing.py |  5 +++++
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/src/rod/sdf/sdf.py b/src/rod/sdf/sdf.py
index 77cc680..a0a5e25 100644
--- a/src/rod/sdf/sdf.py
+++ b/src/rod/sdf/sdf.py
@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import dataclasses
-import os
 import pathlib
 
 import mashumaro
@@ -60,34 +59,28 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf:
             The parsed SDF file.
         """
 
-        # Handle the max path length depending on the OS
-        try:
-            from ctypes.wintypes import MAX_PATH
-        except ValueError:
-            MAX_PATH = os.pathconf("/", "PC_PATH_MAX")
-
-        if is_urdf is not None:
-            sdf_string = sdf
-        else:
-            match sdf:
-                # Case 1: It's a Path object
-                case pathlib.Path():
-                    sdf_string = sdf.read_text()
-                    is_urdf = sdf.suffix == ".urdf"
-
-                # Case 2: It's a string with a path
-                case str() if len(sdf) <= MAX_PATH and pathlib.Path(sdf).is_file():
-                    sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8")
-                    is_urdf = pathlib.Path(sdf).suffix == ".urdf"
+        path = pathlib.Path(sdf)
 
-                # Case 3: It's an SDF/URDF string
-                case str():
+        match sdf:
+            # Case 1: Handle strings.
+            case str():
+                if path.suffix:
+                    # Assuming that if the string has a suffix, it is a path.
+                    sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8")
+                else:
+                    # Otherwise, it is an SDF string.
                     sdf_string = sdf
-                    is_urdf = "<robot" in sdf_string
 
-                # Case 4: Raise an error for unsupported types
-                case _:
-                    raise TypeError(f"Unsupported type for 'sdf': {type(sdf)}")
+                is_urdf = is_urdf if is_urdf is not None else "<robot" in sdf_string
+
+            # Case 2: Handle pathlib.Path.
+            case pathlib.Path():
+                sdf_string = path.read_text(encoding="utf-8")
+                is_urdf = is_urdf if is_urdf is not None else path.suffix == ".urdf"
+
+            # Case 3: Raise an error for unsupported types.
+            case _:
+                raise TypeError(f"Unsupported type for 'sdf': {type(sdf)}")
 
         # Convert SDF to URDF if needed (it requires system executables)
         if is_urdf:
diff --git a/tests/test_urdf_parsing.py b/tests/test_urdf_parsing.py
index 88d592a..4591693 100644
--- a/tests/test_urdf_parsing.py
+++ b/tests/test_urdf_parsing.py
@@ -35,6 +35,11 @@ def test_urdf_parsing(robot: Robot) -> None:
     with pytest.raises(RuntimeError):
         _ = rod.Sdf.load(sdf=urdf_path.read_text(), is_urdf=False)
 
+    # Check that it fails is is_urdf=True and the resource is a non-existing path
+    with pytest.raises(FileNotFoundError):
+        _ = rod.Sdf.load(sdf="/non/existing/path", is_urdf=None)
+        _ = rod.Sdf.load(sdf="/non/existing/path", is_urdf=False)
+
     # The following instead should succeed
     _ = rod.Sdf.load(sdf=urdf_path, is_urdf=None)
     _ = rod.Sdf.load(sdf=urdf_path, is_urdf=True)

From e0cf6ad046a62865cca67e105ccd40ed6df3010d Mon Sep 17 00:00:00 2001
From: Filippo Luca Ferretti <filippoluca.ferretti@outlook.com>
Date: Mon, 11 Nov 2024 17:47:59 +0100
Subject: [PATCH 2/2] Add additional test for non-existing string path

---
 tests/test_urdf_parsing.py | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/tests/test_urdf_parsing.py b/tests/test_urdf_parsing.py
index 4591693..87332a7 100644
--- a/tests/test_urdf_parsing.py
+++ b/tests/test_urdf_parsing.py
@@ -1,3 +1,5 @@
+import pathlib
+
 import pytest
 import robot_descriptions
 import robot_descriptions.loaders.idyntree
@@ -37,8 +39,11 @@ def test_urdf_parsing(robot: Robot) -> None:
 
     # Check that it fails is is_urdf=True and the resource is a non-existing path
     with pytest.raises(FileNotFoundError):
-        _ = rod.Sdf.load(sdf="/non/existing/path", is_urdf=None)
-        _ = rod.Sdf.load(sdf="/non/existing/path", is_urdf=False)
+        _ = rod.Sdf.load(sdf="/non/existing/path.sdf", is_urdf=False)
+
+    # Check that it fails is is_urdf=True and the resource is an empty string
+    with pytest.raises(FileNotFoundError):
+        _ = rod.Sdf.load(sdf=pathlib.Path("/non/existing/path.sdf"), is_urdf=False)
 
     # The following instead should succeed
     _ = rod.Sdf.load(sdf=urdf_path, is_urdf=None)
@@ -46,6 +51,18 @@ def test_urdf_parsing(robot: Robot) -> None:
     _ = rod.Sdf.load(sdf=str(urdf_path), is_urdf=None)
     _ = rod.Sdf.load(sdf=str(urdf_path), is_urdf=True)
     _ = rod.Sdf.load(sdf=urdf_path.read_text(), is_urdf=True)
+    _ = rod.Sdf.load(
+        sdf="<robot name='minimal_robot'> \
+                <link name='base_link'/> \
+                <joint name='base_to_link1' type='continuous'> \
+                    <parent link='base_link'/> \
+                    <child link='link1'/> \
+                    <origin xyz='0 0 1' rpy='0 0 0'/> \
+                </joint> \
+                <link name='link1'/> \
+                </robot> \
+    "
+    )
 
     # Load once again the urdf
     rod_sdf = rod.Sdf.load(sdf=urdf_path)