simplify API for BlockService to handle multiple HTTP paths

Add a helper BlockService.RegisterHandlers() that registers handlers for
multiple HTTP paths, in preparation for adding more handlers.

catchup/pref_test.go

@@ -50,7 +50,7 @@ func BenchmarkServiceFetchBlocks(b *testing.B) {
 	net := &httpTestPeerSource{}
 	ls := rpcs.MakeBlockService(logging.TestingLog(b), config.GetDefaultLocal(), remote, net, "test genesisID")
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()

catchup/service_test.go

@@ -150,7 +150,7 @@ func TestServiceFetchBlocksSameRange(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -201,7 +201,7 @@ func TestSyncRound(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -291,7 +291,7 @@ func TestPeriodicSync(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -357,7 +357,7 @@ func TestServiceFetchBlocksOneBlock(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -421,7 +421,7 @@ func TestAbruptWrites(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -479,7 +479,7 @@ func TestServiceFetchBlocksMultiBlocks(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -534,7 +534,7 @@ func TestServiceFetchBlocksMalformed(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -688,7 +688,7 @@ func helperTestOnSwitchToUnSupportedProtocol(
 	ls := rpcs.MakeBlockService(logging.Base(), config, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()
@@ -899,7 +899,7 @@ func TestCatchupUnmatchedCertificate(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, remote, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()

catchup/universalFetcher_test.go

@@ -100,7 +100,7 @@ func TestUGetBlockHTTP(t *testing.T) {
 	ls := rpcs.MakeBlockService(logging.Base(), blockServiceConfig, ledger, net, "test genesisID")
 
 	nodeA := basicRPCNode{}
-	nodeA.RegisterHTTPHandler(rpcs.BlockServiceBlockPath, ls)
+	ls.RegisterHandlers(&nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 	rootURL := nodeA.rootURL()

network/gossipNode.go

@@ -54,8 +54,9 @@ type GossipNode interface {
 	Disconnect(badnode Peer)
 	DisconnectPeers() // only used by testing
 
-	// RegisterHTTPHandler path accepts gorilla/mux path annotations
+	// RegisterHTTPHandler and RegisterHTTPHandlerFunc: path accepts gorilla/mux path annotations
 	RegisterHTTPHandler(path string, handler http.Handler)
+	RegisterHTTPHandlerFunc(path string, handler func(http.ResponseWriter, *http.Request))
 
 	// RequestConnectOutgoing asks the system to actually connect to peers.
 	// `replace` optionally drops existing connections before making new ones.

network/p2pNetwork.go

@@ -241,6 +241,9 @@ func (n *P2PNetwork) DisconnectPeers() {
 func (n *P2PNetwork) RegisterHTTPHandler(path string, handler http.Handler) {
 }
 
+func (n *P2PNetwork) RegisterHTTPHandlerFunc(path string, handler func(http.ResponseWriter, *http.Request)) {
+}
+
 // RequestConnectOutgoing asks the system to actually connect to peers.
 // `replace` optionally drops existing connections before making new ones.
 // `quit` chan allows cancellation.

network/wsNetwork.go

@@ -504,6 +504,11 @@ func (wn *WebsocketNetwork) RegisterHTTPHandler(path string, handler http.Handle
 	wn.router.Handle(path, handler)
 }
 
+// RegisterHTTPHandlerFunc path accepts gorilla/mux path annotations
+func (wn *WebsocketNetwork) RegisterHTTPHandlerFunc(path string, handler func(http.ResponseWriter, *http.Request)) {
+	wn.router.HandleFunc(path, handler)
+}
+
 // RequestConnectOutgoing tries to actually do the connect to new peers.
 // `replace` drop all connections first and find new peers.
 func (wn *WebsocketNetwork) RequestConnectOutgoing(replace bool, quit <-chan struct{}) {

rpcs/blockService.go

@@ -55,7 +55,7 @@ const blockServerMaxBodyLength = 512
 const blockServerCatchupRequestBufferSize = 10
 
 // BlockServiceBlockPath is the path to register BlockService as a handler for when using gorilla/mux
-// e.g. .Handle(BlockServiceBlockPath, &ls)
+// e.g. .HandleFunc(BlockServiceBlockPath, ls.ServeBlockPath)
 const BlockServiceBlockPath = "/v{version:[0-9.]+}/{genesisID}/block/{round:[0-9a-z]+}"
 
 // Constant strings used as keys for topics
@@ -145,11 +145,22 @@ func MakeBlockService(log logging.Logger, config config.Local, ledger LedgerForB
 		memoryCap:               config.BlockServiceMemCap,
 	}
 	if service.enableService {
-		net.RegisterHTTPHandler(BlockServiceBlockPath, service)
+		service.RegisterHandlers(net)
 	}
 	return service
 }
 
+// HTTPRegistrar represents an HTTP request router that can be used by BlockService
+// to register its request handlers.
+type HTTPRegistrar interface {
+	RegisterHTTPHandlerFunc(path string, handler func(response http.ResponseWriter, request *http.Request))
+}
+
+// RegisterHTTP registers the request handlers for BlockService's paths with the registrar.
+func (bs *BlockService) RegisterHandlers(registrar HTTPRegistrar) {
+	registrar.RegisterHTTPHandlerFunc(BlockServiceBlockPath, bs.ServeBlockPath)
+}
+
 // Start listening to catchup requests over ws
 func (bs *BlockService) Start() {
 	bs.mu.Lock()
@@ -174,10 +185,10 @@ func (bs *BlockService) Stop() {
 	bs.closeWaitGroup.Wait()
 }
 
-// ServerHTTP returns blocks
+// ServeBlockPath returns blocks
 // Either /v{version}/{genesisID}/block/{round} or ?b={round}&v={version}
 // Uses gorilla/mux for path argument parsing.
-func (bs *BlockService) ServeHTTP(response http.ResponseWriter, request *http.Request) {
+func (bs *BlockService) ServeBlockPath(response http.ResponseWriter, request *http.Request) {
 	pathVars := mux.Vars(request)
 	versionStr, hasVersionStr := pathVars["version"]
 	roundStr, hasRoundStr := pathVars["round"]
@@ -254,13 +265,13 @@ func (bs *BlockService) ServeHTTP(response http.ResponseWriter, request *http.Re
 			if !ok {
 				response.Header().Set("Retry-After", blockResponseRetryAfter)
 				response.WriteHeader(http.StatusServiceUnavailable)
-				bs.log.Debugf("ServeHTTP: returned retry-after: %v", err)
+				bs.log.Debugf("ServeBlockPath: returned retry-after: %v", err)
 			}
 			httpBlockMessagesDroppedCounter.Inc(nil)
 			return
 		default:
 			// unexpected error.
-			bs.log.Warnf("ServeHTTP : failed to retrieve block %d %v", round, err)
+			bs.log.Warnf("ServeBlockPath: failed to retrieve block %d %v", round, err)
 			response.WriteHeader(http.StatusInternalServerError)
 			return
 		}

rpcs/blockService_test.go

@@ -149,11 +149,11 @@ func TestRedirectFallbackArchiver(t *testing.T) {
 	nodeA := &basicRPCNode{}
 	nodeB := &basicRPCNode{}
 
-	nodeA.RegisterHTTPHandler(BlockServiceBlockPath, bs1)
+	bs1.RegisterHandlers(nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 
-	nodeB.RegisterHTTPHandler(BlockServiceBlockPath, bs2)
+	bs2.RegisterHandlers(nodeB)
 	nodeB.start()
 	defer nodeB.stop()
 
@@ -200,7 +200,7 @@ func TestBlockServiceShutdown(t *testing.T) {
 
 	nodeA := &basicRPCNode{}
 
-	nodeA.RegisterHTTPHandler(BlockServiceBlockPath, bs1)
+	bs1.RegisterHandlers(nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 
@@ -261,8 +261,8 @@ func TestRedirectFallbackEndpoints(t *testing.T) {
 	bs1 := MakeBlockService(log, config, ledger1, net1, "{genesisID}")
 	bs2 := MakeBlockService(log, config, ledger2, net2, "{genesisID}")
 
-	nodeA.RegisterHTTPHandler(BlockServiceBlockPath, bs1)
-	nodeB.RegisterHTTPHandler(BlockServiceBlockPath, bs2)
+	bs1.RegisterHandlers(nodeA)
+	bs2.RegisterHandlers(nodeB)
 
 	parsedURL, err := network.ParseHostOrURL(nodeA.rootURL())
 	require.NoError(t, err)
@@ -320,11 +320,11 @@ func TestRedirectOnFullCapacity(t *testing.T) {
 	nodeA := &basicRPCNode{}
 	nodeB := &basicRPCNode{}
 
-	nodeA.RegisterHTTPHandler(BlockServiceBlockPath, bs1)
+	bs1.RegisterHandlers(nodeA)
 	nodeA.start()
 	defer nodeA.stop()
 
-	nodeB.RegisterHTTPHandler(BlockServiceBlockPath, bs2)
+	bs2.RegisterHandlers(nodeB)
 	nodeB.start()
 	defer nodeB.stop()
 
@@ -491,7 +491,7 @@ func TestRedirectExceptions(t *testing.T) {
 
 	nodeA := &basicRPCNode{}
 
-	nodeA.RegisterHTTPHandler(BlockServiceBlockPath, bs1)
+	bs1.RegisterHandlers(nodeA)
 	nodeA.start()
 	defer nodeA.stop()