[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3403: Fix makepipelineplan

[GitBox <gi...@apache.org>]

warren830 commented on code in PR #3403:
URL: https://github.com/apache/incubator-devlake/pull/3403#discussion_r994095371


##########
plugins/github/api/blueprint.go:
##########
@@ -38,179 +38,178 @@ import (
 
 func MakePipelinePlan(subtaskMetas []core.SubTaskMeta, connectionId uint64, scope []*core.BlueprintScopeV100) (core.PipelinePlan, errors.Error) {
 	var err errors.Error
-	plan := make(core.PipelinePlan, len(scope))
-	for i, scopeElem := range scope {
-		plan, err = processScope(subtaskMetas, connectionId, scopeElem, i, plan, nil, nil)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return plan, nil
-}
-func processScope(subtaskMetas []core.SubTaskMeta, connectionId uint64, scopeElem *core.BlueprintScopeV100, i int, plan core.PipelinePlan, apiRepo *tasks.GithubApiRepo, connection *models.GithubConnection) (core.PipelinePlan, errors.Error) {
-	var err errors.Error
-	// handle taskOptions and transformationRules, by dumping them to taskOptions
-	transformationRules := make(map[string]interface{})
-	if len(scopeElem.Transformation) > 0 {
-		err = errors.Convert(json.Unmarshal(scopeElem.Transformation, &transformationRules))
-		if err != nil {
-			return nil, err
-		}
-	}
-	// refdiff
-	if refdiffRules, ok := transformationRules["refdiff"]; ok && refdiffRules != nil {
-		// add a new task to next stage
-		j := i + 1
-		if j == len(plan) {
-			plan = append(plan, nil)
-		}
-		plan[j] = core.PipelineStage{
-			{
-				Plugin:  "refdiff",
-				Options: refdiffRules.(map[string]interface{}),
-			},
-		}
-		// remove it from github transformationRules
-		delete(transformationRules, "refdiff")
-	}
-	// construct task options for github
-	options := make(map[string]interface{})
-	err = errors.Convert(json.Unmarshal(scopeElem.Options, &options))
+	connection := new(models.GithubConnection)
+	err = connectionHelper.FirstById(connection, connectionId)
 	if err != nil {
 		return nil, err
 	}
-	options["connectionId"] = connectionId
-	options["transformationRules"] = transformationRules
-	// make sure task options is valid
-	op, err := tasks.DecodeAndValidateTaskOptions(options)
+	token := strings.Split(connection.Token, ",")[0]
+
+	apiClient, err := helper.NewApiClient(
+		context.TODO(),
+		connection.Endpoint,
+		map[string]string{
+			"Authorization": fmt.Sprintf("Bearer %s", token),
+		},
+		10*time.Second,
+		connection.Proxy,
+		basicRes,
+	)
 	if err != nil {
 		return nil, err
 	}
-	// construct subtasks
-	subtasks, err := helper.MakePipelinePlanSubtasks(subtaskMetas, scopeElem.Entities)
+	plan, err := makePipelinePlan(subtaskMetas, scope, apiClient, connection)
 	if err != nil {
 		return nil, err
 	}
-	stage := plan[i]
-	if stage == nil {
-		stage = core.PipelineStage{}
+	return plan, nil
+}
+
+func makePipelinePlan(subtaskMetas []core.SubTaskMeta, scope []*core.BlueprintScopeV100, apiClient helper.ApiClientGetter, connection *models.GithubConnection) (core.PipelinePlan, errors.Error) {
+	var err errors.Error
+	var repo *tasks.GithubApiRepo
+	getApiRepoIfNil := func(op *tasks.GithubOptions) (*tasks.GithubApiRepo, errors.Error) {
+		if repo == nil {
+			repo, err = getApiRepo(op, apiClient)
+		}
+		return repo, err
 	}
-	stage = append(stage, &core.PipelineTask{
-		Plugin:   "github",
-		Subtasks: subtasks,
-		Options:  options,
-	})
-	// collect git data by gitextractor if CODE was requested
-	if utils.StringsContains(scopeElem.Entities, core.DOMAIN_TYPE_CODE) {
-		// here is the tricky part, we have to obtain the repo id beforehand
-		if connection == nil {
-			connection = new(models.GithubConnection)
-			err = connectionHelper.FirstById(connection, connectionId)
+	plan := make(core.PipelinePlan, len(scope))
+	for i, scopeElem := range scope {
+		// handle taskOptions and transformationRules, by dumping them to taskOptions
+		transformationRules := make(map[string]interface{})
+		if len(scopeElem.Transformation) > 0 {
+			err = errors.Convert(json.Unmarshal(scopeElem.Transformation, &transformationRules))
 			if err != nil {
 				return nil, err
 			}
 		}
-		token := strings.Split(connection.Token, ",")[0]
-		if apiRepo == nil {
-			apiRepo = new(tasks.GithubApiRepo)
-			err = getApiRepo(connection, token, op, apiRepo)
-			if err != nil {
-				return nil, err
+		// refdiff
+		if refdiffRules, ok := transformationRules["refdiff"]; ok && refdiffRules != nil {
+			// add a new task to next stage
+			j := i + 1
+			if j == len(plan) {
+				plan = append(plan, nil)
+			}
+			plan[j] = core.PipelineStage{
+				{
+					Plugin:  "refdiff",
+					Options: refdiffRules.(map[string]interface{}),
+				},
 			}
+			// remove it from github transformationRules
+			delete(transformationRules, "refdiff")
 		}
-		cloneUrl, err := errors.Convert01(url.Parse(apiRepo.CloneUrl))
+		// construct task options for github
+		options := make(map[string]interface{})
+		err = errors.Convert(json.Unmarshal(scopeElem.Options, &options))
 		if err != nil {
 			return nil, err
 		}
-		cloneUrl.User = url.UserPassword("git", token)
-		stage = append(stage, &core.PipelineTask{
-			Plugin: "gitextractor",
-			Options: map[string]interface{}{
-				"url":    cloneUrl.String(),
-				"repoId": didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connectionId, apiRepo.GithubId),
-				"proxy":  connection.Proxy,
-			},
-		})
-	}
-	// dora
-	if productionPattern, ok := transformationRules["productionPattern"]; ok && productionPattern != nil {
-		j := i + 1
-		if j == len(plan) {
-			plan = append(plan, nil)
-		}
-		// add a new task to next stage
-		if plan[j] != nil {
-			j++
-		}
-		if j == len(plan) {
-			plan = append(plan, nil)
+		options["connectionId"] = connection.ID
+		options["transformationRules"] = transformationRules
+		// make sure task options is valid
+		op, err := tasks.DecodeAndValidateTaskOptions(options)
+		if err != nil {
+			return nil, err
 		}
+		// construct subtasks
+		subtasks, err := helper.MakePipelinePlanSubtasks(subtaskMetas, scopeElem.Entities)
 		if err != nil {
 			return nil, err
 		}
-		if apiRepo == nil {
-			if connection == nil {
-				connection = new(models.GithubConnection)
-				err = connectionHelper.FirstById(connection, connectionId)
-				if err != nil {
-					return nil, err
-				}
-			}
+		stage := plan[i]
+		if stage == nil {
+			stage = core.PipelineStage{}
+		}
+		stage = append(stage, &core.PipelineTask{
+			Plugin:   "github",
+			Subtasks: subtasks,
+			Options:  options,
+		})
+		// collect git data by gitextractor if CODE was requested
+		if utils.StringsContains(scopeElem.Entities, core.DOMAIN_TYPE_CODE) {
+			// here is the tricky part, we have to obtain the repo id beforehand
 			token := strings.Split(connection.Token, ",")[0]
-			apiRepo = new(tasks.GithubApiRepo)
-			err = getApiRepo(connection, token, op, apiRepo)
+			repo, err = getApiRepoIfNil(op)
+			if err != nil {
+				return nil, err
+			}
+			cloneUrl, err := errors.Convert01(url.Parse(repo.CloneUrl))
+			if err != nil {
+				return nil, err
+			}
+			cloneUrl.User = url.UserPassword("git", token)
+			stage = append(stage, &core.PipelineTask{
+				Plugin: "gitextractor",
+				Options: map[string]interface{}{
+					"url":    cloneUrl.String(),
+					"repoId": didgen.NewDomainIdGenerator(&models.GithubRepo{}).Generate(connection.ID, repo.GithubId),
+					"proxy":  connection.Proxy,
+				},
+			})
+		}
+		// dora
+		if productionPattern, ok := transformationRules["productionPattern"]; ok && productionPattern != nil {
+			j := i + 1
+			if j == len(plan) {
+				plan = append(plan, nil)
+			}
+			// add a new task to next stage
+			if plan[j] != nil {
+				j++
+			}
+			if j == len(plan) {
+				plan = append(plan, nil)
+			}
+			if err != nil {
+				return nil, err
+			}
+			repo, err = getApiRepoIfNil(op)
 			if err != nil {
 				return nil, err
 			}
+
+			doraOption := make(map[string]interface{})

Review Comment:
   fixed



##########
plugins/github/api/blueprint.go:
##########
@@ -38,179 +38,178 @@ import (
 
 func MakePipelinePlan(subtaskMetas []core.SubTaskMeta, connectionId uint64, scope []*core.BlueprintScopeV100) (core.PipelinePlan, errors.Error) {
 	var err errors.Error
-	plan := make(core.PipelinePlan, len(scope))
-	for i, scopeElem := range scope {
-		plan, err = processScope(subtaskMetas, connectionId, scopeElem, i, plan, nil, nil)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return plan, nil
-}
-func processScope(subtaskMetas []core.SubTaskMeta, connectionId uint64, scopeElem *core.BlueprintScopeV100, i int, plan core.PipelinePlan, apiRepo *tasks.GithubApiRepo, connection *models.GithubConnection) (core.PipelinePlan, errors.Error) {
-	var err errors.Error
-	// handle taskOptions and transformationRules, by dumping them to taskOptions
-	transformationRules := make(map[string]interface{})
-	if len(scopeElem.Transformation) > 0 {
-		err = errors.Convert(json.Unmarshal(scopeElem.Transformation, &transformationRules))
-		if err != nil {
-			return nil, err
-		}
-	}
-	// refdiff
-	if refdiffRules, ok := transformationRules["refdiff"]; ok && refdiffRules != nil {
-		// add a new task to next stage
-		j := i + 1
-		if j == len(plan) {
-			plan = append(plan, nil)
-		}
-		plan[j] = core.PipelineStage{
-			{
-				Plugin:  "refdiff",
-				Options: refdiffRules.(map[string]interface{}),
-			},
-		}
-		// remove it from github transformationRules
-		delete(transformationRules, "refdiff")
-	}
-	// construct task options for github
-	options := make(map[string]interface{})
-	err = errors.Convert(json.Unmarshal(scopeElem.Options, &options))
+	connection := new(models.GithubConnection)
+	err = connectionHelper.FirstById(connection, connectionId)
 	if err != nil {
 		return nil, err
 	}
-	options["connectionId"] = connectionId
-	options["transformationRules"] = transformationRules
-	// make sure task options is valid
-	op, err := tasks.DecodeAndValidateTaskOptions(options)
+	token := strings.Split(connection.Token, ",")[0]
+
+	apiClient, err := helper.NewApiClient(
+		context.TODO(),
+		connection.Endpoint,
+		map[string]string{
+			"Authorization": fmt.Sprintf("Bearer %s", token),
+		},
+		10*time.Second,
+		connection.Proxy,
+		basicRes,
+	)
 	if err != nil {
 		return nil, err
 	}
-	// construct subtasks
-	subtasks, err := helper.MakePipelinePlanSubtasks(subtaskMetas, scopeElem.Entities)
+	plan, err := makePipelinePlan(subtaskMetas, scope, apiClient, connection)
 	if err != nil {
 		return nil, err
 	}
-	stage := plan[i]
-	if stage == nil {
-		stage = core.PipelineStage{}
+	return plan, nil
+}
+
+func makePipelinePlan(subtaskMetas []core.SubTaskMeta, scope []*core.BlueprintScopeV100, apiClient helper.ApiClientGetter, connection *models.GithubConnection) (core.PipelinePlan, errors.Error) {
+	var err errors.Error
+	var repo *tasks.GithubApiRepo
+	getApiRepoIfNil := func(op *tasks.GithubOptions) (*tasks.GithubApiRepo, errors.Error) {

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org