likyh commented on code in PR #4359: URL: https://github.com/apache/incubator-devlake/pull/4359#discussion_r1102774217
########## backend/plugins/gitlab/tasks/account_detail_collector.go: ########## @@ -0,0 +1,117 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "net/url" + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/helpers/pluginhelper/api" + "github.com/apache/incubator-devlake/plugins/gitlab/models" + "golang.org/x/mod/semver" +) + +const RAW_USER_DETAIL_TABLE = "gitlab_api_user_details" + +var CollectAccountDetailsMeta = plugin.SubTaskMeta{ + Name: "collectAccountDetails", + EntryPoint: CollectAccountDetails, + EnabledByDefault: true, + Description: "collect gitlab user details", + DomainTypes: []string{plugin.DOMAIN_TYPE_CROSS}, +} + +func CollectAccountDetails(taskCtx plugin.SubTaskContext) errors.Error { + + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_USER_DETAIL_TABLE) + logger := taskCtx.GetLogger() + logger.Info("collect gitlab user details") Review Comment: delete debug code ########## backend/plugins/gitlab/tasks/mr_detail_collector.go: ########## @@ -0,0 +1,89 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" +) + +const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details" + +var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{ + Name: "collectApiMergeRequestDetails", + EntryPoint: CollectApiMergeRequestDetails, + EnabledByDefault: true, + Description: "Collect merge request Details data from gitlab api", + DomainTypes: []string{plugin.DOMAIN_TYPE_CODE_REVIEW}, +} + +func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error { + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE) + collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter) + if err != nil { + return err + } + + iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState) + if err != nil { + return err + } + + err = collectorWithState.InitCollector(helper.ApiCollectorArgs{ + ApiClient: data.ApiClient, + PageSize: 100, + Incremental: false, + Input: iterator, + UrlTemplate: "projects/{{ .Params.ProjectId }}/merge_requests/{{ .Input.Iid }}", + Query: GetQuery, Review Comment: why this collector can run with pagination? ########## backend/plugins/gitlab/tasks/mr_extractor.go: ########## @@ -155,7 +170,12 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error { return errors.Convert(err) } - return extractor.Execute() + err = extractor.Execute() Review Comment: why expand this err? ########## backend/plugins/gitlab/tasks/mr_extractor.go: ########## @@ -107,6 +113,15 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error { if err != nil { return nil, err } + + // if we can not find merged_at and closed_at info in the detail + // we need get detail for gitlab v11 + if !strings.Contains(s, "\"merged_at\":") { + if !strings.Contains(s, "\"closed_at\":") { + gitlabMergeRequest.IsDetailRequired = true + } + } Review Comment: I think this solution is a little confusing. Because all MRs have the same IsDetailRequired. ########## backend/plugins/gitlab/models/connection.go: ########## @@ -18,19 +18,96 @@ limitations under the License. package models import ( - helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" + "fmt" + "net/http" + + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/helpers/pluginhelper/api" + "github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract" ) // GitlabConn holds the essential information to connect to the Gitlab API type GitlabConn struct { - helper.RestConnection `mapstructure:",squash"` - helper.AccessToken `mapstructure:",squash"` + api.RestConnection `mapstructure:",squash"` + api.AccessToken `mapstructure:",squash"` +} + +const GitlabApiClientData_UserId string = "UserId" +const GitlabApiClientData_ApiVersion string = "ApiVersion" + +// this function is used to rewrite the same function of AccessToken +func (conn *GitlabConn) SetupAuthentication(request *http.Request) errors.Error { + return nil +} + +// PrepareApiClient test api and set the IsPrivateToken,version,UserId and so on. +func (conn *GitlabConn) PrepareApiClient(apiClient apihelperabstract.ApiClientAbstract) errors.Error { + header1 := http.Header{} + header1.Set("Authorization", fmt.Sprintf("Bearer %v", conn.Token)) + // test request for access token + userResBody := &ApiUserResponse{} + res, err := apiClient.Get("user", nil, header1) + if res.StatusCode != http.StatusUnauthorized { + if err != nil { + return errors.Convert(err) + } + err = api.UnmarshalResponse(res, userResBody) + if err != nil { + return errors.Convert(err) + } + + if res.StatusCode != http.StatusOK { + return errors.HttpStatus(res.StatusCode).New("unexpected status code while testing connection") + } + apiClient.SetHeaders(map[string]string{ + "Authorization": fmt.Sprintf("Bearer %v", conn.Token), + }) + } else { + header2 := http.Header{} + header2.Set("Private-Token", conn.Token) + res, err = apiClient.Get("user", nil, header2) + if err != nil { + return errors.Convert(err) + } + err = api.UnmarshalResponse(res, userResBody) + if err != nil { + return errors.Convert(err) + } + + if res.StatusCode != http.StatusOK { + return errors.HttpStatus(res.StatusCode).New("unexpected status code while testing connection[PrivateToken]") + } + apiClient.SetHeaders(map[string]string{ + "Private-Token": conn.Token, + }) + } + // get gitlab version + versionResBody := &ApiVersionResponse{} + res, err = apiClient.Get("version", nil, nil) + if err != nil { + return errors.Convert(err) + } + + err = api.UnmarshalResponse(res, versionResBody) + if err != nil { + return errors.Convert(err) + } + + // add v for semver compare + if versionResBody.Version[0] != 'v' { + versionResBody.Version = "v" + versionResBody.Version + } + + apiClient.SetData(GitlabApiClientData_UserId, userResBody.Id) Review Comment: Maybe we can set these data into GitlabTaskData? Because it's not apiClient's data but plugin's data. ########## backend/plugins/gitlab/tasks/account_detail_collector.go: ########## @@ -0,0 +1,117 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "net/url" + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/helpers/pluginhelper/api" + "github.com/apache/incubator-devlake/plugins/gitlab/models" + "golang.org/x/mod/semver" +) + +const RAW_USER_DETAIL_TABLE = "gitlab_api_user_details" + +var CollectAccountDetailsMeta = plugin.SubTaskMeta{ + Name: "collectAccountDetails", + EntryPoint: CollectAccountDetails, + EnabledByDefault: true, + Description: "collect gitlab user details", + DomainTypes: []string{plugin.DOMAIN_TYPE_CROSS}, +} + +func CollectAccountDetails(taskCtx plugin.SubTaskContext) errors.Error { + + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_USER_DETAIL_TABLE) + logger := taskCtx.GetLogger() + logger.Info("collect gitlab user details") + + if !NeedAccountDetails(data.ApiClient) { + logger.Info("Don't need collect gitlab user details,skip") + return nil + } + + iterator, err := GetAccountsIterator(taskCtx) + if err != nil { + return err + } + defer iterator.Close() + + collector, err := api.NewApiCollector(api.ApiCollectorArgs{ + RawDataSubTaskArgs: *rawDataSubTaskArgs, + ApiClient: data.ApiClient, + UrlTemplate: "/projects/{{ .Params.ProjectId }}/members/{{ .Input.GitlabId }}", + Input: iterator, + //PageSize: 100, + Query: func(reqData *api.RequestData) (url.Values, errors.Error) { + query := url.Values{} + // query.Set("sort", "asc") Review Comment: delete unused code ########## backend/plugins/gitlab/tasks/account_extractor.go: ########## @@ -65,5 +67,10 @@ func ExtractAccounts(taskCtx plugin.SubTaskContext) errors.Error { return err } - return extractor.Execute() + err = extractor.Execute() Review Comment: why do you expand this? ########## backend/plugins/gitlab/tasks/mr_detail_collector.go: ########## @@ -0,0 +1,89 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" +) + +const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details" + +var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{ + Name: "collectApiMergeRequestDetails", + EntryPoint: CollectApiMergeRequestDetails, + EnabledByDefault: true, + Description: "Collect merge request Details data from gitlab api", + DomainTypes: []string{plugin.DOMAIN_TYPE_CODE_REVIEW}, +} + +func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error { + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE) + collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter) + if err != nil { + return err + } + + iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState) + if err != nil { + return err + } + + err = collectorWithState.InitCollector(helper.ApiCollectorArgs{ + ApiClient: data.ApiClient, + PageSize: 100, + Incremental: false, + Input: iterator, + UrlTemplate: "projects/{{ .Params.ProjectId }}/merge_requests/{{ .Input.Iid }}", + Query: GetQuery, + GetTotalPages: GetTotalPagesFromResponse, Review Comment: delete page size if not used ########## backend/plugins/gitlab/tasks/mr_detail_collector.go: ########## @@ -0,0 +1,89 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" +) + +const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details" + +var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{ + Name: "collectApiMergeRequestDetails", + EntryPoint: CollectApiMergeRequestDetails, + EnabledByDefault: true, + Description: "Collect merge request Details data from gitlab api", + DomainTypes: []string{plugin.DOMAIN_TYPE_CODE_REVIEW}, +} + +func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error { + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE) + collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter) + if err != nil { + return err + } + + iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState) + if err != nil { + return err + } + + err = collectorWithState.InitCollector(helper.ApiCollectorArgs{ + ApiClient: data.ApiClient, + PageSize: 100, Review Comment: delete page size if not used ########## backend/plugins/gitlab/tasks/mr_detail_extractor.go: ########## @@ -0,0 +1,121 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "encoding/json" + "regexp" + + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + "github.com/apache/incubator-devlake/helpers/pluginhelper/api" + "github.com/apache/incubator-devlake/plugins/gitlab/models" +) + +var ExtractApiMergeRequestDetailsMeta = plugin.SubTaskMeta{ + Name: "extractApiMergeRequestDetails", + EntryPoint: ExtractApiMergeRequestDetails, + EnabledByDefault: true, + Description: "Extract raw merge request Details data into tool layer table GitlabMergeRequest and GitlabReviewer", + DomainTypes: []string{plugin.DOMAIN_TYPE_CODE_REVIEW}, +} + +func ExtractApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error { + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE) + config := data.Options.GitlabTransformationRule + var labelTypeRegex *regexp.Regexp + var labelComponentRegex *regexp.Regexp + var prType = config.PrType + var err error + if len(prType) > 0 { + labelTypeRegex, err = regexp.Compile(prType) + if err != nil { + return errors.Default.Wrap(err, "regexp Compile prType failed") + } + } + var prComponent = config.PrComponent + if len(prComponent) > 0 { + labelComponentRegex, err = regexp.Compile(prComponent) + if err != nil { + return errors.Default.Wrap(err, "regexp Compile prComponent failed") + } + } + extractor, err := api.NewApiExtractor(api.ApiExtractorArgs{ + RawDataSubTaskArgs: *rawDataSubTaskArgs, + Extract: func(row *api.RawData) ([]interface{}, errors.Error) { + mr := &MergeRequestRes{} + err := errors.Convert(json.Unmarshal(row.Data, mr)) + if err != nil { + return nil, err + } + + gitlabMergeRequest, err := convertMergeRequest(mr) + if err != nil { + return nil, err + } + results := make([]interface{}, 0, len(mr.Reviewers)+1) + gitlabMergeRequest.ConnectionId = data.Options.ConnectionId + gitlabMergeRequest.IsDetailRequired = true Review Comment: why set `IsDetailRequired` here? I think it only should set on mr extracter. ########## backend/plugins/gitlab/tasks/pipeline_detail_collector.go: ########## @@ -0,0 +1,98 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "net/url" + "reflect" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/core/plugin" + helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" +) + +const RAW_PIPELINE_DETAILS_TABLE = "gitlab_api_pipeline_details" + +var CollectApiPipelineDetailsMeta = plugin.SubTaskMeta{ + Name: "collectApiPipelineDetails", + EntryPoint: CollectApiPipelineDetails, + EnabledByDefault: true, + Description: "Collect pipeline details data from gitlab api", + DomainTypes: []string{plugin.DOMAIN_TYPE_CICD}, +} + +func CollectApiPipelineDetails(taskCtx plugin.SubTaskContext) errors.Error { + rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_PIPELINE_DETAILS_TABLE) + collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter) + if err != nil { + return err + } + + incremental := collectorWithState.IsIncremental() Review Comment: why do you add incremental but query all data without filter by latestUpdated. ########## backend/plugins/gitlab/tasks/pipeline_extractor.go: ########## @@ -112,5 +116,10 @@ func ExtractApiPipelines(taskCtx plugin.SubTaskContext) errors.Error { return err } - return extractor.Execute() + err = extractor.Execute() Review Comment: as above -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
