From 8712be6a6becbf17a280665b68ad0465b4fefb07 Mon Sep 17 00:00:00 2001 From: lvxinyu-1117 Date: Thu, 7 Aug 2025 19:15:53 +0800 Subject: [PATCH] fix: Correctly handle 'Array Item' when converting APIParameter (#634) --- backend/application/workflow/workflow.go | 17 +- backend/crossdomain/workflow/plugin/plugin.go | 75 +---- .../workflow/plugin/plugin_test.go | 316 ++++++++---------- .../workflow/crossdomain/plugin/plugin.go | 1 - .../plugin/pluginmock/plugin_mock.go | 14 - 5 files changed, 168 insertions(+), 255 deletions(-) diff --git a/backend/application/workflow/workflow.go b/backend/application/workflow/workflow.go index b324ea03..cc32d68e 100644 --- a/backend/application/workflow/workflow.go +++ b/backend/application/workflow/workflow.go @@ -2506,12 +2506,6 @@ func (w *ApplicationService) GetApiDetail(ctx context.Context, req *workflow.Get return nil, err } - for _, v := range outputVars { - if err := crossplugin.GetPluginService().UnwrapArrayItemFieldsInVariable(v); err != nil { - return nil, err - } - } - toolDetailInfo := &vo.ToolDetailInfo{ ApiDetailData: &workflow.ApiDetailData{ PluginID: req.GetPluginID(), @@ -3520,8 +3514,14 @@ func toVariable(p *workflow.APIParameter) (*vo.Variable, error) { v.Type = vo.VariableTypeBoolean case workflow.ParameterType_Array: v.Type = vo.VariableTypeList - if len(p.SubParameters) > 0 { - subVs := make([]*vo.Variable, 0) + if len(p.SubParameters) == 1 { + av, err := toVariable(p.SubParameters[0]) + if err != nil { + return nil, err + } + v.Schema = &av + } else if len(p.SubParameters) > 1 { + subVs := make([]any, 0) for _, ap := range p.SubParameters { av, err := toVariable(ap) if err != nil { @@ -3534,7 +3534,6 @@ func toVariable(p *workflow.APIParameter) (*vo.Variable, error) { Schema: subVs, } } - case workflow.ParameterType_Object: v.Type = vo.VariableTypeObject vs := make([]*vo.Variable, 0) diff --git a/backend/crossdomain/workflow/plugin/plugin.go b/backend/crossdomain/workflow/plugin/plugin.go index c21d0a2c..74415afa 100644 --- a/backend/crossdomain/workflow/plugin/plugin.go +++ b/backend/crossdomain/workflow/plugin/plugin.go @@ -220,63 +220,6 @@ func (t *pluginService) GetPluginToolsInfo(ctx context.Context, req *crossplugin return response, nil } -func (t *pluginService) UnwrapArrayItemFieldsInVariable(v *vo.Variable) error { - if v == nil { - return nil - } - - if v.Type == vo.VariableTypeObject { - subVars, ok := v.Schema.([]*vo.Variable) - if !ok { - return nil - } - - newSubVars := make([]*vo.Variable, 0, len(subVars)) - for _, subVar := range subVars { - if subVar.Name == "[Array Item]" { - if err := t.UnwrapArrayItemFieldsInVariable(subVar); err != nil { - return err - } - // If the array item is an object, append its children - if subVar.Type == vo.VariableTypeObject { - if innerSubVars, ok := subVar.Schema.([]*vo.Variable); ok { - newSubVars = append(newSubVars, innerSubVars...) - } - } else { - // If the array item is a primitive type, clear its name and append it - subVar.Name = "" - newSubVars = append(newSubVars, subVar) - } - } else { - // For other sub-variables, recursively unwrap and append - if err := t.UnwrapArrayItemFieldsInVariable(subVar); err != nil { - return err - } - newSubVars = append(newSubVars, subVar) - } - } - v.Schema = newSubVars - - } else if v.Type == vo.VariableTypeList { - if v.Schema != nil { - subVar, ok := v.Schema.(*vo.Variable) - if !ok { - return nil - } - - if err := t.UnwrapArrayItemFieldsInVariable(subVar); err != nil { - return err - } - // If the array item definition itself has "[Array Item]" name, clear it - if subVar.Name == "[Array Item]" { - subVar.Name = "" - } - v.Schema = subVar - } - } - return nil -} - func (t *pluginService) GetPluginInvokableTools(ctx context.Context, req *crossplugin.ToolsInvokableRequest) ( _ map[int64]crossplugin.InvokableTool, err error) { defer func() { @@ -563,7 +506,23 @@ func toWorkflowAPIParameter(parameter *common.APIParameter) *workflow3.APIParame p.AssistType = ptr.Of(workflow3.AssistParameterType(*parameter.AssistType)) } - if len(parameter.SubParameters) > 0 { + // Check if it's an array that needs unwrapping. + if parameter.Type == common.ParameterType_Array && len(parameter.SubParameters) == 1 && parameter.SubParameters[0].Name == "[Array Item]" { + arrayItem := parameter.SubParameters[0] + p.SubType = ptr.Of(workflow3.ParameterType(arrayItem.Type)) + // If the "[Array Item]" is an object, its sub-parameters become the array's sub-parameters. + if arrayItem.Type == common.ParameterType_Object { + p.SubParameters = make([]*workflow3.APIParameter, 0, len(arrayItem.SubParameters)) + for _, subParam := range arrayItem.SubParameters { + p.SubParameters = append(p.SubParameters, toWorkflowAPIParameter(subParam)) + } + } else { + // The array's SubType is the Type of the "[Array Item]". + p.SubParameters = make([]*workflow3.APIParameter, 0, 1) + p.SubParameters = append(p.SubParameters, toWorkflowAPIParameter(arrayItem)) + p.SubParameters[0].Name = "" // Remove the "[Array Item]" name. + } + } else if len(parameter.SubParameters) > 0 { // A simple object or a non-wrapped array. p.SubParameters = make([]*workflow3.APIParameter, 0, len(parameter.SubParameters)) for _, subParam := range parameter.SubParameters { p.SubParameters = append(p.SubParameters, toWorkflowAPIParameter(subParam)) diff --git a/backend/crossdomain/workflow/plugin/plugin_test.go b/backend/crossdomain/workflow/plugin/plugin_test.go index eaf632b7..e6a0a460 100644 --- a/backend/crossdomain/workflow/plugin/plugin_test.go +++ b/backend/crossdomain/workflow/plugin/plugin_test.go @@ -19,200 +19,170 @@ package plugin import ( "testing" + common "github.com/coze-dev/coze-studio/backend/api/model/plugin_develop/common" + workflow3 "github.com/coze-dev/coze-studio/backend/api/model/workflow" + "github.com/coze-dev/coze-studio/backend/pkg/lang/ptr" "github.com/stretchr/testify/assert" - - "github.com/coze-dev/coze-studio/backend/domain/workflow/entity/vo" ) -func TestPluginService_UnwrapArrayItemFieldsInVariable(t *testing.T) { - s := &pluginService{} - t.Run("unwraps a simple array item", func(t *testing.T) { - input := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - { - Name: "[Array Item]", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - {Name: "field2", Type: vo.VariableTypeInteger}, +func TestToWorkflowAPIParameter(t *testing.T) { + cases := []struct { + name string + param *common.APIParameter + expected *workflow3.APIParameter + }{ + { + name: "nil parameter", + param: nil, + expected: nil, + }, + { + name: "simple string parameter", + param: &common.APIParameter{ + Name: "prompt", + Type: common.ParameterType_String, + Desc: "User's prompt", + }, + expected: &workflow3.APIParameter{ + Name: "prompt", + Type: workflow3.ParameterType_String, + Desc: "User's prompt", + }, + }, + { + name: "simple object parameter", + param: &common.APIParameter{ + Name: "user_info", + Type: common.ParameterType_Object, + SubParameters: []*common.APIParameter{ + { + Name: "name", + Type: common.ParameterType_String, + }, + { + Name: "age", + Type: common.ParameterType_Number, }, }, }, - } - - expected := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - {Name: "field2", Type: vo.VariableTypeInteger}, + expected: &workflow3.APIParameter{ + Name: "user_info", + Type: workflow3.ParameterType_Object, + SubParameters: []*workflow3.APIParameter{ + { + Name: "name", + Type: workflow3.ParameterType_String, + }, + { + Name: "age", + Type: workflow3.ParameterType_Number, + }, + }, }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("handles nested array items", func(t *testing.T) { - input := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - { - Name: "[Array Item]", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - { - Name: "[Array Item]", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "nestedField", Type: vo.VariableTypeBoolean}, - }, - }, + }, + { + name: "array of strings", + param: &common.APIParameter{ + Name: "tags", + Type: common.ParameterType_Array, + SubParameters: []*common.APIParameter{ + { + Name: "[Array Item]", + Type: common.ParameterType_String, }, }, }, - } - - expected := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - {Name: "nestedField", Type: vo.VariableTypeBoolean}, + expected: &workflow3.APIParameter{ + Name: "tags", + Type: workflow3.ParameterType_Array, + SubType: ptr.Of(workflow3.ParameterType_String), + SubParameters: []*workflow3.APIParameter{ + { + Type: workflow3.ParameterType_String, + }, + }, }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("handles array item within a list", func(t *testing.T) { - input := &vo.Variable{ - Name: "rootList", - Type: vo.VariableTypeList, - Schema: &vo.Variable{ - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ + }, + { + name: "array of objects", + param: &common.APIParameter{ + Name: "users", + Type: common.ParameterType_Array, + SubParameters: []*common.APIParameter{ { Name: "[Array Item]", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "itemField", Type: vo.VariableTypeString}, + Type: common.ParameterType_Object, + SubParameters: []*common.APIParameter{ + { + Name: "name", + Type: common.ParameterType_String, + }, + { + Name: "id", + Type: common.ParameterType_Number, + }, }, }, }, }, - } - - expected := &vo.Variable{ - Name: "rootList", - Type: vo.VariableTypeList, - Schema: &vo.Variable{ - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "itemField", Type: vo.VariableTypeString}, + expected: &workflow3.APIParameter{ + Name: "users", + Type: workflow3.ParameterType_Array, + SubType: ptr.Of(workflow3.ParameterType_Object), + SubParameters: []*workflow3.APIParameter{ + { + Name: "name", + Type: workflow3.ParameterType_String, + }, + { + Name: "id", + Type: workflow3.ParameterType_Number, + }, }, }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("does nothing if no array item is present", func(t *testing.T) { - input := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - {Name: "field2", Type: vo.VariableTypeInteger}, - }, - } - - // Create a copy for comparison as the input will be modified in place. - expected := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - {Name: "field1", Type: vo.VariableTypeString}, - {Name: "field2", Type: vo.VariableTypeInteger}, - }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("handles primitive type array item in object", func(t *testing.T) { - input := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - { - Name: "[Array Item]", - Type: vo.VariableTypeString, - }, - { - Name: "anotherField", - Type: vo.VariableTypeInteger, + }, + { + name: "array of array of strings", + param: &common.APIParameter{ + Name: "matrix", + Type: common.ParameterType_Array, + SubParameters: []*common.APIParameter{ + { + Name: "[Array Item]", + Type: common.ParameterType_Array, + SubParameters: []*common.APIParameter{ + { + Name: "[Array Item]", + Type: common.ParameterType_String, + }, + }, + }, }, }, - } - - expected := &vo.Variable{ - Name: "root", - Type: vo.VariableTypeObject, - Schema: []*vo.Variable{ - { - Name: "", - Type: vo.VariableTypeString, - }, - { - Name: "anotherField", - Type: vo.VariableTypeInteger, + expected: &workflow3.APIParameter{ + Name: "matrix", + Type: workflow3.ParameterType_Array, + SubType: ptr.Of(workflow3.ParameterType_Array), + SubParameters: []*workflow3.APIParameter{ + { + Name: "", // Name is cleared + Type: workflow3.ParameterType_Array, + SubType: ptr.Of(workflow3.ParameterType_String), + SubParameters: []*workflow3.APIParameter{ + { + Type: workflow3.ParameterType_String, + }, + }, + }, }, }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("handles list of primitives", func(t *testing.T) { - input := &vo.Variable{ - Name: "listOfStrings", - Type: vo.VariableTypeList, - Schema: &vo.Variable{ - Name: "[Array Item]", - Type: vo.VariableTypeString, - }, - } - - expected := &vo.Variable{ - Name: "listOfStrings", - Type: vo.VariableTypeList, - Schema: &vo.Variable{ - Name: "", - Type: vo.VariableTypeString, - }, - } - - err := s.UnwrapArrayItemFieldsInVariable(input) - assert.NoError(t, err) - assert.Equal(t, expected, input) - }) - - t.Run("handles nil input", func(t *testing.T) { - err := s.UnwrapArrayItemFieldsInVariable(nil) - assert.NoError(t, err) - }) + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + actual := toWorkflowAPIParameter(tc.param) + assert.Equal(t, tc.expected, actual) + }) + } } diff --git a/backend/domain/workflow/crossdomain/plugin/plugin.go b/backend/domain/workflow/crossdomain/plugin/plugin.go index 2810048c..b57183aa 100644 --- a/backend/domain/workflow/crossdomain/plugin/plugin.go +++ b/backend/domain/workflow/crossdomain/plugin/plugin.go @@ -30,7 +30,6 @@ import ( //go:generate mockgen -destination pluginmock/plugin_mock.go --package pluginmock -source plugin.go type Service interface { GetPluginToolsInfo(ctx context.Context, req *ToolsInfoRequest) (*ToolsInfoResponse, error) - UnwrapArrayItemFieldsInVariable(v *vo.Variable) error GetPluginInvokableTools(ctx context.Context, req *ToolsInvokableRequest) (map[int64]InvokableTool, error) ExecutePlugin(ctx context.Context, input map[string]any, pe *Entity, toolID int64, cfg ExecConfig) (map[string]any, error) diff --git a/backend/domain/workflow/crossdomain/plugin/pluginmock/plugin_mock.go b/backend/domain/workflow/crossdomain/plugin/pluginmock/plugin_mock.go index 3a5e9522..fb2f53ff 100644 --- a/backend/domain/workflow/crossdomain/plugin/pluginmock/plugin_mock.go +++ b/backend/domain/workflow/crossdomain/plugin/pluginmock/plugin_mock.go @@ -31,7 +31,6 @@ import ( schema "github.com/cloudwego/eino/schema" plugin "github.com/coze-dev/coze-studio/backend/domain/workflow/crossdomain/plugin" - vo "github.com/coze-dev/coze-studio/backend/domain/workflow/entity/vo" gomock "go.uber.org/mock/gomock" ) @@ -104,19 +103,6 @@ func (mr *MockServiceMockRecorder) GetPluginToolsInfo(ctx, req any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPluginToolsInfo", reflect.TypeOf((*MockService)(nil).GetPluginToolsInfo), ctx, req) } -// UnwrapArrayItemFieldsInVariable mocks base method. -func (m *MockService) UnwrapArrayItemFieldsInVariable(v *vo.Variable) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UnwrapArrayItemFieldsInVariable", v) - ret0, _ := ret[0].(error) - return ret0 -} - -// UnwrapArrayItemFieldsInVariable indicates an expected call of UnwrapArrayItemFieldsInVariable. -func (mr *MockServiceMockRecorder) UnwrapArrayItemFieldsInVariable(v any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnwrapArrayItemFieldsInVariable", reflect.TypeOf((*MockService)(nil).UnwrapArrayItemFieldsInVariable), v) -} // MockInvokableTool is a mock of InvokableTool interface. type MockInvokableTool struct {