From 35f136d2e91b50678a686c527d8cbbd83ce0309f Mon Sep 17 00:00:00 2001 From: Derek Wang Date: Tue, 23 Mar 2021 09:56:13 -0700 Subject: [PATCH] fix(GithubEventSource): Compare events ignoring order and duplicate (#1124) * fix(GithubEventSource): Sort events before compare Signed-off-by: Derek Wang --- eventsources/sources/github/hook_util.go | 32 ++++++++++++++----- eventsources/sources/github/hook_util_test.go | 12 +++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/eventsources/sources/github/hook_util.go b/eventsources/sources/github/hook_util.go index 078a73e0f44e..4191604e72d6 100644 --- a/eventsources/sources/github/hook_util.go +++ b/eventsources/sources/github/hook_util.go @@ -6,24 +6,40 @@ import ( // sliceEqual returns true if the two provided string slices are equal. func sliceEqual(first []string, second []string) bool { - if len(first) != len(second) { + if len(first) == 0 && len(second) == 0 { + return true + } + if len(first) == 0 || len(second) == 0 { return false } - if first == nil && second == nil { - return true + tmp := make(map[string]int) + for _, i := range first { + tmp[i] = 1 } - if first == nil || second == nil { - return false + for _, i := range second { + v, ok := tmp[i] + if !ok || v == -1 { + tmp[i] = -1 + } else { + tmp[i] = 2 + } + } + + if v, ok := tmp["*"]; ok { + // If both slices contain "*", return true directly + return v == 2 } - for index := range first { - if first[index] != second[index] { + for _, v := range tmp { + // -1: only exists in second + // 1: only exists in first + // 2: exists in both + if v < 2 { return false } } - return true } diff --git a/eventsources/sources/github/hook_util_test.go b/eventsources/sources/github/hook_util_test.go index fbcbac2ee102..70ab05cc2c87 100644 --- a/eventsources/sources/github/hook_util_test.go +++ b/eventsources/sources/github/hook_util_test.go @@ -19,6 +19,18 @@ func TestSliceEqual(t *testing.T) { assert.False(t, sliceEqual([]string{"hello"}, []string{"hello", "world"})) assert.False(t, sliceEqual([]string{"hello", "world"}, []string{"hello"})) assert.False(t, sliceEqual([]string{"hello", "world"}, []string{"hello", "moon"})) + assert.True(t, sliceEqual([]string{"hello", "world"}, []string{"world", "hello"})) + assert.True(t, sliceEqual([]string{"hello", "*"}, []string{"*"})) + assert.True(t, sliceEqual([]string{"hello", "*"}, []string{"*", "world"})) + assert.True(t, sliceEqual([]string{"hello", "world", "hello"}, []string{"hello", "hello", "world", "world"})) + assert.True(t, sliceEqual([]string{"world", "hello"}, []string{"hello", "hello", "world", "world"})) + assert.True(t, sliceEqual([]string{"hello", "hello", "world", "world"}, []string{"world", "hello"})) + assert.False(t, sliceEqual([]string{"hello"}, []string{"*", "hello"})) + assert.False(t, sliceEqual([]string{"hello", "*"}, []string{"hello"})) + assert.False(t, sliceEqual([]string{"*", "hello", "*"}, []string{"hello"})) + assert.False(t, sliceEqual([]string{"hello"}, []string{"world", "world"})) + assert.False(t, sliceEqual([]string{"hello", "hello"}, []string{"world", "world"})) + assert.True(t, sliceEqual([]string{"*", "hello", "*"}, []string{"*", "world", "hello", "world"})) } func TestCompareHook(t *testing.T) {