From 7c04dd1accad5a68087dbd3368499d23dc404684 Mon Sep 17 00:00:00 2001 From: paul Date: Sat, 23 Jun 2018 19:11:33 -0400 Subject: [PATCH 01/16] Fix #445: Implemented status filter for ipfs-cluster-ctl License: MIT Signed-off-by: Paul Jewell --- api/rest/client/client.go | 3 ++ api/rest/client/methods.go | 2 +- api/rest/restapi.go | 65 ++++++++++++++++++++++++++++++++++++ cmd/ipfs-cluster-ctl/main.go | 10 ++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/api/rest/client/client.go b/api/rest/client/client.go index d802153b5..b05175441 100644 --- a/api/rest/client/client.go +++ b/api/rest/client/client.go @@ -156,6 +156,9 @@ type Config struct { // LogLevel defines the verbosity of the logging facility LogLevel string + + // List of filters for limiting results from ipfs-cluster-ctl + Filter string } // DefaultClient provides methods to interact with the ipfs-cluster API. Use diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index f919b34a1..480970453 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -138,7 +138,7 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error // StatusAll gathers Status() for all tracked items. func (c *defaultClient) StatusAll(local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial - err := c.do("GET", fmt.Sprintf("/pins?local=%t", local), nil, nil, &gpis) + err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, c.config.Filter), nil, nil, &gpis) result := make([]api.GlobalPinInfo, len(gpis)) for i, p := range gpis { result[i] = p.ToGlobalPinInfo() diff --git a/api/rest/restapi.go b/api/rest/restapi.go index 12ddeec67..efa16fdc7 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -669,6 +669,69 @@ func (api *API) allocationHandler(w http.ResponseWriter, r *http.Request) { } } +// defined groups / aliases for filtering +var filterGroups = map[string][]string{ + "error": {"error", "pin_error", "unpin_error", "cluster_error"}, + "queued": {"queued", "pin_queued", "unpin_queued"}, +} + +// check if any of the strings in matches are in the list +func anyStringInSlice(matches, list []string) bool { + for _, b := range list { + for _, d := range matches { + if len(b) > 0 && d == b { + return true + } + } + } + return false +} + +func checkFilterMatches(filterList []string, value string) bool { + // in case we have not defined any alises for the passed string, just use the string itself + if alias, exists := filterGroups[value]; exists { + return anyStringInSlice(alias, filterList) + } + return anyStringInSlice([]string{value}, filterList) +} + +func filterStatusAll(filter string, pinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { + if filter == "" { + return pinInfos + } + filterList := strings.Split(strings.ToLower(filter), ",") + for i := len(pinInfos) - 1; i >= 0; i-- { + removeThisEntry := true + for _, entry := range pinInfos[i].PeerMap { + if checkFilterMatches(filterList, entry.Status) { + removeThisEntry = false + break + } + } + // if we get to this point, no peers matched, so remove the group + if removeThisEntry == true { + pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) + } + } + return pinInfos +} + +func filterStatus(filter string, pinInfos []types.PinInfoSerial) []types.PinInfoSerial { + if filter == "" { + return pinInfos + } + filterList := strings.Split(strings.ToLower(filter), ",") + for i := len(pinInfos) - 1; i >= 0; i-- { + if checkFilterMatches(filterList, pinInfos[i].Status) { + // one peer matched, so we can just exit + break + } + pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) + + } + return pinInfos +} + func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { queryValues := r.URL.Query() local := queryValues.Get("local") @@ -680,6 +743,7 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { "StatusAllLocal", struct{}{}, &pinInfos) + pinInfos = filterStatus(queryValues.Get("filter"), pinInfos) api.sendResponse(w, autoStatus, err, pinInfosToGlobal(pinInfos)) } else { var pinInfos []types.GlobalPinInfoSerial @@ -688,6 +752,7 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { "StatusAll", struct{}{}, &pinInfos) + pinInfos = filterStatusAll(queryValues.Get("filter"), pinInfos) api.sendResponse(w, autoStatus, err, pinInfos) } } diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 56a5bec7b..34fea6829 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -98,6 +98,11 @@ func main() { Value: "", Usage: "cluster secret (32 byte pnet-key) as needed. Only when using the LibP2P endpoint", }, + cli.StringFlag{ + Name: "filter", + Value: "", + Usage: "filter for the 'status' command, may be one of status flags like 'pinned', 'error', 'queued' or a comma separated list", + }, cli.BoolFlag{ Name: "https, s", Usage: "use https to connect to the API", @@ -172,6 +177,11 @@ requires authorization. implies --https, which you can disable with --force-http checkErr("", errors.New("unsupported encoding")) } + filter := c.String("filter") + if filter != "" { + cfg.Filter = filter + } + globalClient, err = client.NewDefaultClient(cfg) checkErr("creating API client", err) return nil From 02af6c121fac78862979a91979e590fde62929e5 Mon Sep 17 00:00:00 2001 From: paul Date: Sat, 23 Jun 2018 21:27:18 -0400 Subject: [PATCH 02/16] Fix #445: Reduced length of filter help string License: MIT Signed-off-by: Paul Jewell --- cmd/ipfs-cluster-ctl/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 34fea6829..efb4dcde2 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -101,7 +101,7 @@ func main() { cli.StringFlag{ Name: "filter", Value: "", - Usage: "filter for the 'status' command, may be one of status flags like 'pinned', 'error', 'queued' or a comma separated list", + Usage: "filter for the 'status' command, may be one of status flags or a comma separated list", }, cli.BoolFlag{ Name: "https, s", From bda9633e82a4b4fc78c1126278476ac131db0fed Mon Sep 17 00:00:00 2001 From: paul Date: Wed, 27 Jun 2018 23:34:29 -0400 Subject: [PATCH 03/16] Fix #445: -Fixed logic issue in match condition of 'filterStatus' function -Added and verified success of test provided by @lanzafame -Attempted to condense code and apply other cleanup provided by @lanzafame License: MIT Signed-off-by: Paul Jewell --- api/rest/restapi.go | 57 ++++++++++++++++++---------------------- api/rest/restapi_test.go | 22 ++++++++++++++++ 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/api/rest/restapi.go b/api/rest/restapi.go index efa16fdc7..ae38dea4c 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -675,59 +675,52 @@ var filterGroups = map[string][]string{ "queued": {"queued", "pin_queued", "unpin_queued"}, } -// check if any of the strings in matches are in the list -func anyStringInSlice(matches, list []string) bool { - for _, b := range list { - for _, d := range matches { - if len(b) > 0 && d == b { - return true +func parseAllFilters(raw_filters string) map[string]bool { + // given the initial string of filters, return a slice of all individual strings + // to match on. This includes replacing alises + rawFilterList := strings.Split(strings.ToLower(raw_filters), ",") + var filterList = map[string]bool{} + + for _, raw_filter := range rawFilterList { + if alias_slice, exists := filterGroups[raw_filter]; exists { + for _, alias := range alias_slice { + filterList[alias] = true } + } else { + filterList[raw_filter] = true } } - return false -} + return filterList -func checkFilterMatches(filterList []string, value string) bool { - // in case we have not defined any alises for the passed string, just use the string itself - if alias, exists := filterGroups[value]; exists { - return anyStringInSlice(alias, filterList) - } - return anyStringInSlice([]string{value}, filterList) } func filterStatusAll(filter string, pinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { if filter == "" { return pinInfos } - filterList := strings.Split(strings.ToLower(filter), ",") - for i := len(pinInfos) - 1; i >= 0; i-- { - removeThisEntry := true - for _, entry := range pinInfos[i].PeerMap { - if checkFilterMatches(filterList, entry.Status) { - removeThisEntry = false - break + filterList := parseAllFilters(filter) + var filtered []types.GlobalPinInfoSerial +NEXTPI: + for _, pi := range pinInfos { + for _, entry := range pi.PeerMap { + if _, exists := filterList[entry.Status]; exists { + filtered = append(filtered, pi) + continue NEXTPI } } - // if we get to this point, no peers matched, so remove the group - if removeThisEntry == true { - pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) - } } - return pinInfos + return filtered } func filterStatus(filter string, pinInfos []types.PinInfoSerial) []types.PinInfoSerial { if filter == "" { return pinInfos } - filterList := strings.Split(strings.ToLower(filter), ",") + filterList := parseAllFilters(filter) for i := len(pinInfos) - 1; i >= 0; i-- { - if checkFilterMatches(filterList, pinInfos[i].Status) { - // one peer matched, so we can just exit - break + if _, exists := filterList[pinInfos[i].Status]; !exists { + pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) } - pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) - } return pinInfos } diff --git a/api/rest/restapi_test.go b/api/rest/restapi_test.go index 3d3a81a85..6c22ec7c2 100644 --- a/api/rest/restapi_test.go +++ b/api/rest/restapi_test.go @@ -641,6 +641,28 @@ func TestAPIStatusEndpoint(t *testing.T) { testBothEndpoints(t, tf) } +func TestAPIStatusAllFilterEndpoint(t *testing.T) { + rest := testAPI(t) + defer rest.Shutdown() + + tf := func(t *testing.T, url urlF) { + var resp []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=error", &resp) + if len(resp) != 1 { + t.Errorf("unexpected statusAll resp:\n %+v", resp) + } + + // Test local=true + var resp2 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?local=true&filter=error", &resp2) + if len(resp2) != 1 { + t.Errorf("unexpected statusAll+local resp:\n %+v \n %d", resp2, len(resp2)) + } + } + + testBothEndpoints(t, tf) +} + func TestAPISyncAllEndpoint(t *testing.T) { rest := testAPI(t) defer rest.Shutdown() From ed20241b3dafdf3ee297dd8fe9d2c1cb0d027626 Mon Sep 17 00:00:00 2001 From: paul Date: Wed, 27 Jun 2018 23:40:49 -0400 Subject: [PATCH 04/16] Fix #445: -Changed some 'snake case' to 'camel case' in accordance with code climate License: MIT Signed-off-by: Paul Jewell --- api/rest/restapi.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/rest/restapi.go b/api/rest/restapi.go index ae38dea4c..e58758c2e 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -675,19 +675,19 @@ var filterGroups = map[string][]string{ "queued": {"queued", "pin_queued", "unpin_queued"}, } -func parseAllFilters(raw_filters string) map[string]bool { +func parseAllFilters(rawFilters string) map[string]bool { // given the initial string of filters, return a slice of all individual strings // to match on. This includes replacing alises - rawFilterList := strings.Split(strings.ToLower(raw_filters), ",") + rawFilterList := strings.Split(strings.ToLower(rawFilters), ",") var filterList = map[string]bool{} - for _, raw_filter := range rawFilterList { - if alias_slice, exists := filterGroups[raw_filter]; exists { - for _, alias := range alias_slice { + for _, rawFilter := range rawFilterList { + if aliasSlice, exists := filterGroups[rawFilter]; exists { + for _, alias := range aliasSlice { filterList[alias] = true } } else { - filterList[raw_filter] = true + filterList[rawFilter] = true } } return filterList From f0321afd541d63ed42e37d3ae9bf82fc65a2049a Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Thu, 20 Dec 2018 13:32:26 +0530 Subject: [PATCH 05/16] Status filters for `ipfs-cluster-ctl status` Added filter option to `ipfs-cluster-ctl status` When the --filter is passed, it will only fetch the peer information where status of the pin matches with the filter value. Valid filter values are tracker status types(i.e., "pinned", "pin_error", "unpinning" etc), an alias of tracker status type (i.e., "queued" or "error"), comma separated list of tracker status type and/or it aliases(i.e., "error,pinning") On passing invalid filter value no status information will be shown In particular, the filter would remove elements from []GlobalPinInfo when none of the peers in GlobalPinInfo match the filter. If one peer in the GlobalPinInfo matches the filter, the whole object is returned, including the information for the other peers which may or not match it. filter option works on statusAll("GET /pins"). For fetching pin status for a CID("GET /pins/"), filter option would have no effect Fixes #445 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/rest/client/client.go | 5 +- api/rest/client/methods.go | 8 ++- api/rest/client/methods_test.go | 2 +- api/rest/restapi.go | 89 +++++++++++++++------------------ api/rest/restapi_test.go | 55 +++++++++++--------- cmd/ipfs-cluster-ctl/main.go | 26 ++++++---- 6 files changed, 94 insertions(+), 91 deletions(-) diff --git a/api/rest/client/client.go b/api/rest/client/client.go index b05175441..b5f177be8 100644 --- a/api/rest/client/client.go +++ b/api/rest/client/client.go @@ -71,7 +71,7 @@ type Client interface { // is fetched from all cluster peers. Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error) // StatusAll gathers Status() for all tracked items. - StatusAll(local bool) ([]api.GlobalPinInfo, error) + StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) // Sync makes sure the state of a Cid corresponds to the state reported // by the ipfs daemon, and returns it. If local is true, this operation @@ -156,9 +156,6 @@ type Config struct { // LogLevel defines the verbosity of the logging facility LogLevel string - - // List of filters for limiting results from ipfs-cluster-ctl - Filter string } // DefaultClient provides methods to interact with the ipfs-cluster API. Use diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index 480970453..e4c6d431a 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -136,9 +136,13 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error } // StatusAll gathers Status() for all tracked items. -func (c *defaultClient) StatusAll(local bool) ([]api.GlobalPinInfo, error) { +// If valid filter value is provided, it would fetch only those status information +// where status is matching the filter value +// Valid filter values are tracker status type, an alias of tracker status type +// (queued or error), comma separated list of track status type and/or it aliases +func (c *defaultClient) StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial - err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, c.config.Filter), nil, nil, &gpis) + err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, filter), nil, nil, &gpis) result := make([]api.GlobalPinInfo, len(gpis)) for i, p := range gpis { result[i] = p.ToGlobalPinInfo() diff --git a/api/rest/client/methods_test.go b/api/rest/client/methods_test.go index 14f579c1d..d6c10d9ee 100644 --- a/api/rest/client/methods_test.go +++ b/api/rest/client/methods_test.go @@ -219,7 +219,7 @@ func TestStatusAll(t *testing.T) { defer shutdown(api) testF := func(t *testing.T, c Client) { - pins, err := c.StatusAll(false) + pins, err := c.StatusAll("", false) if err != nil { t.Fatal(err) } diff --git a/api/rest/restapi.go b/api/rest/restapi.go index e58758c2e..6b1e8d4e4 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -669,85 +669,74 @@ func (api *API) allocationHandler(w http.ResponseWriter, r *http.Request) { } } -// defined groups / aliases for filtering -var filterGroups = map[string][]string{ - "error": {"error", "pin_error", "unpin_error", "cluster_error"}, - "queued": {"queued", "pin_queued", "unpin_queued"}, -} - -func parseAllFilters(rawFilters string) map[string]bool { - // given the initial string of filters, return a slice of all individual strings - // to match on. This includes replacing alises - rawFilterList := strings.Split(strings.ToLower(rawFilters), ",") - var filterList = map[string]bool{} - - for _, rawFilter := range rawFilterList { - if aliasSlice, exists := filterGroups[rawFilter]; exists { - for _, alias := range aliasSlice { - filterList[alias] = true - } - } else { - filterList[rawFilter] = true +func match(filters []string, status string) bool { + for _, filter := range filters { + if status == filter { + return true } - } - return filterList - -} - -func filterStatusAll(filter string, pinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { - if filter == "" { - return pinInfos - } - filterList := parseAllFilters(filter) - var filtered []types.GlobalPinInfoSerial -NEXTPI: - for _, pi := range pinInfos { - for _, entry := range pi.PeerMap { - if _, exists := filterList[entry.Status]; exists { - filtered = append(filtered, pi) - continue NEXTPI + if filter == "queued" || filter == "error" { + if strings.Contains(status, filter) { + return true } } } - return filtered + return false } -func filterStatus(filter string, pinInfos []types.PinInfoSerial) []types.PinInfoSerial { +func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { if filter == "" { - return pinInfos + return globalPinInfos } - filterList := parseAllFilters(filter) - for i := len(pinInfos) - 1; i >= 0; i-- { - if _, exists := filterList[pinInfos[i].Status]; !exists { - pinInfos = append(pinInfos[:i], pinInfos[i+1:]...) + filters := strings.Split(strings.ToLower(filter), ",") + + var filteredGlobalPinInfos []types.GlobalPinInfoSerial + + for _, globalPinInfo := range globalPinInfos { + for _, pinInfo := range globalPinInfo.PeerMap { + if match(filters, pinInfo.Status) { + filteredGlobalPinInfos = append(filteredGlobalPinInfos, globalPinInfo) + break + } } } - return pinInfos + + return filteredGlobalPinInfos } func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { queryValues := r.URL.Query() local := queryValues.Get("local") + var globalPinInfos []types.GlobalPinInfoSerial + if local == "true" { var pinInfos []types.PinInfoSerial + err := api.rpcClient.Call("", "Cluster", "StatusAllLocal", struct{}{}, &pinInfos) - pinInfos = filterStatus(queryValues.Get("filter"), pinInfos) - api.sendResponse(w, autoStatus, err, pinInfosToGlobal(pinInfos)) + if err != nil { + api.sendResponse(w, autoStatus, err, nil) + return + } + globalPinInfos = pinInfosToGlobal(pinInfos) } else { - var pinInfos []types.GlobalPinInfoSerial err := api.rpcClient.Call("", "Cluster", "StatusAll", struct{}{}, - &pinInfos) - pinInfos = filterStatusAll(queryValues.Get("filter"), pinInfos) - api.sendResponse(w, autoStatus, err, pinInfos) + &globalPinInfos) + if err != nil { + api.sendResponse(w, autoStatus, err, nil) + return + } } + + globalPinInfos = globalPinInfosByStatus(queryValues.Get("filter"), globalPinInfos) + + api.sendResponse(w, autoStatus, nil, globalPinInfos) } func (api *API) statusHandler(w http.ResponseWriter, r *http.Request) { diff --git a/api/rest/restapi_test.go b/api/rest/restapi_test.go index 6c22ec7c2..0db16421d 100644 --- a/api/rest/restapi_test.go +++ b/api/rest/restapi_test.go @@ -596,7 +596,38 @@ func TestAPIStatusAllEndpoint(t *testing.T) { var resp2 []api.GlobalPinInfoSerial makeGet(t, rest, url(rest)+"/pins?local=true", &resp2) if len(resp2) != 2 { - t.Errorf("unexpected statusAll+local resp:\n %+v", resp) + t.Errorf("unexpected statusAll+local resp:\n %+v", resp2) + } + + // Test with filter + var resp3 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=queue", &resp3) + if len(resp3) != 0 { + t.Errorf("unexpected statusAll+filter=queue resp:\n %+v", resp3) + } + + var resp4 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=pinned", &resp4) + if len(resp4) != 1 { + t.Errorf("unexpected statusAll+filter=pinned resp:\n %+v", resp4) + } + + var resp5 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=pin_error", &resp5) + if len(resp5) != 1 { + t.Errorf("unexpected statusAll+filter=pin_error resp:\n %+v", resp5) + } + + var resp6 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=error", &resp6) + if len(resp6) != 1 { + t.Errorf("unexpected statusAll+filter=error resp:\n %+v", resp6) + } + + var resp7 []api.GlobalPinInfoSerial + makeGet(t, rest, url(rest)+"/pins?filter=error,pinned", &resp7) + if len(resp7) != 2 { + t.Errorf("unexpected statusAll+filter=error,pinned resp:\n %+v", resp7) } } @@ -641,28 +672,6 @@ func TestAPIStatusEndpoint(t *testing.T) { testBothEndpoints(t, tf) } -func TestAPIStatusAllFilterEndpoint(t *testing.T) { - rest := testAPI(t) - defer rest.Shutdown() - - tf := func(t *testing.T, url urlF) { - var resp []api.GlobalPinInfoSerial - makeGet(t, rest, url(rest)+"/pins?filter=error", &resp) - if len(resp) != 1 { - t.Errorf("unexpected statusAll resp:\n %+v", resp) - } - - // Test local=true - var resp2 []api.GlobalPinInfoSerial - makeGet(t, rest, url(rest)+"/pins?local=true&filter=error", &resp2) - if len(resp2) != 1 { - t.Errorf("unexpected statusAll+local resp:\n %+v \n %d", resp2, len(resp2)) - } - } - - testBothEndpoints(t, tf) -} - func TestAPISyncAllEndpoint(t *testing.T) { rest := testAPI(t) defer rest.Shutdown() diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index efb4dcde2..697c49ceb 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -98,11 +98,6 @@ func main() { Value: "", Usage: "cluster secret (32 byte pnet-key) as needed. Only when using the LibP2P endpoint", }, - cli.StringFlag{ - Name: "filter", - Value: "", - Usage: "filter for the 'status' command, may be one of status flags or a comma separated list", - }, cli.BoolFlag{ Name: "https, s", Usage: "use https to connect to the API", @@ -177,11 +172,6 @@ requires authorization. implies --https, which you can disable with --force-http checkErr("", errors.New("unsupported encoding")) } - filter := c.String("filter") - if filter != "" { - cfg.Filter = filter - } - globalClient, err = client.NewDefaultClient(cfg) checkErr("creating API client", err) return nil @@ -624,10 +614,24 @@ with "sync". When the --local flag is passed, it will only fetch the status from the contacted cluster peer. By default, status will be fetched from all peers. + +When the --filter is passed, it will only fetch the peer information +where status of the pin matches with the filter value. +Valid filter values are tracker status types("pinned", "pin_error", "unpinning" etc), +an alias of tracker status type (queued or error), comma separated list of +tracker status type and/or it aliases ("error,pinning") +On passing invalid filter value no status information will be shown +List of tracker status types +https://github.com/ipfs/ipfs-cluster/blob/319c41cbf195b0453b8d1987991280d3121bac93/api/types.go#L66 + `, ArgsUsage: "[CID]", Flags: []cli.Flag{ localFlag(), + cli.StringFlag{ + Name: "filter", + Usage: "Comma seperated list of type tracker status and their aliases", + }, }, Action: func(c *cli.Context) error { cidStr := c.Args().First() @@ -637,7 +641,7 @@ contacted cluster peer. By default, status will be fetched from all peers. resp, cerr := globalClient.Status(ci, c.Bool("local")) formatResponse(c, resp, cerr) } else { - resp, cerr := globalClient.StatusAll(c.Bool("local")) + resp, cerr := globalClient.StatusAll(c.String("filter"), c.Bool("local")) formatResponse(c, resp, cerr) } return nil From 02e129fbfe9ca801d3696575f73fe8acc802e371 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Thu, 20 Dec 2018 16:42:04 +0530 Subject: [PATCH 06/16] Status filters for `ipfs-cluster-ctl status` Added clients tests Fixes #445 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/rest/client/methods_test.go | 26 ++++++++++++++++++++++++++ cmd/ipfs-cluster-ctl/main.go | 1 - 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/api/rest/client/methods_test.go b/api/rest/client/methods_test.go index d6c10d9ee..a0246101d 100644 --- a/api/rest/client/methods_test.go +++ b/api/rest/client/methods_test.go @@ -227,6 +227,32 @@ func TestStatusAll(t *testing.T) { if len(pins) == 0 { t.Error("there should be some pins") } + + // With local true + pins, err = c.StatusAll("", true) + if err != nil { + t.Fatal(err) + } + if len(pins) != 2 { + t.Error("there should be two pins") + } + + // With filter option + pins, err = c.StatusAll("pinning", false) + if err != nil { + t.Fatal(err) + } + if len(pins) != 1 { + t.Error("there should be one pin") + } + + pins, err = c.StatusAll("pinned,error", false) + if err != nil { + t.Fatal(err) + } + if len(pins) != 2 { + t.Error("there should be two pins") + } } testClients(t, api, testF) diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 697c49ceb..aa388f2c2 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -623,7 +623,6 @@ tracker status type and/or it aliases ("error,pinning") On passing invalid filter value no status information will be shown List of tracker status types https://github.com/ipfs/ipfs-cluster/blob/319c41cbf195b0453b8d1987991280d3121bac93/api/types.go#L66 - `, ArgsUsage: "[CID]", Flags: []cli.Flag{ From bed9672a7416c39e942f410f6d4a2050fd6479c3 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 26 Dec 2018 11:15:30 +0530 Subject: [PATCH 07/16] Status filters for `ipfs-cluster-ctl status` Added a fail case where an invalid filter is passed in. Update `api.rpcClient.Call` to `api.rpcClient.CallContext` and pass in the Request context r.Context() so that context can be cancelled when the request is cancelled by caller Fixes #445 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/rest/client/methods.go | 11 ++- api/rest/client/methods_test.go | 5 + api/rest/restapi.go | 158 +++++++++++++++++++++++--------- api/rest/restapi_test.go | 4 +- api/types.go | 30 ++++++ cmd/ipfs-cluster-ctl/main.go | 14 +-- 6 files changed, 165 insertions(+), 57 deletions(-) diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index e4c6d431a..f6ff16778 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -137,12 +137,17 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error // StatusAll gathers Status() for all tracked items. // If valid filter value is provided, it would fetch only those status information -// where status is matching the filter value +// where status is matching the filter value. // Valid filter values are tracker status type, an alias of tracker status type -// (queued or error), comma separated list of track status type and/or it aliases +// (queued or error), comma separated list of track status type and/or it aliases. func (c *defaultClient) StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial - err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, filter), nil, nil, &gpis) + + if !api.IsFilterValid(filter) { + return make([]api.GlobalPinInfo, len(gpis)), errors.New("invalid filter value") + } + + err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, url.QueryEscape(filter)), nil, nil, &gpis) result := make([]api.GlobalPinInfo, len(gpis)) for i, p := range gpis { result[i] = p.ToGlobalPinInfo() diff --git a/api/rest/client/methods_test.go b/api/rest/client/methods_test.go index a0246101d..6ed55a202 100644 --- a/api/rest/client/methods_test.go +++ b/api/rest/client/methods_test.go @@ -253,6 +253,11 @@ func TestStatusAll(t *testing.T) { if len(pins) != 2 { t.Error("there should be two pins") } + + pins, err = c.StatusAll("invalidfilter", false) + if err == nil { + t.Fatal("expected error") + } } testClients(t, api, testF) diff --git a/api/rest/restapi.go b/api/rest/restapi.go index 6b1e8d4e4..5d0e4e7e5 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -482,33 +482,42 @@ func (api *API) SetClient(c *rpc.Client) { func (api *API) idHandler(w http.ResponseWriter, r *http.Request) { idSerial := types.IDSerial{} - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "ID", struct{}{}, - &idSerial) + &idSerial, + ) api.sendResponse(w, autoStatus, err, idSerial) } func (api *API) versionHandler(w http.ResponseWriter, r *http.Request) { var v types.Version - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Version", struct{}{}, - &v) + &v, + ) api.sendResponse(w, autoStatus, err, v) } func (api *API) graphHandler(w http.ResponseWriter, r *http.Request) { var graph types.ConnectGraphSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "ConnectGraph", struct{}{}, - &graph) + &graph, + ) api.sendResponse(w, autoStatus, err, graph) } @@ -517,11 +526,14 @@ func (api *API) metricsHandler(w http.ResponseWriter, r *http.Request) { name := vars["name"] var metrics []types.Metric - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "PeerMonitorLatestMetrics", name, - &metrics) + &metrics, + ) api.sendResponse(w, autoStatus, err, metrics) } @@ -555,12 +567,14 @@ func (api *API) addHandler(w http.ResponseWriter, r *http.Request) { func (api *API) peerListHandler(w http.ResponseWriter, r *http.Request) { var peersSerial []types.IDSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Peers", struct{}{}, - &peersSerial) - + &peersSerial, + ) api.sendResponse(w, autoStatus, err, peersSerial) } @@ -582,21 +596,27 @@ func (api *API) peerAddHandler(w http.ResponseWriter, r *http.Request) { } var ids types.IDSerial - err = api.rpcClient.Call("", + err = api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "PeerAdd", addInfo.PeerID, - &ids) + &ids, + ) api.sendResponse(w, autoStatus, err, ids) } func (api *API) peerRemoveHandler(w http.ResponseWriter, r *http.Request) { if p := api.parsePidOrError(w, r); p != "" { - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "PeerRemove", p, - &struct{}{}) + &struct{}{}, + ) api.sendResponse(w, autoStatus, err, nil) } } @@ -605,11 +625,14 @@ func (api *API) pinHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { logger.Debugf("rest api pinHandler: %s", ps.Cid) - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Pin", ps, - &struct{}{}) + &struct{}{}, + ) api.sendResponse(w, http.StatusAccepted, err, nil) logger.Debug("rest api pinHandler done") } @@ -618,11 +641,14 @@ func (api *API) pinHandler(w http.ResponseWriter, r *http.Request) { func (api *API) unpinHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { logger.Debugf("rest api unpinHandler: %s", ps.Cid) - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Unpin", ps, - &struct{}{}) + &struct{}{}, + ) api.sendResponse(w, http.StatusAccepted, err, nil) logger.Debug("rest api unpinHandler done") } @@ -636,7 +662,8 @@ func (api *API) allocationsHandler(w http.ResponseWriter, r *http.Request) { filter |= types.PinTypeFromString(f) } var pins []types.PinSerial - err := api.rpcClient.Call( + err := api.rpcClient.CallContext( + r.Context(), "", "Cluster", "Pins", @@ -656,11 +683,14 @@ func (api *API) allocationsHandler(w http.ResponseWriter, r *http.Request) { func (api *API) allocationHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { var pin types.PinSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "PinGet", ps, - &pin) + &pin, + ) if err != nil { // errors here are 404s api.sendResponse(w, http.StatusNotFound, err, nil) return @@ -709,32 +739,43 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { var globalPinInfos []types.GlobalPinInfoSerial + filter := queryValues.Get("filter") + if !types.IsFilterValid(filter) { + api.sendResponse(w, autoStatus, errors.New("invalid filter value"), globalPinInfos) + } + if local == "true" { var pinInfos []types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "StatusAllLocal", struct{}{}, - &pinInfos) + &pinInfos, + ) if err != nil { api.sendResponse(w, autoStatus, err, nil) return } globalPinInfos = pinInfosToGlobal(pinInfos) } else { - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "StatusAll", struct{}{}, - &globalPinInfos) + &globalPinInfos, + ) if err != nil { api.sendResponse(w, autoStatus, err, nil) return } } - globalPinInfos = globalPinInfosByStatus(queryValues.Get("filter"), globalPinInfos) + globalPinInfos = globalPinInfosByStatus(filter, globalPinInfos) api.sendResponse(w, autoStatus, nil, globalPinInfos) } @@ -746,19 +787,25 @@ func (api *API) statusHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { if local == "true" { var pinInfo types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "StatusLocal", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfoToGlobal(pinInfo)) } else { var pinInfo types.GlobalPinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Status", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfo) } } @@ -770,19 +817,25 @@ func (api *API) syncAllHandler(w http.ResponseWriter, r *http.Request) { if local == "true" { var pinInfos []types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "SyncAllLocal", struct{}{}, - &pinInfos) + &pinInfos, + ) api.sendResponse(w, autoStatus, err, pinInfosToGlobal(pinInfos)) } else { var pinInfos []types.GlobalPinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "SyncAll", struct{}{}, - &pinInfos) + &pinInfos, + ) api.sendResponse(w, autoStatus, err, pinInfos) } } @@ -794,19 +847,25 @@ func (api *API) syncHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { if local == "true" { var pinInfo types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "SyncLocal", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfoToGlobal(pinInfo)) } else { var pinInfo types.GlobalPinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Sync", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfo) } } @@ -817,11 +876,14 @@ func (api *API) recoverAllHandler(w http.ResponseWriter, r *http.Request) { local := queryValues.Get("local") if local == "true" { var pinInfos []types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "RecoverAllLocal", struct{}{}, - &pinInfos) + &pinInfos, + ) api.sendResponse(w, autoStatus, err, pinInfosToGlobal(pinInfos)) } else { api.sendResponse(w, http.StatusBadRequest, errors.New("only requests with parameter local=true are supported"), nil) @@ -835,19 +897,25 @@ func (api *API) recoverHandler(w http.ResponseWriter, r *http.Request) { if ps := api.parseCidOrError(w, r); ps.Cid != "" { if local == "true" { var pinInfo types.PinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "RecoverLocal", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfoToGlobal(pinInfo)) } else { var pinInfo types.GlobalPinInfoSerial - err := api.rpcClient.Call("", + err := api.rpcClient.CallContext( + r.Context(), + "", "Cluster", "Recover", ps, - &pinInfo) + &pinInfo, + ) api.sendResponse(w, autoStatus, err, pinInfo) } } diff --git a/api/rest/restapi_test.go b/api/rest/restapi_test.go index 0db16421d..6e873761c 100644 --- a/api/rest/restapi_test.go +++ b/api/rest/restapi_test.go @@ -601,9 +601,9 @@ func TestAPIStatusAllEndpoint(t *testing.T) { // Test with filter var resp3 []api.GlobalPinInfoSerial - makeGet(t, rest, url(rest)+"/pins?filter=queue", &resp3) + makeGet(t, rest, url(rest)+"/pins?filter=queued", &resp3) if len(resp3) != 0 { - t.Errorf("unexpected statusAll+filter=queue resp:\n %+v", resp3) + t.Errorf("unexpected statusAll+filter=queued resp:\n %+v", resp3) } var resp4 []api.GlobalPinInfoSerial diff --git a/api/types.go b/api/types.go index 6f9fba4c9..b72f469d2 100644 --- a/api/types.go +++ b/api/types.go @@ -93,6 +93,36 @@ func TrackerStatusFromString(str string) TrackerStatus { return TrackerStatusBug } +func IsFilterValid(filter string) bool { + if filter == "" { + return true + } + + filters := strings.Split(strings.ToLower(filter), ",") + var valid bool + + for _, v := range filters { + valid = false + if v == "queued" || v == "error" { + valid = true + continue + } + + for _, v1 := range trackerStatusString { + if v1 == v { + valid = true + break + } + } + + if valid == false { + return false + } + } + + return true +} + // IPFSPinStatus values // FIXME include maxdepth const ( diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index aa388f2c2..01af022d8 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -617,19 +617,19 @@ contacted cluster peer. By default, status will be fetched from all peers. When the --filter is passed, it will only fetch the peer information where status of the pin matches with the filter value. -Valid filter values are tracker status types("pinned", "pin_error", "unpinning" etc), -an alias of tracker status type (queued or error), comma separated list of -tracker status type and/or it aliases ("error,pinning") -On passing invalid filter value no status information will be shown -List of tracker status types -https://github.com/ipfs/ipfs-cluster/blob/319c41cbf195b0453b8d1987991280d3121bac93/api/types.go#L66 +Valid filter values are tracker status types(full list of tracker status +types includes "bug", "cluster_error", "pin_error", "unpin_error", "pinned", +"pinning", "unpinning", "remote", "pin_queued" and "unpin_queued"), an +alias of tracker status type ("queued" or "error"), comma separated list of +tracker status type and/or it aliases ("error,pinning"). On passing invalid +filter value will throw an error. `, ArgsUsage: "[CID]", Flags: []cli.Flag{ localFlag(), cli.StringFlag{ Name: "filter", - Usage: "Comma seperated list of type tracker status and their aliases", + Usage: "Comma-separated list of filters", }, }, Action: func(c *cli.Context) error { From d0d903403ef34d0a349779ffdfd8636aab21a5ca Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 26 Dec 2018 13:50:54 +0530 Subject: [PATCH 08/16] Status filters for `ipfs-cluster-ctl status` Passing `make check` Fixes #445 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/types.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/types.go b/api/types.go index b72f469d2..36cc28b40 100644 --- a/api/types.go +++ b/api/types.go @@ -93,6 +93,7 @@ func TrackerStatusFromString(str string) TrackerStatus { return TrackerStatusBug } +// IsFilterValid checks if given filter is valid func IsFilterValid(filter string) bool { if filter == "" { return true @@ -108,8 +109,8 @@ func IsFilterValid(filter string) bool { continue } - for _, v1 := range trackerStatusString { - if v1 == v { + for _, tracker := range trackerStatusString { + if tracker == v { valid = true break } From 2c364ddf5e6976d7edc663e1dca136b7b07e4d2d Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Fri, 4 Jan 2019 23:27:26 +0530 Subject: [PATCH 09/16] Status filters for `ipfs-cluster-ctl status` Improved matching of filters and tracker status Fixes #445 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/rest/client/methods.go | 2 +- api/rest/restapi.go | 18 ++-------- api/types.go | 65 +++++++++++++++++++++++++++--------- api/types_test.go | 2 +- cmd/ipfs-cluster-ctl/main.go | 7 ++-- 5 files changed, 57 insertions(+), 37 deletions(-) diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index f6ff16778..fbe35145a 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -139,7 +139,7 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error // If valid filter value is provided, it would fetch only those status information // where status is matching the filter value. // Valid filter values are tracker status type, an alias of tracker status type -// (queued or error), comma separated list of track status type and/or it aliases. +// (queued or error), comma separated list of tracker status type and/or it aliases. func (c *defaultClient) StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial diff --git a/api/rest/restapi.go b/api/rest/restapi.go index 5d0e4e7e5..dbd368df3 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -699,20 +699,6 @@ func (api *API) allocationHandler(w http.ResponseWriter, r *http.Request) { } } -func match(filters []string, status string) bool { - for _, filter := range filters { - if status == filter { - return true - } - if filter == "queued" || filter == "error" { - if strings.Contains(status, filter) { - return true - } - } - } - return false -} - func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { if filter == "" { return globalPinInfos @@ -723,7 +709,9 @@ func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoS for _, globalPinInfo := range globalPinInfos { for _, pinInfo := range globalPinInfo.PeerMap { - if match(filters, pinInfo.Status) { + // silenced the error because we should have detected earlier if filters were invalid + pass, _ := types.Match(filters, pinInfo.Status) + if pass { filteredGlobalPinInfos = append(filteredGlobalPinInfos, globalPinInfo) break } diff --git a/api/types.go b/api/types.go index 36cc28b40..e710abda2 100644 --- a/api/types.go +++ b/api/types.go @@ -11,6 +11,7 @@ package api import ( "bytes" "encoding/json" + "errors" "fmt" "regexp" "sort" @@ -34,7 +35,7 @@ var logger = logging.Logger("apitypes") // TrackerStatus values const ( // IPFSStatus should never take this value - TrackerStatusBug TrackerStatus = iota + TrackerStatusBug TrackerStatus = 1 << iota // The cluster node is offline or not responding TrackerStatusClusterError // An error occurred pinning @@ -60,6 +61,12 @@ const ( TrackerStatusSharded ) +// Composite TrackerStatus +const ( + TrackerStatusError = TrackerStatusClusterError | TrackerStatusPinError | TrackerStatusUnpinError + TrackerStatusQueued = TrackerStatusPinQueued | TrackerStatusUnpinQueued +) + // TrackerStatus represents the status of a tracked Cid in the PinTracker type TrackerStatus int @@ -68,6 +75,7 @@ var trackerStatusString = map[TrackerStatus]string{ TrackerStatusClusterError: "cluster_error", TrackerStatusPinError: "pin_error", TrackerStatusUnpinError: "unpin_error", + TrackerStatusError: "error", TrackerStatusPinned: "pinned", TrackerStatusPinning: "pinning", TrackerStatusUnpinning: "unpinning", @@ -75,6 +83,7 @@ var trackerStatusString = map[TrackerStatus]string{ TrackerStatusRemote: "remote", TrackerStatusPinQueued: "pin_queued", TrackerStatusUnpinQueued: "unpin_queued", + TrackerStatusQueued: "queued", } // String converts a TrackerStatus into a readable string. @@ -82,6 +91,15 @@ func (st TrackerStatus) String() string { return trackerStatusString[st] } +// isValid checks if given tracker status is valid +func (st TrackerStatus) isValid() bool { + if st == TrackerStatusBug { + return false + } + + return true +} + // TrackerStatusFromString parses a string and returns the matching // TrackerStatus value. func TrackerStatusFromString(str string) TrackerStatus { @@ -93,6 +111,17 @@ func TrackerStatusFromString(str string) TrackerStatus { return TrackerStatusBug } +// TrackerStatusListString returns a string containing list of all +// tracker status values +func TrackerStatusListString() string { + var list strings.Builder + for _, v := range trackerStatusString { + list.WriteString(v + ", ") + } + + return list.String() +} + // IsFilterValid checks if given filter is valid func IsFilterValid(filter string) bool { if filter == "" { @@ -100,23 +129,9 @@ func IsFilterValid(filter string) bool { } filters := strings.Split(strings.ToLower(filter), ",") - var valid bool for _, v := range filters { - valid = false - if v == "queued" || v == "error" { - valid = true - continue - } - - for _, tracker := range trackerStatusString { - if tracker == v { - valid = true - break - } - } - - if valid == false { + if !TrackerStatusFromString(v).isValid() { return false } } @@ -124,6 +139,24 @@ func IsFilterValid(filter string) bool { return true } +// Match checks if given tracker status matches with the provided filters +func Match(filters []string, status string) (bool, error) { + trackerStatus := TrackerStatusFromString(status) + if trackerStatus == TrackerStatusBug { + return false, errors.New("invalid tracker status string") + } + for _, filter := range filters { + trackerStatusFilter := TrackerStatusFromString(filter) + if trackerStatusFilter == TrackerStatusBug { + return false, errors.New("invalid tracker status filter value") + } + if trackerStatus == trackerStatusFilter { + return true, nil + } + } + return false, nil +} + // IPFSPinStatus values // FIXME include maxdepth const ( diff --git a/api/types_test.go b/api/types_test.go index dc56d0912..03a447668 100644 --- a/api/types_test.go +++ b/api/types_test.go @@ -29,7 +29,7 @@ var testPeerID6, _ = peer.IDB58Decode("QmR8Vu6kZk7JvAN2rWVWgiduHatgBq2bb15Yyq8RR func TestTrackerFromString(t *testing.T) { testcases := []string{"bug", "cluster_error", "pin_error", "unpin_error", "pinned", "pinning", "unpinning", "unpinned", "remote"} for i, tc := range testcases { - if TrackerStatusFromString(tc).String() != TrackerStatus(i).String() { + if TrackerStatusFromString(tc).String() != TrackerStatus(1< Date: Sat, 5 Jan 2019 00:24:32 +0530 Subject: [PATCH 10/16] Wrap help message for less than 120 characters License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/types.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api/types.go b/api/types.go index e710abda2..0bdba5eec 100644 --- a/api/types.go +++ b/api/types.go @@ -115,8 +115,15 @@ func TrackerStatusFromString(str string) TrackerStatus { // tracker status values func TrackerStatusListString() string { var list strings.Builder + counter := 0 for _, v := range trackerStatusString { + counter += (len(v) + 2) list.WriteString(v + ", ") + + if counter > 90 { + list.WriteString("\n") + counter = 0 + } } return list.String() From 706558cdc92cb1a1ceb639aacda7d2dcf5d1a650 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Sat, 5 Jan 2019 14:40:54 +0530 Subject: [PATCH 11/16] Status filters for `ipfs-cluster-ctl status` Optimized filter to tracker status matching by using bitwise comparisions License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya --- api/rest/restapi.go | 3 +- api/types.go | 77 ++++++++++++++++-------------- cmd/ipfs-cluster-ctl/formatters.go | 12 ++++- cmd/ipfs-cluster-ctl/main.go | 4 +- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/api/rest/restapi.go b/api/rest/restapi.go index dbd368df3..f61877b1e 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -703,14 +703,13 @@ func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoS if filter == "" { return globalPinInfos } - filters := strings.Split(strings.ToLower(filter), ",") var filteredGlobalPinInfos []types.GlobalPinInfoSerial for _, globalPinInfo := range globalPinInfos { for _, pinInfo := range globalPinInfo.PeerMap { // silenced the error because we should have detected earlier if filters were invalid - pass, _ := types.Match(filters, pinInfo.Status) + pass, _ := types.Match(filter, pinInfo.Status) if pass { filteredGlobalPinInfos = append(filteredGlobalPinInfos, globalPinInfo) break diff --git a/api/types.go b/api/types.go index 0bdba5eec..decbeaff2 100644 --- a/api/types.go +++ b/api/types.go @@ -61,7 +61,7 @@ const ( TrackerStatusSharded ) -// Composite TrackerStatus +// Composite TrackerStatus. const ( TrackerStatusError = TrackerStatusClusterError | TrackerStatusPinError | TrackerStatusUnpinError TrackerStatusQueued = TrackerStatusPinQueued | TrackerStatusUnpinQueued @@ -91,13 +91,15 @@ func (st TrackerStatus) String() string { return trackerStatusString[st] } -// isValid checks if given tracker status is valid -func (st TrackerStatus) isValid() bool { - if st == TrackerStatusBug { - return false - } +// IsValid checks if given tracker status is valid. +func (st TrackerStatus) IsValid() bool { + return st != TrackerStatusBug +} - return true +// Match returns true if the tracker status matches the given filter. +// For example TrackerStatusPinError will match TrackerStatusPinError and TrackerStatusError +func (st TrackerStatus) Match(filter TrackerStatus) bool { + return st&filter > 0 } // TrackerStatusFromString parses a string and returns the matching @@ -111,25 +113,18 @@ func TrackerStatusFromString(str string) TrackerStatus { return TrackerStatusBug } -// TrackerStatusListString returns a string containing list of all -// tracker status values -func TrackerStatusListString() string { - var list strings.Builder - counter := 0 - for _, v := range trackerStatusString { - counter += (len(v) + 2) - list.WriteString(v + ", ") - - if counter > 90 { - list.WriteString("\n") - counter = 0 - } +// TrackerStatusAll returns a string containing list of all +// tracker status values. +func TrackerStatusAll() []TrackerStatus { + list := make([]TrackerStatus, 0) + for k := range trackerStatusString { + list = append(list, k) } - return list.String() + return list } -// IsFilterValid checks if given filter is valid +// IsFilterValid checks if given filter is valid. func IsFilterValid(filter string) bool { if filter == "" { return true @@ -138,7 +133,7 @@ func IsFilterValid(filter string) bool { filters := strings.Split(strings.ToLower(filter), ",") for _, v := range filters { - if !TrackerStatusFromString(v).isValid() { + if !TrackerStatusFromString(v).IsValid() { return false } } @@ -146,22 +141,30 @@ func IsFilterValid(filter string) bool { return true } -// Match checks if given tracker status matches with the provided filters -func Match(filters []string, status string) (bool, error) { - trackerStatus := TrackerStatusFromString(status) - if trackerStatus == TrackerStatusBug { - return false, errors.New("invalid tracker status string") +func toCombinedFilter(filter string) TrackerStatus { + filters := strings.Split(strings.ToLower(filter), ",") + + var combinedFilter TrackerStatus + for _, v := range filters { + combinedFilter |= TrackerStatusFromString(strings.TrimSpace(v)) } - for _, filter := range filters { - trackerStatusFilter := TrackerStatusFromString(filter) - if trackerStatusFilter == TrackerStatusBug { - return false, errors.New("invalid tracker status filter value") - } - if trackerStatus == trackerStatusFilter { - return true, nil - } + + return combinedFilter +} + +// Match checks if given tracker status matches with the provided filters. +func Match(filter string, status string) (bool, error) { + combinedFilter := toCombinedFilter(filter) + if !combinedFilter.IsValid() { + return false, errors.New("invalid tracker status filter value") } - return false, nil + + trackerStatus := TrackerStatusFromString(status) + if !trackerStatus.IsValid() { + return false, errors.New("invalid tracker status value") + } + + return trackerStatus.Match(combinedFilter), nil } // IPFSPinStatus values diff --git a/cmd/ipfs-cluster-ctl/formatters.go b/cmd/ipfs-cluster-ctl/formatters.go index 3cd081683..1b24d125f 100644 --- a/cmd/ipfs-cluster-ctl/formatters.go +++ b/cmd/ipfs-cluster-ctl/formatters.go @@ -8,8 +8,9 @@ import ( "strings" "time" - "github.com/ipfs/ipfs-cluster/api" peer "github.com/libp2p/go-libp2p-peer" + + "github.com/ipfs/ipfs-cluster/api" ) func jsonFormatObject(resp interface{}) { @@ -227,3 +228,12 @@ func textFormatPrintError(obj *api.Error) { fmt.Printf(" Code: %d\n", obj.Code) fmt.Printf(" Message: %s\n", obj.Message) } + +func trackerStatusAllString(list []api.TrackerStatus) string { + var builder strings.Builder + for _, ts := range list { + builder.WriteString("- " + ts.String() + "\n") + } + + return builder.String() +} diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 6bac316db..34aace77b 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -621,8 +621,10 @@ Valid filter values are tracker status types, an alias of tracker status type ("queued" or "error"), comma separated list of tracker status type and/or it aliases ("error,pinning"). On passing invalid filter value will throw an error. + Full list of tracker status types includes -` + api.TrackerStatusListString(), + +` + trackerStatusAllString(api.TrackerStatusAll()), ArgsUsage: "[CID]", Flags: []cli.Flag{ localFlag(), From 7de930b79686267f68f12024b625863dc1581dd2 Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Mon, 7 Jan 2019 19:45:07 +0100 Subject: [PATCH 12/16] Feat #445: Use TrackerStatus as filter. Simplify and small misc. License: MIT Signed-off-by: Hector Sanjuan --- api/rest/client/client.go | 2 +- api/rest/client/methods.go | 23 +++--- api/rest/client/methods_test.go | 12 +-- api/rest/restapi.go | 21 ++++-- api/types.go | 116 +++++++++++++---------------- cluster.go | 4 +- cmd/ipfs-cluster-ctl/formatters.go | 11 +-- cmd/ipfs-cluster-ctl/main.go | 20 +++-- pintracker/optracker/operation.go | 6 +- 9 files changed, 104 insertions(+), 111 deletions(-) diff --git a/api/rest/client/client.go b/api/rest/client/client.go index b5f177be8..97967cbf3 100644 --- a/api/rest/client/client.go +++ b/api/rest/client/client.go @@ -71,7 +71,7 @@ type Client interface { // is fetched from all cluster peers. Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error) // StatusAll gathers Status() for all tracked items. - StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) + StatusAll(filter api.TrackerStatus, local bool) ([]api.GlobalPinInfo, error) // Sync makes sure the state of a Cid corresponds to the state reported // by the ipfs daemon, and returns it. If local is true, this operation diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index fbe35145a..edef0861f 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -135,19 +135,22 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error return gpi.ToGlobalPinInfo(), err } -// StatusAll gathers Status() for all tracked items. -// If valid filter value is provided, it would fetch only those status information -// where status is matching the filter value. -// Valid filter values are tracker status type, an alias of tracker status type -// (queued or error), comma separated list of tracker status type and/or it aliases. -func (c *defaultClient) StatusAll(filter string, local bool) ([]api.GlobalPinInfo, error) { +// StatusAll gathers Status() for all tracked items. If a filter is +// provided, only entries matching the given filter statuses +// will be returned. A filter can be built by merging TrackerStatuses with +// a bitwise OR operation (st1 | st2 | ...). +func (c *defaultClient) StatusAll(filter api.TrackerStatus, local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial - if !api.IsFilterValid(filter) { - return make([]api.GlobalPinInfo, len(gpis)), errors.New("invalid filter value") + filterStr := "" + if filter != api.TrackerStatusUndefined { // undefined filter means "all" + filterStr = filter.String() + if filterStr == "" { + return nil, errors.New("invalid filter value") + } } - err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, url.QueryEscape(filter)), nil, nil, &gpis) + err := c.do("GET", fmt.Sprintf("/pins?local=%t&filter=%s", local, url.QueryEscape(filterStr)), nil, nil, &gpis) result := make([]api.GlobalPinInfo, len(gpis)) for i, p := range gpis { result[i] = p.ToGlobalPinInfo() @@ -341,7 +344,7 @@ func statusReached(target api.TrackerStatus, gblPinInfo api.GlobalPinInfo) (bool switch pinInfo.Status { case target: continue - case api.TrackerStatusBug, api.TrackerStatusClusterError, api.TrackerStatusPinError, api.TrackerStatusUnpinError: + case api.TrackerStatusUndefined, api.TrackerStatusClusterError, api.TrackerStatusPinError, api.TrackerStatusUnpinError: return false, fmt.Errorf("error has occurred while attempting to reach status: %s", target.String()) case api.TrackerStatusRemote: if target == api.TrackerStatusPinned { diff --git a/api/rest/client/methods_test.go b/api/rest/client/methods_test.go index 6ed55a202..d6269e59a 100644 --- a/api/rest/client/methods_test.go +++ b/api/rest/client/methods_test.go @@ -219,7 +219,7 @@ func TestStatusAll(t *testing.T) { defer shutdown(api) testF := func(t *testing.T, c Client) { - pins, err := c.StatusAll("", false) + pins, err := c.StatusAll(0, false) if err != nil { t.Fatal(err) } @@ -229,7 +229,7 @@ func TestStatusAll(t *testing.T) { } // With local true - pins, err = c.StatusAll("", true) + pins, err = c.StatusAll(0, true) if err != nil { t.Fatal(err) } @@ -238,7 +238,7 @@ func TestStatusAll(t *testing.T) { } // With filter option - pins, err = c.StatusAll("pinning", false) + pins, err = c.StatusAll(types.TrackerStatusPinning, false) if err != nil { t.Fatal(err) } @@ -246,7 +246,7 @@ func TestStatusAll(t *testing.T) { t.Error("there should be one pin") } - pins, err = c.StatusAll("pinned,error", false) + pins, err = c.StatusAll(types.TrackerStatusPinned|types.TrackerStatusError, false) if err != nil { t.Fatal(err) } @@ -254,9 +254,9 @@ func TestStatusAll(t *testing.T) { t.Error("there should be two pins") } - pins, err = c.StatusAll("invalidfilter", false) + pins, err = c.StatusAll(1<<25, false) if err == nil { - t.Fatal("expected error") + t.Error("expected an error") } } diff --git a/api/rest/restapi.go b/api/rest/restapi.go index f61877b1e..4bfb19d7e 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -699,8 +699,11 @@ func (api *API) allocationHandler(w http.ResponseWriter, r *http.Request) { } } -func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoSerial) []types.GlobalPinInfoSerial { - if filter == "" { +// filterGlobalPinInfos takes a GlobalPinInfo slice and discards +// any item in it which does not carry a PinInfo matching the +// filter (OR-wise). +func filterGlobalPinInfos(globalPinInfos []types.GlobalPinInfoSerial, filter types.TrackerStatus) []types.GlobalPinInfoSerial { + if filter == types.TrackerStatusUndefined { return globalPinInfos } @@ -708,9 +711,9 @@ func globalPinInfosByStatus(filter string, globalPinInfos []types.GlobalPinInfoS for _, globalPinInfo := range globalPinInfos { for _, pinInfo := range globalPinInfo.PeerMap { + st := types.TrackerStatusFromString(pinInfo.Status) // silenced the error because we should have detected earlier if filters were invalid - pass, _ := types.Match(filter, pinInfo.Status) - if pass { + if st.Match(filter) { filteredGlobalPinInfos = append(filteredGlobalPinInfos, globalPinInfo) break } @@ -726,9 +729,11 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { var globalPinInfos []types.GlobalPinInfoSerial - filter := queryValues.Get("filter") - if !types.IsFilterValid(filter) { - api.sendResponse(w, autoStatus, errors.New("invalid filter value"), globalPinInfos) + filterStr := queryValues.Get("filter") + filter := types.TrackerStatusFromString(filterStr) + if filter == types.TrackerStatusUndefined && filterStr != "" { + api.sendResponse(w, autoStatus, errors.New("invalid filter value"), nil) + return } if local == "true" { @@ -762,7 +767,7 @@ func (api *API) statusAllHandler(w http.ResponseWriter, r *http.Request) { } } - globalPinInfos = globalPinInfosByStatus(filter, globalPinInfos) + globalPinInfos = filterGlobalPinInfos(globalPinInfos, filter) api.sendResponse(w, autoStatus, nil, globalPinInfos) } diff --git a/api/types.go b/api/types.go index decbeaff2..433a93442 100644 --- a/api/types.go +++ b/api/types.go @@ -11,7 +11,6 @@ package api import ( "bytes" "encoding/json" - "errors" "fmt" "regexp" "sort" @@ -32,12 +31,21 @@ import ( var logger = logging.Logger("apitypes") +func init() { + // intialize trackerStatusString + stringTrackerStatus = make(map[string]TrackerStatus) + for k, v := range trackerStatusString { + stringTrackerStatus[v] = k + } +} + // TrackerStatus values const ( - // IPFSStatus should never take this value - TrackerStatusBug TrackerStatus = 1 << iota + // IPFSStatus should never take this value. + // When used as a filter. It means "all". + TrackerStatusUndefined TrackerStatus = 0 // The cluster node is offline or not responding - TrackerStatusClusterError + TrackerStatusClusterError TrackerStatus = 1 << iota // An error occurred pinning TrackerStatusPinError // An error occurred unpinning @@ -71,7 +79,7 @@ const ( type TrackerStatus int var trackerStatusString = map[TrackerStatus]string{ - TrackerStatusBug: "bug", + TrackerStatusUndefined: "undefined", TrackerStatusClusterError: "cluster_error", TrackerStatusPinError: "pin_error", TrackerStatusUnpinError: "unpin_error", @@ -86,85 +94,63 @@ var trackerStatusString = map[TrackerStatus]string{ TrackerStatusQueued: "queued", } +// values autofilled in init() +var stringTrackerStatus map[string]TrackerStatus + // String converts a TrackerStatus into a readable string. +// If the given TrackerStatus is a filter (with several +// bits set), it will return a comma-separated list. func (st TrackerStatus) String() string { - return trackerStatusString[st] -} + var values []string + + // simple and known composite values + if v, ok := trackerStatusString[st]; ok { + return v + } + + // other filters + for k, v := range trackerStatusString { + if st&k > 0 { + values = append(values, v) + } + } -// IsValid checks if given tracker status is valid. -func (st TrackerStatus) IsValid() bool { - return st != TrackerStatusBug + return strings.Join(values, ",") } // Match returns true if the tracker status matches the given filter. -// For example TrackerStatusPinError will match TrackerStatusPinError and TrackerStatusError +// For example TrackerStatusPinError will match TrackerStatusPinError +// and TrackerStatusError func (st TrackerStatus) Match(filter TrackerStatus) bool { - return st&filter > 0 + return filter == 0 || st&filter > 0 } // TrackerStatusFromString parses a string and returns the matching -// TrackerStatus value. +// TrackerStatus value. The string can be a comma-separated list +// representing a TrackerStatus filter. Unknown status names are +// ignored. func TrackerStatusFromString(str string) TrackerStatus { - for k, v := range trackerStatusString { - if v == str { - return k + values := strings.Split(strings.Replace(str, " ", "", -1), ",") + var status TrackerStatus + for _, v := range values { + st, ok := stringTrackerStatus[v] + if ok { + status |= st } } - return TrackerStatusBug + return status } -// TrackerStatusAll returns a string containing list of all -// tracker status values. +// TrackerStatusAll all known TrackerStatus values. func TrackerStatusAll() []TrackerStatus { - list := make([]TrackerStatus, 0) + var list []TrackerStatus for k := range trackerStatusString { - list = append(list, k) - } - - return list -} - -// IsFilterValid checks if given filter is valid. -func IsFilterValid(filter string) bool { - if filter == "" { - return true - } - - filters := strings.Split(strings.ToLower(filter), ",") - - for _, v := range filters { - if !TrackerStatusFromString(v).IsValid() { - return false + if k != TrackerStatusUndefined { + list = append(list, k) } } - return true -} - -func toCombinedFilter(filter string) TrackerStatus { - filters := strings.Split(strings.ToLower(filter), ",") - - var combinedFilter TrackerStatus - for _, v := range filters { - combinedFilter |= TrackerStatusFromString(strings.TrimSpace(v)) - } - - return combinedFilter -} - -// Match checks if given tracker status matches with the provided filters. -func Match(filter string, status string) (bool, error) { - combinedFilter := toCombinedFilter(filter) - if !combinedFilter.IsValid() { - return false, errors.New("invalid tracker status filter value") - } - - trackerStatus := TrackerStatusFromString(status) - if !trackerStatus.IsValid() { - return false, errors.New("invalid tracker status value") - } - - return trackerStatus.Match(combinedFilter), nil + return list } // IPFSPinStatus values @@ -227,7 +213,7 @@ var ipfsPinStatus2TrackerStatusMap = map[IPFSPinStatus]TrackerStatus{ IPFSPinStatusRecursive: TrackerStatusPinned, IPFSPinStatusIndirect: TrackerStatusUnpinned, IPFSPinStatusUnpinned: TrackerStatusUnpinned, - IPFSPinStatusBug: TrackerStatusBug, + IPFSPinStatusBug: TrackerStatusUndefined, IPFSPinStatusError: TrackerStatusClusterError, //TODO(ajl): check suitability } diff --git a/cluster.go b/cluster.go index 9d11ad28d..a75696d3d 100644 --- a/cluster.go +++ b/cluster.go @@ -1169,7 +1169,7 @@ func (c *Cluster) globalPinInfoCid(method string, h cid.Cid) (api.GlobalPinInfo, // Potentially rserial is empty. But ToPinInfo ignores all // errors from underlying libraries. In that case .Status - // will be TrackerStatusBug (0) + // will be TrackerStatusUndefined (0) r := rserial.ToPinInfo() // No error. Parse and continue @@ -1182,7 +1182,7 @@ func (c *Cluster) globalPinInfoCid(method string, h cid.Cid) (api.GlobalPinInfo, // In this case, we had no answer at all. The contacted peer // must be offline or unreachable. - if r.Status == api.TrackerStatusBug { + if r.Status == api.TrackerStatusUndefined { logger.Errorf("%s: error in broadcast response from %s: %s ", c.id, members[i], e) pin.PeerMap[members[i]] = api.PinInfo{ Cid: h, diff --git a/cmd/ipfs-cluster-ctl/formatters.go b/cmd/ipfs-cluster-ctl/formatters.go index 1b24d125f..2f68f3c27 100644 --- a/cmd/ipfs-cluster-ctl/formatters.go +++ b/cmd/ipfs-cluster-ctl/formatters.go @@ -229,11 +229,12 @@ func textFormatPrintError(obj *api.Error) { fmt.Printf(" Message: %s\n", obj.Message) } -func trackerStatusAllString(list []api.TrackerStatus) string { - var builder strings.Builder - for _, ts := range list { - builder.WriteString("- " + ts.String() + "\n") +func trackerStatusAllString() string { + var strs []string + for _, st := range api.TrackerStatusAll() { + strs = append(strs, " - "+st.String()) } - return builder.String() + sort.Strings(strs) + return strings.Join(strs, "\n") } diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 34aace77b..542dd142f 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -615,22 +615,17 @@ with "sync". When the --local flag is passed, it will only fetch the status from the contacted cluster peer. By default, status will be fetched from all peers. -When the --filter is passed, it will only fetch the peer information -where status of the pin matches with the filter value. -Valid filter values are tracker status types, an -alias of tracker status type ("queued" or "error"), comma separated list of -tracker status type and/or it aliases ("error,pinning"). On passing invalid -filter value will throw an error. +When the --filter flag is passed, it will only fetch the peer information +where status of the pin matches at least one of the filter values (a comma +separated list). The following are valid status values: -Full list of tracker status types includes - -` + trackerStatusAllString(api.TrackerStatusAll()), +` + trackerStatusAllString(), ArgsUsage: "[CID]", Flags: []cli.Flag{ localFlag(), cli.StringFlag{ Name: "filter", - Usage: "Comma-separated list of filters", + Usage: "comma-separated list of filters", }, }, Action: func(c *cli.Context) error { @@ -641,7 +636,10 @@ Full list of tracker status types includes resp, cerr := globalClient.Status(ci, c.Bool("local")) formatResponse(c, resp, cerr) } else { - resp, cerr := globalClient.StatusAll(c.String("filter"), c.Bool("local")) + resp, cerr := globalClient.StatusAll( + api.TrackerStatusFromString(c.String("filter")), + c.Bool("local"), + ) formatResponse(c, resp, cerr) } return nil diff --git a/pintracker/optracker/operation.go b/pintracker/optracker/operation.go index 321cd1f8f..cf5c462ec 100644 --- a/pintracker/optracker/operation.go +++ b/pintracker/optracker/operation.go @@ -175,7 +175,7 @@ func (op *Operation) ToTrackerStatus() api.TrackerStatus { case PhaseDone: return api.TrackerStatusPinned default: - return api.TrackerStatusBug + return api.TrackerStatusUndefined } case OperationUnpin: switch ph { @@ -188,14 +188,14 @@ func (op *Operation) ToTrackerStatus() api.TrackerStatus { case PhaseDone: return api.TrackerStatusUnpinned default: - return api.TrackerStatusBug + return api.TrackerStatusUndefined } case OperationRemote: return api.TrackerStatusRemote case OperationShard: return api.TrackerStatusSharded default: - return api.TrackerStatusBug + return api.TrackerStatusUndefined } } From 75702c21c9381516a470d527ed3d63005902861f Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Mon, 7 Jan 2019 19:47:44 +0100 Subject: [PATCH 13/16] Feat #445: Clarify about 0 filter value. License: MIT Signed-off-by: Hector Sanjuan --- api/rest/client/methods.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/rest/client/methods.go b/api/rest/client/methods.go index edef0861f..611bf1ed1 100644 --- a/api/rest/client/methods.go +++ b/api/rest/client/methods.go @@ -138,7 +138,8 @@ func (c *defaultClient) Status(ci cid.Cid, local bool) (api.GlobalPinInfo, error // StatusAll gathers Status() for all tracked items. If a filter is // provided, only entries matching the given filter statuses // will be returned. A filter can be built by merging TrackerStatuses with -// a bitwise OR operation (st1 | st2 | ...). +// a bitwise OR operation (st1 | st2 | ...). A "0" filter value (or +// api.TrackerStatusUndefined), means all. func (c *defaultClient) StatusAll(filter api.TrackerStatus, local bool) ([]api.GlobalPinInfo, error) { var gpis []api.GlobalPinInfoSerial From fa5906cc19619412c60b39eac706f1750bd50dcf Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Tue, 8 Jan 2019 12:28:29 +0100 Subject: [PATCH 14/16] Feat #445: Catch invalid filter strings in ipfs-cluster-ctl. License: MIT Signed-off-by: Hector Sanjuan --- cmd/ipfs-cluster-ctl/main.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/ipfs-cluster-ctl/main.go b/cmd/ipfs-cluster-ctl/main.go index 542dd142f..fcd61b1cd 100644 --- a/cmd/ipfs-cluster-ctl/main.go +++ b/cmd/ipfs-cluster-ctl/main.go @@ -636,10 +636,12 @@ separated list). The following are valid status values: resp, cerr := globalClient.Status(ci, c.Bool("local")) formatResponse(c, resp, cerr) } else { - resp, cerr := globalClient.StatusAll( - api.TrackerStatusFromString(c.String("filter")), - c.Bool("local"), - ) + filterFlag := c.String("filter") + filter := api.TrackerStatusFromString(c.String("filter")) + if filter == api.TrackerStatusUndefined && filterFlag != "" { + checkErr("parsing filter flag", errors.New("invalid filter name")) + } + resp, cerr := globalClient.StatusAll(filter, c.Bool("local")) formatResponse(c, resp, cerr) } return nil From 2e692e8dd1128436859847a53e5a913c6a0352a9 Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Tue, 8 Jan 2019 12:42:10 +0100 Subject: [PATCH 15/16] Issue #445: Fix test License: MIT Signed-off-by: Hector Sanjuan --- api/types_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/types_test.go b/api/types_test.go index 03a447668..de80eefa6 100644 --- a/api/types_test.go +++ b/api/types_test.go @@ -27,7 +27,7 @@ var testPeerID5, _ = peer.IDB58Decode("QmZVAo3wd8s5eTTy2kPYs34J9PvfxpKPuYsePPYGj var testPeerID6, _ = peer.IDB58Decode("QmR8Vu6kZk7JvAN2rWVWgiduHatgBq2bb15Yyq8RRhYSbx") func TestTrackerFromString(t *testing.T) { - testcases := []string{"bug", "cluster_error", "pin_error", "unpin_error", "pinned", "pinning", "unpinning", "unpinned", "remote"} + testcases := []string{"undefined", "cluster_error", "pin_error", "unpin_error", "pinned", "pinning", "unpinning", "unpinned", "remote"} for i, tc := range testcases { if TrackerStatusFromString(tc).String() != TrackerStatus(1< Date: Tue, 8 Jan 2019 16:18:33 +0100 Subject: [PATCH 16/16] Feat #445: Fix string test with TrackerStatus == 0 License: MIT Signed-off-by: Hector Sanjuan --- api/types_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/api/types_test.go b/api/types_test.go index de80eefa6..93367a0a0 100644 --- a/api/types_test.go +++ b/api/types_test.go @@ -27,12 +27,17 @@ var testPeerID5, _ = peer.IDB58Decode("QmZVAo3wd8s5eTTy2kPYs34J9PvfxpKPuYsePPYGj var testPeerID6, _ = peer.IDB58Decode("QmR8Vu6kZk7JvAN2rWVWgiduHatgBq2bb15Yyq8RRhYSbx") func TestTrackerFromString(t *testing.T) { - testcases := []string{"undefined", "cluster_error", "pin_error", "unpin_error", "pinned", "pinning", "unpinning", "unpinned", "remote"} + testcases := []string{"cluster_error", "pin_error", "unpin_error", "pinned", "pinning", "unpinning", "unpinned", "remote"} for i, tc := range testcases { - if TrackerStatusFromString(tc).String() != TrackerStatus(1<