From 0fbeac3f9204190f1cd6c079502634052ea77d41 Mon Sep 17 00:00:00 2001 From: Antonio Navarro Perez Date: Wed, 29 Jun 2022 18:32:43 +0200 Subject: [PATCH] Add tests and remove uneeded code --- config/profile.go | 3 +- core/node/libp2p/routing.go | 11 ++- core/node/libp2p/topicdiscovery.go | 5 +- routing/delegated.go | 4 - routing/delegated_test.go | 123 +++++++++++++++++++++++++++++ routing/wrapper.go | 104 +++++------------------- routing/wrapper_test.go | 101 +++++++++++++++++++++++ 7 files changed, 255 insertions(+), 96 deletions(-) create mode 100644 routing/delegated_test.go create mode 100644 routing/wrapper_test.go diff --git a/config/profile.go b/config/profile.go index 3f4fbf1f67d4..8252d1ab91ec 100644 --- a/config/profile.go +++ b/config/profile.go @@ -174,8 +174,7 @@ functionality - performance of content discovery and data fetching may be degraded. `, Transform: func(c *Config) error { - rt := "dhtclient" - c.Routing.Type = &OptionalString{value: &rt} + c.Routing.Type = NewOptionalString("dhtclient") c.AutoNAT.ServiceMode = AutoNATServiceDisabled c.Reprovider.Interval = "0" diff --git a/core/node/libp2p/routing.go b/core/node/libp2p/routing.go index 76aa1b9d312d..d4e6c79ec042 100644 --- a/core/node/libp2p/routing.go +++ b/core/node/libp2p/routing.go @@ -168,8 +168,15 @@ type p2pOnlineContentRoutingIn struct { ContentRouter []routing.ContentRouting `group:"content-routers"` } -func ContentRouting(in p2pOnlineContentRoutingIn) irouting.TieredContentRouter { - return &irouting.ContentRoutingWrapper{ContentRoutings: in.ContentRouter} +func ContentRouting(in p2pOnlineContentRoutingIn) routing.ContentRouting { + var routers []routing.Routing + for _, cr := range in.ContentRouter { + routers = append(routers, irouting.NewContentRoutingWrapper(cr)) + } + + return routinghelpers.Tiered{ + Routers: routers, + } } type p2pOnlineRoutingIn struct { diff --git a/core/node/libp2p/topicdiscovery.go b/core/node/libp2p/topicdiscovery.go index 51f29d68653c..61aca74de92b 100644 --- a/core/node/libp2p/topicdiscovery.go +++ b/core/node/libp2p/topicdiscovery.go @@ -6,13 +6,12 @@ import ( "github.com/libp2p/go-libp2p-core/discovery" "github.com/libp2p/go-libp2p-core/host" + "github.com/libp2p/go-libp2p-core/routing" disc "github.com/libp2p/go-libp2p-discovery" - - irouting "github.com/ipfs/go-ipfs/routing" ) func TopicDiscovery() interface{} { - return func(host host.Host, cr irouting.TieredContentRouter) (service discovery.Discovery, err error) { + return func(host host.Host, cr routing.ContentRouting) (service discovery.Discovery, err error) { baseDisc := disc.NewRoutingDiscovery(cr) minBackoff, maxBackoff := time.Second*60, time.Hour rng := rand.New(rand.NewSource(rand.Int63())) diff --git a/routing/delegated.go b/routing/delegated.go index 022e530bc0c8..15a50d03ddee 100644 --- a/routing/delegated.go +++ b/routing/delegated.go @@ -15,10 +15,6 @@ type TieredRouter interface { ProviderManyWrapper() ProvideMany } -type TieredContentRouter interface { - routing.ContentRouting -} - var _ TieredRouter = &Tiered{} // Tiered is a routing Tiered implementation providing some extra methods to fill diff --git a/routing/delegated_test.go b/routing/delegated_test.go new file mode 100644 index 000000000000..e413119433b4 --- /dev/null +++ b/routing/delegated_test.go @@ -0,0 +1,123 @@ +package routing + +import ( + "context" + "testing" + + "github.com/ipfs/go-cid" + "github.com/ipfs/go-ipfs/config" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/routing" + routinghelpers "github.com/libp2p/go-libp2p-routing-helpers" + "github.com/multiformats/go-multihash" + "github.com/stretchr/testify/require" +) + +func TestPriority(t *testing.T) { + require := require.New(t) + params := make(map[string]string) + p := GetPriority(params) + + require.Equal(defaultPriority, p) + + params[string(config.RouterParamPriority)] = "101" + + p = GetPriority(params) + + require.Equal(101, p) + + params[string(config.RouterParamPriority)] = "NAN" + + p = GetPriority(params) + + require.Equal(defaultPriority, p) +} + +func TestRoutingFromConfig(t *testing.T) { + require := require.New(t) + + r, err := RoutingFromConfig(config.Router{ + Type: "unknown", + }) + + require.Nil(r) + require.EqualError(err, "router type unknown is not supported") + + r, err = RoutingFromConfig(config.Router{ + Type: string(config.RouterTypeReframe), + Parameters: make(map[string]string), + }) + + require.Nil(r) + require.EqualError(err, "configuration param 'address' is needed for reframe delegated routing types") + + r, err = RoutingFromConfig(config.Router{ + Type: string(config.RouterTypeReframe), + Parameters: map[string]string{ + string(config.RouterParamAddress): "test", + }, + }) + + require.NotNil(r) + require.NoError(err) + + // TODO add dht +} + +func TestTieredRouter(t *testing.T) { + require := require.New(t) + + tr := &Tiered{ + Tiered: routinghelpers.Tiered{ + Routers: []routing.Routing{routinghelpers.Null{}}, + }, + } + + pm := tr.ProviderManyWrapper() + require.Nil(pm) + + tr.Tiered.Routers = append(tr.Tiered.Routers, &dummyRouter{}) + + pm = tr.ProviderManyWrapper() + require.NotNil(pm) +} + +type dummyRouter struct { +} + +func (dr *dummyRouter) Provide(context.Context, cid.Cid, bool) error { + panic("not implemented") + +} + +func (dr *dummyRouter) FindProvidersAsync(context.Context, cid.Cid, int) <-chan peer.AddrInfo { + panic("not implemented") +} + +func (dr *dummyRouter) FindPeer(context.Context, peer.ID) (peer.AddrInfo, error) { + panic("not implemented") +} + +func (dr *dummyRouter) PutValue(context.Context, string, []byte, ...routing.Option) error { + panic("not implemented") +} + +func (dr *dummyRouter) GetValue(context.Context, string, ...routing.Option) ([]byte, error) { + panic("not implemented") +} + +func (dr *dummyRouter) SearchValue(context.Context, string, ...routing.Option) (<-chan []byte, error) { + panic("not implemented") +} + +func (dr *dummyRouter) Bootstrap(context.Context) error { + panic("not implemented") +} + +func (dr *dummyRouter) ProvideMany(ctx context.Context, keys []multihash.Multihash) error { + panic("not implemented") +} + +func (dr *dummyRouter) Ready() bool { + panic("not implemented") +} diff --git a/routing/wrapper.go b/routing/wrapper.go index fbf6392d22cf..1a5b95108b95 100644 --- a/routing/wrapper.go +++ b/routing/wrapper.go @@ -2,9 +2,7 @@ package routing import ( "context" - "sync" - "github.com/hashicorp/go-multierror" "github.com/ipfs/go-cid" drc "github.com/ipfs/go-delegated-routing/client" "github.com/libp2p/go-libp2p-core/peer" @@ -67,97 +65,33 @@ func (pmw *ProvideManyWrapper) Ready() bool { return out } -var _ TieredContentRouter = &ContentRoutingWrapper{} - type ContentRoutingWrapper struct { - ContentRoutings []routing.ContentRouting + routing.ContentRouting + routing.ValueStore } -// Provide adds the given cid to the content routing system. If 'true' is -// passed, it also announces it, otherwise it is just kept in the local -// accounting of which objects are being provided. -func (crw *ContentRoutingWrapper) Provide(ctx context.Context, cid cid.Cid, announce bool) error { - c := len(crw.ContentRoutings) - wg := sync.WaitGroup{} - wg.Add(c) - - errors := make([]error, c) - - for i, cr := range crw.ContentRoutings { - go func(cr routing.ContentRouting, i int) { - errors[i] = cr.Provide(ctx, cid, announce) - wg.Done() - }(cr, i) - } - - wg.Wait() - - var out []error - success := false - for _, e := range errors { - switch e { - case nil: - success = true - case routing.ErrNotSupported: - default: - out = append(out, e) - } - } - switch len(out) { - case 0: - if success { - // No errors and at least one router succeeded. - return nil - } - // No routers supported this operation. - return routing.ErrNotSupported - case 1: - return out[0] - default: - return &multierror.Error{Errors: out} +func NewContentRoutingWrapper(cr routing.ContentRouting) *ContentRoutingWrapper { + return &ContentRoutingWrapper{ + ContentRouting: cr, } } -// Search for peers who are able to provide a given key -// -// When count is 0, this method will return an unbounded number of -// results. -func (crw *ContentRoutingWrapper) FindProvidersAsync(ctx context.Context, cid cid.Cid, count int) <-chan peer.AddrInfo { - subCtx, cancel := context.WithCancel(ctx) - - aich := make(chan peer.AddrInfo) - - for _, ri := range crw.ContentRoutings { - fpch := ri.FindProvidersAsync(subCtx, cid, count) - go func() { - for ai := range fpch { - aich <- ai - } - }() - } - - out := make(chan peer.AddrInfo) - - go func() { - defer cancel() - c := 0 - doCount := true - if count <= 0 { - doCount = false - } +func (crw *ContentRoutingWrapper) Bootstrap(context.Context) error { + return nil +} - for ai := range aich { - if c >= count && doCount { - return - } +func (crw *ContentRoutingWrapper) FindPeer(context.Context, peer.ID) (peer.AddrInfo, error) { + return peer.AddrInfo{}, routing.ErrNotSupported +} - out <- ai +func (crw *ContentRoutingWrapper) PutValue(context.Context, string, []byte, ...routing.Option) error { + return routing.ErrNotSupported +} - if doCount { - c++ - } - } - }() +func (crw *ContentRoutingWrapper) GetValue(context.Context, string, ...routing.Option) ([]byte, error) { + return nil, routing.ErrNotSupported +} - return out +func (crw *ContentRoutingWrapper) SearchValue(context.Context, string, ...routing.Option) (<-chan []byte, error) { + return nil, routing.ErrNotSupported } diff --git a/routing/wrapper_test.go b/routing/wrapper_test.go new file mode 100644 index 000000000000..dd5f2f446903 --- /dev/null +++ b/routing/wrapper_test.go @@ -0,0 +1,101 @@ +package routing + +import ( + "context" + "errors" + "testing" + + "github.com/multiformats/go-multihash" +) + +func TestProvideManyWrapper_ProvideMany(t *testing.T) { + type fields struct { + pms []ProvideMany + } + tests := []struct { + name string + fields fields + wantErr bool + ready bool + }{ + { + name: "one provider", + fields: fields{ + pms: []ProvideMany{ + newDummyProvideMany(true, false), + }, + }, + wantErr: false, + ready: true, + }, + { + name: "two providers, no errors and ready", + fields: fields{ + pms: []ProvideMany{ + newDummyProvideMany(true, false), + newDummyProvideMany(true, false), + }, + }, + wantErr: false, + ready: true, + }, + { + name: "two providers, no ready, no error", + fields: fields{ + pms: []ProvideMany{ + newDummyProvideMany(true, false), + newDummyProvideMany(false, false), + }, + }, + wantErr: false, + ready: false, + }, + { + name: "two providers, no ready, and one erroing", + fields: fields{ + pms: []ProvideMany{ + newDummyProvideMany(true, false), + newDummyProvideMany(false, true), + }, + }, + wantErr: true, + ready: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pmw := &ProvideManyWrapper{ + pms: tt.fields.pms, + } + if err := pmw.ProvideMany(context.Background(), nil); (err != nil) != tt.wantErr { + t.Errorf("ProvideManyWrapper.ProvideMany() error = %v, wantErr %v", err, tt.wantErr) + } + + if ready := pmw.Ready(); ready != tt.ready { + t.Errorf("ProvideManyWrapper.Ready() unexpected output = %v, want %v", ready, tt.ready) + } + }) + } +} + +func newDummyProvideMany(ready, failProviding bool) *dummyProvideMany { + return &dummyProvideMany{ + ready: ready, + failProviding: failProviding, + } +} + +type dummyProvideMany struct { + ready, failProviding bool +} + +func (dpm *dummyProvideMany) ProvideMany(ctx context.Context, keys []multihash.Multihash) error { + if dpm.failProviding { + return errors.New("error providing many") + } + + return nil +} +func (dpm *dummyProvideMany) Ready() bool { + return dpm.ready +}