Move peer.PrepareURL to net.SubstituteGenesisID (algorand#1796)

The `wsPeer` doesn't provide any value in performing the GenesisID substitution; it's just reading the current value from the `wsNetwork` and calling the `strings.Replace` function. Moving this function to `wsNetwork` makes it much more encapsulated and avoid unneeded referencing.
mangoplane · Jan 11, 2021 · 9a0b439 · 9a0b439
1 parent 1de697e
commit 9a0b439
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 25 deletions.
diff --git a/catchup/fetcher_test.go b/catchup/fetcher_test.go
@@ -94,9 +94,6 @@ func (client *mockRPCClient) GetAddress() string {
 func (client *mockRPCClient) GetHTTPClient() *http.Client {
 	return nil
 }
-func (client *mockRPCClient) PrepareURL(x string) string {
-	return strings.Replace(x, "{genesisID}", "test genesisID", -1)
-}
 
 type mockClientAggregator struct {
 	mocks.MockNetwork
@@ -535,6 +532,10 @@ func (b *basicRPCNode) GetPeers(options ...network.PeerOption) []network.Peer {
 	return b.peers
 }
 
+func (b *basicRPCNode) SubstituteGenesisID(rawURL string) string {
+	return strings.Replace(rawURL, "{genesisID}", "test genesisID", -1)
+}
+
 type httpTestPeerSource struct {
 	peers []network.Peer
 	mocks.MockNetwork
@@ -549,15 +550,16 @@ func (s *httpTestPeerSource) RegisterHandlers(dispatch []network.TaggedMessageHa
 	s.dispatchHandlers = append(s.dispatchHandlers, dispatch...)
 }
 
+func (s *httpTestPeerSource) SubstituteGenesisID(rawURL string) string {
+	return strings.Replace(rawURL, "{genesisID}", "test genesisID", -1)
+}
+
 // implement network.HTTPPeer
 type testHTTPPeer string
 
 func (p *testHTTPPeer) GetAddress() string {
 	return string(*p)
 }
-func (p *testHTTPPeer) PrepareURL(x string) string {
-	return strings.Replace(x, "{genesisID}", "test genesisID", -1)
-}
 func (p *testHTTPPeer) GetHTTPClient() *http.Client {
 	return &http.Client{}
 }

diff --git a/catchup/httpFetcher.go b/catchup/httpFetcher.go
@@ -75,7 +75,8 @@ func (hf *HTTPFetcher) GetBlockBytes(ctx context.Context, r basics.Round) (data
 	if err != nil {
 		return nil, err
 	}
-	parsedURL.Path = hf.peer.PrepareURL(path.Join(parsedURL.Path, "/v1/{genesisID}/block/"+strconv.FormatUint(uint64(r), 36)))
+
+	parsedURL.Path = hf.net.SubstituteGenesisID(path.Join(parsedURL.Path, "/v1/{genesisID}/block/"+strconv.FormatUint(uint64(r), 36)))
 	blockURL := parsedURL.String()
 	hf.log.Debugf("block GET %#v peer %#v %T", blockURL, hf.peer, hf.peer)
 	request, err := http.NewRequest("GET", blockURL, nil)

diff --git a/catchup/ledgerFetcher.go b/catchup/ledgerFetcher.go
@@ -94,7 +94,7 @@ func (lf *ledgerFetcher) getPeerLedger(ctx context.Context, peer network.HTTPPee
 		return err
 	}
 
-	parsedURL.Path = peer.PrepareURL(path.Join(parsedURL.Path, "/v1/{genesisID}/ledger/"+strconv.FormatUint(uint64(round), 36)))
+	parsedURL.Path = lf.net.SubstituteGenesisID(path.Join(parsedURL.Path, "/v1/{genesisID}/ledger/"+strconv.FormatUint(uint64(round), 36)))
 	ledgerURL := parsedURL.String()
 	lf.log.Debugf("ledger GET %#v peer %#v %T", ledgerURL, peer, peer)
 	request, err := http.NewRequest(http.MethodGet, ledgerURL, nil)

diff --git a/components/mocks/mockNetwork.go b/components/mocks/mockNetwork.go
@@ -105,3 +105,8 @@ func (network *MockNetwork) OnNetworkAdvance() {}
 func (network *MockNetwork) GetHTTPRequestConnection(request *http.Request) (conn net.Conn) {
 	return nil
 }
+
+// SubstituteGenesisID - empty implementation
+func (network *MockNetwork) SubstituteGenesisID(rawURL string) string {
+	return rawURL
+}
diff --git a/network/wsNetwork.go b/network/wsNetwork.go
@@ -198,6 +198,9 @@ type GossipNode interface {
 	// newly connecting peers.  This should be called before the network
 	// is started.
 	RegisterMessageInterest(protocol.Tag) error
+
+	// SubstituteGenesisID substitutes the "{genesisID}" with their network-specific genesisID.
+	SubstituteGenesisID(rawURL string) string
 }
 
 // IncomingMessage represents a message arriving from some peer in our p2p network
@@ -2080,3 +2083,8 @@ func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) error {
 	wn.messagesOfInterest[t] = true
 	return nil
 }
+
+// SubstituteGenesisID substitutes the "{genesisID}" with their network-specific genesisID.
+func (wn *WebsocketNetwork) SubstituteGenesisID(rawURL string) string {
+	return strings.Replace(rawURL, "{genesisID}", wn.GenesisID, -1)
+}
diff --git a/network/wsPeer.go b/network/wsPeer.go
@@ -25,7 +25,6 @@ import (
 	"net"
 	"net/http"
 	"runtime"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -214,10 +213,6 @@ type wsPeer struct {
 type HTTPPeer interface {
 	GetAddress() string
 	GetHTTPClient() *http.Client
-
-	// PrepareURL takes a URL that may have substitution parameters in it and returns a URL with those parameters filled in.
-	// E.g. /v1/{genesisID}/gossip -> /v1/1234/gossip
-	PrepareURL(string) string
 }
 
 // UnicastPeer is another possible interface for the opaque Peer.
@@ -254,11 +249,6 @@ func (wp *wsPeerCore) GetHTTPClient() *http.Client {
 	return &wp.client
 }
 
-// PrepareURL substitutes placeholders like "{genesisID}" for their values.
-func (wp *wsPeerCore) PrepareURL(rawURL string) string {
-	return strings.Replace(rawURL, "{genesisID}", wp.net.GenesisID, -1)
-}
-
 // Version returns the matching version from network.SupportedProtocolVersions
 func (wp *wsPeer) Version() string {
 	return wp.version

diff --git a/rpcs/httpTxSync.go b/rpcs/httpTxSync.go
@@ -113,7 +113,7 @@ func (hts *HTTPTxSync) Sync(ctx context.Context, bloom *bloom.Filter) (txgroups
 		hts.log.Warnf("txSync bad url %v: %s", hts.rootURL, err)
 		return nil, err
 	}
-	parsedURL.Path = hpeer.PrepareURL(path.Join(parsedURL.Path, TxServiceHTTPPath))
+	parsedURL.Path = hts.peers.SubstituteGenesisID(path.Join(parsedURL.Path, TxServiceHTTPPath))
 	syncURL := parsedURL.String()
 	hts.log.Infof("http sync from %s", syncURL)
 	params := url.Values{}

diff --git a/rpcs/txService_test.go b/rpcs/txService_test.go
@@ -57,9 +57,6 @@ type testHTTPPeer string
 func (p testHTTPPeer) GetAddress() string {
 	return string(p)
 }
-func (p *testHTTPPeer) PrepareURL(x string) string {
-	return strings.Replace(x, "{genesisID}", "test genesisID", -1)
-}
 func (p *testHTTPPeer) GetHTTPClient() *http.Client {
 	return &http.Client{}
 }
@@ -113,6 +110,10 @@ func (b *basicRPCNode) GetPeers(options ...network.PeerOption) []network.Peer {
 	return b.peers
 }
 
+func (b *basicRPCNode) SubstituteGenesisID(rawURL string) string {
+	return strings.Replace(rawURL, "{genesisID}", "test genesisID", -1)
+}
+
 func nodePair() (*basicRPCNode, *basicRPCNode) {
 	nodeA := &basicRPCNode{}
 	nodeA.start()

diff --git a/rpcs/txSyncer_test.go b/rpcs/txSyncer_test.go
@@ -141,9 +141,6 @@ func (client *mockRPCClient) GetAddress() string {
 func (client *mockRPCClient) GetHTTPClient() *http.Client {
 	return nil
 }
-func (client *mockRPCClient) PrepareURL(x string) string {
-	return strings.Replace(x, "{genesisID}", "test genesisID", -1)
-}
 
 type mockClientAggregator struct {
 	mocks.MockNetwork
@@ -153,6 +150,9 @@ type mockClientAggregator struct {
 func (mca *mockClientAggregator) GetPeers(options ...network.PeerOption) []network.Peer {
 	return mca.peers
 }
+func (mca *mockClientAggregator) SubstituteGenesisID(rawURL string) string {
+	return strings.Replace(rawURL, "{genesisID}", "test genesisID", -1)
+}
 
 const numberOfPeers = 10