diff --git a/api/api.go b/api/api.go index 4ba72b18..87f3d23c 100644 --- a/api/api.go +++ b/api/api.go @@ -6,10 +6,6 @@ import ( "bytes" "encoding/json" "fmt" - "io" - "io/ioutil" - "os" - "path/filepath" "strconv" "github.com/github/git-lfs/config" @@ -21,6 +17,36 @@ import ( "github.com/github/git-lfs/vendor/_nuts/github.com/rubyist/tracerx" ) +// BatchOrLegacy calls the Batch API and falls back on the Legacy API +// This is for simplicity, legacy route is not most optimal (serial) +// TODO remove when legacy API removed +func BatchOrLegacy(objects []*ObjectResource, operation string) ([]*ObjectResource, error) { + if !config.Config.BatchTransfer() { + return Legacy(objects, operation) + } + objs, err := Batch(objects, operation) + if err != nil { + if errutil.IsNotImplementedError(err) { + git.Config.SetLocal("", "lfs.batch", "false") + return Legacy(objects, operation) + } + return nil, err + } + return objs, nil +} + +func BatchOrLegacySingle(inobj *ObjectResource, operation string) (*ObjectResource, error) { + objs, err := BatchOrLegacy([]*ObjectResource{inobj}, operation) + if err != nil { + return nil, err + } + if len(objs) > 0 { + return objs[0], nil + } + return nil, fmt.Errorf("Object not found") +} + +// Batch calls the batch API and returns object results func Batch(objects []*ObjectResource, operation string) ([]*ObjectResource, error) { if len(objects) == 0 { return nil, nil @@ -80,61 +106,30 @@ func Batch(objects []*ObjectResource, operation string) ([]*ObjectResource, erro return objs, nil } -// Download will attempt to download the object with the given oid. The batched -// API will be used, but if the server does not implement the batch operations -// it will fall back to the legacy API. -func Download(oid string, size int64) (io.ReadCloser, int64, error) { - if !config.Config.BatchTransfer() { - return DownloadLegacy(oid) - } - - objects := []*ObjectResource{ - &ObjectResource{Oid: oid, Size: size}, - } - - objs, err := Batch(objects, "download") - if err != nil { - if errutil.IsNotImplementedError(err) { - git.Config.SetLocal("", "lfs.batch", "false") - return DownloadLegacy(oid) +// Legacy calls the legacy API serially and returns ObjectResources +// TODO remove when legacy API removed +func Legacy(objects []*ObjectResource, operation string) ([]*ObjectResource, error) { + retobjs := make([]*ObjectResource, 0, len(objects)) + dl := operation == "download" + var globalErr error + for _, o := range objects { + var ret *ObjectResource + var err error + if dl { + ret, err = DownloadCheck(o.Oid) + } else { + ret, err = UploadCheck(o.Oid, o.Size) } - return nil, 0, err + if err != nil { + // Store for the end, likely only one + globalErr = err + } + retobjs = append(retobjs, ret) } - - if len(objs) != 1 { // Expecting to find one object - return nil, 0, errutil.Error(fmt.Errorf("Object not found: %s", oid)) - } - - return DownloadObject(objs[0]) -} - -// DownloadLegacy attempts to download the object for the given oid using the -// legacy API. -func DownloadLegacy(oid string) (io.ReadCloser, int64, error) { - req, err := NewRequest("GET", oid) - if err != nil { - return nil, 0, errutil.Error(err) - } - - res, obj, err := DoLegacyRequest(req) - if err != nil { - return nil, 0, err - } - httputil.LogTransfer("lfs.download", res) - req, err = obj.NewRequest("download", "GET") - if err != nil { - return nil, 0, errutil.Error(err) - } - - res, err = httputil.DoHttpRequest(req, true) - if err != nil { - return nil, 0, err - } - httputil.LogTransfer("lfs.data.download", res) - - return res.Body, res.ContentLength, nil + return retobjs, globalErr } +// TODO remove when legacy API removed func DownloadCheck(oid string) (*ObjectResource, error) { req, err := NewRequest("GET", oid) if err != nil { @@ -155,32 +150,12 @@ func DownloadCheck(oid string) (*ObjectResource, error) { return obj, nil } -func DownloadObject(obj *ObjectResource) (io.ReadCloser, int64, error) { - req, err := obj.NewRequest("download", "GET") - if err != nil { - return nil, 0, errutil.Error(err) - } - - res, err := httputil.DoHttpRequest(req, true) - if err != nil { - return nil, 0, errutil.NewRetriableError(err) - } - httputil.LogTransfer("lfs.data.download", res) - - return res.Body, res.ContentLength, nil -} - -func UploadCheck(oidPath string) (*ObjectResource, error) { - oid := filepath.Base(oidPath) - - stat, err := os.Stat(oidPath) - if err != nil { - return nil, errutil.Error(err) - } +// TODO remove when legacy API removed +func UploadCheck(oid string, size int64) (*ObjectResource, error) { reqObj := &ObjectResource{ Oid: oid, - Size: stat.Size(), + Size: size, } by, err := json.Marshal(reqObj) @@ -204,7 +179,7 @@ func UploadCheck(oidPath string) (*ObjectResource, error) { if err != nil { if errutil.IsAuthError(err) { httputil.SetAuthType(req, res) - return UploadCheck(oidPath) + return UploadCheck(oid, size) } return nil, errutil.NewRetriableError(err) @@ -224,72 +199,3 @@ func UploadCheck(oidPath string) (*ObjectResource, error) { return obj, nil } - -func UploadObject(obj *ObjectResource, reader io.Reader) error { - - req, err := obj.NewRequest("upload", "PUT") - if err != nil { - return errutil.Error(err) - } - - if len(req.Header.Get("Content-Type")) == 0 { - req.Header.Set("Content-Type", "application/octet-stream") - } - - if req.Header.Get("Transfer-Encoding") == "chunked" { - req.TransferEncoding = []string{"chunked"} - } else { - req.Header.Set("Content-Length", strconv.FormatInt(obj.Size, 10)) - } - - req.ContentLength = obj.Size - req.Body = ioutil.NopCloser(reader) - - res, err := httputil.DoHttpRequest(req, true) - if err != nil { - return errutil.NewRetriableError(err) - } - httputil.LogTransfer("lfs.data.upload", res) - - // A status code of 403 likely means that an authentication token for the - // upload has expired. This can be safely retried. - if res.StatusCode == 403 { - return errutil.NewRetriableError(err) - } - - if res.StatusCode > 299 { - return errutil.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) - } - - io.Copy(ioutil.Discard, res.Body) - res.Body.Close() - - if _, ok := obj.Rel("verify"); !ok { - return nil - } - - req, err = obj.NewRequest("verify", "POST") - if err != nil { - return errutil.Error(err) - } - - by, err := json.Marshal(obj) - if err != nil { - return errutil.Error(err) - } - - req.Header.Set("Content-Type", MediaType) - req.Header.Set("Content-Length", strconv.Itoa(len(by))) - req.ContentLength = int64(len(by)) - req.Body = ioutil.NopCloser(bytes.NewReader(by)) - res, err = DoRequest(req, true) - if err != nil { - return err - } - - httputil.LogTransfer("lfs.data.verify", res) - io.Copy(ioutil.Discard, res.Body) - res.Body.Close() - - return err -} diff --git a/api/download_test.go b/api/download_test.go index 5bebebff..87b9744f 100644 --- a/api/download_test.go +++ b/api/download_test.go @@ -73,55 +73,22 @@ func TestSuccessfulDownload(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != "" { - t.Error("Invalid Accept") - } - - if r.Header.Get("A") != "1" { - t.Error("invalid A") - } - - head := w.Header() - head.Set("Content-Type", "application/octet-stream") - head.Set("Content-Length", "4") - w.WriteHeader(200) - w.Write([]byte("test")) - }) - defer config.Config.ResetConfig() config.Config.SetConfig("lfs.batch", "false") config.Config.SetConfig("lfs.url", server.URL+"/media") - reader, size, err := api.Download("oid", 0) + obj, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: "oid"}, "download") if err != nil { if isDockerConnectionError(err) { return } t.Fatalf("unexpected error: %s", err) } - defer reader.Close() - if size != 4 { - t.Errorf("unexpected size: %d", size) + if obj.Size != 4 { + t.Errorf("unexpected size: %d", obj.Size) } - by, err := ioutil.ReadAll(reader) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if body := string(by); body != "test" { - t.Errorf("unexpected body: %s", body) - } } // nearly identical to TestSuccessfulDownload @@ -212,36 +179,12 @@ func TestSuccessfulDownloadWithRedirects(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != "" { - t.Error("Invalid Accept") - } - - if r.Header.Get("A") != "1" { - t.Error("invalid A") - } - - head := w.Header() - head.Set("Content-Type", "application/octet-stream") - head.Set("Content-Length", "4") - w.WriteHeader(200) - w.Write([]byte("test")) - }) - defer config.Config.ResetConfig() config.Config.SetConfig("lfs.batch", "false") config.Config.SetConfig("lfs.url", server.URL+"/redirect") for _, redirect := range redirectCodes { - reader, size, err := api.Download("oid", 0) + obj, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: "oid"}, "download") if err != nil { if isDockerConnectionError(err) { return @@ -249,19 +192,10 @@ func TestSuccessfulDownloadWithRedirects(t *testing.T) { t.Fatalf("unexpected error for %d status: %s", redirect, err) } - if size != 4 { - t.Errorf("unexpected size for %d status: %d", redirect, size) + if obj.Size != 4 { + t.Errorf("unexpected size for %d status: %d", redirect, obj.Size) } - by, err := ioutil.ReadAll(reader) - reader.Close() - if err != nil { - t.Fatalf("unexpected error for %d status: %s", redirect, err) - } - - if body := string(by); body != "test" { - t.Errorf("unexpected body for %d status: %s", redirect, body) - } } } @@ -323,310 +257,21 @@ func TestSuccessfulDownloadWithAuthorization(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != "" { - t.Error("Invalid Accept") - } - - if r.Header.Get("Authorization") != "custom" { - t.Error("Invalid Authorization") - } - - if r.Header.Get("A") != "1" { - t.Error("invalid A") - } - - head := w.Header() - head.Set("Content-Type", "application/octet-stream") - head.Set("Content-Length", "4") - w.WriteHeader(200) - w.Write([]byte("test")) - }) - defer config.Config.ResetConfig() config.Config.SetConfig("lfs.batch", "false") config.Config.SetConfig("lfs.url", server.URL+"/media") - reader, size, err := api.Download("oid", 0) + obj, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: "oid"}, "download") if err != nil { if isDockerConnectionError(err) { return } t.Fatalf("unexpected error: %s", err) } - defer reader.Close() - if size != 4 { - t.Errorf("unexpected size: %d", size) + if obj.Size != 4 { + t.Errorf("unexpected size: %d", obj.Size) } - by, err := ioutil.ReadAll(reader) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if body := string(by); body != "test" { - t.Errorf("unexpected body: %s", body) - } -} - -// nearly identical to TestSuccessfulDownload -// download is served from a second server -func TestSuccessfulDownloadFromSeparateHost(t *testing.T) { - SetupTestCredentialsFunc() - defer func() { - RestoreCredentialsFunc() - }() - - mux := http.NewServeMux() - server := httptest.NewServer(mux) - defer server.Close() - - mux2 := http.NewServeMux() - server2 := httptest.NewServer(mux2) - defer server2.Close() - - tmp := tempdir(t) - defer os.RemoveAll(tmp) - - mux.HandleFunc("/media/objects/oid", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Error("Invalid Accept") - } - - if r.Header.Get("Authorization") != expectedAuth(t, server) { - t.Error("Invalid Authorization") - } - - obj := &api.ObjectResource{ - Oid: "oid", - Size: 4, - Actions: map[string]*api.LinkRelation{ - "download": &api.LinkRelation{ - Href: server2.URL + "/download", - Header: map[string]string{"A": "1"}, - }, - }, - } - - by, err := json.Marshal(obj) - if err != nil { - t.Fatal(err) - } - - head := w.Header() - head.Set("Content-Type", api.MediaType) - head.Set("Content-Length", strconv.Itoa(len(by))) - w.WriteHeader(200) - w.Write(by) - }) - - mux2.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != "" { - t.Error("Invalid Accept") - } - - if r.Header.Get("A") != "1" { - t.Error("invalid A") - } - - head := w.Header() - head.Set("Content-Type", "application/octet-stream") - head.Set("Content-Length", "4") - w.WriteHeader(200) - w.Write([]byte("test")) - }) - - defer config.Config.ResetConfig() - config.Config.SetConfig("lfs.batch", "false") - config.Config.SetConfig("lfs.url", server.URL+"/media") - reader, size, err := api.Download("oid", 0) - if err != nil { - if isDockerConnectionError(err) { - return - } - t.Fatalf("unexpected error: %s", err) - } - defer reader.Close() - - if size != 4 { - t.Errorf("unexpected size: %d", size) - } - - by, err := ioutil.ReadAll(reader) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if body := string(by); body != "test" { - t.Errorf("unexpected body: %s", body) - } -} - -// nearly identical to TestSuccessfulDownload -// download is served from a second server -func TestSuccessfulDownloadFromSeparateRedirectedHost(t *testing.T) { - SetupTestCredentialsFunc() - defer func() { - RestoreCredentialsFunc() - }() - - mux := http.NewServeMux() - server := httptest.NewServer(mux) - defer server.Close() - - mux2 := http.NewServeMux() - server2 := httptest.NewServer(mux2) - defer server2.Close() - - mux3 := http.NewServeMux() - server3 := httptest.NewServer(mux3) - defer server3.Close() - - tmp := tempdir(t) - defer os.RemoveAll(tmp) - - // all of these should work for GET requests - redirectCodes := []int{301, 302, 303, 307} - redirectIndex := 0 - - mux.HandleFunc("/media/objects/oid", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server 1: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Error("Invalid Accept") - } - - if r.Header.Get("Authorization") != expectedAuth(t, server) { - t.Error("Invalid Authorization") - } - - w.Header().Set("Location", server2.URL+"/media/objects/oid") - w.WriteHeader(redirectCodes[redirectIndex]) - t.Logf("redirect with %d", redirectCodes[redirectIndex]) - redirectIndex += 1 - }) - - mux2.HandleFunc("/media/objects/oid", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server 2: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Error("Invalid Accept") - } - - if r.Header.Get("Authorization") != "" { - t.Error("Invalid Authorization") - } - - obj := &api.ObjectResource{ - Oid: "oid", - Size: 4, - Actions: map[string]*api.LinkRelation{ - "download": &api.LinkRelation{ - Href: server3.URL + "/download", - Header: map[string]string{"A": "1"}, - }, - }, - } - - by, err := json.Marshal(obj) - if err != nil { - t.Fatal(err) - } - - head := w.Header() - head.Set("Content-Type", api.MediaType) - head.Set("Content-Length", strconv.Itoa(len(by))) - w.WriteHeader(200) - w.Write(by) - }) - - mux3.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server 3: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != "" { - t.Error("Invalid Accept") - } - - if r.Header.Get("A") != "1" { - t.Error("invalid A") - } - - head := w.Header() - head.Set("Content-Type", "application/octet-stream") - head.Set("Content-Length", "4") - w.WriteHeader(200) - w.Write([]byte("test")) - }) - - defer config.Config.ResetConfig() - config.Config.SetConfig("lfs.batch", "false") - config.Config.SetConfig("lfs.url", server.URL+"/media") - - for _, redirect := range redirectCodes { - reader, size, err := api.Download("oid", 0) - if err != nil { - if isDockerConnectionError(err) { - return - } - t.Fatalf("unexpected error for %d status: %s", redirect, err) - } - - if size != 4 { - t.Errorf("unexpected size for %d status: %d", redirect, size) - } - - by, err := ioutil.ReadAll(reader) - reader.Close() - if err != nil { - t.Fatalf("unexpected error for %d status: %s", redirect, err) - } - - if body := string(by); body != "test" { - t.Errorf("unexpected body for %d status: %s", redirect, body) - } - } } func TestDownloadAPIError(t *testing.T) { @@ -649,7 +294,7 @@ func TestDownloadAPIError(t *testing.T) { defer config.Config.ResetConfig() config.Config.SetConfig("lfs.batch", "false") config.Config.SetConfig("lfs.url", server.URL+"/media") - _, _, err := api.Download("oid", 0) + _, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: "oid"}, "download") if err == nil { t.Fatal("no error?") } @@ -668,84 +313,6 @@ func TestDownloadAPIError(t *testing.T) { } -func TestDownloadStorageError(t *testing.T) { - SetupTestCredentialsFunc() - defer func() { - RestoreCredentialsFunc() - }() - - mux := http.NewServeMux() - server := httptest.NewServer(mux) - defer server.Close() - - tmp := tempdir(t) - defer os.RemoveAll(tmp) - - mux.HandleFunc("/media/objects/oid", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - t.Logf("request header: %v", r.Header) - - if r.Method != "GET" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Error("Invalid Accept") - } - - if r.Header.Get("Authorization") != expectedAuth(t, server) { - t.Error("Invalid Authorization") - } - - obj := &api.ObjectResource{ - Oid: "oid", - Size: 4, - Actions: map[string]*api.LinkRelation{ - "download": &api.LinkRelation{ - Href: server.URL + "/download", - Header: map[string]string{"A": "1"}, - }, - }, - } - - by, err := json.Marshal(obj) - if err != nil { - t.Fatal(err) - } - - head := w.Header() - head.Set("Content-Type", api.MediaType) - head.Set("Content-Length", strconv.Itoa(len(by))) - w.WriteHeader(200) - w.Write(by) - }) - - mux.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(500) - }) - - defer config.Config.ResetConfig() - config.Config.SetConfig("lfs.batch", "false") - config.Config.SetConfig("lfs.url", server.URL+"/media") - _, _, err := api.Download("oid", 0) - if err == nil { - t.Fatal("no error?") - } - - if isDockerConnectionError(err) { - return - } - - if !errutil.IsFatalError(err) { - t.Fatal("should panic") - } - - if err.Error() != fmt.Sprintf(httputil.GetDefaultError(500), server.URL+"/download") { - t.Fatalf("Unexpected error: %s", err.Error()) - } -} - // guards against connection errors that only seem to happen on debian docker // images. func isDockerConnectionError(err error) bool { diff --git a/api/upload_test.go b/api/upload_test.go index ad7d5303..750846df 100644 --- a/api/upload_test.go +++ b/api/upload_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "strconv" "testing" @@ -17,7 +18,6 @@ import ( "github.com/github/git-lfs/errutil" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/lfs" - "github.com/github/git-lfs/progress" "github.com/github/git-lfs/test" ) @@ -38,8 +38,6 @@ func TestExistingUpload(t *testing.T) { defer os.RemoveAll(tmp) postCalled := false - putCalled := false - verifyCalled := false mux.HandleFunc("/media/objects", func(w http.ResponseWriter, r *http.Request) { t.Logf("Server: %s %s", r.Method, r.URL) @@ -103,18 +101,6 @@ func TestExistingUpload(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/upload", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - putCalled = true - w.WriteHeader(200) - }) - - mux.HandleFunc("/verify", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - verifyCalled = true - w.WriteHeader(200) - }) - defer config.Config.ResetConfig() config.Config.SetConfig("lfs.url", server.URL+"/media") @@ -123,7 +109,9 @@ func TestExistingUpload(t *testing.T) { t.Fatal(err) } - o, err := api.UploadCheck(oidPath) + oid := filepath.Base(oidPath) + stat, _ := os.Stat(oidPath) + o, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: oid, Size: stat.Size()}, "upload") if err != nil { if isDockerConnectionError(err) { return @@ -138,13 +126,6 @@ func TestExistingUpload(t *testing.T) { t.Errorf("POST not called") } - if putCalled { - t.Errorf("PUT not skipped") - } - - if verifyCalled { - t.Errorf("verify not skipped") - } } func TestUploadWithRedirect(t *testing.T) { @@ -254,7 +235,9 @@ func TestUploadWithRedirect(t *testing.T) { t.Fatal(err) } - obj, err := api.UploadCheck(oidPath) + oid := filepath.Base(oidPath) + stat, _ := os.Stat(oidPath) + o, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: oid, Size: stat.Size()}, "upload") if err != nil { if isDockerConnectionError(err) { return @@ -262,7 +245,7 @@ func TestUploadWithRedirect(t *testing.T) { t.Fatal(err) } - if obj != nil { + if o != nil { t.Fatal("Received an object") } } @@ -284,7 +267,6 @@ func TestSuccessfulUploadWithVerify(t *testing.T) { defer os.RemoveAll(tmp) postCalled := false - putCalled := false verifyCalled := false mux.HandleFunc("/media/objects", func(w http.ResponseWriter, r *http.Request) { @@ -349,46 +331,6 @@ func TestSuccessfulUploadWithVerify(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/upload", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - - if r.Method != "PUT" { - w.WriteHeader(405) - return - } - - if r.Header.Get("A") != "1" { - t.Error("Invalid A") - } - - if r.Header.Get("Content-Type") != "application/octet-stream" { - t.Error("Invalid Content-Type") - } - - if r.Header.Get("Content-Length") != "4" { - t.Error("Invalid Content-Length") - } - - if r.Header.Get("Transfer-Encoding") != "" { - t.Fatal("Transfer-Encoding is set") - } - - by, err := ioutil.ReadAll(r.Body) - if err != nil { - t.Error(err) - } - - t.Logf("request header: %v", r.Header) - t.Logf("request body: %s", string(by)) - - if str := string(by); str != "test" { - t.Errorf("unexpected body: %s", str) - } - - putCalled = true - w.WriteHeader(200) - }) - mux.HandleFunc("/verify", func(w http.ResponseWriter, r *http.Request) { t.Logf("Server: %s %s", r.Method, r.URL) @@ -435,193 +377,25 @@ func TestSuccessfulUploadWithVerify(t *testing.T) { t.Fatal(err) } - // stores callbacks - calls := make([][]int64, 0, 5) - cb := func(total int64, written int64, current int) error { - calls = append(calls, []int64{total, written}) - return nil - } - - obj, err := api.UploadCheck(oidPath) + oid := filepath.Base(oidPath) + stat, _ := os.Stat(oidPath) + o, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: oid, Size: stat.Size()}, "upload") if err != nil { if isDockerConnectionError(err) { return } t.Fatal(err) } - err = uploadObject(obj, cb) - if err != nil { - t.Fatal(err) - } + api.VerifyUpload(o) if !postCalled { t.Errorf("POST not called") } - if !putCalled { - t.Errorf("PUT not called") - } - if !verifyCalled { t.Errorf("verify not called") } - t.Logf("CopyCallback: %v", calls) - - if len(calls) < 1 { - t.Errorf("CopyCallback was not used") - } - - lastCall := calls[len(calls)-1] - if lastCall[0] != 4 || lastCall[1] != 4 { - t.Errorf("Last CopyCallback call should be the total") - } -} - -func TestSuccessfulUploadWithoutVerify(t *testing.T) { - SetupTestCredentialsFunc() - repo := test.NewRepo(t) - repo.Pushd() - defer func() { - repo.Popd() - repo.Cleanup() - RestoreCredentialsFunc() - }() - - mux := http.NewServeMux() - server := httptest.NewServer(mux) - tmp := tempdir(t) - defer server.Close() - defer os.RemoveAll(tmp) - - postCalled := false - putCalled := false - - mux.HandleFunc("/media/objects", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - - if r.Method != "POST" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Errorf("Invalid Accept") - } - - if r.Header.Get("Content-Type") != api.MediaType { - t.Errorf("Invalid Content-Type") - } - - buf := &bytes.Buffer{} - tee := io.TeeReader(r.Body, buf) - reqObj := &api.ObjectResource{} - err := json.NewDecoder(tee).Decode(reqObj) - t.Logf("request header: %v", r.Header) - t.Logf("request body: %s", buf.String()) - if err != nil { - t.Fatal(err) - } - - if reqObj.Oid != "988881adc9fc3655077dc2d4d757d480b5ea0e11" { - t.Errorf("invalid oid from request: %s", reqObj.Oid) - } - - if reqObj.Size != 4 { - t.Errorf("invalid size from request: %d", reqObj.Size) - } - - obj := &api.ObjectResource{ - Oid: reqObj.Oid, - Size: reqObj.Size, - Actions: map[string]*api.LinkRelation{ - "upload": &api.LinkRelation{ - Href: server.URL + "/upload", - Header: map[string]string{"A": "1"}, - }, - }, - } - - by, err := json.Marshal(obj) - if err != nil { - t.Fatal(err) - } - - postCalled = true - head := w.Header() - head.Set("Content-Type", api.MediaType) - head.Set("Content-Length", strconv.Itoa(len(by))) - w.WriteHeader(202) - w.Write(by) - }) - - mux.HandleFunc("/upload", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - - if r.Method != "PUT" { - w.WriteHeader(405) - return - } - - if a := r.Header.Get("A"); a != "1" { - t.Errorf("Invalid A: %s", a) - } - - if r.Header.Get("Content-Type") != "application/octet-stream" { - t.Error("Invalid Content-Type") - } - - if r.Header.Get("Content-Length") != "4" { - t.Error("Invalid Content-Length") - } - - if r.Header.Get("Transfer-Encoding") != "" { - t.Fatal("Transfer-Encoding is set") - } - - by, err := ioutil.ReadAll(r.Body) - if err != nil { - t.Error(err) - } - - t.Logf("request header: %v", r.Header) - t.Logf("request body: %s", string(by)) - - if str := string(by); str != "test" { - t.Errorf("unexpected body: %s", str) - } - - putCalled = true - w.WriteHeader(200) - }) - - defer config.Config.ResetConfig() - config.Config.SetConfig("lfs.url", server.URL+"/media") - - oidPath, _ := lfs.LocalMediaPath("988881adc9fc3655077dc2d4d757d480b5ea0e11") - if err := ioutil.WriteFile(oidPath, []byte("test"), 0744); err != nil { - t.Fatal(err) - } - - obj, err := api.UploadCheck(oidPath) - if err != nil { - if isDockerConnectionError(err) { - return - } - t.Fatal(err) - } - err = uploadObject(obj, nil) - if err != nil { - t.Fatal(err) - } - - if !postCalled { - t.Errorf("POST not called") - } - - if !putCalled { - t.Errorf("PUT not called") - } } func TestUploadApiError(t *testing.T) { @@ -655,7 +429,9 @@ func TestUploadApiError(t *testing.T) { t.Fatal(err) } - _, err := api.UploadCheck(oidPath) + oid := filepath.Base(oidPath) + stat, _ := os.Stat(oidPath) + _, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: oid, Size: stat.Size()}, "upload") if err == nil { t.Fatal(err) } @@ -677,129 +453,6 @@ func TestUploadApiError(t *testing.T) { } } -func TestUploadStorageError(t *testing.T) { - SetupTestCredentialsFunc() - repo := test.NewRepo(t) - repo.Pushd() - defer func() { - repo.Popd() - repo.Cleanup() - RestoreCredentialsFunc() - }() - - mux := http.NewServeMux() - server := httptest.NewServer(mux) - tmp := tempdir(t) - defer server.Close() - defer os.RemoveAll(tmp) - - postCalled := false - putCalled := false - - mux.HandleFunc("/media/objects", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - - if r.Method != "POST" { - w.WriteHeader(405) - return - } - - if r.Header.Get("Accept") != api.MediaType { - t.Errorf("Invalid Accept") - } - - if r.Header.Get("Content-Type") != api.MediaType { - t.Errorf("Invalid Content-Type") - } - - buf := &bytes.Buffer{} - tee := io.TeeReader(r.Body, buf) - reqObj := &api.ObjectResource{} - err := json.NewDecoder(tee).Decode(reqObj) - t.Logf("request header: %v", r.Header) - t.Logf("request body: %s", buf.String()) - if err != nil { - t.Fatal(err) - } - - if reqObj.Oid != "988881adc9fc3655077dc2d4d757d480b5ea0e11" { - t.Errorf("invalid oid from request: %s", reqObj.Oid) - } - - if reqObj.Size != 4 { - t.Errorf("invalid size from request: %d", reqObj.Size) - } - - obj := &api.ObjectResource{ - Oid: reqObj.Oid, - Size: reqObj.Size, - Actions: map[string]*api.LinkRelation{ - "upload": &api.LinkRelation{ - Href: server.URL + "/upload", - Header: map[string]string{"A": "1"}, - }, - "verify": &api.LinkRelation{ - Href: server.URL + "/verify", - Header: map[string]string{"B": "2"}, - }, - }, - } - - by, err := json.Marshal(obj) - if err != nil { - t.Fatal(err) - } - - postCalled = true - head := w.Header() - head.Set("Content-Type", api.MediaType) - head.Set("Content-Length", strconv.Itoa(len(by))) - w.WriteHeader(202) - w.Write(by) - }) - - mux.HandleFunc("/upload", func(w http.ResponseWriter, r *http.Request) { - putCalled = true - w.WriteHeader(404) - }) - - defer config.Config.ResetConfig() - config.Config.SetConfig("lfs.url", server.URL+"/media") - - oidPath, _ := lfs.LocalMediaPath("988881adc9fc3655077dc2d4d757d480b5ea0e11") - if err := ioutil.WriteFile(oidPath, []byte("test"), 0744); err != nil { - t.Fatal(err) - } - - obj, err := api.UploadCheck(oidPath) - if err != nil { - if isDockerConnectionError(err) { - return - } - t.Fatal(err) - } - err = uploadObject(obj, nil) - if err == nil { - t.Fatal("Expected an error") - } - - if errutil.IsFatalError(err) { - t.Fatal("should not panic") - } - - if err.Error() != fmt.Sprintf(httputil.GetDefaultError(404), server.URL+"/upload") { - t.Fatalf("Unexpected error: %s", err.Error()) - } - - if !postCalled { - t.Errorf("POST not called") - } - - if !putCalled { - t.Errorf("PUT not called") - } -} - func TestUploadVerifyError(t *testing.T) { SetupTestCredentialsFunc() repo := test.NewRepo(t) @@ -817,7 +470,6 @@ func TestUploadVerifyError(t *testing.T) { defer os.RemoveAll(tmp) postCalled := false - putCalled := false verifyCalled := false mux.HandleFunc("/media/objects", func(w http.ResponseWriter, r *http.Request) { @@ -882,38 +534,6 @@ func TestUploadVerifyError(t *testing.T) { w.Write(by) }) - mux.HandleFunc("/upload", func(w http.ResponseWriter, r *http.Request) { - t.Logf("Server: %s %s", r.Method, r.URL) - - if r.Method != "PUT" { - w.WriteHeader(405) - return - } - - if r.Header.Get("A") != "1" { - t.Error("Invalid A") - } - - if r.Header.Get("Content-Type") != "application/octet-stream" { - t.Error("Invalid Content-Type") - } - - by, err := ioutil.ReadAll(r.Body) - if err != nil { - t.Error(err) - } - - t.Logf("request header: %v", r.Header) - t.Logf("request body: %s", string(by)) - - if str := string(by); str != "test" { - t.Errorf("unexpected body: %s", str) - } - - putCalled = true - w.WriteHeader(200) - }) - mux.HandleFunc("/verify", func(w http.ResponseWriter, r *http.Request) { verifyCalled = true w.WriteHeader(404) @@ -927,16 +547,18 @@ func TestUploadVerifyError(t *testing.T) { t.Fatal(err) } - obj, err := api.UploadCheck(oidPath) + oid := filepath.Base(oidPath) + stat, _ := os.Stat(oidPath) + o, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: oid, Size: stat.Size()}, "upload") if err != nil { if isDockerConnectionError(err) { return } t.Fatal(err) } - err = uploadObject(obj, nil) + err = api.VerifyUpload(o) if err == nil { - t.Fatal("Expected an error") + t.Fatal("verify should fail") } if errutil.IsFatalError(err) { @@ -951,34 +573,8 @@ func TestUploadVerifyError(t *testing.T) { t.Errorf("POST not called") } - if !putCalled { - t.Errorf("PUT not called") - } - if !verifyCalled { t.Errorf("verify not called") } } - -func uploadObject(o *api.ObjectResource, cb progress.CopyCallback) error { - path, err := lfs.LocalMediaPath(o.Oid) - if err != nil { - return errutil.Error(err) - } - - file, err := os.Open(path) - if err != nil { - return errutil.Error(err) - } - defer file.Close() - - reader := &progress.CallbackReader{ - C: cb, - TotalSize: o.Size, - Reader: file, - } - - return api.UploadObject(o, reader) - -} diff --git a/lfs/pointer_smudge.go b/lfs/pointer_smudge.go index fad97759..7caea2be 100644 --- a/lfs/pointer_smudge.go +++ b/lfs/pointer_smudge.go @@ -3,10 +3,11 @@ package lfs import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" + "github.com/github/git-lfs/transfer" + "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" "github.com/github/git-lfs/errutil" @@ -57,7 +58,6 @@ func PointerSmudge(writer io.Writer, ptr *Pointer, workingfile string, download if statErr != nil || stat == nil { if download { - // TODO @sinbad use adapter, use readLocalFile on completion callback err = downloadFile(writer, ptr, workingfile, mediafile, cb) } else { return errutil.NewDownloadDeclinedError(nil) @@ -73,146 +73,38 @@ func PointerSmudge(writer io.Writer, ptr *Pointer, workingfile string, download return nil } -// PointerSmudgeObject uses a Pointer and ObjectResource to download the object to the -// media directory. It does not write the file to the working directory. -func PointerSmudgeObject(ptr *Pointer, obj *api.ObjectResource, cb progress.CopyCallback) error { - mediafile, err := LocalMediaPath(obj.Oid) - if err != nil { - return err - } - - stat, statErr := os.Stat(mediafile) - if statErr == nil && stat != nil { - fileSize := stat.Size() - if fileSize == 0 || fileSize != obj.Size { - tracerx.Printf("Removing %s, size %d is invalid", mediafile, fileSize) - os.RemoveAll(mediafile) - stat = nil - } - } - - if statErr != nil || stat == nil { - // TODO @sinbad use adapter, use readLocalFile on completion callback - err := downloadObject(ptr, obj, mediafile, cb) - - if err != nil { - return errutil.NewSmudgeError(err, obj.Oid, mediafile) - } - } - - return nil -} - -// TODO @sinbad remove -func downloadObject(ptr *Pointer, obj *api.ObjectResource, mediafile string, cb progress.CopyCallback) error { - reader, size, err := api.DownloadObject(obj) - if reader != nil { - defer reader.Close() - } - - if err != nil { - return errutil.Errorf(err, "Error downloading %s", mediafile) - } - - if ptr.Size == 0 { - ptr.Size = size - } - - if err := bufferDownloadedFile(mediafile, reader, ptr.Size, cb); err != nil { - return errutil.Errorf(err, "Error buffering media file: %s", err) - } - - return nil -} - -// TODO @sinbad remove func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, cb progress.CopyCallback) error { fmt.Fprintf(os.Stderr, "Downloading %s (%s)\n", workingfile, pb.FormatBytes(ptr.Size)) - reader, size, err := api.Download(filepath.Base(mediafile), ptr.Size) - if reader != nil { - defer reader.Close() - } + obj, err := api.BatchOrLegacySingle(&api.ObjectResource{Oid: ptr.Oid, Size: ptr.Size}, "download") if err != nil { return errutil.Errorf(err, "Error downloading %s: %s", filepath.Base(mediafile), err) } if ptr.Size == 0 { - ptr.Size = size + ptr.Size = obj.Size } - if err := bufferDownloadedFile(mediafile, reader, ptr.Size, cb); err != nil { - return errutil.Errorf(err, "Error buffering media file: %s", err) + adapter := transfer.GetDownloadAdapter(transfer.BasicAdapterName) + tcb := func(name string, totalSize, readSoFar int64, readSinceLast int) error { + return cb(totalSize, readSoFar, readSinceLast) + } + // Single download + adapterResultChan := make(chan transfer.TransferResult, 1) + adapter.Begin(1, tcb, nil) + adapter.Add(transfer.NewTransfer(filepath.Base(workingfile), obj, mediafile)) + fmt.Println("xxx JUST BEFORE END") + adapter.End() + fmt.Println("xxx JUST BEFORE CHAN") + res := <-adapterResultChan + + if res.Error != nil { + return errutil.Errorf(err, "Error buffering media file: %s", res.Error) } return readLocalFile(writer, ptr, mediafile, workingfile, nil) } -// TODO @sinbad remove bufferDownloadedFile -// Writes the content of reader to filename atomically by writing to a temp file -// first, and confirming the content SHA-256 is valid. This is basically a copy -// of atomic.WriteFile() at: -// -// https://github.com/natefinch/atomic/blob/a62ce929ffcc871a51e98c6eba7b20321e3ed62d/atomic.go#L12-L17 -// -// filename - Absolute path to a file to write, with the filename a 64 character -// SHA-256 hex signature. -// reader - Any io.Reader -// size - Expected byte size of the content. Used for the progress bar in -// the optional CopyCallback. -// cb - Optional CopyCallback object for providing download progress to -// external Git LFS tools. -func bufferDownloadedFile(filename string, reader io.Reader, size int64, cb progress.CopyCallback) error { - oid := filepath.Base(filename) - f, err := ioutil.TempFile(LocalObjectTempDir(), oid+"-") - if err != nil { - return fmt.Errorf("cannot create temp file: %v", err) - } - - defer func() { - if err != nil { - // Don't leave the temp file lying around on error. - _ = os.Remove(f.Name()) // yes, ignore the error, not much we can do about it. - } - }() - - hasher := tools.NewHashingReader(reader) - - // ensure we always close f. Note that this does not conflict with the - // close below, as close is idempotent. - defer f.Close() - name := f.Name() - written, err := tools.CopyWithCallback(f, hasher, size, cb) - if err != nil { - return fmt.Errorf("cannot write data to tempfile %q: %v", name, err) - } - if err := f.Close(); err != nil { - return fmt.Errorf("can't close tempfile %q: %v", name, err) - } - - if actual := hasher.Hash(); actual != oid { - return fmt.Errorf("Expected OID %s, got %s after %d bytes written", oid, actual, written) - } - - // get the file mode from the original file and use that for the replacement - // file, too. - info, err := os.Stat(filename) - if os.IsNotExist(err) { - // no original file - } else if err != nil { - return err - } else { - if err := os.Chmod(name, info.Mode()); err != nil { - return fmt.Errorf("can't set filemode on tempfile %q: %v", name, err) - } - } - - if err := os.Rename(name, filename); err != nil { - return fmt.Errorf("cannot replace %q with tempfile %q: %v", filename, name, err) - } - return nil -} - func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile string, cb progress.CopyCallback) error { reader, err := os.Open(mediafile) if err != nil { diff --git a/lfs/upload_queue.go b/lfs/upload_queue.go index c497fdc2..31ad36b0 100644 --- a/lfs/upload_queue.go +++ b/lfs/upload_queue.go @@ -46,7 +46,7 @@ func (u *Uploadable) Path() string { // TODO remove this legacy method & only support batch func (u *Uploadable) LegacyCheck() (*api.ObjectResource, error) { - return api.UploadCheck(u.OidPath) + return api.UploadCheck(u.Oid(), u.Size()) } // NewUploadable builds the Uploadable from the given information.