Pinsvcapi: address comments from review

- Add "Created" field to pinInfo.
- Support before/after filter
- 404 when something is unpinned or on a non-recognize state
This commit is contained in:
Hector Sanjuan 2022-03-14 12:21:08 +01:00
parent 0c556ef4fa
commit e4b11b783b
7 changed files with 101 additions and 88 deletions

View File

@ -43,7 +43,7 @@ func (pname PinName) MarshalJSON() ([]byte, error) {
// UnmarshalJSON reads the JSON string and errors if over 256 chars.
func (pname *PinName) UnmarshalJSON(data []byte) error {
if len(data) > 255 { // should cast to string?
if len(data) > 257 { // "a_string" 255 + 2 for quotes
return errors.New("pin name is over 255 chars")
}
var v string
@ -97,10 +97,11 @@ func (p Pin) MatchesMeta(metaOpts map[string]string) bool {
return true
}
// Status represents a pin status.
// Status represents a pin status, which defines the current state of the pin
// in the system.
type Status int
// Status values
// Values for the Status type.
const (
StatusUndefined Status = 0
StatusQueued = 1 << iota
@ -171,7 +172,7 @@ func (st *Status) UnmarshalJSON(data []byte) error {
// ignored.
func StatusFromString(str string) Status {
values := strings.Split(strings.Replace(str, " ", "", -1), ",")
var status Status
status := StatusUndefined
for _, v := range values {
st, ok := stringStatus[v]
if ok {
@ -181,26 +182,10 @@ func StatusFromString(str string) Status {
return status
}
// PinStatus provides information about a Pin stored by the Pinning API.
type PinStatus struct {
RequestID string `json:"requestid"`
Status Status `json:"status"`
Created time.Time `json:"created"`
Pin Pin `json:"pin"`
Delegates []types.Multiaddr `json:"delegates"`
Info map[string]string `json:"info"`
}
// PinList is the result of a call to List pins
type PinList struct {
Count int `json:"count"`
Results []PinStatus `json:"results"`
}
// Match defines a type of match for filtering pin lists.
// MatchingStrategy defines a type of match for filtering pin lists.
type MatchingStrategy int
// Values for matches.
// Values for MatchingStrategy.
const (
MatchingStrategyUndefined MatchingStrategy = iota
MatchingStrategyExact
@ -225,6 +210,22 @@ func MatchingStrategyFromString(str string) MatchingStrategy {
}
}
// PinStatus provides information about a Pin stored by the Pinning API.
type PinStatus struct {
RequestID string `json:"requestid"`
Status Status `json:"status"`
Created time.Time `json:"created"`
Pin Pin `json:"pin"`
Delegates []types.Multiaddr `json:"delegates"`
Info map[string]string `json:"info"`
}
// PinList is the result of a call to List pins
type PinList struct {
Count int `json:"count"`
Results []PinStatus `json:"results"`
}
// ListOptions represents possible options given to the List endpoint.
type ListOptions struct {
Cids []cid.Cid
@ -261,6 +262,8 @@ func (lo *ListOptions) FromQuery(q url.Values) error {
}
statusStr := q.Get("status")
lo.Status = StatusFromString(statusStr)
// FIXME: This is a bit lazy, as "invalidxx,pinned" would result in a
// valid "pinned" filter.
if statusStr != "" && lo.Status == StatusUndefined {
return fmt.Errorf("error decoding 'status' query param: no valid filter")
}

View File

@ -33,6 +33,12 @@ var (
apiLogger = logging.Logger("pinsvcapilog")
)
var apiInfo map[string]string = map[string]string{
"source": "IPFS cluster API",
"warning1": "CID used for requestID. Conflicts possible",
"warning2": "experimental",
}
func trackerStatusToSvcStatus(st types.TrackerStatus) pinsvc.Status {
switch {
case st.Match(types.TrackerStatusError):
@ -95,26 +101,19 @@ func globalPinInfoToSvcPinStatus(
}
status.Status = trackerStatusToSvcStatus(statusMask)
status.Created = gpi.Created
status.Pin = pinsvc.Pin{
Cid: gpi.Cid.String(),
Name: pinsvc.PinName(gpi.Name),
Origins: gpi.Origins,
Meta: gpi.Metadata,
}
status.Info = apiInfo
for _, pi := range gpi.PeerMap {
status.Delegates = append(status.Delegates, pi.IPFSAddresses...)
// Set created to the oldest known timestamp
if status.Created.IsZero() || pi.TS.Before(status.Created) {
status.Created = pi.TS
}
}
status.Info = map[string]string{
"source": "IPFS cluster API",
"warning1": "CID used for requestID. Conflicts possible",
"warning2": "experimental",
}
return status
}
@ -206,21 +205,6 @@ func (api *API) parseRequestIDOrFail(w http.ResponseWriter, r *http.Request) (ci
return c, true
}
func (api *API) getPinStatus(ctx context.Context, c cid.Cid) (types.GlobalPinInfo, error) {
var pinInfo types.GlobalPinInfo
err := api.rpcClient.CallContext(
ctx,
"",
"Cluster",
"Status",
c,
&pinInfo,
)
return pinInfo, err
}
func (api *API) addPin(w http.ResponseWriter, r *http.Request) {
if pin := api.parseBodyOrFail(w, r); pin != nil {
api.config.Logger.Debugf("addPin: %s", pin.Cid)
@ -254,12 +238,21 @@ func (api *API) addPin(w http.ResponseWriter, r *http.Request) {
}
}
func (api *API) getPinObject(ctx context.Context, c cid.Cid) (pinsvc.PinStatus, types.GlobalPinInfo, error) {
clusterPinStatus, err := api.getPinStatus(ctx, c)
func (api *API) getPinSvcStatus(ctx context.Context, c cid.Cid) (pinsvc.PinStatus, error) {
var pinInfo types.GlobalPinInfo
err := api.rpcClient.CallContext(
ctx,
"",
"Cluster",
"Status",
c,
&pinInfo,
)
if err != nil {
return pinsvc.PinStatus{}, types.GlobalPinInfo{}, err
return pinsvc.PinStatus{}, err
}
return globalPinInfoToSvcPinStatus(c.String(), clusterPinStatus), clusterPinStatus, nil
return globalPinInfoToSvcPinStatus(c.String(), pinInfo), nil
}
@ -269,7 +262,11 @@ func (api *API) getPin(w http.ResponseWriter, r *http.Request) {
return
}
api.config.Logger.Debugf("getPin: %s", c)
status, _, err := api.getPinObject(r.Context(), c)
status, err := api.getPinSvcStatus(r.Context(), c)
if status.Status == pinsvc.StatusUndefined {
api.SendResponse(w, http.StatusNotFound, errors.New("pin not found"), nil)
return
}
api.SendResponse(w, common.SetStatusAutomatically, err, status)
}
@ -323,7 +320,7 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
for _, ci := range opts.Cids {
go func(c cid.Cid) {
defer wg.Done()
st, _, err := api.getPinObject(r.Context(), c)
st, err := api.getPinSvcStatus(r.Context(), c)
stCh <- statusResult{st: st, err: err}
}(ci)
}
@ -331,6 +328,11 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
var err error
i := 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 {
@ -359,6 +361,18 @@ func (api *API) listPins(w http.ResponseWriter, r *http.Request) {
}
for i, gpi := range globalPinInfos {
st := globalPinInfoToSvcPinStatus(gpi.Cid.String(), *gpi)
if st.Status == pinsvc.StatusUndefined {
// i.e things unpinning
continue
}
if st.Created.Before(opts.After) {
continue
}
if st.Created.After(opts.Before) {
continue
}
if !st.Pin.MatchesName(opts.Name, opts.MatchingStrategy) {
continue
}
@ -387,6 +401,7 @@ func (api *API) pinToSvcPinStatus(ctx context.Context, rID string, pin types.Pin
Origins: pin.Origins,
Meta: pin.Metadata,
},
Info: apiInfo,
}
var peers []peer.ID
@ -423,10 +438,5 @@ func (api *API) pinToSvcPinStatus(ctx context.Context, rID string, pin types.Pin
status.Delegates = append(status.Delegates, ipfsid.Addresses...)
}
status.Info = map[string]string{
"source": "IPFS cluster API",
"warning1": "CID used for requestID. Conflicts possible",
"warning2": "experimental",
}
return status
}

View File

@ -16,18 +16,6 @@ import (
ma "github.com/multiformats/go-multiaddr"
)
const (
SSLCertFile = "test/server.crt"
SSLKeyFile = "test/server.key"
clientOrigin = "myorigin"
validUserName = "validUserName"
validUserPassword = "validUserPassword"
adminUserName = "adminUserName"
adminUserPassword = "adminUserPassword"
invalidUserName = "invalidUserName"
invalidUserPassword = "invalidUserPassword"
)
func testAPIwithConfig(t *testing.T, cfg *Config, name string) *API {
ctx := context.Background()
apiMAddr, _ := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/0")
@ -38,22 +26,22 @@ func testAPIwithConfig(t *testing.T, cfg *Config, name string) *API {
cfg.HTTPListenAddr = []ma.Multiaddr{apiMAddr}
rest, err := NewAPIWithHost(ctx, cfg, h)
svcapi, err := NewAPIWithHost(ctx, cfg, h)
if err != nil {
t.Fatalf("should be able to create a new %s API: %s", name, err)
}
// No keep alive for tests
rest.SetKeepAlivesEnabled(false)
rest.SetClient(clustertest.NewMockRPCClient(t))
svcapi.SetKeepAlivesEnabled(false)
svcapi.SetClient(clustertest.NewMockRPCClient(t))
return rest
return svcapi
}
func testAPI(t *testing.T) *API {
cfg := NewConfig()
cfg.Default()
cfg.CORSAllowedOrigins = []string{clientOrigin}
cfg.CORSAllowedOrigins = []string{"myorigin"}
cfg.CORSAllowedMethods = []string{"GET", "POST", "DELETE"}
//cfg.CORSAllowedHeaders = []string{"Content-Type"}
cfg.CORSMaxAge = 10 * time.Minute
@ -165,8 +153,8 @@ func TestAPIListEndpoint(t *testing.T) {
func TestAPIPinEndpoint(t *testing.T) {
ctx := context.Background()
rest := testAPI(t)
defer rest.Shutdown(ctx)
svcapi := testAPI(t)
defer svcapi.Shutdown(ctx)
ma, _ := api.NewMultiaddr("/ip4/1.2.3.4/ipfs/" + clustertest.PeerID1.String())
@ -187,7 +175,7 @@ func TestAPIPinEndpoint(t *testing.T) {
if err != nil {
t.Fatal(err)
}
test.MakePost(t, rest, url(rest)+"/pins", pinJSON, &status)
test.MakePost(t, svcapi, url(svcapi)+"/pins", pinJSON, &status)
if status.Pin.Cid != pin.Cid {
t.Error("cids should match")
@ -211,7 +199,7 @@ func TestAPIPinEndpoint(t *testing.T) {
if err != nil {
t.Fatal(err)
}
test.MakePost(t, rest, url(rest)+"/pins", pinJSON, &errName)
test.MakePost(t, svcapi, url(svcapi)+"/pins", pinJSON, &errName)
if !strings.Contains(errName.Reason, "255") {
t.Error("expected name error")
}
@ -222,13 +210,13 @@ func TestAPIPinEndpoint(t *testing.T) {
func TestAPIGetPinEndpoint(t *testing.T) {
ctx := context.Background()
rest := testAPI(t)
defer rest.Shutdown(ctx)
svcapi := testAPI(t)
defer svcapi.Shutdown(ctx)
tf := func(t *testing.T, url test.URLFunc) {
// test existing pin
var status pinsvc.PinStatus
test.MakeGet(t, rest, url(rest)+"/pins/"+clustertest.Cid1.String(), &status)
test.MakeGet(t, svcapi, url(svcapi)+"/pins/"+clustertest.Cid1.String(), &status)
if status.Pin.Cid != clustertest.Cid1.String() {
t.Error("Cid should be set")
@ -242,7 +230,7 @@ func TestAPIGetPinEndpoint(t *testing.T) {
}
var err pinsvc.APIError
test.MakeGet(t, rest, url(rest)+"/pins/"+clustertest.ErrorCid.String(), &err)
test.MakeGet(t, svcapi, url(svcapi)+"/pins/"+clustertest.ErrorCid.String(), &err)
if err.Reason == "" {
t.Error("expected an error")
}
@ -253,12 +241,12 @@ func TestAPIGetPinEndpoint(t *testing.T) {
func TestAPIRemovePinEndpoint(t *testing.T) {
ctx := context.Background()
rest := testAPI(t)
defer rest.Shutdown(ctx)
svcapi := testAPI(t)
defer svcapi.Shutdown(ctx)
tf := func(t *testing.T, url test.URLFunc) {
// test existing pin
test.MakeDelete(t, rest, url(rest)+"/pins/"+clustertest.Cid1.String(), nil)
test.MakeDelete(t, svcapi, url(svcapi)+"/pins/"+clustertest.Cid1.String(), nil)
}
test.BothEndpoints(t, tf)

View File

@ -507,6 +507,8 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) {
filterStr := queryValues.Get("filter")
filter := types.TrackerStatusFromString(filterStr)
// FIXME: This is a bit lazy, as "invalidxx,pinned" would result in a
// valid "pinned" filter.
if filter == types.TrackerStatusUndefined && filterStr != "" {
api.SendResponse(w, http.StatusBadRequest, errors.New("invalid filter value"), nil)
return

View File

@ -254,6 +254,7 @@ type GlobalPinInfo struct {
Name string `json:"name" codec:"n"`
Allocations []peer.ID `json:"allocations" codec:"a,omitempty"`
Origins []Multiaddr `json:"origins" codec:"g,omitempty"`
Created time.Time `json:"created" codec:"t,omitempty"`
Metadata map[string]string `json:"metadata" codec:"m,omitempty"`
// https://github.com/golang/go/issues/28827
@ -279,6 +280,7 @@ func (gpi *GlobalPinInfo) Add(pi PinInfo) {
gpi.Name = pi.Name
gpi.Allocations = pi.Allocations
gpi.Origins = pi.Origins
gpi.Created = pi.Created
gpi.Metadata = pi.Metadata
}
@ -321,6 +323,7 @@ type PinInfo struct {
Peer peer.ID `json:"peer" codec:"p,omitempty"`
Allocations []peer.ID `json:"allocations" codec:"o,omitempty"`
Origins []Multiaddr `json:"origins" codec:"g,omitempty"`
Created time.Time `json:"created" codec:"t,omitempty"`
Metadata map[string]string `json:"metadata" codec:"md,omitempty"`
PinInfoShort

View File

@ -1758,6 +1758,7 @@ func (c *Cluster) setTrackerStatus(gpin *api.GlobalPinInfo, h cid.Cid, peers []p
Name: pin.Name,
Allocations: pin.Allocations,
Origins: pin.Origins,
Created: pin.Timestamp,
Metadata: pin.Metadata,
Peer: p,
PinInfoShort: api.PinInfoShort{
@ -1887,6 +1888,7 @@ func (c *Cluster) globalPinInfoCid(ctx context.Context, comp, method string, h c
Peer: dests[i],
Allocations: pin.Allocations,
Origins: pin.Origins,
Created: pin.Timestamp,
Metadata: pin.Metadata,
PinInfoShort: api.PinInfoShort{
PeerName: pv.Peername,
@ -1982,7 +1984,8 @@ func (c *Cluster) globalPinInfoSlice(ctx context.Context, comp, method string, a
Peer: p,
Allocations: nil,
Origins: nil,
Metadata: nil,
// Created: // leave unitialized
Metadata: nil,
PinInfoShort: api.PinInfoShort{
PeerName: pv.Peername,
IPFS: pv.IPFSID,

View File

@ -407,6 +407,7 @@ func (spt *Tracker) Status(ctx context.Context, c cid.Cid) *api.PinInfo {
pinInfo.TS = gpin.Timestamp
pinInfo.Allocations = gpin.Allocations
pinInfo.Origins = gpin.Origins
pinInfo.Created = gpin.Timestamp
pinInfo.Metadata = gpin.Metadata
// check if pin is a meta pin
@ -552,8 +553,9 @@ func (spt *Tracker) ipfsStatusAll(ctx context.Context) (map[cid.Cid]*api.PinInfo
Name: "", // to be filled later
Allocations: nil, // to be filled later
Origins: nil, // to be filled later
Metadata: nil, // to be filled later
Peer: spt.peerID,
//Created: nil, // to be filled later
Metadata: nil, // to be filled later
Peer: spt.peerID,
PinInfoShort: api.PinInfoShort{
PeerName: spt.peerName,
IPFS: ipfsid.ID,
@ -622,6 +624,7 @@ func (spt *Tracker) localStatus(ctx context.Context, incExtra bool, filter api.T
Peer: spt.peerID,
Allocations: p.Allocations,
Origins: p.Origins,
Created: p.Timestamp,
Metadata: p.Metadata,
PinInfoShort: api.PinInfoShort{
PeerName: spt.peerName,
@ -651,6 +654,7 @@ func (spt *Tracker) localStatus(ctx context.Context, incExtra bool, filter api.T
ipfsInfo.TS = p.Timestamp
ipfsInfo.Allocations = p.Allocations
ipfsInfo.Origins = p.Origins
ipfsInfo.Created = p.Timestamp
ipfsInfo.Metadata = p.Metadata
pininfos[p.Cid] = ipfsInfo
default: