Merge lp:~jtv/gwacl/compose-url into lp:gwacl

[Jeroen T. Vermeulen]

Make composeURL a method of x509Session, and clean up.

Making composeURL a method helps clarify its name (it composes a URL relative to the session) but also simplifies calls, because they no longer need to extract the session's subscription ID.

Those cleanups I mentioned were:
 * Unit tests. There wasn't time for them earlier.
 * Replaced the somewhat fuzzy "URL" and "URI" nomenclature with "path" for relative paths.
 * Did away with some upper-casing in variable/parameter names that we've been told was undesirable.
 * Checked against absolute paths, which composeURL wouldn't handle properly.

Finally, a few tests used absolute paths. So those now ran into that check I added, and I had to fix them up.

Jeroen

[Raphaël Badin (rvb)]

review: Approve

[lp:~jtv/gwacl/compose-url]

74. By Jeroen T. Vermeulen on 2013-03-29

Merge trunk, resolve expected semantic conflict.

=== modified file 'managementapi_test.go'
--- managementapi_test.go	2013-03-29 12:22:47 +0000
+++ managementapi_test.go	2013-03-29 12:28:20 +0000
@@ -227,7 +227,7 @@
         secondaryKey = "secondarykey"
     )
     api := suite.makeAPI(c)
-    url := composeURL("services/storageservices/"+accountName, api.session.subscriptionId)
+    url := api.session.composeURL("services/storageservices/"+accountName)
     body := fmt.Sprintf(
         `<StorageService>
             <Url>%s</Url>

=== modified file 'x509session.go'
--- x509session.go	2013-03-27 17:08:58 +0000
+++ x509session.go	2013-03-29 12:28:20 +0000
@@ -4,7 +4,9 @@
 package gwacl
 
 import (
+    "fmt"
     "net/http"
+    "strings"
 )
 
 type x509Session struct {
@@ -20,9 +22,14 @@
 
 var AZURE_URL = "https://management.core.windows.net/"
 
-// TODO: Test
-func composeURL(URI, subscriptionId string) string {
-    return AZURE_URL + subscriptionId + "/" + URI
+// composeURL puts together a URL for an item on the Azure API based on
+// the starting point used by the session, and a given relative path from
+// there.
+func (session *x509Session) composeURL(path string) string {
+    if strings.HasPrefix(path, "/") {
+        panic(fmt.Errorf("got absolute API path '%s' instead of relative one", path))
+    }
+    return AZURE_URL + session.subscriptionId + "/" + path
 }
 
 // _X509Dispatcher is the function used to dispatch requests.  We call the
@@ -43,9 +50,8 @@
 // It returns the response body and/or an error.  If the error is a
 // ServerError, the returned body will be the one received from the server.
 // For any other kind of error, the returned body will be nil.
-func (session *x509Session) get(url string) (*x509Response, error) {
-    fullUrl := composeURL(url, session.subscriptionId)
-    request := newX509RequestGET(fullUrl)
+func (session *x509Session) get(path string) (*x509Response, error) {
+    request := newX509RequestGET(session.composeURL(path))
     response, err := _X509Dispatcher(session, request)
     if err != nil {
         return nil, err
@@ -58,9 +64,8 @@
 // It returns the response body and/or an error.  If the error is a
 // ServerError, the returned body will be the one received from the server.
 // For any other kind of error, the returned body will be nil.
-func (session *x509Session) post(url string, body []byte, contentType string) (*x509Response, error) {
-    fullUrl := composeURL(url, session.subscriptionId)
-    request := newX509RequestPOST(fullUrl, body, contentType)
+func (session *x509Session) post(path string, body []byte, contentType string) (*x509Response, error) {
+    request := newX509RequestPOST(session.composeURL(path), body, contentType)
     response, err := _X509Dispatcher(session, request)
     if err != nil {
         return nil, err
@@ -70,9 +75,8 @@
 }
 
 // delete performs a DELETE request to the Azure management API.
-func (session *x509Session) delete(url string) error {
-    fullUrl := composeURL(url, session.subscriptionId)
-    request := newX509RequestDELETE(fullUrl)
+func (session *x509Session) delete(path string) error {
+    request := newX509RequestDELETE(session.composeURL(path))
     response, err := _X509Dispatcher(session, request)
     if err != nil {
         return err
@@ -81,9 +85,8 @@
 }
 
 // put performs a PUT request to the Azure management API.
-func (session *x509Session) put(url string, body []byte) (*x509Response, error) {
-    fullUrl := composeURL(url, session.subscriptionId)
-    request := newX509RequestPUT(fullUrl, body)
+func (session *x509Session) put(path string, body []byte) (*x509Response, error) {
+    request := newX509RequestPUT(session.composeURL(path), body)
     response, err := _X509Dispatcher(session, request)
     if err != nil {
         return nil, err

=== modified file 'x509session_test.go'
--- x509session_test.go	2013-03-27 17:08:58 +0000
+++ x509session_test.go	2013-03-29 12:28:20 +0000
@@ -109,6 +109,30 @@
     c.Assert(err, IsNil)
 }
 
+func (suite *x509SessionSuite) TestComposeURLComposesURLWithRelativePath(c *C) {
+    const subscriptionID = "subscriptionid"
+    const path = "foo/bar"
+    session, err := newX509Session(subscriptionID, "cert.pem")
+    c.Assert(err, IsNil)
+
+    url := session.composeURL(path)
+
+    c.Check(url, Matches, AZURE_URL+subscriptionID+"/"+path)
+}
+
+func (suite *x509SessionSuite) TestComposeURLRejectsAbsolutePath(c *C) {
+    defer func() {
+        err := recover()
+        c.Assert(err, NotNil)
+        c.Check(err, ErrorMatches, ".*absolute.*path.*")
+    }()
+    session, err := newX509Session("subscriptionid", "cert.pem")
+    c.Assert(err, IsNil)
+
+    // This panics because we're passing an absolute path.
+    session.composeURL("/foo")
+}
+
 func (suite *x509SessionSuite) TestGetServerErrorProducesServerError(c *C) {
     msg := "huhwhat"
     status := http.StatusNotFound
@@ -183,7 +207,7 @@
     msg := "could not dispatch request"
     rigFailingDispatcher(fmt.Errorf(msg))
 
-    body, err := session.get("/flop")
+    body, err := session.get("flop")
     c.Assert(err, NotNil)
 
     c.Check(body, IsNil)
@@ -198,7 +222,7 @@
     }
     rigFixedResponseDispatcher(&fixedResponse)
 
-    response, err := session.get("/fail")
+    response, err := session.get("fail")
     c.Assert(err, NotNil)
 
     serverError := err.(*ServerError)
@@ -239,7 +263,7 @@
     msg := "could not dispatch request"
     rigFailingDispatcher(fmt.Errorf(msg))
 
-    body, err := session.post("/flop", []byte("body"), "contentType")
+    body, err := session.post("flop", []byte("body"), "contentType")
     c.Assert(err, NotNil)
 
     c.Check(body, IsNil)
@@ -254,7 +278,7 @@
     }
     rigFixedResponseDispatcher(&fixedResponse)
 
-    reponse, err := session.post("/fail", []byte("request body"), "contentType")
+    reponse, err := session.post("fail", []byte("request body"), "contentType")
     c.Assert(err, NotNil)
 
     serverError := err.(*ServerError)