From 0f5d3ca583303a95e01b221d489a21b73561da0f Mon Sep 17 00:00:00 2001 From: Juan Hernandez Date: Thu, 20 May 2021 11:53:34 +0200 Subject: [PATCH] Merge search parameter into search query for `list clusters` Currently when the `--parameter search="..."` flag is used in the `list clusters` command the result is that two `search` query parameters are sent to the server, one containing the value given by the `--parameter` flag and another containing the search query calculated from the rest of the flags. For example, the following command: ``` $ ocm list clusters --managed --parameter search="state is 'ready'" ``` Results in a URL like this: ``` https://api.openshift.com/foo/bar?search=managed+%27t%27&search=state+is+%27ready%27 ``` When the server receive that it will just ignore one of the values, so the result will not the expected combination of both search criteria. To address that this patch changes the `list clusters` command so that it will explicitly combine the search query calculated from other parameters with the values given with the `--parameter` flag. In the above example the result will be a URL like this: ``` https://api.openshift.com/api/clusters_mgmt/v1/clusters?search=%28managed+%3D+%27t%27%29+and+%28state+is+%27ready%27%29 ``` Note that this only applies to the `list clusters` command. The `--parameter search="..."` flag will still have the previous meaning in other places. Related: https://issues.redhat.com/browse/SDA-4201 Signed-off-by: Juan Hernandez --- cmd/ocm/list/cluster/cmd.go | 90 ++++++++++++++++++++++--------------- pkg/arguments/arguments.go | 22 +++++---- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/cmd/ocm/list/cluster/cmd.go b/cmd/ocm/list/cluster/cmd.go index 33e527bc..23cee07d 100644 --- a/cmd/ocm/list/cluster/cmd.go +++ b/cmd/ocm/list/cluster/cmd.go @@ -97,15 +97,55 @@ func run(cmd *cobra.Command, argv []string) error { } defer connection.Close() - // Get the client for the resource that manages the collection of clusters: - ocmClient := connection.ClustersMgmt().V1().Clusters() + // This will contain the terms used to construct the search query: + var searchTerms []string // If there is a parameter specified, assume its a filter: - var argFilter string if len(argv) == 1 && argv[0] != "" { - argFilter = fmt.Sprintf("(name like '%%%s%%' or id like '%%%s%%')", argv[0], argv[0]) + term := fmt.Sprintf("name like '%%%s%%' or id like '%%%s%%'", argv[0], argv[0]) + searchTerms = append(searchTerms, term) } + // Add the search term for the `--managed` flag: + if cmd.Flags().Changed("managed") { + var value string + if args.managed { + value = "t" + } else { + value = "f" + } + term := fmt.Sprintf("managed = '%s'", value) + searchTerms = append(searchTerms, term) + } + + // If the `search` parameter has been specified with the `--parameter` flag then we have to + // remove it and add the values to the list of search terms, otherwise we will be sending + // multiple `search` query parameters and the server will ignore all but one of them. Note + // that this modification of the `search` parameter isn't applicable in general, as other + // endpoints may assign a different meaning to the `search` parameter, so be careful if you + // try to apply this in other places. + var cleanParameters []string + for _, parameter := range args.parameter { + name, value := arguments.ParseNameValuePair(parameter) + if name == "search" { + searchTerms = append(searchTerms, value) + } else { + cleanParameters = append(cleanParameters, parameter) + } + } + args.parameter = cleanParameters + + // If there are more than one search term then we need to soround each of them with + // parenthesis before joining them with the `and` connective. + if len(searchTerms) > 1 { + for i, term := range searchTerms { + searchTerms[i] = fmt.Sprintf("(%s)", term) + } + } + + // Join all the search terms using the `and` connective: + searchQuery := strings.Join(searchTerms, " and ") + // Update our column name and padding variables: args.columns = strings.Replace(args.columns, " ", "", -1) colUpper := strings.ToUpper(args.columns) @@ -124,43 +164,19 @@ func run(cmd *cobra.Command, argv []string) error { table.PrintPadded(os.Stdout, columnNames, paddingByColumn) } + // Create the request. Note that this request can be created outside of the loop and used + // for all the iterations just changing the values of the `size` and `page` parameters. + request := connection.ClustersMgmt().V1().Clusters().List().Search(searchQuery) + arguments.ApplyParameterFlag(request, args.parameter) + arguments.ApplyHeaderFlag(request, args.header) + + // Send the request till we receive a page with less items than requested: size := 100 index := 1 for { // Fetch the next page: - request := ocmClient.List().Size(size).Page(index) - arguments.ApplyParameterFlag(request, args.parameter) - arguments.ApplyHeaderFlag(request, args.header) - var search strings.Builder - if cmd.Flags().Changed("managed") { - if search.Len() > 0 { - _, err = search.WriteString(" and ") - if err != nil { - return fmt.Errorf("Can't write to string: %v", err) - } - } - if args.managed { - _, err = search.WriteString("managed = 't'") - } else { - _, err = search.WriteString("managed = 'f'") - } - if err != nil { - return fmt.Errorf("Can't write to string: %v", err) - } - } - if argFilter != "" { - if search.Len() > 0 { - _, err = search.WriteString(" and ") - if err != nil { - return fmt.Errorf("Can't write to string: %v", err) - } - } - _, err = search.WriteString(argFilter) - if err != nil { - return fmt.Errorf("Can't write to string: %v", err) - } - } - request.Search(strings.TrimSpace(search.String())) + request.Size(size) + request.Page(index) response, err := request.Send() if err != nil { return fmt.Errorf("Can't retrieve clusters: %v", err) diff --git a/pkg/arguments/arguments.go b/pkg/arguments/arguments.go index 6d2e27d4..9f514e9c 100644 --- a/pkg/arguments/arguments.go +++ b/pkg/arguments/arguments.go @@ -252,14 +252,7 @@ func applyNVFlag(request interface{}, method string, values []string) { // Split the values into name value pairs and call the method for each one: for _, value := range values { var name string - position := strings.Index(value, "=") - if position != -1 { - name = value[:position] - value = value[position+1:] - } else { - name = value - value = "" - } + name, value = ParseNameValuePair(value) args := []reflect.Value{ reflect.ValueOf(name), reflect.ValueOf(value), @@ -307,3 +300,16 @@ func ApplyPathArg(request *sdk.Request, value string) error { func Split(r rune) bool { return r == '=' || r == ':' } + +// ParseNameValuePair parses a name value pair. +func ParseNameValuePair(text string) (name, value string) { + position := strings.Index(text, "=") + if position != -1 { + name = strings.TrimSpace(text[:position]) + value = text[position+1:] + } else { + name = strings.TrimSpace(text) + value = "" + } + return +}