Skip to content

Commit

Permalink
feat: use custom IDOrName type for schemas (#545)
Browse files Browse the repository at this point in the history
Add typing information to the fields that support passing either an ID
as int, an ID as string or a Name as string. An empty value will be
marshalled to `null` to reflect the behavior of an empty interface.

After this change, we do not have to cast IDs to float64 or to string to
be able to read the values.

Since this is part of the schema package, this is not a breaking change
for our customers, but might break our tests that rely on casting the
interface to the desired type.
  • Loading branch information
jooola authored Nov 5, 2024
1 parent 9903f4d commit 1d97017
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 63 deletions.
6 changes: 2 additions & 4 deletions hcloud/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -935,10 +935,8 @@ type LoadBalancerChangeTypeOpts struct {
// ChangeType changes a Load Balancer's type.
func (c *LoadBalancerClient) ChangeType(ctx context.Context, loadBalancer *LoadBalancer, opts LoadBalancerChangeTypeOpts) (*Action, *Response, error) {
reqBody := schema.LoadBalancerActionChangeTypeRequest{}
if opts.LoadBalancerType.ID != 0 {
reqBody.LoadBalancerType = opts.LoadBalancerType.ID
} else {
reqBody.LoadBalancerType = opts.LoadBalancerType.Name
if opts.LoadBalancerType.ID != 0 || opts.LoadBalancerType.Name != "" {
reqBody.LoadBalancerType = schema.IDOrName{ID: opts.LoadBalancerType.ID, Name: opts.LoadBalancerType.Name}
}
reqBodyData, err := json.Marshal(reqBody)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions hcloud/load_balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func TestLoadBalancerCreate(t *testing.T) {
}
expectedReqBody := schema.LoadBalancerCreateRequest{
Name: "load-balancer",
LoadBalancerType: "lb1",
LoadBalancerType: schema.IDOrName{Name: "lb1"},
Algorithm: &schema.LoadBalancerCreateRequestAlgorithm{
Type: "round_robin",
},
Expand Down Expand Up @@ -813,7 +813,7 @@ func TestLoadBalancerClientChangeType(t *testing.T) {
if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil {
t.Fatal(err)
}
if id, ok := reqBody.LoadBalancerType.(float64); !ok || id != 1 {
if reqBody.LoadBalancerType.ID != 1 {
t.Errorf("unexpected Load Balancer type ID: %v", reqBody.LoadBalancerType)
}
json.NewEncoder(w).Encode(schema.LoadBalancerActionChangeTypeResponse{
Expand Down Expand Up @@ -844,7 +844,7 @@ func TestLoadBalancerClientChangeType(t *testing.T) {
if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil {
t.Fatal(err)
}
if name, ok := reqBody.LoadBalancerType.(string); !ok || name != "type" {
if reqBody.LoadBalancerType.Name != "type" {
t.Errorf("unexpected Load Balancer type name: %v", reqBody.LoadBalancerType)
}
json.NewEncoder(w).Encode(schema.LoadBalancerActionChangeTypeResponse{
Expand Down
68 changes: 68 additions & 0 deletions hcloud/schema/id_or_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package schema

import (
"bytes"
"encoding/json"
"reflect"
"strconv"
)

// IDOrName can be used in API requests where either a resource id or name can be
// specified.
type IDOrName struct {
ID int64
Name string
}

var _ json.Unmarshaler = (*IDOrName)(nil)
var _ json.Marshaler = (*IDOrName)(nil)

func (o IDOrName) MarshalJSON() ([]byte, error) {
if o.ID != 0 {
return json.Marshal(o.ID)
}
if o.Name != "" {
return json.Marshal(o.Name)
}

// We want to preserve the behavior of an empty interface{} to prevent breaking
// changes (marshaled to null when empty).
return json.Marshal(nil)
}

func (o *IDOrName) UnmarshalJSON(data []byte) error {
d := json.NewDecoder(bytes.NewBuffer(data))
// This ensures we won't lose precision on large IDs, see json.Number below
d.UseNumber()

var v any
if err := d.Decode(&v); err != nil {
return err
}

switch typed := v.(type) {
case string:
id, err := strconv.ParseInt(typed, 10, 64)
if err == nil {
o.ID = id
} else if typed != "" {
o.Name = typed
}
case json.Number:
id, err := typed.Int64()
if err != nil {
return &json.UnmarshalTypeError{
Value: string(data),
Type: reflect.TypeOf(*o),
}
}
o.ID = id
default:
return &json.UnmarshalTypeError{
Value: string(data),
Type: reflect.TypeOf(*o),
}
}

return nil
}
105 changes: 105 additions & 0 deletions hcloud/schema/id_or_name_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package schema

import (
"encoding/json"
"testing"

"github.com/stretchr/testify/require"
)

func TestIDOrNameMarshall(t *testing.T) {
t.Run("id", func(t *testing.T) {
i := IDOrName{ID: 1}

got, err := i.MarshalJSON()
require.NoError(t, err)
require.Equal(t, `1`, string(got))
})

t.Run("name", func(t *testing.T) {
i := IDOrName{Name: "name"}

got, err := i.MarshalJSON()
require.NoError(t, err)
require.Equal(t, `"name"`, string(got))
})

t.Run("id and name", func(t *testing.T) {
i := IDOrName{ID: 1, Name: "name"}

got, err := i.MarshalJSON()
require.NoError(t, err)
require.Equal(t, `1`, string(got))
})

t.Run("null", func(t *testing.T) {
i := IDOrName{}

got, err := i.MarshalJSON()
require.NoError(t, err)
require.Equal(t, `null`, string(got))
})
}

func TestIDOrNameUnMarshall(t *testing.T) {
t.Run("id", func(t *testing.T) {
i := IDOrName{}

err := i.UnmarshalJSON([]byte(`1`))
require.NoError(t, err)
require.Equal(t, IDOrName{ID: 1}, i)
})
t.Run("name", func(t *testing.T) {
i := IDOrName{}

err := i.UnmarshalJSON([]byte(`"name"`))
require.NoError(t, err)
require.Equal(t, IDOrName{Name: "name"}, i)
})
t.Run("id string", func(t *testing.T) {
i := IDOrName{}

err := i.UnmarshalJSON([]byte(`"1"`))
require.NoError(t, err)
require.Equal(t, IDOrName{ID: 1}, i)
})
t.Run("id float", func(t *testing.T) {
i := IDOrName{}

err := i.UnmarshalJSON([]byte(`1.0`))
require.EqualError(t, err, "json: cannot unmarshal 1.0 into Go value of type schema.IDOrName")
})
t.Run("null", func(t *testing.T) {
i := IDOrName{}

err := i.UnmarshalJSON([]byte(`null`))
require.EqualError(t, err, "json: cannot unmarshal null into Go value of type schema.IDOrName")
})
}

func TestIDOrName(t *testing.T) {
// Make sure the behavior does not change from the use of an interface{}.
type FakeRequest struct {
Old interface{} `json:"old"`
New IDOrName `json:"new"`
}

t.Run("null", func(t *testing.T) {
o := FakeRequest{}
body, err := json.Marshal(o)
require.NoError(t, err)
require.Equal(t, `{"old":null,"new":null}`, string(body))
})
t.Run("id", func(t *testing.T) {
o := FakeRequest{Old: int64(1), New: IDOrName{ID: 1}}
body, err := json.Marshal(o)
require.NoError(t, err)
require.Equal(t, `{"old":1,"new":1}`, string(body))
})
t.Run("name", func(t *testing.T) {
o := FakeRequest{Old: "name", New: IDOrName{Name: "name"}}
body, err := json.Marshal(o)
require.NoError(t, err)
require.Equal(t, `{"old":"name","new":"name"}`, string(body))
})
}
4 changes: 2 additions & 2 deletions hcloud/schema/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ type LoadBalancerDeleteServiceResponse struct {

type LoadBalancerCreateRequest struct {
Name string `json:"name"`
LoadBalancerType interface{} `json:"load_balancer_type"` // int or string
LoadBalancerType IDOrName `json:"load_balancer_type"`
Algorithm *LoadBalancerCreateRequestAlgorithm `json:"algorithm,omitempty"`
Location *string `json:"location,omitempty"`
NetworkZone *string `json:"network_zone,omitempty"`
Expand Down Expand Up @@ -380,7 +380,7 @@ type LoadBalancerActionDisablePublicInterfaceResponse struct {
}

type LoadBalancerActionChangeTypeRequest struct {
LoadBalancerType interface{} `json:"load_balancer_type"` // int or string
LoadBalancerType IDOrName `json:"load_balancer_type"`
}

type LoadBalancerActionChangeTypeResponse struct {
Expand Down
12 changes: 6 additions & 6 deletions hcloud/schema/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ type ServerListResponse struct {
// create a server.
type ServerCreateRequest struct {
Name string `json:"name"`
ServerType interface{} `json:"server_type"` // int or string
Image interface{} `json:"image"` // int or string
ServerType IDOrName `json:"server_type"`
Image IDOrName `json:"image"`
SSHKeys []int64 `json:"ssh_keys,omitempty"`
Location string `json:"location,omitempty"`
Datacenter string `json:"datacenter,omitempty"`
Expand Down Expand Up @@ -257,7 +257,7 @@ type ServerActionDisableRescueResponse struct {
// ServerActionRebuildRequest defines the schema for the request to
// rebuild a server.
type ServerActionRebuildRequest struct {
Image interface{} `json:"image"` // int or string
Image IDOrName `json:"image"`
}

// ServerActionRebuildResponse defines the schema of the response when
Expand All @@ -270,7 +270,7 @@ type ServerActionRebuildResponse struct {
// ServerActionAttachISORequest defines the schema for the request to
// attach an ISO to a server.
type ServerActionAttachISORequest struct {
ISO interface{} `json:"iso"` // int or string
ISO IDOrName `json:"iso"`
}

// ServerActionAttachISOResponse defines the schema of the response when
Expand Down Expand Up @@ -308,8 +308,8 @@ type ServerActionDisableBackupResponse struct {
// ServerActionChangeTypeRequest defines the schema for the request to
// change a server's type.
type ServerActionChangeTypeRequest struct {
ServerType interface{} `json:"server_type"` // int or string
UpgradeDisk bool `json:"upgrade_disk"`
ServerType IDOrName `json:"server_type"`
UpgradeDisk bool `json:"upgrade_disk"`
}

// ServerActionChangeTypeResponse defines the schema of the response when
Expand Down
2 changes: 1 addition & 1 deletion hcloud/schema/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type VolumeCreateRequest struct {
Name string `json:"name"`
Size int `json:"size"`
Server *int64 `json:"server,omitempty"`
Location interface{} `json:"location,omitempty"` // int, string, or nil
Location *IDOrName `json:"location,omitempty"`
Labels *map[string]string `json:"labels,omitempty"`
Automount *bool `json:"automount,omitempty"`
Format *string `json:"format,omitempty"`
Expand Down
11 changes: 0 additions & 11 deletions hcloud/schema_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ You can find a documentation of goverter here: https://goverter.jmattheis.de/
// goverter:extend durationFromIntSeconds
// goverter:extend intSecondsFromDuration
// goverter:extend serverFromImageCreatedFromSchema
// goverter:extend anyFromLoadBalancerType
// goverter:extend serverMetricsTimeSeriesFromSchema
// goverter:extend loadBalancerMetricsTimeSeriesFromSchema
// goverter:extend stringPtrFromLoadBalancerServiceProtocol
Expand Down Expand Up @@ -808,16 +807,6 @@ func volumePricingFromSchema(s schema.Pricing) VolumePricing {
}
}

func anyFromLoadBalancerType(t *LoadBalancerType) interface{} {
if t == nil {
return nil
}
if t.ID != 0 {
return t.ID
}
return t.Name
}

func serverMetricsTimeSeriesFromSchema(s schema.ServerTimeSeriesVals) ([]ServerMetricsValue, error) {
vals := make([]ServerMetricsValue, len(s.Values))

Expand Down
4 changes: 2 additions & 2 deletions hcloud/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1508,7 +1508,7 @@ func TestLoadBalancerCreateOptsToSchema(t *testing.T) {
},
Request: schema.LoadBalancerCreateRequest{
Name: "test",
LoadBalancerType: "lb11",
LoadBalancerType: schema.IDOrName{Name: "lb11"},
Algorithm: &schema.LoadBalancerCreateRequestAlgorithm{
Type: string(LoadBalancerAlgorithmTypeRoundRobin),
},
Expand Down Expand Up @@ -1593,7 +1593,7 @@ func TestLoadBalancerCreateOptsToSchema(t *testing.T) {
},
Request: schema.LoadBalancerCreateRequest{
Name: "test",
LoadBalancerType: "lb11",
LoadBalancerType: schema.IDOrName{Name: "lb11"},
Algorithm: &schema.LoadBalancerCreateRequestAlgorithm{
Type: string(LoadBalancerAlgorithmTypeRoundRobin),
},
Expand Down
30 changes: 10 additions & 20 deletions hcloud/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,15 +378,11 @@ func (c *ServerClient) Create(ctx context.Context, opts ServerCreateOpts) (Serve
reqBody.Name = opts.Name
reqBody.Automount = opts.Automount
reqBody.StartAfterCreate = opts.StartAfterCreate
if opts.ServerType.ID != 0 {
reqBody.ServerType = opts.ServerType.ID
} else if opts.ServerType.Name != "" {
reqBody.ServerType = opts.ServerType.Name
if opts.ServerType.ID != 0 || opts.ServerType.Name != "" {
reqBody.ServerType = schema.IDOrName{ID: opts.ServerType.ID, Name: opts.ServerType.Name}
}
if opts.Image.ID != 0 {
reqBody.Image = opts.Image.ID
} else if opts.Image.Name != "" {
reqBody.Image = opts.Image.Name
if opts.Image.ID != 0 || opts.Image.Name != "" {
reqBody.Image = schema.IDOrName{ID: opts.Image.ID, Name: opts.Image.Name}
}
if opts.Labels != nil {
reqBody.Labels = &opts.Labels
Expand Down Expand Up @@ -778,10 +774,8 @@ func (c *ServerClient) Rebuild(ctx context.Context, server *Server, opts ServerR
// RebuildWithResult rebuilds a server.
func (c *ServerClient) RebuildWithResult(ctx context.Context, server *Server, opts ServerRebuildOpts) (ServerRebuildResult, *Response, error) {
reqBody := schema.ServerActionRebuildRequest{}
if opts.Image.ID != 0 {
reqBody.Image = opts.Image.ID
} else {
reqBody.Image = opts.Image.Name
if opts.Image.ID != 0 || opts.Image.Name != "" {
reqBody.Image = schema.IDOrName{ID: opts.Image.ID, Name: opts.Image.Name}
}
reqBodyData, err := json.Marshal(reqBody)
if err != nil {
Expand Down Expand Up @@ -813,10 +807,8 @@ func (c *ServerClient) RebuildWithResult(ctx context.Context, server *Server, op
// AttachISO attaches an ISO to a server.
func (c *ServerClient) AttachISO(ctx context.Context, server *Server, iso *ISO) (*Action, *Response, error) {
reqBody := schema.ServerActionAttachISORequest{}
if iso.ID != 0 {
reqBody.ISO = iso.ID
} else {
reqBody.ISO = iso.Name
if iso.ID != 0 || iso.Name != "" {
reqBody.ISO = schema.IDOrName{ID: iso.ID, Name: iso.Name}
}
reqBodyData, err := json.Marshal(reqBody)
if err != nil {
Expand Down Expand Up @@ -899,10 +891,8 @@ func (c *ServerClient) ChangeType(ctx context.Context, server *Server, opts Serv
reqBody := schema.ServerActionChangeTypeRequest{
UpgradeDisk: opts.UpgradeDisk,
}
if opts.ServerType.ID != 0 {
reqBody.ServerType = opts.ServerType.ID
} else {
reqBody.ServerType = opts.ServerType.Name
if opts.ServerType.ID != 0 || opts.ServerType.Name != "" {
reqBody.ServerType = schema.IDOrName{ID: opts.ServerType.ID, Name: opts.ServerType.Name}
}
reqBodyData, err := json.Marshal(reqBody)
if err != nil {
Expand Down
Loading

0 comments on commit 1d97017

Please sign in to comment.