Skip to content

Commit

Permalink
refactor: address feedback (POST, sig prefix, etc)
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias committed Feb 6, 2024
1 parent 49d524a commit b2ea42a
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 48 deletions.
6 changes: 2 additions & 4 deletions routing/http/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ func (c *Client) Provide(ctx context.Context, announcements ...types.Announcemen
records[i] = record
}

// TODO: trailing slash?
url := c.baseURL + "/routing/v1/providers"
req := jsontypes.AnnounceProvidersRequest{
Providers: records,
Expand All @@ -293,7 +292,7 @@ func (c *Client) provide(ctx context.Context, url string, req interface{}) (iter
return nil, err
}

httpReq, err := http.NewRequestWithContext(ctx, http.MethodPut, url, bytes.NewBuffer(b))
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(b))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -465,10 +464,9 @@ func (c *Client) ProvidePeer(ctx context.Context, ttl time.Duration, metadata []
c.afterSignCallback(record)
}

// TODO: trailing slash?
url := c.baseURL + "/routing/v1/peers"
req := jsontypes.AnnouncePeersRequest{
Providers: []types.Record{record},
Peers: []types.Record{record},
}

return c.provide(ctx, url, req)
Expand Down
2 changes: 0 additions & 2 deletions routing/http/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ func TestClient_Provide(t *testing.T) {
Timestamp: clock.Now().UTC().Truncate(time.Millisecond),
TTL: c.ttl,
Addrs: drAddrsToAddrs(client.addrs),
Protocols: []string{},
ID: client.peerID,
}).Return(c.routerAdvisoryTTL, c.routerErr)
}
Expand Down Expand Up @@ -713,7 +712,6 @@ func TestClient_ProvidePeer(t *testing.T) {
Timestamp: clock.Now().UTC().Truncate(time.Second),
TTL: c.ttl,
Addrs: drAddrsToAddrs(client.addrs),
Protocols: []string{},
ID: client.peerID,
}).Return(c.routerAdvisoryTTL, c.routerErr)

