Skip to content

Commit e1aa41c

Browse files
hacdiashacdias
committed
refactor: simplify provide and providepeer handlers
1 parent 342b01d commit e1aa41c

File tree

3 files changed

+52
-117
lines changed

3 files changed

+52
-117
lines changed

‎routing/http/client/client_test.go

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,16 @@ func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid, limi
3636
return args.Get(0).(iter.ResultIter[types.Record]), args.Error(1)
3737
}
3838

39-
func (m *mockContentRouter) Provide(ctx context.Context, req *server.ProvideRequest) (time.Duration, error) {
39+
func (m *mockContentRouter) Provide(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error) {
4040
// Ensure timestamps within tests are within the millisecond.
41-
req.Timestamp = req.Timestamp.Truncate(time.Millisecond)
41+
req.Payload.Timestamp = req.Payload.Timestamp.Truncate(time.Millisecond)
42+
43+
// Signature must be always present, but not possible to test with the mock.
44+
// We test it here and then empty it so that it matches what the mock expects.
45+
if req.Signature == "" {
46+
return 0, errors.New("signature not present")
47+
}
48+
req.Signature = ""
4249

4350
args := m.Called(ctx, req)
4451
return args.Get(0).(time.Duration), args.Error(1)
@@ -49,9 +56,16 @@ func (m *mockContentRouter) FindPeers(ctx context.Context, pid peer.ID, limit in
4956
return args.Get(0).(iter.ResultIter[*types.PeerRecord]), args.Error(1)
5057
}
5158

52-
func (m *mockContentRouter) ProvidePeer(ctx context.Context, req *server.ProvidePeerRequest) (time.Duration, error) {
59+
func (m *mockContentRouter) ProvidePeer(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error) {
5360
// Ensure timestamps within tests are within the millisecond.
54-
req.Timestamp = req.Timestamp.Truncate(time.Second)
61+
req.Payload.Timestamp = req.Payload.Timestamp.Truncate(time.Second)
62+
63+
// Signature must be always present, but not possible to test with the mock.
64+
// We test it here and then empty it so that it matches what the mock expects.
65+
if req.Signature == "" {
66+
return 0, errors.New("signature not present")
67+
}
68+
req.Signature = ""
5569

5670
args := m.Called(ctx, req)
5771
return args.Get(0).(time.Duration), args.Error(1)
@@ -155,13 +169,6 @@ func makeCID() cid.Cid {
155169
return c
156170
}
157171

158-
func drAddrsToAddrs(drmas []types.Multiaddr) (addrs []multiaddr.Multiaddr) {
159-
for _, a := range drmas {
160-
addrs = append(addrs, a.Multiaddr)
161-
}
162-
return
163-
}
164-
165172
func addrsToDRAddrs(addrs []multiaddr.Multiaddr) (drmas []types.Multiaddr) {
166173
for _, a := range addrs {
167174
drmas = append(drmas, types.Multiaddr{Multiaddr: a})
@@ -440,12 +447,15 @@ func TestClient_Provide(t *testing.T) {
440447
}
441448

442449
for _, cid := range c.cids {
443-
router.On("Provide", mock.Anything, &server.ProvideRequest{
444-
CID: cid,
445-
Timestamp: clock.Now().UTC().Truncate(time.Millisecond),
446-
TTL: c.ttl,
447-
Addrs: drAddrsToAddrs(client.addrs),
448-
ID: client.peerID,
450+
router.On("Provide", mock.Anything, &types.AnnouncementRecord{
451+
Schema: types.SchemaAnnouncement,
452+
Payload: types.AnnouncementPayload{
453+
CID: cid,
454+
Timestamp: clock.Now().UTC().Truncate(time.Millisecond),
455+
TTL: c.ttl,
456+
Addrs: client.addrs,
457+
ID: &client.peerID,
458+
},
449459
}).Return(c.routerAdvisoryTTL, c.routerErr)
450460
}
451461

@@ -708,11 +718,14 @@ func TestClient_ProvidePeer(t *testing.T) {
708718
}
709719
}
710720

711-
router.On("ProvidePeer", mock.Anything, &server.ProvidePeerRequest{
712-
Timestamp: clock.Now().UTC().Truncate(time.Second),
713-
TTL: c.ttl,
714-
Addrs: drAddrsToAddrs(client.addrs),
715-
ID: client.peerID,
721+
router.On("ProvidePeer", mock.Anything, &types.AnnouncementRecord{
722+
Schema: types.SchemaAnnouncement,
723+
Payload: types.AnnouncementPayload{
724+
Timestamp: clock.Now().UTC().Truncate(time.Second),
725+
TTL: c.ttl,
726+
Addrs: client.addrs,
727+
ID: &client.peerID,
728+
},
716729
}).Return(c.routerAdvisoryTTL, c.routerErr)
717730

718731
recs, err := client.ProvidePeer(ctx, c.ttl, nil)

‎routing/http/server/server.go

Lines changed: 9 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
jsontypes "github.com/ipfs/boxo/routing/http/types/json"
2323
"github.com/ipfs/go-cid"
2424
"github.com/libp2p/go-libp2p/core/peer"
25-
"github.com/multiformats/go-multiaddr"
2625

2726
logging "github.com/ipfs/go-log/v2"
2827
)
@@ -57,17 +56,17 @@ type ContentRouter interface {
5756
// Limit indicates the maximum amount of results to return; 0 means unbounded.
5857
FindProviders(ctx context.Context, cid cid.Cid, limit int) (iter.ResultIter[types.Record], error)
5958

60-
// Provide stores the provided [ProvideRequest] record for CIDs. Can return
59+
// Provide stores the provided [types.AnnouncementRecord] record for CIDs. Can return
6160
// a different TTL than the provided.
62-
Provide(ctx context.Context, req *ProvideRequest) (time.Duration, error)
61+
Provide(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error)
6362

6463
// FindPeers searches for peers who have the provided [peer.ID].
6564
// Limit indicates the maximum amount of results to return; 0 means unbounded.
6665
FindPeers(ctx context.Context, pid peer.ID, limit int) (iter.ResultIter[*types.PeerRecord], error)
6766

68-
// ProvidePeer stores the provided [ProvidePeerRequest] record for peers. Can
67+
// ProvidePeer stores the provided [types.AnnouncementRecord] record for peers. Can
6968
// return a different TTL than the provided.
70-
ProvidePeer(ctx context.Context, req *ProvidePeerRequest) (time.Duration, error)
69+
ProvidePeer(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error)
7170

7271
// GetIPNS searches for an [ipns.Record] for the given [ipns.Name].
7372
GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, error)
@@ -77,28 +76,6 @@ type ContentRouter interface {
7776
PutIPNS(ctx context.Context, name ipns.Name, record *ipns.Record) error
7877
}
7978

80-
// ProvideRequest is a content provide request.
81-
type ProvideRequest struct {
82-
CID cid.Cid
83-
Scope types.AnnouncementScope
84-
Timestamp time.Time
85-
TTL time.Duration
86-
ID peer.ID
87-
Addrs []multiaddr.Multiaddr
88-
Protocols []string
89-
Metadata string
90-
}
91-
92-
// ProvidePeerRequest is a peer provide request.
93-
type ProvidePeerRequest struct {
94-
Timestamp time.Time
95-
TTL time.Duration
96-
ID peer.ID
97-
Addrs []multiaddr.Multiaddr
98-
Protocols []string
99-
Metadata string
100-
}
101-
10279
type Option func(s *server)
10380

10481
// WithStreamingResultsDisabled disables ndjson responses, so that the server only supports JSON responses.
@@ -315,27 +292,14 @@ func (s *server) providePeers(w http.ResponseWriter, r *http.Request) {
315292
return resRecord
316293
}
317294

318-
req := &ProvidePeerRequest{
319-
Timestamp: reqRecord.Payload.Timestamp,
320-
TTL: reqRecord.Payload.TTL,
321-
ID: *reqRecord.Payload.ID,
322-
Addrs: make([]multiaddr.Multiaddr, len(reqRecord.Payload.Addrs)),
323-
Protocols: reqRecord.Payload.Protocols,
324-
Metadata: reqRecord.Payload.Metadata,
325-
}
326-
327-
for i, addr := range reqRecord.Payload.Addrs {
328-
req.Addrs[i] = addr.Multiaddr
329-
}
330-
331-
ttl, err := s.svc.ProvidePeer(r.Context(), req)
295+
ttl, err := s.svc.ProvidePeer(r.Context(), reqRecord)
332296
if err != nil {
333297
resRecord.Error = err.Error()
334298
return resRecord
335299
}
336300

337301
resRecord.Payload.TTL = ttl
338-
resRecord.Payload.ID = &req.ID
302+
resRecord.Payload.ID = reqRecord.Payload.ID
339303
return resRecord
340304
})
341305

@@ -374,30 +338,15 @@ func (s *server) provide(w http.ResponseWriter, r *http.Request) {
374338
return resRecord
375339
}
376340

377-
req := &ProvideRequest{
378-
CID: reqRecord.Payload.CID,
379-
Scope: reqRecord.Payload.Scope,
380-
Timestamp: reqRecord.Payload.Timestamp,
381-
TTL: reqRecord.Payload.TTL,
382-
ID: *reqRecord.Payload.ID,
383-
Addrs: make([]multiaddr.Multiaddr, len(reqRecord.Payload.Addrs)),
384-
Protocols: reqRecord.Payload.Protocols,
385-
Metadata: reqRecord.Payload.Metadata,
386-
}
387-
388-
for i, addr := range reqRecord.Payload.Addrs {
389-
req.Addrs[i] = addr.Multiaddr
390-
}
391-
392-
ttl, err := s.svc.Provide(r.Context(), req)
341+
ttl, err := s.svc.Provide(r.Context(), reqRecord)
393342
if err != nil {
394343
resRecord.Error = err.Error()
395344
return resRecord
396345
}
397346

398347
resRecord.Payload.TTL = ttl
399-
resRecord.Payload.CID = req.CID
400-
resRecord.Payload.ID = &req.ID
348+
resRecord.Payload.CID = reqRecord.Payload.CID
349+
resRecord.Payload.ID = reqRecord.Payload.ID
401350
return resRecord
402351
})
403352

‎routing/http/server/server_test.go

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/libp2p/go-libp2p/core/crypto"
2121
"github.com/libp2p/go-libp2p/core/peer"
2222
b58 "github.com/mr-tron/base58/base58"
23-
"github.com/multiformats/go-multiaddr"
2423
"github.com/multiformats/go-multihash"
2524
"github.com/stretchr/testify/mock"
2625
"github.com/stretchr/testify/require"
@@ -184,23 +183,9 @@ func TestProviders(t *testing.T) {
184183

185184
serverAddr := "http://" + server.Listener.Addr().String()
186185

187-
router.On("Provide", mock.Anything, &ProvideRequest{
188-
CID: cid1,
189-
Timestamp: rec1.Payload.Timestamp,
190-
TTL: rec1.Payload.TTL,
191-
ID: pid1,
192-
Addrs: []multiaddr.Multiaddr{},
193-
Protocols: rec1.Payload.Protocols,
194-
}).Return(time.Hour, nil)
195-
196-
router.On("Provide", mock.Anything, &ProvideRequest{
197-
CID: cid1,
198-
Timestamp: rec2.Payload.Timestamp,
199-
TTL: rec2.Payload.TTL,
200-
ID: pid2,
201-
Addrs: []multiaddr.Multiaddr{},
202-
Protocols: rec2.Payload.Protocols,
203-
}).Return(time.Minute, nil)
186+
router.On("Provide", mock.Anything, rec1).Return(time.Hour, nil)
187+
188+
router.On("Provide", mock.Anything, rec2).Return(time.Minute, nil)
204189

205190
urlStr := serverAddr + "/routing/v1/providers"
206191

@@ -368,21 +353,9 @@ func TestPeers(t *testing.T) {
368353

369354
serverAddr := "http://" + server.Listener.Addr().String()
370355

371-
router.On("ProvidePeer", mock.Anything, &ProvidePeerRequest{
372-
Timestamp: rec1.Payload.Timestamp,
373-
TTL: rec1.Payload.TTL,
374-
ID: pid1,
375-
Addrs: []multiaddr.Multiaddr{},
376-
Protocols: rec1.Payload.Protocols,
377-
}).Return(time.Hour, nil)
378-
379-
router.On("ProvidePeer", mock.Anything, &ProvidePeerRequest{
380-
Timestamp: rec2.Payload.Timestamp,
381-
TTL: rec2.Payload.TTL,
382-
ID: pid2,
383-
Addrs: []multiaddr.Multiaddr{},
384-
Protocols: rec2.Payload.Protocols,
385-
}).Return(time.Minute, nil)
356+
router.On("ProvidePeer", mock.Anything, rec1).Return(time.Hour, nil)
357+
358+
router.On("ProvidePeer", mock.Anything, rec2).Return(time.Minute, nil)
386359

387360
urlStr := serverAddr + "/routing/v1/peers"
388361

@@ -545,7 +518,7 @@ func (m *mockContentRouter) FindProviders(ctx context.Context, key cid.Cid, limi
545518
return args.Get(0).(iter.ResultIter[types.Record]), args.Error(1)
546519
}
547520

548-
func (m *mockContentRouter) Provide(ctx context.Context, req *ProvideRequest) (time.Duration, error) {
521+
func (m *mockContentRouter) Provide(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error) {
549522
args := m.Called(ctx, req)
550523
return args.Get(0).(time.Duration), args.Error(1)
551524
}
@@ -555,7 +528,7 @@ func (m *mockContentRouter) FindPeers(ctx context.Context, pid peer.ID, limit in
555528
return args.Get(0).(iter.ResultIter[*types.PeerRecord]), args.Error(1)
556529
}
557530

558-
func (m *mockContentRouter) ProvidePeer(ctx context.Context, req *ProvidePeerRequest) (time.Duration, error) {
531+
func (m *mockContentRouter) ProvidePeer(ctx context.Context, req *types.AnnouncementRecord) (time.Duration, error) {
559532
args := m.Called(ctx, req)
560533
return args.Get(0).(time.Duration), args.Error(1)
561534
}

0 commit comments

Comments
 (0)