expose Client keys and http client, from review

ovh · Mar 18, 2016 · 7a2c34c · 7a2c34c
1 parent cb886b9
commit 7a2c34c
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 73 deletions.
diff --git a/configuration.go b/configuration.go
@@ -67,16 +67,16 @@ func (c *Client) loadConfig(endpointName string) error {
 		endpointName = getConfigValue(cfg, "default", "endpoint", "ovh-eu")
 	}
 
-	if c.appKey == "" {
-		c.appKey = getConfigValue(cfg, endpointName, "application_key", "")
+	if c.AppKey == "" {
+		c.AppKey = getConfigValue(cfg, endpointName, "application_key", "")
 	}
 
-	if c.appSecret == "" {
-		c.appSecret = getConfigValue(cfg, endpointName, "application_secret", "")
+	if c.AppSecret == "" {
+		c.AppSecret = getConfigValue(cfg, endpointName, "application_secret", "")
 	}
 
-	if c.consumerKey == "" {
-		c.consumerKey = getConfigValue(cfg, endpointName, "consumer_key", "")
+	if c.ConsumerKey == "" {
+		c.ConsumerKey = getConfigValue(cfg, endpointName, "consumer_key", "")
 	}
 
 	// Load real endpoint URL by name. If endpoint contains a '/', consider it as a URL
@@ -86,14 +86,14 @@ func (c *Client) loadConfig(endpointName string) error {
 		c.endpoint = Endpoints[endpointName]
 	}
 
-	// If we still have no valid endpoint, appKey or appSecret, return an error
+	// If we still have no valid endpoint, AppKey or AppSecret, return an error
 	if c.endpoint == Endpoint("") {
 		return fmt.Errorf("Unknown endpoint '%s'. Consider checking 'Endpoints' list of using an URL.", endpointName)
 	}
-	if c.appKey == "" {
+	if c.AppKey == "" {
 		return fmt.Errorf("Missing application key. Please check your configuration or consult the documentation to create one.")
 	}
-	if c.appSecret == "" {
+	if c.AppSecret == "" {
 		return fmt.Errorf("Missing application secret. Please check your configuration or consult the documentation to create one.")
 	}
 

diff --git a/configuration_test.go b/configuration_test.go
@@ -65,14 +65,14 @@ consumer_key=local
 	if err != nil {
 		t.Fatalf("loadConfig failed with: '%v'", err)
 	}
-	if client.appKey != "system" {
-		t.Fatalf("client.appKey should be 'system'. Got '%s'", client.appKey)
+	if client.AppKey != "system" {
+		t.Fatalf("client.AppKey should be 'system'. Got '%s'", client.AppKey)
 	}
-	if client.appSecret != "user" {
-		t.Fatalf("client.appSecret should be 'user'. Got '%s'", client.appSecret)
+	if client.AppSecret != "user" {
+		t.Fatalf("client.AppSecret should be 'user'. Got '%s'", client.AppSecret)
 	}
-	if client.consumerKey != "local" {
-		t.Fatalf("client.consumerKey should be 'local'. Got '%s'", client.consumerKey)
+	if client.ConsumerKey != "local" {
+		t.Fatalf("client.ConsumerKey should be 'local'. Got '%s'", client.ConsumerKey)
 	}
 }
 
@@ -100,14 +100,14 @@ consumer_key=user
 	if err != nil {
 		t.Fatalf("loadConfig failed with: '%v'", err)
 	}
-	if client.appKey != "user" {
-		t.Fatalf("client.appKey should be 'user'. Got '%s'", client.appKey)
+	if client.AppKey != "user" {
+		t.Fatalf("client.AppKey should be 'user'. Got '%s'", client.AppKey)
 	}
-	if client.appSecret != "user" {
-		t.Fatalf("client.appSecret should be 'user'. Got '%s'", client.appSecret)
+	if client.AppSecret != "user" {
+		t.Fatalf("client.AppSecret should be 'user'. Got '%s'", client.AppSecret)
 	}
-	if client.consumerKey != "user" {
-		t.Fatalf("client.consumerKey should be 'user'. Got '%s'", client.consumerKey)
+	if client.ConsumerKey != "user" {
+		t.Fatalf("client.ConsumerKey should be 'user'. Got '%s'", client.ConsumerKey)
 	}
 }
 
@@ -141,25 +141,25 @@ consumer_key=fail
 		t.Fatalf("loadConfig failed with: '%v'", err)
 	}
 	if client.endpoint != OvhEU {
-		t.Fatalf("client.appKey should be 'env'. Got '%s'", client.appKey)
+		t.Fatalf("client.AppKey should be 'env'. Got '%s'", client.AppKey)
 	}
-	if client.appKey != "env" {
-		t.Fatalf("client.appKey should be 'env'. Got '%s'", client.appKey)
+	if client.AppKey != "env" {
+		t.Fatalf("client.AppKey should be 'env'. Got '%s'", client.AppKey)
 	}
-	if client.appSecret != "env" {
-		t.Fatalf("client.appSecret should be 'env'. Got '%s'", client.appSecret)
+	if client.AppSecret != "env" {
+		t.Fatalf("client.AppSecret should be 'env'. Got '%s'", client.AppSecret)
 	}
-	if client.consumerKey != "env" {
-		t.Fatalf("client.consumerKey should be 'env'. Got '%s'", client.consumerKey)
+	if client.ConsumerKey != "env" {
+		t.Fatalf("client.ConsumerKey should be 'env'. Got '%s'", client.ConsumerKey)
 	}
 }
 
 func TestConfigFromArgs(t *testing.T) {
 	// Test
 	client := Client{
-		appKey:      "param",
-		appSecret:   "param",
-		consumerKey: "param",
+		AppKey:      "param",
+		AppSecret:   "param",
+		ConsumerKey: "param",
 	}
 	err := client.loadConfig("ovh-eu")
 
@@ -168,16 +168,16 @@ func TestConfigFromArgs(t *testing.T) {
 		t.Fatalf("loadConfig failed with: '%v'", err)
 	}
 	if client.endpoint != OvhEU {
-		t.Fatalf("client.appKey should be 'param'. Got '%s'", client.appKey)
+		t.Fatalf("client.AppKey should be 'param'. Got '%s'", client.AppKey)
 	}
-	if client.appKey != "param" {
-		t.Fatalf("client.appKey should be 'param'. Got '%s'", client.appKey)
+	if client.AppKey != "param" {
+		t.Fatalf("client.AppKey should be 'param'. Got '%s'", client.AppKey)
 	}
-	if client.appSecret != "param" {
-		t.Fatalf("client.appSecret should be 'param'. Got '%s'", client.appSecret)
+	if client.AppSecret != "param" {
+		t.Fatalf("client.AppSecret should be 'param'. Got '%s'", client.AppSecret)
 	}
-	if client.consumerKey != "param" {
-		t.Fatalf("client.consumerKey should be 'param'. Got '%s'", client.consumerKey)
+	if client.ConsumerKey != "param" {
+		t.Fatalf("client.ConsumerKey should be 'param'. Got '%s'", client.ConsumerKey)
 	}
 }
 
@@ -204,8 +204,8 @@ consumer_key=example.com
 	if err != nil {
 		t.Fatalf("loadConfig should not fail for endpoint 'ovh-eu'. Got '%v'", err)
 	}
-	if client.appKey != "ovh" {
-		t.Fatalf("configured value should be 'ovh' for endpoint 'ovh-eu'. Got '%s'", client.appKey)
+	if client.AppKey != "ovh" {
+		t.Fatalf("configured value should be 'ovh' for endpoint 'ovh-eu'. Got '%s'", client.AppKey)
 	}
 
 	// Test: by URL
@@ -214,8 +214,8 @@ consumer_key=example.com
 	if err != nil {
 		t.Fatalf("loadConfig should not fail for endpoint 'https://api.example.com:4242'. Got '%v'", err)
 	}
-	if client.appKey != "example.com" {
-		t.Fatalf("configured value should be 'example.com' for endpoint 'https://api.example.com:4242'. Got '%s'", client.appKey)
+	if client.AppKey != "example.com" {
+		t.Fatalf("configured value should be 'example.com' for endpoint 'https://api.example.com:4242'. Got '%s'", client.AppKey)
 	}
 
 }
@@ -224,9 +224,9 @@ func TestMissingParam(t *testing.T) {
 	// Setup
 	var err error
 	client := Client{
-		appKey:      "param",
-		appSecret:   "param",
-		consumerKey: "param",
+		AppKey:      "param",
+		AppSecret:   "param",
+		ConsumerKey: "param",
 	}
 
 	// Test
@@ -235,17 +235,17 @@ func TestMissingParam(t *testing.T) {
 		t.Fatalf("loadConfig should fail when client.endpoint is missing. Got '%s'", client.endpoint)
 	}
 
-	client.appKey = ""
+	client.AppKey = ""
 	if err = client.loadConfig("ovh-eu"); err == nil {
-		t.Fatalf("loadConfig should fail when client.appKey is missing. Got '%s'", client.appKey)
+		t.Fatalf("loadConfig should fail when client.AppKey is missing. Got '%s'", client.AppKey)
 	}
-	client.appKey = "param"
+	client.AppKey = "param"
 
-	client.appSecret = ""
+	client.AppSecret = ""
 	if err = client.loadConfig("ovh-eu"); err == nil {
-		t.Fatalf("loadConfig should fail when client.appSecret is missing. Got '%s'", client.appSecret)
+		t.Fatalf("loadConfig should fail when client.AppSecret is missing. Got '%s'", client.AppSecret)
 	}
-	client.appSecret = "param"
+	client.AppSecret = "param"
 }
 
 //

diff --git a/consumer_key.go b/consumer_key.go
@@ -65,7 +65,7 @@ func (ck *CkRequest) Do() (*CkValidationState, error) {
 	err := ck.client.PostUnAuth("/auth/credential", ck, &state)
 
 	if err == nil {
-		ck.client.consumerKey = state.ConsumerKey
+		ck.client.ConsumerKey = state.ConsumerKey
 	}
 
 	return &state, err

diff --git a/consumer_key_test.go b/consumer_key_test.go
@@ -16,10 +16,10 @@ func TestNewCkReqest(t *testing.T) {
 	var InputRequestBody string
 	ts, client := initMockServer(&InputRequest, 200, `{
 		"validationUrl":"https://validation.url",
-		"consumerKey":"`+MockConsumerKey+`",
+		"ConsumerKey":"`+MockConsumerKey+`",
 		"state":"pendingValidation"
 	}`, &InputRequestBody)
-	client.consumerKey = ""
+	client.ConsumerKey = ""
 	defer ts.Close()
 
 	// Test
@@ -33,8 +33,8 @@ func TestNewCkReqest(t *testing.T) {
 	if err != nil {
 		t.Fatalf("CkRequest.Do() should not return an error. Got: %q", err)
 	}
-	if client.consumerKey != MockConsumerKey {
-		t.Fatalf("CkRequest.Do() should set client.consumerKey to %s. Got %s", MockConsumerKey, client.consumerKey)
+	if client.ConsumerKey != MockConsumerKey {
+		t.Fatalf("CkRequest.Do() should set client.ConsumerKey to %s. Got %s", MockConsumerKey, client.ConsumerKey)
 	}
 	if got.ConsumerKey != MockConsumerKey {
 		t.Fatalf("CkRequest.Do() should set CkValidationState.ConsumerKey to %s. Got %s", MockConsumerKey, got.ConsumerKey)
@@ -54,7 +54,7 @@ func TestInvalidCkReqest(t *testing.T) {
 	var InputRequest *http.Request
 	var InputRequestBody string
 	ts, client := initMockServer(&InputRequest, http.StatusForbidden, `{"message":"Invalid application key"}`, &InputRequestBody)
-	client.consumerKey = ""
+	client.ConsumerKey = ""
 	defer ts.Close()
 
 	// Test

diff --git a/govh.go b/govh.go
@@ -51,17 +51,20 @@ var (
 type Client struct {
 	// Self generated tokens. Create one by visiting
 	// https://eu.api.ovh.com/createApp/
-	appKey    string
-	appSecret string
+	// AppKey holds the Application key
+	AppKey string
 
-	// Token that must me validated
-	consumerKey string
+	// AppSecret holds the Application secret key
+	AppSecret string
+
+	// ConsumerKey holds the user/app specific token. It must have been validated before use.
+	ConsumerKey string
 
 	// API endpoint
 	endpoint Endpoint
 
-	// Request client
-	client *http.Client
+	// Client is the underlying HTTP client used to run the requests. It may be overloaded but a default one is instanciated in ``NewClient`` by default.
+	Client *http.Client
 
 	// Ensures that the timeDelta function is only ran once
 	// sync.Once would consider init done, even in case of error
@@ -75,10 +78,10 @@ type Client struct {
 // 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{},
+		AppKey:         appKey,
+		AppSecret:      appSecret,
+		ConsumerKey:    consumerKey,
+		Client:         &http.Client{},
 		timeDeltaMutex: &sync.Mutex{},
 		timeDeltaDone:  false,
 		Timeout:        time.Duration(DefaultTimeout),
@@ -289,7 +292,7 @@ func (c *Client) CallAPI(method, path string, reqBody, resType interface{}, need
 	if body != nil {
 		req.Header.Add("Content-Type", "application/json;charset=utf-8")
 	}
-	req.Header.Add("X-Ovh-Application", c.appKey)
+	req.Header.Add("X-Ovh-Application", c.AppKey)
 	req.Header.Add("Accept", "application/json")
 
 	// Inject signature. Some methods do not need authentication, especially /time,
@@ -303,12 +306,12 @@ func (c *Client) CallAPI(method, path string, reqBody, resType interface{}, need
 		timestamp := getLocalTime().Add(timeDelta).Unix()
 
 		req.Header.Add("X-Ovh-Timestamp", strconv.FormatInt(timestamp, 10))
-		req.Header.Add("X-Ovh-Consumer", c.consumerKey)
+		req.Header.Add("X-Ovh-Consumer", c.ConsumerKey)
 
 		h := sha1.New()
 		h.Write([]byte(fmt.Sprintf("%s+%s+%s+%s%s+%s+%d",
-			c.appSecret,
-			c.consumerKey,
+			c.AppSecret,
+			c.ConsumerKey,
 			method,
 			getEndpointForSignature(c),
 			path,
@@ -319,8 +322,8 @@ func (c *Client) CallAPI(method, path string, reqBody, resType interface{}, need
 	}
 
 	// Send the request with requested timeout
-	c.client.Timeout = c.Timeout
-	response, err := c.client.Do(req)
+	c.Client.Timeout = c.Timeout
+	response, err := c.Client.Do(req)
 
 	if err != nil {
 		return err