Merge lp:~julian-edwards/gwacl/get-container-properties into lp:gwacl

Proposed by Julian Edwards on 2013-07-31
Status: Merged
Approved by: Julian Edwards on 2013-07-31
Approved revision: 210
Merged at revision: 206
Proposed branch: lp:~julian-edwards/gwacl/get-container-properties
Merge into: lp:gwacl
Diff against target: 284 lines (+116/-22)
2 files modified
storage_base.go (+51/-22)
storage_base_test.go (+65/-0)
To merge this branch: bzr merge lp:~julian-edwards/gwacl/get-container-properties
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) 2013-07-31 Approve on 2013-07-31
Review via email: mp+177736@code.launchpad.net

Commit message

Implement GetContainerProperties.

Description of the change

Implement GetContainerProperties - it's useful to use as a "tell me if this container exists or not" thing.

To post a comment you must log in.
Julian Edwards (julian-edwards) wrote :

The response from this call is strangely all in response headers instead of XML in the body like most other calls, so I had to re-jig performRequest a bit so the headers are returned if you provide an non-nil http.Header in the request parameters.

It's annoying to have to copy the headers like it does, and could be avoided if we choose to turn the request parameter struct into a pointer so that arbitrary contained pointers could be returned.

Julian Edwards (julian-edwards) wrote :

On 31/07/13 14:03, Julian Edwards wrote:
> The response from this call is strangely all in response headers instead of XML in the body like most other calls, so I had to re-jig performRequest a bit so the headers are returned if you provide an non-nil http.Header in the request parameters.
>
> It's annoying to have to copy the headers like it does, and could be avoided if we choose to turn the request parameter struct into a pointer so that arbitrary contained pointers could be returned.
>

I caved and updated all the call sites to take an additional headers
returned.

Jeroen T. Vermeulen (jtv) wrote :

Something weird in the diff... I see the number of return values changing for performRequest, but only one of the "return" statements changes. Do you perchance have some more diff sitting on your machine?

Julian Edwards (julian-edwards) wrote :

On 31/07/13 16:06, Jeroen T. Vermeulen wrote:
> Something weird in the diff... I see the number of return values changing for performRequest, but only one of the "return" statements changes. Do you perchance have some more diff sitting on your machine?
>

No that's it, it all works!

Jeroen T. Vermeulen (jtv) wrote :

Ah, of course it just returned the result from a send() call. Basically the language supports tuples, it's just that it won't let us use them.

Anyway, good branch. The documentation comment for GetContainerProperties is in the imperative — python-style. In Go it should be in the third person, and start with the name of the function.

TestServerError could do with one small change: the c.Check(ok, Equals, true) should be an Assert, because the next check becomes meaningless if you continue — and you'll get a panic drowning out the root cause in the output.

review: Approve
Julian Edwards (julian-edwards) wrote :

On 31/07/13 16:18, Jeroen T. Vermeulen wrote:
> Review: Approve
>
> Ah, of course it just returned the result from a send() call. Basically the language supports tuples, it's just that it won't let us use them.
>
> Anyway, good branch. The documentation comment for GetContainerProperties is in the imperative — python-style. In Go it should be in the third person, and start with the name of the function.

It's the same for all the functions like that, I kept it consistent. If
we're going to change it, we should do them all at once.

> TestServerError could do with one small change: the c.Check(ok, Equals, true) should be an Assert, because the next check becomes meaningless if you continue — and you'll get a panic drowning out the root cause in the output.

Right, thanks!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storage_base.go'
2--- storage_base.go 2013-07-30 06:49:29 +0000
3+++ storage_base.go 2013-07-31 06:34:25 +0000
4@@ -259,12 +259,12 @@
5
6 // performRequest issues an HTTP request to Azure.
7 //
8-// It returns the response body contents.
9-func (context *StorageContext) performRequest(params requestParams) ([]byte, error) {
10+// It returns the response body contents and the response headers.
11+func (context *StorageContext) performRequest(params requestParams) ([]byte, http.Header, error) {
12 params.Check()
13 req, err := http.NewRequest(params.Method, params.URL, params.Body)
14 if err != nil {
15- return nil, err
16+ return nil, nil, err
17 }
18 // net/http has no way of adding headers en-masse, hence this abomination.
19 for header, values := range params.ExtraHeaders {
20@@ -285,37 +285,37 @@
21 //
22 // If the response's HTTP status code is not the same as "expectedStatus"
23 // then an HTTPError will be returned as the error.
24-func (context *StorageContext) send(req *http.Request, res Deserializer, expectedStatus HTTPStatus) ([]byte, error) {
25+func (context *StorageContext) send(req *http.Request, res Deserializer, expectedStatus HTTPStatus) ([]byte, http.Header, error) {
26 client := context.getClient()
27 resp, err := client.Do(req)
28 if err != nil {
29- return nil, err
30+ return nil, nil, err
31 }
32
33 body, err := readAndClose(resp.Body)
34 if err != nil {
35- return nil, fmt.Errorf("failed to read response data: %v", err)
36+ return nil, nil, fmt.Errorf("failed to read response data: %v", err)
37 }
38
39 if resp.StatusCode != int(expectedStatus) {
40 msg := newHTTPError(resp.StatusCode, body, "Azure request failed")
41- return body, msg
42+ return body, resp.Header, msg
43 }
44
45 // If the caller didn't supply an object to deserialize the message into
46 // then just return.
47 if res == nil {
48- return body, nil
49+ return body, resp.Header, nil
50 }
51
52 // TODO: Also deserialize response headers into the "res" object.
53 err = res.Deserialize(body)
54 if err != nil {
55 msg := fmt.Errorf("Failed to deserialize data: %s", err)
56- return body, msg
57+ return body, resp.Header, msg
58 }
59
60- return body, nil
61+ return body, resp.Header, nil
62 }
63
64 // getAccountURL returns the base URL for the context's storage account.
65@@ -360,7 +360,7 @@
66 uri = addURLQueryParams(uri, "marker", request.Marker)
67 }
68 containers := ContainerEnumerationResults{}
69- _, err := context.performRequest(requestParams{
70+ _, _, err := context.performRequest(requestParams{
71 Method: "GET",
72 URL: uri,
73 APIVersion: "2012-02-12",
74@@ -397,7 +397,7 @@
75 uri = addURLQueryParams(uri, "prefix", request.Prefix)
76 }
77 blobs := BlobEnumerationResults{}
78- _, err := context.performRequest(requestParams{
79+ _, _, err := context.performRequest(requestParams{
80 Method: "GET",
81 URL: uri,
82 APIVersion: "2012-02-12",
83@@ -417,7 +417,7 @@
84 uri := addURLQueryParams(
85 context.getContainerURL(container),
86 "restype", "container")
87- _, err := context.performRequest(requestParams{
88+ _, _, err := context.performRequest(requestParams{
89 Method: "PUT",
90 URL: uri,
91 APIVersion: "2012-02-12",
92@@ -436,7 +436,7 @@
93 uri := addURLQueryParams(
94 context.getContainerURL(container),
95 "restype", "container")
96- _, err := context.performRequest(requestParams{
97+ _, _, err := context.performRequest(requestParams{
98 Method: "DELETE",
99 URL: uri,
100 APIVersion: "2012-02-12",
101@@ -449,6 +449,35 @@
102 return nil
103 }
104
105+// Send a request to the storage service to retrieve a container's properties.
106+// Also doubles as a handy way to see if a container exists.
107+func (context *StorageContext) GetContainerProperties(container string) (*Properties, error) {
108+ uri := addURLQueryParams(
109+ context.getContainerURL(container),
110+ "restype", "container")
111+ params := requestParams{
112+ Method: "GET",
113+ URL: uri,
114+ APIVersion: "2012-02-12",
115+ ExpectedStatus: http.StatusOK,
116+ }
117+ _, headers, err := context.performRequest(params)
118+ if err != nil {
119+ msg := fmt.Sprintf("failed to find container %s: ", container)
120+ return nil, extendError(err, msg)
121+ }
122+
123+ props := &Properties{
124+ LastModified: headers[http.CanonicalHeaderKey("Last-Modified")][0],
125+ ETag: headers[http.CanonicalHeaderKey("ETag")][0],
126+ LeaseStatus: headers[http.CanonicalHeaderKey("X-Ms-Lease-Status")][0],
127+ LeaseState: headers[http.CanonicalHeaderKey("X-Ms-Lease-State")][0],
128+ LeaseDuration: headers[http.CanonicalHeaderKey("X-Ms-Lease-Duration")][0],
129+ }
130+
131+ return props, nil
132+}
133+
134 type PutBlobRequest struct {
135 Container string // Container name in the storage account
136 BlobType string // Pass "page" or "block"
137@@ -482,7 +511,7 @@
138 extraHeaders.Add("x-ms-blob-content-length", size)
139 }
140
141- _, err := context.performRequest(requestParams{
142+ _, _, err := context.performRequest(requestParams{
143 Method: "PUT",
144 URL: context.GetFileURL(req.Container, req.Filename),
145 APIVersion: "2012-02-12",
146@@ -523,7 +552,7 @@
147 extraHeaders.Add("x-ms-range", rangeData)
148 extraHeaders.Add("x-ms-page-write", "update")
149
150- _, err := context.performRequest(requestParams{
151+ _, _, err := context.performRequest(requestParams{
152 Method: "PUT",
153 URL: uri,
154 Body: req.Data,
155@@ -546,7 +575,7 @@
156 "comp", "blocklist",
157 "blocklisttype", "all")
158 bl := GetBlockList{}
159- _, err := context.performRequest(requestParams{
160+ _, _, err := context.performRequest(requestParams{
161 Method: "GET",
162 URL: uri,
163 APIVersion: "2012-02-12",
164@@ -568,7 +597,7 @@
165 context.GetFileURL(container, filename),
166 "comp", "block",
167 "blockid", base64ID)
168- _, err := context.performRequest(requestParams{
169+ _, _, err := context.performRequest(requestParams{
170 Method: "PUT",
171 URL: uri,
172 Body: data,
173@@ -593,7 +622,7 @@
174 }
175 dataReader := bytes.NewReader(data)
176
177- _, err = context.performRequest(requestParams{
178+ _, _, err = context.performRequest(requestParams{
179 Method: "PUT",
180 URL: uri,
181 Body: dataReader,
182@@ -610,7 +639,7 @@
183 // Delete the specified blob from the given container. Deleting a non-existant
184 // blob will return without an error.
185 func (context *StorageContext) DeleteBlob(container, filename string) error {
186- _, err := context.performRequest(requestParams{
187+ _, _, err := context.performRequest(requestParams{
188 Method: "DELETE",
189 URL: context.GetFileURL(container, filename),
190 APIVersion: "2012-02-12",
191@@ -630,7 +659,7 @@
192
193 // Get the specified blob from the given container.
194 func (context *StorageContext) GetBlob(container, filename string) (io.ReadCloser, error) {
195- body, err := context.performRequest(requestParams{
196+ body, _, err := context.performRequest(requestParams{
197 Method: "GET",
198 URL: context.GetFileURL(container, filename),
199 APIVersion: "2012-02-12",
200@@ -666,7 +695,7 @@
201 panic("Access must be one of 'container', 'blob' or 'private'")
202 }
203
204- _, err := context.performRequest(requestParams{
205+ _, _, err := context.performRequest(requestParams{
206 Method: "PUT",
207 URL: uri,
208 APIVersion: "2009-09-19",
209
210=== modified file 'storage_base_test.go'
211--- storage_base_test.go 2013-07-29 00:43:12 +0000
212+++ storage_base_test.go 2013-07-31 06:34:25 +0000
213@@ -743,6 +743,71 @@
214 c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound)
215 }
216
217+type TestGetContainerProperties struct{}
218+
219+var _ = Suite(&TestGetContainerProperties{})
220+
221+// The GetContainerProperties Storage API call returns without error when the
222+// container has been created successfully.
223+func (suite *TestGetContainerProperties) Test(c *C) {
224+ header := make(http.Header)
225+ header.Add("Last-Modified", "last-modified")
226+ header.Add("ETag", "etag")
227+ header.Add("X-Ms-Lease-Status", "status")
228+ header.Add("X-Ms-Lease-State", "state")
229+ header.Add("X-Ms-Lease-Duration", "duration")
230+ response := &http.Response{
231+ Status: fmt.Sprintf("%d", http.StatusOK),
232+ StatusCode: http.StatusOK,
233+ Body: makeResponseBody(""),
234+ Header: header,
235+ }
236+
237+ transport := &TestTransport{Response: response}
238+ context := makeStorageContext(transport)
239+ containerName := MakeRandomString(10)
240+ props, err := context.GetContainerProperties(containerName)
241+ c.Assert(err, IsNil)
242+ c.Check(transport.Request.URL.String(), Equals, fmt.Sprintf(
243+ "http://%s.blob.core.windows.net/%s?restype=container",
244+ context.Account, containerName))
245+ c.Check(transport.Request.Method, Equals, "GET")
246+ c.Check(transport.Request.Header.Get("Authorization"), Not(Equals), "")
247+
248+ c.Check(props.LastModified, Equals, "last-modified")
249+ c.Check(props.ETag, Equals, "etag")
250+ c.Check(props.LeaseStatus, Equals, "status")
251+ c.Check(props.LeaseState, Equals, "state")
252+ c.Check(props.LeaseDuration, Equals, "duration")
253+}
254+
255+// Client-side errors from the HTTP client are propagated back to the caller.
256+func (suite *TestGetContainerProperties) TestError(c *C) {
257+ error := fmt.Errorf("canned-error")
258+ context := makeStorageContext(&TestTransport{Error: error})
259+ _, err := context.GetContainerProperties("container")
260+ c.Assert(err, ErrorMatches, ".*canned-error.*")
261+}
262+
263+// Server-side errors are propagated back to the caller.
264+func (suite *TestGetContainerProperties) TestErrorResponse(c *C) {
265+ response := makeHttpResponse(http.StatusNotFound, "not found")
266+ context := makeStorageContext(&TestTransport{Response: response})
267+ _, err := context.GetContainerProperties("container")
268+ c.Assert(err, ErrorMatches, ".*Not Found.*")
269+}
270+
271+// Azure HTTP errors (for instance 404 responses) are propagated back to
272+// the caller as ServerError objects.
273+func (suite *TestGetContainerProperties) TestServerError(c *C) {
274+ response := makeHttpResponse(http.StatusNotFound, "not found")
275+ context := makeStorageContext(&TestTransport{Response: response})
276+ _, err := context.GetContainerProperties("container")
277+ serverError, ok := err.(*ServerError)
278+ c.Assert(ok, Equals, true)
279+ c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound)
280+}
281+
282 type TestPutPage struct{}
283
284 var _ = Suite(&TestPutPage{})

Subscribers

People subscribed via source and target branches

to all changes: