From fb1d5197fa8a39cae762c43e2c72566d455794ff Mon Sep 17 00:00:00 2001
From: laser <l@s3r.me>
Date: Wed, 10 Jun 2020 09:07:47 -0700
Subject: [PATCH] pass SetConfig a mutator func, as per PR feedback

- fsLockedRepo.config gets a mutex
- add missing checkToken call to lockedMemRepo#GetStorage and lockedMemRepo#SetStorage
- LockedRepo#SetConfig accepts a mutating function as per @magik
---
 node/repo/fsrepo.go    | 30 +++++++++++++++++++++++-------
 node/repo/interface.go |  2 +-
 node/repo/memrepo.go   | 41 +++++++++++++++++++++++++++++++----------
 node/repo/repo_test.go |  7 ++++---
 4 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/node/repo/fsrepo.go b/node/repo/fsrepo.go
index a0530f0f7..b223731d9 100644
--- a/node/repo/fsrepo.go
+++ b/node/repo/fsrepo.go
@@ -230,6 +230,7 @@ type fsLockedRepo struct {
 	dsOnce sync.Once
 
 	storageLk sync.Mutex
+	configLk  sync.Mutex
 }
 
 func (fsr *fsLockedRepo) Path() string {
@@ -266,28 +267,43 @@ func (fsr *fsLockedRepo) stillValid() error {
 }
 
 func (fsr *fsLockedRepo) Config() (interface{}, error) {
-	if err := fsr.stillValid(); err != nil {
-		return nil, err
-	}
+	fsr.configLk.Lock()
+	defer fsr.configLk.Unlock()
+
+	return fsr.loadConfigFromDisk()
+}
+
+func (fsr *fsLockedRepo) loadConfigFromDisk() (interface{}, error) {
 	return config.FromFile(fsr.join(fsConfig), defConfForType(fsr.repoType))
 }
 
-func (fsr *fsLockedRepo) SetConfig(cfg interface{}) error {
+func (fsr *fsLockedRepo) SetConfig(c func(interface{})) error {
 	if err := fsr.stillValid(); err != nil {
 		return err
 	}
 
-	tmp, err := ioutil.TempFile("", "lotus-config-temp")
+	fsr.configLk.Lock()
+	defer fsr.configLk.Unlock()
+
+	cfg, err := fsr.loadConfigFromDisk()
 	if err != nil {
 		return err
 	}
 
-	err = toml.NewEncoder(tmp).Encode(cfg)
+	// mutate in-memory representation of config
+	c(cfg)
+
+	// buffer into which we write TOML bytes
+	buf := new(bytes.Buffer)
+
+	// encode now-mutated config as TOML and write to buffer
+	err = toml.NewEncoder(buf).Encode(cfg)
 	if err != nil {
 		return err
 	}
 
-	err = os.Rename(tmp.Name(), fsr.join(fsConfig))
+	// write buffer of TOML bytes to config file
+	err = ioutil.WriteFile(fsr.join(fsConfig), buf.Bytes(), 0644)
 	if err != nil {
 		return err
 	}
diff --git a/node/repo/interface.go b/node/repo/interface.go
index 560cd969c..5950f813f 100644
--- a/node/repo/interface.go
+++ b/node/repo/interface.go
@@ -38,7 +38,7 @@ type LockedRepo interface {
 
 	// Returns config in this repo
 	Config() (interface{}, error)
-	SetConfig(interface{}) error
+	SetConfig(func(interface{})) error
 
 	GetStorage() (stores.StorageConfig, error)
 	SetStorage(func(*stores.StorageConfig)) error
diff --git a/node/repo/memrepo.go b/node/repo/memrepo.go
index d942b6bd9..399b239c1 100644
--- a/node/repo/memrepo.go
+++ b/node/repo/memrepo.go
@@ -36,7 +36,10 @@ type MemRepo struct {
 	configF func(t RepoType) interface{}
 
 	// holds the current config value
-	config interface{}
+	config struct {
+		sync.Mutex
+		val interface{}
+	}
 }
 
 type lockedMemRepo struct {
@@ -50,6 +53,10 @@ type lockedMemRepo struct {
 }
 
 func (lmem *lockedMemRepo) GetStorage() (stores.StorageConfig, error) {
+	if err := lmem.checkToken(); err != nil {
+		return stores.StorageConfig{}, err
+	}
+
 	if lmem.sc == nil {
 		lmem.sc = &stores.StorageConfig{StoragePaths: []stores.LocalPath{
 			{Path: lmem.Path()},
@@ -60,6 +67,10 @@ func (lmem *lockedMemRepo) GetStorage() (stores.StorageConfig, error) {
 }
 
 func (lmem *lockedMemRepo) SetStorage(c func(*stores.StorageConfig)) error {
+	if err := lmem.checkToken(); err != nil {
+		return err
+	}
+
 	_, _ = lmem.GetStorage()
 
 	c(lmem.sc)
@@ -227,23 +238,33 @@ func (lmem *lockedMemRepo) Config() (interface{}, error) {
 		return nil, err
 	}
 
-	if lmem.mem.config == nil {
-		lmem.mem.config = lmem.mem.configF(lmem.t)
+	lmem.mem.config.Lock()
+	defer lmem.mem.config.Unlock()
+
+	if lmem.mem.config.val == nil {
+		lmem.mem.config.val = lmem.mem.configF(lmem.t)
 	}
 
-	return lmem.mem.config, nil
+	return lmem.mem.config.val, nil
 }
 
-func (lmem *lockedMemRepo) SetConfig(cfg interface{}) error {
-	lmem.mem.config = cfg
+func (lmem *lockedMemRepo) SetConfig(c func(interface{})) error {
+	if err := lmem.checkToken(); err != nil {
+		return err
+	}
+
+	lmem.mem.config.Lock()
+	defer lmem.mem.config.Unlock()
+
+	if lmem.mem.config.val == nil {
+		lmem.mem.config.val = lmem.mem.configF(lmem.t)
+	}
+
+	c(lmem.mem.config.val)
 
 	return nil
 }
 
-func (lmem *lockedMemRepo) Storage() (stores.StorageConfig, error) {
-	panic("implement me")
-}
-
 func (lmem *lockedMemRepo) SetAPIEndpoint(ma multiaddr.Multiaddr) error {
 	if err := lmem.checkToken(); err != nil {
 		return err
diff --git a/node/repo/repo_test.go b/node/repo/repo_test.go
index 5da4bcadc..444fab267 100644
--- a/node/repo/repo_test.go
+++ b/node/repo/repo_test.go
@@ -54,9 +54,10 @@ func basicTest(t *testing.T, repo Repo) {
 	assert.NoError(t, err, "config should not error")
 
 	// mutate config and persist back to repo
-	cfg1 := c1.(*config.FullNode)
-	cfg1.Client.IpfsMAddr = "duvall"
-	err = lrepo.SetConfig(cfg1)
+	err = lrepo.SetConfig(func(c interface{}) {
+		cfg := c.(*config.FullNode)
+		cfg.Client.IpfsMAddr = "duvall"
+	})
 	assert.NoError(t, err)
 
 	// load config and verify changes