filtermanager: skip method was broken when there are multi plugins (#308

) This bug is introduced during optimization. Signed-off-by: spacewander <[email protected]> --------- Signed-off-by: spacewander <[email protected]>
mosn · Feb 23, 2024 · bd0357f · bd0357f
1 parent 94046d4
commit bd0357f
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/pkg/filtermanager/filtermanager.go b/pkg/filtermanager/filtermanager.go
@@ -350,14 +350,18 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str
 	fm := conf.pool.Get().(*filterManager)
 	fm.callbacks.FilterCallbackHandler = cb
 
+	canSkipMethod := fm.canSkipMethod
+	if canSkipMethod == nil {
+		canSkipMethod = newSkipMethodsMap()
+	}
+
 	filters := make([]*filterWrapper, len(newConfig))
 	for i, fc := range newConfig {
 		factory := fc.Factory
 		config := fc.ParsedConfig
 		f := factory(config, fm.callbacks)
 		// Technically, the factory might create different f for different calls. We don't support this edge case for now.
 		if fm.canSkipMethod == nil {
-			canSkipMethod := newSkipMethodsMap()
 			for meth := range canSkipMethod {
 				ok, err := isMethodFromPassThroughFilter(f, meth)
 				if err != nil {
@@ -366,11 +370,13 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str
 				}
 				canSkipMethod[meth] = canSkipMethod[meth] && ok
 			}
-			fm.canSkipMethod = canSkipMethod
 		}
 		filters[i] = newFilterWrapper(fc.Name, f)
 	}
 
+	if fm.canSkipMethod == nil {
+		fm.canSkipMethod = canSkipMethod
+	}
 	fm.filters = filters
 
 	if conf.consumerFiltersEndAt != 0 {

diff --git a/pkg/filtermanager/filtermanager_test.go b/pkg/filtermanager/filtermanager_test.go
@@ -343,6 +343,31 @@ func (f *addReqFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream boo
 	return api.Continue
 }
 
+func TestSkipMethodWhenThereAreMultiFilters(t *testing.T) {
+	cb := envoy.NewCAPIFilterCallbackHandler()
+	config := initFilterManagerConfig("ns")
+	config.current = []*model.ParsedFilterConfig{
+		{
+			Name:    "add_req",
+			Factory: addReqFactory,
+			ParsedConfig: addReqConf{
+				hdrName: "x-htnn-route",
+			},
+		},
+		{
+			Name:    "on_log",
+			Factory: onLogFactory,
+		},
+	}
+
+	for i := 0; i < 2; i++ {
+		m := FilterManagerFactory(config, cb).(*filterManager)
+		assert.Equal(t, false, m.canSkipOnLog)
+		assert.Equal(t, false, m.canSkipDecodeHeaders)
+		assert.Equal(t, true, m.canSkipDecodeData)
+	}
+}
+
 func TestFiltersFromConsumer(t *testing.T) {
 	cb := envoy.NewCAPIFilterCallbackHandler()
 	config := initFilterManagerConfig("ns")