Fix remote path mapping checking edge cases (#56)

#49

* Add a validator to the `remote_path` and `local_path` fields that ensures a trailing slash is added to the values of the, if the values defined in the configuration don't have them. This matches Sonarr's behaviour when setting these values, and fixes an issue where Buildarr would always try to add remote path mappings without trailing spaces in the paths to Sonarr, even if they already existed.
* Switch the `remote_path` and `local_path` fields for remote path mappings to a custom `OSAgnosticPath` type, that semi-transparently handles differences between POSIX and Windows paths. Windows paths will be compared case-insensitively.
* Update resource management logging for remote path mappings to the latest standards for Buildarr, and remove extraneous logging.

buildarr_sonarr/config/download_clients/remote_path_mappings.py

@@ -24,10 +24,12 @@
 
 from buildarr.config import RemoteMapEntry
 from buildarr.types import BaseEnum, NonEmptyStr
+from pydantic import validator
 from typing_extensions import Self
 
 from ...api import api_delete, api_get, api_post, api_put
 from ...secrets import SonarrSecrets
+from ...types import OSAgnosticPath
 from ..types import SonarrConfigBase
 
 logger = getLogger(__name__)
@@ -46,6 +48,9 @@ class Ensure(BaseEnum):
     present = "present"
     absent = "absent"
 
+    def __repr__(self) -> str:
+        return repr(self.name)
+
 
 class RemotePathMapping(SonarrConfigBase):
     """
@@ -61,14 +66,22 @@ class RemotePathMapping(SonarrConfigBase):
     The name of the host, as specified for the remote download client.
     """
 
-    remote_path: NonEmptyStr
+    remote_path: OSAgnosticPath
     """
     Root path to the directory that the download client accesses.
+
+    *Changed in version 0.6.4*: Path checking will now match paths
+    whether or not the defined path ends in a trailing slash.
+    Path checking on Windows paths is now case-insensitive.
     """
 
-    local_path: NonEmptyStr
+    local_path: OSAgnosticPath
     """
     The path that Sonarr should use to access the remote path locally.
+
+    *Changed in version 0.6.4*: Path checking will now match paths
+    whether or not the defined path ends in a trailing slash.
+    Path checking on Windows paths is now case-insensitive.
     """
 
     ensure: Ensure = Ensure.present
@@ -88,6 +101,12 @@ class RemotePathMapping(SonarrConfigBase):
         ("local_path", "localPath", {}),
     ]
 
+    @validator("remote_path", "local_path")
+    def add_trailing_slash(cls, value: OSAgnosticPath) -> OSAgnosticPath:
+        if value.is_windows():
+            return (value + "\\") if not value.endswith("\\\\") else value
+        return (value + "/") if not value.endswith("/") else value
+
     @classmethod
     def _from_remote(cls, remote_attrs: Mapping[str, Any]) -> Self:
         return cls(**cls.get_local_attrs(cls._remote_map, remote_attrs))
@@ -122,8 +141,18 @@ def _update_remote(
             return True
         return False
 
-    def _delete_remote(self, secrets: SonarrSecrets, remotepathmapping_id: int) -> None:
-        api_delete(secrets, f"/api/v3/remotepathmapping/{remotepathmapping_id}")
+    def _delete_remote(
+        self,
+        tree: str,
+        secrets: SonarrSecrets,
+        remotepathmapping_id: int,
+        delete: bool,
+    ) -> bool:
+        self.log_delete_remote_attrs(tree=tree, remote_map=self._remote_map, delete=delete)
+        if delete:
+            api_delete(secrets, f"/api/v3/remotepathmapping/{remotepathmapping_id}")
+            return True
+        return False
 
 
 class SonarrRemotePathMappingsSettingsConfig(SonarrConfigBase):
@@ -195,11 +224,15 @@ def _update_remote(
         changed = False
         # Get required resource IDs from the remote, and create
         # data structures.
-        remote_rpm_ids: Dict[Tuple[str, str, str], int] = {
-            (rpm["host"], rpm["remotePath"], rpm["localPath"]): rpm["id"]
+        remote_rpm_ids: Dict[Tuple[str, OSAgnosticPath, OSAgnosticPath], int] = {
+            (
+                rpm["host"],
+                OSAgnosticPath(rpm["remotePath"]),
+                OSAgnosticPath(rpm["localPath"]),
+            ): rpm["id"]
             for rpm in api_get(secrets, "/api/v3/remotepathmapping")
         }
-        remote_rpms: Dict[Tuple[str, str, str], RemotePathMapping] = {
+        remote_rpms: Dict[Tuple[str, OSAgnosticPath, OSAgnosticPath], RemotePathMapping] = {
             (rpm.host, rpm.remote_path, rpm.local_path): rpm for rpm in remote.definitions
         }
         # Handle managed remote path mappings.
@@ -212,16 +245,16 @@ def _update_remote(
                 if rpm_tuple in remote_rpms:
                     logger.debug("%s: %s (exists)", rpm_tree, repr(rpm))
                 else:
-                    logger.info("%s: %s -> (created)", rpm_tree, repr(rpm))
                     rpm._create_remote(tree=rpm_tree, secrets=secrets)
                     changed = True
             # If the remote path mapping should not exist, check that it does not
             # exist in the remote, and if it does, delete it.
             elif rpm_tuple in remote_rpms:
-                logger.info("%s: %s -> (deleted)", rpm_tree, repr(rpm))
                 rpm._delete_remote(
+                    tree=rpm_tree,
                     secrets=secrets,
                     remotepathmapping_id=remote_rpm_ids[rpm_tuple],
+                    delete=True,
                 )
                 changed = True
             else:
@@ -231,26 +264,28 @@ def _update_remote(
 
     def _delete_remote(self, tree: str, secrets: SonarrSecrets, remote: Self) -> bool:
         changed = False
-        remote_rpm_ids: Dict[Tuple[str, str, str], int] = {
-            (rpm["host"], rpm["remotePath"], rpm["localPath"]): rpm["id"]
+        remote_rpm_ids: Dict[Tuple[str, OSAgnosticPath, OSAgnosticPath], int] = {
+            (
+                rpm["host"],
+                OSAgnosticPath(rpm["remotePath"]),
+                OSAgnosticPath(rpm["localPath"]),
+            ): rpm["id"]
             for rpm in api_get(secrets, "/api/v3/remotepathmapping")
         }
-        local_rpms: Dict[Tuple[str, str, str], RemotePathMapping] = {
+        local_rpms: Dict[Tuple[str, OSAgnosticPath, OSAgnosticPath], RemotePathMapping] = {
             (rpm.host, rpm.remote_path, rpm.local_path): rpm for rpm in self.definitions
         }
         i = -1
         for rpm in remote.definitions:
             rpm_tuple = (rpm.host, rpm.remote_path, rpm.local_path)
             if rpm_tuple not in local_rpms:
                 rpm_tree = f"{tree}.definitions[{i}]"
-                if self.delete_unmanaged:
-                    logger.info("%s: %s -> (deleted)", rpm_tree, repr(rpm))
-                    rpm._delete_remote(
-                        secrets=secrets,
-                        remotepathmapping_id=remote_rpm_ids[rpm_tuple],
-                    )
+                if rpm._delete_remote(
+                    tree=rpm_tree,
+                    secrets=secrets,
+                    remotepathmapping_id=remote_rpm_ids[rpm_tuple],
+                    delete=self.delete_unmanaged,
+                ):
                     changed = True
-                else:
-                    logger.debug("%s: %s (unmanaged)", rpm_tree, repr(rpm))
                 i -= 1
         return changed

buildarr_sonarr/types.py

@@ -19,13 +19,51 @@
 
 from __future__ import annotations
 
-from typing import Literal
+import re
+
+from pathlib import PurePosixPath, PureWindowsPath
+from typing import Any, Callable, Generator, Literal
 
 from pydantic import SecretStr
+from typing_extensions import Self
 
 SonarrProtocol = Literal["http", "https"]
 
 
+class OSAgnosticPath(str):
+    def is_windows(self) -> bool:
+        return bool(re.match(r"^[A-Za-z]:", self) or self.startswith("\\\\"))
+
+    def is_posix(self) -> bool:
+        return not self.is_windows()
+
+    def __add__(self, other: Any) -> OSAgnosticPath:
+        return OSAgnosticPath(super().__add__(other))
+
+    def __eq__(self, other: Any) -> bool:
+        try:
+            if self.is_windows():
+                return PureWindowsPath(self) == PureWindowsPath(other)
+            else:
+                return PurePosixPath(self) == PurePosixPath(other)
+        except TypeError:
+            return False
+
+    def __hash__(self) -> int:
+        if self.is_windows():
+            return hash(PureWindowsPath(self))
+        else:
+            return hash(PurePosixPath(self))
+
+    @classmethod
+    def __get_validators__(cls) -> Generator[Callable[[Any], Self], None, None]:
+        yield cls.validate
+
+    @classmethod
+    def validate(cls, value: Any) -> Self:
+        return cls(value)
+
+
 class SonarrApiKey(SecretStr):
     """
     Constrained secret string type for a Sonarr API key.