diff --git a/CHANGELOG.md b/CHANGELOG.md index 61512a5..e71cd4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.16.0 (Unreleased) + +BUG FIXES: + +* Distinguish unset vs empty zone.networks + ## 2.15.0 (August 28th, 2025) FEATURES: diff --git a/rest/_examples/networks_empty.go b/rest/_examples/networks_empty.go new file mode 100644 index 0000000..aab0ba5 --- /dev/null +++ b/rest/_examples/networks_empty.go @@ -0,0 +1,47 @@ +package main + +import ( + "encoding/json" + "fmt" + "strings" + + "gopkg.in/ns1/ns1-go.v2/rest/model/dns" +) + +func main() { + fmt.Println("NS1 SDK Empty Networks Example") + fmt.Println("------------------------------") + + // Case 1: Default behavior (nil NetworkIDs) + zone1 := dns.NewZone("example.com") + // NetworkIDs is nil by default + data1, _ := json.Marshal(zone1) + fmt.Println("Case 1 - Default (nil NetworkIDs):") + fmt.Printf(" JSON: %s\n", string(data1)) + fmt.Printf(" Contains 'networks': %v\n\n", strings.Contains(string(data1), "networks")) + + // Case 2: Empty NetworkIDs (explicit empty array) + zone2 := dns.NewZone("empty.example.com") + zone2.NetworkIDs = []int{} // Empty slice + data2, _ := json.Marshal(zone2) + fmt.Println("Case 2 - Empty NetworkIDs (explicit empty array):") + fmt.Printf(" JSON: %s\n", string(data2)) + fmt.Printf(" Contains 'networks': %v\n", strings.Contains(string(data2), "networks")) + fmt.Printf(" Has empty networks array: %v\n\n", strings.Contains(string(data2), `"networks":[]`)) + + // Case 3: Populated NetworkIDs + zone3 := dns.NewZone("populated.example.com") + zone3.NetworkIDs = []int{1, 2} + data3, _ := json.Marshal(zone3) + fmt.Println("Case 3 - Populated NetworkIDs:") + fmt.Printf(" JSON: %s\n", string(data3)) + fmt.Printf(" Contains networks: %v\n\n", strings.Contains(string(data3), `"networks":[1,2]`)) + + // Case 4: Unmarshal JSON with networks field + jsonStr := `{"zone":"example.com","networks":[3,4]}` + var zone4 dns.Zone + json.Unmarshal([]byte(jsonStr), &zone4) + fmt.Println("Case 4 - Unmarshal JSON with networks field:") + fmt.Printf(" Zone: %s\n", zone4.Zone) + fmt.Printf(" NetworkIDs: %v\n", zone4.NetworkIDs) +} diff --git a/rest/model/dns/networks_test.go b/rest/model/dns/networks_test.go new file mode 100644 index 0000000..87273a3 --- /dev/null +++ b/rest/model/dns/networks_test.go @@ -0,0 +1,138 @@ +package dns + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestZoneNetworks_MarshalJSON(t *testing.T) { + // Test Case 1: nil NetworkIDs should be omitted + z := Zone{ + Zone: "example.com", + NetworkIDs: nil, + } + + data, err := json.Marshal(z) + assert.NoError(t, err) + assert.NotContains(t, string(data), "networks") + + // Test Case 2: Empty NetworkIDs should be included as [] + z = Zone{ + Zone: "example.com", + NetworkIDs: []int{}, + } + + data, err = json.Marshal(z) + assert.NoError(t, err) + assert.Contains(t, string(data), `"networks":[]`) + + // Test Case 3: Populated NetworkIDs should be included + z = Zone{ + Zone: "example.com", + NetworkIDs: []int{1, 2, 3}, + } + + data, err = json.Marshal(z) + assert.NoError(t, err) + assert.Contains(t, string(data), `"networks":[1,2,3]`) +} + +func TestZoneNetworks_UnmarshalJSON(t *testing.T) { + // Test Case 1: JSON with networks field should populate NetworkIDs + jsonStr := `{"zone":"example.com","networks":[1,2,3]}` + var z Zone + err := json.Unmarshal([]byte(jsonStr), &z) + assert.NoError(t, err) + assert.Equal(t, []int{1, 2, 3}, z.NetworkIDs) + + // Test Case 2: JSON with empty networks array should result in empty NetworkIDs + jsonStr = `{"zone":"example.com","networks":[]}` + z = Zone{} + err = json.Unmarshal([]byte(jsonStr), &z) + assert.NoError(t, err) + assert.Equal(t, 0, len(z.NetworkIDs)) + + // Test Case 3: JSON without networks field should result in nil NetworkIDs + jsonStr = `{"zone":"example.com"}` + z = Zone{} + err = json.Unmarshal([]byte(jsonStr), &z) + assert.NoError(t, err) + assert.Nil(t, z.NetworkIDs) +} + +func TestZone_EnsureNetworksFromLegacy(t *testing.T) { + // This method is now deprecated but we still test it for compatibility + + // Test Case 1: When networks is nil and NetworkIDs has values + networkIDs := []int{1, 2, 3} + z := Zone{ + Zone: "example.com", + NetworkIDs: networkIDs, + } + // Force the internal networks field to nil + z.networks = nil + + z.EnsureNetworksFromLegacy() + // Check that the internal field is now set + assert.NotNil(t, z.networks) + assert.Equal(t, networkIDs, *z.networks) + + // Test Case 2: When networks is already set, it shouldn't change + otherNetworks := []int{4, 5, 6} + z = Zone{ + Zone: "example.com", + NetworkIDs: networkIDs, + } + z.networks = &otherNetworks + + z.EnsureNetworksFromLegacy() + assert.Equal(t, otherNetworks, *z.networks) + assert.NotEqual(t, networkIDs, *z.networks) + + // Test Case 3: When both are empty/nil + z = Zone{ + Zone: "example.com", + NetworkIDs: nil, + } + z.networks = nil + + z.EnsureNetworksFromLegacy() + assert.Nil(t, z.networks) + assert.Nil(t, z.NetworkIDs) +} + +// Integration test to verify the entire flow works as expected +func TestZoneNetworks_IntegrationFlow(t *testing.T) { + // Starting with a Zone using NetworkIDs + z := Zone{ + Zone: "example.com", + NetworkIDs: []int{1, 2, 3}, + } + + // Step 1: Marshal to JSON - should automatically populate networks field + data, err := json.Marshal(z) + assert.NoError(t, err) + assert.Contains(t, string(data), `"networks":[1,2,3]`) + + // Step 2: Change to empty networks array + z.NetworkIDs = []int{} + + // Step 3: Marshal again to verify empty array is sent + data, err = json.Marshal(z) + assert.NoError(t, err) + assert.Contains(t, string(data), `"networks":[]`) + + // Step 4: Unmarshal from JSON with networks field + jsonStr := `{"zone":"example.com","networks":[4,5]}` + err = json.Unmarshal([]byte(jsonStr), &z) + assert.NoError(t, err) + assert.Equal(t, []int{4, 5}, z.NetworkIDs) + + // Step 5: Unmarshal from JSON without networks field + jsonStr = `{"zone":"example.com"}` + err = json.Unmarshal([]byte(jsonStr), &z) + assert.NoError(t, err) + assert.Nil(t, z.NetworkIDs) +} diff --git a/rest/model/dns/zone.go b/rest/model/dns/zone.go index dea00ee..0ccade8 100644 --- a/rest/model/dns/zone.go +++ b/rest/model/dns/zone.go @@ -35,8 +35,11 @@ type Zone struct { // Networks contains the network ids the zone is available. Most zones // will be in the NSONE Global Network(which is id 0). - NetworkIDs []int `json:"networks,omitempty"` - Records []*ZoneRecord `json:"records,omitempty"` + NetworkIDs []int `json:"-"` + // networks is the internal field that distinguishes between unset (nil) + // vs explicit empty ([]) during JSON marshaling. + networks *[]int `json:"networks,omitempty"` + Records []*ZoneRecord `json:"records,omitempty"` // Primary contains info to enable slaving of the zone by third party dns servers. Primary *ZonePrimary `json:"primary,omitempty"` @@ -51,6 +54,92 @@ type Zone struct { Tags map[string]string `json:"tags,omitempty"` // Only relevant for DDI } +// Networks returns the networks field pointer value. +// Deprecated: Use NetworkIDs instead. This method will be removed in a future version. +func (z *Zone) Networks() *[]int { + return z.networks +} + +// SetNetworks sets the networks field to the provided value. +// Deprecated: Use NetworkIDs instead. This method will be removed in a future version. +func (z *Zone) SetNetworks(networks *[]int) { + z.networks = networks +} + +// MarshalJSON ensures NetworkIDs is properly serialized to JSON +func (z Zone) MarshalJSON() ([]byte, error) { + // Create a clone of the zone to avoid infinite recursion + type Alias Zone + aux := struct { + *Alias + Networks []int `json:"networks,omitempty"` + }{ + Alias: (*Alias)(&z), + } + + // Only include networks field if NetworkIDs is non-nil + if z.NetworkIDs != nil { + // For empty slices, we need a special case to ensure [] + // is included in the output despite the omitempty tag + if len(z.NetworkIDs) == 0 { + // Create a temporary struct without omitempty + type AuxWithoutOmitEmpty struct { + *Alias + Networks []int `json:"networks"` + } + return json.Marshal(&AuxWithoutOmitEmpty{ + Alias: aux.Alias, + Networks: []int{}, + }) + } + aux.Networks = z.NetworkIDs + } + + return json.Marshal(&aux) +} + +// UnmarshalJSON ensures backward compatibility by populating NetworkIDs from networks +func (z *Zone) UnmarshalJSON(data []byte) error { + type Alias Zone + aux := &struct { + *Alias + Networks *[]int `json:"networks"` + }{ + Alias: (*Alias)(z), + } + + if err := json.Unmarshal(data, aux); err != nil { + return err + } + + // Preserve presence semantics + z.networks = aux.Networks + + // Update NetworkIDs from networks field + z.ensureNetworkIDsFromJSON() + + return nil +} + +// EnsureNetworksFromLegacy ensures the networks field is populated from NetworkIDs +// Deprecated: This is now handled automatically during JSON marshaling +func (z *Zone) EnsureNetworksFromLegacy() { + if z.networks == nil && len(z.NetworkIDs) > 0 { + v := append([]int(nil), z.NetworkIDs...) + z.networks = &v + } +} + +// ensureNetworkIDsFromJSON is an internal function to ensure NetworkIDs is populated +// from the networks field after JSON unmarshaling +func (z *Zone) ensureNetworkIDsFromJSON() { + if z.networks != nil { + z.NetworkIDs = append([]int(nil), *z.networks...) + } else { + z.NetworkIDs = nil + } +} + func (z Zone) String() string { return z.Zone } @@ -176,6 +265,7 @@ func (z *Zone) LinkTo(to string) { z.Primary = nil z.DNSServers = nil z.NetworkIDs = nil + z.networks = nil // Also clear the new field z.NetworkPools = nil z.Hostmaster = "" z.Pool = "" diff --git a/rest/zone.go b/rest/zone.go index 25df260..0f14a0b 100644 --- a/rest/zone.go +++ b/rest/zone.go @@ -67,6 +67,9 @@ func (s *ZonesService) Get(zone string, records bool) (*dns.Zone, *http.Response return nil, resp, err } + // NetworkIDs is already populated from the networks field + // during JSON unmarshaling in the Zone.UnmarshalJSON method + return &z, resp, nil } @@ -74,6 +77,9 @@ func (s *ZonesService) Get(zone string, records bool) (*dns.Zone, *http.Response // // NS1 API docs: https://ns1.com/api/#zones-put func (s *ZonesService) Create(z *dns.Zone) (*http.Response, error) { + // Serialization of NetworkIDs to JSON is now handled automatically + // in the Zone.MarshalJSON method + path := fmt.Sprintf("zones/%s", z.Zone) req, err := s.client.NewRequest("PUT", path, &z) @@ -102,6 +108,9 @@ func (s *ZonesService) Create(z *dns.Zone) (*http.Response, error) { // // NS1 API docs: https://ns1.com/api/#zones-post func (s *ZonesService) Update(z *dns.Zone) (*http.Response, error) { + // Serialization of NetworkIDs to JSON is now handled automatically + // in the Zone.MarshalJSON method + path := fmt.Sprintf("zones/%s", z.Zone) req, err := s.client.NewRequest("POST", path, &z)