Merge pull request #1704 from ipfs-cluster/pinsvc-compliance

PinSVC API: fix bugs and increase compliance

api/pinsvcapi/config.go

@@ -67,7 +67,9 @@ 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(),
+			},
 		}
 	}
 	return &cfg

api/pinsvcapi/pinsvc/pinsvc.go

@@ -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 != "" {

api/pinsvcapi/pinsvcapi.go

@@ -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(

api/pinsvcapi/pinsvcapi_test.go

@@ -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")
 		}
 	}