refactor: route the snapshot cmdlets through SnapshotService (#126, pattern) (#196)

New-PveSnapshot, Remove-PveSnapshot and Restore-PveSnapshot built their own
PveHttpClient and parsed the response inline while SnapshotService carried
the same three requests with no callers. The cmdlets now call the service,
so the path and form each sends is asserted offline (ADR 0021).

Reconciled toward the shipped cmdlet behaviour: CreateSnapshot omits
vmstate unless it is true, and ParseTask stamps Status = "running" on a
UPID-string response.

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

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

@@ -13,6 +13,14 @@ namespace PSProxmoxVE.Core.Tests.Services
     {
         private const string Node = "pve1";
         private const int VmId = 100;
+        private const string CreateUpid = "UPID:pve1:000ABC:00000001:5F1234AB:qmsnapshot:100:root@pam:";
+
+        private sealed class CapturedPost
+        {
+            public int Calls { get; set; }
+            public string? Path { get; set; }
+            public Dictionary<string, string>? Form { get; set; }
+        }
 
         private static PveSession CreateSession()
         {
@@ -20,6 +28,22 @@ namespace PSProxmoxVE.Core.Tests.Services
                 "root@pam!testtoken=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee");
         }
 
+        private static string UpidJson(string upid) => $@"{{""data"": ""{upid}""}}";
+
+        private static CapturedPost CapturePost(Mock<IPveHttpClient> mockClient, string json)
+        {
+            var captured = new CapturedPost();
+            mockClient.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
+                .Callback<string, Dictionary<string, string>?>((path, form) =>
+                {
+                    captured.Calls++;
+                    captured.Path = path;
+                    captured.Form = form;
+                })
+                .ReturnsAsync(json);
+            return captured;
+        }
+
         [Fact]
         public void GetSnapshots_ReturnsSnapshotArray()
         {
@@ -86,65 +110,154 @@ namespace PSProxmoxVE.Core.Tests.Services
         }
 
         [Fact]
-        public void CreateSnapshot_CallsPostAsync_ReturnsUpid()
+        public void CreateSnapshot_NameOnly_SendsOnlySnapname()
         {
             // Arrange
-            const string upid = "UPID:pve1:000ABC:00000001:5F1234AB:qmsnapshot:100:root@pam:";
-            var json = $@"{{""data"": ""{upid}""}}";
             var mockClient = new Mock<IPveHttpClient>();
-            mockClient.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
-                .ReturnsAsync(json);
+            var captured = CapturePost(mockClient, UpidJson(CreateUpid));
+            var service = new SnapshotService(mockClient.Object);
 
+            // Act
+            var task = service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap");
+
+            // Assert
+            Assert.Equal(1, captured.Calls);
+            Assert.Equal($"nodes/{Node}/qemu/{VmId}/snapshot", captured.Path);
+            Assert.NotNull(captured.Form);
+            Assert.Equal("my-snap", captured.Form!["snapname"]);
+            Assert.False(captured.Form.ContainsKey("description"));
+            Assert.False(captured.Form.ContainsKey("vmstate"));
+            Assert.Single(captured.Form);
+
+            Assert.Equal(CreateUpid, task.Upid);
+            Assert.Equal(Node, task.Node);
+            Assert.Equal("running", task.Status);
+        }
+
+        [Fact]
+        public void CreateSnapshot_WithDescription_SendsDescription()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            var captured = CapturePost(mockClient, UpidJson(CreateUpid));
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap", "Test snapshot");
+
+            // Assert
+            Assert.Equal(1, captured.Calls);
+            Assert.NotNull(captured.Form);
+            Assert.Equal("my-snap", captured.Form!["snapname"]);
+            Assert.Equal("Test snapshot", captured.Form["description"]);
+            Assert.False(captured.Form.ContainsKey("vmstate"));
+            Assert.Equal(2, captured.Form.Count);
+        }
+
+        [Fact]
+        public void CreateSnapshot_WithVmState_SendsVmstateOne()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            var captured = CapturePost(mockClient, UpidJson(CreateUpid));
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap", vmstate: true);
+
+            // Assert
+            Assert.Equal(1, captured.Calls);
+            Assert.NotNull(captured.Form);
+            Assert.Equal("my-snap", captured.Form!["snapname"]);
+            Assert.Equal("1", captured.Form["vmstate"]);
+            Assert.False(captured.Form.ContainsKey("description"));
+            Assert.Equal(2, captured.Form.Count);
+        }
+
+        [Fact]
+        public void CreateSnapshot_AllFields_SendsExactForm()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            var captured = CapturePost(mockClient, UpidJson(CreateUpid));
             var service = new SnapshotService(mockClient.Object);
 
             // Act
             var task = service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap", "Test snapshot", vmstate: true);
 
             // Assert
