refactor: one task-response parser, and the dead code #154 names (part A) (#207)

* refactor: one task-response parser, and the dead code #154 names (part A)

Unifies the 8 byte-similar private ParseTask methods in BackupService,
ContainerService, NetworkService, NodeService, SnapshotService,
StorageService, TemplateService, and VmService into one shared
PveTaskResponse.Parse(json, node) utility, on the variant that stamps
Status = "running" for a bare-UPID response. BackupService, NodeService,
TemplateService, and VmService previously left Status null for that case;
their non--Wait task output now reports "running" like the other four
services already did.

Removes the duplicate ClusterConfigService.GetClusterStatus in favor of
ClusterService's; ClusterConfigService now holds a ClusterService built
from the same injected/default client, and WaitForQuorum and
Get-PveClusterStatus go through it.

Removes dead code named in issue #154 and its fold-in comments: the
never-called PveHttpClient/IPveHttpClient sync wrappers Put/Delete, the
never-thrown PveAuthenticationException, a #pragma around an
already-nullable field, the unreferenced TestHelper mock-handler helpers,
three hand-rolled version-warning blocks (now PveCmdletBase.WarnIfBelowVersion),
an unreachable catch(HttpRequestException) arm in WaitForStatusTransition,
dead ExitStatus-checking branches in ImportPveOvaCmdlet after WaitForTask
(which already throws on failure), and an unreachable int branch in
ApiValueHelper.IsExited.

Fixes the WaitForStatusTransition catch removal's premise: PveHttpClient
read the response body outside its HttpRequestException try block, so a
mid-body stream drop could still escape unwrapped. Moves the body read
inside the try so every HttpRequestException the client can throw becomes
a PveApiException, matching what the removed catch assumed.

Part of #154.

* Add service files: BackupService, ClusterConfigService, ContainerService, NetworkService

* Add service files: NodeService, SnapshotService, StorageService, TemplateService

* Add VmService and Utilities files

* Remove unused PveAuthenticationException (never thrown, caught, or tested)

* Add cmdlet files: GetPveClusterStatus, SDN subnets, PveCmdletBase, SendPveFile, ImportPveOva

* Add test files: BackupServiceTests, ClusterConfigServiceTests, NodeServiceTests

* Add remaining test files: TemplateServiceTests, VmServiceTests, TestHelper, ApiValueHelperTests, PveTaskResponseTests

* Add VmServiceTests

