Merge pull request #57 from ovh/fixes

Add User-Agent into requests + various fixes

CODEOWNERS

@@ -0,0 +1,3 @@
+@amstuta
+@mxpetit
+@rbeuque74

go.sum

@@ -10,7 +10,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/tools v0.0.0-20190328211700-ab21143f2384 h1:TFlARGu6Czu1z7q93HTxcP1P+/ZFC/IKythI5RzrnRg=
 golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 gopkg.in/ini.v1 v1.57.0 h1:9unxIsFcTt4I55uWluz+UmL95q4kdJ0buvQ1ZIqVQww=
 gopkg.in/ini.v1 v1.57.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=

ovh/consumer_key_test.go

@@ -137,8 +137,8 @@ func TestCkRequestString(t *testing.T) {
 		ValidationURL: "fakeURL",
 	}
 
-	expected := fmt.Sprintf("CK: \"ck\"\nStatus: \"pending\"\nValidation URL: \"fakeURL\"\n")
-	got := fmt.Sprintf("%s", ckValidationState)
+	expected := "CK: \"ck\"\nStatus: \"pending\"\nValidation URL: \"fakeURL\"\n"
+	got := fmt.Sprint(ckValidationState)
 
 	if got != expected {
 		t.Errorf("expected %q, got %q", expected, got)

ovh/error.go

@@ -17,5 +17,9 @@ type APIError struct {
 }
 
 func (err *APIError) Error() string {
-	return fmt.Sprintf("Error %d: %q", err.Code, err.Message)
+	if err.Class == "" {
+		return fmt.Sprintf("HTTP Error %d: %q", err.Code, err.Message)
+	}
+
+	return fmt.Sprintf("HTTP Error %d: %s: %q (X-OVH-Query-Id: %s)", err.Code, err.Class, err.Message, err.QueryID)
 }

ovh/error_test.go

@@ -12,10 +12,22 @@ func TestErrorString(t *testing.T) {
 		Message: "Bad request",
 	}
 
-	expected := `Error 400: "Bad request"`
+	expected := `HTTP Error 400: "Bad request"`
 	got := fmt.Sprintf("%s", err)
 
 	if got != expected {
 		t.Errorf("expected %q, got %q", expected, got)
 	}
+
+	err.Class = "CartAlreadyExists"
+	err.Code = http.StatusConflict
+	err.Message = `the cart id "foobar" already exists`
+	err.QueryID = "EU.ext-99.foobar"
+
+	expected = `HTTP Error 409: CartAlreadyExists: "the cart id \"foobar\" already exists" (X-OVH-Query-Id: EU.ext-99.foobar)`
+	got = fmt.Sprintf("%s", err)
+
+	if got != expected {
+		t.Errorf("expected %q, got %q", expected, got)
+	}
 }

ovh/ovh.go

@@ -11,7 +11,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strconv"
-	"sync"
+	"sync/atomic"
 	"time"
 )
 
@@ -42,7 +42,7 @@ var Endpoints = map[string]string{
 
 // Errors
 var (
-	ErrAPIDown = errors.New("go-vh: the OVH API is down, it does't respond to /time anymore")
+	ErrAPIDown = errors.New("go-ovh: the OVH API is not reachable: failed to get /auth/time response")
 )
 
 // Client represents a client to call the OVH API
@@ -70,22 +70,20 @@ type Client struct {
 	// Ensures that the timeDelta function is only ran once
 	// sync.Once would consider init done, even in case of error
 	// hence a good old flag
-	timeDeltaMutex *sync.Mutex
-	timeDeltaDone  bool
-	timeDelta      time.Duration
-	Timeout        time.Duration
+	timeDelta atomic.Value
+
+	// Timeout configures the maximum duration to wait for an API requests to complete
+	Timeout time.Duration
 }
 
 // NewClient represents a new client to call the API
 func NewClient(endpoint, appKey, appSecret, consumerKey string) (*Client, error) {
 	client := Client{
-		AppKey:         appKey,
-		AppSecret:      appSecret,
-		ConsumerKey:    consumerKey,
-		Client:         &http.Client{},
-		timeDeltaMutex: &sync.Mutex{},
-		timeDeltaDone:  false,
-		Timeout:        time.Duration(DefaultTimeout),
+		AppKey:      appKey,
+		AppSecret:   appSecret,
+		ConsumerKey: consumerKey,
+		Client:      &http.Client{},
+		Timeout:     time.Duration(DefaultTimeout),
 	}
 
 	// Get and check the configuration
@@ -214,29 +212,22 @@ func (c *Client) DeleteUnAuthWithContext(ctx context.Context, url string, resTyp
 	return c.CallAPIWithContext(ctx, "DELETE", url, nil, resType, false)
 }
 
-// timeDelta returns the time  delta between the host and the remote API
+// timeDelta returns the time delta between the host and the remote API
 func (c *Client) getTimeDelta() (time.Duration, error) {
+	d, ok := c.timeDelta.Load().(time.Duration)
+	if ok {
+		return d, nil
+	}
 
-	if !c.timeDeltaDone {
-		// Ensure only one thread is updating
-		c.timeDeltaMutex.Lock()
-
-		// Ensure that the mutex will be released on return
-		defer c.timeDeltaMutex.Unlock()
-
-		// Did we wait ? Maybe no more needed
-		if !c.timeDeltaDone {
-			ovhTime, err := c.getTime()
-			if err != nil {
-				return 0, err
-			}
-
-			c.timeDelta = time.Since(*ovhTime)
-			c.timeDeltaDone = true
-		}
+	ovhTime, err := c.getTime()
+	if err != nil {
+		return 0, err
 	}
 
-	return c.timeDelta, nil
+	d = time.Since(*ovhTime)
+	c.timeDelta.Store(d)
+
+	return d, nil
 }
 
 // getTime t returns time from for a given api client endpoint
@@ -318,6 +309,8 @@ func (c *Client) NewRequest(method, path string, reqBody interface{}, needAuth b
 	// Send the request with requested timeout
 	c.Client.Timeout = c.Timeout
 
+	req.Header.Set("User-Agent", "github.com/ovh/go-ovh")
+
 	return req, nil
 }
 

ovh/ovh_test.go

@@ -11,6 +11,7 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 	"unicode"
@@ -71,7 +72,7 @@ func initMockServer(InputRequest **http.Request, status int, responseBody string
 
 	// Create client
 	client, _ := NewClient(ts.URL, MockApplicationKey, MockApplicationSecret, MockConsumerKey)
-	client.timeDeltaDone = true
+	client.timeDelta.Store(time.Duration(0))
 
 	return ts, client
 }
@@ -402,7 +403,7 @@ func TestGetResponseUnmarshalNumber(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Client.UnmarshalResponse should be able to decode the body")
 	}
-	if "1234567890" != fmt.Sprint(output["orderId"]) {
+	if fmt.Sprint(output["orderId"]) != "1234567890" {
 		t.Fatalf("Client.UnmarshalResponse should unmarshal long integer as json.Number instead of float64, stringified incorrectly")
 	}
 
@@ -522,7 +523,7 @@ func TestGetTimeDelta(t *testing.T) {
 	defer ts.Close()
 
 	// Test
-	client.timeDeltaDone = false
+	client.timeDelta = atomic.Value{}
 	delta, err := client.getTimeDelta()
 
 	if err != nil {