Fix list-response JSON tags + guard getRawValue against empty arrays#154
Open
jmsperu wants to merge 1 commit into
Open
Fix list-response JSON tags + guard getRawValue against empty arrays#154jmsperu wants to merge 1 commit into
jmsperu wants to merge 1 commit into
Conversation
Several List<X>Response structs use a json tag that does not match the key CloudStack actually returns (the generator derives it from parseSingular()), so Count parses but the slice stays nil — silent data loss. Verified against the CloudStack source object names (setObjectName): - listHypervisorCapabilities: hypervisorcapability -> hypervisorCapabilities - listGuestNetworkIpv6Prefixes: guestnetworkipv6prefixe -> guestnetworkipv6prefix - listLBHealthCheckPolicies: lbhealthcheckpolicy -> healthcheckpolicies - listLBStickinessPolicies: lbstickinesspolicy -> stickinesspolicies Fixed via explicit cases in the generator's override switch (alongside the existing metrics cases) and the regenerated tags. Also guards getRawValue() against a count-wrapped response carrying an empty data array, which previously panicked on resp[0] (index out of range); it now returns a descriptive error. Adds regression tests for both. Signed-off-by: James Peru <james@xcobean.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes several list-response robustness bugs in the same family as #135/#136 (PR #137).
1. List wrapper JSON-tag mismatches (silent data loss)
The generator derives the list wrapper tag from
parseSingular(), which does not always match the key CloudStack returns. When it does not,Countunmarshals but the slice stays nil — the caller silently gets an empty list, and theGet<X>ByIDhelper then panics onslice[0]. I verified each against the CloudStack source object names (setObjectName(...)):listHypervisorCapabilitieshypervisorcapabilityhypervisorCapabilitieslistGuestNetworkIpv6Prefixesguestnetworkipv6prefixeguestnetworkipv6prefixlistLBHealthCheckPolicieslbhealthcheckpolicyhealthcheckpolicieslistLBStickinessPolicieslbstickinesspolicystickinesspoliciesFixed via explicit cases in the generator override
switch(right where the metrics cases from #135/#136 already live) plus the regenerated tags. The innerhealthcheckpolicy/stickinesspolicytags are correct and left untouched.(I also checked the other
-ies/-essiblings —autoscalepolicy,backuprepository,oscategory,snapshotpolicy,webhookdelivery— and they match the source, so they are not touched here.)2.
getRawValue()panic on empty count-wrapped arrayIn the count-wrapped branch,
getRawValuedidreturn resp[0], nilwith no length check, so a{"count":0,"<entity>":[]}body panics with index-out-of-range. Now returns a descriptive error. Latent/defensive (only a malformed/edge-case body reaches it), not a live crash.Tests
test/ListResponseJSONTagsRegression_test.go— unmarshals each of the 4 responses under the correct key; fails on the old tags (nil slice), passes with the fix.cloudstack/GetRawValueGuard_test.go— panics without the guard (reproduces the index-out-of-range), passes with it.Verification
go build ./...clean; both regression tests pass with the fix and fail/panic when reverted.Note (follow-up, not in this PR)
The root cause is that wrapper keys are fabricated from
parseSingular()rather than sourced from the API’s declared object name. A systemic fix would source the key fromlistApis.json/ a curated override map. Happy to open a separate issue for that.