PinSVC API: fix bugs and increase compliance

These changes fix a few bugs in the PinSVC API and have been discovered by
running the compliance tool at https://github.com/ipfs-shipyard/pinning-service-compliance.

In particular, the error objects did not respect the spec, filters and counts
were not done right. An additional PR will follow with fixes to the pintracker
because some pin status information was not correctly set.
This commit is contained in:
Hector Sanjuan 2022-06-17 16:59:22 +02:00
parent 57c3b180b5
commit bb8e8725e8
4 changed files with 58 additions and 28 deletions

View File

@ -67,7 +67,10 @@ func NewConfig() *Config {
cfg.DefaultFunc = defaultFunc
cfg.APIErrorFunc = func(err error, status int) error {
return pinsvc.APIError{
Reason: err.Error(),
Details: pinsvc.APIErrorDetails{
Reason: err.Error(),
Details: err.Error(),
},
}
}
return &cfg

View File

@ -24,12 +24,17 @@ func init() {
// APIError is returned by the API as a body when an error
// occurs. It implements the error interface.
type APIError struct {
Details APIErrorDetails `json:"error"`
}
// APIErrorDetails contains details about the APIError.
type APIErrorDetails struct {
Reason string `json:"reason"`
Details string `json:"details"`
Details string `json:"details,omitempty"`
}
func (apiErr APIError) Error() string {
return apiErr.Reason
return apiErr.Details.Reason
}
// PinName is a string limited to 255 chars when serializing JSON.
@ -54,9 +59,9 @@ func (pname *PinName) UnmarshalJSON(data []byte) error {
// Pin contains basic information about a Pin and pinning options.
type Pin struct {
Cid types.Cid `json:"cid"`
Name PinName `json:"name"`
Origins []types.Multiaddr `json:"origins"`
Meta map[string]string `json:"meta"`
Name PinName `json:"name,omitempty"`
Origins []types.Multiaddr `json:"origins,omitempty"`
Meta map[string]string `json:"meta,omitempty"`
}
// Defined returns if the pinis empty (Cid not set).
@ -221,12 +226,12 @@ type PinStatus struct {
Created time.Time `json:"created"`
Pin Pin `json:"pin"`
Delegates []types.Multiaddr `json:"delegates"`
Info map[string]string `json:"info"`
Info map[string]string `json:"info,omitempty"`
}
// PinList is the result of a call to List pins
type PinList struct {
Count int `json:"count"`
Count uint64 `json:"count"`
Results []PinStatus `json:"results"`
}
@ -293,6 +298,8 @@ func (lo *ListOptions) FromQuery(q url.Values) error {
return fmt.Errorf("error parsing 'limit' query param: %s: %w", v, err)
}
lo.Limit = lim
} else {
lo.Limit = 10 // implicit default
}
if meta := q.Get("meta"); meta != "" {

View File

@ -103,8 +103,10 @@ func globalPinInfoToSvcPinStatus(
Origins: gpi.Origins,
Meta: gpi.Metadata,
}
status.Info = apiInfo
status.Delegates = []types.Multiaddr{}
for _, pi := range gpi.PeerMap {
status.Delegates = append(status.Delegates, pi.IPFSAddresses...)
}
@ -228,6 +230,23 @@ func (api *API) addPin(w http.ResponseWriter, r *http.Request) {
return
}
// Unpin old item
if clusterPin.PinUpdate.Defined() {
var oldPin types.Pin
err = api.rpcClient.CallContext(
r.Context(),
"",
"Cluster",
"Unpin",
types.PinCid(clusterPin.PinUpdate),
&oldPin,
)
if err != nil {
api.SendResponse(w, common.SetStatusAutomatically, err, nil)
return
}
}
status := api.pinToSvcPinStatus(r.Context(), pin.Cid.String(), pinObj)
api.SendResponse(w, common.SetStatusAutomatically, nil, status)
}
@ -297,6 +316,9 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
tst := svcStatusToTrackerStatus(opts.Status)
var pinList pinsvc.PinList
pinList.Results = []pinsvc.PinStatus{}
count := uint64(0)
if len(opts.Cids) > 0 {
// copy approach from restapi
type statusResult struct {
@ -321,19 +343,18 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
}
var err error
i := uint64(0)
for stResult := range stCh {
if stResult.st.Status == pinsvc.StatusUndefined && stResult.err == nil {
// ignore things unpinning
continue
}
pinList.Results = append(pinList.Results, stResult.st)
err = multierr.Append(err, stResult.err)
if i+1 == opts.Limit {
break
if count < opts.Limit {
pinList.Results = append(pinList.Results, stResult.st)
err = multierr.Append(err, stResult.err)
}
i++
count++
}
if err != nil {
@ -360,18 +381,17 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
)
}()
i := uint64(0)
for gpi := range out {
st := globalPinInfoToSvcPinStatus(gpi.Cid.String(), gpi)
if st.Status == pinsvc.StatusUndefined {
// i.e things unpinning
continue
}
if st.Created.Before(opts.After) {
if !opts.After.IsZero() && st.Created.Before(opts.After) {
continue
}
if st.Created.After(opts.Before) {
if !opts.Before.IsZero() && st.Created.After(opts.Before) {
continue
}
@ -381,11 +401,10 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
if !st.Pin.MatchesMeta(opts.Meta) {
continue
}
pinList.Results = append(pinList.Results, st)
i++
if i == opts.Limit {
break
if count < opts.Limit {
pinList.Results = append(pinList.Results, st)
}
count++
}
err := <-errCh
@ -395,7 +414,7 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
}
}
pinList.Count = len(pinList.Results)
pinList.Count = count
api.SendResponse(w, common.SetStatusAutomatically, err, pinList)
}
@ -431,6 +450,7 @@ func (api *API) pinToSvcPinStatus(ctx context.Context, rID string, pin types.Pin
peers = pin.Allocations
}
status.Delegates = []types.Multiaddr{}
for _, peer := range peers {
var ipfsid types.IPFSID
err := api.rpcClient.CallContext(

View File

@ -116,14 +116,14 @@ func TestAPIListEndpoint(t *testing.T) {
// Test with cids+limit
var resp8 pinsvc.PinList
test.MakeGet(t, svcapi, url(svcapi)+"/pins?cid=QmP63DkAFEnDYNjDYBpyNDfttu1fvUw99x1brscPzpqmmq,QmP63DkAFEnDYNjDYBpyNDfttu1fvUw99x1brscPzpqmmb&limit=1", &resp8)
if resp8.Count != 1 {
if resp8.Count != 2 || len(resp8.Results) != 1 {
t.Errorf("unexpected statusAll+cids+limit resp:\n %+v", resp8)
}
// Test with limit
var resp9 pinsvc.PinList
test.MakeGet(t, svcapi, url(svcapi)+"/pins?limit=1", &resp9)
if resp9.Count != 1 {
if resp9.Count != 3 || len(resp9.Results) != 1 {
t.Errorf("unexpected statusAll+limit=1 resp:\n %+v", resp9)
}
@ -143,8 +143,8 @@ func TestAPIListEndpoint(t *testing.T) {
var errorResp pinsvc.APIError
test.MakeGet(t, svcapi, url(svcapi)+"/pins?status=invalid", &errorResp)
if errorResp.Reason == "" {
t.Errorf("expected an error: %s", errorResp.Reason)
if errorResp.Details.Reason == "" {
t.Errorf("expected an error: %s", errorResp.Details.Reason)
}
}
@ -200,7 +200,7 @@ func TestAPIPinEndpoint(t *testing.T) {
t.Fatal(err)
}
test.MakePost(t, svcapi, url(svcapi)+"/pins", pinJSON, &errName)
if !strings.Contains(errName.Reason, "255") {
if !strings.Contains(errName.Details.Reason, "255") {
t.Error("expected name error")
}
}
@ -231,7 +231,7 @@ func TestAPIGetPinEndpoint(t *testing.T) {
var err pinsvc.APIError
test.MakeGet(t, svcapi, url(svcapi)+"/pins/"+clustertest.ErrorCid.String(), &err)
if err.Reason == "" {
if err.Details.Reason == "" {
t.Error("expected an error")
}
}