Merge pull request #411 from MichaelMure/repo-rework

Michael MurΓ© created

Repository rework

Change summary

bridge/github/export_test.go         |   4 
bridge/github/import_test.go         |   2 
bridge/gitlab/export_test.go         |   4 
bridge/gitlab/import_test.go         |   2 
bug/bug.go                           |  27 +++
bug/bug_actions_test.go              |  16 +-
bug/clocks.go                        |  42 +++--
bug/operation_test.go                |   2 
cache/repo_cache.go                  |   6 
cache/repo_cache_test.go             |   6 
commands/root.go                     |   2 
commands/select/select_test.go       |   2 
graphql/graphql_test.go              |   2 
identity/identity.go                 |   9 +
identity/identity_actions_test.go    |   4 
identity/version.go                  |   3 
misc/random_bugs/main.go             |   4 
repository/config_mem.go             |   8 +
repository/config_mem_test.go        |   7 +
repository/config_testing.go         |  63 +++++++++
repository/git.go                    | 202 +++++++++++------------------
repository/git_config.go             |   0 
repository/git_test.go               |  61 --------
repository/git_testing.go            |  44 -----
repository/mock_repo.go              |  45 +----
repository/mock_repo_test.go         |  10 +
repository/repo.go                   |  54 +++----
repository/repo_testing.go           |  70 ++++++++++
repository/tree_entry_test.go        |   8 
termui/show_bug.go                   |   2 
tests/read_bugs_test.go              |   4 
util/lamport/clock.go                |  15 ++
util/lamport/clock_testing.go        |  28 ++++
util/lamport/lamport_test.go         |  66 ---------
util/lamport/mem_clock.go            |  39 ++--
util/lamport/mem_clock_test.go       |   8 +
util/lamport/persisted_clock.go      | 100 ++++++++++++++
util/lamport/persisted_clock_test.go |  19 ++
util/lamport/persisted_lamport.go    |  84 ------------
39 files changed, 563 insertions(+), 511 deletions(-)

Detailed changes

bridge/github/export_test.go πŸ”—

@@ -138,7 +138,7 @@ func TestPushPull(t *testing.T) {
 
 	// create repo backend
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	backend, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)
@@ -210,7 +210,7 @@ func TestPushPull(t *testing.T) {
 	fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds())
 
 	repoTwo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repoTwo)
+	defer repository.CleanupTestRepos(repoTwo)
 
 	// create a second backend
 	backendTwo, err := cache.NewRepoCache(repoTwo)

bridge/github/import_test.go πŸ”—

@@ -128,7 +128,7 @@ func Test_Importer(t *testing.T) {
 	}
 
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	backend, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)

bridge/gitlab/export_test.go πŸ”—

@@ -143,7 +143,7 @@ func TestPushPull(t *testing.T) {
 
 	// create repo backend
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	backend, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)
@@ -216,7 +216,7 @@ func TestPushPull(t *testing.T) {
 	fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds())
 
 	repoTwo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repoTwo)
+	defer repository.CleanupTestRepos(repoTwo)
 
 	// create a second backend
 	backendTwo, err := cache.NewRepoCache(repoTwo)

bridge/gitlab/import_test.go πŸ”—

@@ -77,7 +77,7 @@ func TestImport(t *testing.T) {
 	}
 
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	backend, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)

bug/bug.go πŸ”—

@@ -27,6 +27,9 @@ const createClockEntryPattern = "create-clock-%d"
 const editClockEntryPrefix = "edit-clock-"
 const editClockEntryPattern = "edit-clock-%d"
 
