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 <[email protected]>

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)

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
+}