Improve xo get all objects struct parsing (#74)

* Pif, Template and storage repository data source panics when there are multiple matches (#66)
* Unify the pattern of retrieving XoObjects from the XO api (#52)
* Resolve PV driver issues with the terraform acceptance test VM template (#57)
* Test suite improvements to make it easy to run the tests against any XO deployment

Makefile

@@ -19,4 +19,4 @@ apply:
 	terraform apply
 
 testacc:
-	TF_ACC=1 go test $(TEST) -v
+	TF_ACC=1 go test $(TEST) -v -timeout 20m

client/client.go

@@ -2,11 +2,13 @@ package client
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"log"
 	"net/http"
 	"os"
+	"reflect"
 	"time"
 
 	gorillawebsocket "github.com/gorilla/websocket"
@@ -87,8 +89,15 @@ func NewClient(config Config) (*Client, error) {
 }
 
 func (c *Client) Call(ctx context.Context, method string, params, result interface{}, opt ...jsonrpc2.CallOption) error {
-	err := c.rpc.Call(ctx, method, params, &result, opt...)
-	log.Printf("[TRACE] Made rpc call `%s` with params: %v and received %+v: result with error: %v\n", method, params, result, err)
+	err := c.rpc.Call(ctx, method, params, result, opt...)
+	var callRes interface{}
+	t := reflect.TypeOf(result)
+	if t == nil || t.Kind() != reflect.Ptr {
+		callRes = result
+	} else {
+		callRes = reflect.ValueOf(result).Elem()
+	}
+	log.Printf("[TRACE] Made rpc call `%s` with params: %v and received %+v: result with error: %v\n", method, params, callRes, err)
 
 	if err != nil {
 		rpcErr, ok := err.(*jsonrpc2.Error)
@@ -109,8 +118,7 @@ func (c *Client) Call(ctx context.Context, method string, params, result interfa
 }
 
 type XoObject interface {
-	Compare(obj map[string]interface{}) bool
-	New(obj map[string]interface{}) XoObject
+	Compare(obj interface{}) bool
 }
 
 func (c *Client) GetAllObjectsOfType(obj XoObject, response interface{}) error {
@@ -139,7 +147,7 @@ func (c *Client) GetAllObjectsOfType(obj XoObject, response interface{}) error {
 		},
 	}
 	ctx, _ := context.WithTimeout(context.Background(), 100*time.Second)
-	return c.Call(ctx, "xo.getAllObjects", params, &response)
+	return c.Call(ctx, "xo.getAllObjects", params, response)
 }
 
 func (c *Client) FindFromGetAllObjects(obj XoObject) (interface{}, error) {
@@ -152,29 +160,35 @@ func (c *Client) FindFromGetAllObjects(obj XoObject) (interface{}, error) {
 	}
 
 	found := false
-	objs := make([]interface{}, 0)
+	t := reflect.TypeOf(obj)
+	value := reflect.New(t)
+	objs := reflect.MakeSlice(reflect.SliceOf(t), 0, 0)
 	for _, resObj := range objsRes.Objects {
 		v, ok := resObj.(map[string]interface{})
 		if !ok {
 			return obj, errors.New("Could not coerce interface{} into map")
 		}
+		b, err := json.Marshal(v)
 
-		if obj.Compare(v) {
+		if err != nil {
+			return objs, err
+		}
+		err = json.Unmarshal(b, value.Interface())
+		if err != nil {
+			return objs, err
+		}
+		if obj.Compare(value.Elem().Interface()) {
 			found = true
-			objs = append(objs, obj.New(v))
+			objs = reflect.Append(objs, value.Elem())
 		}
 	}
 	if !found {
-		return obj, NotFound{Query: obj}
+		return objs, NotFound{Query: obj}
 	}
 
 	log.Printf("[TRACE] Found the following objects from xo.getAllObjects: %+v\n", objs)
-	if len(objs) == 1 {
-
-		return objs[0], nil
-	}
 
-	return objs, nil
+	return objs.Interface(), nil
 }
 
 type handler struct{}

client/network.go

@@ -12,45 +12,30 @@ import (
 type Network struct {
 	Id        string `json:"id"`
 	NameLabel string `json:"name_label"`
-	Bridge    string
-	PoolId    string
+	Bridge    string `json:"bridge"`
+	PoolId    string `json:"$poolId"`
 }
 
-func (net Network) Compare(obj map[string]interface{}) bool {
-	id := obj["id"].(string)
-	nameLabel := obj["name_label"].(string)
-	poolId := obj["$poolId"].(string)
-	if net.Id == id {
+func (net Network) Compare(obj interface{}) bool {
+	otherNet := obj.(Network)
+	if net.Id == otherNet.Id {
 		return true
 	}
 
 	labelsMatch := false
-	if net.NameLabel == nameLabel {
+	if net.NameLabel == otherNet.NameLabel {
 		labelsMatch = true
 	}
 
 	if net.PoolId == "" && labelsMatch {
 		return true
-	} else if net.PoolId == poolId && labelsMatch {
+	} else if net.PoolId == otherNet.PoolId && labelsMatch {
 		return true
 	}
 
 	return false
 }
 
-func (net Network) New(obj map[string]interface{}) XoObject {
-	id := obj["id"].(string)
-	poolId := obj["$poolId"].(string)
-	nameLabel := obj["name_label"].(string)
-	bridge := obj["bridge"].(string)
-	return Network{
-		Id:        id,
-		Bridge:    bridge,
-		PoolId:    poolId,
-		NameLabel: nameLabel,
-	}
-}
-
 func (c *Client) CreateNetwork(netReq Network) (*Network, error) {
 	var id string
 	params := map[string]interface{}{
@@ -74,12 +59,13 @@ func (c *Client) GetNetwork(netReq Network) (*Network, error) {
 		return nil, err
 	}
 
-	if _, ok := obj.([]interface{}); ok {
+	nets := obj.([]Network)
+
+	if len(nets) > 1 {
 		return nil, errors.New("Your query returned more than one result. Use `pool_id` or other attributes to filter the result down to a single network")
 	}
 
-	net := obj.(Network)
-	return &net, nil
+	return &nets[0], nil
 }
 
 func (c *Client) GetNetworks() ([]Network, error) {

client/network_test.go

@@ -11,17 +11,17 @@ func TestNetworkCompare(t *testing.T) {
 	cases := []struct {
 		net    Network
 		result bool
-		obj    map[string]interface{}
+		other  Network
 	}{
 		{
 			net: Network{
 				NameLabel: nameLabel,
 			},
 			result: true,
-			obj: map[string]interface{}{
-				"id":         "355ee47d-ff4c-4924-3db2-fd86ae629676",
-				"name_label": nameLabel,
-				"$poolId":    "355ee47d-ff4c-4924-3db2-fd86ae629676",
+			other: Network{
+				Id:        "355ee47d-ff4c-4924-3db2-fd86ae629676",
+				NameLabel: nameLabel,
+				PoolId:    "355ee47d-ff4c-4924-3db2-fd86ae629676",
 			},
 		},
 		{
@@ -30,10 +30,10 @@ func TestNetworkCompare(t *testing.T) {
 				PoolId:    poolId,
 			},
 			result: false,
-			obj: map[string]interface{}{
-				"id":         "355ee47d-ff4c-4924-3db2-fd86ae629676",
-				"name_label": nameLabel,
-				"$poolId":    "355ee47d-ff4c-4924-3db2-fd86ae629676",
+			other: Network{
+				Id:        "355ee47d-ff4c-4924-3db2-fd86ae629676",
+				NameLabel: "name_label",
+				PoolId:    "355ee47d-ff4c-4924-3db2-fd86ae629676",
 			},
 		},
 		{
@@ -42,21 +42,21 @@ func TestNetworkCompare(t *testing.T) {
 				PoolId:    poolId,
 			},
 			result: true,
-			obj: map[string]interface{}{
-				"id":         "355ee47d-ff4c-4924-3db2-fd86ae629676",
-				"name_label": nameLabel,
-				"$poolId":    poolId,
+			other: Network{
+				Id:        "355ee47d-ff4c-4924-3db2-fd86ae629676",
+				NameLabel: nameLabel,
+				PoolId:    poolId,
 			},
 		},
 	}
 
 	for _, test := range cases {
 		net := test.net
-		obj := test.obj
+		other := test.other
 		result := test.result
 
-		if net.Compare(obj) != result {
-			t.Errorf("expected network `%+v` to Compare '%t' with object `%v`", net, result, obj)
+		if net.Compare(other) != result {
+			t.Errorf("expected network `%+v` to Compare '%t' with other `%v`", net, result, other)
 		}
 	}
 }
@@ -83,4 +83,12 @@ func TestGetNetwork(t *testing.T) {
 	if net.NameLabel != testNetworkName {
 		t.Errorf("expected network name_label `%s` to match `%s`", net.NameLabel, testNetworkName)
 	}
+
+	if net.Bridge == "" {
+		t.Errorf("expected network bridge to not be an empty string")
+	}
+
+	if net.PoolId == "" {
+		t.Errorf("expected network pool id to not be an empty string")
+	}
 }

client/pif.go

@@ -1,53 +1,60 @@
 package client
 
+import (
+	"errors"
+)
+
 type PIF struct {
-	Device   string
-	Host     string
-	Network  string
-	Id       string
-	Uuid     string
-	PoolId   string
-	Attached bool
-	Vlan     int
+	Device   string `json:"device"`
+	Host     string `json:"$host"`
+	Network  string `json:"$network"`
+	Id       string `json:"id"`
+	Uuid     string `json:"uuid"`
+	PoolId   string `json:"$poolId"`
+	Attached bool   `json:"attached"`
+	Vlan     int    `json:"vlan"`
 }
 
-func (p PIF) Compare(obj map[string]interface{}) bool {
-	device := obj["device"].(string)
-	vlan := int(obj["vlan"].(float64))
-	if p.Vlan == vlan && p.Device == device {
+func (p PIF) Compare(obj interface{}) bool {
+	otherPif := obj.(PIF)
+
+	hostIdExists := p.Host != ""
+	if hostIdExists && p.Host != otherPif.Host {
+		return false
+	}
+
+	if p.Vlan == otherPif.Vlan && p.Device == otherPif.Device {
 		return true
 	}
 	return false
 }
 
-func (p PIF) New(obj map[string]interface{}) XoObject {
-	id := obj["id"].(string)
-	device := obj["device"].(string)
-	attached := obj["attached"].(bool)
-	network := obj["$network"].(string)
-	uuid := obj["uuid"].(string)
-	poolId := obj["$poolId"].(string)
-	host := obj["$host"].(string)
-	vlan := int(obj["vlan"].(float64))
-	return PIF{
-		Device:   device,
-		Host:     host,
-		Network:  network,
-		Id:       id,
-		Uuid:     uuid,
-		PoolId:   poolId,
-		Attached: attached,
-		Vlan:     vlan,
+func (c *Client) GetPIFByDevice(dev string, vlan int) ([]PIF, error) {
+	obj, err := c.FindFromGetAllObjects(PIF{Device: dev, Vlan: vlan})
+
+	if err != nil {
+		return []PIF{}, err
 	}
+	pifs, ok := obj.([]PIF)
+
+	if !ok {
+		return pifs, errors.New("failed to coerce response into PIF slice")
+	}
+
+	return pifs, nil
 }
 
-func (c *Client) GetPIFByDevice(dev string, vlan int) (PIF, error) {
-	obj, err := c.FindFromGetAllObjects(PIF{Device: dev, Vlan: vlan})
-	pif := obj.(PIF)
+func (c *Client) GetPIF(pifReq PIF) (pifs []PIF, err error) {
+	obj, err := c.FindFromGetAllObjects(pifReq)
 
 	if err != nil {
-		return pif, err
+		return
+	}
+	pifs, ok := obj.([]PIF)
+
+	if !ok {
+		return pifs, errors.New("failed to coerce response into PIF slice")
 	}
 
-	return pif, nil
+	return pifs, nil
 }

client/pif_test.go

@@ -1,6 +1,8 @@
 package client
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestGetPIFByDevice(t *testing.T) {
 	c, err := NewClient(GetConfigFromEnv())
@@ -11,12 +13,14 @@ func TestGetPIFByDevice(t *testing.T) {
 
 	device := "eth0"
 	vlan_id := -1
-	pif, err := c.GetPIFByDevice(device, vlan_id)
+	pifs, err := c.GetPIFByDevice(device, vlan_id)
 
 	if err != nil {
-		t.Errorf("failed to find PIF with device: %s with error: %v", device, err)
+		t.Fatalf("failed to find PIF with device: %s with error: %v", device, err)
 	}
 
+	pif := pifs[0]
+
 	if pif.Device != device {
 		t.Errorf("PIF's device %s should have matched %s", pif.Device, device)
 	}