From ff1e1ff161d2c2c5f86f5db2c41df41bdfaf2b14 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Mon, 27 Jul 2026 23:21:01 +0100 Subject: [PATCH] cmd/snapd/cli: search all given sections instead of only the last one snap find --section=games --section=social silently searched only "social", discarding "games" with no error - Section is a scalar field, so repeated flags just overwrote each other. The store's /v2/find "category" parameter (section is an alias for it) already accepts a comma-separated list and matches any of them - confirmed directly against the real store. Make SectionName implement flags.Unmarshaler so repeated --section flags, and comma-separated values within a single occurrence, all flatten into one combined comma-joined value instead of overwriting; validate each requested section individually against the known list and list all unrecognized ones, not just the first. No daemon or client changes needed, since the comma-joined string flows through the existing Section field unchanged. Fixes: LP#1986425 --- cmd/snapd/cli/cmd_find.go | 64 ++++++++++++++++++++----- cmd/snapd/cli/cmd_find_test.go | 86 +++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 14 deletions(-) diff --git a/cmd/snapd/cli/cmd_find.go b/cmd/snapd/cli/cmd_find.go index 035bab124a5..b77352b0dd3 100644 --- a/cmd/snapd/cli/cmd_find.go +++ b/cmd/snapd/cli/cmd_find.go @@ -79,8 +79,40 @@ func getPrice(prices map[string]float64, currency string) (float64, string, erro return val, currency, nil } +const ( + // sectionShowAllValue is used when the user specified --section + // without any argument. + sectionShowAllValue = "show-all-sections-please" + // sectionDefaultValue is used when "--section" was not specified on + // the commandline at all. + sectionDefaultValue = "no-section-specified" +) + type SectionName string +// UnmarshalFlag accumulates repeated --section flags into a comma-separated +// list instead of overwriting, since the store already matches against any +// of them. +func (s *SectionName) UnmarshalFlag(value string) error { + switch string(*s) { + case "", sectionDefaultValue: + *s = SectionName(value) + case sectionShowAllValue: + // a bare --section (show all sections) was already given; an + // explicit value after that doesn't combine sensibly with "show + // all", so let the explicit value take over instead of joining + *s = SectionName(value) + default: + if value == sectionShowAllValue { + // bare --section after one or more explicit values: keep + // filtering rather than switching to "show all sections" + return nil + } + *s += "," + SectionName(value) + } + return nil +} + func (s SectionName) Complete(match string) []flags.Completion { if ret, err := completeFromSortedFile(dirs.SnapSectionsFile, match); err == nil { return ret @@ -100,6 +132,16 @@ func (s SectionName) Complete(match string) []flags.Completion { return ret } +func unknownSections(requested, known []string) []string { + var unknown []string + for _, r := range requested { + if !strutil.ListContains(known, r) { + unknown = append(unknown, r) + } + } + return unknown +} + func cachedSections() (sections []string, err error) { cachedSections, err := os.Open(dirs.SnapSectionsFile) if err != nil { @@ -188,15 +230,10 @@ func (x *cmdFind) Execute(args []string) error { query = "" } - // section will be: - // - "show-all-sections-please" if the user specified --section - // without any argument - // - "no-section-specified" if "--section" was not specified on - // the commandline at all switch x.Section { - case "show-all-sections-please": + case sectionShowAllValue: return showSections(x.client) - case "no-section-specified": + case sectionDefaultValue: x.Section = "" } @@ -207,19 +244,22 @@ func (x *cmdFind) Execute(args []string) error { } if x.Section != "" && x.Section != "featured" { + requested := strutil.CommaSeparatedList(string(x.Section)) sections, err := cachedSections() if err != nil { return err } - if !strutil.ListContains(sections, string(x.Section)) { - // try the store just in case it was added in the last 24 hours + unknown := unknownSections(requested, sections) + if len(unknown) > 0 { + // try the store just in case they were added in the last 24 hours sections, err = x.client.Sections() if err != nil { return err } - if !strutil.ListContains(sections, string(x.Section)) { - // TRANSLATORS: the %q is the (quoted) name of the section the user entered - return fmt.Errorf(i18n.G("No matching section %q, use --section to list existing sections"), x.Section) + unknown = unknownSections(requested, sections) + if len(unknown) > 0 { + // TRANSLATORS: %s is a comma-separated list of one or more (quoted) section names the user entered + return fmt.Errorf(i18n.G("No matching section(s) %s, use --section to list existing sections"), strutil.Quoted(unknown)) } } } diff --git a/cmd/snapd/cli/cmd_find_test.go b/cmd/snapd/cli/cmd_find_test.go index 7d126b27a0b..86ea8611280 100644 --- a/cmd/snapd/cli/cmd_find_test.go +++ b/cmd/snapd/cli/cmd_find_test.go @@ -624,7 +624,7 @@ func (s *SnapSuite) TestFindSnapInvalidSection(c *check.C) { n++ }) _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=foobar", "hello"}) - c.Assert(err, check.ErrorMatches, `No matching section "foobar", use --section to list existing sections`) + c.Assert(err, check.ErrorMatches, `No matching section\(s\) "foobar", use --section to list existing sections`) } func (s *SnapSuite) TestFindSnapNotFoundInSection(c *check.C) { @@ -679,7 +679,7 @@ func (s *SnapSuite) TestFindSnapCachedSection(c *check.C) { _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=foobar", "hello"}) c.Logf("stdout: %s", s.Stdout()) - c.Assert(err, check.ErrorMatches, `No matching section "foobar", use --section to list existing sections`) + c.Assert(err, check.ErrorMatches, `No matching section\(s\) "foobar", use --section to list existing sections`) s.ResetStdStreams() @@ -698,3 +698,85 @@ Please try 'snap find --section=' s.ResetStdStreams() c.Check(numHits, check.Equals, 1) } + +func (s *SnapSuite) TestFindRepeatedSectionFlagSearchesBoth(c *check.C) { + n := 0 + s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { + switch n { + case 0: + c.Check(r.Method, check.Equals, "GET") + c.Check(r.URL.Path, check.Equals, "/v2/sections") + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{"games", "social"}, + }) + case 1: + c.Check(r.Method, check.Equals, "GET") + c.Check(r.URL.Path, check.Equals, "/v2/find") + v, ok := r.URL.Query()["section"] + c.Check(ok, check.Equals, true) + c.Check(v, check.DeepEquals, []string{"games,social"}) + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{}, + }) + default: + c.Fatalf("expected to get 2 requests, now on #%d", n+1) + } + n++ + }) + + _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=games", "--section=social", "steam"}) + c.Assert(err, check.IsNil) +} + +func (s *SnapSuite) TestFindSectionFlagCommaAndRepeatCombo(c *check.C) { + s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2/sections": + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{"x", "y", "z", "a", "b", "c"}, + }) + case "/v2/find": + v, ok := r.URL.Query()["section"] + c.Check(ok, check.Equals, true) + c.Check(v, check.DeepEquals, []string{"x,y,z,a,b,c"}) + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{}, + }) + default: + c.Fatalf("unexpected request to %s", r.URL.Path) + } + }) + + _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=x,y", "--section=z", "--section=a,b,c", "steam"}) + c.Assert(err, check.IsNil) +} + +func (s *SnapSuite) TestFindRepeatedSectionFlagUnknownSection(c *check.C) { + s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { + c.Check(r.URL.Path, check.Equals, "/v2/sections") + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{"games"}, + }) + }) + + _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=games", "--section=bogus", "steam"}) + c.Assert(err, check.ErrorMatches, `No matching section\(s\) "bogus", use --section to list existing sections`) +} + +func (s *SnapSuite) TestFindRepeatedSectionFlagAllUnknownSections(c *check.C) { + s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { + c.Check(r.URL.Path, check.Equals, "/v2/sections") + EncodeResponseBody(c, w, map[string]any{ + "type": "sync", + "result": []string{"games", "social"}, + }) + }) + + _, err := snap.Parser(snap.Client()).ParseArgs([]string{"find", "--section=bogus1", "--section=bogus2", "steam"}) + c.Assert(err, check.ErrorMatches, `No matching section\(s\) "bogus1", "bogus2", use --section to list existing sections`) +}