-            Assert.Equal(upid, task.Upid);
+            Assert.Equal(1, captured.Calls);
+            Assert.Equal($"nodes/{Node}/qemu/{VmId}/snapshot", captured.Path);
+            Assert.NotNull(captured.Form);
+            Assert.Equal("my-snap", captured.Form!["snapname"]);
+            Assert.Equal("Test snapshot", captured.Form["description"]);
+            Assert.Equal("1", captured.Form["vmstate"]);
+            Assert.Equal(3, captured.Form.Count);
+
+            Assert.Equal(CreateUpid, task.Upid);
             Assert.Equal(Node, task.Node);
-            mockClient.Verify(c => c.PostAsync(
-                $"nodes/{Node}/qemu/{VmId}/snapshot",
-                It.Is<Dictionary<string, string>>(d =>
-                    d["snapname"] == "my-snap" &&
-                    d["vmstate"] == "1" &&
-                    d["description"] == "Test snapshot")),
-                Times.Once);
+            Assert.Equal("running", task.Status);
         }
 
         [Fact]
-        public void CreateSnapshot_WithoutDescription_OmitsDescriptionField()
+        public void CreateSnapshot_EscapesNodeInPath()
         {
             // Arrange
-            const string upid = "UPID:pve1:000ABC:00000001:5F1234AB:qmsnapshot:100:root@pam:";
-            var json = $@"{{""data"": ""{upid}""}}";
             var mockClient = new Mock<IPveHttpClient>();
-            mockClient.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
-                .ReturnsAsync(json);
-
+            var captured = CapturePost(mockClient, UpidJson(CreateUpid));
             var service = new SnapshotService(mockClient.Object);
 
             // Act
-            service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap");
+            service.CreateSnapshot(CreateSession(), "pve node", VmId, "my-snap");
 
             // Assert
-            mockClient.Verify(c => c.PostAsync(
-                It.IsAny<string>(),
-                It.Is<Dictionary<string, string>>(d =>
-                    !d.ContainsKey("description") &&
-                    d["vmstate"] == "0")),
-                Times.Once);
+            Assert.Equal($"nodes/pve%20node/qemu/{VmId}/snapshot", captured.Path);
         }
 
         [Fact]
