feat(routing/http/client): DefaultProtocolFilter for IPIP-484 (#689)

ipfs · Oct 16, 2024 · c2487a2 · c2487a2
1 parent 08fb71d
commit c2487a2
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,8 @@ The following emojis are used to highlight certain changes:
 
 ### Changed
 
+- `routing/http/client`: creating delegated routing client with `New` now defaults to querying delegated routing server with `DefaultProtocolFilter`  ([IPIP-484](https://github.com/ipfs/specs/pull/484)) [#689](https://github.com/ipfs/boxo/pull/689)
+
 ### Removed
 
 ### Fixed

diff --git a/examples/routing/delegated-routing-client/main_test.go b/examples/routing/delegated-routing-client/main_test.go
@@ -25,15 +25,16 @@ func TestFindProviders(t *testing.T) {
 		if r.URL.Path == "/routing/v1/providers/"+cidStr {
 			w.Header().Set("Content-Type", "application/x-ndjson")
 			w.Write([]byte(`{"Schema":"peer","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Addrs":["/ip4/111.222.222.111/tcp/5734"],"Protocols":["transport-bitswap"]}` + "\n"))
-			w.Write([]byte(`{"Schema":"peer","ID":"12D3KooWB6RAWgcmHAP7TGEGK7utV2ZuqSzX1DNjRa97TtJ7139n","Addrs":["/ip4/127.0.0.1/tcp/5734"],"Protocols":["transport-horse"]}` + "\n"))
+			w.Write([]byte(`{"Schema":"peer","ID":"12D3KooWS6BmwfQEZcRqCHCBbDL2DF5a6F7dZnbPFkwmZCuLEK5f","Addrs":["/ip4/127.0.0.1/tcp/6434"],"Protocols":["horse"]}` + "\n")) // this one will be skipped by DefaultProtocolFilter
+			w.Write([]byte(`{"Schema":"peer","ID":"12D3KooWB6RAWgcmHAP7TGEGK7utV2ZuqSzX1DNjRa97TtJ7139n","Addrs":["/ip4/127.0.0.1/tcp/5734"],"Protocols":[]}` + "\n"))
 		}
 	}))
 	t.Cleanup(ts.Close)
 
 	out := &bytes.Buffer{}
 	err := run(out, ts.URL, cidStr, "", "", 1)
 	assert.Contains(t, out.String(), "12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn\n\tProtocols: [transport-bitswap]\n\tAddresses: [/ip4/111.222.222.111/tcp/5734]\n")
-	assert.Contains(t, out.String(), "12D3KooWB6RAWgcmHAP7TGEGK7utV2ZuqSzX1DNjRa97TtJ7139n\n\tProtocols: [transport-horse]\n\tAddresses: [/ip4/127.0.0.1/tcp/5734]\n")
+	assert.Contains(t, out.String(), "12D3KooWB6RAWgcmHAP7TGEGK7utV2ZuqSzX1DNjRa97TtJ7139n\n\tProtocols: []\n\tAddresses: [/ip4/127.0.0.1/tcp/5734]\n")
 	assert.NoError(t, err)
 }
 

