Refactor and fix moderator panic

Author: cpacia

api/endpoints.go

@@ -258,6 +258,6 @@ func gatewayAllowedPath(path, method string) bool {
 }
 
 func blockingStartupMiddleware(i *jsonAPIHandler, w http.ResponseWriter, r *http.Request, requestFunc func(w http.ResponseWriter, r *http.Request)) {
-	<-i.node.DHT.BootstrapChan
+	i.node.Service.WaitForReady()
 	requestFunc(w, r)
 }

api/jsonapi.go

@@ -8,7 +8,6 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-
 	"gx/ipfs/QmPSQnBKM9g7BaUcZCvswUJVscQ1ipjmwxN5PXCjkp9EQ7/go-cid"
 	mh "gx/ipfs/QmPnFwZ2JXKnXgMw8CdBPxn7FWh6LLdjUjxV1fKHuJnkr8/go-multihash"
 	"gx/ipfs/QmTRhk7cgjUf2gfQ3p2M9KPECNZEW9XUrmHcFCgog4cPgB/go-libp2p-peer"
@@ -857,7 +856,8 @@ func (i *jsonAPIHandler) POSTSettings(w http.ResponseWriter, r *http.Request) {
 		i.node.BanManager.SetBlockedIds(blockedIds)
 	}
 	if settings.StoreModerators != nil {
-		go i.node.NotifyModerators(*settings.StoreModerators)
+		modsToAdd, modsToDelete := extractModeratorChanges(*settings.StoreModerators, nil)
+		go i.node.NotifyModerators(modsToAdd, modsToDelete)
 		if err := i.node.SetModeratorsOnListings(*settings.StoreModerators); err != nil {
 			ErrorResponse(w, http.StatusInternalServerError, err.Error())
 		}
@@ -895,7 +895,7 @@ func (i *jsonAPIHandler) PUTSettings(w http.ResponseWriter, r *http.Request) {
 		ErrorResponse(w, http.StatusBadRequest, err.Error())
 		return
 	}
-	_, err = i.node.Datastore.Settings().Get()
+	currentSettings, err := i.node.Datastore.Settings().Get()
 	if err != nil {
 		ErrorResponse(w, http.StatusNotFound, "Settings is not yet set. Use POST.")
 		return
@@ -912,7 +912,8 @@ func (i *jsonAPIHandler) PUTSettings(w http.ResponseWriter, r *http.Request) {
 		i.node.BanManager.SetBlockedIds(blockedIds)
 	}
 	if settings.StoreModerators != nil {
-		go i.node.NotifyModerators(*settings.StoreModerators)
+		modsToAdd, modsToDelete := extractModeratorChanges(*settings.StoreModerators, currentSettings.StoreModerators)
+		go i.node.NotifyModerators(modsToAdd, modsToDelete)
 		if err := i.node.SetModeratorsOnListings(*settings.StoreModerators); err != nil {
 			ErrorResponse(w, http.StatusInternalServerError, err.Error())
 		}
@@ -957,6 +958,11 @@ func (i *jsonAPIHandler) PATCHSettings(w http.ResponseWriter, r *http.Request) {
 		}
 		return
 	}
+	currentSettings, err := i.node.Datastore.Settings().Get()
+	if err != nil {
+		ErrorResponse(w, http.StatusNotFound, "Settings is not yet set. Use POST.")
+		return
+	}
 	if err = validateSMTPSettings(settings); err != nil {
 		ErrorResponse(w, http.StatusBadRequest, err.Error())
 		return
@@ -966,7 +972,8 @@ func (i *jsonAPIHandler) PATCHSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if settings.StoreModerators != nil {
-		go i.node.NotifyModerators(*settings.StoreModerators)
+		modsToAdd, modsToDelete := extractModeratorChanges(*settings.StoreModerators, currentSettings.StoreModerators)
+		go i.node.NotifyModerators(modsToAdd, modsToDelete)
 		if err := i.node.SetModeratorsOnListings(*settings.StoreModerators); err != nil {
 			ErrorResponse(w, http.StatusInternalServerError, err.Error())
 		}

api/utils.go

@@ -59,3 +59,31 @@ func convertOrderStates(states []int) []pb.OrderState {
 	}
 	return orderStates
 }
+
+func extractModeratorChanges(newModList []string, currentModList *[]string) (toAdd, toDelete []string) {
+	currentModMap := make(map[string]bool)
+	if currentModList != nil {
+		for _, mod := range *currentModList {
+			currentModMap[mod] = true
+		}
+	}
+
+	newModMap := make(map[string]bool)
+	for _, mod := range newModList {
+		newModMap[mod] = true
+	}
+
+	for _, mod := range newModList {
+		if !currentModMap[mod] {
+			toAdd = append(toAdd, mod)
+		}
+	}
+
+	for mod := range currentModMap {
+		if !newModMap[mod] {
+			toDelete = append(toDelete, mod)
+		}
+	}
+
+	return toAdd, toDelete
+}

api/utils_test.go

@@ -0,0 +1,25 @@
+package api
+
+import (
+	"testing"
+)
+
+func Test_extractModeratorChanges(t *testing.T) {
+	currentModList := []string{"a", "b", "c"}
+	newModList := []string{"a", "x", "c"}
+
+	toAdd, toDelete := extractModeratorChanges(newModList, &currentModList)
+	if len(toAdd) != 1 {
+		t.Errorf("Test_extractModeratorChanges returned incorrect number of additions: expected %d got %d", 1, len(toAdd))
+	}
+	if len(toDelete) != 1 {
+		t.Errorf("Test_extractModeratorChanges returned incorrect number of deletions: expected %d got %d", 1, len(toDelete))
+	}
+
+	if toAdd[0] != "x" {
+		t.Errorf("Test_extractModeratorChanges returned incorrect addition: expected a got %s", toAdd[0])
+	}
+	if toDelete[0] != "b" {
+		t.Errorf("Test_extractModeratorChanges returned incorrect deletion: expected b got %s", toDelete[0])
+	}
+}

cmd/start.go

@@ -713,8 +713,8 @@ func (x *Start) Execute(args []string) error {
 				}()
 			}
 		}
-		<-core.Node.DHT.BootstrapChan
 		core.Node.Service = service.New(core.Node, sqliteDB)
+		core.Node.Service.WaitForReady()
 
 		core.Node.StartMessageRetriever()
 		core.Node.StartPointerRepublisher()

core/moderation.go

@@ -262,32 +262,12 @@ func (n *OpenBazaarNode) SetModeratorsOnListings(moderators []string) error {
 }
 
 // NotifyModerators - notify moderators(peers)
-func (n *OpenBazaarNode) NotifyModerators(moderators []string) error {
-	settings, err := n.Datastore.Settings().Get()
-	if err != nil {
-		return err
-	}
-	currentMods := make(map[string]bool)
-	if settings.StoreModerators != nil {
-		for _, mod := range *settings.StoreModerators {
-			currentMods[mod] = true
-		}
-	}
-	var addedMods []string
-	for _, mod := range moderators {
-		if !currentMods[mod] {
-			addedMods = append(addedMods, mod)
-		} else {
-			delete(currentMods, mod)
-		}
-	}
-
-	removedMods := currentMods
-
+func (n *OpenBazaarNode) NotifyModerators(addedMods, removedMods []string) error {
+	n.Service.WaitForReady()
 	for _, mod := range addedMods {
 		go n.SendModeratorAdd(mod)
 	}
-	for mod := range removedMods {
+	for _, mod := range removedMods {
 		go n.SendModeratorRemove(mod)
 	}
 	return nil

mobile/node.go

@@ -383,8 +383,8 @@ func (n *Node) Start() error {
 				}()
 			}
 		}
-		<-n.OpenBazaarNode.DHT.BootstrapChan
 		n.OpenBazaarNode.Service = service.New(n.OpenBazaarNode, n.OpenBazaarNode.Datastore)
+		n.OpenBazaarNode.Service.WaitForReady()
 		MR := ret.NewMessageRetriever(ret.MRConfig{
 			Db:        n.OpenBazaarNode.Datastore,
 			IPFSNode:  n.OpenBazaarNode.IpfsNode,

net/networkservice.go

@@ -30,4 +30,7 @@ type NetworkService interface {
 
 	// Disconnect from the given peer
 	DisconnectFromPeer(p peer.ID) error
+
+	// Block until the service is available
+	WaitForReady()
 }

net/service/service.go

@@ -54,6 +54,10 @@ func New(node *core.OpenBazaarNode, datastore repo.Datastore) *OpenBazaarService
 	return service
 }
 
+func (service *OpenBazaarService) WaitForReady() {
+	<-service.node.DHT.BootstrapChan
+}
+
 func (service *OpenBazaarService) DisconnectFromPeer(p peer.ID) error {
 	log.Debugf("Disconnecting from %s", p.Pretty())
 	service.senderlk.Lock()