A set of device list cleanups (#397)

* Cleanup: pass device filter options as a map

This allows to fully move an argument parsing logic from the client into the command.
Before the change, an argument parsing logic was split between the two.

* Cleanup: use name instead of name_ilike to filter device list

A new API has a newer name argument for few months already.
Use it instead of the obsolete name_ilike.

* Cleanup: use net/url to escape special characters in device list

Currently, we onlhy special-cased the URL escaping of a ? for device name pattern.
But, other characters are not escaped, and uuid pattern is not escaped at all.

Most of the time this just works, but may produce unexpected results.
For example `fioctl devices list 'a*&'` will ignore the trailing ampersand,
and behave just like `fioctl device list 'a*'`, matching unexpected devices.

Using the `net/url` to URL escape the query makes filtering more predictable for such edge cases.

* Cleanup: only pass non-empty filters to device list API

This shortens the URL, making it easier to debug.

Signed-off-by: Volodymyr Khoroz <[email protected]>

client/foundries.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	netUrl "net/url"
 	"os"
 	"strconv"
 	"strings"
@@ -780,26 +781,22 @@ func (a *Api) DeviceGet(factory, device string) (*Device, error) {
 	return &d, nil
 }
 
-func (a *Api) DeviceList(
-	mine, prod, nonProd bool, matchTag, byFactory, byGroup, nameIlike, uuid, byTarget, sortBy string, page, limit uint64,
-) (*DeviceList, error) {
-	var (
-		mineInt int
-		prodStr string
-	)
-	if mine {
-		mineInt = 1
+func (a *Api) DeviceList(filterBy map[string]string, sortBy string, page, limit uint64) (*DeviceList, error) {
+	url := a.serverUrl + "/ota/devices/?"
+	query := netUrl.Values{}
+	for key, val := range filterBy {
+		if len(val) > 0 {
+			query.Set(key, val)
+		}
 	}
-	switch {
-	case prod:
-		prodStr = "1"
-	case nonProd:
-		prodStr = "0"
+	if len(sortBy) > 0 {
+		query.Set("sortby", sortBy)
 	}
-	url := a.serverUrl + "/ota/devices/?"
-	url += fmt.Sprintf(
-		"mine=%d&prod=%s&match_tag=%s&name_ilike=%s&factory=%s&uuid=%s&group=%s&target_name=%s&sortby=%s&page=%d&limit=%d",
-		mineInt, prodStr, matchTag, nameIlike, byFactory, uuid, byGroup, byTarget, sortBy, page, limit)
+	if page > 1 {
+		query.Set("page", strconv.FormatUint(page, 10))
+	}
+	query.Set("limit", strconv.FormatUint(limit, 10))
+	url += query.Encode()
 	logrus.Debugf("DeviceList with url: %s", url)
 	return a.DeviceListCont(url)
 }

subcommands/devices/list.go

@@ -159,22 +159,6 @@ func init() {
 	listCmd.MarkFlagsMutuallyExclusive("sort-by-name", "sort-by-last-seen")
 }
 
-// We allow pattern matching using filepath.Match type * and ?
-// ie * matches everything and ? matches a single character.
-// In sql we need * and ? to maps to % and _ respectively
-// Since _ is a special character we need to escape that. And
-//
-// Soo... a pattern like: H?st_b* would become: H_st\_b%
-// and would match stuff like host_b and hast_FOOO
-func sqlLikeIfy(filePathLike string) string {
-	// %25 = urlencode("%")
-	sql := strings.Replace(filePathLike, "*", "%25", -1)
-	sql = strings.Replace(sql, "_", "\\_", -1)
-	sql = strings.Replace(sql, "?", "_", -1)
-	logrus.Debugf("Converted query(%s) -> %s", filePathLike, sql)
-	return sql
-}
-
 func assertPagination() {
 	// hack until: https://github.com/spf13/pflag/issues/236
 	for _, x := range paginationLimits {
@@ -220,24 +204,26 @@ func doList(cmd *cobra.Command, args []string) {
 	sortBy = appendSortFlagValue(sortBy, cmd, "sort-by-last-seen", "last_seen")
 	sortBy = appendSortFlagValue(sortBy, cmd, "sort-by-name", "name")
 
-	name_ilike := ""
+	filterBy := map[string]string{
+		"factory":     factory,
+		"group":       deviceByGroup,
+		"match_tag":   deviceByTag,
+		"target_name": deviceByTarget,
+		"uuid":        deviceUuid,
+	}
 	if len(args) == 1 {
-		name_ilike = sqlLikeIfy(args[0])
-	}
-	dl, err := api.DeviceList(
-		deviceMine,
-		deviceOnlyProd,
-		deviceOnlyNonProd,
-		deviceByTag,
-		factory,
-		deviceByGroup,
-		name_ilike,
-		deviceUuid,
-		deviceByTarget,
-		strings.Join(sortBy, ","),
-		showPage,
-		paginationLimit,
-	)
+		filterBy["name"] = args[0]
+	}
+	if deviceMine {
+		filterBy["mine"] = "1"
+	}
+	if deviceOnlyProd {
+		filterBy["prod"] = "1"
+	} else if deviceOnlyNonProd {
+		filterBy["prod"] = "0"
+	}
+
+	dl, err := api.DeviceList(filterBy, strings.Join(sortBy, ","), showPage, paginationLimit)
 	subcommands.DieNotNil(err)
 	showDeviceList(dl, showColumns)
 }