From a3276fcc5be14c6d3f41e87604158b81ff3654d6 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 19 May 2015 19:18:30 -0700 Subject: [PATCH] Feedback update Update comments and TODOs Fix switch style Updated parse http response to take in reader Add Cancel implementation Update blobstore variable name Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/client/blob_writer.go | 23 ++++++-- registry/client/blob_writer_test.go | 10 ---- registry/client/errors.go | 7 ++- registry/client/repository.go | 90 +++++++++++++++-------------- 4 files changed, 68 insertions(+), 62 deletions(-) diff --git a/registry/client/blob_writer.go b/registry/client/blob_writer.go index 06ca8738..55223520 100644 --- a/registry/client/blob_writer.go +++ b/registry/client/blob_writer.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "errors" "fmt" "io" "io/ioutil" @@ -49,7 +48,6 @@ func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) { return 0, hbu.handleErrorResponse(resp) } - // TODO(dmcgowan): Validate headers hbu.uuid = resp.Header.Get("Docker-Upload-UUID") hbu.location, err = sanitizeLocation(resp.Header.Get("Location"), hbu.location) if err != nil { @@ -85,7 +83,6 @@ func (hbu *httpBlobUpload) Write(p []byte) (n int, err error) { return 0, hbu.handleErrorResponse(resp) } - // TODO(dmcgowan): Validate headers hbu.uuid = resp.Header.Get("Docker-Upload-UUID") hbu.location, err = sanitizeLocation(resp.Header.Get("Location"), hbu.location) if err != nil { @@ -110,7 +107,7 @@ func (hbu *httpBlobUpload) Seek(offset int64, whence int) (int64, error) { case os.SEEK_CUR: newOffset += int64(offset) case os.SEEK_END: - return newOffset, errors.New("Cannot seek from end on incomplete upload") + newOffset += int64(offset) case os.SEEK_SET: newOffset = int64(offset) } @@ -143,6 +140,7 @@ func (hbu *httpBlobUpload) Commit(ctx context.Context, desc distribution.Descrip if err != nil { return distribution.Descriptor{}, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusCreated { return distribution.Descriptor{}, hbu.handleErrorResponse(resp) @@ -152,7 +150,22 @@ func (hbu *httpBlobUpload) Commit(ctx context.Context, desc distribution.Descrip } func (hbu *httpBlobUpload) Cancel(ctx context.Context) error { - panic("not implemented") + req, err := http.NewRequest("DELETE", hbu.location, nil) + if err != nil { + return err + } + resp, err := hbu.client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusNoContent, http.StatusNotFound: + return nil + default: + return hbu.handleErrorResponse(resp) + } } func (hbu *httpBlobUpload) Close() error { diff --git a/registry/client/blob_writer_test.go b/registry/client/blob_writer_test.go index 4d2ae862..674d6e01 100644 --- a/registry/client/blob_writer_test.go +++ b/registry/client/blob_writer_test.go @@ -205,13 +205,3 @@ func TestUploadReadFrom(t *testing.T) { t.Fatalf("Unexpected response status: %s, expected %s", uploadErr.Status, expected) } } - -//repo distribution.Repository -//client *http.Client - -//uuid string -//startedAt time.Time - -//location string // always the last value of the location header. -//offset int64 -//closed bool diff --git a/registry/client/errors.go b/registry/client/errors.go index c4296fa3..c6c802a2 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -3,6 +3,7 @@ package client import ( "encoding/json" "fmt" + "io" "io/ioutil" "net/http" @@ -34,9 +35,9 @@ func (e *UnexpectedHTTPResponseError) Error() string { return fmt.Sprintf("Error parsing HTTP response: %s: %q", e.ParseErr.Error(), shortenedResponse) } -func parseHTTPErrorResponse(response *http.Response) error { +func parseHTTPErrorResponse(r io.Reader) error { var errors v2.Errors - body, err := ioutil.ReadAll(response.Body) + body, err := ioutil.ReadAll(r) if err != nil { return err } @@ -52,7 +53,7 @@ func parseHTTPErrorResponse(response *http.Response) error { func handleErrorResponse(resp *http.Response) error { if resp.StatusCode >= 400 && resp.StatusCode < 500 { - return parseHTTPErrorResponse(resp) + return parseHTTPErrorResponse(resp.Body) } return &UnexpectedHTTPStatusError{Status: resp.Status} } diff --git a/registry/client/repository.go b/registry/client/repository.go index 788e7904..123ef6ce 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -11,12 +11,10 @@ import ( "strconv" "time" - "github.com/docker/distribution/manifest" - - "github.com/docker/distribution/digest" - "github.com/docker/distribution" "github.com/docker/distribution/context" + "github.com/docker/distribution/digest" + "github.com/docker/distribution/manifest" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client/transport" "github.com/docker/distribution/registry/storage/cache" @@ -108,8 +106,8 @@ func (ms *manifests) Tags() ([]string, error) { } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusOK: + switch resp.StatusCode { + case http.StatusOK: b, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err @@ -123,7 +121,7 @@ func (ms *manifests) Tags() ([]string, error) { } return tagsResponse.Tags, nil - case resp.StatusCode == http.StatusNotFound: + case http.StatusNotFound: return nil, nil default: return nil, handleErrorResponse(resp) @@ -131,6 +129,8 @@ func (ms *manifests) Tags() ([]string, error) { } func (ms *manifests) Exists(dgst digest.Digest) (bool, error) { + // Call by Tag endpoint since the API uses the same + // URL endpoint for tags and digests. return ms.ExistsByTag(dgst.String()) } @@ -145,10 +145,10 @@ func (ms *manifests) ExistsByTag(tag string) (bool, error) { return false, err } - switch { - case resp.StatusCode == http.StatusOK: + switch resp.StatusCode { + case http.StatusOK: return true, nil - case resp.StatusCode == http.StatusNotFound: + case http.StatusNotFound: return false, nil default: return false, handleErrorResponse(resp) @@ -156,6 +156,8 @@ func (ms *manifests) ExistsByTag(tag string) (bool, error) { } func (ms *manifests) Get(dgst digest.Digest) (*manifest.SignedManifest, error) { + // Call by Tag endpoint since the API uses the same + // URL endpoint for tags and digests. return ms.GetByTag(dgst.String()) } @@ -171,8 +173,8 @@ func (ms *manifests) GetByTag(tag string) (*manifest.SignedManifest, error) { } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusOK: + switch resp.StatusCode { + case http.StatusOK: var sm manifest.SignedManifest decoder := json.NewDecoder(resp.Body) @@ -203,9 +205,9 @@ func (ms *manifests) Put(m *manifest.SignedManifest) error { } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusAccepted: - // TODO(dmcgowan): Use or check digest header + switch resp.StatusCode { + case http.StatusAccepted: + // TODO(dmcgowan): make use of digest header return nil default: return handleErrorResponse(resp) @@ -228,8 +230,8 @@ func (ms *manifests) Delete(dgst digest.Digest) error { } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusOK: + switch resp.StatusCode { + case http.StatusOK: return nil default: return handleErrorResponse(resp) @@ -263,17 +265,17 @@ func sanitizeLocation(location, source string) (string, error) { return location, nil } -func (ls *blobs) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - return ls.statter.Stat(ctx, dgst) +func (bs *blobs) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { + return bs.statter.Stat(ctx, dgst) } -func (ls *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { - desc, err := ls.Stat(ctx, dgst) +func (bs *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { + desc, err := bs.Stat(ctx, dgst) if err != nil { return nil, err } - reader, err := ls.Open(ctx, desc.Digest) + reader, err := bs.Open(ctx, desc.Digest) if err != nil { return nil, err } @@ -282,26 +284,26 @@ func (ls *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { return ioutil.ReadAll(reader) } -func (ls *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { - stat, err := ls.statter.Stat(ctx, dgst) +func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { + stat, err := bs.statter.Stat(ctx, dgst) if err != nil { return nil, err } - blobURL, err := ls.ub.BuildBlobURL(ls.Name(), stat.Digest) + blobURL, err := bs.ub.BuildBlobURL(bs.Name(), stat.Digest) if err != nil { return nil, err } - return transport.NewHTTPReadSeeker(ls.repository.client, blobURL, stat.Length), nil + return transport.NewHTTPReadSeeker(bs.repository.client, blobURL, stat.Length), nil } -func (ls *blobs) ServeBlob(ctx context.Context, w http.ResponseWriter, r *http.Request, dgst digest.Digest) error { - return nil +func (bs *blobs) ServeBlob(ctx context.Context, w http.ResponseWriter, r *http.Request, dgst digest.Digest) error { + panic("not implemented") } -func (ls *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { - writer, err := ls.Create(ctx) +func (bs *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { + writer, err := bs.Create(ctx) if err != nil { return distribution.Descriptor{}, err } @@ -323,17 +325,17 @@ func (ls *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribut return writer.Commit(ctx, desc) } -func (ls *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { - u, err := ls.ub.BuildBlobUploadURL(ls.name) +func (bs *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { + u, err := bs.ub.BuildBlobUploadURL(bs.name) - resp, err := ls.client.Post(u, "", nil) + resp, err := bs.client.Post(u, "", nil) if err != nil { return nil, err } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusAccepted: + switch resp.StatusCode { + case http.StatusAccepted: // TODO(dmcgowan): Check for invalid UUID uuid := resp.Header.Get("Docker-Upload-UUID") location, err := sanitizeLocation(resp.Header.Get("Location"), u) @@ -342,8 +344,8 @@ func (ls *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { } return &httpBlobUpload{ - repo: ls.repository, - client: ls.client, + repo: bs.repository, + client: bs.client, uuid: uuid, startedAt: time.Now(), location: location, @@ -353,7 +355,7 @@ func (ls *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { } } -func (ls *blobs) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { +func (bs *blobs) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { panic("not implemented") } @@ -361,20 +363,20 @@ type blobStatter struct { *repository } -func (ls *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - u, err := ls.ub.BuildBlobURL(ls.name, dgst) +func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { + u, err := bs.ub.BuildBlobURL(bs.name, dgst) if err != nil { return distribution.Descriptor{}, err } - resp, err := ls.client.Head(u) + resp, err := bs.client.Head(u) if err != nil { return distribution.Descriptor{}, err } defer resp.Body.Close() - switch { - case resp.StatusCode == http.StatusOK: + switch resp.StatusCode { + case http.StatusOK: lengthHeader := resp.Header.Get("Content-Length") length, err := strconv.ParseInt(lengthHeader, 10, 64) if err != nil { @@ -386,7 +388,7 @@ func (ls *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi Length: length, Digest: dgst, }, nil - case resp.StatusCode == http.StatusNotFound: + case http.StatusNotFound: return distribution.Descriptor{}, distribution.ErrBlobUnknown default: return distribution.Descriptor{}, handleErrorResponse(resp)