* deep copy of fetchers map

* use read lock
* add checks inside incremental peers test for listPeers method
* fix doc comments for ListPeers

galaxycache.go

@@ -232,7 +232,8 @@ func (universe *Universe) RemovePeers(ids ...string) error {
 	return universe.peerPicker.remove(ids...)
 }
 
-// ListPeers returns Universe's remote fetchers map keyed by peer IDs
+// ListPeers returns a map of remote fetchers keyed by Peer ID,
+// useful for testing incremental changes to galaxycache peers.
 func (universe *Universe) ListPeers() map[string]RemoteFetcher {
 	return universe.peerPicker.listPeers()
 }

peers.go

@@ -396,9 +396,15 @@ func (pp *PeerPicker) remove(ids ...string) error {
 }
 
 func (pp *PeerPicker) listPeers() map[string]RemoteFetcher {
-	pp.mu.Lock()
-	defer pp.mu.Unlock()
-	fetchers := pp.fetchers
+	pp.mu.RLock()
+	defer pp.mu.RUnlock()
+
+	// deep copy of pp.fetchers map.
+	fetchers := make(map[string]RemoteFetcher)
+	for p, f := range pp.fetchers {
+		fetchers[p] = f
+	}
+
 	return fetchers
 }
 

peers_test.go

@@ -244,6 +244,14 @@ func TestPeersIncremental(t *testing.T) {
 					fetcherURIs[f.(*TestFetcher).uri] = struct{}{}
 				}
 
+				allFetchers := u.peerPicker.listPeers()
+				allPeerIDs := make(map[string]struct{}, len(allFetchers))
+				allFetcherURIs := make(map[string]struct{}, len(allFetchers))
+				for peerID, fetcher := range allFetchers {
+					allPeerIDs[peerID] = struct{}{}
+					allFetcherURIs[fetcher.(*TestFetcher).uri] = struct{}{}
+				}
+
 				for _, expPeer := range step.expectedPeers {
 					if _, ok := allPeers[expPeer.ID]; !ok {
 						t.Errorf("missing peer %q from hashring at step %d", expPeer.ID, si)
@@ -258,6 +266,17 @@ func TestPeersIncremental(t *testing.T) {
 							expPeer.ID, expPeer.URI, si)
 					}
 					delete(fetcherURIs, expPeer.URI)
+					// Checks for peer IDs and fetchers from listPeers method.
+					if _, ok := allPeerIDs[expPeer.ID]; !ok {
+						t.Errorf("missing peer %q from copy of fetchers map keys at step %d",
+							expPeer.ID, si)
+					}
+					delete(allPeerIDs, expPeer.ID)
+					if _, ok := allFetcherURIs[expPeer.URI]; !ok {
+						t.Errorf("missing peer %q (URI %q) from copy of fetchers values at step %d",
+							expPeer.ID, expPeer.URI, si)
+					}
+					delete(allFetcherURIs, expPeer.URI)
 				}
 				if step.includeSelf {
 					if _, ok := allPeers[selfID]; !ok {
@@ -274,6 +293,14 @@ func TestPeersIncremental(t *testing.T) {
 				if len(fetcherURIs) > 0 {
 					t.Errorf("unexpected peer(s)' URI(s) in fetcher-map at step %d: %v", si, fetcherURIs)
 				}
+				// Checks for peer IDs and fetchers from listPeers method.
+				if len(allPeerIDs) > 0 {
+					t.Errorf("unexpected peer(s) in copy of fetcher-map at step %d: %v", si, allPeerIDs)
+				}
+				if len(allFetcherURIs) > 0 {
+					t.Errorf("unexpected peer(s)' URI(s) in copy of fetcher-map at step %d: %v",
+						si, allFetcherURIs)
+				}
 			}
 		})
 	}