diff --git a/routing/http/client/client.go b/routing/http/client/client.go
@@ -33,6 +33,8 @@ import (
 var (
 	_      contentrouter.Client = &Client{}
 	logger                      = logging.Logger("routing/http/client")
+
+	DefaultProtocolFilter = []string{"unknown", "transport-bitswap"} // IPIP-484
 )
 
 const (
@@ -175,10 +177,11 @@ func WithStreamResultsRequired() Option {
 // The Provider and identity parameters are option. If they are nil, the [client.ProvideBitswap] method will not function.
 func New(baseURL string, opts ...Option) (*Client, error) {
 	client := &Client{
-		baseURL:    baseURL,
-		httpClient: newDefaultHTTPClient(defaultUserAgent),
-		clock:      clock.New(),
-		accepts:    strings.Join([]string{mediaTypeNDJSON, mediaTypeJSON}, ","),
+		baseURL:        baseURL,
+		httpClient:     newDefaultHTTPClient(defaultUserAgent),
+		clock:          clock.New(),
+		accepts:        strings.Join([]string{mediaTypeNDJSON, mediaTypeJSON}, ","),
+		protocolFilter: DefaultProtocolFilter, // can be customized via WithProtocolFilter
 	}
 
 	for _, opt := range opts {

diff --git a/routing/http/client/client_test.go b/routing/http/client/client_test.go
@@ -223,11 +223,22 @@ func (e *osErrContains) errContains(t *testing.T, err error) {
 }
 
 func TestClient_FindProviders(t *testing.T) {
+	unknownPeerRecord := makePeerRecord([]string{})
 	bitswapPeerRecord := makePeerRecord([]string{"transport-bitswap"})
 	httpPeerRecord := makePeerRecord([]string{"transport-ipfs-gateway-http"})
+	fooPeerRecord := makePeerRecord([]string{"transport-foo"})
+
 	peerProviders := []iter.Result[types.Record]{
+		{Val: &unknownPeerRecord},
 		{Val: &bitswapPeerRecord},
 		{Val: &httpPeerRecord},
+		{Val: &fooPeerRecord},
+	}
+
+	// DefaultProtocolFilter
+	defaultFilterPeerProviders := []iter.Result[types.Record]{
+		{Val: &unknownPeerRecord},
+		{Val: &bitswapPeerRecord},
 	}
 
 	bitswapRecord := makeBitswapRecord()
@@ -249,16 +260,23 @@ func TestClient_FindProviders(t *testing.T) {
 		expJSONResponse      bool
 	}{
 		{
-			name:                 "happy case",
+			name:                 "happy case with DefaultProtocolFilter",
+			routerResult:         peerProviders,
+			expResult:            defaultFilterPeerProviders,
+			expStreamingResponse: true,
+		},
+		{
+			name:                 "pass through with protocol filter disabled",
+			filterProtocols:      []string{},
 			routerResult:         peerProviders,
 			expResult:            peerProviders,
 			expStreamingResponse: true,
 		},
 		{
-			name:                 "happy case with protocol filter",
-			filterProtocols:      []string{"transport-bitswap"},
+			name:                 "happy case with custom protocol filter",
+			filterProtocols:      []string{"transport-foo"},
 			routerResult:         peerProviders,
-			expResult:            []iter.Result[types.Record]{{Val: &bitswapPeerRecord}},
+			expResult:            []iter.Result[types.Record]{{Val: &fooPeerRecord}},
 			expStreamingResponse: true,
 		},
 		{
@@ -270,7 +288,7 @@ func TestClient_FindProviders(t *testing.T) {
 		{
 			name:                    "server doesn't support streaming",
 			routerResult:            peerProviders,
-			expResult:               peerProviders,
+			expResult:               defaultFilterPeerProviders,
 			serverStreamingDisabled: true,
 			expJSONResponse:         true,
 		},
@@ -497,13 +515,25 @@ func TestClient_Provide(t *testing.T) {
 }
 
 func TestClient_FindPeers(t *testing.T) {
-	peerRecord1 := makePeerRecord([]string{"transport-bitswap"})
-	peerRecord2 := makePeerRecord([]string{"transport-ipfs-gateway-http"})
+	unknownPeerRecord := makePeerRecord([]string{})
+	bitswapPeerRecord := makePeerRecord([]string{"transport-bitswap"})
+	httpPeerRecord := makePeerRecord([]string{"transport-ipfs-gateway-http"})
+	fooPeerRecord := makePeerRecord([]string{"transport-foo"})
+
 	peerRecords := []iter.Result[*types.PeerRecord]{
-		{Val: &peerRecord1},
-		{Val: &peerRecord2},
+		{Val: &unknownPeerRecord},
+		{Val: &bitswapPeerRecord},
+		{Val: &httpPeerRecord},
+		{Val: &fooPeerRecord},
+	}
+
+	// DefaultProtocolFilter
+	defaultFilterPeerRecords := []iter.Result[*types.PeerRecord]{
+		{Val: &unknownPeerRecord},
+		{Val: &bitswapPeerRecord},
 	}
-	pid := *peerRecord1.ID
+
+	pid := *bitswapPeerRecord.ID
 
 	cases := []struct {
 		name                    string
@@ -521,22 +551,29 @@ func TestClient_FindPeers(t *testing.T) {
 		expJSONResponse      bool
 	}{
 		{
-			name:                 "happy case",
+			name:                 "happy case with DefaultProtocolFilter",
+			routerResult:         peerRecords,
+			expResult:            defaultFilterPeerRecords,
+			expStreamingResponse: true,
+		},
+		{
+			name:                 "pass through with protocol filter disabled",
+			filterProtocols:      []string{},
 			routerResult:         peerRecords,
 			expResult:            peerRecords,
 			expStreamingResponse: true,
 		},
 		{
-			name:                 "happy case with protocol filter",
-			filterProtocols:      []string{"transport-bitswap"},
+			name:                 "happy case with custom protocol filter",
+			filterProtocols:      []string{"transport-foo"},
 			routerResult:         peerRecords,
-			expResult:            []iter.Result[*types.PeerRecord]{{Val: &peerRecord1}},
+			expResult:            []iter.Result[*types.PeerRecord]{{Val: &fooPeerRecord}},
 			expStreamingResponse: true,
 		},
 		{
 			name:                    "server doesn't support streaming",
 			routerResult:            peerRecords,
-			expResult:               peerRecords,
+			expResult:               defaultFilterPeerRecords,
 			serverStreamingDisabled: true,
 			expJSONResponse:         true,
 		},