---------

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
This commit is contained in:
goodolclint-claude[bot]
2026-09-03 00:59:51 +00:00
committed by GitHub
parent 109dca657a
commit 089bb7c81e
28 changed files with 188 additions and 333 deletions
@@ -50,6 +50,7 @@ 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.PostAsync(
$"nodes/{Node}/vzdump",
It.Is<Dictionary<string, string>>(d => d["vmid"] == "100" && d["storage"] == "local")),
@@ -318,34 +318,6 @@ namespace PSProxmoxVE.Core.Tests.Services
Times.Once);
}
[Fact]
public void GetClusterStatus_ReturnsStatusArray()
{
// Arrange
var json = @"{""data"": [
{""type"": ""cluster"", ""name"": ""pve-cluster"", ""nodes"": 3, ""quorate"": 1, ""version"": 5},
{""type"": ""node"", ""name"": ""pve1"", ""online"": 1, ""local"": 1, ""nodeid"": 1, ""ip"": ""10.0.0.1""}
]}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("cluster/status")).ReturnsAsync(json);
var service = new ClusterConfigService(mockClient.Object);
// Act
var statuses = service.GetClusterStatus(CreateSession());
// Assert
Assert.Equal(2, statuses.Length);
Assert.Equal("cluster", statuses[0].Type);
Assert.Equal("pve-cluster", statuses[0].Name);
Assert.Equal(3, statuses[0].Nodes);
Assert.Equal(1, statuses[0].Quorate);
Assert.Equal("node", statuses[1].Type);
Assert.Equal("pve1", statuses[1].Name);
Assert.Equal(1, statuses[1].Online);
Assert.Equal("10.0.0.1", statuses[1].Ip);
mockClient.Verify(c => c.GetAsync("cluster/status"), Times.Once);
}
[Fact]
public void GetNextId_ReturnsInt()
{
@@ -452,17 +424,6 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Throws<ArgumentNullException>(() => service.GetNextId(null!));
}
[Fact]
public void GetClusterStatus_NullSession_ThrowsArgumentNullException()
{
// Arrange
var mockClient = new Mock<IPveHttpClient>();
var service = new ClusterConfigService(mockClient.Object);
// Act & Assert
Assert.Throws<ArgumentNullException>(() => service.GetClusterStatus(null!));
}
[Fact]
public void Constructor_NullClient_ThrowsArgumentNullException()
{
@@ -250,6 +250,7 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.NotNull(task);
Assert.Contains("startall", task.Upid);
Assert.Equal("pve1", task.Node);
Assert.Equal("running", task.Status);
mockClient.Verify(c => c.PostAsync(
"nodes/pve1/startall",
It.IsAny<Dictionary<string, string>>()),
@@ -275,6 +276,7 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.NotNull(task);
Assert.Contains("stopall", task.Upid);
Assert.Equal("pve1", task.Node);
Assert.Equal("running", task.Status);
mockClient.Verify(c => c.PostAsync(
"nodes/pve1/stopall",
It.IsAny<Dictionary<string, string>>()),
@@ -40,6 +40,7 @@ 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.PostAsync(
$"nodes/{Node}/qemu/{VmId}/template",
It.IsAny<Dictionary<string, string>>()),
@@ -130,6 +130,7 @@ namespace PSProxmoxVE.Core.Tests.Services
// config lock; the native endpoint keeps the whole restart server-side.
Assert.Equal($"nodes/{TestNode}/qemu/{TestVmId}/status/reboot", resource);
Assert.Contains("qmreboot", task.Upid);
Assert.Equal("running", task.Status);
}
[Fact]
@@ -1,10 +1,4 @@
using System.IO;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using Moq;
using Moq.Protected;
namespace PSProxmoxVE.Core.Tests
{
@@ -15,38 +9,5 @@ namespace PSProxmoxVE.Core.Tests
var path = Path.Combine("Fixtures", filename);
return File.ReadAllText(path);
}
public static Mock<HttpMessageHandler> CreateMockHandler(string responseBody, HttpStatusCode statusCode = HttpStatusCode.OK)
{
var mock = new Mock<HttpMessageHandler>();
mock.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync",
ItExpr.IsAny<HttpRequestMessage>(),
ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(new HttpResponseMessage
{
StatusCode = statusCode,
Content = new StringContent(responseBody)
});
return mock;
}
public static Mock<HttpMessageHandler> CreateMockHandlerSequence(params (string body, HttpStatusCode status)[] responses)
{
var mock = new Mock<HttpMessageHandler>();
var setup = mock.Protected()
.SetupSequence<Task<HttpResponseMessage>>("SendAsync",
ItExpr.IsAny<HttpRequestMessage>(),
ItExpr.IsAny<CancellationToken>());
foreach (var (body, status) in responses)
{
setup = setup.ReturnsAsync(new HttpResponseMessage
{
StatusCode = status,
Content = new StringContent(body)
});
}
return mock;
}
}
}
@@ -9,7 +9,6 @@ namespace PSProxmoxVE.Core.Tests.Utilities
[Theory]
[InlineData(true)]
[InlineData(1L)]
[InlineData(1)]
[InlineData("1")]
public void IsExited_TrueValues_ReturnsTrue(object value)
{
@@ -19,12 +18,12 @@ namespace PSProxmoxVE.Core.Tests.Utilities
[Theory]
[InlineData(false)]
[InlineData(0L)]
[InlineData(0)]
[InlineData("0")]
[InlineData(null)]
[InlineData("")]
[InlineData("true")]
[InlineData(2L)]
[InlineData(1)]
[InlineData(42)]
public void IsExited_FalseValues_ReturnsFalse(object? value)
{
@@ -0,0 +1,45 @@
using PSProxmoxVE.Core.Utilities;
using Xunit;
namespace PSProxmoxVE.Core.Tests.Utilities
{
public class PveTaskResponseTests
{
[Fact]
public void Parse_UpidString_ReturnsRunningTaskWithNode()
{
var task = PveTaskResponse.Parse(
"{\"data\":\"UPID:pve1:00001234:00005678:AABBCCDD:qmstart:100:root@pam:\"}",
"pve1");
Assert.Equal("UPID:pve1:00001234:00005678:AABBCCDD:qmstart:100:root@pam:", task.Upid);
Assert.Equal("pve1", task.Node);
Assert.Equal("running", task.Status);
}
[Fact]
public void Parse_TaskObject_DeserializesAndStampsNode()
{
var task = PveTaskResponse.Parse(
"{\"data\":{\"upid\":\"UPID:pve2:00000001:00000002:00000003:vzdump::root@pam:\",\"status\":\"stopped\",\"exitstatus\":\"OK\",\"node\":\"ignored\"}}",
"pve2");
Assert.Equal("UPID:pve2:00000001:00000002:00000003:vzdump::root@pam:", task.Upid);
Assert.Equal("stopped", task.Status);
Assert.Equal("OK", task.ExitStatus);
Assert.Equal("pve2", task.Node);
}
[Theory]
[InlineData("{\"data\":null}")]
[InlineData("{}")]
public void Parse_NullOrAbsentData_ReturnsEmptyTaskWithNode(string json)
{
var task = PveTaskResponse.Parse(json, "pve3");
Assert.Equal(string.Empty, task.Upid);
Assert.Null(task.Status);
Assert.Equal("pve3", task.Node);
}
}
}