locking: "locks --verify" supports "--cached"

"locks --verify" can be combined with "--cached" option now in the same way as for "plain" remote queries. Cached data is stored in a separate file "path/to/ref/verifiable", next to the "plain" remote query cache file. This resolves issue #3252.
git-lfs · Mar 28, 2019 · ce1c1c05f5c8050d1090cfe7284520ee234bc247 · ce1c1c0
1 parent ffe6220
commit ce1c1c05f5c8050d1090cfe7284520ee234bc247
Unified Split

Showing with 157 additions and 96 deletions.

+1 −4 commands/command_locks.go

+1 −1 commands/lockverifier.go

+92 −80 locking/locks.go

+63 −11 locking/locks_test.go
diff --git a/commands/command_locks.go b/commands/command_locks.go
@@ -51,17 +51,14 @@ func locksCommand(cmd *cobra.Command, args []string) {
 		if locksCmdFlags.Local {
 			Exit("--verify option can't be combined with --local")
 		}
-  
-		if locksCmdFlags.Cached {
-  
-			Exit("--verify option can't be combined with --cached")
-  
-		}
 	}
 	var locks []locking.Lock
 	var locksOwned map[locking.Lock]bool
 	var jsonWriteFunc func(io.Writer) error
 	if locksCmdFlags.Verify {
 		var ourLocks, theirLocks []locking.Lock
-  
-		ourLocks, theirLocks, err = lockClient.SearchLocksVerifiable(locksCmdFlags.Limit)
+  
+		ourLocks, theirLocks, err = lockClient.SearchLocksVerifiable(locksCmdFlags.Limit, locksCmdFlags.Cached)
 		jsonWriteFunc = func(writer io.Writer) error {
 			return lockClient.EncodeLocksVerifiable(ourLocks, theirLocks, writer)
 		}

diff --git a/commands/lockverifier.go b/commands/lockverifier.go
@@ -56,7 +56,7 @@ func (lv *lockVerifier) Verify(ref *git.Ref) {
 	lockClient := newLockClient()
 	lockClient.RemoteRef = ref
-  
-	ours, theirs, err := lockClient.SearchLocksVerifiable(0)
+  
+	ours, theirs, err := lockClient.SearchLocksVerifiable(0, false)
 	if err != nil {
 		if errors.IsNotImplementedError(err) {
 			disableFor(lv.endpoint.Url)

diff --git a/locking/locks.go b/locking/locks.go
@@ -219,102 +219,107 @@ func (c *Client) SearchLocks(filter map[string]string, limit int, localOnly bool
 			return []Lock{}, errors.New("can't search cached locks when filter or limit is set")
 		}
-  
-		cacheFile, err := c.prepareCacheDirectory()
-  
-		if err != nil {
-  
-			return []Lock{}, err
-  
-		}
-  
-
-  
-		_, err = os.Stat(cacheFile)
-  
-		if err != nil {
-  
-			if os.IsNotExist(err) {
-  
-				return []Lock{}, errors.New("no cached locks present")
-  
-			}
-  
-
-  
-			return []Lock{}, err
-  
-		}
-  
-
-  
-		return c.readLocksFromCacheFile(cacheFile)
+  
+		locks := []Lock{}
+  
+		err := c.readLocksFromCacheFile("remote", func(decoder *json.Decoder) error {
+  
+			return decoder.Decode(&locks)
+  
+		})
+  
+		return locks, err
 	} else {
 		locks, err := c.searchRemoteLocks(filter, limit)
 		if err != nil {
 			return locks, err
 		}
 		if len(filter) == 0 && limit == 0 {
-  
-			cacheFile, err := c.prepareCacheDirectory()
-  
-			if err != nil {
-  
-				return locks, err
-  
-			}
-  
-
-  
-			err = c.writeLocksToCacheFile(cacheFile, locks)
+  
+			err = c.writeLocksToCacheFile("remote", func(writer io.Writer) error {
+  
+				return c.EncodeLocks(locks, writer)
+  
+			})
 		}
 		return locks, err
 	}
 }
-  
-func (c *Client) SearchLocksVerifiable(limit int) (ourLocks, theirLocks []Lock, err error) {
+  
+func (c *Client) SearchLocksVerifiable(limit int, cached bool) (ourLocks, theirLocks []Lock, err error) {
 	ourLocks = make([]Lock, 0, limit)
 	theirLocks = make([]Lock, 0, limit)
-  
-	var requestRef *lockRef
-  
-	if c.RemoteRef != nil {
-  
-		requestRef = &lockRef{Name: c.RemoteRef.Refspec()}
-  
-	}
+  
+	if cached {
+  
+		if limit != 0 {
+  
+			return []Lock{}, []Lock{}, errors.New("can't search cached locks when limit is set")
+  
+		}
-  
-	body := &lockVerifiableRequest{
-  
-		Ref:   requestRef,
-  
-		Limit: limit,
-  
-	}
+  
+		locks := &lockVerifiableList{}
+  
+		err := c.readLocksFromCacheFile("verifiable", func(decoder *json.Decoder) error {
+  
+			return decoder.Decode(&locks)
+  
+		})
+  
+		return locks.Ours, locks.Theirs, err
+  
+	} else {
+  
+		var requestRef *lockRef
+  
+		if c.RemoteRef != nil {
+  
+			requestRef = &lockRef{Name: c.RemoteRef.Refspec()}
+  
+		}
-  
-	c.cache.Clear()
+  
+		body := &lockVerifiableRequest{
+  
+			Ref:   requestRef,
+  
+			Limit: limit,
+  
+		}
-  
-	for {
-  
-		list, res, err := c.client.SearchVerifiable(c.Remote, body)
-  
-		if res != nil {
-  
-			switch res.StatusCode {
-  
-			case http.StatusNotFound, http.StatusNotImplemented:
-  
-				return ourLocks, theirLocks, errors.NewNotImplementedError(err)
-  
-			case http.StatusForbidden:
-  
-				return ourLocks, theirLocks, errors.NewAuthError(err)
+  
+		c.cache.Clear()
+  
+
+  
+		for {
+  
+			list, res, err := c.client.SearchVerifiable(c.Remote, body)
+  
+			if res != nil {
+  
+				switch res.StatusCode {
+  
+				case http.StatusNotFound, http.StatusNotImplemented:
+  
+					return ourLocks, theirLocks, errors.NewNotImplementedError(err)
+  
+				case http.StatusForbidden:
+  
+					return ourLocks, theirLocks, errors.NewAuthError(err)
+  
+				}
 			}
-  
-		}
-  
-		if err != nil {
-  
-			return ourLocks, theirLocks, err
-  
-		}
+  
+			if err != nil {
+  
+				return ourLocks, theirLocks, err
+  
+			}
-  
-		if list.Message != "" {
-  
-			if len(list.RequestID) > 0 {
-  
-				tracerx.Printf("Server Request ID: %s", list.RequestID)
+  
+			if list.Message != "" {
+  
+				if len(list.RequestID) > 0 {
+  
+					tracerx.Printf("Server Request ID: %s", list.RequestID)
+  
+				}
+  
+				return ourLocks, theirLocks, fmt.Errorf("Server error searching locks: %s", list.Message)
 			}
-  
-			return ourLocks, theirLocks, fmt.Errorf("Server error searching locks: %s", list.Message)
-  
-		}
-  
-		for _, l := range list.Ours {
-  
-			c.cache.Add(l)
-  
-			ourLocks = append(ourLocks, l)
-  
-			if limit > 0 && (len(ourLocks)+len(theirLocks)) >= limit {
-  
-				return ourLocks, theirLocks, nil
+  
+			for _, l := range list.Ours {
+  
+				c.cache.Add(l)
+  
+				ourLocks = append(ourLocks, l)
+  
+				if limit > 0 && (len(ourLocks)+len(theirLocks)) >= limit {
+  
+					return ourLocks, theirLocks, nil
+  
+				}
 			}
-  
-		}
-  
-		for _, l := range list.Theirs {
-  
-			c.cache.Add(l)
-  
-			theirLocks = append(theirLocks, l)
-  
-			if limit > 0 && (len(ourLocks)+len(theirLocks)) >= limit {
-  
-				return ourLocks, theirLocks, nil
+  
+			for _, l := range list.Theirs {
+  
+				c.cache.Add(l)
+  
+				theirLocks = append(theirLocks, l)
+  
+				if limit > 0 && (len(ourLocks)+len(theirLocks)) >= limit {
+  
+					return ourLocks, theirLocks, nil
+  
+				}
+  
+			}
+  
+
+  
+			if list.NextCursor != "" {
+  
+				body.Cursor = list.NextCursor
+  
+			} else {
+  
+				break
 			}
 		}
-  
-		if list.NextCursor != "" {
-  
-			body.Cursor = list.NextCursor
-  
-		} else {
-  
-			break
+  
+		if limit == 0 {
+  
+			err = c.writeLocksToCacheFile("verifiable", func(writer io.Writer) error {
+  
+				return c.EncodeLocksVerifiable(ourLocks, theirLocks, writer)
+  
+			})
 		}
-  
-	}
-  
-	return ourLocks, theirLocks, nil
+  
+		return ourLocks, theirLocks, err
+  
+	}
 }
 func (c *Client) searchLocalLocks(filter map[string]string, limit int) ([]Lock, error) {
@@ -429,7 +434,7 @@ func init() {
 	kv.RegisterTypeForStorage(&Lock{})
 }
-  
-func (c *Client) prepareCacheDirectory() (string, error) {
+  
+func (c *Client) prepareCacheDirectory(kind string) (string, error) {
 	cacheDir := filepath.Join(c.cacheDir, "locks")
 	if c.RemoteRef != nil {
 		cacheDir = filepath.Join(cacheDir, c.RemoteRef.Refspec())
@@ -449,24 +454,31 @@ func (c *Client) prepareCacheDirectory() (string, error) {
 		return cacheDir, errors.Wrap(err, "init cache directory "+cacheDir+" failed")
 	}
-  
-	return filepath.Join(cacheDir, "remote"), nil
+  
+	return filepath.Join(cacheDir, kind), nil
 }
-  
-func (c *Client) readLocksFromCacheFile(path string) ([]Lock, error) {
-  
-	file, err := os.Open(path)
+  
+func (c *Client) readLocksFromCacheFile(kind string, decoder func(*json.Decoder) error) error {
+  
+	cacheFile, err := c.prepareCacheDirectory(kind)
 	if err != nil {
-  
-		return []Lock{}, err
+  
+		return err
 	}
-  
-	defer file.Close()
+  
+	_, err = os.Stat(cacheFile)
+  
+	if err != nil {
+  
+		if os.IsNotExist(err) {
+  
+			return errors.New("no cached locks present")
+  
+		}
+  
+
+  
+		return err
+  
+	}
-  
-	locks := []Lock{}
-  
-	err = json.NewDecoder(file).Decode(&locks)
+  
+	file, err := os.Open(cacheFile)
 	if err != nil {
-  
-		return []Lock{}, err
+  
+		return err
 	}
-  
-	return locks, nil
+  
+	defer file.Close()
+  
+	return decoder(json.NewDecoder(file))
 }
 func (c *Client) EncodeLocks(locks []Lock, writer io.Writer) error {
@@ -480,19 +492,19 @@ func (c *Client) EncodeLocksVerifiable(ourLocks, theirLocks []Lock, writer io.Wr
 	})
 }
-  
-func (c *Client) writeLocksToCacheFile(path string, locks []Lock) error {
-  
-	file, err := os.Create(path)
+  
+func (c *Client) writeLocksToCacheFile(kind string, writer func(io.Writer) error) error {
+  
+	cacheFile, err := c.prepareCacheDirectory(kind)
 	if err != nil {
 		return err
 	}
-  
-	err = c.EncodeLocks(locks, file)
+  
+	file, err := os.Create(cacheFile)
 	if err != nil {
-  
-		file.Close()
 		return err
 	}
-  
-	return file.Close()
+  
+	defer file.Close()
+  
+	return writer(file)
 }
 type nilLockCacher struct{}

diff --git a/locking/locks_test.go b/locking/locks_test.go
@@ -63,7 +63,7 @@ func TestRemoteLocksWithCache(t *testing.T) {
 	assert.Nil(t, client.SetupFileCache(tempDir))
 	client.RemoteRef = &git.Ref{Name: "refs/heads/master"}
-  
-	cacheFile, err := client.prepareCacheDirectory()
+  
+	cacheFile, err := client.prepareCacheDirectory("remote")
 	assert.Nil(t, err)
 	// Cache file should not exist
@@ -174,7 +174,7 @@ func TestRefreshCache(t *testing.T) {
 	assert.Empty(t, locks)
 	client.RemoteRef = &git.Ref{Name: "refs/heads/master"}
-  
-	_, _, err = client.SearchLocksVerifiable(100)
+  
+	_, _, err = client.SearchLocksVerifiable(100, false)
 	assert.Nil(t, err)
 	locks, err = client.SearchLocks(nil, 0, true, false)
@@ -193,8 +193,15 @@ func TestRefreshCache(t *testing.T) {
 	}, locks)
 }
-  
-func TestSearchLocksVerifiable(t *testing.T) {
+  
+func TestSearchLocksVerifiableWithCache(t *testing.T) {
+  
+	var err error
+  
+	tempDir, err := ioutil.TempDir("", "testCacheLock")
+  
+	assert.Nil(t, err)
+  
+
+  
+	remoteQueries := 0
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+  
+		remoteQueries++
+  
+
 		assert.Equal(t, "POST", r.Method)
 		assert.Equal(t, "/api/locks/verify", r.URL.Path)
@@ -238,27 +245,72 @@ func TestSearchLocksVerifiable(t *testing.T) {
 	require.Nil(t, err)
 	client, err := NewClient("", lfsclient, config.New())
-  
-	assert.Nil(t, err)
+  
+	assert.Nil(t, client.SetupFileCache(tempDir))
 	client.RemoteRef = &git.Ref{Name: "refs/heads/master"}
-  
-	ourLocks, theirLocks, err := client.SearchLocksVerifiable(0)
+  
+	cacheFile, err := client.prepareCacheDirectory("verifiable")
 	assert.Nil(t, err)
+  
+	// Cache file should not exist
+  
+	fi, err := os.Stat(cacheFile)
+  
+	assert.True(t, os.IsNotExist(err))
+  
+
+  
+	// Querying non-existing cache file will report nothing
+  
+	ourLocks, theirLocks, err := client.SearchLocksVerifiable(0, true)
+  
+	assert.NotNil(t, err)
+  
+	assert.Empty(t, ourLocks)
+  
+	assert.Empty(t, theirLocks)
+  
+	assert.Equal(t, 0, remoteQueries)
+  
+
+  
+	// REMOTE QUERY: No cache file will be created when querying with a limit
+  
+	ourLocks, theirLocks, err = client.SearchLocksVerifiable(1, false)
+  
+	assert.Nil(t, err)
+  
+	// Just make sure we have have received anything, content doesn't matter
+  
+	assert.Equal(t, 1, len(ourLocks))
+  
+	assert.Equal(t, 0, len(theirLocks))
+  
+	assert.Equal(t, 1, remoteQueries)
+  
+
+  
+	fi, err = os.Stat(cacheFile)
+  
+	assert.True(t, os.IsNotExist(err))
+  
+
+  
+	// REMOTE QUERY: locks will be reported and cache file should be created
+  
+	ourLocks, theirLocks, err = client.SearchLocksVerifiable(0, false)
+  
+	assert.Nil(t, err)
+  
+	assert.Equal(t, 3, remoteQueries)
+  
+
+  
+	fi, err = os.Stat(cacheFile)
+  
+	assert.Nil(t, err)
+  
+	const size int64 = 478
+  
+	assert.Equal(t, size, fi.Size())
+  
+
 	// Need to include zero time in structure for equal to work
 	zeroTime := time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)
 	// Sort locks for stable comparison
-  
-	sort.Sort(LocksById(ourLocks))
-  
-	assert.Equal(t, []Lock{
+  
+	expectedOurLocks := []Lock{
 		Lock{Path: "folder/0/test1.dat", Id: "101", LockedAt: zeroTime},
 		Lock{Path: "folder/0/test2.dat", Id: "102", LockedAt: zeroTime},
 		Lock{Path: "folder/1/test1.dat", Id: "111", LockedAt: zeroTime},
-  
-	}, ourLocks)
+  
+	}
-  
-	sort.Sort(LocksById(theirLocks))
-  
-	assert.Equal(t, []Lock{
+  
+	expectedTheirLocks := []Lock{
 		Lock{Path: "folder/0/test3.dat", Id: "103", LockedAt: zeroTime},
 		Lock{Path: "folder/1/test2.dat", Id: "112", LockedAt: zeroTime},
 		Lock{Path: "folder/1/test3.dat", Id: "113", LockedAt: zeroTime},
-  
-	}, theirLocks)
+  
+	}
+  
+
+  
+	sort.Sort(LocksById(ourLocks))
+  
+	assert.Equal(t, expectedOurLocks, ourLocks)
+  
+	sort.Sort(LocksById(theirLocks))
+  
+	assert.Equal(t, expectedTheirLocks, theirLocks)
+  
+
+  
+	// Querying cache file should report same locks
+  
+	ourLocks, theirLocks, err = client.SearchLocksVerifiable(0, true)
+  
+	assert.Nil(t, err)
+  
+	assert.Equal(t, 3, remoteQueries)
+  
+
+  
+	sort.Sort(LocksById(ourLocks))
+  
+	assert.Equal(t, expectedOurLocks, ourLocks)
+  
+	sort.Sort(LocksById(theirLocks))
+  
+	assert.Equal(t, expectedTheirLocks, theirLocks)
 }