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

Proposed by Julian Edwards
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
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
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.
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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?

Revision history for this message
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!

Revision history for this message
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
Revision history for this message
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
=== modified file 'storage_base.go'
--- storage_base.go 2013-07-30 06:49:29 +0000
+++ storage_base.go 2013-07-31 06:34:25 +0000
@@ -259,12 +259,12 @@
259259
260// performRequest issues an HTTP request to Azure.260// performRequest issues an HTTP request to Azure.
261//261//
262// It returns the response body contents.262// It returns the response body contents and the response headers.
263func (context *StorageContext) performRequest(params requestParams) ([]byte, error) {263func (context *StorageContext) performRequest(params requestParams) ([]byte, http.Header, error) {
264 params.Check()264 params.Check()
265 req, err := http.NewRequest(params.Method, params.URL, params.Body)265 req, err := http.NewRequest(params.Method, params.URL, params.Body)
266 if err != nil {266 if err != nil {
267 return nil, err267 return nil, nil, err
268 }268 }
269 // net/http has no way of adding headers en-masse, hence this abomination.269 // net/http has no way of adding headers en-masse, hence this abomination.
270 for header, values := range params.ExtraHeaders {270 for header, values := range params.ExtraHeaders {
@@ -285,37 +285,37 @@
285//285//
286// If the response's HTTP status code is not the same as "expectedStatus"286// If the response's HTTP status code is not the same as "expectedStatus"
287// then an HTTPError will be returned as the error.287// then an HTTPError will be returned as the error.
288func (context *StorageContext) send(req *http.Request, res Deserializer, expectedStatus HTTPStatus) ([]byte, error) {288func (context *StorageContext) send(req *http.Request, res Deserializer, expectedStatus HTTPStatus) ([]byte, http.Header, error) {
289 client := context.getClient()289 client := context.getClient()
290 resp, err := client.Do(req)290 resp, err := client.Do(req)
291 if err != nil {291 if err != nil {
292 return nil, err292 return nil, nil, err
293 }293 }
294294
295 body, err := readAndClose(resp.Body)295 body, err := readAndClose(resp.Body)
296 if err != nil {296 if err != nil {
297 return nil, fmt.Errorf("failed to read response data: %v", err)297 return nil, nil, fmt.Errorf("failed to read response data: %v", err)
298 }298 }
299299
300 if resp.StatusCode != int(expectedStatus) {300 if resp.StatusCode != int(expectedStatus) {
301 msg := newHTTPError(resp.StatusCode, body, "Azure request failed")301 msg := newHTTPError(resp.StatusCode, body, "Azure request failed")
302 return body, msg302 return body, resp.Header, msg
303 }303 }
304304
305 // If the caller didn't supply an object to deserialize the message into305 // If the caller didn't supply an object to deserialize the message into
306 // then just return.306 // then just return.
307 if res == nil {307 if res == nil {
308 return body, nil308 return body, resp.Header, nil
309 }309 }
310310
311 // TODO: Also deserialize response headers into the "res" object.311 // TODO: Also deserialize response headers into the "res" object.
312 err = res.Deserialize(body)312 err = res.Deserialize(body)
313 if err != nil {313 if err != nil {
314 msg := fmt.Errorf("Failed to deserialize data: %s", err)314 msg := fmt.Errorf("Failed to deserialize data: %s", err)
315 return body, msg315 return body, resp.Header, msg
316 }316 }
317317
318 return body, nil318 return body, resp.Header, nil
319}319}
320320
321// getAccountURL returns the base URL for the context's storage account.321// getAccountURL returns the base URL for the context's storage account.
@@ -360,7 +360,7 @@
360 uri = addURLQueryParams(uri, "marker", request.Marker)360 uri = addURLQueryParams(uri, "marker", request.Marker)
361 }361 }
362 containers := ContainerEnumerationResults{}362 containers := ContainerEnumerationResults{}
363 _, err := context.performRequest(requestParams{363 _, _, err := context.performRequest(requestParams{
364 Method: "GET",364 Method: "GET",
365 URL: uri,365 URL: uri,
366 APIVersion: "2012-02-12",366 APIVersion: "2012-02-12",
@@ -397,7 +397,7 @@
397 uri = addURLQueryParams(uri, "prefix", request.Prefix)397 uri = addURLQueryParams(uri, "prefix", request.Prefix)
398 }398 }
399 blobs := BlobEnumerationResults{}399 blobs := BlobEnumerationResults{}
400 _, err := context.performRequest(requestParams{400 _, _, err := context.performRequest(requestParams{
401 Method: "GET",401 Method: "GET",
402 URL: uri,402 URL: uri,
403 APIVersion: "2012-02-12",403 APIVersion: "2012-02-12",
@@ -417,7 +417,7 @@
417 uri := addURLQueryParams(417 uri := addURLQueryParams(
418 context.getContainerURL(container),418 context.getContainerURL(container),
419 "restype", "container")419 "restype", "container")
420 _, err := context.performRequest(requestParams{420 _, _, err := context.performRequest(requestParams{
421 Method: "PUT",421 Method: "PUT",
422 URL: uri,422 URL: uri,
423 APIVersion: "2012-02-12",423 APIVersion: "2012-02-12",
@@ -436,7 +436,7 @@
436 uri := addURLQueryParams(436 uri := addURLQueryParams(
437 context.getContainerURL(container),437 context.getContainerURL(container),
438 "restype", "container")438 "restype", "container")
439 _, err := context.performRequest(requestParams{439 _, _, err := context.performRequest(requestParams{
440 Method: "DELETE",440 Method: "DELETE",
441 URL: uri,441 URL: uri,
442 APIVersion: "2012-02-12",442 APIVersion: "2012-02-12",
@@ -449,6 +449,35 @@
449 return nil449 return nil
450}450}
451451
452// Send a request to the storage service to retrieve a container's properties.
453// Also doubles as a handy way to see if a container exists.
454func (context *StorageContext) GetContainerProperties(container string) (*Properties, error) {
455 uri := addURLQueryParams(
456 context.getContainerURL(container),
457 "restype", "container")
458 params := requestParams{
459 Method: "GET",
460 URL: uri,
461 APIVersion: "2012-02-12",
462 ExpectedStatus: http.StatusOK,
463 }
464 _, headers, err := context.performRequest(params)
465 if err != nil {
466 msg := fmt.Sprintf("failed to find container %s: ", container)
467 return nil, extendError(err, msg)
468 }
469
470 props := &Properties{
471 LastModified: headers[http.CanonicalHeaderKey("Last-Modified")][0],
472 ETag: headers[http.CanonicalHeaderKey("ETag")][0],
473 LeaseStatus: headers[http.CanonicalHeaderKey("X-Ms-Lease-Status")][0],
474 LeaseState: headers[http.CanonicalHeaderKey("X-Ms-Lease-State")][0],
475 LeaseDuration: headers[http.CanonicalHeaderKey("X-Ms-Lease-Duration")][0],
476 }
477
478 return props, nil
479}
480
452type PutBlobRequest struct {481type PutBlobRequest struct {
453 Container string // Container name in the storage account482 Container string // Container name in the storage account
454 BlobType string // Pass "page" or "block"483 BlobType string // Pass "page" or "block"
@@ -482,7 +511,7 @@
482 extraHeaders.Add("x-ms-blob-content-length", size)511 extraHeaders.Add("x-ms-blob-content-length", size)
483 }512 }
484513
485 _, err := context.performRequest(requestParams{514 _, _, err := context.performRequest(requestParams{
486 Method: "PUT",515 Method: "PUT",
487 URL: context.GetFileURL(req.Container, req.Filename),516 URL: context.GetFileURL(req.Container, req.Filename),
488 APIVersion: "2012-02-12",517 APIVersion: "2012-02-12",
@@ -523,7 +552,7 @@
523 extraHeaders.Add("x-ms-range", rangeData)552 extraHeaders.Add("x-ms-range", rangeData)
524 extraHeaders.Add("x-ms-page-write", "update")553 extraHeaders.Add("x-ms-page-write", "update")
525554
526 _, err := context.performRequest(requestParams{555 _, _, err := context.performRequest(requestParams{
527 Method: "PUT",556 Method: "PUT",
528 URL: uri,557 URL: uri,
529 Body: req.Data,558 Body: req.Data,
@@ -546,7 +575,7 @@
546 "comp", "blocklist",575 "comp", "blocklist",
547 "blocklisttype", "all")576 "blocklisttype", "all")
548 bl := GetBlockList{}577 bl := GetBlockList{}
549 _, err := context.performRequest(requestParams{578 _, _, err := context.performRequest(requestParams{
550 Method: "GET",579 Method: "GET",
551 URL: uri,580 URL: uri,
552 APIVersion: "2012-02-12",581 APIVersion: "2012-02-12",
@@ -568,7 +597,7 @@
568 context.GetFileURL(container, filename),597 context.GetFileURL(container, filename),
569 "comp", "block",598 "comp", "block",
570 "blockid", base64ID)599 "blockid", base64ID)
571 _, err := context.performRequest(requestParams{600 _, _, err := context.performRequest(requestParams{
572 Method: "PUT",601 Method: "PUT",
573 URL: uri,602 URL: uri,
574 Body: data,603 Body: data,
@@ -593,7 +622,7 @@
593 }622 }
594 dataReader := bytes.NewReader(data)623 dataReader := bytes.NewReader(data)
595624
596 _, err = context.performRequest(requestParams{625 _, _, err = context.performRequest(requestParams{
597 Method: "PUT",626 Method: "PUT",
598 URL: uri,627 URL: uri,
599 Body: dataReader,628 Body: dataReader,
@@ -610,7 +639,7 @@
610// Delete the specified blob from the given container. Deleting a non-existant639// Delete the specified blob from the given container. Deleting a non-existant
611// blob will return without an error.640// blob will return without an error.
612func (context *StorageContext) DeleteBlob(container, filename string) error {641func (context *StorageContext) DeleteBlob(container, filename string) error {
613 _, err := context.performRequest(requestParams{642 _, _, err := context.performRequest(requestParams{
614 Method: "DELETE",643 Method: "DELETE",
615 URL: context.GetFileURL(container, filename),644 URL: context.GetFileURL(container, filename),
616 APIVersion: "2012-02-12",645 APIVersion: "2012-02-12",
@@ -630,7 +659,7 @@
630659
631// Get the specified blob from the given container.660// Get the specified blob from the given container.
632func (context *StorageContext) GetBlob(container, filename string) (io.ReadCloser, error) {661func (context *StorageContext) GetBlob(container, filename string) (io.ReadCloser, error) {
633 body, err := context.performRequest(requestParams{662 body, _, err := context.performRequest(requestParams{
634 Method: "GET",663 Method: "GET",
635 URL: context.GetFileURL(container, filename),664 URL: context.GetFileURL(container, filename),
636 APIVersion: "2012-02-12",665 APIVersion: "2012-02-12",
@@ -666,7 +695,7 @@
666 panic("Access must be one of 'container', 'blob' or 'private'")695 panic("Access must be one of 'container', 'blob' or 'private'")
667 }696 }
668697
669 _, err := context.performRequest(requestParams{698 _, _, err := context.performRequest(requestParams{
670 Method: "PUT",699 Method: "PUT",
671 URL: uri,700 URL: uri,
672 APIVersion: "2009-09-19",701 APIVersion: "2009-09-19",
673702
=== modified file 'storage_base_test.go'
--- storage_base_test.go 2013-07-29 00:43:12 +0000
+++ storage_base_test.go 2013-07-31 06:34:25 +0000
@@ -743,6 +743,71 @@
743 c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound)743 c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound)
744}744}
745745
746type TestGetContainerProperties struct{}
747
748var _ = Suite(&TestGetContainerProperties{})
749
750// The GetContainerProperties Storage API call returns without error when the
751// container has been created successfully.
752func (suite *TestGetContainerProperties) Test(c *C) {
753 header := make(http.Header)
754 header.Add("Last-Modified", "last-modified")
755 header.Add("ETag", "etag")
756 header.Add("X-Ms-Lease-Status", "status")
757 header.Add("X-Ms-Lease-State", "state")
758 header.Add("X-Ms-Lease-Duration", "duration")
759 response := &http.Response{
760 Status: fmt.Sprintf("%d", http.StatusOK),
761 StatusCode: http.StatusOK,
762 Body: makeResponseBody(""),
763 Header: header,
764 }
765
766 transport := &TestTransport{Response: response}
767 context := makeStorageContext(transport)
768 containerName := MakeRandomString(10)
769 props, err := context.GetContainerProperties(containerName)
770 c.Assert(err, IsNil)
771 c.Check(transport.Request.URL.String(), Equals, fmt.Sprintf(
772 "http://%s.blob.core.windows.net/%s?restype=container",
773 context.Account, containerName))
774 c.Check(transport.Request.Method, Equals, "GET")
775 c.Check(transport.Request.Header.Get("Authorization"), Not(Equals), "")
776
777 c.Check(props.LastModified, Equals, "last-modified")
778 c.Check(props.ETag, Equals, "etag")
779 c.Check(props.LeaseStatus, Equals, "status")
780 c.Check(props.LeaseState, Equals, "state")
781 c.Check(props.LeaseDuration, Equals, "duration")
782}
783
784// Client-side errors from the HTTP client are propagated back to the caller.
785func (suite *TestGetContainerProperties) TestError(c *C) {
786 error := fmt.Errorf("canned-error")
787 context := makeStorageContext(&TestTransport{Error: error})
788 _, err := context.GetContainerProperties("container")
789 c.Assert(err, ErrorMatches, ".*canned-error.*")
790}
791
792// Server-side errors are propagated back to the caller.
793func (suite *TestGetContainerProperties) TestErrorResponse(c *C) {
794 response := makeHttpResponse(http.StatusNotFound, "not found")
795 context := makeStorageContext(&TestTransport{Response: response})
796 _, err := context.GetContainerProperties("container")
797 c.Assert(err, ErrorMatches, ".*Not Found.*")
798}
799
800// Azure HTTP errors (for instance 404 responses) are propagated back to
801// the caller as ServerError objects.
802func (suite *TestGetContainerProperties) TestServerError(c *C) {
803 response := makeHttpResponse(http.StatusNotFound, "not found")
804 context := makeStorageContext(&TestTransport{Response: response})
805 _, err := context.GetContainerProperties("container")
806 serverError, ok := err.(*ServerError)
807 c.Assert(ok, Equals, true)
808 c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound)
809}
810
746type TestPutPage struct{}811type TestPutPage struct{}
747812
748var _ = Suite(&TestPutPage{})813var _ = Suite(&TestPutPage{})

Subscribers

People subscribed via source and target branches

to all changes: