Improve CLI experience by avoiding version gaps (#204)

* improve CLI experience by avoiding version gaps

Signed-off-by: Asra Ali <[email protected]>

* fix init

Signed-off-by: Asra Ali <[email protected]>

* regenerate

Signed-off-by: Asra Ali <[email protected]>

* Address comments

Signed-off-by: Asra Ali <[email protected]>

Author: asraa

local_store.go

@@ -63,6 +63,11 @@ func (m *memoryStore) SetMeta(name string, meta json.RawMessage) error {
 	return nil
 }
 
+func (m *memoryStore) FileIsStaged(name string) bool {
+	_, ok := m.stagedMeta[name]
+	return ok
+}
+
 func (m *memoryStore) WalkStagedTargets(paths []string, targetsFn TargetsWalkFunc) error {
 	if len(paths) == 0 {
 		for path, data := range m.files {
@@ -91,6 +96,10 @@ func (m *memoryStore) Commit(consistentSnapshot bool, versions map[string]int, h
 		for _, path := range paths {
 			m.meta[path] = meta
 		}
+		// Remove from staged metadata.
+		// This will prompt incrementing version numbers again now that we've
+		// successfully committed the metadata to the local store.
+		delete(m.stagedMeta, name)
 	}
 	return nil
 }
@@ -170,6 +179,11 @@ func (f *fileSystemStore) SetMeta(name string, meta json.RawMessage) error {
 	return nil
 }
 
+func (f *fileSystemStore) FileIsStaged(name string) bool {
+	_, err := os.Stat(filepath.Join(f.stagedDir(), name))
+	return err == nil
+}
+
 func (f *fileSystemStore) createDirs() error {
 	for _, dir := range []string{"keys", "repository", "staged/targets"} {
 		if err := os.MkdirAll(filepath.Join(f.dir, dir), 0755); err != nil {

repo.go

@@ -50,7 +50,15 @@ type LocalStore interface {
 	// If paths is empty, all staged target files will be walked.
 	WalkStagedTargets(paths []string, targetsFn TargetsWalkFunc) error
 
+	// FileIsStaged determines if a metadata file is currently staged, to avoid incrementing
+	// version numbers repeatedly while staged.
+	FileIsStaged(filename string) bool
+
 	// Commit is used to publish staged files to the repository
+	//
+	// This will also reset the staged meta to signal incrementing version numbers.
+	// TUF 1.0 requires that the root metadata version numbers in the repository does not
+	// gaps. To avoid this, we will only increment the number once until we commit.
 	Commit(bool, map[string]int, map[string]data.Hashes) error
 
 	// GetSigners return a list of signers for a role.
@@ -74,11 +82,6 @@ type Repo struct {
 	meta           map[string]json.RawMessage
 	prefix         string
 	indent         string
-
-	// TUF 1.0 requires that the root metadata version numbers in the
-	// repository does not have any gaps. To avoid this, we will only
-	// increment the number once until we commit.
-	versionUpdated map[string]struct{}
 }
 
 func NewRepo(local LocalStore, hashAlgorithms ...string) (*Repo, error) {
@@ -91,7 +94,6 @@ func NewRepoIndent(local LocalStore, prefix string, indent string, hashAlgorithm
 		hashAlgorithms: hashAlgorithms,
 		prefix:         prefix,
 		indent:         indent,
-		versionUpdated: make(map[string]struct{}),
 	}
 
 	var err error
@@ -112,6 +114,8 @@ func (r *Repo) Init(consistentSnapshot bool) error {
 	}
 	root := data.NewRoot()
 	root.ConsistentSnapshot = consistentSnapshot
+	// Set root version to 1 for a new root.
+	root.Version = 1
 	err = r.setTopLevelMeta("root.json", root)
 	if err == nil {
 		fmt.Println("Repository initialized")
@@ -218,9 +222,8 @@ func (r *Repo) SetThreshold(keyRole string, t int) error {
 		return nil
 	}
 	role.Threshold = t
-	if _, ok := r.versionUpdated["root.json"]; !ok {
+	if !r.local.FileIsStaged("root.json") {
 		root.Version++
-		r.versionUpdated["root.json"] = struct{}{}
 	}
 	return r.setTopLevelMeta("root.json", root)
 }
@@ -256,7 +259,6 @@ func (r *Repo) SetTimestampVersion(v int) error {
 		return err
 	}
 	ts.Version = v
-	r.versionUpdated["timestamp.json"] = struct{}{}
 	return r.setTopLevelMeta("timestamp.json", ts)
 }
 
@@ -275,7 +277,6 @@ func (r *Repo) SetSnapshotVersion(v int) error {
 	}
 
 	s.Version = v
-	r.versionUpdated["snapshot.json"] = struct{}{}
 	return r.setTopLevelMeta("snapshot.json", s)
 }
 
@@ -401,9 +402,8 @@ func (r *Repo) AddVerificationKeyWithExpiration(keyRole string, pk *data.PublicK
 	}
 
 	root.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["root.json"]; !ok {
+	if !r.local.FileIsStaged("root.json") {
 		root.Version++
-		r.versionUpdated["root.json"] = struct{}{}
 	}
 
 	return r.setTopLevelMeta("root.json", root)
@@ -507,9 +507,8 @@ func (r *Repo) RevokeKeyWithExpires(keyRole, id string, expires time.Time) error
 		}
 	}
 	root.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["root.json"]; !ok {
+	if !r.local.FileIsStaged("root.json") {
 		root.Version++
-		r.versionUpdated["root.json"] = struct{}{}
 	}
 
 	err = r.setTopLevelMeta("root.json", root)
@@ -744,9 +743,8 @@ func (r *Repo) AddTargetsWithExpires(paths []string, custom json.RawMessage, exp
 		return err
 	}
 	t.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["targets.json"]; !ok {
+	if !r.local.FileIsStaged("targets.json") {
 		t.Version++
-		r.versionUpdated["targets.json"] = struct{}{}
 	}
 
 	err = r.setTopLevelMeta("targets.json", t)
@@ -806,9 +804,8 @@ func (r *Repo) RemoveTargetsWithExpires(paths []string, expires time.Time) error
 		}
 	}
 	t.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["targets.json"]; !ok {
+	if !r.local.FileIsStaged("targets.json") {
 		t.Version++
-		r.versionUpdated["targets.json"] = struct{}{}
 	}
 
 	err = r.setTopLevelMeta("targets.json", t)
@@ -858,9 +855,8 @@ func (r *Repo) SnapshotWithExpires(expires time.Time) error {
 		}
 	}
 	snapshot.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["snapshot.json"]; !ok {
+	if !r.local.FileIsStaged("snapshot.json") {
 		snapshot.Version++
-		r.versionUpdated["snapshot.json"] = struct{}{}
 	}
 	err = r.setTopLevelMeta("snapshot.json", snapshot)
 	if err == nil {
@@ -894,9 +890,8 @@ func (r *Repo) TimestampWithExpires(expires time.Time) error {
 		return err
 	}
 	timestamp.Expires = expires.Round(time.Second)
-	if _, ok := r.versionUpdated["timestamp.json"]; !ok {
+	if !r.local.FileIsStaged("timestamp.json") {
 		timestamp.Version++
-		r.versionUpdated["timestamp.json"] = struct{}{}
 	}
 
 	err = r.setTopLevelMeta("timestamp.json", timestamp)
@@ -1028,9 +1023,6 @@ func (r *Repo) Commit() error {
 
 	err = r.local.Commit(root.ConsistentSnapshot, versions, hashes)
 	if err == nil {
-		// We can start incrementing version numbers again now that we've
-		// successfully committed the metadata to the local store.
-		r.versionUpdated = make(map[string]struct{})
 		fmt.Println("Committed successfully")
 	}
 	return err

repo_test.go

@@ -499,7 +499,7 @@ func (rs *RepoSuite) TestAddPrivateKey(c *C) {
 	newRoot, err := r.root()
 	c.Assert(err, IsNil)
 	c.Assert(oldRoot, DeepEquals, newRoot)
-	if _, ok := r.versionUpdated["root.json"]; ok {
+	if r.local.FileIsStaged("root.json") {
 		c.Fatal("root should not be marked dirty")
 	}
 }