From 8d898ad6672e0ccb62c5a29b6fccab24d980f104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Mon, 27 May 2019 22:57:57 +0200 Subject: [PATCH] tpl/collections: Unwrap any interface value in sort and where Hugo `0.55.0` introduced some new interface types for `Page` etc. This worked great in general, but there were cases where this would fail in `where` and `sort`. One such example would be sorting by `MenuItem.Page.Date` where `Page` on `MenuItem` was a small subset of the bigger `page.Page` interface. This commit fixes that by unwrapping such interface values. Fixes #5989 --- hugolib/menu_test.go | 51 ++++++++++++++++++ tpl/collections/collections_test.go | 31 +++++++++++ tpl/collections/where.go | 10 +++- tpl/collections/where_test.go | 83 +++++++++++++++++++++-------- 4 files changed, 153 insertions(+), 22 deletions(-) diff --git a/hugolib/menu_test.go b/hugolib/menu_test.go index f1db3cb3a..4a2b17603 100644 --- a/hugolib/menu_test.go +++ b/hugolib/menu_test.go @@ -222,3 +222,54 @@ menu: "main" b.AssertFileContent("public/index.html", "AMP and HTML|/blog/html-amp/|AMP only|/amp/blog/amp/|HTML only|/blog/html/|Home Sweet Home|/|") b.AssertFileContent("public/amp/index.html", "AMP and HTML|/amp/blog/html-amp/|AMP only|/amp/blog/amp/|HTML only|/blog/html/|Home Sweet Home|/amp/|") } + +// https://github.com/gohugoio/hugo/issues/5989 +func TestMenuPageSortByDate(t *testing.T) { + + b := newTestSitesBuilder(t).WithSimpleConfigFile() + + b.WithContent("blog/a.md", ` +--- +Title: A +date: 2019-01-01 +menu: + main: + identifier: "a" + weight: 1 +--- + +`) + + b.WithContent("blog/b.md", ` +--- +Title: B +date: 2018-01-02 +menu: + main: + parent: "a" + weight: 100 +--- + +`) + + b.WithContent("blog/c.md", ` +--- +Title: C +date: 2019-01-03 +menu: + main: + parent: "a" + weight: 10 +--- + +`) + + b.WithTemplatesAdded("index.html", `{{ range .Site.Menus.main }}{{ .Title }}|Children: +{{- $children := sort .Children ".Page.Date" "desc" }}{{ range $children }}{{ .Title }}|{{ end }}{{ end }} + +`) + + b.Build(BuildCfg{}) + + b.AssertFileContent("public/index.html", "A|Children:C|B|") +} diff --git a/tpl/collections/collections_test.go b/tpl/collections/collections_test.go index 137c6fa3a..662536a24 100644 --- a/tpl/collections/collections_test.go +++ b/tpl/collections/collections_test.go @@ -819,6 +819,10 @@ func (x TstX) TstRv() string { return "r" + x.B } +func (x TstX) TstRv2() string { + return "r" + x.B +} + func (x TstX) unexportedMethod() string { return x.unexported } @@ -850,6 +854,33 @@ type TstX struct { unexported string } +type TstXIHolder struct { + XI TstXI +} + +// Partially implemented by the TstX struct. +type TstXI interface { + TstRv2() string +} + +func ToTstXIs(slice interface{}) []TstXI { + s := reflect.ValueOf(slice) + if s.Kind() != reflect.Slice { + return nil + } + tis := make([]TstXI, s.Len()) + + for i := 0; i < s.Len(); i++ { + tsti, ok := s.Index(i).Interface().(TstXI) + if !ok { + return nil + } + tis[i] = tsti + } + + return tis +} + func newDeps(cfg config.Provider) *deps.Deps { l := langs.NewLanguage("en", cfg) l.Set("i18nDir", "i18n") diff --git a/tpl/collections/where.go b/tpl/collections/where.go index 17d6552e6..42f0d370f 100644 --- a/tpl/collections/where.go +++ b/tpl/collections/where.go @@ -280,8 +280,16 @@ func evaluateSubElem(obj reflect.Value, elemName string) (reflect.Value, error) typ := obj.Type() obj, isNil := indirect(obj) + if obj.Kind() == reflect.Interface { + // If obj is an interface, we need to inspect the value it contains + // to see the full set of methods and fields. + // Indirect returns the value that it points to, which is what's needed + // below to be able to reflect on its fields. + obj = reflect.Indirect(obj.Elem()) + } + // first, check whether obj has a method. In this case, obj is - // an interface, a struct or its pointer. If obj is a struct, + // a struct or its pointer. If obj is a struct, // to check all T and *T method, use obj pointer type Value objPtr := obj if objPtr.Kind() != reflect.Interface && objPtr.CanAddr() { diff --git a/tpl/collections/where_test.go b/tpl/collections/where_test.go index 295b89051..cdef7aefb 100644 --- a/tpl/collections/where_test.go +++ b/tpl/collections/where_test.go @@ -38,13 +38,31 @@ func TestWhere(t *testing.T) { d5 := d4.Add(1 * time.Hour) d6 := d5.Add(1 * time.Hour) - for i, test := range []struct { + type testt struct { seq interface{} key interface{} op string match interface{} expect interface{} - }{ + } + + createTestVariants := func(test testt) []testt { + testVariants := []testt{test} + if islice := ToTstXIs(test.seq); islice != nil { + variant := test + variant.seq = islice + expect := ToTstXIs(test.expect) + if expect != nil { + variant.expect = expect + } + testVariants = append(testVariants, variant) + } + + return testVariants + + } + + for i, test := range []testt{ { seq: []map[int]string{ {1: "a", 2: "m"}, {1: "c", 2: "d"}, {1: "e", 3: "m"}, @@ -216,6 +234,24 @@ func TestWhere(t *testing.T) { {"foo": &TstX{A: "c", B: "d"}}, }, }, + { + seq: []TstXIHolder{ + {&TstX{A: "a", B: "b"}}, {&TstX{A: "c", B: "d"}}, {&TstX{A: "e", B: "f"}}, + }, + key: "XI.TstRp", match: "rc", + expect: []TstXIHolder{ + {&TstX{A: "c", B: "d"}}, + }, + }, + { + seq: []TstXIHolder{ + {&TstX{A: "a", B: "b"}}, {&TstX{A: "c", B: "d"}}, {&TstX{A: "e", B: "f"}}, + }, + key: "XI.A", match: "e", + expect: []TstXIHolder{ + {&TstX{A: "e", B: "f"}}, + }, + }, { seq: []map[string]Mid{ {"foo": Mid{Tst: TstX{A: "a", B: "b"}}}, {"foo": Mid{Tst: TstX{A: "c", B: "d"}}}, {"foo": Mid{Tst: TstX{A: "e", B: "f"}}}, @@ -522,28 +558,33 @@ func TestWhere(t *testing.T) { }, }, } { - t.Run(fmt.Sprintf("test case %d for key %s", i, test.key), func(t *testing.T) { - var results interface{} - var err error - if len(test.op) > 0 { - results, err = ns.Where(test.seq, test.key, test.op, test.match) - } else { - results, err = ns.Where(test.seq, test.key, test.match) - } - if b, ok := test.expect.(bool); ok && !b { - if err == nil { - t.Errorf("[%d] Where didn't return an expected error", i) + testVariants := createTestVariants(test) + for j, test := range testVariants { + name := fmt.Sprintf("[%d/%d] %T %s %s", i, j, test.seq, test.op, test.key) + t.Run(name, func(t *testing.T) { + var results interface{} + var err error + + if len(test.op) > 0 { + results, err = ns.Where(test.seq, test.key, test.op, test.match) + } else { + results, err = ns.Where(test.seq, test.key, test.match) } - } else { - if err != nil { - t.Errorf("[%d] failed: %s", i, err) + if b, ok := test.expect.(bool); ok && !b { + if err == nil { + t.Fatalf("[%d] Where didn't return an expected error", i) + } + } else { + if err != nil { + t.Fatalf("[%d] failed: %s", i, err) + } + if !reflect.DeepEqual(results, test.expect) { + t.Fatalf("Where clause matching %v with %v in seq %v (%T),\ngot\n%v (%T) but expected\n%v (%T)", test.key, test.match, test.seq, test.seq, results, results, test.expect, test.expect) + } } - if !reflect.DeepEqual(results, test.expect) { - t.Errorf("[%d] Where clause matching %v with %v, got %v but expected %v", i, test.key, test.match, results, test.expect) - } - } - }) + }) + } } var err error