Merge chunks.RootTracker interface into chunks.ChunkStore (#3408)

You can't fully specify RootTracker without referring to the
ChunkStore interface, so they should just merge.

Fixes #3402

cmd/noms/noms_root.go

@@ -43,10 +43,10 @@ func runRoot(args []string) int {
 	}
 
 	cfg := config.NewResolver()
-	rt, err := cfg.GetRootTracker(args[0])
+	cs, err := cfg.GetChunkStore(args[0])
 	d.CheckErrorNoUsage(err)
 
-	currRoot := rt.Root()
+	currRoot := cs.Root()
 
 	if updateRoot == "" {
 		fmt.Println(currRoot)
@@ -62,6 +62,7 @@ func runRoot(args []string) int {
 		return 1
 	}
 
+	// If BUG 3407 is correct, we might be able to just take cs and make a Database directly from that.
 	db, err := cfg.GetDatabase(args[0])
 	d.CheckErrorNoUsage(err)
 	defer db.Close()
@@ -85,7 +86,7 @@ Continue?
 		return 0
 	}
 
-	ok = rt.Commit(h, currRoot)
+	ok = cs.Commit(h, currRoot)
 	if !ok {
 		fmt.Fprintln(os.Stderr, "Optimistic concurrency failure")
 		return 1

go/chunks/chunk_store.go

@@ -10,48 +10,9 @@ import (
 	"github.com/attic-labs/noms/go/hash"
 )
 
-// ChunkStore is the core storage abstraction in noms. We can put data anyplace we have a
-// ChunkStore implementation for.
+// ChunkStore is the core storage abstraction in noms. We can put data
+// anyplace we have a ChunkStore implementation for.
 type ChunkStore interface {
-	ChunkSource
-	RootTracker
-}
-
-// Factory allows the creation of namespaced ChunkStore instances. The details
-// of how namespaces are separated is left up to the particular implementation
-// of Factory and ChunkStore.
-type Factory interface {
-	CreateStore(ns string) ChunkStore
-
-	// Shutter shuts down the factory. Subsequent calls to CreateStore() will fail.
-	Shutter()
-}
-
-// RootTracker allows querying and management of the root of an entire tree of
-// references. The "root" is the single mutable variable in a ChunkStore. It
-// can store any hash, but it is typically used by higher layers (such as
-// Database) to store a hash to a value that represents the current state and
-// entire history of a database.
-// TODO: Does having a separate RootTracker make sense anymore? BUG 3402
-type RootTracker interface {
-	// Rebase brings this RootTracker into sync with the persistent storage's
-	// current root.
-	Rebase()
-
-	// Root returns the root of the database as of the time the RootTracker
-	// was opened or the most recent call to Rebase.
-	Root() hash.Hash
-
-	// Commit atomically attempts to persist all novel Chunks and update the
-	// persisted root hash from last to current. If last doesn't match the
-	// root in persistent storage, returns false.
-	// TODO: is last now redundant? Maybe this should just try to update from
-	// the cached root to current?
-	Commit(current, last hash.Hash) bool
-}
-
-// ChunkSource is a place chunks live.
-type ChunkSource interface {
 	// Get the Chunk for the value of the hash in the store. If the hash is
 	// absent from the store nil is returned.
 	Get(h hash.Hash) Chunk
@@ -89,5 +50,30 @@ type ChunkSource interface {
 	// call Flush() concurrently with Put() or PutMany().
 	Flush()
 
+	// Rebase brings this ChunkStore into sync with the persistent storage's
+	// current root.
+	Rebase()
+
+	// Root returns the root of the database as of the time the ChunkStore
+	// was opened or the most recent call to Rebase.
+	Root() hash.Hash
+
+	// Commit atomically attempts to persist all novel Chunks and update the
+	// persisted root hash from last to current. If last doesn't match the
+	// root in persistent storage, returns false.
+	// TODO: is last now redundant? Maybe this should just try to update from
+	// the cached root to current?
+	Commit(current, last hash.Hash) bool
+
 	io.Closer
 }
+
+// Factory allows the creation of namespaced ChunkStore instances. The details
+// of how namespaces are separated is left up to the particular implementation
+// of Factory and ChunkStore.
+type Factory interface {
+	CreateStore(ns string) ChunkStore
+
+	// Shutter shuts down the factory. Subsequent calls to CreateStore() will fail.
+	Shutter()
+}

go/config/resolver.go

@@ -104,19 +104,6 @@ func (r *Resolver) GetChunkStore(str string) (chunks.ChunkStore, error) {
 	return sp.NewChunkStore(), nil
 }
 
-// Resolve string to a RootTracker. Like ResolveDatabase, but returns a RootTracker instead
-func (r *Resolver) GetRootTracker(str string) (chunks.RootTracker, error) {
-	sp, err := spec.ForDatabase(r.verbose(str, r.ResolveDbSpec(str)))
-	if err != nil {
-		return nil, err
-	}
-	var rt chunks.RootTracker = sp.NewChunkStore()
-	if rt == nil {
-		rt = datas.NewHTTPChunkStore(sp.String(), "")
-	}
-	return rt, nil
-}
-
 // Resolve string to a dataset. If a config is present,
 //  - if no db prefix is present, assume the default db
 //  - if the db prefix is an alias, replace it

go/datas/completeness_checker.go

@@ -30,7 +30,7 @@ func (cc *completenessChecker) AddRefs(v types.Value) {
 
 // PanicIfDangling panics if any refs in unresolved point to chunks not
 // present in cs.
-func (cc *completenessChecker) PanicIfDangling(cs chunks.ChunkSource) {
+func (cc *completenessChecker) PanicIfDangling(cs chunks.ChunkStore) {
 	present := cs.HasMany(cc.unresolved)
 	absent := hash.HashSlice{}
 	for h := range cc.unresolved {

go/datas/database_common.go

@@ -17,7 +17,7 @@ import (
 type databaseCommon struct {
 	*types.ValueStore
 	cch      *cachingChunkHaver
-	rt       chunks.RootTracker
+	rt       rootTracker
 	rootHash hash.Hash
 	datasets *types.Map
 }
@@ -27,6 +27,12 @@ var (
 	ErrMergeNeeded          = errors.New("Dataset head is not ancestor of commit")
 )
 
+// rootTracker is a narrowing of the ChunkStore interface, to keep Database disciplined about working directly with Chunks
+type rootTracker interface {
+	Root() hash.Hash
+	Commit(current, last hash.Hash) bool
+}
+
 func newDatabaseCommon(cs chunks.ChunkStore) databaseCommon {
 	return databaseCommon{
 		ValueStore: types.NewValueStore(cs),

go/nbs/root_tracker_test.go

@@ -155,7 +155,7 @@ func createMemTable(chunks [][]byte) *memTable {
 	return mt
 }
 
-func assertDataInStore(slices [][]byte, store chunks.ChunkSource, assert *assert.Assertions) {
+func assertDataInStore(slices [][]byte, store chunks.ChunkStore, assert *assert.Assertions) {
 	for _, data := range slices {
 		assert.True(store.Has(chunks.NewChunk(data).Hash()))
 	}