Expand Down
10 changes: 5 additions & 5 deletions routing/http/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ const (
var logger = logging.Logger("routing/http/server")

const (
providePath = "/routing/v1/providers" // TODO: with trailing slash?
providePath = "/routing/v1/providers"
findProvidersPath = "/routing/v1/providers/{cid}"
providePeersPath = "/routing/v1/peers" // TODO: with trailing slash?
providePeersPath = "/routing/v1/peers"
findPeersPath = "/routing/v1/peers/{peer-id}"
getIPNSPath = "/routing/v1/ipns/{cid}"
)
Expand Down Expand Up @@ -139,9 +139,9 @@ func Handler(svc ContentRouter, opts ...Option) http.Handler {

r := mux.NewRouter()
r.HandleFunc(findProvidersPath, server.findProviders).Methods(http.MethodGet)
r.HandleFunc(providePath, server.provide).Methods(http.MethodPut)
r.HandleFunc(providePath, server.provide).Methods(http.MethodPost)
r.HandleFunc(findPeersPath, server.findPeers).Methods(http.MethodGet)
r.HandleFunc(providePeersPath, server.providePeers).Methods(http.MethodPut)
r.HandleFunc(providePeersPath, server.providePeers).Methods(http.MethodPost)
r.HandleFunc(getIPNSPath, server.GetIPNS).Methods(http.MethodGet)
r.HandleFunc(getIPNSPath, server.PutIPNS).Methods(http.MethodPut)
return r
Expand Down Expand Up @@ -304,7 +304,7 @@ func (s *server) providePeers(w http.ResponseWriter, r *http.Request) {
return
}

responseIter := iter.Map[types.Record, *types.AnnouncementRecord](iter.FromSlice(req.Providers), func(t types.Record) *types.AnnouncementRecord {
responseIter := iter.Map[types.Record, *types.AnnouncementRecord](iter.FromSlice(req.Peers), func(t types.Record) *types.AnnouncementRecord {
resRecord := &types.AnnouncementRecord{
Schema: types.SchemaAnnouncement,
}
Expand Down
22 changes: 11 additions & 11 deletions routing/http/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func TestProviders(t *testing.T) {

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

httpReq, err := http.NewRequest(http.MethodPut, urlStr, bytes.NewReader(body))
httpReq, err := http.NewRequest(http.MethodPost, urlStr, bytes.NewReader(body))
require.NoError(t, err)
httpReq.Header.Set("Accept", contentType)

Expand All @@ -220,12 +220,12 @@ func TestProviders(t *testing.T) {
require.Equal(t, expectedBody, string(body))
}

t.Run("PUT /routing/v1/providers (JSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeJSON, `{"ProvideResults":[{"Schema":"announcement","Payload":{"Addrs":[],"CID":"`+cid1Str+`","ID":"`+pid1Str+`","Protocols":[],"TTL":3600000}},{"Schema":"announcement","Payload":{"Addrs":[],"CID":"`+cid1Str+`","ID":"`+pid2Str+`","Protocols":[],"TTL":60000}}]}`)
t.Run("POST /routing/v1/providers (JSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeJSON, `{"ProvideResults":[{"Schema":"announcement","Payload":{"CID":"`+cid1Str+`","ID":"`+pid1Str+`","TTL":3600000}},{"Schema":"announcement","Payload":{"CID":"`+cid1Str+`","ID":"`+pid2Str+`","TTL":60000}}]}`)
})

t.Run("PUT /routing/v1/providers (NDJSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeNDJSON, `{"Schema":"announcement","Payload":{"Addrs":[],"CID":"`+cid1Str+`","ID":"`+pid1Str+`","Protocols":[],"TTL":3600000}}`+"\n"+`{"Schema":"announcement","Payload":{"Addrs":[],"CID":"`+cid1Str+`","ID":"`+pid2Str+`","Protocols":[],"TTL":60000}}`+"\n")
t.Run("POST /routing/v1/providers (NDJSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeNDJSON, `{"Schema":"announcement","Payload":{"CID":"`+cid1Str+`","ID":"`+pid1Str+`","TTL":3600000}}`+"\n"+`{"Schema":"announcement","Payload":{"CID":"`+cid1Str+`","ID":"`+pid2Str+`","TTL":60000}}`+"\n")
})
}

Expand Down Expand Up @@ -358,7 +358,7 @@ func TestPeers(t *testing.T) {
err = rec2.Sign(pid2, sk2)
require.NoError(t, err)

req := tjson.AnnounceProvidersRequest{Providers: []types.Record{rec1, rec2}}
req := tjson.AnnouncePeersRequest{Peers: []types.Record{rec1, rec2}}
body, err := json.Marshal(req)
require.NoError(t, err)

Expand Down Expand Up @@ -386,7 +386,7 @@ func TestPeers(t *testing.T) {

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

httpReq, err := http.NewRequest(http.MethodPut, urlStr, bytes.NewReader(body))
httpReq, err := http.NewRequest(http.MethodPost, urlStr, bytes.NewReader(body))
require.NoError(t, err)
httpReq.Header.Set("Accept", contentType)

Expand All @@ -402,12 +402,12 @@ func TestPeers(t *testing.T) {
require.Equal(t, expectedBody, string(body))
}

t.Run("PUT /routing/v1/peers (JSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeJSON, `{"ProvideResults":[{"Schema":"announcement","Payload":{"Addrs":[],"ID":"`+pid1.String()+`","Protocols":[],"TTL":3600000}},{"Schema":"announcement","Payload":{"Addrs":[],"ID":"`+pid2.String()+`","Protocols":[],"TTL":60000}}]}`)
t.Run("POST /routing/v1/peers (JSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeJSON, `{"ProvideResults":[{"Schema":"announcement","Payload":{"ID":"`+pid1.String()+`","TTL":3600000}},{"Schema":"announcement","Payload":{"ID":"`+pid2.String()+`","TTL":60000}}]}`)
})

t.Run("PUT /routing/v1/peers (NDJSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeNDJSON, `{"Schema":"announcement","Payload":{"Addrs":[],"ID":"`+pid1.String()+`","Protocols":[],"TTL":3600000}}`+"\n"+`{"Schema":"announcement","Payload":{"Addrs":[],"ID":"`+pid2.String()+`","Protocols":[],"TTL":60000}}`+"\n")
t.Run("POST /routing/v1/peers (NDJSON Response)", func(t *testing.T) {
runPutTest(t, mediaTypeNDJSON, `{"Schema":"announcement","Payload":{"ID":"`+pid1.String()+`","TTL":3600000}}`+"\n"+`{"Schema":"announcement","Payload":{"ID":"`+pid2.String()+`","TTL":60000}}`+"\n")
})
}

Expand Down
10 changes: 5 additions & 5 deletions routing/http/types/json/requests.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package json

// AnnounceProvidersRequest is the content of a PUT Providers request.
// AnnounceProvidersRequest is the content of a POST Providers request.
type AnnounceProvidersRequest struct {
Error string
Providers RecordsArray
}

// AnnouncePeersRequest is the content of a PUT Peers request.
// TODO: is the the same? Shouldn't Providers be Peers?
type AnnouncePeersRequest = AnnounceProvidersRequest
// AnnouncePeersRequest is the content of a POST Peers request.
type AnnouncePeersRequest struct {
Peers RecordsArray
}
4 changes: 2 additions & 2 deletions routing/http/types/json/responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ func (r *RecordsArray) UnmarshalJSON(b []byte) error {
return nil
}

// AnnounceProvidersResponse is the result of a PUT Providers request.
// AnnounceProvidersResponse is the result of a POST Providers request.
type AnnounceProvidersResponse struct {
ProvideResults []*types.AnnouncementRecord
}

// AnnouncePeersResponse is the result of a PUT Peers request.
// AnnouncePeersResponse is the result of a POST Peers request.
type AnnouncePeersResponse = AnnounceProvidersResponse
38 changes: 19 additions & 19 deletions routing/http/types/record_announcement.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
)

const SchemaAnnouncement = "announcement"
const announcementSignaturePrefix = "PUT /routing/v1 announcement:"
const announcementSignaturePrefix = "routing-record:"

var _ Record = &AnnouncementRecord{}

Expand Down Expand Up @@ -57,12 +57,10 @@ func (ap AnnouncementPayload) MarshalJSON() ([]byte, error) {
m := make(map[string]ipld.Node)
var err error

// TODO: or goes empty? Affects signature.
if ap.CID.Defined() {
m["CID"] = basicnode.NewString(ap.CID.String())
}

// TODO: is scope not set when empty? Affects signature.
if ap.Scope != "" {
m["Scope"] = basicnode.NewString(string(ap.Scope))
}
Expand All @@ -83,24 +81,26 @@ func (ap AnnouncementPayload) MarshalJSON() ([]byte, error) {
m["Metadata"] = basicnode.NewString(ap.Metadata)
}

// TODO: or goes empty if len == 0? Affects signature.
addrs := []ipld.Node{}
for _, addr := range ap.Addrs {
addrs = append(addrs, basicnode.NewString(addr.String()))
}
m["Addrs"], err = makeIPLDList(addrs)
if err != nil {
return nil, err
if len(ap.Addrs) != 0 {
addrs := []ipld.Node{}
for _, addr := range ap.Addrs {
addrs = append(addrs, basicnode.NewString(addr.String()))
}
m["Addrs"], err = makeIPLDList(addrs)
if err != nil {
return nil, err
}
}

// TODO: or goes empty if len == 0? Affects signature.
protocols := []ipld.Node{}
for _, protocol := range ap.Protocols {
protocols = append(addrs, basicnode.NewString(protocol))
}
m["Protocols"], err = makeIPLDList(protocols)
if err != nil {
return nil, err
if len(ap.Protocols) != 0 {
protocols := []ipld.Node{}
for _, protocol := range ap.Protocols {
protocols = append(protocols, basicnode.NewString(protocol))
}
m["Protocols"], err = makeIPLDList(protocols)
if err != nil {
return nil, err
}
}

// Make final IPLD node.
Expand Down

0 comments on commit b2ea42a

Please sign in to comment.