From f6c6fa14fa39db0682b7c1a66c7ab074bfe4335f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Mur=C3=A9?= <batolettre@gmail.com>
Date: Tue, 1 Dec 2020 21:19:23 +0100
Subject: [PATCH] repo: more work towards RepoStorage

---
 go.mod                      |  1 -
 repository/git.go           | 23 +++++++++++++++++++++++
 repository/git_testing.go   |  6 ++----
 repository/gogit.go         | 33 +++++++++++++++++++++------------
 repository/gogit_test.go    |  2 +-
 repository/gogit_testing.go |  6 ++----
 repository/mock_repo.go     | 32 +++++++++++++++++++++++++++-----
 repository/repo.go          | 17 ++++++++++++++---
 repository/repo_testing.go  | 28 +++++++---------------------
 9 files changed, 97 insertions(+), 51 deletions(-)

diff --git a/go.mod b/go.mod
index e6c5b2b9..8d39746d 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,6 @@ require (
 	github.com/corpix/uarand v0.1.1 // indirect
 	github.com/dustin/go-humanize v1.0.0
 	github.com/fatih/color v1.10.0
-	github.com/go-git/go-billy v4.2.0+incompatible
 	github.com/go-git/go-billy/v5 v5.0.0
 	github.com/go-git/go-git/v5 v5.2.0
 	github.com/golang/protobuf v1.4.3 // indirect
diff --git a/repository/git.go b/repository/git.go
index 504cdd89..8c319285 100644
--- a/repository/git.go
+++ b/repository/git.go
@@ -4,10 +4,15 @@ package repository
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"sync"
 
+	"github.com/go-git/go-billy/v5"
+	"github.com/go-git/go-billy/v5/osfs"
+
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
@@ -174,6 +179,11 @@ func (repo *GitRepo) GetRemotes() (map[string]string, error) {
 	return remotes, nil
 }
 
+// LocalStorage return a billy.Filesystem giving access to $RepoPath/.git/git-bug
+func (repo *GitRepo) LocalStorage() billy.Filesystem {
+	return osfs.New(repo.path)
+}
+
 // FetchRefs fetch git refs from a remote
 func (repo *GitRepo) FetchRefs(remote, refSpec string) (string, error) {
 	stdout, err := repo.runGitCommand("fetch", remote, refSpec)
@@ -408,3 +418,16 @@ func (repo *GitRepo) AddRemote(name string, url string) error {
 
 	return err
 }
+
+// GetLocalRemote return the URL to use to add this repo as a local remote
+func (repo *GitRepo) GetLocalRemote() string {
+	return repo.path
+}
+
+// EraseFromDisk delete this repository entirely from the disk
+func (repo *GitRepo) EraseFromDisk() error {
+	path := filepath.Clean(strings.TrimSuffix(repo.path, string(filepath.Separator)+".git"))
+
+	// fmt.Println("Cleaning repo:", path)
+	return os.RemoveAll(path)
+}
diff --git a/repository/git_testing.go b/repository/git_testing.go
index 874cc86c..2168d53e 100644
--- a/repository/git_testing.go
+++ b/repository/git_testing.go
@@ -48,14 +48,12 @@ func SetupReposAndRemote() (repoA, repoB, remote TestedRepo) {
 	repoB = CreateGoGitTestRepo(false)
 	remote = CreateGoGitTestRepo(true)
 
-	remoteAddr := "file://" + remote.GetPath()
-
-	err := repoA.AddRemote("origin", remoteAddr)
+	err := repoA.AddRemote("origin", remote.GetLocalRemote())
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	err = repoB.AddRemote("origin", remoteAddr)
+	err = repoB.AddRemote("origin", remote.GetLocalRemote())
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/repository/gogit.go b/repository/gogit.go
index 45906ea4..874885db 100644
--- a/repository/gogit.go
+++ b/repository/gogit.go
@@ -13,7 +13,8 @@ import (
 	"sync"
 	"time"
 
-	"github.com/go-git/go-billy"
+	"github.com/go-git/go-billy/v5"
+	"github.com/go-git/go-billy/v5/osfs"
 	gogit "github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
@@ -24,6 +25,7 @@ import (
 )
 
 var _ ClockedRepo = &GoGitRepo{}
+var _ TestedRepo = &GoGitRepo{}
 
 type GoGitRepo struct {
 	r    *gogit.Repository
@@ -33,12 +35,6 @@ type GoGitRepo struct {
 	clocks      map[string]lamport.Clock
 
 	keyring Keyring
-	RepoStorage
-}
-
-type RepoStorage interface {
-	// Storage returns a billy.Filesystem giving access to $RepoPath/.git/git-bug
-	Storage() billy.Filesystem
 }
 
 func NewGoGitRepo(path string, clockLoaders []ClockLoader) (*GoGitRepo, error) {
@@ -202,11 +198,6 @@ func (repo *GoGitRepo) Keyring() Keyring {
 	return repo.keyring
 }
 
-// GetPath returns the path to the repo.
-func (repo *GoGitRepo) GetPath() string {
-	return repo.path
-}
-
 // GetUserName returns the name the the user has used to configure git
 func (repo *GoGitRepo) GetUserName() (string, error) {
 	return repo.AnyConfig().ReadString("user.name")
@@ -277,6 +268,11 @@ func (repo *GoGitRepo) GetRemotes() (map[string]string, error) {
 	return result, nil
 }
 
+// LocalStorage return a billy.Filesystem giving access to $RepoPath/.git/git-bug
+func (repo *GoGitRepo) LocalStorage() billy.Filesystem {
+	return osfs.New(repo.path)
+}
+
 // FetchRefs fetch git refs from a remote
 func (repo *GoGitRepo) FetchRefs(remote string, refSpec string) (string, error) {
 	buf := bytes.NewBuffer(nil)
@@ -660,3 +656,16 @@ func (repo *GoGitRepo) AddRemote(name string, url string) error {
 
 	return err
 }
+
+// GetLocalRemote return the URL to use to add this repo as a local remote
+func (repo *GoGitRepo) GetLocalRemote() string {
+	return repo.path
+}
+
+// EraseFromDisk delete this repository entirely from the disk
+func (repo *GoGitRepo) EraseFromDisk() error {
+	path := filepath.Clean(strings.TrimSuffix(repo.path, string(filepath.Separator)+".git"))
+
+	// fmt.Println("Cleaning repo:", path)
+	return os.RemoveAll(path)
+}
diff --git a/repository/gogit_test.go b/repository/gogit_test.go
index fba990d3..a1f67664 100644
--- a/repository/gogit_test.go
+++ b/repository/gogit_test.go
@@ -58,7 +58,7 @@ func TestNewGoGitRepo(t *testing.T) {
 			require.Error(t, err, i)
 		} else {
 			require.NoError(t, err, i)
-			assert.Equal(t, filepath.ToSlash(tc.outPath), filepath.ToSlash(r.GetPath()), i)
+			assert.Equal(t, filepath.ToSlash(tc.outPath), filepath.ToSlash(r.path), i)
 		}
 	}
 }
diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go
index f20ff6be..a8bff41e 100644
--- a/repository/gogit_testing.go
+++ b/repository/gogit_testing.go
@@ -42,14 +42,12 @@ func SetupGoGitReposAndRemote() (repoA, repoB, remote TestedRepo) {
 	repoB = CreateGoGitTestRepo(false)
 	remote = CreateGoGitTestRepo(true)
 
-	remoteAddr := "file://" + remote.GetPath()
-
-	err := repoA.AddRemote("origin", remoteAddr)
+	err := repoA.AddRemote("origin", remote.GetLocalRemote())
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	err = repoB.AddRemote("origin", remoteAddr)
+	err = repoB.AddRemote("origin", remote.GetLocalRemote())
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index 628939aa..74d4cae7 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -6,6 +6,8 @@ import (
 	"strings"
 
 	"github.com/99designs/keyring"
+	"github.com/go-git/go-billy/v5"
+	"github.com/go-git/go-billy/v5/memfs"
 
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
@@ -18,6 +20,7 @@ type mockRepoForTest struct {
 	*mockRepoConfig
 	*mockRepoKeyring
 	*mockRepoCommon
+	*mockRepoStorage
 	*mockRepoData
 	*mockRepoClock
 }
@@ -27,6 +30,7 @@ func NewMockRepoForTest() *mockRepoForTest {
 		mockRepoConfig:  NewMockRepoConfig(),
 		mockRepoKeyring: NewMockRepoKeyring(),
 		mockRepoCommon:  NewMockRepoCommon(),
+		mockRepoStorage: NewMockRepoStorage(),
 		mockRepoData:    NewMockRepoData(),
 		mockRepoClock:   NewMockRepoClock(),
 	}
@@ -86,11 +90,6 @@ func NewMockRepoCommon() *mockRepoCommon {
 	return &mockRepoCommon{}
 }
 
-// GetPath returns the path to the repo.
-func (r *mockRepoCommon) GetPath() string {
-	return "~/mockRepo/"
-}
-
 func (r *mockRepoCommon) GetUserName() (string, error) {
 	return "René Descartes", nil
 }
@@ -112,6 +111,20 @@ func (r *mockRepoCommon) GetRemotes() (map[string]string, error) {
 	}, nil
 }
 
+var _ RepoStorage = &mockRepoStorage{}
+
+type mockRepoStorage struct {
+	localFs billy.Filesystem
+}
+
+func NewMockRepoStorage() *mockRepoStorage {
+	return &mockRepoStorage{localFs: memfs.New()}
+}
+
+func (m *mockRepoStorage) LocalStorage() billy.Filesystem {
+	return m.localFs
+}
+
 var _ RepoData = &mockRepoData{}
 
 type commit struct {
@@ -314,6 +327,15 @@ func (r *mockRepoData) AddRemote(name string, url string) error {
 	panic("implement me")
 }
 
+func (m mockRepoForTest) GetLocalRemote() string {
+	panic("implement me")
+}
+
+func (m mockRepoForTest) EraseFromDisk() error {
+	// nothing to do
+	return nil
+}
+
 type mockRepoClock struct {
 	clocks map[string]lamport.Clock
 }
diff --git a/repository/repo.go b/repository/repo.go
index 4b45a1c5..d34995f9 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -4,6 +4,8 @@ package repository
 import (
 	"errors"
 
+	"github.com/go-git/go-billy/v5"
+
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
@@ -20,6 +22,7 @@ type Repo interface {
 	RepoKeyring
 	RepoCommon
 	RepoData
+	RepoStorage
 }
 
 // ClockedRepo is a Repo that also has Lamport clocks
@@ -48,9 +51,6 @@ type RepoKeyring interface {
 
 // RepoCommon represent the common function the we want all the repo to implement
 type RepoCommon interface {
-	// GetPath returns the path to the repo.
-	GetPath() string
-
 	// GetUserName returns the name the the user has used to configure git
 	GetUserName() (string, error)
 
@@ -64,6 +64,11 @@ type RepoCommon interface {
 	GetRemotes() (map[string]string, error)
 }
 
+type RepoStorage interface {
+	// LocalStorage return a billy.Filesystem giving access to $RepoPath/.git/git-bug
+	LocalStorage() billy.Filesystem
+}
+
 // RepoData give access to the git data storage
 type RepoData interface {
 	// FetchRefs fetch git refs from a remote
@@ -145,4 +150,10 @@ type TestedRepo interface {
 type repoTest interface {
 	// AddRemote add a new remote to the repository
 	AddRemote(name string, url string) error
+
+	// GetLocalRemote return the URL to use to add this repo as a local remote
+	GetLocalRemote() string
+
+	// EraseFromDisk delete this repository entirely from the disk
+	EraseFromDisk() error
 }
diff --git a/repository/repo_testing.go b/repository/repo_testing.go
index 41b3609e..c0e1fa79 100644
--- a/repository/repo_testing.go
+++ b/repository/repo_testing.go
@@ -3,8 +3,6 @@ package repository
 import (
 	"log"
 	"math/rand"
-	"os"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -15,25 +13,13 @@ import (
 func CleanupTestRepos(repos ...Repo) {
 	var firstErr error
 	for _, repo := range repos {
-		path := repo.GetPath()
-		if strings.HasSuffix(path, "/.git") {
-			// for a normal repository (not --bare), we want to remove everything
-			// including the parent directory where files are checked out
-			path = strings.TrimSuffix(path, "/.git")
-
-			// Testing non-bare repo should also check path is
-			// only .git (i.e. ./.git), but doing so, we should
-			// try to remove the current directory and hav some
-			// trouble. In the present case, this case should not
-			// occur.
-			// TODO consider warning or error when path == ".git"
-		}
-		// fmt.Println("Cleaning repo:", path)
-		err := os.RemoveAll(path)
-		if err != nil {
-			log.Println(err)
-			if firstErr == nil {
-				firstErr = err
+		if repo, ok := repo.(TestedRepo); ok {
+			err := repo.EraseFromDisk()
+			if err != nil {
+				log.Println(err)
+				if firstErr == nil {
+					firstErr = err
+				}
 			}
 		}
 	}
-- 
GitLab