Skip to content

fix: get pipeline list filter by permission's datasource#3067

Merged
Seechi-Yolo merged 2 commits intomainfrom
fix_pipeline_permission
Jun 26, 2025
Merged

fix: get pipeline list filter by permission's datasource#3067
Seechi-Yolo merged 2 commits intomainfrom
fix_pipeline_permission

Conversation

@Jarvis1105
Copy link
Copy Markdown
Contributor

@Jarvis1105 Jarvis1105 commented Jun 26, 2025

User description

关联的 issue

https://github.com/actiontech/sqle-ee/issues/2387

描述你的变更

  • 流水线列表展示关联的数据源
  • 查询流水线需要根据权限的数据源范围进行筛选

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 修正流水线列表权限过滤逻辑

  • 增加 data_sources 字段展示数据源信息

  • 调整权限检查调用 GetOnePermission 方法

  • 更新 API 文档和 Swagger 定义


Changes walkthrough 📝

Relevant files
Enhancement
2 files
pipeline.go
更新流水线详情和权限过滤逻辑                                                                                     
+36/-10 
permission.go
添加 GetOnePermission 权限查询方法                                                             
+9/-0     
Documentation
3 files
docs.go
增加 data_sources 文档描述                                                                         
+14/-0   
swagger.json
更新 Swagger 中 data_sources 定义                                                         
+14/-0   
swagger.yaml
补充 Swagger data_sources 属性定义                                                         
+10/-0   
Bug fix
2 files
pipline.go
修改 GetPipelineList 签名及查询条件                                                             
+6/-1     
pipeline.go
调整流水线列表查询参数传递                                                                                       
+2/-2     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    github-actions Bot commented Jun 26, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 75a0597
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    转换数据类型匹配

    建议确认数据库查询中使用的 rangeDatasourceIds 与数据库中 instance_id 字段的数据类型匹配。若 instance_id 为数值类型,须将
    rangeDatasourceIds 的字符串转换为数值,避免因类型不一致导致的查询错误或 panic。

    sqle/model/pipline.go [114-118]

     if len(rangeDatasourceIds) > 0 {
    +    numIDs := make([]int, 0, len(rangeDatasourceIds))
    +    for _, idStr := range rangeDatasourceIds {
    +        if id, err := strconv.Atoi(idStr); err == nil {
    +            numIDs = append(numIDs, id)
    +        }
    +    }
         query = query.Joins("JOIN pipeline_nodes ON pipelines.id = pipeline_nodes.pipeline_id").
    -        Where("pipeline_nodes.instance_id IN (?)", rangeDatasourceIds).
    +        Where("pipeline_nodes.instance_id IN (?)", numIDs).
             Group("pipelines.id")
     }
    Suggestion importance[1-10]: 7

    __

    Why: 建议对 rangeDatasourceIds 的字符串进行数值转换,确保与数据库中 instance_id 类型一致,改进合理且实现完备,但属于类型匹配的检查,故影响有限。

    Medium
    General
    使用传入上下文

    建议在调用 dms.GetInstancesById 时,采用传入的上下文参数而非 context.TODO()。最好在 fillWith 方法中增加一个
    context 参数,这样可以正确传播请求的上下文并处理取消信号。

    sqle/api/controller/v1/pipeline.go [34]

    -instance, exist, err := dms.GetInstancesById(context.TODO(), fmt.Sprint(node.InstanceID))
    +instance, exist, err := dms.GetInstancesById(ctx, fmt.Sprint(node.InstanceID))
    Suggestion importance[1-10]: 6

    __

    Why: 建议将 context.TODO() 替换为传入的 ctx 以便传播请求上下文,但 improved_code 仅修改了调用处而没有更新 fillWith 的函数签名,导致不完整。

    Low

    Previous suggestions

    Suggestions up to commit 1fde6a5
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    修正返回循环变量指针

    建议修改返回指针的方式,避免对循环变量地址的引用问题,该问题可能导致获取的权限数据不正确。请使用索引访问 p.opPermissionItem
    并取其地址以确保返回的是正确的元素地址。

    sqle/dms/permission.go [101-105]

    -for _, userOpPermission := range p.opPermissionItem {
    -		if userOpPermission.OpPermissionType == opPermissionType {
    -			return &userOpPermission
    +for i := range p.opPermissionItem {
    +		if p.opPermissionItem[i].OpPermissionType == opPermissionType {
    +			return &p.opPermissionItem[i]
     		}
     }
    Suggestion importance[1-10]: 9

    __

    Why: The change addresses a subtle Go bug regarding pointers to loop variables and ensures that the proper permission element is returned. This correction is accurate and significantly improves the reliability of the function.

    High
    检查 fillWith 错误

    建议在调用 fillWith 后检查返回的 err,防止出现未处理的错误情况引发程序 panic。应在循环中加入错误判断,并及时返回错误信息。

    sqle/api/controller/v1/pipeline.go [256-257]

     for idx, pipe := range pipelineList {
    -		err = data[idx].fillWith(pipe)
    +		if err = data[idx].fillWith(pipe); err != nil {
    +			return controller.JSONBaseErrorReq(c, err)
    +		}
     }
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion improves error handling by checking the error return from fillWith, preventing possible panics. However, as it only adds an error check in a loop, its impact is important but within typical defensive programming practices.

    Medium

    opt for GetOnePermission
    @Seechi-Yolo Seechi-Yolo merged commit 874f22c into main Jun 26, 2025
    4 checks passed
    @Jarvis1105 Jarvis1105 deleted the fix_pipeline_permission branch June 26, 2025 10:04
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants