From ef70cf9dc18c6f64956b9785e49a7f0e5ac4ee14 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 22 Dec 2016 16:59:54 -0700 Subject: [PATCH] locking: use lfsapi for searches --- commands/command_lock.go | 2 +- commands/command_locks.go | 2 +- commands/command_unlock.go | 2 +- commands/commands.go | 9 +++++ locking/locks.go | 54 +++++++++++++++------------ locking/locks_test.go | 70 +++++++++++++++-------------------- test/cmd/lfstest-gitserver.go | 13 ++++++- 7 files changed, 85 insertions(+), 67 deletions(-) diff --git a/commands/command_lock.go b/commands/command_lock.go index 8ccf63e5..d3e6ab84 100644 --- a/commands/command_lock.go +++ b/commands/command_lock.go @@ -32,7 +32,7 @@ func lockCommand(cmd *cobra.Command, args []string) { cfg.CurrentRemote = lockRemote } - lockClient, err := locking.NewClient(cfg) + lockClient, err := locking.NewClient(lockRemote, APIClient(), cfg) if err != nil { Exit("Unable to create lock system: %v", err.Error()) } diff --git a/commands/command_locks.go b/commands/command_locks.go index 13fc6965..de3c49c4 100644 --- a/commands/command_locks.go +++ b/commands/command_locks.go @@ -19,7 +19,7 @@ func locksCommand(cmd *cobra.Command, args []string) { if len(lockRemote) > 0 { cfg.CurrentRemote = lockRemote } - lockClient, err := locking.NewClient(cfg) + lockClient, err := locking.NewClient(lockRemote, APIClient(), cfg) if err != nil { Exit("Unable to create lock system: %v", err.Error()) } diff --git a/commands/command_unlock.go b/commands/command_unlock.go index 505d57fa..3ad5d4be 100644 --- a/commands/command_unlock.go +++ b/commands/command_unlock.go @@ -26,7 +26,7 @@ func unlockCommand(cmd *cobra.Command, args []string) { cfg.CurrentRemote = lockRemote } - lockClient, err := locking.NewClient(cfg) + lockClient, err := locking.NewClient(lockRemote, APIClient(), cfg) if err != nil { Exit("Unable to create lock system: %v", err.Error()) } diff --git a/commands/commands.go b/commands/commands.go index cc45317c..9c9fb36b 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -16,6 +16,7 @@ import ( "github.com/git-lfs/git-lfs/filepathfilter" "github.com/git-lfs/git-lfs/git" "github.com/git-lfs/git-lfs/lfs" + "github.com/git-lfs/git-lfs/lfsapi" "github.com/git-lfs/git-lfs/progress" "github.com/git-lfs/git-lfs/tools" "github.com/git-lfs/git-lfs/tq" @@ -36,6 +37,14 @@ var ( excludeArg string ) +func APIClient() *lfsapi.Client { + c, err := lfsapi.NewClient(cfg.Os, cfg.Git) + if err != nil { + ExitWithError(err) + } + return c +} + // TransferManifest builds a tq.Manifest from the commands package global // cfg var. func TransferManifest() *tq.Manifest { diff --git a/locking/locks.go b/locking/locks.go index e1dd8ac2..9b43df09 100644 --- a/locking/locks.go +++ b/locking/locks.go @@ -1,7 +1,6 @@ package locking import ( - "errors" "fmt" "os" "path/filepath" @@ -9,7 +8,9 @@ import ( "github.com/git-lfs/git-lfs/api" "github.com/git-lfs/git-lfs/config" + "github.com/git-lfs/git-lfs/errors" "github.com/git-lfs/git-lfs/git" + "github.com/git-lfs/git-lfs/lfsapi" "github.com/git-lfs/git-lfs/tools/kv" ) @@ -24,6 +25,8 @@ var ( // Client is the main interface object for the locking package type Client struct { + Remote string + client *lockClient cfg *config.Configuration apiClient *api.Client cache *LockCache @@ -32,7 +35,7 @@ type Client struct { // NewClient creates a new locking client with the given configuration // You must call the returned object's `Close` method when you are finished with // it -func NewClient(cfg *config.Configuration) (*Client, error) { +func NewClient(remote string, lfsClient *lfsapi.Client, cfg *config.Configuration) (*Client, error) { apiClient := api.NewClient(api.NewHttpLifecycle(cfg)) @@ -46,7 +49,14 @@ func NewClient(cfg *config.Configuration) (*Client, error) { if err != nil { return nil, err } - return &Client{cfg, apiClient, cache}, nil + + return &Client{ + Remote: remote, + client: &lockClient{Client: lfsClient}, + cfg: cfg, + apiClient: apiClient, + cache: cache, + }, nil } // Close this client instance; must be called to dispose of resources @@ -152,7 +162,6 @@ func (c *Client) newLockFromApi(a api.Lock) Lock { // If limit > 0 then search stops at that number of locks // If localOnly = true, don't query the server & report only own local locks func (c *Client) SearchLocks(filter map[string]string, limit int, localOnly bool) (locks []Lock, err error) { - if localOnly { return c.searchCachedLocks(filter, limit) } else { @@ -184,38 +193,37 @@ func (c *Client) searchCachedLocks(filter map[string]string, limit int) ([]Lock, func (c *Client) searchRemoteLocks(filter map[string]string, limit int) ([]Lock, error) { locks := make([]Lock, 0, limit) - apifilters := make([]api.Filter, 0, len(filter)) + apifilters := make([]lockFilter, 0, len(filter)) for k, v := range filter { - apifilters = append(apifilters, api.Filter{k, v}) + apifilters = append(apifilters, lockFilter{Property: k, Value: v}) } - query := &api.LockSearchRequest{Filters: apifilters} + query := &lockSearchRequest{Filters: apifilters} for { - s, resp := c.apiClient.Locks.Search(query) - if _, err := c.apiClient.Do(s); err != nil { - return locks, fmt.Errorf("Error communicating with LFS API: %v", err) + list, _, err := c.client.Search(c.Remote, query) + if err != nil { + return locks, errors.Wrap(err, "locking") } - if resp.Err != "" { - return locks, fmt.Errorf("Error response from LFS API: %v", resp.Err) + if list.Err != "" { + return locks, errors.Wrap(err, "locking") } - for _, l := range resp.Locks { - locks = append(locks, c.newLockFromApi(l)) + for _, l := range list.Locks { + locks = append(locks, l) if limit > 0 && len(locks) >= limit { // Exit outer loop too return locks, nil } } - if resp.NextCursor != "" { - query.Cursor = resp.NextCursor + if list.NextCursor != "" { + query.Cursor = list.NextCursor } else { break } } return locks, nil - } // lockIdFromPath makes a call to the LFS API and resolves the ID for the locked @@ -228,21 +236,21 @@ func (c *Client) searchRemoteLocks(filter map[string]string, limit int) ([]Lock, // If the API call is successful, and only one lock matches the given filepath, // then its ID will be returned, along with a value of "nil" for the error. func (c *Client) lockIdFromPath(path string) (string, error) { - s, resp := c.apiClient.Locks.Search(&api.LockSearchRequest{ - Filters: []api.Filter{ - {"path", path}, + list, _, err := c.client.Search(c.Remote, &lockSearchRequest{ + Filters: []lockFilter{ + {Property: "path", Value: path}, }, }) - if _, err := c.apiClient.Do(s); err != nil { + if err != nil { return "", err } - switch len(resp.Locks) { + switch len(list.Locks) { case 0: return "", ErrNoMatchingLocks case 1: - return resp.Locks[0].Id, nil + return list.Locks[0].Id, nil default: return "", ErrLockAmbiguous } diff --git a/locking/locks_test.go b/locking/locks_test.go index a5f8d7f2..9c591241 100644 --- a/locking/locks_test.go +++ b/locking/locks_test.go @@ -1,55 +1,21 @@ package locking import ( - "bytes" "encoding/json" "io/ioutil" "net/http" + "net/http/httptest" "os" "sort" "testing" "time" - "github.com/git-lfs/git-lfs/api" "github.com/git-lfs/git-lfs/config" + "github.com/git-lfs/git-lfs/lfsapi" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -type TestLifecycle struct { -} - -func (l *TestLifecycle) Build(schema *api.RequestSchema) (*http.Request, error) { - return http.NewRequest("GET", "http://dummy", nil) -} - -func (l *TestLifecycle) Execute(req *http.Request, into interface{}) (api.Response, error) { - // Return test data including other users - locks := api.LockList{Locks: []api.Lock{ - api.Lock{Id: "99", Path: "folder/test3.dat", Committer: api.Committer{Name: "Alice", Email: "alice@wonderland.com"}}, - api.Lock{Id: "101", Path: "folder/test1.dat", Committer: api.Committer{Name: "Fred", Email: "fred@bloggs.com"}}, - api.Lock{Id: "102", Path: "folder/test2.dat", Committer: api.Committer{Name: "Fred", Email: "fred@bloggs.com"}}, - api.Lock{Id: "103", Path: "root.dat", Committer: api.Committer{Name: "Fred", Email: "fred@bloggs.com"}}, - api.Lock{Id: "199", Path: "other/test1.dat", Committer: api.Committer{Name: "Charles", Email: "charles@incharge.com"}}, - }} - locksJson, _ := json.Marshal(locks) - r := &http.Response{ - Status: "200 OK", - StatusCode: 200, - Proto: "HTTP/1.0", - Body: ioutil.NopCloser(bytes.NewReader(locksJson)), - } - if into != nil { - decoder := json.NewDecoder(r.Body) - if err := decoder.Decode(into); err != nil { - return nil, err - } - } - return api.WrapHttpResponse(r), nil -} -func (l *TestLifecycle) Cleanup(resp api.Response) error { - return resp.Body().Close() -} - type LocksById []Lock func (a LocksById) Len() int { return len(a) } @@ -61,17 +27,41 @@ func TestRefreshCache(t *testing.T) { oldStore := config.LocalGitStorageDir config.LocalGitStorageDir, err = ioutil.TempDir("", "testCacheLock") assert.Nil(t, err) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "GET", r.Method) + assert.Equal(t, "/api/locks", r.URL.Path) + + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(lockList{ + Locks: []Lock{ + Lock{Id: "99", Path: "folder/test3.dat", Name: "Alice", Email: "alice@wonderland.com"}, + Lock{Id: "101", Path: "folder/test1.dat", Name: "Fred", Email: "fred@bloggs.com"}, + Lock{Id: "102", Path: "folder/test2.dat", Name: "Fred", Email: "fred@bloggs.com"}, + Lock{Id: "103", Path: "root.dat", Name: "Fred", Email: "fred@bloggs.com"}, + Lock{Id: "199", Path: "other/test1.dat", Name: "Charles", Email: "charles@incharge.com"}, + }, + }) + assert.Nil(t, err) + })) + defer func() { + srv.Close() os.RemoveAll(config.LocalGitStorageDir) config.LocalGitStorageDir = oldStore }() cfg := config.NewFrom(config.Values{ Git: map[string]string{"user.name": "Fred", "user.email": "fred@bloggs.com"}}) - client, err := NewClient(cfg) + lfsclient, err := lfsapi.NewClient(nil, lfsapi.Env(map[string]string{ + "lfs.url": srv.URL + "/api", + "user.name": "Fred", + "user.email": "fred@bloggs.com", + })) + require.Nil(t, err) + + client, err := NewClient("", lfsclient, cfg) assert.Nil(t, err) - // Override api client for testing - client.apiClient = api.NewClient(&TestLifecycle{}) // Should start with no cached items locks, err := client.SearchLocks(nil, 0, true) diff --git a/test/cmd/lfstest-gitserver.go b/test/cmd/lfstest-gitserver.go index 8d15c4c1..cf666df6 100644 --- a/test/cmd/lfstest-gitserver.go +++ b/test/cmd/lfstest-gitserver.go @@ -76,6 +76,14 @@ func main() { return } + if strings.Contains(r.URL.Path, "/info/lfs/locks") { + if !skipIfBadAuth(w, r, id) { + locksHandler(w, r) + } + + return + } + if strings.Contains(r.URL.Path, "/info/lfs") { if !skipIfBadAuth(w, r, id) { lfsHandler(w, r, id) @@ -784,7 +792,9 @@ func locksHandler(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": if !lockRe.MatchString(r.URL.Path) { - http.NotFound(w, r) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"message":"unknown path: ` + r.URL.Path + `"}`)) } else { if err := r.ParseForm(); err != nil { http.Error(w, "could not parse form values", http.StatusInternalServerError) @@ -793,6 +803,7 @@ func locksHandler(w http.ResponseWriter, r *http.Request) { ll := &LockList{} locks := getLocks() + w.Header().Set("Content-Type", "application/json") if cursor := r.FormValue("cursor"); cursor != "" { lastSeen := -1