refactor: route the Nodes cmdlets through NodeService (#126) (#197)

Get-PveNode and Get-PveNodeStatus built their own PveHttpClient and
parsed the response inline. Both now call the existing NodeService
methods (GetNodes, GetNodeStatus), which already carried matching
requests. NodeService.GetNodeStatus gained the node-name stamp the
cmdlet used to apply after deserializing, and both GetNodes and
GetNodeStatus regained the missing-data guard the cmdlets had, which
the pre-existing service implementation lacked.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>

src/PSProxmoxVE.Core/Services/NodeService.cs

@@ -38,8 +38,9 @@ namespace PSProxmoxVE.Core.Services
             return Invoke(session, client =>
             {
                 var response = client.GetAsync("nodes").GetAwaiter().GetResult();
-                var data = JObject.Parse(response)["data"];
-                return data?.ToObject<PveNode[]>() ?? Array.Empty<PveNode>();
+                var data = JObject.Parse(response)["data"]
+                    ?? throw new InvalidOperationException("Response did not contain a 'data' field.");
+                return data.ToObject<PveNode[]>() ?? Array.Empty<PveNode>();
             });
         }
 
@@ -54,8 +55,14 @@ namespace PSProxmoxVE.Core.Services
             return Invoke(session, client =>
             {
                 var response = client.GetAsync($"nodes/{Uri.EscapeDataString(node)}/status").GetAwaiter().GetResult();
-                var data = JObject.Parse(response)["data"];
-                return data?.ToObject<PveNodeStatus>() ?? new PveNodeStatus();
+                var data = JObject.Parse(response)["data"]
+                    ?? throw new InvalidOperationException("Response did not contain a 'data' field.");
+                var status = data.ToObject<PveNodeStatus>()
+                    ?? throw new InvalidOperationException("Failed to deserialize node status.");
+                // The /nodes/{node}/status response does not include the node name.
+                if (string.IsNullOrEmpty(status.Node))
+                    status.Node = node;
+                return status;
             });
         }
 

src/PSProxmoxVE/Cmdlets/Nodes/GetPveNodeCmdlet.cs

@@ -1,10 +1,7 @@
 using System;
-using System.Collections.Generic;
 using System.Management.Automation;
-using Newtonsoft.Json;
-using Newtonsoft.Json.Linq;
-using PSProxmoxVE.Core.Client;
 using PSProxmoxVE.Core.Models.Nodes;
+using PSProxmoxVE.Core.Services;
 
 namespace PSProxmoxVE.Cmdlets.Nodes
 {
@@ -30,27 +27,8 @@ namespace PSProxmoxVE.Cmdlets.Nodes
             var session = GetSession();
 
             WriteVerbose("Getting cluster nodes...");
-            string responseBody;
-            try
-            {
-                using var client = new PveHttpClient(session);
-                responseBody = client.GetAsync("nodes").GetAwaiter().GetResult();
-            }
-            catch (Exception ex)
-            {
-                ThrowTerminatingError(new ErrorRecord(
-                    ex,
-                    "GetPveNodeFailed",
-                    ErrorCategory.ConnectionError,
-                    session.Hostname));
-                return;
-            }
-
-            var json = JObject.Parse(responseBody);
-            var dataToken = json["data"] ?? throw new InvalidOperationException("Response did not contain a 'data' field.");
-
-            var nodes = dataToken.ToObject<List<PveNode>>(
-                JsonSerializer.CreateDefault()) ?? new List<PveNode>();
+            var service = new NodeService();
+            var nodes = service.GetNodes(session);
 
             foreach (var node in nodes)
             {

src/PSProxmoxVE/Cmdlets/Nodes/GetPveNodeStatusCmdlet.cs

@@ -1,9 +1,6 @@
-using System;
 using System.Management.Automation;
-using Newtonsoft.Json;
-using Newtonsoft.Json.Linq;
-using PSProxmoxVE.Core.Client;
 using PSProxmoxVE.Core.Models.Nodes;
+using PSProxmoxVE.Core.Services;
 
 namespace PSProxmoxVE.Cmdlets.Nodes
 {
@@ -32,33 +29,8 @@ namespace PSProxmoxVE.Cmdlets.Nodes
             var session = GetSession();
 
             WriteVerbose($"Getting status for node '{Node}'...");
-            var resource = $"nodes/{Uri.EscapeDataString(Node)}/status";
-
-            string responseBody;
-            try
-            {
-                using var client = new PveHttpClient(session);
-                responseBody = client.GetAsync(resource).GetAwaiter().GetResult();
-            }
-            catch (Exception ex)
-            {
-                ThrowTerminatingError(new ErrorRecord(
-                    ex,
-                    "GetPveNodeStatusFailed",
-                    ErrorCategory.ConnectionError,
-                    Node));
-                return;
-            }
-
-            var json = JObject.Parse(responseBody);
-            var dataToken = json["data"] ?? throw new InvalidOperationException("Response did not contain a 'data' field.");
-
-            var status = dataToken.ToObject<PveNodeStatus>(JsonSerializer.CreateDefault())
-                ?? throw new InvalidOperationException("Failed to deserialize node status.");
-
-            // The /nodes/{node}/status response does not include the node name; populate it.
-            if (string.IsNullOrEmpty(status.Node))
-                status.Node = Node;
+            var service = new NodeService();
+            var status = service.GetNodeStatus(session, Node);
 
             WriteObject(status);
         }

tests/PSProxmoxVE.Core.Tests/Services/NodeServiceTests.cs

@@ -44,6 +44,18 @@ namespace PSProxmoxVE.Core.Tests.Services
             mockClient.Verify(c => c.GetAsync("nodes"), Times.Once);
         }
 
+        [Fact]
+        public void GetNodes_MissingDataField_Throws()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient.Setup(c => c.GetAsync("nodes")).ReturnsAsync("{}");
+            var service = new NodeService(mockClient.Object);
+
+            // Act & Assert
+            Assert.Throws<InvalidOperationException>(() => service.GetNodes(CreateSession()));
+        }
+
         [Fact]
         public void GetNodeStatus_ReturnsPveNodeStatus()
         {
@@ -70,6 +82,61 @@ namespace PSProxmoxVE.Core.Tests.Services
             mockClient.Verify(c => c.GetAsync("nodes/pve1/status"), Times.Once);
         }
 
+        [Fact]
+        public void GetNodeStatus_StampsNodeWhenResponseOmitsIt()
+        {
+            // Arrange
+            var json = @"{""data"": {""status"": ""online"", ""maxcpu"": 16}}";
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient.Setup(c => c.GetAsync("nodes/pve1/status")).ReturnsAsync(json);
+            var service = new NodeService(mockClient.Object);
+
+            // Act
+            var status = service.GetNodeStatus(CreateSession(), "pve1");
+
+            // Assert
+            Assert.Equal("pve1", status.Node);
+            mockClient.Verify(c => c.GetAsync("nodes/pve1/status"), Times.Once);
+        }
+
+        [Fact]
+        public void GetNodeStatus_EscapesNodeInPath()
+        {
+            // Arrange
+            var json = @"{""data"": {""node"": ""pve node"", ""status"": ""online""}}";
+            string? capturedPath = null;
+            var calls = 0;
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient.Setup(c => c.GetAsync(It.IsAny<string>()))
+                .Callback<string>(path =>
+                {
+                    capturedPath = path;
+                    calls++;
+                })
+                .ReturnsAsync(json);
+            var service = new NodeService(mockClient.Object);
+
+            // Act
+            var status = service.GetNodeStatus(CreateSession(), "pve node");
+
+            // Assert
+            Assert.Equal("nodes/pve%20node/status", capturedPath);
+            Assert.Equal(1, calls);
+            Assert.Equal("pve node", status.Node);
+        }
+
+        [Fact]
+        public void GetNodeStatus_MissingDataField_Throws()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient.Setup(c => c.GetAsync("nodes/pve1/status")).ReturnsAsync("{}");
+            var service = new NodeService(mockClient.Object);
+
+            // Act & Assert
+            Assert.Throws<InvalidOperationException>(() => service.GetNodeStatus(CreateSession(), "pve1"));
+        }
+
         [Fact]
         public void GetNodeConfig_ReturnsDictionary()
         {