Add Close() to iter interface, remove Closer interfaces

This is always needed, so we can simplify things by just including it in the main iterator interface.
ipfs · Feb 10, 2023 · 498cfce · 498cfce
1 parent 2ee38dc
commit 498cfce
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 97 deletions.
diff --git a/routing/http/client/client.go b/routing/http/client/client.go
@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"mime"
 	"net/http"
 	"strings"
@@ -160,10 +159,7 @@ func (c *measuringIter[T]) Val() T {
 
 func (c *measuringIter[T]) Close() error {
 	c.m.record(c.ctx)
-	if closer, ok := c.Iter.(io.Closer); ok {
-		return closer.Close()
-	}
-	return nil
+	return c.Iter.Close()
 }
 
 func (c *client) FindProviders(ctx context.Context, key cid.Cid) (provs iter.ResultIter[types.ProviderResponse], err error) {

diff --git a/routing/http/client/client_test.go b/routing/http/client/client_test.go
@@ -26,9 +26,9 @@ import (
 
 type mockContentRouter struct{ mock.Mock }
 
-func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIterCloser[types.ProviderResponse], error) {
+func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIter[types.ProviderResponse], error) {
 	args := m.Called(ctx, key)
-	return args.Get(0).(iter.ResultIterCloser[types.ProviderResponse]), args.Error(1)
+	return args.Get(0).(iter.ResultIter[types.ProviderResponse]), args.Error(1)
 }
 func (m *mockContentRouter) ProvideBitswap(ctx context.Context, req *server.BitswapWriteProvideRequest) (time.Duration, error) {
 	args := m.Called(ctx, req)
@@ -211,8 +211,7 @@ func TestClient_FindProviders(t *testing.T) {
 			}
 			cid := makeCID()
 
-			sliceIter := iter.FromSlice(c.routerProvs)
-			findProvsIter := iter.IterCloserNoop[iter.Result[types.ProviderResponse]](sliceIter)
+			findProvsIter := iter.FromSlice(c.routerProvs)
 
 			router.On("FindProviders", mock.Anything, cid).
 				Return(findProvsIter, c.routerErr)

diff --git a/routing/http/contentrouter/contentrouter.go b/routing/http/contentrouter/contentrouter.go
@@ -105,6 +105,7 @@ func (c *contentRouter) Ready() bool {
 // readProviderResponses reads bitswap records from the iterator into the given channel, dropping non-bitswap records.
 func readProviderResponses(iter iter.ResultIter[types.ProviderResponse], ch chan<- peer.AddrInfo) {
 	defer close(ch)
+	defer iter.Close()
 	for iter.Next() {
 		res := iter.Val()
 		if res.Err != nil {

diff --git a/routing/http/server/server.go b/routing/http/server/server.go
@@ -39,7 +39,7 @@ type FindProvidersAsyncResponse struct {
 }
 
 type ContentRouter interface {
-	FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIterCloser[types.ProviderResponse], error)
+	FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIter[types.ProviderResponse], error)
 	ProvideBitswap(ctx context.Context, req *BitswapWriteProvideRequest) (time.Duration, error)
 	Provide(ctx context.Context, req *WriteProvideRequest) (types.ProviderResponse, error)
 }
@@ -164,7 +164,7 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) {
 		return
 	}
 
-	var handlerFunc func(w http.ResponseWriter, provIter iter.ResultIterCloser[types.ProviderResponse])
+	var handlerFunc func(w http.ResponseWriter, provIter iter.ResultIter[types.ProviderResponse])
 
 	var supportsJSONSeq bool
 	var supportsJSON bool
@@ -206,7 +206,7 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) {
 	handlerFunc(w, provIter)
 }
 
-func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIterCloser[types.ProviderResponse]) {
+func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIter[types.ProviderResponse]) {
 	defer provIter.Close()
 
 	var (
@@ -227,7 +227,7 @@ func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIt
 	writeJSONResult(w, "FindProviders", response)
 }
 
-func (s *server) findProvidersJSONSeq(w http.ResponseWriter, provIter iter.ResultIterCloser[types.ProviderResponse]) {
+func (s *server) findProvidersJSONSeq(w http.ResponseWriter, provIter iter.ResultIter[types.ProviderResponse]) {
 	defer provIter.Close()
 
 	w.Header().Set("Content-Type", mediaTypeJSONSeq)

diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go
@@ -20,13 +20,12 @@ func TestHeaders(t *testing.T) {
 	t.Cleanup(server.Close)
 	serverAddr := "http://" + server.Listener.Addr().String()
 
-	sliceIter := iter.FromSlice([]iter.Result[types.ProviderResponse]{
+	results := iter.FromSlice([]iter.Result[types.ProviderResponse]{
 		{Val: &types.ReadBitswapProviderRecord{
 			Protocol: "transport-bitswap",
 			Schema:   types.SchemaBitswap,
 		}}},
 	)
-	results := iter.IterCloserNoop[iter.Result[types.ProviderResponse]](sliceIter)
 
 	c := "baeabep4vu3ceru7nerjjbk37sxb7wmftteve4hcosmyolsbsiubw2vr6pqzj6mw7kv6tbn6nqkkldnklbjgm5tzbi4hkpkled4xlcr7xz4bq"
 	cb, err := cid.Decode(c)
@@ -50,9 +49,9 @@ func TestHeaders(t *testing.T) {
 
 type mockContentRouter struct{ mock.Mock }
 
-func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIterCloser[types.ProviderResponse], error) {
+func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid) (iter.ResultIter[types.ProviderResponse], error) {
 	args := m.Called(ctx, key)
-	return args.Get(0).(iter.ResultIterCloser[types.ProviderResponse]), args.Error(1)
+	return args.Get(0).(iter.ResultIter[types.ProviderResponse]), args.Error(1)
 }
 func (m *mockContentRouter) ProvideBitswap(ctx context.Context, req *BitswapWriteProvideRequest) (time.Duration, error) {
 	args := m.Called(ctx, req)

diff --git a/routing/http/types/iter/iter.go b/routing/http/types/iter/iter.go
@@ -1,7 +1,5 @@
 package iter
 
-import "io"
-
 // Iter is an iterator of arbitrary values.
 // Iterators are generally not goroutine-safe, to make them safe just read from them into a channel.
 // For our use cases, these usually have a single reader. This motivates iterators instead of channels,
@@ -17,12 +15,10 @@ type Iter[T any] interface {
 	// Next sets the iterator to the next value, returning true if an attempt was made to get the next value.
 	Next() bool
 	Val() T
+	Close() error
 }
 
-type ResultIter[T any] interface {
-	Next() bool
-	Val() Result[T]
-}
+type ResultIter[T any] interface{ Iter[Result[T]] }
 
 type Result[T any] struct {
 	Val T
@@ -36,16 +32,6 @@ func ToResultIter[T any](iter Iter[T]) Iter[Result[T]] {
 	})
 }
 
-type IterCloser[T any] interface {
-	Iter[T]
-	io.Closer
-}
-
-type ResultIterCloser[T any] interface {
-	ResultIter[T]
-	io.Closer
-}
-
 func ReadAll[T any](iter Iter[T]) []T {
 	if iter == nil {
 		return nil
@@ -56,12 +42,3 @@ func ReadAll[T any](iter Iter[T]) []T {
 	}
 	return vs
 }
-
-// iterCloserNoop creates an io.Closer from an Iter that does nothing on close.
-type iterCloserNoop[T any] struct{ Iter[T] }
-
-func (n *iterCloserNoop[T]) Close() error { return nil }
-
-func IterCloserNoop[T any](it Iter[T]) IterCloser[T] {
-	return &iterCloserNoop[T]{it}
-}
diff --git a/routing/http/types/iter/map.go b/routing/http/types/iter/map.go
@@ -1,21 +1,19 @@
 package iter
 
-import "io"
-
 // Map invokes f on each element of iter.
-func Map[T any, U any](iter Iter[T], f func(t T) U) Iter[U] {
-	return &mapIter[T, U]{iter: iter, f: f}
+func Map[T any, U any](iter Iter[T], f func(t T) U) *MapIter[T, U] {
+	return &MapIter[T, U]{iter: iter, f: f}
 }
 
-type mapIter[T any, U any] struct {
+type MapIter[T any, U any] struct {
 	iter Iter[T]
 	f    func(T) U
 
 	done bool
 	val  U
 }
 
-func (m *mapIter[T, U]) Next() bool {
+func (m *MapIter[T, U]) Next() bool {
 	if m.done {
 		return false
 	}
@@ -32,13 +30,10 @@ func (m *mapIter[T, U]) Next() bool {
 	return true
 }
 
-func (m *mapIter[T, U]) Val() U {
+func (m *MapIter[T, U]) Val() U {
 	return m.val
 }
 
-func (m *mapIter[T, U]) Close() error {
-	if closer, ok := m.iter.(io.Closer); ok {
-		return closer.Close()
-	}
-	return nil
+func (m *MapIter[T, U]) Close() error {
+	return m.iter.Close()
 }
diff --git a/routing/http/types/iter/map_test.go b/routing/http/types/iter/map_test.go
@@ -7,29 +7,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// type nthErrIter[T any] struct {
-// 	Iter[T]
-// 	i   int
-// 	n   int
-// 	err error
-// }
-
-// func (n *nthErrIter[T]) Next() (T, bool) {
-// 	v, ok := n.Iter.Next()
-// 	n.i++
-// 	return v, ok
-// }
-// func (n *nthErrIter[T]) Err() error {
-// 	if n.i-1 == n.n {
-// 		return n.err
-// 	}
-// 	return n.Iter.Err()
-// }
-
-// func nthErr[T any](iter Iter[T], n int, err error) Iter[T] {
-// 	return &nthErrIter[T]{Iter: iter, n: n, err: err}
-// }
-
 func TestMap(t *testing.T) {
 	for _, c := range []struct {
 		input      Iter[int]
@@ -41,25 +18,16 @@ func TestMap(t *testing.T) {
 			f:          func(i int) int { return i + 1 },
 			expResults: []int{2, 3, 4},
 		},
-		// {
-		// 	input:      FromSlice([]int{}),
-		// 	f:          func(i int) int { return i + 1 },
-		// 	expResults: []int{},
-		// },
-		// {
-		// 	input:      FromSlice([]int{1}),
-		// 	f:          func(i int) int { return i + 1 },
-		// 	expResults: []int{2},
-		// },
-		// {
-		// 	input: FromSlice([]int{1, 2, 3}), 2, errors.New("boom"),
-		// 	f: func(i int) (int, error) { return i + 1, nil },
-		// 	expResults: []result{
-		// 		{val: 2},
-		// 		{val: 3},
-		// 		{errContains: "boom"},
-		// 	},
-		// },
+		{
+			input:      FromSlice([]int{}),
+			f:          func(i int) int { return i + 1 },
+			expResults: nil,
+		},
+		{
+			input:      FromSlice([]int{1}),
+			f:          func(i int) int { return i + 1 },
+			expResults: []int{2},
+		},
 	} {
 		t.Run(fmt.Sprintf("%v", c.input), func(t *testing.T) {
 			iter := Map(c.input, c.f)

diff --git a/routing/http/types/iter/slice.go b/routing/http/types/iter/slice.go
@@ -23,3 +23,7 @@ func (s *SliceIter[T]) Next() bool {
 func (s *SliceIter[T]) Val() T {
 	return s.val
 }
+
+func (s *SliceIter[T]) Close() error {
+	return nil
+}
diff --git a/routing/http/types/jsonseq/provider.go b/routing/http/types/jsonseq/provider.go
@@ -8,8 +8,10 @@ import (
 	"github.com/ipfs/go-libipfs/routing/http/types/iter"
 )
 
+type readProvidersResponseIter iter.Iter[iter.Result[types.UnknownProviderRecord]]
+
 // NewReadProvidersResponseIter returns an iterator that reads Read Provider Records from the given reader.
-func NewReadProvidersResponseIter(r io.Reader) iter.ResultIter[types.ProviderResponse] {
+func NewReadProvidersResponseIter(r io.Reader) iter.Iter[iter.Result[types.ProviderResponse]] {
 	jsonIter := iter.FromReaderJSON[types.UnknownProviderRecord](r)
 	mapFn := func(upr iter.Result[types.UnknownProviderRecord]) iter.Result[types.ProviderResponse] {
 		var result iter.Result[types.ProviderResponse]