From d4ac595596038eba65a939434c2bc56c5a7e092c Mon Sep 17 00:00:00 2001 From: Walter Schulze Date: Thu, 10 Oct 2024 08:25:09 -0700 Subject: [PATCH] use ProcessorFunctionMap instead of GetProcessorFunction in Processor interface Summary: use ProcessorFunctionMap instead of GetProcessorFunction in Processor interface This aligns better with thwork, which we are refactoring to bring into the open source Reviewed By: podtserkovskiy Differential Revision: D64172011 fbshipit-source-id: 6c799d1d5ad1f57fdf0a021b404b269babc7bcfc --- thrift/lib/go/thrift/header_server_test.go | 7 ++----- thrift/lib/go/thrift/interceptor.go | 19 ++++++++++--------- thrift/lib/go/thrift/interceptor_test.go | 4 ++-- thrift/lib/go/thrift/processor.go | 7 +++++-- thrift/lib/go/thrift/rocket_server_test.go | 7 ++----- .../lib/go/thrift/rocket_upgrade_processor.go | 17 ++++++++++------- thrift/lib/go/thrift/server_test.go | 7 ++----- thrift/lib/go/thrift/types/processor.go | 8 ++++---- 8 files changed, 37 insertions(+), 39 deletions(-) diff --git a/thrift/lib/go/thrift/header_server_test.go b/thrift/lib/go/thrift/header_server_test.go index 1396ee044f9..75f837eb766 100644 --- a/thrift/lib/go/thrift/header_server_test.go +++ b/thrift/lib/go/thrift/header_server_test.go @@ -29,11 +29,8 @@ type headerServerTestProcessor struct { requests chan<- *MyTestStruct } -func (t *headerServerTestProcessor) GetProcessorFunction(name string) types.ProcessorFunction { - if name == "test" { - return &headerServerTestProcessorFunction{&testProcessorFunction{}, t.requests} - } - return nil +func (t *headerServerTestProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { + return map[string]types.ProcessorFunction{"test": &headerServerTestProcessorFunction{&testProcessorFunction{}, t.requests}} } type headerServerTestProcessorFunction struct { diff --git a/thrift/lib/go/thrift/interceptor.go b/thrift/lib/go/thrift/interceptor.go index 9c112dc9837..76fae6fcb57 100644 --- a/thrift/lib/go/thrift/interceptor.go +++ b/thrift/lib/go/thrift/interceptor.go @@ -47,16 +47,17 @@ func WrapInterceptor(interceptor Interceptor, p types.Processor) types.Processor } } -func (p *interceptorProcessor) GetProcessorFunction(name string) types.ProcessorFunction { - pf := p.Processor.GetProcessorFunction(name) - if pf == nil { - return nil // see ProcessContext, this semantic means 'no such function'. - } - return &interceptorProcessorFunction{ - interceptor: p.interceptor, - methodName: name, - ProcessorFunction: pf, +func (p *interceptorProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { + m := p.Processor.ProcessorFunctionMap() + mi := make(map[string]types.ProcessorFunction) + for name, pf := range m { + mi[name] = &interceptorProcessorFunction{ + interceptor: p.interceptor, + methodName: name, + ProcessorFunction: pf, + } } + return mi } type interceptorProcessorFunction struct { diff --git a/thrift/lib/go/thrift/interceptor_test.go b/thrift/lib/go/thrift/interceptor_test.go index 229d4820723..57f0131d771 100644 --- a/thrift/lib/go/thrift/interceptor_test.go +++ b/thrift/lib/go/thrift/interceptor_test.go @@ -28,7 +28,7 @@ type exampleProcessor struct { hit *bool } -func (ep *exampleProcessor) GetProcessorFunction(name string) types.ProcessorFunction { +func (ep *exampleProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { *ep.hit = true return nil // happens in "no such method" case } @@ -38,7 +38,7 @@ func TestInterceptorWrapperNilFunctionContext(t *testing.T) { proc := &exampleProcessor{nil, &hit} derivedProc := WrapInterceptor(emptyInterceptor, proc) - pFunc := derivedProc.GetProcessorFunction("blah") + pFunc := derivedProc.ProcessorFunctionMap()["blah"] if hit != true { t.Fatalf("interceptor should have called underlying processor function handler.") } diff --git a/thrift/lib/go/thrift/processor.go b/thrift/lib/go/thrift/processor.go index 4f8e97749ee..c183f84da9e 100644 --- a/thrift/lib/go/thrift/processor.go +++ b/thrift/lib/go/thrift/processor.go @@ -36,8 +36,11 @@ func getProcessorFunction(processor types.Processor, messageType types.MessageTy // case one: invalid message type return nil, types.NewApplicationException(types.UNKNOWN_METHOD, fmt.Sprintf("unexpected message type: %d", messageType)) } - if pf := processor.GetProcessorFunction(name); pf != nil { - return pf, nil + pmap := processor.ProcessorFunctionMap() + if pmap != nil { + if pf := pmap[name]; pf != nil { + return pf, nil + } } return nil, types.NewApplicationException(types.UNKNOWN_METHOD, fmt.Sprintf("no such function: %q", name)) } diff --git a/thrift/lib/go/thrift/rocket_server_test.go b/thrift/lib/go/thrift/rocket_server_test.go index 15218aac0f0..4c480a3f74f 100644 --- a/thrift/lib/go/thrift/rocket_server_test.go +++ b/thrift/lib/go/thrift/rocket_server_test.go @@ -30,11 +30,8 @@ type rocketServerTestProcessor struct { requests chan<- *MyTestStruct } -func (t *rocketServerTestProcessor) GetProcessorFunction(name string) types.ProcessorFunction { - if name == "test" { - return &rocketServerTestProcessorFunction{&testProcessorFunction{}, t.requests} - } - return nil +func (t *rocketServerTestProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { + return map[string]types.ProcessorFunction{"test": &rocketServerTestProcessorFunction{&testProcessorFunction{}, t.requests}} } type rocketServerTestProcessorFunction struct { diff --git a/thrift/lib/go/thrift/rocket_upgrade_processor.go b/thrift/lib/go/thrift/rocket_upgrade_processor.go index c8189caba1f..561ea3b1d3d 100644 --- a/thrift/lib/go/thrift/rocket_upgrade_processor.go +++ b/thrift/lib/go/thrift/rocket_upgrade_processor.go @@ -18,6 +18,7 @@ package thrift import ( "context" + "maps" "github.com/facebook/fbthrift/thrift/lib/go/thrift/types" ) @@ -31,19 +32,20 @@ func newRocketUpgradeProcessor(processor types.Processor) *rocketUpgradeProcesso return &rocketUpgradeProcessor{processor: processor} } -func (r *rocketUpgradeProcessor) GetProcessorFunction(name string) types.ProcessorFunction { +func (r *rocketUpgradeProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { + m := r.processor.ProcessorFunctionMap() // The upgradeToRocket function in thrift/lib/thrift/RocketUpgrade.thrift // asks the server to upgrade to using the rocket protocol. // If the server does not respond with an error, // it is assumed that the server has upgraded to rocket. - if name == "upgradeToRocket" { - r.upgraded = true - return &rocketUpgradeProcessorFunction{} - } - return r.processor.GetProcessorFunction(name) + mr := map[string]types.ProcessorFunction{"upgradeToRocket": &rocketUpgradeProcessorFunction{&r.upgraded}} + maps.Copy(mr, m) + return mr } -type rocketUpgradeProcessorFunction struct{} +type rocketUpgradeProcessorFunction struct { + upgraded *bool +} func (r *rocketUpgradeProcessorFunction) Read(prot types.Decoder) (types.Struct, types.Exception) { args := &reqServiceUpgradeToRocket{} @@ -77,5 +79,6 @@ func (r *rocketUpgradeProcessorFunction) Write(seqID int32, result types.Writabl if err2 = prot.Flush(); err == nil && err2 != nil { err = err2 } + *r.upgraded = true return err } diff --git a/thrift/lib/go/thrift/server_test.go b/thrift/lib/go/thrift/server_test.go index d1ae5c3eb1b..c49656a1630 100644 --- a/thrift/lib/go/thrift/server_test.go +++ b/thrift/lib/go/thrift/server_test.go @@ -62,11 +62,8 @@ func TestSimpleServer(t *testing.T) { type testProcessor struct { } -func (t *testProcessor) GetProcessorFunction(name string) types.ProcessorFunction { - if name == "test" { - return &testProcessorFunction{} - } - return nil +func (t *testProcessor) ProcessorFunctionMap() map[string]types.ProcessorFunction { + return map[string]types.ProcessorFunction{"test": &testProcessorFunction{}} } type testProcessorFunction struct{} diff --git a/thrift/lib/go/thrift/types/processor.go b/thrift/lib/go/thrift/types/processor.go index e94daf0c04c..ba01942c117 100644 --- a/thrift/lib/go/thrift/types/processor.go +++ b/thrift/lib/go/thrift/types/processor.go @@ -24,15 +24,15 @@ import ( // manage I/O and processing of a input message for a specific // server function type Processor interface { - // GetProcessorFunction is given the name of a thrift function and - // the type of the inbound thrift message. It is expected to return + // GetProcessorFunctionMap is given the name of a thrift function + // of the inbound thrift message. It is expected to return // a non-nil GetProcessorFunction when the function can be successfully // found. // - // If ProcessorFunction is nil, a generic error will be + // If GetProcessorFunctionMap is nil or a value in the map is nil, a generic error will be // sent which explains that no processor function exists with the specified // name on this server. - GetProcessorFunction(name string) ProcessorFunction + ProcessorFunctionMap() map[string]ProcessorFunction } // ProcessorFunction is the interface that must be implemented in