+const creationClockName = "bug-create"
+const editClockName = "bug-edit"
+
 var ErrBugNotExist = errors.New("bug doesn't exist")
 
 func NewErrMultipleMatchBug(matching []entity.Id) *entity.ErrMultipleMatch {
@@ -197,10 +200,18 @@ func readBug(repo repository.ClockedRepo, ref string) (*Bug, error) {
 		}
 
 		// Update the clocks
-		if err := repo.WitnessCreate(bug.createTime); err != nil {
+		createClock, err := repo.GetOrCreateClock(creationClockName)
+		if err != nil {
+			return nil, err
+		}
+		if err := createClock.Witness(bug.createTime); err != nil {
 			return nil, errors.Wrap(err, "failed to update create lamport clock")
 		}
-		if err := repo.WitnessEdit(bug.editTime); err != nil {
+		editClock, err := repo.GetOrCreateClock(editClockName)
+		if err != nil {
+			return nil, err
+		}
+		if err := editClock.Witness(bug.editTime); err != nil {
 			return nil, errors.Wrap(err, "failed to update edit lamport clock")
 		}
 
@@ -412,7 +423,11 @@ func (bug *Bug) Commit(repo repository.ClockedRepo) error {
 		return err
 	}
 
-	bug.editTime, err = repo.EditTimeIncrement()
+	editClock, err := repo.GetOrCreateClock(editClockName)
+	if err != nil {
+		return err
+	}
+	bug.editTime, err = editClock.Increment()
 	if err != nil {
 		return err
 	}
@@ -423,7 +438,11 @@ func (bug *Bug) Commit(repo repository.ClockedRepo) error {
 		Name:       fmt.Sprintf(editClockEntryPattern, bug.editTime),
 	})
 	if bug.lastCommit == "" {
-		bug.createTime, err = repo.CreateTimeIncrement()
+		createClock, err := repo.GetOrCreateClock(creationClockName)
+		if err != nil {
+			return err
+		}
+		bug.createTime, err = createClock.Increment()
 		if err != nil {
 			return err
 		}

bug/bug_actions_test.go πŸ”—

@@ -12,8 +12,8 @@ import (
 )
 
 func TestPushPull(t *testing.T) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	reneA := identity.NewIdentity("RenΓ© Descartes", "rene@descartes.fr")
 
@@ -87,8 +87,8 @@ func BenchmarkRebaseTheirs(b *testing.B) {
 }
 
 func _RebaseTheirs(t testing.TB) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	reneA := identity.NewIdentity("RenΓ© Descartes", "rene@descartes.fr")
 
@@ -165,8 +165,8 @@ func BenchmarkRebaseOurs(b *testing.B) {
 }
 
 func _RebaseOurs(t testing.TB) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	reneA := identity.NewIdentity("RenΓ© Descartes", "rene@descartes.fr")
 
@@ -254,8 +254,8 @@ func BenchmarkRebaseConflict(b *testing.B) {
 }
 
 func _RebaseConflict(t testing.TB) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	reneA := identity.NewIdentity("RenΓ© Descartes", "rene@descartes.fr")
 

bug/clocks.go πŸ”—

@@ -4,24 +4,34 @@ import (
 	"github.com/MichaelMure/git-bug/repository"
 )
 
-// Witnesser will read all the available Bug to recreate the different logical
-// clocks
-func Witnesser(repo repository.ClockedRepo) error {
-	for b := range ReadAllLocalBugs(repo) {
-		if b.Err != nil {
-			return b.Err
-		}
+// ClockLoader is the repository.ClockLoader for the Bug entity
+var ClockLoader = repository.ClockLoader{
+	Clocks: []string{creationClockName, editClockName},
+	Witnesser: func(repo repository.ClockedRepo) error {
+		for b := range ReadAllLocalBugs(repo) {
+			if b.Err != nil {
+				return b.Err
+			}
 
-		err := repo.WitnessCreate(b.Bug.createTime)
-		if err != nil {
-			return err
-		}
+			createClock, err := repo.GetOrCreateClock(creationClockName)
+			if err != nil {
+				return err
+			}
+			err = createClock.Witness(b.Bug.createTime)
+			if err != nil {
+				return err
+			}
 
-		err = repo.WitnessEdit(b.Bug.editTime)
-		if err != nil {
-			return err
+			editClock, err := repo.GetOrCreateClock(editClockName)
+			if err != nil {
+				return err
+			}
+			err = editClock.Witness(b.Bug.editTime)
+			if err != nil {
+				return err
+			}
 		}
-	}
 
-	return nil
+		return nil
+	},
 }

bug/operation_test.go πŸ”—

@@ -81,7 +81,7 @@ func TestMetadata(t *testing.T) {
 
 func TestID(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	repos := []repository.ClockedRepo{
 		repository.NewMockRepoForTest(),

cache/repo_cache.go πŸ”—

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"sort"
 	"strconv"
 	"sync"
@@ -149,6 +150,11 @@ func (c *RepoCache) lock() error {
 		return err
 	}
 
+	err = os.MkdirAll(filepath.Dir(lockPath), 0777)
+	if err != nil {
+		return err
+	}
+
 	f, err := os.Create(lockPath)
 	if err != nil {
 		return err

cache/repo_cache_test.go πŸ”—

@@ -11,7 +11,7 @@ import (
 
 func TestCache(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	cache, err := NewRepoCache(repo)
 	require.NoError(t, err)
@@ -104,8 +104,8 @@ func TestCache(t *testing.T) {
 }
 
 func TestPushPull(t *testing.T) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	cacheA, err := NewRepoCache(repoA)
 	require.NoError(t, err)

commands/root.go πŸ”—

@@ -63,7 +63,7 @@ func loadRepo(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("unable to get the current working directory: %q", err)
 	}
 
-	repo, err = repository.NewGitRepo(cwd, bug.Witnesser)
+	repo, err = repository.NewGitRepo(cwd, []repository.ClockLoader{bug.ClockLoader})
 	if err == repository.ErrNotARepo {
 		return fmt.Errorf("%s must be run from within a git repo", rootCommandName)
 	}

commands/select/select_test.go πŸ”—

@@ -12,7 +12,7 @@ import (
 
 func TestSelect(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	repoCache, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)

graphql/graphql_test.go πŸ”—

@@ -12,7 +12,7 @@ import (
 
 func TestQueries(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	random_bugs.FillRepoWithSeed(repo, 10, 42)
 

identity/identity.go πŸ”—

@@ -332,7 +332,14 @@ func (i *Identity) Commit(repo repository.ClockedRepo) error {
 		}
 
 		// get the times where new versions starts to be valid
-		v.time = repo.EditTime()
+		// TODO: instead of this hardcoded clock for bugs only, this need to be
+		// a vector of edit clock, one for each entity (bug, PR, config ..)
+		bugEditClock, err := repo.GetOrCreateClock("bug-edit")
+		if err != nil {
+			return err
+		}
+
+		v.time = bugEditClock.Time()
 		v.unixTime = time.Now().Unix()
 
 		blobHash, err := v.Write(repo)

identity/identity_actions_test.go πŸ”—

@@ -9,8 +9,8 @@ import (
 )
 
 func TestPushPull(t *testing.T) {
-	repoA, repoB, remote := repository.SetupReposAndRemote(t)
-	defer repository.CleanupTestRepos(t, repoA, repoB, remote)
+	repoA, repoB, remote := repository.SetupReposAndRemote()
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	identity1 := NewIdentity("name1", "email1")
 	err := identity1.Commit(repoA)

identity/version.go πŸ”—

@@ -20,6 +20,9 @@ type Version struct {
 	// The lamport time at which this version become effective
 	// The reference time is the bug edition lamport clock
 	// It must be the first field in this struct due to https://github.com/golang/go/issues/599
+	//
+	// TODO: BREAKING CHANGE - this need to actually be one edition lamport time **per entity**
+	// This is not a problem right now but will be when more entities are added (pull-request, config ...)
 	time     lamport.Time
 	unixTime int64
 

misc/random_bugs/main.go πŸ”—

@@ -17,9 +17,7 @@ func main() {
 		panic(err)
 	}
 
-	repo, err := repository.NewGitRepo(dir, func(repo repository.ClockedRepo) error {
-		return nil
-	})
+	repo, err := repository.NewGitRepo(dir)
 	if err != nil {
 		panic(err)
 	}

repository/config_mem.go πŸ”—

@@ -1,6 +1,7 @@
 package repository
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -77,10 +78,17 @@ func (mc *MemConfig) ReadTimestamp(key string) (time.Time, error) {
 
 // RmConfigs remove all key/value pair matching the key prefix
 func (mc *MemConfig) RemoveAll(keyPrefix string) error {
+	found := false
 	for key := range mc.config {
 		if strings.HasPrefix(key, keyPrefix) {
 			delete(mc.config, key)
+			found = true
 		}
 	}
+
+	if !found {
+		return fmt.Errorf("section not found")
+	}
+
 	return nil
 }

repository/config_testing.go πŸ”—

@@ -0,0 +1,63 @@
+package repository
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testConfig(t *testing.T, config Config) {
+	err := config.StoreString("section.key", "value")
+	assert.NoError(t, err)
+
+	val, err := config.ReadString("section.key")
+	assert.NoError(t, err)
+	assert.Equal(t, "value", val)
+
+	err = config.StoreString("section.true", "true")
+	assert.NoError(t, err)
+
+	val2, err := config.ReadBool("section.true")
+	assert.NoError(t, err)
+	assert.Equal(t, true, val2)
+
+	configs, err := config.ReadAll("section")
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"section.key":  "value",
+		"section.true": "true",
+	}, configs)
+
+	err = config.RemoveAll("section.true")
+	assert.NoError(t, err)
+
+	configs, err = config.ReadAll("section")
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"section.key": "value",
+	}, configs)
+
+	_, err = config.ReadBool("section.true")
+	assert.Equal(t, ErrNoConfigEntry, err)
+
+	err = config.RemoveAll("section.nonexistingkey")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section.key")
+	assert.NoError(t, err)
+
+	_, err = config.ReadString("section.key")
+	assert.Equal(t, ErrNoConfigEntry, err)
+
+	err = config.RemoveAll("nonexistingsection")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section")
+	assert.Error(t, err)
+
+	_, err = config.ReadString("section.key")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section.key")
+	assert.Error(t, err)
+}

repository/git.go πŸ”—

@@ -8,30 +8,25 @@ import (
 	"os/exec"
 	"path"
 	"strings"
-
-	"github.com/pkg/errors"
+	"sync"
 
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
 const (
-	createClockFile = "/git-bug/create-clock"
-	editClockFile   = "/git-bug/edit-clock"
-)
-
-var (
-	// ErrNotARepo is the error returned when the git repo root wan't be found
-	ErrNotARepo = errors.New("not a git repository")
+	clockPath = "git-bug"
 )
 
 var _ ClockedRepo = &GitRepo{}
+var _ TestedRepo = &GitRepo{}
 
 // GitRepo represents an instance of a (local) git repository.
 type GitRepo struct {
-	Path        string
-	createClock *lamport.Persisted
-	editClock   *lamport.Persisted
+	path string
+
+	clocksMutex sync.Mutex
+	clocks      map[string]lamport.Clock
 }
 
 // LocalConfig give access to the repository scoped configuration
@@ -46,19 +41,14 @@ func (repo *GitRepo) GlobalConfig() Config {
 
 // Run the given git command with the given I/O reader/writers, returning an error if it fails.
 func (repo *GitRepo) runGitCommandWithIO(stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
-	repopath := repo.Path
-	if repopath == ".git" {
-		// seeduvax> trangely the git command sometimes fail for very unknown
-		// reason wihtout this replacement.
-		// observed with rev-list command when git-bug is called from git
-		// hook script, even the same command with same args runs perfectly
-		// when called directly from the same hook script.
-		repopath = ""
-	}
-	// fmt.Printf("[%s] Running git %s\n", repopath, strings.Join(args, " "))
+	// make sure that the working directory for the command
+	// always exist, in particular when running "git init".
+	path := strings.TrimSuffix(repo.path, ".git")
+
+	// fmt.Printf("[%s] Running git %s\n", path, strings.Join(args, " "))
 
 	cmd := exec.Command("git", args...)
-	cmd.Dir = repopath
+	cmd.Dir = path
 	cmd.Stdin = stdin
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
@@ -93,8 +83,11 @@ func (repo *GitRepo) runGitCommand(args ...string) (string, error) {
 
 // NewGitRepo determines if the given working directory is inside of a git repository,
 // and returns the corresponding GitRepo instance if it is.
-func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
+func NewGitRepo(path string, clockLoaders []ClockLoader) (*GitRepo, error) {
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
+	}
 
 	// Check the repo and retrieve the root path
 	stdout, err := repo.runGitCommand("rev-parse", "--git-dir")
@@ -107,28 +100,22 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 	}
 
 	// Fix the path to be sure we are at the root
-	repo.Path = stdout
-
-	err = repo.LoadClocks()
-
-	if err != nil {
-		// No clock yet, trying to initialize them
-		err = repo.createClocks()
-		if err != nil {
-			return nil, err
+	repo.path = stdout
+
+	for _, loader := range clockLoaders {
+		allExist := true
+		for _, name := range loader.Clocks {
+			if _, err := repo.getClock(name); err != nil {
+				allExist = false
+			}
 		}
 
-		err = witnesser(repo)
-		if err != nil {
-			return nil, err
+		if !allExist {
+			err = loader.Witnesser(repo)
+			if err != nil {
+				return nil, err
+			}
 		}
-
-		err = repo.WriteClocks()
-		if err != nil {
-			return nil, err
-		}
-
-		return repo, nil
 	}
 
 	return repo, nil
@@ -136,13 +123,12 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 
 // InitGitRepo create a new empty git repo at the given path
 func InitGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path + "/.git"}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path + "/.git",
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", path)
+	_, err := repo.runGitCommand("init", path)
 	if err != nil {
 		return nil, err
 	}
@@ -152,13 +138,12 @@ func InitGitRepo(path string) (*GitRepo, error) {
 
 // InitBareGitRepo create a new --bare empty git repo at the given path
 func InitBareGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", "--bare", path)
+	_, err := repo.runGitCommand("init", "--bare", path)
 	if err != nil {
 		return nil, err
 	}
@@ -168,7 +153,7 @@ func InitBareGitRepo(path string) (*GitRepo, error) {
 
 // GetPath returns the path to the repo.
 func (repo *GitRepo) GetPath() string {
-	return repo.Path
+	return repo.path
 }
 
 // GetUserName returns the name the the user has used to configure git
@@ -385,93 +370,56 @@ func (repo *GitRepo) GetTreeHash(commit git.Hash) (git.Hash, error) {
 	return git.Hash(stdout), nil
 }
 
-// AddRemote add a new remote to the repository
-// Not in the interface because it's only used for testing
-func (repo *GitRepo) AddRemote(name string, url string) error {
-	_, err := repo.runGitCommand("remote", "add", name, url)
-
-	return err
-}
-
-func (repo *GitRepo) createClocks() error {
-	createPath := path.Join(repo.Path, createClockFile)
-	createClock, err := lamport.NewPersisted(createPath)
-	if err != nil {
-		return err
+// GetOrCreateClock return a Lamport clock stored in the Repo.
+// If the clock doesn't exist, it's created.
+func (repo *GitRepo) GetOrCreateClock(name string) (lamport.Clock, error) {
+	c, err := repo.getClock(name)
+	if err == nil {
+		return c, nil
 	}
-
-	editPath := path.Join(repo.Path, editClockFile)
-	editClock, err := lamport.NewPersisted(editPath)
-	if err != nil {
-		return err
+	if err != ErrClockNotExist {
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	return nil
-}
-
-// LoadClocks read the clocks values from the on-disk repo
-func (repo *GitRepo) LoadClocks() error {
-	createClock, err := lamport.LoadPersisted(repo.GetPath() + createClockFile)
-	if err != nil {
-		return err
-	}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-	editClock, err := lamport.LoadPersisted(repo.GetPath() + editClockFile)
+	c, err = lamport.NewPersistedClock(p)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
-	return nil
+	repo.clocks[name] = c
+	return c, nil
 }
 
-// WriteClocks write the clocks values into the repo
-func (repo *GitRepo) WriteClocks() error {
-	err := repo.createClock.Write()
-	if err != nil {
-		return err
-	}
+func (repo *GitRepo) getClock(name string) (lamport.Clock, error) {
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	err = repo.editClock.Write()
-	if err != nil {
-		return err
+	if c, ok := repo.clocks[name]; ok {
+		return c, nil
 	}
 
-	return nil
-}
-
-// CreateTime return the current value of the creation clock
-func (repo *GitRepo) CreateTime() lamport.Time {
-	return repo.createClock.Time()
-}
-
-// CreateTimeIncrement increment the creation clock and return the new value.
-func (repo *GitRepo) CreateTimeIncrement() (lamport.Time, error) {
-	return repo.createClock.Increment()
-}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-// EditTime return the current value of the edit clock
-func (repo *GitRepo) EditTime() lamport.Time {
-	return repo.editClock.Time()
-}
-
-// EditTimeIncrement increment the edit clock and return the new value.
-func (repo *GitRepo) EditTimeIncrement() (lamport.Time, error) {
-	return repo.editClock.Increment()
+	c, err := lamport.LoadPersistedClock(p)
+	if err == nil {
+		repo.clocks[name] = c
+		return c, nil
+	}
+	if err == lamport.ErrClockNotExist {
+		return nil, ErrClockNotExist
+	}
+	return nil, err
 }
 
-// WitnessCreate witness another create time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessCreate(time lamport.Time) error {
-	return repo.createClock.Witness(time)
-}
+// AddRemote add a new remote to the repository
+// Not in the interface because it's only used for testing
+func (repo *GitRepo) AddRemote(name string, url string) error {
+	_, err := repo.runGitCommand("remote", "add", name, url)
 
-// WitnessEdit witness another edition time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessEdit(time lamport.Time) error {
-	return repo.editClock.Witness(time)
+	return err
 }

repository/git_test.go πŸ”—

@@ -3,65 +3,8 @@ package repository
 
 import (
 	"testing"
-
-	"github.com/stretchr/testify/assert"
 )
 
-func TestConfig(t *testing.T) {
-	repo := CreateTestRepo(false)
-	defer CleanupTestRepos(t, repo)
-
-	err := repo.LocalConfig().StoreString("section.key", "value")
-	assert.NoError(t, err)
-
-	val, err := repo.LocalConfig().ReadString("section.key")
-	assert.NoError(t, err)
-	assert.Equal(t, "value", val)
-
-	err = repo.LocalConfig().StoreString("section.true", "true")
-	assert.NoError(t, err)
-
-	val2, err := repo.LocalConfig().ReadBool("section.true")
-	assert.NoError(t, err)
-	assert.Equal(t, true, val2)
-
-	configs, err := repo.LocalConfig().ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, configs, map[string]string{
-		"section.key":  "value",
-		"section.true": "true",
-	})
-
-	err = repo.LocalConfig().RemoveAll("section.true")
-	assert.NoError(t, err)
-
-	configs, err = repo.LocalConfig().ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, configs, map[string]string{
-		"section.key": "value",
-	})
-
-	_, err = repo.LocalConfig().ReadBool("section.true")
-	assert.Equal(t, ErrNoConfigEntry, err)
-
-	err = repo.LocalConfig().RemoveAll("section.nonexistingkey")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section.key")
-	assert.NoError(t, err)
-
-	_, err = repo.LocalConfig().ReadString("section.key")
-	assert.Equal(t, ErrNoConfigEntry, err)
-
-	err = repo.LocalConfig().RemoveAll("nonexistingsection")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section")
-	assert.Error(t, err)
-
-	_, err = repo.LocalConfig().ReadString("section.key")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section.key")
-	assert.Error(t, err)
+func TestGitRepo(t *testing.T) {
+	RepoTest(t, CreateTestRepo, CleanupTestRepos)
 }

repository/git_testing.go πŸ”—

@@ -3,21 +3,16 @@ package repository
 import (
 	"io/ioutil"
 	"log"
-	"os"
-	"strings"
-	"testing"
 )
 
 // This is intended for testing only
 
-func CreateTestRepo(bare bool) *GitRepo {
+func CreateTestRepo(bare bool) TestedRepo {
 	dir, err := ioutil.TempDir("", "")
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	// fmt.Println("Creating repo:", dir)
-
 	var creator func(string) (*GitRepo, error)
 
 	if bare {
@@ -42,38 +37,7 @@ func CreateTestRepo(bare bool) *GitRepo {
 	return repo
 }
 
-func CleanupTestRepos(t testing.TB, 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 firstErr != nil {
-		t.Fatal(firstErr)
-	}
-}
-
-func SetupReposAndRemote(t testing.TB) (repoA, repoB, remote *GitRepo) {
+func SetupReposAndRemote() (repoA, repoB, remote TestedRepo) {
 	repoA = CreateTestRepo(false)
 	repoB = CreateTestRepo(false)
 	remote = CreateTestRepo(true)
@@ -82,12 +46,12 @@ func SetupReposAndRemote(t testing.TB) (repoA, repoB, remote *GitRepo) {
 
 	err := repoA.AddRemote("origin", remoteAddr)
 	if err != nil {
-		t.Fatal(err)
+		log.Fatal(err)
 	}
 
 	err = repoB.AddRemote("origin", remoteAddr)
 	if err != nil {
-		t.Fatal(err)
+		log.Fatal(err)
 	}
 
 	return repoA, repoB, remote

repository/mock_repo.go πŸ”—

@@ -9,6 +9,7 @@ import (
 )
 
 var _ ClockedRepo = &mockRepoForTest{}
+var _ TestedRepo = &mockRepoForTest{}
 
 // mockRepoForTest defines an instance of Repo that can be used for testing.
 type mockRepoForTest struct {
@@ -18,8 +19,7 @@ type mockRepoForTest struct {
 	trees        map[git.Hash]string
 	commits      map[git.Hash]commit
 	refs         map[string]git.Hash
-	createClock  lamport.Clock
-	editClock    lamport.Clock
+	clocks       map[string]lamport.Clock
 }
 
 type commit struct {
@@ -35,8 +35,7 @@ func NewMockRepoForTest() *mockRepoForTest {
 		trees:        make(map[git.Hash]string),
 		commits:      make(map[git.Hash]commit),
 		refs:         make(map[string]git.Hash),
-		createClock:  lamport.NewClock(),
-		editClock:    lamport.NewClock(),
+		clocks:       make(map[string]lamport.Clock),
 	}
 }
 
@@ -213,36 +212,16 @@ func (r *mockRepoForTest) GetTreeHash(commit git.Hash) (git.Hash, error) {
 	panic("implement me")
 }
 
-func (r *mockRepoForTest) LoadClocks() error {
-	return nil
-}
-
-func (r *mockRepoForTest) WriteClocks() error {
-	return nil
-}
-
-func (r *mockRepoForTest) CreateTime() lamport.Time {
-	return r.createClock.Time()
-}
-
-func (r *mockRepoForTest) CreateTimeIncrement() (lamport.Time, error) {
-	return r.createClock.Increment(), nil
-}
-
-func (r *mockRepoForTest) EditTime() lamport.Time {
-	return r.editClock.Time()
-}
-
-func (r *mockRepoForTest) EditTimeIncrement() (lamport.Time, error) {
-	return r.editClock.Increment(), nil
-}
+func (r *mockRepoForTest) GetOrCreateClock(name string) (lamport.Clock, error) {
+	if c, ok := r.clocks[name]; ok {
+		return c, nil
+	}
 
-func (r *mockRepoForTest) WitnessCreate(time lamport.Time) error {
-	r.createClock.Witness(time)
-	return nil
+	c := lamport.NewMemClock()
+	r.clocks[name] = c
+	return c, nil
 }
 
-func (r *mockRepoForTest) WitnessEdit(time lamport.Time) error {
-	r.editClock.Witness(time)
-	return nil
+func (r *mockRepoForTest) AddRemote(name string, url string) error {
+	panic("implement me")
 }

repository/mock_repo_test.go πŸ”—

@@ -0,0 +1,10 @@
+package repository
+
+import "testing"
+
+func TestMockRepo(t *testing.T) {
+	creator := func(bare bool) TestedRepo { return NewMockRepoForTest() }
+	cleaner := func(repos ...Repo) {}
+
+	RepoTest(t, creator, cleaner)
+}

repository/repo.go πŸ”—

@@ -13,6 +13,10 @@ import (
 var (
 	ErrNoConfigEntry       = errors.New("no config entry for the given key")
 	ErrMultipleConfigEntry = errors.New("multiple config entry for the given key")
+	// ErrNotARepo is the error returned when the git repo root wan't be found
+	ErrNotARepo = errors.New("not a git repository")
+	// ErrClockNotExist is the error returned when a clock can't be found
+	ErrClockNotExist = errors.New("clock doesn't exist")
 )
 
 // RepoConfig access the configuration of a repository
@@ -97,36 +101,22 @@ type Repo interface {
 type ClockedRepo interface {
 	Repo
 
-	// LoadClocks read the clocks values from the on-disk repo
-	LoadClocks() error
-
-	// WriteClocks write the clocks values into the repo
-	WriteClocks() error
-
-	// CreateTime return the current value of the creation clock
-	CreateTime() lamport.Time
-
-	// CreateTimeIncrement increment the creation clock and return the new value.
-	CreateTimeIncrement() (lamport.Time, error)
-
-	// EditTime return the current value of the edit clock
-	EditTime() lamport.Time
-
-	// EditTimeIncrement increment the edit clock and return the new value.
-	EditTimeIncrement() (lamport.Time, error)
-
-	// WitnessCreate witness another create time and increment the corresponding
-	// clock if needed.
-	WitnessCreate(time lamport.Time) error
-
-	// WitnessEdit witness another edition time and increment the corresponding
-	// clock if needed.
-	WitnessEdit(time lamport.Time) error
+	// GetOrCreateClock return a Lamport clock stored in the Repo.
+	// If the clock doesn't exist, it's created.
+	GetOrCreateClock(name string) (lamport.Clock, error)
 }
 
-// Witnesser is a function that will initialize the clocks of a repo
-// from scratch
-type Witnesser func(repo ClockedRepo) error
+// ClockLoader hold which logical clock need to exist for an entity and
+// how to create them if they don't.
+type ClockLoader struct {
+	// Clocks hold the name of all the clocks this loader deal with.
+	// Those clocks will be checked when the repo load. If not present or broken,
+	// Witnesser will be used to create them.
+	Clocks []string
+	// Witnesser is a function that will initialize the clocks of a repo
+	// from scratch
+	Witnesser func(repo ClockedRepo) error
+}
 
 func prepareTreeEntries(entries []TreeEntry) bytes.Buffer {
 	var buffer bytes.Buffer
@@ -158,3 +148,11 @@ func readTreeEntries(s string) ([]TreeEntry, error) {
 
 	return casted, nil
 }
+
+// TestedRepo is an extended ClockedRepo with function for testing only
+type TestedRepo interface {
+	ClockedRepo
+
+	// AddRemote add a new remote to the repository
+	AddRemote(name string, url string) error
+}

repository/repo_testing.go πŸ”—

@@ -0,0 +1,70 @@
+package repository
+
+import (
+	"log"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+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 firstErr != nil {
+		log.Fatal(firstErr)
+	}
+}
+
+type RepoCreator func(bare bool) TestedRepo
+type RepoCleaner func(repos ...Repo)
+
+// Test suite for a Repo implementation
+func RepoTest(t *testing.T, creator RepoCreator, cleaner RepoCleaner) {
+	t.Run("Read/Write data", func(t *testing.T) {
+		repo := creator(false)
+		defer cleaner(repo)
+
+		data := []byte("hello")
+
+		h, err := repo.StoreData(data)
+		require.NoError(t, err)
+		assert.NotEmpty(t, h)
+
+		read, err := repo.ReadData(h)
+		require.NoError(t, err)
+		assert.Equal(t, data, read)
+	})
+
+	t.Run("Local config", func(t *testing.T) {
+		repo := creator(false)
+		defer cleaner(repo)
+
+		testConfig(t, repo.LocalConfig())
+	})
+}

repository/tree_entry_test.go πŸ”—

@@ -3,11 +3,12 @@ package repository
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/MichaelMure/git-bug/util/git"
 )
 
 func TestTreeEntryFormat(t *testing.T) {
-
 	entries := []TreeEntry{
 		{Blob, git.Hash("a85730cf5287d40a1e32d3a671ba2296c73387cb"), "name"},
 		{Tree, git.Hash("a85730cf5287d40a1e32d3a671ba2296c73387cb"), "name"},
@@ -26,10 +27,7 @@ func TestTreeEntryParse(t *testing.T) {
 
 	for _, line := range lines {
 		_, err := ParseTreeEntry(line)
-
-		if err != nil {
-			t.Fatal(err)
-		}
+		assert.NoError(t, err)
 	}
 
 }

termui/show_bug.go πŸ”—

@@ -222,7 +222,7 @@ func (sb *showBug) renderMain(g *gocui.Gui, mainView *gocui.View) error {
 		snap.CreateTime.Format(timeLayout),
 		edited,
 	)
-	bugHeader, lines := text.Wrap(bugHeader, maxX)
+	bugHeader, lines := text.Wrap(bugHeader, maxX, text.WrapIndent("   "))
 
 	v, err := sb.createOpView(g, showBugHeaderView, x0, y0, maxX+1, lines, false)
 	if err != nil {

tests/read_bugs_test.go πŸ”—

@@ -10,7 +10,7 @@ import (
 
 func TestReadBugs(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	random_bugs.FillRepoWithSeed(repo, 15, 42)
 
@@ -24,7 +24,7 @@ func TestReadBugs(t *testing.T) {
 
 func benchmarkReadBugs(bugNumber int, t *testing.B) {
 	repo := repository.CreateTestRepo(false)
-	defer repository.CleanupTestRepos(t, repo)
+	defer repository.CleanupTestRepos(repo)
 
 	random_bugs.FillRepoWithSeed(repo, bugNumber, 42)
 	t.ResetTimer()

util/lamport/clock.go πŸ”—

@@ -0,0 +1,15 @@
+package lamport
+
+// Time is the value of a Clock.
+type Time uint64
+
+// Clock is a Lamport logical clock
+type Clock interface {
+	// Time is used to return the current value of the lamport clock
+	Time() Time
+	// Increment is used to return the value of the lamport clock and increment it afterwards
+	Increment() (Time, error)
+	// Witness is called to update our local clock if necessary after
+	// witnessing a clock value received from another process
+	Witness(time Time) error
+}

util/lamport/clock_testing.go πŸ”—

@@ -0,0 +1,28 @@
+package lamport
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testClock(t *testing.T, c Clock) {
+	assert.Equal(t, Time(1), c.Time())
+
+	val, err := c.Increment()
+	assert.NoError(t, err)
+	assert.Equal(t, Time(1), val)
+	assert.Equal(t, Time(2), c.Time())
+
+	err = c.Witness(41)
+	assert.NoError(t, err)
+	assert.Equal(t, Time(42), c.Time())
+
+	err = c.Witness(41)
+	assert.NoError(t, err)
+	assert.Equal(t, Time(42), c.Time())
+
+	err = c.Witness(30)
+	assert.NoError(t, err)
+	assert.Equal(t, Time(42), c.Time())
+}

util/lamport/lamport_test.go πŸ”—

@@ -1,66 +0,0 @@
-/*
-
-	This Source Code Form is subject to the terms of the Mozilla Public
-	License, v. 2.0. If a copy of the MPL was not distributed with this file,
-	You can obtain one at http://mozilla.org/MPL/2.0/.
-
-	Copyright (c) 2013, Armon Dadgar armon.dadgar@gmail.com
-	Copyright (c) 2013, Mitchell Hashimoto mitchell.hashimoto@gmail.com
-
-	Alternatively, the contents of this file may be used under the terms
-	of the GNU General Public License Version 3 or later, as described below:
-
-	This file is free software: you may copy, redistribute and/or modify
-	it under the terms of the GNU General Public License as published by the
-	Free Software Foundation, either version 3 of the License, or (at your
-	option) any later version.
-
-	This file is distributed in the hope that it will be useful, but
-	WITHOUT ANY WARRANTY; without even the implied warranty of
-	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
-	Public License for more details.
-
-	You should have received a copy of the GNU General Public License
-	along with this program. If not, see http://www.gnu.org/licenses/.
-
-*/
-
-package lamport
-
-import (
-	"testing"
-)
-
-func TestClock(t *testing.T) {
-	l := &Clock{}
-
-	if l.Time() != 0 {
-		t.Fatalf("bad time value")
-	}
-
-	if l.Increment() != 0 {
-		t.Fatalf("bad time value")
-	}
-
-	if l.Time() != 1 {
-		t.Fatalf("bad time value")
-	}
-
-	l.Witness(41)
-
-	if l.Time() != 42 {
-		t.Fatalf("bad time value")
-	}
-
-	l.Witness(41)
-
-	if l.Time() != 42 {
-		t.Fatalf("bad time value")
-	}
-
-	l.Witness(30)
-
-	if l.Time() != 42 {
-		t.Fatalf("bad time value")
-	}
-}

util/lamport/lamport.go β†’ util/lamport/mem_clock.go πŸ”—

@@ -31,58 +31,59 @@ import (
 	"sync/atomic"
 )
 
-// Clock is a thread safe implementation of a lamport clock. It
+var _ Clock = &MemClock{}
+
+// MemClock is a thread safe implementation of a lamport clock. It
 // uses efficient atomic operations for all of its functions, falling back
 // to a heavy lock only if there are enough CAS failures.
-type Clock struct {
+type MemClock struct {
 	counter uint64
 }
 
-// Time is the value of a Clock.
-type Time uint64
-
-// NewClock create a new clock with the value 1.
+// NewMemClock create a new clock with the value 1.
 // Value 0 is considered as invalid.
-func NewClock() Clock {
-	return Clock{
+func NewMemClock() *MemClock {
+	return &MemClock{
 		counter: 1,
 	}
 }
 
-// NewClockWithTime create a new clock with a value.
-func NewClockWithTime(time uint64) Clock {
-	return Clock{
+// NewMemClockWithTime create a new clock with a value.
+func NewMemClockWithTime(time uint64) *MemClock {
+	return &MemClock{
 		counter: time,
 	}
 }
 
 // Time is used to return the current value of the lamport clock
-func (l *Clock) Time() Time {
-	return Time(atomic.LoadUint64(&l.counter))
+func (mc *MemClock) Time() Time {
+	return Time(atomic.LoadUint64(&mc.counter))
 }
 
 // Increment is used to return the value of the lamport clock and increment it afterwards
-func (l *Clock) Increment() Time {
-	return Time(atomic.AddUint64(&l.counter, 1) - 1)
+func (mc *MemClock) Increment() (Time, error) {
+	return Time(atomic.AddUint64(&mc.counter, 1) - 1), nil
 }
 
 // Witness is called to update our local clock if necessary after
 // witnessing a clock value received from another process
-func (l *Clock) Witness(v Time) {
+func (mc *MemClock) Witness(v Time) error {
 WITNESS:
 	// If the other value is old, we do not need to do anything
-	cur := atomic.LoadUint64(&l.counter)
+	cur := atomic.LoadUint64(&mc.counter)
 	other := uint64(v)
 	if other < cur {
-		return
+		return nil
 	}
 
 	// Ensure that our local clock is at least one ahead.
-	if !atomic.CompareAndSwapUint64(&l.counter, cur, other+1) {
+	if !atomic.CompareAndSwapUint64(&mc.counter, cur, other+1) {
 		// CAS: CompareAndSwap
 		// The CAS failed, so we just retry. Eventually our CAS should
 		// succeed or a future witness will pass us by and our witness
 		// will end.
 		goto WITNESS
 	}
+
+	return nil
 }

util/lamport/persisted_clock.go πŸ”—

@@ -0,0 +1,100 @@
+package lamport
+
+import (
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+)
+
+var ErrClockNotExist = errors.New("clock doesn't exist")
+
+type PersistedClock struct {
+	*MemClock
+	filePath string
+}
+
+// NewPersistedClock create a new persisted Lamport clock
+func NewPersistedClock(filePath string) (*PersistedClock, error) {
+	clock := &PersistedClock{
+		MemClock: NewMemClock(),
+		filePath: filePath,
+	}
+
+	dir := filepath.Dir(filePath)
+	err := os.MkdirAll(dir, 0777)
+	if err != nil {
+		return nil, err
+	}
+
+	err = clock.Write()
+	if err != nil {
+		return nil, err
+	}
+
+	return clock, nil
+}
+
+// LoadPersistedClock load a persisted Lamport clock from a file
+func LoadPersistedClock(filePath string) (*PersistedClock, error) {
+	clock := &PersistedClock{
+		filePath: filePath,
+	}
+
+	err := clock.read()
+	if err != nil {
+		return nil, err
+	}
+
+	return clock, nil
+}
+
+// Increment is used to return the value of the lamport clock and increment it afterwards
+func (pc *PersistedClock) Increment() (Time, error) {
+	time, err := pc.MemClock.Increment()
+	if err != nil {
+		return 0, err
+	}
+	return time, pc.Write()
+}
+
+// Witness is called to update our local clock if necessary after
+// witnessing a clock value received from another process
+func (pc *PersistedClock) Witness(time Time) error {
+	// TODO: rework so that we write only when the clock was actually updated
+	err := pc.MemClock.Witness(time)
+	if err != nil {
+		return err
+	}
+	return pc.Write()
+}
+
+func (pc *PersistedClock) read() error {
+	content, err := ioutil.ReadFile(pc.filePath)
+	if os.IsNotExist(err) {
+		return ErrClockNotExist
+	}
+	if err != nil {
+		return err
+	}
+
+	var value uint64
+	n, err := fmt.Sscanf(string(content), "%d", &value)
+	if err != nil {
+		return err
+	}
+
+	if n != 1 {
+		return fmt.Errorf("could not read the clock")
+	}
+
+	pc.MemClock = NewMemClockWithTime(value)
+
+	return nil
+}
+
+func (pc *PersistedClock) Write() error {
+	data := []byte(fmt.Sprintf("%d", pc.counter))
+	return ioutil.WriteFile(pc.filePath, data, 0644)
+}

util/lamport/persisted_clock_test.go πŸ”—

@@ -0,0 +1,19 @@
+package lamport
+
+import (
+	"io/ioutil"
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPersistedClock(t *testing.T) {
+	dir, err := ioutil.TempDir("", "")
+	require.NoError(t, err)
+
+	c, err := NewPersistedClock(path.Join(dir, "test-clock"))
+	require.NoError(t, err)
+
+	testClock(t, c)
+}

util/lamport/persisted_lamport.go πŸ”—

@@ -1,84 +0,0 @@
-package lamport
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-)
-
-type Persisted struct {
-	Clock
-	filePath string
-}
-
-// NewPersisted create a new persisted Lamport clock
-func NewPersisted(filePath string) (*Persisted, error) {
-	clock := &Persisted{
-		Clock:    NewClock(),
-		filePath: filePath,
-	}
-
-	dir := filepath.Dir(filePath)
-	err := os.MkdirAll(dir, 0777)
-	if err != nil {
-		return nil, err
-	}
-
-	return clock, nil
-}
-
-// LoadPersisted load a persisted Lamport clock from a file
-func LoadPersisted(filePath string) (*Persisted, error) {
-	clock := &Persisted{
-		filePath: filePath,
-	}
-
-	err := clock.read()
-	if err != nil {
-		return nil, err
-	}
-
-	return clock, nil
-}
-
-// Increment is used to return the value of the lamport clock and increment it afterwards
-func (c *Persisted) Increment() (Time, error) {
-	time := c.Clock.Increment()
-	return time, c.Write()
-}
-
-// Witness is called to update our local clock if necessary after
-// witnessing a clock value received from another process
-func (c *Persisted) Witness(time Time) error {
-	// TODO: rework so that we write only when the clock was actually updated
-	c.Clock.Witness(time)
-	return c.Write()
-}
-
-func (c *Persisted) read() error {
-	content, err := ioutil.ReadFile(c.filePath)
-	if err != nil {
-		return err
-	}
-
-	var value uint64
-	n, err := fmt.Sscanf(string(content), "%d", &value)
-
-	if err != nil {
-		return err
-	}
-
-	if n != 1 {
-		return fmt.Errorf("could not read the clock")
-	}
-
-	c.Clock = NewClockWithTime(value)
-
-	return nil
-}
-
-func (c *Persisted) Write() error {
-	data := []byte(fmt.Sprintf("%d", c.counter))
-	return ioutil.WriteFile(c.filePath, data, 0644)
-}