gsadmin/PSProxmoxVE
1
0
Fork 0
mirror of https://github.com/GoodOlClint/PSProxmoxVE.git synced 2026-09-07 12:35:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: typed models for the remaining dictionary-returning services (#157) (#229)

Browse Source 
NodeService.GetNodeConfig/GetNodeDns, ClusterConfigService.GetClusterConfig,
BackupService.GetNotBackedUp and VmService.GetGuestExecStatus returned raw
Dictionary/List<Dictionary> instead of a Pve* model, per issue #157. Each now
has a typed model under Models/{Nodes,Cluster,Backup,Vms}/ with [JsonProperty]
for documented fields and a [JsonExtensionData]-backed AdditionalProperties
catch-all, following the PveVmConfig pattern. The five consuming cmdlets and
their [OutputType] attributes are updated to match.

GetClusterConfig also fixes a latent bug: GET /cluster/config returns a JSON
array (a directory index), but the old code did `data is JObject obj ? ... :
empty dict`, which silently always returned an empty dictionary since data was
a JArray. PveClusterConfigEntry decodes the array correctly and exposes a
typed Name property (the array items' schema documents no named fields, but
the endpoint's "links" metadata gives the child-URL template as "{name}").

The guest-exec poll loop in InvokePveVmGuestExecCmdlet keeps its exact
Stopwatch + Thread.Sleep(1000) structure (ADR 0001 accepted exception); only
the type it reads from changed. A TolerantBooleanConverter was added so
PveGuestExecStatus.Exited keeps accepting PVE's boolean/integer/string forms,
matching what ApiValueHelper.IsExited already tolerated for the old
dictionary path.

Reviewed with codex-rescue, correctness-reviewer and api-compat-reviewer
before commit; both real findings above (the name/subdir key and the Exited
string-form regression) came from that pass and are mutation-tested.

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 19:16:29 +00:00
committed by GitHub
parent 9a92577794
commit 0026ef2b57
29 changed files with 652 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/PSProxmoxVE.Core.Tests/Models/BackupModelTests.cs
+28
View File
@@ -122,5 +122,33 @@ namespace PSProxmoxVE.Core.Tests.Models
Assert.Null(jobs[1].Node);
Assert.Null(jobs[1].Exclude);
}
[Fact]
public void PveBackupInfo_Deserialize_HasDocumentedFields()
{
var json = @"{""data"": [
{ ""vmid"": 100, ""name"": ""webserver"", ""type"": ""qemu"" }
]}";
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var items = data.ToObject<PveBackupInfo[]>();
Assert.NotNull(items);
Assert.Equal(100, items[0].VmId);
Assert.Equal("webserver", items[0].Name);
Assert.Equal("qemu", items[0].Type);
}
[Fact]
public void PveBackupInfo_UnmappedKey_LandsInAdditionalProperties()
{
var json = @"{""data"": [
{ ""vmid"": 200, ""name"": ""db"", ""type"": ""lxc"", ""comment"": ""prod"" }
]}";
var data = JObject.Parse(json)["data"];
var items = data!.ToObject<PveBackupInfo[]>();
Assert.NotNull(items);
Assert.Equal("prod", items![0].AdditionalProperties["comment"]);
Assert.False(items[0].AdditionalProperties.ContainsKey("vmid"));
}
}
}
tests/PSProxmoxVE.Core.Tests/Models/ClusterModelTests.cs
+27
View File
@@ -143,5 +143,32 @@ namespace PSProxmoxVE.Core.Tests.Models
Assert.Null(entries[1].Nodes);
Assert.Null(entries[1].Quorate);
}
[Fact]
public void PveClusterConfigEntry_Deserialize_HasNameFromLinksTemplate()
{
// GET /cluster/config is a directory index: the item schema documents
// no named fields, but the endpoint's "links" metadata gives the
// child-URL template as "{name}", so every entry carries a "name" key.
var json = @"{""data"": [{""name"": ""nodes""}, {""name"": ""totem""}]}";
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var entries = data.ToObject<System.Collections.Generic.List<PveClusterConfigEntry>>();
Assert.NotNull(entries);
Assert.Equal(2, entries!.Count);
Assert.Equal("nodes", entries[0].Name);
Assert.Equal("totem", entries[1].Name);
}
[Fact]
public void PveClusterConfigEntry_UnmappedKey_LandsInAdditionalProperties()
{
var json = @"{""data"": [{""name"": ""nodes"", ""extra"": ""x""}]}";
var data = JObject.Parse(json)["data"];
var entries = data!.ToObject<System.Collections.Generic.List<PveClusterConfigEntry>>();
Assert.NotNull(entries);
Assert.Equal("x", entries![0].AdditionalProperties["extra"]);
Assert.False(entries[0].AdditionalProperties.ContainsKey("name"));
}
}
}
tests/PSProxmoxVE.Core.Tests/Models/NodeModelTests.cs
+57
View File
@@ -108,5 +108,62 @@ namespace PSProxmoxVE.Core.Tests.Models
Assert.NotNull(nodes[0].LoadAverage);
Assert.Equal(3, nodes[0].LoadAverage!.Length);
}
[Fact]
public void PveNodeConfig_Deserialize_HasDocumentedFields()
{
var json = @"{""data"": {
""description"": ""Primary node"",
""wakeonlan"": ""AA:BB:CC:DD:EE:FF"",
""ballooning-target"": 80,
""startall-onboot-delay"": 30,
""digest"": ""abc123""
}}";
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var config = data.ToObject<PveNodeConfig>();
Assert.NotNull(config);
Assert.Equal("Primary node", config.Description);
Assert.Equal("AA:BB:CC:DD:EE:FF", config.WakeOnLan);
Assert.Equal(80, config.BallooningTarget);
Assert.Equal(30, config.StartAllOnbootDelay);
Assert.Equal("abc123", config.Digest);
}
[Fact]
public void PveNodeConfig_UnmappedKey_LandsInAdditionalProperties()
{
var json = @"{""data"": {""description"": ""n1"", ""acmedomain0"": ""example.com,plugin=dns""}}";
var data = JObject.Parse(json)["data"];
var config = data!.ToObject<PveNodeConfig>();
Assert.NotNull(config);
Assert.Equal("example.com,plugin=dns", config!.AdditionalProperties["acmedomain0"]);
Assert.False(config.AdditionalProperties.ContainsKey("description"));
}
[Fact]
public void PveNodeDns_Deserialize_HasDocumentedFields()
{
var json = @"{""data"": {""dns1"": ""8.8.8.8"", ""dns2"": ""8.8.4.4"", ""dns3"": ""1.1.1.1"", ""search"": ""example.com""}}";
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var dns = data.ToObject<PveNodeDns>();
Assert.NotNull(dns);
Assert.Equal("8.8.8.8", dns.Dns1);
Assert.Equal("8.8.4.4", dns.Dns2);
Assert.Equal("1.1.1.1", dns.Dns3);
Assert.Equal("example.com", dns.Search);
}
[Fact]
public void PveNodeDns_UnmappedKey_LandsInAdditionalProperties()
{
var json = @"{""data"": {""dns1"": ""8.8.8.8"", ""dns4"": ""9.9.9.9""}}";
var data = JObject.Parse(json)["data"];
var dns = data!.ToObject<PveNodeDns>();
Assert.NotNull(dns);
Assert.Equal("9.9.9.9", dns!.AdditionalProperties["dns4"]);
Assert.False(dns.AdditionalProperties.ContainsKey("dns1"));
}
}
}
tests/PSProxmoxVE.Core.Tests/Models/VmModelTests.cs
+48
View File
@@ -225,5 +225,53 @@ namespace PSProxmoxVE.Core.Tests.Models
Assert.False(config!.AdditionalProperties.ContainsKey("scsihw"));
Assert.False(config.AdditionalProperties.ContainsKey("cores"));
}
[Fact]
public void PveGuestExecStatus_Deserialize_HasDocumentedFields()
{
var json = @"{""data"": {
""exited"": true,
""exitcode"": 0,
""out-data"": ""aGVsbG8="",
""err-data"": """",
""out-truncated"": false
}}";
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var status = data.ToObject<PveGuestExecStatus>();
Assert.NotNull(status);
Assert.True(status.Exited);
Assert.Equal(0, status.ExitCode);
Assert.Equal("aGVsbG8=", status.OutData);
Assert.Equal(string.Empty, status.ErrData);
Assert.False(status.OutTruncated);
}
[Theory]
[InlineData("true", true)]
[InlineData("false", false)]
[InlineData("1", true)]
[InlineData("0", false)]
[InlineData("\"1\"", true)]
[InlineData("\"0\"", false)]
public void PveGuestExecStatus_Exited_ToleratesBooleanIntegerAndStringForms(string exitedLiteral, bool expected)
{
var json = $@"{{""data"": {{""exited"": {exitedLiteral}}}}}";
var data = JObject.Parse(json)["data"];
var status = data!.ToObject<PveGuestExecStatus>();
Assert.NotNull(status);
Assert.Equal(expected, status!.Exited);
}
[Fact]
public void PveGuestExecStatus_UnmappedKey_LandsInAdditionalProperties()
{
var json = @"{""data"": {""exited"": false, ""newfield"": ""future""}}";
var data = JObject.Parse(json)["data"];
var status = data!.ToObject<PveGuestExecStatus>();
Assert.NotNull(status);
Assert.Equal("future", status!.AdditionalProperties["newfield"]);
Assert.False(status.AdditionalProperties.ContainsKey("exited"));
}
}
}
tests/PSProxmoxVE.Core.Tests/Services/BackupServiceTests.cs
+6 -5
View File
@@ -285,12 +285,12 @@ namespace PSProxmoxVE.Core.Tests.Services
// ---------------------------------------------------------------
[Fact]
public void GetNotBackedUp_ReturnsListOfDictionaries()
public void GetNotBackedUp_ReturnsTypedEntriesWithUnknownKeyInAdditionalProperties()
{
// Arrange
var json = @"{
""data"": [
{ ""vmid"": 100, ""name"": ""webserver"", ""type"": ""qemu"" },
{ ""vmid"": 100, ""name"": ""webserver"", ""type"": ""qemu"", ""comment"": ""prod"" },
{ ""vmid"": 200, ""name"": ""database"", ""type"": ""lxc"" }
]
}";
@@ -304,10 +304,11 @@ namespace PSProxmoxVE.Core.Tests.Services
var result = service.GetNotBackedUp(CreateSession());
// Assert
Assert.IsType<List<Dictionary<string, object?>>>(result);
Assert.Equal(2, result.Count);
Assert.Equal(100L, result[0]["vmid"]);
Assert.Equal("webserver", result[0]["name"]);
Assert.Equal(100, result[0].VmId);
Assert.Equal("webserver", result[0].Name);
Assert.Equal("qemu", result[0].Type);
Assert.Equal("prod", result[0].AdditionalProperties["comment"]);
}
[Fact]
tests/PSProxmoxVE.Core.Tests/Services/ClusterConfigServiceTests.cs
+10 -6
View File
@@ -21,10 +21,13 @@ namespace PSProxmoxVE.Core.Tests.Services
}
[Fact]
public void GetClusterConfig_ReturnsJObject()
public void GetClusterConfig_ReturnsTypedEntriesWithUnknownKeyInAdditionalProperties()
{
// Arrange
var json = @"{""data"": {""nodes"": {""pve1"": {}}, ""totem"": {""version"": ""2""}}}";
// Arrange — GET /cluster/config is a directory index: an array of
// entries, not a single object. The item schema documents no named
// fields, but the endpoint's "links" metadata gives the child-URL
// template as "{name}", so every entry carries a "name" key.
var json = @"{""data"": [{""name"": ""nodes""}, {""name"": ""totem"", ""extra"": ""x""}]}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("cluster/config")).ReturnsAsync(json);
var service = new ClusterConfigService(mockClient.Object);
@@ -33,9 +36,10 @@ namespace PSProxmoxVE.Core.Tests.Services
var config = service.GetClusterConfig(CreateSession());
// Assert
Assert.NotNull(config);
Assert.NotNull(config["nodes"]);
Assert.NotNull(config["totem"]);
Assert.Equal(2, config.Count);
Assert.Equal("nodes", config[0].Name);
Assert.Equal("totem", config[1].Name);
Assert.Equal("x", config[1].AdditionalProperties["extra"]);
mockClient.Verify(c => c.GetAsync("cluster/config"), Times.Once);
}
tests/PSProxmoxVE.Core.Tests/Services/NodeServiceTests.cs
+11 -11
View File
@@ -138,10 +138,10 @@ namespace PSProxmoxVE.Core.Tests.Services
}
[Fact]
public void GetNodeConfig_ReturnsDictionary()
public void GetNodeConfig_ReturnsTypedModelWithUnknownKeyInAdditionalProperties()
{
// Arrange
var json = @"{""data"": {""description"": ""Primary node"", ""wakeonlan"": ""AA:BB:CC:DD:EE:FF""}}";
var json = @"{""data"": {""description"": ""Primary node"", ""wakeonlan"": ""AA:BB:CC:DD:EE:FF"", ""acmedomain0"": ""example.com,plugin=dns""}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes/pve1/config")).ReturnsAsync(json);
var service = new NodeService(mockClient.Object);
@@ -151,9 +151,9 @@ namespace PSProxmoxVE.Core.Tests.Services
// Assert
Assert.NotNull(config);
Assert.IsType<Dictionary<string, object?>>(config);
Assert.Equal("Primary node", config["description"]?.ToString());
Assert.Equal("AA:BB:CC:DD:EE:FF", config["wakeonlan"]?.ToString());
Assert.Equal("Primary node", config.Description);
Assert.Equal("AA:BB:CC:DD:EE:FF", config.WakeOnLan);
Assert.Equal("example.com,plugin=dns", config.AdditionalProperties["acmedomain0"]);
mockClient.Verify(c => c.GetAsync("nodes/pve1/config"), Times.Once);
}
@@ -184,10 +184,10 @@ namespace PSProxmoxVE.Core.Tests.Services
}
[Fact]
public void GetNodeDns_ReturnsDictionary()
public void GetNodeDns_ReturnsTypedModelWithUnknownKeyInAdditionalProperties()
{
// Arrange
var json = @"{""data"": {""dns1"": ""8.8.8.8"", ""dns2"": ""8.8.4.4"", ""search"": ""example.com""}}";
var json = @"{""data"": {""dns1"": ""8.8.8.8"", ""dns2"": ""8.8.4.4"", ""search"": ""example.com"", ""dns4"": ""1.1.1.1""}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes/pve1/dns")).ReturnsAsync(json);
var service = new NodeService(mockClient.Object);
@@ -197,10 +197,10 @@ namespace PSProxmoxVE.Core.Tests.Services
// Assert
Assert.NotNull(dns);
Assert.IsType<Dictionary<string, object?>>(dns);
Assert.Equal("8.8.8.8", dns["dns1"]?.ToString());
Assert.Equal("8.8.4.4", dns["dns2"]?.ToString());
Assert.Equal("example.com", dns["search"]?.ToString());
Assert.Equal("8.8.8.8", dns.Dns1);
Assert.Equal("8.8.4.4", dns.Dns2);
Assert.Equal("example.com", dns.Search);
Assert.Equal("1.1.1.1", dns.AdditionalProperties["dns4"]);
mockClient.Verify(c => c.GetAsync("nodes/pve1/dns"), Times.Once);
}
tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
+42
View File
@@ -113,6 +113,48 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Equal("args", ex.ParamName);
}
[Fact]
public void GetGuestExecStatus_ReturnsTypedModelWithUnknownKeyInAdditionalProperties()
{
var json = @"{""data"": {
""exited"": true,
""exitcode"": 0,
""out-data"": ""aGVsbG8="",
""err-data"": """",
""newfield"": ""future""
}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/agent/exec-status?pid=4242"))
.ReturnsAsync(json);
var service = new VmService(mockClient.Object);
var status = service.GetGuestExecStatus(CreateSession(), TestNode, TestVmId, 4242);
Assert.True(status.Exited);
Assert.Equal(0, status.ExitCode);
Assert.Equal("aGVsbG8=", status.OutData);
Assert.Equal(string.Empty, status.ErrData);
Assert.Equal("future", status.AdditionalProperties["newfield"]);
}
[Fact]
public void GetGuestExecStatus_ExitedAsString_StillPollsToCompletion()
{
// PVE has been observed sending "exited" as the string "1"/"0" as well
// as a JSON boolean or integer; the poll loop in InvokePveVmGuestExecCmdlet
// must not throw on this shape.
var json = @"{""data"": {""exited"": ""1"", ""exitcode"": 0}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/agent/exec-status?pid=99"))
.ReturnsAsync(json);
var service = new VmService(mockClient.Object);
var status = service.GetGuestExecStatus(CreateSession(), TestNode, TestVmId, 99);
Assert.True(status.Exited);
Assert.Equal(0, status.ExitCode);
}
[Fact]
public void RebootVm_PostsToTheNativeRebootEndpoint()
{