From 2dd212e295dc9622fe6a4cbdf705215bd31de1de Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Mon, 19 May 2014 16:31:15 -0400 Subject: [PATCH] filter flag: split out for separate --filter flags adding tests and allowing for easy passing of filters.Args from client to server. Docker-DCO-1.1-Signed-off-by: Vincent Batts (github: vbatts) Upstream-commit: f1cc7ce5d728d6bdb1b3ab668e36fb1fc66515ff Component: engine --- components/engine/api/client/commands.go | 8 +-- components/engine/api/server/server.go | 2 +- components/engine/server/server.go | 13 +++- components/engine/utils/filters/parse.go | 47 ++++++++------- components/engine/utils/filters/parse_test.go | 60 +++++++++++++++++++ 5 files changed, 101 insertions(+), 29 deletions(-) create mode 100644 components/engine/utils/filters/parse_test.go diff --git a/components/engine/api/client/commands.go b/components/engine/api/client/commands.go index d0ac621483..1dd6194958 100644 --- a/components/engine/api/client/commands.go +++ b/components/engine/api/client/commands.go @@ -1160,10 +1160,10 @@ func (cli *DockerCli) CmdImages(args ...string) error { // Consolidate all filter flags, and sanity check them early. // They'll get process in the daemon/server. - imageFilters := map[string]string{} + imageFilterArgs := filters.Args{} for _, f := range flFilter.GetAll() { var err error - imageFilters, err = filters.ParseFlag(f, imageFilters) + imageFilterArgs, err = filters.ParseFlag(f, imageFilterArgs) if err != nil { return err } @@ -1174,7 +1174,7 @@ func (cli *DockerCli) CmdImages(args ...string) error { if *flViz || *flTree { v := url.Values{ "all": []string{"1"}, - "filters": []string{filters.ToParam(imageFilters)}, + "filters": []string{filters.ToParam(imageFilterArgs)}, } body, _, err := readBody(cli.call("GET", "/images/json?"+v.Encode(), nil, false)) if err != nil { @@ -1238,7 +1238,7 @@ func (cli *DockerCli) CmdImages(args ...string) error { } } else { v := url.Values{ - "filters": []string{filters.ToParam(imageFilters)}, + "filters": []string{filters.ToParam(imageFilterArgs)}, } if cmd.NArg() == 1 { // FIXME rename this parameter, to not be confused with the filters flag diff --git a/components/engine/api/server/server.go b/components/engine/api/server/server.go index 0bed582260..43b2c62323 100644 --- a/components/engine/api/server/server.go +++ b/components/engine/api/server/server.go @@ -189,7 +189,7 @@ func getImagesJSON(eng *engine.Engine, version version.Version, w http.ResponseW ) job.Setenv("filters", r.Form.Get("filters")) - // FIXME rename this parameter, to not be confused with the filters flag + // FIXME this parameter could just be a match filter job.Setenv("filter", r.Form.Get("filter")) job.Setenv("all", r.Form.Get("all")) diff --git a/components/engine/server/server.go b/components/engine/server/server.go index dd54ab67de..0495b0ad5d 100644 --- a/components/engine/server/server.go +++ b/components/engine/server/server.go @@ -700,12 +700,19 @@ func (srv *Server) Images(job *engine.Job) engine.Status { filt_tagged = true ) - imageFilters, err := filters.ParseFlag(job.Getenv("filters"), nil) + utils.Debugf("SUCH JOB: %#v", job) + utils.Debugf("SUCH ENV: %#v", *job.Env()) + imageFilters, err := filters.FromParam(job.Getenv("filters")) if err != nil { return job.Error(err) } - if i, ok := imageFilters["untagged"]; ok && strings.ToLower(i) == "true" { - filt_tagged = false + utils.Debugf("SUCH FILTERS: %#v", imageFilters) + if i, ok := imageFilters["untagged"]; ok { + for _, value := range i { + if strings.ToLower(value) == "true" { + filt_tagged = false + } + } } if job.GetenvBool("all") && !filt_tagged { diff --git a/components/engine/utils/filters/parse.go b/components/engine/utils/filters/parse.go index a813be9ff9..1eb46830ae 100644 --- a/components/engine/utils/filters/parse.go +++ b/components/engine/utils/filters/parse.go @@ -2,46 +2,51 @@ package filters import ( "errors" + "github.com/dotcloud/docker/pkg/beam/data" "strings" ) +type Args map[string][]string + /* Parse the argument to the filter flag. Like - `docker ps -f 'created=today;image.name=ubuntu*'` - -Filters delimited by ';', and expected to be 'name=value' + `docker ps -f 'created=today' -f 'image.name=ubuntu*'` If prev map is provided, then it is appended to, and returned. By default a new map is created. */ -func ParseFlag(arg string, prev map[string]string) (map[string]string, error) { - var filters map[string]string - if prev != nil { - filters = prev - } else { - filters = map[string]string{} +func ParseFlag(arg string, prev Args) (Args, error) { + var filters Args = prev + if prev == nil { + filters = Args{} } if len(arg) == 0 { return filters, nil } - for _, chunk := range strings.Split(arg, ";") { - if !strings.Contains(chunk, "=") { - return filters, ErrorBadFormat - } - f := strings.SplitN(chunk, "=", 2) - filters[f[0]] = f[1] + if !strings.Contains(arg, "=") { + return filters, ErrorBadFormat } + + f := strings.SplitN(arg, "=", 2) + filters[f[0]] = append(filters[f[0]], f[1]) + return filters, nil } var ErrorBadFormat = errors.New("bad format of filter (expected name=value)") -func ToParam(f map[string]string) string { - fs := []string{} - for k, v := range f { - fs = append(fs, k+"="+v) - } - return strings.Join(fs, ";") +/* +packs the Args into an string for easy transport from client to server +*/ +func ToParam(a Args) string { + return data.Encode(a) +} + +/* +unpacks the filter Args +*/ +func FromParam(p string) (Args, error) { + return data.Decode(p) } diff --git a/components/engine/utils/filters/parse_test.go b/components/engine/utils/filters/parse_test.go new file mode 100644 index 0000000000..c24fe3949a --- /dev/null +++ b/components/engine/utils/filters/parse_test.go @@ -0,0 +1,60 @@ +package filters + +import ( + "sort" + "testing" +) + +func TestParseArgs(t *testing.T) { + // equivalent of `docker ps -f 'created=today' -f 'image.name=ubuntu*' -f 'image.name=*untu'` + flagArgs := []string{ + "created=today", + "image.name=ubuntu*", + "image.name=*untu", + } + var ( + args = Args{} + err error + ) + for i := range flagArgs { + args, err = ParseFlag(flagArgs[i], args) + if err != nil { + t.Errorf("failed to parse %s: %s", flagArgs[i], err) + } + } + if len(args["created"]) != 1 { + t.Errorf("failed to set this arg") + } + if len(args["image.name"]) != 2 { + t.Errorf("the args should have collapsed") + } +} + +func TestParam(t *testing.T) { + a := Args{ + "created": []string{"today"}, + "image.name": []string{"ubuntu*", "*untu"}, + } + + v := ToParam(a) + v1, err := FromParam(v) + if err != nil { + t.Errorf("%s", err) + } + for key, vals := range v1 { + if _, ok := a[key]; !ok { + t.Errorf("could not find key %s in original set", key) + } + sort.Strings(vals) + sort.Strings(a[key]) + if len(vals) != len(a[key]) { + t.Errorf("value lengths ought to match") + continue + } + for i := range vals { + if vals[i] != a[key][i] { + t.Errorf("expected %s, but got %s", a[key][i], vals[i]) + } + } + } +}