This is an automated email from the ASF dual-hosted git repository.

chenjunxu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git


The following commit(s) were added to refs/heads/master by this push:
     new c0241ee  fix: search labels with the same key  (#1151)
c0241ee is described below

commit c0241ee83450e2f26695aed12dbb17d6f9da59d3
Author: Peter Zhu <[email protected]>
AuthorDate: Thu Dec 31 10:08:53 2020 +0800

    fix: search labels with the same key  (#1151)
    
    * fix: search labels with the same key (#1130)
---
 api/internal/handler/label/label.go               |  9 ++-
 api/internal/handler/label/label_test.go          | 69 +++++++++++++++--------
 api/internal/utils/utils.go                       | 21 ++++---
 api/internal/utils/utils_test.go                  | 37 ++++++++----
 api/test/e2e/label_test.go                        | 40 +++++++++++++
 api/test/e2e/route_with_management_fileds_test.go | 11 ++++
 6 files changed, 145 insertions(+), 42 deletions(-)

diff --git a/api/internal/handler/label/label.go 
b/api/internal/handler/label/label.go
index 93ff0de..9a29f8c 100644
--- a/api/internal/handler/label/label.go
+++ b/api/internal/handler/label/label.go
@@ -78,15 +78,18 @@ type ListInput struct {
        Label string `auto_read:"label,query"`
 }
 
-func subsetOf(reqLabels, labels map[string]string) map[string]string {
+func subsetOf(reqLabels map[string]struct{}, labels map[string]string) 
map[string]string {
        if len(reqLabels) == 0 {
                return labels
        }
 
        var res = make(map[string]string)
        for k, v := range labels {
-               l, exist := reqLabels[k]
-               if exist && ((l == "") || v == l) {
+               if _, exist := reqLabels[k]; exist {
+                       res[k] = v
+               }
+
+               if _, exist := reqLabels[k+":"+v]; exist {
                        res[k] = v
                }
        }
diff --git a/api/internal/handler/label/label_test.go 
b/api/internal/handler/label/label_test.go
index 1d0205b..25f9fda 100644
--- a/api/internal/handler/label/label_test.go
+++ b/api/internal/handler/label/label_test.go
@@ -221,30 +221,43 @@ func TestLabel(t *testing.T) {
                        }
                }
 
+               var testCases []*testCase
+
                expect := []interface{}{
                        Pair{"label1", "value1"},
                        Pair{"label1", "value2"},
                        Pair{"label2", "value2"},
                }
-               case1 := newCase(giveData, expect)
-               case1.giveInput.Type = typ
+               tc := newCase(giveData, expect)
+               tc.giveInput.Type = typ
+               testCases = append(testCases, tc)
 
                expect = []interface{}{
                        Pair{"label1", "value1"},
                        Pair{"label1", "value2"},
                }
-               case2 := newCase(giveData, expect)
-               case2.giveInput.Type = typ
-               case2.giveInput.Label = "label1"
+               tc = newCase(giveData, expect)
+               tc.giveInput.Type = typ
+               tc.giveInput.Label = "label1"
+               testCases = append(testCases, tc)
+
+               expect = []interface{}{
+                       Pair{"label1", "value2"},
+               }
+               tc = newCase(giveData, expect)
+               tc.giveInput.Type = typ
+               tc.giveInput.Label = "label1:value2"
+               testCases = append(testCases, tc)
 
                expect = []interface{}{
+                       Pair{"label1", "value1"},
                        Pair{"label1", "value2"},
                }
-               case3 := newCase(giveData, expect)
-               case3.giveInput.Type = typ
-               case3.giveInput.Label = "label1:value2"
+               tc = newCase(giveData, expect)
+               tc.giveInput.Type = typ
+               tc.giveInput.Label = "label1:value1,label1:value2"
+               testCases = append(testCases, tc)
 
-               testCases := []*testCase{case1, case2, case3}
                handler := Handler{}
                for _, tc := range testCases {
                        switch typ {
@@ -290,6 +303,8 @@ func TestLabel(t *testing.T) {
                serviceStore:  genMockStore(t, []interface{}{genService(m5)}),
        }
 
+       var testCases []*testCase
+
        expect := []interface{}{
                Pair{"label1", "value1"},
                Pair{"label1", "value2"},
@@ -298,35 +313,45 @@ func TestLabel(t *testing.T) {
                Pair{"label4", "value4"},
                Pair{"label5", "value5"},
        }
-       case1 := newCase(nil, expect)
-       case1.giveInput.Type = "all"
+       tc := newCase(nil, expect)
+       tc.giveInput.Type = "all"
+       testCases = append(testCases, tc)
 
        expect = []interface{}{
                Pair{"label1", "value1"},
                Pair{"label1", "value2"},
        }
-       case2 := newCase(nil, expect)
-       case2.giveInput.Type = "all"
-       case2.giveInput.Label = "label1"
+       tc = newCase(nil, expect)
+       tc.giveInput.Type = "all"
+       tc.giveInput.Label = "label1"
+       testCases = append(testCases, tc)
 
        expect = []interface{}{
                Pair{"label1", "value2"},
        }
-       case3 := newCase(nil, expect)
-       case3.giveInput.Type = "all"
-       case3.giveInput.Label = "label1:value2"
+       tc = newCase(nil, expect)
+       tc.giveInput.Type = "all"
+       tc.giveInput.Label = "label1:value2"
+       testCases = append(testCases, tc)
 
        expect = []interface{}{
                Pair{"label1", "value1"},
                Pair{"label1", "value2"},
                Pair{"label5", "value5"},
        }
-       case4 := newCase(nil, expect)
-       case4.giveInput.Type = "all"
-       case4.giveInput.Label = "label1,label5:value5"
+       tc = newCase(nil, expect)
+       tc.giveInput.Type = "all"
+       tc.giveInput.Label = "label1,label5:value5"
+
+       expect = []interface{}{
+               Pair{"label1", "value1"},
+               Pair{"label1", "value2"},
+       }
+       tc = newCase(nil, expect)
+       tc.giveInput.Type = "all"
+       tc.giveInput.Label = "label1=value1,label1=value2"
 
-       testcase := []*testCase{case1, case2, case3, case4}
-       for _, tc := range testcase {
+       for _, tc := range testCases {
                ctx := droplet.NewContext()
                ctx.SetInput(tc.giveInput)
                ret, err := handler.List(ctx)
diff --git a/api/internal/utils/utils.go b/api/internal/utils/utils.go
index b8d192c..6b7ea10 100644
--- a/api/internal/utils/utils.go
+++ b/api/internal/utils/utils.go
@@ -109,9 +109,9 @@ func ObjectClone(origin, copy interface{}) error {
        return err
 }
 
-func GenLabelMap(label string) (map[string]string, error) {
+func GenLabelMap(label string) (map[string]struct{}, error) {
        var err = errors.New("malformed label")
-       mp := make(map[string]string)
+       mp := make(map[string]struct{})
 
        if label == "" {
                return mp, nil
@@ -125,13 +125,15 @@ func GenLabelMap(label string) (map[string]string, error) 
{
                                return nil, err
                        }
 
-                       mp[kv[0]] = kv[1]
+                       // Because the labels may contain the same key, like 
this: label=version:v1,version:v2
+                       // we need to combine them as a map's key
+                       mp[l] = struct{}{}
                } else if len(kv) == 1 {
                        if kv[0] == "" {
                                return nil, err
                        }
 
-                       mp[kv[0]] = ""
+                       mp[kv[0]] = struct{}{}
                } else {
                        return nil, err
                }
@@ -140,14 +142,19 @@ func GenLabelMap(label string) (map[string]string, error) 
{
        return mp, nil
 }
 
-func LabelContains(labels, reqLabels map[string]string) bool {
+func LabelContains(labels map[string]string, reqLabels map[string]struct{}) 
bool {
        if len(reqLabels) == 0 {
                return true
        }
 
        for k, v := range labels {
-               l, exist := reqLabels[k]
-               if exist && ((l == "") || v == l) {
+               // first check the key
+               if _, exist := reqLabels[k]; exist {
+                       return true
+               }
+
+               // second check the key:value
+               if _, exist := reqLabels[k+":"+v]; exist {
                        return true
                }
        }
diff --git a/api/internal/utils/utils_test.go b/api/internal/utils/utils_test.go
index 8da295f..1d7c154 100644
--- a/api/internal/utils/utils_test.go
+++ b/api/internal/utils/utils_test.go
@@ -66,12 +66,17 @@ func TestGenLabelMap(t *testing.T) {
        expectedErr := errors.New("malformed label")
        mp, err := GenLabelMap("l1")
        assert.Nil(t, err)
-       assert.Equal(t, mp["l1"], "")
+       assert.Equal(t, mp["l1"], struct{}{})
 
        mp, err = GenLabelMap("l1,l2:v2")
        assert.Nil(t, err)
-       assert.Equal(t, mp["l1"], "")
-       assert.Equal(t, mp["l2"], "v2")
+       assert.Equal(t, mp["l1"], struct{}{})
+       assert.Equal(t, mp["l2:v2"], struct{}{})
+
+       mp, err = GenLabelMap("l1:v1,l1:v2")
+       assert.Nil(t, err)
+       assert.Equal(t, mp["l1:v1"], struct{}{})
+       assert.Equal(t, mp["l1:v2"], struct{}{})
 
        mp, err = GenLabelMap(",")
        assert.Equal(t, expectedErr, err)
@@ -83,20 +88,32 @@ func TestGenLabelMap(t *testing.T) {
 }
 
 func TestLabelContains(t *testing.T) {
-       mp1, _ := GenLabelMap("l1,l2:v2")
-       mp2 := map[string]string{
+       reqMap, _ := GenLabelMap("l1,l2:v2")
+       mp := map[string]string{
                "l1": "v1",
        }
-       assert.True(t, LabelContains(mp2, mp1))
+       assert.True(t, LabelContains(mp, reqMap))
 
-       mp3 := map[string]string{
+       mp = map[string]string{
                "l1": "v1",
                "l2": "v3",
        }
-       assert.True(t, LabelContains(mp3, mp1))
+       assert.True(t, LabelContains(mp, reqMap))
 
-       mp4 := map[string]string{
+       mp = map[string]string{
                "l2": "v3",
        }
-       assert.False(t, LabelContains(mp4, mp1))
+       assert.False(t, LabelContains(mp, reqMap))
+
+       reqMap, _ = GenLabelMap("l1:v1,l1:v2")
+       mp = map[string]string{
+               "l1": "v1",
+       }
+       assert.True(t, LabelContains(mp, reqMap))
+
+       reqMap, _ = GenLabelMap("l1:v1,l1:v2")
+       mp = map[string]string{
+               "l1": "v2",
+       }
+       assert.True(t, LabelContains(mp, reqMap))
 }
diff --git a/api/test/e2e/label_test.go b/api/test/e2e/label_test.go
index b984c08..690d913 100644
--- a/api/test/e2e/label_test.go
+++ b/api/test/e2e/label_test.go
@@ -201,6 +201,16 @@ func TestLabel(t *testing.T) {
                        ExpectBody:   "{\"build\":\"16\"},{\"build\":\"17\"}",
                },
                {
+                       Desc:         "get labels with the same key (key = 
build)",
+                       Object:       ManagerApiExpect(t),
+                       Method:       http.MethodGet,
+                       Headers:      map[string]string{"Authorization": token},
+                       Query:        "label=build:16,build:17",
+                       Path:         "/apisix/admin/labels/all",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "{\"build\":\"16\"},{\"build\":\"17\"}",
+               },
+               {
                        Desc:         "get labels (key = build) with page",
                        Object:       ManagerApiExpect(t),
                        Method:       http.MethodGet,
@@ -211,6 +221,26 @@ func TestLabel(t *testing.T) {
                        ExpectBody:   "{\"build\":\"17\"}",
                },
                {
+                       Desc:         "get labels with same key (key = build) 
and page",
+                       Object:       ManagerApiExpect(t),
+                       Method:       http.MethodGet,
+                       Headers:      map[string]string{"Authorization": token},
+                       Query:        
"label=build:16,build:17&page=1&page_size=2",
+                       Path:         "/apisix/admin/labels/all",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "{\"build\":\"16\"},{\"build\":\"17\"}",
+               },
+               {
+                       Desc:         "get labels with same key (key = build) 
and page",
+                       Object:       ManagerApiExpect(t),
+                       Method:       http.MethodGet,
+                       Headers:      map[string]string{"Authorization": token},
+                       Query:        
"label=build:16,build:17&page=2&page_size=1",
+                       Path:         "/apisix/admin/labels/all",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "{\"build\":\"17\"}",
+               },
+               {
                        Desc:         "get labels (key = build && env = 
production)",
                        Object:       ManagerApiExpect(t),
                        Method:       http.MethodGet,
@@ -221,6 +251,16 @@ func TestLabel(t *testing.T) {
                        ExpectBody:   
"{\"build\":\"16\"},{\"build\":\"17\"},{\"env\":\"production\"}",
                },
                {
+                       Desc:         "get labels (build=16 | 17 and env = 
production)",
+                       Object:       ManagerApiExpect(t),
+                       Method:       http.MethodGet,
+                       Headers:      map[string]string{"Authorization": token},
+                       Query:        "label=build:16,build:17,env:production",
+                       Path:         "/apisix/admin/labels/all",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   
"{\"build\":\"16\"},{\"build\":\"17\"},{\"env\":\"production\"}",
+               },
+               {
                        Desc:         "get labels (key = build && env = 
production) with page",
                        Object:       ManagerApiExpect(t),
                        Method:       http.MethodGet,
diff --git a/api/test/e2e/route_with_management_fileds_test.go 
b/api/test/e2e/route_with_management_fileds_test.go
index 207cba9..3c4bb3f 100644
--- a/api/test/e2e/route_with_management_fileds_test.go
+++ b/api/test/e2e/route_with_management_fileds_test.go
@@ -334,6 +334,17 @@ func TestRoute_search_by_label(t *testing.T) {
                        Sleep:        sleepTime,
                },
                {
+                       Desc:         "search the route by label (combination)",
+                       Object:       ManagerApiExpect(t),
+                       Path:         "/apisix/admin/routes",
+                       Query:        "label=build:16,build:17",
+                       Method:       http.MethodGet,
+                       Headers:      map[string]string{"Authorization": token},
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "\"total_size\":2",
+                       Sleep:        sleepTime,
+               },
+               {
                        Desc:         "delete the route (r1)",
                        Object:       ManagerApiExpect(t),
                        Method:       http.MethodDelete,

Reply via email to