storer: separate loose and packed object mgmt into optional ifaces

Suggested by mcuadros.

Author: strib

plumbing/storer/object.go

@@ -40,6 +40,26 @@ type EncodedObjectStorer interface {
 	// HasEncodedObject returns ErrObjNotFound if the object doesn't
 	// exist.  If the object does exist, it returns nil.
 	HasEncodedObject(plumbing.Hash) error
+}
+
+// DeltaObjectStorer is an EncodedObjectStorer that can return delta
+// objects.
+type DeltaObjectStorer interface {
+	// DeltaObject is the same as EncodedObject but without resolving deltas.
+	// Deltas will be returned as plumbing.DeltaObject instances.
+	DeltaObject(plumbing.ObjectType, plumbing.Hash) (plumbing.EncodedObject, error)
+}
+
+// Transactioner is a optional method for ObjectStorer, it enable transaction
+// base write and read operations in the storage
+type Transactioner interface {
+	// Begin starts a transaction.
+	Begin() Transaction
+}
+
+// LooseObjectStorer is an optional interface for managing "loose"
+// objects, i.e. those not in packfiles.
+type LooseObjectStorer interface {
 	// ForEachObjectHash iterates over all the (loose) object hashes
 	// in the repository without necessarily having to read those objects.
 	// Objects only inside pack files may be omitted.
@@ -52,6 +72,11 @@ type EncodedObjectStorer interface {
 	LooseObjectTime(plumbing.Hash) (time.Time, error)
 	// DeleteLooseObject deletes a loose object if it exists.
 	DeleteLooseObject(plumbing.Hash) error
+}
+
+// PackedObjectStorer is an optional interface for managing objects in
+// packfiles.
+type PackedObjectStorer interface {
 	// ObjectPacks returns hashes of object packs if the underlying
 	// implementation has pack files.
 	ObjectPacks() ([]plumbing.Hash, error)
@@ -60,21 +85,6 @@ type EncodedObjectStorer interface {
 	DeleteOldObjectPackAndIndex(plumbing.Hash, time.Time) error
 }
 
-// DeltaObjectStorer is an EncodedObjectStorer that can return delta
-// objects.
-type DeltaObjectStorer interface {
-	// DeltaObject is the same as EncodedObject but without resolving deltas.
-	// Deltas will be returned as plumbing.DeltaObject instances.
-	DeltaObject(plumbing.ObjectType, plumbing.Hash) (plumbing.EncodedObject, error)
-}
-
-// Transactioner is a optional method for ObjectStorer, it enable transaction
-// base write and read operations in the storage
-type Transactioner interface {
-	// Begin starts a transaction.
-	Begin() Transaction
-}
-
 // PackfileWriter is a optional method for ObjectStorer, it enable direct write
 // of packfile to the storage
 type PackfileWriter interface {

plumbing/storer/object_test.go

@@ -3,7 +3,6 @@ package storer
 import (
 	"fmt"
 	"testing"
-	"time"
 
 	. "gopkg.in/check.v1"
 	"gopkg.in/src-d/go-git.v4/plumbing"
@@ -158,23 +157,3 @@ func (o *MockObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (EncodedOb
 func (o *MockObjectStorage) Begin() Transaction {
 	return nil
 }
-
-func (o *MockObjectStorage) ForEachObjectHash(fun func(plumbing.Hash) error) error {
-	return nil
-}
-
-func (o *MockObjectStorage) LooseObjectTime(plumbing.Hash) (time.Time, error) {
-	return time.Time{}, plumbing.ErrObjectNotFound
-}
-
-func (o *MockObjectStorage) DeleteLooseObject(plumbing.Hash) error {
-	return plumbing.ErrObjectNotFound
-}
-
-func (o *MockObjectStorage) ObjectPacks() ([]plumbing.Hash, error) {
-	return nil, nil
-}
-
-func (o *MockObjectStorage) DeleteOldObjectPackAndIndex(plumbing.Hash, time.Time) error {
-	return plumbing.ErrObjectNotFound
-}

prune.go

@@ -1,9 +1,11 @@
 package git
 
 import (
+	"errors"
 	"time"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/storer"
 )
 
 type PruneHandler func(unreferencedObjectHash plumbing.Hash) error
@@ -15,20 +17,32 @@ type PruneOptions struct {
 	Handler PruneHandler
 }
 
+var ErrLooseObjectsNotSupported = errors.New("Loose objects not supported")
+
 // DeleteObject deletes an object from a repository.
 // The type conveniently matches PruneHandler.
 func (r *Repository) DeleteObject(hash plumbing.Hash) error {
-	return r.Storer.DeleteLooseObject(hash)
+	los, ok := r.Storer.(storer.LooseObjectStorer)
+	if !ok {
+		return ErrLooseObjectsNotSupported
+	}
+
+	return los.DeleteLooseObject(hash)
 }
 
 func (r *Repository) Prune(opt PruneOptions) error {
+	los, ok := r.Storer.(storer.LooseObjectStorer)
+	if !ok {
+		return ErrLooseObjectsNotSupported
+	}
+
 	pw := newObjectWalker(r.Storer)
 	err := pw.walkAllRefs()
 	if err != nil {
 		return err
 	}
 	// Now walk all (loose) objects in storage.
-	return r.Storer.ForEachObjectHash(func(hash plumbing.Hash) error {
+	return los.ForEachObjectHash(func(hash plumbing.Hash) error {
 		// Get out if we have seen this object.
 		if pw.isSeen(hash) {
 			return nil
@@ -38,7 +52,7 @@ func (r *Repository) Prune(opt PruneOptions) error {
 		if opt.OnlyObjectsOlderThan != (time.Time{}) {
 			// Errors here are non-fatal. The object may be e.g. packed.
 			// Or concurrently deleted. Skip such objects.
-			t, err := r.Storer.LooseObjectTime(hash)
+			t, err := los.LooseObjectTime(hash)
 			if err != nil {
 				return nil
 			}

repository.go

@@ -25,14 +25,15 @@ import (
 )
 
 var (
-	ErrInvalidReference        = errors.New("invalid reference, should be a tag or a branch")
-	ErrRepositoryNotExists     = errors.New("repository does not exist")
-	ErrRepositoryAlreadyExists = errors.New("repository already exists")
-	ErrRemoteNotFound          = errors.New("remote not found")
-	ErrRemoteExists            = errors.New("remote already exists	")
-	ErrWorktreeNotProvided     = errors.New("worktree should be provided")
-	ErrIsBareRepository        = errors.New("worktree not available in a bare repository")
-	ErrUnableToResolveCommit   = errors.New("unable to resolve commit")
+	ErrInvalidReference          = errors.New("invalid reference, should be a tag or a branch")
+	ErrRepositoryNotExists       = errors.New("repository does not exist")
+	ErrRepositoryAlreadyExists   = errors.New("repository already exists")
+	ErrRemoteNotFound            = errors.New("remote not found")
+	ErrRemoteExists              = errors.New("remote already exists	")
+	ErrWorktreeNotProvided       = errors.New("worktree should be provided")
+	ErrIsBareRepository          = errors.New("worktree not available in a bare repository")
+	ErrUnableToResolveCommit     = errors.New("unable to resolve commit")
+	ErrPackedObjectsNotSupported = errors.New("Packed objects not supported")
 )
 
 // Repository represents a git repository
@@ -1024,8 +1025,13 @@ type RepackConfig struct {
 }
 
 func (r *Repository) RepackObjects(cfg *RepackConfig) (err error) {
+	pos, ok := r.Storer.(storer.PackedObjectStorer)
+	if !ok {
+		return ErrPackedObjectsNotSupported
+	}
+
 	// Get the existing object packs.
-	hs, err := r.Storer.ObjectPacks()
+	hs, err := pos.ObjectPacks()
 	if err != nil {
 		return err
 	}
@@ -1042,7 +1048,7 @@ func (r *Repository) RepackObjects(cfg *RepackConfig) (err error) {
 		if h == nh {
 			continue
 		}
-		err = r.Storer.DeleteOldObjectPackAndIndex(h, cfg.OnlyDeletePacksOlderThan)
+		err = pos.DeleteOldObjectPackAndIndex(h, cfg.OnlyDeletePacksOlderThan)
 		if err != nil {
 			return err
 		}