Skip to content

Commit eb8e67d

Browse files
committed
fix(evaluation): panic
fix(evaluation): BatchGetExperimentResult with sorted turn results fix(evaluation): BatchGetExperimentResult panic fix(evaluation): kill expt fix(evaluation): kill expt fix(evaluation): ut
1 parent b42010f commit eb8e67d

29 files changed

+1162
-140
lines changed

backend/modules/evaluation/application/experiment_app.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"fmt"
99
"strconv"
10+
"time"
1011

1112
"github.com/bytedance/gg/gptr"
1213

@@ -520,18 +521,24 @@ func (e *experimentApplication) KillExperiment(ctx context.Context, req *expt.Ki
520521
return nil, err
521522
}
522523

523-
err = e.auth.AuthorizationWithoutSPI(ctx, &rpc.AuthorizationWithoutSPIParam{
524-
ObjectID: strconv.FormatInt(req.GetExptID(), 10),
525-
SpaceID: req.GetWorkspaceID(),
526-
ActionObjects: []*rpc.ActionObject{{Action: gptr.Of(consts.Run), EntityType: gptr.Of(rpc.AuthEntityType_EvaluationExperiment)}},
527-
OwnerID: gptr.Of(got.CreatedBy),
528-
ResourceSpaceID: req.GetWorkspaceID(),
529-
})
530-
if err != nil {
524+
if !e.configer.GetMaintainerUserIDs(ctx)[session.UserID] {
525+
if err := e.auth.AuthorizationWithoutSPI(ctx, &rpc.AuthorizationWithoutSPIParam{
526+
ObjectID: strconv.FormatInt(req.GetExptID(), 10),
527+
SpaceID: req.GetWorkspaceID(),
528+
ActionObjects: []*rpc.ActionObject{{Action: gptr.Of(consts.Run), EntityType: gptr.Of(rpc.AuthEntityType_EvaluationExperiment)}},
529+
OwnerID: gptr.Of(got.CreatedBy),
530+
ResourceSpaceID: req.GetWorkspaceID(),
531+
}); err != nil {
532+
return nil, err
533+
}
534+
}
535+
536+
if err := e.manager.CompleteRun(ctx, req.GetExptID(), got.LatestRunID, req.GetWorkspaceID(), session, entity.WithStatus(entity.ExptStatus_Terminated)); err != nil {
531537
return nil, err
532538
}
533539

534-
if err := e.manager.CompleteExpt(ctx, req.GetExptID(), req.GetWorkspaceID(), session, entity.WithStatus(entity.ExptStatus_Terminated)); err != nil {
540+
if err := e.manager.CompleteExpt(ctx, req.GetExptID(), req.GetWorkspaceID(), session, entity.WithStatus(entity.ExptStatus_Terminated),
541+
entity.WithCompleteInterval(time.Second), entity.NoCompleteItemTurn(), entity.NoAggrCalculate()); err != nil {
535542
return nil, err
536543
}
537544

backend/modules/evaluation/application/experiment_app_test.go

Lines changed: 145 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
repo_mocks "github.com/coze-dev/coze-loop/backend/modules/evaluation/domain/repo/mocks"
1919

2020
idgenmock "github.com/coze-dev/coze-loop/backend/infra/idgen/mocks"
21+
"github.com/coze-dev/coze-loop/backend/infra/middleware/session"
2122
"github.com/coze-dev/coze-loop/backend/kitex_gen/base"
2223
"github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/data/domain/tag"
2324
"github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/evaluation/domain/common"
@@ -1814,11 +1815,13 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
18141815
// Create mock objects
18151816
mockManager := servicemocks.NewMockIExptManager(ctrl)
18161817
mockAuth := rpcmocks.NewMockIAuthProvider(ctrl)
1818+
mockConfiger := componentMocks.NewMockIConfiger(ctrl)
18171819

18181820
// Test data
18191821
validWorkspaceID := int64(123)
18201822
validExptID := int64(456)
18211823
validUserID := int64(789)
1824+
validRunID := int64(999)
18221825

18231826
tests := []struct {
18241827
name string
@@ -1828,19 +1831,56 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
18281831
wantErr bool
18291832
}{
18301833
{
1831-
name: "successfully terminate experiment",
1834+
name: "successfully terminate experiment with maintainer permission",
18321835
req: &exptpb.KillExperimentRequest{
18331836
WorkspaceID: gptr.Of(validWorkspaceID),
18341837
ExptID: gptr.Of(validExptID),
18351838
},
18361839
mockSetup: func() {
18371840
// 获取实验信息
18381841
mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(&entity.Experiment{
1839-
ID: validExptID,
1840-
SpaceID: validWorkspaceID,
1841-
CreatedBy: strconv.FormatInt(validUserID, 10),
1842+
ID: validExptID,
1843+
SpaceID: validWorkspaceID,
1844+
CreatedBy: strconv.FormatInt(validUserID, 10),
1845+
LatestRunID: validRunID,
18421846
}, nil)
18431847

1848+
// Maintainer权限检查 - 用户是maintainer
1849+
mockConfiger.EXPECT().GetMaintainerUserIDs(gomock.Any()).Return(map[string]bool{
1850+
strconv.FormatInt(validUserID, 10): true,
1851+
})
1852+
1853+
// 终止运行 - 不需要权限验证,因为是maintainer
1854+
mockManager.EXPECT().CompleteRun(gomock.Any(), validExptID, validRunID, validWorkspaceID, gomock.Any(), gomock.Any()).Return(nil)
1855+
1856+
// 终止实验 - 注意参数顺序:ctx, exptID, spaceID, session, opts...
1857+
mockManager.EXPECT().CompleteExpt(gomock.Any(), validExptID, validWorkspaceID, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
1858+
},
1859+
wantResp: &exptpb.KillExperimentResponse{
1860+
BaseResp: base.NewBaseResp(),
1861+
},
1862+
wantErr: false,
1863+
},
1864+
{
1865+
name: "successfully terminate experiment with regular permission",
1866+
req: &exptpb.KillExperimentRequest{
1867+
WorkspaceID: gptr.Of(validWorkspaceID),
1868+
ExptID: gptr.Of(validExptID),
1869+
},
1870+
mockSetup: func() {
1871+
// 获取实验信息
1872+
mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(&entity.Experiment{
1873+
ID: validExptID,
1874+
SpaceID: validWorkspaceID,
1875+
CreatedBy: strconv.FormatInt(validUserID, 10),
1876+
LatestRunID: validRunID,
1877+
}, nil)
1878+
1879+
// Maintainer权限检查 - 用户不是maintainer
1880+
mockConfiger.EXPECT().GetMaintainerUserIDs(gomock.Any()).Return(map[string]bool{
1881+
"other_user": true,
1882+
})
1883+
18441884
// 权限验证
18451885
mockAuth.EXPECT().AuthorizationWithoutSPI(gomock.Any(), &rpc.AuthorizationWithoutSPIParam{
18461886
ObjectID: strconv.FormatInt(validExptID, 10),
@@ -1850,19 +1890,11 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
18501890
ResourceSpaceID: validWorkspaceID,
18511891
}).Return(nil)
18521892

1853-
// 终止实验
1854-
mockManager.EXPECT().CompleteExpt(gomock.Any(), validExptID, validWorkspaceID, gomock.Any(), gomock.Any()).DoAndReturn(
1855-
func(ctx context.Context, exptID, spaceID int64, session *entity.Session, opts ...entity.CompleteExptOptionFn) error {
1856-
// 验证传入的 opts 是否包含正确的状态设置
1857-
opt := &entity.CompleteExptOption{}
1858-
for _, fn := range opts {
1859-
fn(opt)
1860-
}
1861-
if opt.Status != entity.ExptStatus_Terminated {
1862-
t.Errorf("expected status %v, got %v", entity.ExptStatus_Terminated, opt.Status)
1863-
}
1864-
return nil
1865-
})
1893+
// 终止运行
1894+
mockManager.EXPECT().CompleteRun(gomock.Any(), validExptID, validRunID, validWorkspaceID, gomock.Any(), gomock.Any()).Return(nil)
1895+
1896+
// 终止实验 - 实际调用有4个选项参数:WithStatus, WithCompleteInterval, NoCompleteItemTurn, NoAggrCalculate
1897+
mockManager.EXPECT().CompleteExpt(gomock.Any(), validExptID, validWorkspaceID, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
18661898
},
18671899
wantResp: &exptpb.KillExperimentResponse{
18681900
BaseResp: base.NewBaseResp(),
@@ -1881,6 +1913,95 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
18811913
wantResp: nil,
18821914
wantErr: true,
18831915
},
1916+
{
1917+
name: "permission validation failed for regular user",
1918+
req: &exptpb.KillExperimentRequest{
1919+
WorkspaceID: gptr.Of(validWorkspaceID),
1920+
ExptID: gptr.Of(validExptID),
1921+
},
1922+
mockSetup: func() {
1923+
// 获取实验信息
1924+
mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(&entity.Experiment{
1925+
ID: validExptID,
1926+
SpaceID: validWorkspaceID,
1927+
CreatedBy: strconv.FormatInt(validUserID, 10),
1928+
LatestRunID: validRunID,
1929+
}, nil)
1930+
1931+
// Maintainer权限检查 - 用户不是maintainer
1932+
mockConfiger.EXPECT().GetMaintainerUserIDs(gomock.Any()).Return(map[string]bool{
1933+
"other_user": true,
1934+
})
1935+
1936+
// 权限验证失败
1937+
mockAuth.EXPECT().AuthorizationWithoutSPI(gomock.Any(), &rpc.AuthorizationWithoutSPIParam{
1938+
ObjectID: strconv.FormatInt(validExptID, 10),
1939+
SpaceID: validWorkspaceID,
1940+
ActionObjects: []*rpc.ActionObject{{Action: gptr.Of(consts.Run), EntityType: gptr.Of(rpc.AuthEntityType_EvaluationExperiment)}},
1941+
OwnerID: gptr.Of(strconv.FormatInt(validUserID, 10)),
1942+
ResourceSpaceID: validWorkspaceID,
1943+
}).Return(errorx.NewByCode(errno.CommonNoPermissionCode))
1944+
},
1945+
wantResp: nil,
1946+
wantErr: true,
1947+
},
1948+
{
1949+
name: "complete run failed",
1950+
req: &exptpb.KillExperimentRequest{
1951+
WorkspaceID: gptr.Of(validWorkspaceID),
1952+
ExptID: gptr.Of(validExptID),
1953+
},
1954+
mockSetup: func() {
1955+
// 获取实验信息
1956+
mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(&entity.Experiment{
1957+
ID: validExptID,
1958+
SpaceID: validWorkspaceID,
1959+
CreatedBy: strconv.FormatInt(validUserID, 10),
1960+
LatestRunID: validRunID,
1961+
}, nil)
1962+
1963+
// Maintainer权限检查 - 用户是maintainer
1964+
mockConfiger.EXPECT().GetMaintainerUserIDs(gomock.Any()).Return(map[string]bool{
1965+
strconv.FormatInt(validUserID, 10): true,
1966+
})
1967+
1968+
// 终止运行失败
1969+
mockManager.EXPECT().CompleteRun(gomock.Any(), validExptID, validRunID, validWorkspaceID, gomock.Any(), gomock.Any()).Return(
1970+
errorx.NewByCode(errno.CommonInternalErrorCode))
1971+
},
1972+
wantResp: nil,
1973+
wantErr: true,
1974+
},
1975+
{
1976+
name: "complete experiment failed",
1977+
req: &exptpb.KillExperimentRequest{
1978+
WorkspaceID: gptr.Of(validWorkspaceID),
1979+
ExptID: gptr.Of(validExptID),
1980+
},
1981+
mockSetup: func() {
1982+
// 获取实验信息
1983+
mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(&entity.Experiment{
1984+
ID: validExptID,
1985+
SpaceID: validWorkspaceID,
1986+
CreatedBy: strconv.FormatInt(validUserID, 10),
1987+
LatestRunID: validRunID,
1988+
}, nil)
1989+
1990+
// Maintainer权限检查 - 用户是maintainer
1991+
mockConfiger.EXPECT().GetMaintainerUserIDs(gomock.Any()).Return(map[string]bool{
1992+
strconv.FormatInt(validUserID, 10): true,
1993+
})
1994+
1995+
// 终止运行
1996+
mockManager.EXPECT().CompleteRun(gomock.Any(), validExptID, validRunID, validWorkspaceID, gomock.Any(), gomock.Any()).Return(nil)
1997+
1998+
// 终止实验失败 - 实际调用有4个选项参数:WithStatus, WithCompleteInterval, NoCompleteItemTurn, NoAggrCalculate
1999+
mockManager.EXPECT().CompleteExpt(gomock.Any(), validExptID, validWorkspaceID, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
2000+
errorx.NewByCode(errno.CommonInternalErrorCode))
2001+
},
2002+
wantResp: nil,
2003+
wantErr: true,
2004+
},
18842005
}
18852006

18862007
for _, tt := range tests {
@@ -1896,7 +2017,7 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
18962017
nil, // scheduler
18972018
nil, // recordEval
18982019
nil,
1899-
nil, // configer
2020+
mockConfiger, // configer
19002021
mockAuth,
19012022
nil, // userInfoService
19022023
nil, // evalTargetService
@@ -1907,8 +2028,13 @@ func TestExperimentApplication_KillExperiment(t *testing.T) {
19072028
nil,
19082029
)
19092030

2031+
// 设置 context 中的 UserID,这样 entity.NewSession 才能获取到 UserID
2032+
ctx := session.WithCtxUser(context.Background(), &session.User{
2033+
ID: strconv.FormatInt(validUserID, 10),
2034+
})
2035+
19102036
// 执行测试
1911-
gotResp, err := app.KillExperiment(context.Background(), tt.req)
2037+
gotResp, err := app.KillExperiment(ctx, tt.req)
19122038

19132039
// 验证结果
19142040
if tt.wantErr {

backend/modules/evaluation/domain/component/conf.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ type IConfiger interface {
1818
GetExptTurnResultFilterBmqProducerCfg(ctx context.Context) *entity.BmqProducerCfg
1919
GetCKDBName(ctx context.Context) *entity.CKDBConfig
2020
GetExptExportWhiteList(ctx context.Context) *entity.ExptExportWhiteList
21+
GetMaintainerUserIDs(ctx context.Context) map[string]bool
2122
}

backend/modules/evaluation/domain/component/mocks/expt_configer.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

backend/modules/evaluation/domain/entity/common.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,3 +355,7 @@ func StorageProviderFromString(s string) (StorageProvider, error) {
355355
}
356356

357357
func StorageProviderPtr(v StorageProvider) *StorageProvider { return &v }
358+
359+
type SystemMaintainerConf struct {
360+
UserIDs []string `json:"user_ids"`
361+
}

backend/modules/evaluation/domain/entity/expt.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,6 @@ type ExptCalculateStats struct {
265265
SuccessItemCnt int
266266
ProcessingItemCnt int
267267
TerminatedItemCnt int
268-
269-
IncompleteTurnIDs []*ItemTurnID
270268
}
271269

272270
type ItemTurnID struct {

backend/modules/evaluation/domain/entity/param.go

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
package entity
55

6+
import (
7+
"time"
8+
)
9+
610
type CreateEvaluationSetParam struct {
711
SpaceID int64
812
Name string
@@ -213,13 +217,34 @@ func WithCheckBenefit() ExptRunCheckOptionFn {
213217
}
214218

215219
type CompleteExptOption struct {
216-
Status ExptStatus
217-
StatusMessage string
218-
CID string
220+
Status ExptStatus
221+
StatusMessage string
222+
CID string
223+
CompleteInterval time.Duration
224+
NoAggrCalculate bool
225+
NoCompleteItemTurn bool
219226
}
220227

221228
type CompleteExptOptionFn func(*CompleteExptOption)
222229

230+
func NoAggrCalculate() CompleteExptOptionFn {
231+
return func(c *CompleteExptOption) {
232+
c.NoAggrCalculate = true
233+
}
234+
}
235+
236+
func NoCompleteItemTurn() CompleteExptOptionFn {
237+
return func(c *CompleteExptOption) {
238+
c.NoCompleteItemTurn = true
239+
}
240+
}
241+
242+
func WithCompleteInterval(interval time.Duration) CompleteExptOptionFn {
243+
return func(c *CompleteExptOption) {
244+
c.CompleteInterval = interval
245+
}
246+
}
247+
223248
func WithStatus(status ExptStatus) CompleteExptOptionFn {
224249
return func(c *CompleteExptOption) {
225250
c.Status = status

backend/modules/evaluation/domain/repo/expt.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
type IExperimentRepo interface {
1515
Create(ctx context.Context, expt *entity.Experiment, exptEvaluatorRefs []*entity.ExptEvaluatorRef) error
1616
Update(ctx context.Context, expt *entity.Experiment) error
17+
UpdateFields(ctx context.Context, exptID int64, ufields map[string]any) error
1718
Delete(ctx context.Context, id, spaceID int64) error
1819
MDelete(ctx context.Context, ids []int64, spaceID int64) error
1920
List(ctx context.Context, page, size int32, filter *entity.ExptListFilter, orders []*entity.OrderBy, spaceID int64) ([]*entity.Experiment, int64, error)
@@ -139,6 +140,7 @@ type IEvalAsyncRepo interface {
139140
GetEvalAsyncCtx(ctx context.Context, invokeID string) (*entity.EvalAsyncCtx, error)
140141
SetEvalAsyncCtx(ctx context.Context, invokeID string, actx *entity.EvalAsyncCtx) error
141142
}
143+
142144
type IExptInsightAnalysisRecordRepo interface {
143145
CreateAnalysisRecord(ctx context.Context, record *entity.ExptInsightAnalysisRecord, opts ...db.Option) (int64, error)
144146
UpdateAnalysisRecord(ctx context.Context, record *entity.ExptInsightAnalysisRecord, opts ...db.Option) error

backend/modules/evaluation/domain/repo/mocks/expt.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)