-        public void RemoveSnapshot_CallsDeleteAsync_ReturnsUpid()
+        public void CreateSnapshot_NullData_ReturnsEmptyUpidWithoutStatus()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            CapturePost(mockClient, @"{""data"": null}");
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            var task = service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap");
+
+            // Assert
+            Assert.Equal(string.Empty, task.Upid);
+            Assert.Equal(Node, task.Node);
+            Assert.Null(task.Status);
+        }
+
+        [Fact]
+        public void CreateSnapshot_ObjectShapedData_ReturnsTaskFields()
+        {
+            // Arrange
+            var json = $@"{{""data"": {{""upid"": ""{CreateUpid}"", ""status"": ""stopped"", ""exitstatus"": ""OK""}}}}";
+            var mockClient = new Mock<IPveHttpClient>();
+            CapturePost(mockClient, json);
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            var task = service.CreateSnapshot(CreateSession(), Node, VmId, "my-snap");
+
+            // Assert
+            Assert.Equal(CreateUpid, task.Upid);
+            Assert.Equal(Node, task.Node);
+            Assert.Equal("stopped", task.Status);
+            Assert.Equal("OK", task.ExitStatus);
+        }
+
+        [Fact]
+        public void RemoveSnapshot_CallsDeleteAsync_ReturnsRunningTask()
         {
             // Arrange
             const string upid = "UPID:pve1:000DEF:00000002:5F1234AC:qmdelsnap:100:root@pam:";
-            var json = $@"{{""data"": ""{upid}""}}";
             var mockClient = new Mock<IPveHttpClient>();
             mockClient.Setup(c => c.DeleteAsync(It.IsAny<string>()))
-                .ReturnsAsync(json);
+                .ReturnsAsync(UpidJson(upid));
 
             var service = new SnapshotService(mockClient.Object);
 
@@ -154,18 +267,35 @@ namespace PSProxmoxVE.Core.Tests.Services
             // Assert
             Assert.Equal(upid, task.Upid);
             Assert.Equal(Node, task.Node);
+            Assert.Equal("running", task.Status);
             mockClient.Verify(c => c.DeleteAsync($"nodes/{Node}/qemu/{VmId}/snapshot/clean-install"), Times.Once);
+            mockClient.VerifyNoOtherCalls();
         }
 
         [Fact]
-        public void RollbackSnapshot_CallsPostAsync_ReturnsUpid()
+        public void RemoveSnapshot_EscapesNodeAndSnapnameInPath()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient.Setup(c => c.DeleteAsync(It.IsAny<string>()))
+                .ReturnsAsync(UpidJson("UPID:pve1:000DEF:00000002:5F1234AC:qmdelsnap:100:root@pam:"));
+
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            service.RemoveSnapshot(CreateSession(), "pve node", VmId, "snap name");
+
+            // Assert
+            mockClient.Verify(c => c.DeleteAsync($"nodes/pve%20node/qemu/{VmId}/snapshot/snap%20name"), Times.Once);
+        }
+
+        [Fact]
+        public void RollbackSnapshot_CallsPostAsync_ReturnsRunningTask()
         {
             // Arrange
             const string upid = "UPID:pve1:000GHI:00000003:5F1234AD:qmrollback:100:root@pam:";
-            var json = $@"{{""data"": ""{upid}""}}";
             var mockClient = new Mock<IPveHttpClient>();
-            mockClient.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
-                .ReturnsAsync(json);
+            var captured = CapturePost(mockClient, UpidJson(upid));
 
             var service = new SnapshotService(mockClient.Object);
 
@@ -175,10 +305,25 @@ namespace PSProxmoxVE.Core.Tests.Services
             // Assert
             Assert.Equal(upid, task.Upid);
             Assert.Equal(Node, task.Node);
-            mockClient.Verify(c => c.PostAsync(
-                $"nodes/{Node}/qemu/{VmId}/snapshot/clean-install/rollback",
-                It.IsAny<Dictionary<string, string>>()),
-                Times.Once);
+            Assert.Equal("running", task.Status);
+            Assert.Equal($"nodes/{Node}/qemu/{VmId}/snapshot/clean-install/rollback", captured.Path);
+            Assert.Null(captured.Form);
+            Assert.Equal(1, captured.Calls);
+        }
+
+        [Fact]
+        public void RollbackSnapshot_EscapesNodeAndSnapnameInPath()
+        {
+            // Arrange
+            var mockClient = new Mock<IPveHttpClient>();
+            var captured = CapturePost(mockClient, UpidJson("UPID:pve1:000GHI:00000003:5F1234AD:qmrollback:100:root@pam:"));
+            var service = new SnapshotService(mockClient.Object);
+
+            // Act
+            service.RollbackSnapshot(CreateSession(), "pve node", VmId, "snap name");
+
+            // Assert
+            Assert.Equal($"nodes/pve%20node/qemu/{VmId}/snapshot/snap%20name/rollback", captured.Path);
         }
 
         [Fact]