launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/gomaasapi/server-address into lp:gomaasapi.

Commit message:
Make NewAnonymousClient & NewAuthenticatedClient take an apiKey which it then uses to work out the api path part from the base URL, rather than expecting the user to pass in the full URL to the api path.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~julian-edwards/gomaasapi/server-address/+merge/157979
-- 
https://code.launchpad.net/~julian-edwards/gomaasapi/server-address/+merge/157979
Your team MAAS Maintainers is requested to review the proposed merge of lp:~julian-edwards/gomaasapi/server-address into lp:gomaasapi.

=== modified file 'client.go'
--- client.go	2013-02-27 09:29:15 +0000
+++ client.go	2013-04-10 01:52:20 +0000
@@ -193,8 +193,12 @@
 var _ OAuthSigner = anonSigner{}
 
 // NewAnonymousClient creates a client that issues anonymous requests.
-func NewAnonymousClient(BaseURL string) (*Client, error) {
+// BaseURL should refer to the root of the MAAS server path, e.g.
+// http://my.maas.server.example.com/MAAS/
+// apiVersion should contain the version of the MAAS API that you want to use.
+func NewAnonymousClient(BaseURL string, apiVersion string) (*Client, error) {
 	BaseURL = EnsureTrailingSlash(BaseURL)
+    BaseURL += "api/" + apiVersion + "/"
 	parsedBaseURL, err := url.Parse(BaseURL)
 	if err != nil {
 		return nil, err
@@ -205,8 +209,12 @@
 // NewAuthenticatedClient parses the given MAAS API key into the individual
 // OAuth tokens and creates an Client that will use these tokens to sign the
 // requests it issues.
-func NewAuthenticatedClient(BaseURL string, apiKey string) (*Client, error) {
+// BaseURL should refer to the root of the MAAS server path, e.g.
+// http://my.maas.server.example.com/MAAS/
+// apiVersion should contain the version of the MAAS API that you want to use.
+func NewAuthenticatedClient(BaseURL string, apiKey string, apiVersion string) (*Client, error) {
 	BaseURL = EnsureTrailingSlash(BaseURL)
+    BaseURL += "api/" + apiVersion + "/"
 	elements := strings.Split(apiKey, ":")
 	if len(elements) != 3 {
 		errString := "invalid API key %q; expected \"<consumer secret>:<token key>:<token secret>\""

=== modified file 'client_test.go'
--- client_test.go	2013-03-04 08:08:11 +0000
+++ client_test.go	2013-04-10 01:52:20 +0000
@@ -21,7 +21,7 @@
 	expectedResult := "expected:result"
 	server := newSingleServingServer(URI, expectedResult, http.StatusBadRequest)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 	request, err := http.NewRequest("GET", server.URL+URI, nil)
 
@@ -33,7 +33,7 @@
 }
 
 func (suite *ClientSuite) TestClientDispatchRequestReturnsNonServerError(c *C) {
-	client, err := NewAnonymousClient("/foo")
+	client, err := NewAnonymousClient("/foo", "1.0")
 	c.Assert(err, IsNil)
 	// Create a bad request that will fail to dispatch.
 	request, err := http.NewRequest("GET", "/", nil)
@@ -53,7 +53,7 @@
 	expectedResult := "expected:result"
 	server := newSingleServingServer(URI, expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAuthenticatedClient(server.URL, "the:api:key")
+	client, err := NewAuthenticatedClient(server.URL, "the:api:key", "1.0")
 	c.Assert(err, IsNil)
 	request, err := http.NewRequest("GET", server.URL+URI, nil)
 	c.Assert(err, IsNil)
@@ -73,7 +73,7 @@
 	fullURI := URI.String() + "?test=123"
 	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 
 	result, err := client.Get(URI, "", params)
@@ -89,7 +89,7 @@
 	fullURI := URI.String() + "?op=list"
 	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 
 	result, err := client.Get(URI, "list", nil)
@@ -106,7 +106,7 @@
 	params := url.Values{"test": {"123"}}
 	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Check(err, IsNil)
 
 	result, err := client.Post(URI, "list", params, nil)
@@ -148,7 +148,7 @@
 	fullURI := URI.String() + "?op=add"
 	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 	fileContent := []byte("content")
 	files := map[string][]byte{"testfile": fileContent}
@@ -169,7 +169,7 @@
 	params := url.Values{"test": {"123"}}
 	server := newSingleServingServer(URI.String(), expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 
 	result, err := client.Put(URI, params)
@@ -185,7 +185,7 @@
 	expectedResult := "expected:result"
 	server := newSingleServingServer(URI.String(), expectedResult, http.StatusOK)
 	defer server.Close()
-	client, err := NewAnonymousClient(server.URL)
+	client, err := NewAnonymousClient(server.URL, "1.0")
 	c.Assert(err, IsNil)
 
 	err = client.Delete(URI)
@@ -194,7 +194,7 @@
 }
 
 func (suite *ClientSuite) TestNewAnonymousClientEnsuresTrailingSlash(c *C) {
-	client, err := NewAnonymousClient("http://example.com/api/1.0";)
+	client, err := NewAnonymousClient("http://example.com/";, "1.0")
 	c.Check(err, IsNil)
 	expectedURL, err := url.Parse("http://example.com/api/1.0/";)
 	c.Assert(err, IsNil)
@@ -202,7 +202,7 @@
 }
 
 func (suite *ClientSuite) TestNewAuthenticatedClientEnsuresTrailingSlash(c *C) {
-	client, err := NewAuthenticatedClient("http://example.com/api/1.0";, "a:b:c")
+	client, err := NewAuthenticatedClient("http://example.com/";, "a:b:c", "1.0")
 	c.Check(err, IsNil)
 	expectedURL, err := url.Parse("http://example.com/api/1.0/";)
 	c.Assert(err, IsNil)
@@ -218,7 +218,7 @@
 	keyElements := []string{consumerKey, tokenKey, tokenSecret}
 	apiKey := strings.Join(keyElements, ":")
 
-	client, err := NewAuthenticatedClient("http://example.com/api";, apiKey)
+	client, err := NewAuthenticatedClient("http://example.com/";, apiKey, "1.0")
 
 	c.Check(err, IsNil)
 	signer := client.Signer.(*plainTextOAuthSigner)
@@ -228,7 +228,7 @@
 }
 
 func (suite *ClientSuite) TestNewAuthenticatedClientFailsIfInvalidKey(c *C) {
-	client, err := NewAuthenticatedClient("", "invalid-key")
+	client, err := NewAuthenticatedClient("", "invalid-key", "1.0")
 
 	c.Check(err, ErrorMatches, "invalid API key.*")
 	c.Check(client, IsNil)

=== modified file 'maas_test.go'
--- maas_test.go	2013-02-05 14:35:15 +0000
+++ maas_test.go	2013-04-10 01:52:20 +0000
@@ -13,7 +13,7 @@
 var _ = Suite(&MAASSuite{})
 
 func (suite *MAASSuite) TestNewMAASUsesBaseURLFromClient(c *C) {
-	baseURLString := "https://server.com:888/path/to/api";
+	baseURLString := "https://server.com:888/";
 	baseURL, _ := url.Parse(baseURLString)
 	client := Client{BaseURL: baseURL}
 	maas := NewMAAS(client)

=== modified file 'testservice.go'
--- testservice.go	2013-03-12 16:27:02 +0000
+++ testservice.go	2013-04-10 01:52:20 +0000
@@ -36,7 +36,7 @@
 // by gomaasapi.NewMAAS().
 func NewTestMAAS(version string) *TestMAASObject {
 	server := NewTestServer(version)
-	authClient, err := NewAnonymousClient(server.URL + fmt.Sprintf("/api/%s/", version))
+	authClient, err := NewAnonymousClient(server.URL, version)
 	checkError(err)
 	maas := NewMAAS(*authClient)
 	return &TestMAASObject{*maas, server}
@@ -195,7 +195,7 @@
 	})
 
 	newServer := httptest.NewServer(serveMux)
-	client, err := NewAnonymousClient(newServer.URL)
+	client, err := NewAnonymousClient(newServer.URL, "1.0")
 	checkError(err)
 	server.Server = newServer
 	server.serveMux = serveMux