This is an automated email from the ASF dual-hosted git repository.

likyh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git


The following commit(s) were added to refs/heads/main by this push:
     new 73075971 refactor(github): divide making scopes and making plans 
(#3868)
73075971 is described below

commit 73075971e98b780fbda4cac906fac1771d8c28f7
Author: Warren Chen <[email protected]>
AuthorDate: Wed Dec 7 14:14:47 2022 +0800

    refactor(github): divide making scopes and making plans (#3868)
---
 plugins/github/api/blueprint_V200_test.go |  87 ++++++-----
 plugins/github/api/blueprint_v200.go      | 231 +++++++++++++++---------------
 2 files changed, 173 insertions(+), 145 deletions(-)

diff --git a/plugins/github/api/blueprint_V200_test.go 
b/plugins/github/api/blueprint_V200_test.go
index f26618d3..c0472fed 100644
--- a/plugins/github/api/blueprint_V200_test.go
+++ b/plugins/github/api/blueprint_V200_test.go
@@ -69,43 +69,23 @@ func TestMakeDataSourcePipelinePlanV200(t *testing.T) {
        
mockMeta.On("RootPkgPath").Return("github.com/apache/incubator-devlake/plugins/github")
        err = core.RegisterPlugin("github", mockMeta)
        assert.Nil(t, err)
+       // Refresh Global Variables and set the sql mock
+       basicRes = NewMockBasicRes()
        bs := &core.BlueprintScopeV200{
                Entities: []string{"CODE", "TICKET"},
-               Id:       "",
+               Id:       "1",
                Name:     "",
        }
        bpScopes := make([]*core.BlueprintScopeV200, 0)
        bpScopes = append(bpScopes, bs)
 
        plan := make(core.PipelinePlan, len(bpScopes))
-       scopes := make([]core.Scope, 0, len(bpScopes))
-       for i, bpScope := range bpScopes {
-               githubRepo := &models.GithubRepo{
-                       ConnectionId: 1,
-                       GithubId:     12345,
-                       Name:         "testRepo",
-                       OwnerLogin:   "test",
-                       CreatedDate:  time.Time{},
-               }
-
-               transformationRule := &models.GithubTransformationRule{
-                       Model: common.Model{
-                               ID: 1,
-                       },
-                       Name:    "github transformation rule",
-                       PrType: "hey,man,wasup",
-                       Refdiff: map[string]interface{}{
-                               "tagsPattern": "pattern",
-                               "tagsLimit":   10,
-                               "tagsOrder":   "reverse semver",
-                       },
-               }
+       plan, err = makeDataSourcePipelinePlanV200(nil, plan, bpScopes, 
connection, mockApiCLient)
+       assert.Nil(t, err)
+       basicRes = NewMockBasicRes()
+       scopes, err := makeScopesV200(bpScopes, connection)
+       assert.Nil(t, err)
 
-               var scope []core.Scope
-               plan, scope, err = makeDataSourcePipelinePlanV200(nil, i, plan, 
bpScope, connection, mockApiCLient, githubRepo, transformationRule)
-               assert.Nil(t, err)
-               scopes = append(scopes, scope...)
-       }
        expectPlan := core.PipelinePlan{
                core.PipelineStage{
                        {
@@ -113,11 +93,12 @@ func TestMakeDataSourcePipelinePlanV200(t *testing.T) {
                                Subtasks:   []string{},
                                SkipOnFail: false,
                                Options: map[string]interface{}{
-                                       "connectionId": uint64(1),
-                                       "owner":        "test",
-                                       "repo":         "testRepo",
+                                       "connectionId":         uint64(1),
+                                       "transformationRuleId": uint64(1),
+                                       "owner":                "test",
+                                       "repo":                 "testRepo",
                                        "transformationRules": 
map[string]interface{}{
-                                               "name":    "github 
transformation rule",
+                                               "name":   "github 
transformation rule",
                                                "prType": "hey,man,wasup",
                                        },
                                },
@@ -167,3 +148,45 @@ func TestMakeDataSourcePipelinePlanV200(t *testing.T) {
        expectScopes = append(expectScopes, scopeRepo, scopeTicket)
        assert.Equal(t, expectScopes, scopes)
 }
+
+// NewMockBasicRes FIXME ...
+func NewMockBasicRes() *mocks.BasicRes {
+       testGithubRepo := &models.GithubRepo{
+               ConnectionId:         1,
+               GithubId:             12345,
+               Name:                 "testRepo",
+               OwnerLogin:           "test",
+               TransformationRuleId: 1,
+               CreatedDate:          time.Time{},
+       }
+
+       testTransformationRule := &models.GithubTransformationRule{
+               Model: common.Model{
+                       ID: 1,
+               },
+               Name:   "github transformation rule",
+               PrType: "hey,man,wasup",
+               Refdiff: map[string]interface{}{
+                       "tagsPattern": "pattern",
+                       "tagsLimit":   10,
+                       "tagsOrder":   "reverse semver",
+               },
+       }
+       mockRes := new(mocks.BasicRes)
+       mockDal := new(mocks.Dal)
+
+       mockDal.On("First", mock.Anything, mock.Anything).Run(func(args 
mock.Arguments) {
+               dst := args.Get(0).(*models.GithubRepo)
+               *dst = *testGithubRepo
+       }).Return(nil).Once()
+
+       mockDal.On("First", mock.Anything, mock.Anything).Run(func(args 
mock.Arguments) {
+               dst := args.Get(0).(*models.GithubTransformationRule)
+               *dst = *testTransformationRule
+       }).Return(nil).Once()
+
+       mockRes.On("GetDal").Return(mockDal)
+       mockRes.On("GetConfig", mock.Anything).Return("")
+
+       return mockRes
+}
diff --git a/plugins/github/api/blueprint_v200.go 
b/plugins/github/api/blueprint_v200.go
index f5781f5a..cf277fd9 100644
--- a/plugins/github/api/blueprint_v200.go
+++ b/plugins/github/api/blueprint_v200.go
@@ -42,7 +42,6 @@ import (
 )
 
 func MakeDataSourcePipelinePlanV200(subtaskMetas []core.SubTaskMeta, 
connectionId uint64, bpScopes []*core.BlueprintScopeV200) (core.PipelinePlan, 
[]core.Scope, errors.Error) {
-       db := basicRes.GetDal()
        connectionHelper := helper.NewConnectionHelper(basicRes, 
validator.New())
        // get the connection info for url
        connection := &models.GithubConnection{}
@@ -66,31 +65,14 @@ func MakeDataSourcePipelinePlanV200(subtaskMetas 
[]core.SubTaskMeta, connectionI
                return nil, nil, err
        }
 
-       plan := make(core.PipelinePlan, 0, len(bpScopes))
-       scopes := make([]core.Scope, 0, len(bpScopes))
-       for i, bpScope := range bpScopes {
-               var githubRepo *models.GithubRepo
-               // get repo from db
-               err = db.First(githubRepo, dal.Where(`id = ?`, bpScope.Id))
-               if err != nil {
-                       return nil, nil, err
-               }
-               var transformationRule *models.GithubTransformationRule
-               // get transformation rules from db
-               err = db.First(transformationRule, dal.Where(`id = ?`, 
githubRepo.TransformationRuleId))
-               if err != nil && goerror.Is(err, gorm.ErrRecordNotFound) {
-                       return nil, nil, err
-               }
-               var scope []core.Scope
-               // make pipeline for each bpScope
-               plan, scope, err = makeDataSourcePipelinePlanV200(subtaskMetas, 
i, plan, bpScope, connection, apiClient, githubRepo, transformationRule)
-               if err != nil {
-                       return nil, nil, err
-               }
-               if len(scope) > 0 {
-                       scopes = append(scopes, scope...)
-               }
-
+       plan := make(core.PipelinePlan, len(bpScopes))
+       plan, err = makeDataSourcePipelinePlanV200(subtaskMetas, plan, 
bpScopes, connection, apiClient)
+       if err != nil {
+               return nil, nil, err
+       }
+       scopes, err := makeScopesV200(bpScopes, connection)
+       if err != nil {
+               return nil, nil, err
        }
 
        return plan, scopes, nil
@@ -98,112 +80,135 @@ func MakeDataSourcePipelinePlanV200(subtaskMetas 
[]core.SubTaskMeta, connectionI
 
 func makeDataSourcePipelinePlanV200(
        subtaskMetas []core.SubTaskMeta,
-       i int,
        plan core.PipelinePlan,
-       bpScope *core.BlueprintScopeV200,
+       bpScopes []*core.BlueprintScopeV200,
        connection *models.GithubConnection,
        apiClient helper.ApiClientGetter,
-       githubRepo *models.GithubRepo,
-       transformationRule *models.GithubTransformationRule,
-) (core.PipelinePlan, []core.Scope, errors.Error) {
+) (core.PipelinePlan, errors.Error) {
        var err errors.Error
        var stage core.PipelineStage
        var repo *tasks.GithubApiRepo
-       scopes := make([]core.Scope, 0)
-       // refdiff
-       if transformationRule != nil && transformationRule.Refdiff != nil {
-               // add a new task to next stage
-               j := i + 1
-               if j == len(plan) {
-                       plan = append(plan, nil)
+       for i, bpScope := range bpScopes {
+               githubRepo := &models.GithubRepo{}
+               // get repo from db
+               err = basicRes.GetDal().First(githubRepo, 
dal.Where(`connection_id = ? AND github_id = ?`, connection.ID, bpScope.Id))
+               if err != nil && goerror.Is(err, gorm.ErrRecordNotFound) {
+                       return nil, errors.Default.Wrap(err, fmt.Sprintf("fail 
to find repo%s", bpScope.Id))
                }
-               refdiffOp := transformationRule.Refdiff
-               if err != nil {
-                       return nil, nil, err
+               transformationRule := &models.GithubTransformationRule{}
+               // get transformation rules from db
+               err = basicRes.GetDal().First(transformationRule, dal.Where(`id 
= ?`, githubRepo.TransformationRuleId))
+               if err != nil && !goerror.Is(err, gorm.ErrRecordNotFound) {
+                       return nil, err
                }
-               plan[j] = core.PipelineStage{
-                       {
-                               Plugin:  "refdiff",
-                               Options: refdiffOp,
-                       },
+               // refdiff
+               if transformationRule != nil && transformationRule.Refdiff != 
nil {
+                       // add a new task to next stage
+                       j := i + 1
+                       if j == len(plan) {
+                               plan = append(plan, nil)
+                       }
+                       refdiffOp := transformationRule.Refdiff
+                       if err != nil {
+                               return nil, err
+                       }
+                       plan[j] = core.PipelineStage{
+                               {
+                                       Plugin:  "refdiff",
+                                       Options: refdiffOp,
+                               },
+                       }
+                       transformationRule.Refdiff = nil
                }
-               transformationRule.Refdiff = nil
-       }
 
-       // construct task options for github
-       op := &tasks.GithubOptions{
-               ConnectionId:         githubRepo.ConnectionId,
-               TransformationRuleId: githubRepo.TransformationRuleId,
-               Owner:                githubRepo.OwnerLogin,
-               Repo:                 githubRepo.Name,
-       }
-       options, err := tasks.ValidateAndEncodeTaskOptions(op)
-       if err != nil {
-               return nil, nil, err
-       }
+               // construct task options for github
+               op := &tasks.GithubOptions{
+                       ConnectionId:         githubRepo.ConnectionId,
+                       TransformationRuleId: githubRepo.TransformationRuleId,
+                       Owner:                githubRepo.OwnerLogin,
+                       Repo:                 githubRepo.Name,
+               }
+               options, err := tasks.ValidateAndEncodeTaskOptions(op)
+               if err != nil {
+                       return nil, err
+               }
 
-       var transformationRuleMap map[string]interface{}
-       err = errors.Convert(mapstructure.Decode(transformationRule, 
&transformationRuleMap))
-       if err != nil {
-               return nil, nil, err
-       }
-       options["transformationRules"] = transformationRuleMap
-       stage, err = addGithub(subtaskMetas, connection, bpScope.Entities, 
stage, options)
-       if err != nil {
-               return nil, nil, err
-       }
-       // add gitex stage and add repo to scopes
-       if utils.StringsContains(bpScope.Entities, core.DOMAIN_TYPE_CODE) {
-               repo, err = memorizedGetApiRepo(repo, op, apiClient)
+               var transformationRuleMap map[string]interface{}
+               err = errors.Convert(mapstructure.Decode(transformationRule, 
&transformationRuleMap))
                if err != nil {
-                       return nil, nil, err
+                       return nil, err
                }
-               stage, err = addGitex(bpScope.Entities, connection, repo, stage)
+               options["transformationRules"] = transformationRuleMap
+               stage, err = addGithub(subtaskMetas, connection, 
bpScope.Entities, stage, options)
                if err != nil {
-                       return nil, nil, err
+                       return nil, err
+               }
+               // add gitex stage and add repo to scopes
+               if utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CODE) {
+                       repo, err = memorizedGetApiRepo(repo, op, apiClient)
+                       if err != nil {
+                               return nil, err
+                       }
+                       stage, err = addGitex(bpScope.Entities, connection, 
repo, stage)
+                       if err != nil {
+                               return nil, err
+                       }
                }
-               scopeRepo := &code.Repo{
-                       DomainEntity: domainlayer.DomainEntity{
-                               Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
-                       },
-                       Name: fmt.Sprintf("%s/%s", githubRepo.OwnerLogin, 
githubRepo.Name),
+               plan[i] = stage
+       }
+       return plan, nil
+}
+
+func makeScopesV200(bpScopes []*core.BlueprintScopeV200, connection 
*models.GithubConnection) ([]core.Scope, errors.Error) {
+       scopes := make([]core.Scope, 0)
+       for _, bpScope := range bpScopes {
+               githubRepo := &models.GithubRepo{}
+               // get repo from db
+               err := basicRes.GetDal().First(githubRepo, 
dal.Where(`connection_id = ? AND github_id = ?`, connection.ID, bpScope.Id))
+               if err != nil && goerror.Is(err, gorm.ErrRecordNotFound) {
+                       return nil, errors.Default.Wrap(err, fmt.Sprintf("fail 
to find repo%s", bpScope.Id))
                }
-               if repo.Parent != nil {
-                       scopeRepo.ForkedFrom = repo.Parent.HTMLUrl
+               transformationRule := &models.GithubTransformationRule{}
+               // get transformation rules from db
+               err = basicRes.GetDal().First(transformationRule, dal.Where(`id 
= ?`, githubRepo.TransformationRuleId))
+               if err != nil && !goerror.Is(err, gorm.ErrRecordNotFound) {
+                       return nil, err
                }
-               scopes = append(scopes, scopeRepo)
-       } else if utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CODE_REVIEW) {
-               // if we don't need to collect gitex, we need to add repo to 
scopes here
-               scopeRepo := &code.Repo{
-                       DomainEntity: domainlayer.DomainEntity{
-                               Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
-                       },
-                       Name: fmt.Sprintf("%s/%s", githubRepo.OwnerLogin, 
githubRepo.Name),
+               if utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CODE_REVIEW) ||
+                       utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CODE) ||
+                       utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CROSS) {
+                       // if we don't need to collect gitex, we need to add 
repo to scopes here
+                       scopeRepo := &code.Repo{
+                               DomainEntity: domainlayer.DomainEntity{
+                                       Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
+                               },
+                               Name: fmt.Sprintf("%s/%s", 
githubRepo.OwnerLogin, githubRepo.Name),
+                       }
+                       if githubRepo.ParentHTMLUrl != "" {
+                               scopeRepo.ForkedFrom = githubRepo.ParentHTMLUrl
+                       }
+                       scopes = append(scopes, scopeRepo)
                }
-               scopes = append(scopes, scopeRepo)
-       }
-       // add cicd_scope to scopes
-       if utils.StringsContains(bpScope.Entities, core.DOMAIN_TYPE_CICD) {
-               scopeCICD := &devops.CicdScope{
-                       DomainEntity: domainlayer.DomainEntity{
-                               Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
-                       },
-                       Name: fmt.Sprintf("%s/%s", githubRepo.OwnerLogin, 
githubRepo.Name),
+               // add cicd_scope to scopes
+               if utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_CICD) {
+                       scopeCICD := &devops.CicdScope{
+                               DomainEntity: domainlayer.DomainEntity{
+                                       Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
+                               },
+                               Name: fmt.Sprintf("%s/%s", 
githubRepo.OwnerLogin, githubRepo.Name),
+                       }
+                       scopes = append(scopes, scopeCICD)
                }
-               scopes = append(scopes, scopeCICD)
-       }
-       // add board to scopes
-       if utils.StringsContains(bpScope.Entities, core.DOMAIN_TYPE_TICKET) {
-               scopeTicket := &ticket.Board{
-                       DomainEntity: domainlayer.DomainEntity{
-                               Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
-                       },
-                       Name: fmt.Sprintf("%s/%s", githubRepo.OwnerLogin, 
githubRepo.Name),
+               // add board to scopes
+               if utils.StringsContains(bpScope.Entities, 
core.DOMAIN_TYPE_TICKET) {
+                       scopeTicket := &ticket.Board{
+                               DomainEntity: domainlayer.DomainEntity{
+                                       Id: 
didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, 
githubRepo.GithubId),
+                               },
+                               Name: fmt.Sprintf("%s/%s", 
githubRepo.OwnerLogin, githubRepo.Name),
+                       }
+                       scopes = append(scopes, scopeTicket)
                }
-               scopes = append(scopes, scopeTicket)
        }
-
-       plan[i] = stage
-
-       return plan, scopes, nil
+       return scopes, nil
 }

Reply via email to