Merge lp:~julian-edwards/gwacl/get-container-properties into lp:gwacl
- get-container-properties
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Julian Edwards |
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email: mp+177736@code.launchpad.net |
Commit message
Implement GetContainerPro
Description of the change
Implement GetContainerPro
Julian Edwards (julian-edwards) wrote : | # |
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 GetContainerPro
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.
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 GetContainerPro
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
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{}) |
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.