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

diff --git a/hugolib/menu_test.go b/hugolib/menu_test.go
index f1db3cb3ab3f140932e7c0ffae1ef8f93b306164..4a2b176039d235b48ed1ac5228c0751e6d2e4dfd 100644 (file)
--- 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 137c6fa3a21627ccd965e2e425d77e5920415153..662536a2466306da5174b5fba6e74ad064e1a27b 100644 (file)
--- 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 17d6552e62914f1cad7267e499fcb9cf55b0b9d6..42f0d370f8df5f067336d191b0dd9a6e2841a353 100644 (file)
--- 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 295b89051b7069fa5a5250440c84ca8c753b3b92..cdef7aefb5b25075d78275b7a7362c7310503082 100644 (file)
--- 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)
-                               }
-                       } else {
-                               if err != nil {
-                                       t.Errorf("[%d] failed: %s", i, err)
+               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)
                                }
-                               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)
+                               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)
+                                       }
                                }
-                       }
-               })
+                       })
+               }
        }
 
        var err error