From 257c6bca56b593994f4733991c8a9c8aad311c37 Mon Sep 17 00:00:00 2001 From: Harold Wanyama Date: Fri, 9 Aug 2024 13:04:16 +0300 Subject: [PATCH] Feature/Gerrit Post response update - Added repo list to gerrit instance POSTing Signed-off-by: Harold Wanyama --- cla-backend-go/api_client/api_client.go | 27 ++++ .../api_client/mocks/mock_client.go | 54 +++++++ cla-backend-go/gerrits/mocks/mock_service.go | 143 ++++++++++++++++++ cla-backend-go/gerrits/service.go | 74 +++++++-- cla-backend-go/gerrits/service_test.go | 123 ++++++++++++--- 5 files changed, 386 insertions(+), 35 deletions(-) create mode 100644 cla-backend-go/api_client/api_client.go create mode 100644 cla-backend-go/api_client/mocks/mock_client.go create mode 100644 cla-backend-go/gerrits/mocks/mock_service.go diff --git a/cla-backend-go/api_client/api_client.go b/cla-backend-go/api_client/api_client.go new file mode 100644 index 000000000..42c0d1999 --- /dev/null +++ b/cla-backend-go/api_client/api_client.go @@ -0,0 +1,27 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package apiclient + +import ( + "context" + "net/http" +) + +type APIClient interface { + GetData(ctx context.Context, url string) (*http.Response, error) +} + +type RestAPIClient struct { + Client *http.Client +} + +// GetData makes a get request to the specified url + +func (c *RestAPIClient) GetData(ctx context.Context, url string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + return c.Client.Do(req) +} diff --git a/cla-backend-go/api_client/mocks/mock_client.go b/cla-backend-go/api_client/mocks/mock_client.go new file mode 100644 index 000000000..08dd5ceba --- /dev/null +++ b/cla-backend-go/api_client/mocks/mock_client.go @@ -0,0 +1,54 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +// Code generated by MockGen. DO NOT EDIT. +// Source: api_client/api_client.go + +// Package mock_apiclient is a generated GoMock package. +package mock_apiclient + +import ( + context "context" + http "net/http" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockAPIClient is a mock of APIClient interface. +type MockAPIClient struct { + ctrl *gomock.Controller + recorder *MockAPIClientMockRecorder +} + +// MockAPIClientMockRecorder is the mock recorder for MockAPIClient. +type MockAPIClientMockRecorder struct { + mock *MockAPIClient +} + +// NewMockAPIClient creates a new mock instance. +func NewMockAPIClient(ctrl *gomock.Controller) *MockAPIClient { + mock := &MockAPIClient{ctrl: ctrl} + mock.recorder = &MockAPIClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAPIClient) EXPECT() *MockAPIClientMockRecorder { + return m.recorder +} + +// GetData mocks base method. +func (m *MockAPIClient) GetData(ctx context.Context, url string) (*http.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetData", ctx, url) + ret0, _ := ret[0].(*http.Response) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetData indicates an expected call of GetData. +func (mr *MockAPIClientMockRecorder) GetData(ctx, url interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetData", reflect.TypeOf((*MockAPIClient)(nil).GetData), ctx, url) +} diff --git a/cla-backend-go/gerrits/mocks/mock_service.go b/cla-backend-go/gerrits/mocks/mock_service.go new file mode 100644 index 000000000..64ba64d65 --- /dev/null +++ b/cla-backend-go/gerrits/mocks/mock_service.go @@ -0,0 +1,143 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +// Code generated by MockGen. DO NOT EDIT. +// Source: gerrits/service.go + +// Package mock_gerrits is a generated GoMock package. +package mock_gerrits + +import ( + context "context" + reflect "reflect" + + models "github.com/communitybridge/easycla/cla-backend-go/gen/v1/models" + gomock "github.com/golang/mock/gomock" +) + +// MockService is a mock of Service interface. +type MockService struct { + ctrl *gomock.Controller + recorder *MockServiceMockRecorder +} + +// MockServiceMockRecorder is the mock recorder for MockService. +type MockServiceMockRecorder struct { + mock *MockService +} + +// NewMockService creates a new mock instance. +func NewMockService(ctrl *gomock.Controller) *MockService { + mock := &MockService{ctrl: ctrl} + mock.recorder = &MockServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockService) EXPECT() *MockServiceMockRecorder { + return m.recorder +} + +// AddGerrit mocks base method. +func (m *MockService) AddGerrit(ctx context.Context, claGroupID, projectSFID string, input *models.AddGerritInput, claGroupModel *models.ClaGroup) (*models.Gerrit, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddGerrit", ctx, claGroupID, projectSFID, input, claGroupModel) + ret0, _ := ret[0].(*models.Gerrit) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AddGerrit indicates an expected call of AddGerrit. +func (mr *MockServiceMockRecorder) AddGerrit(ctx, claGroupID, projectSFID, input, claGroupModel interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddGerrit", reflect.TypeOf((*MockService)(nil).AddGerrit), ctx, claGroupID, projectSFID, input, claGroupModel) +} + +// DeleteClaGroupGerrits mocks base method. +func (m *MockService) DeleteClaGroupGerrits(ctx context.Context, claGroupID string) (int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteClaGroupGerrits", ctx, claGroupID) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteClaGroupGerrits indicates an expected call of DeleteClaGroupGerrits. +func (mr *MockServiceMockRecorder) DeleteClaGroupGerrits(ctx, claGroupID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteClaGroupGerrits", reflect.TypeOf((*MockService)(nil).DeleteClaGroupGerrits), ctx, claGroupID) +} + +// DeleteGerrit mocks base method. +func (m *MockService) DeleteGerrit(ctx context.Context, gerritID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteGerrit", ctx, gerritID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteGerrit indicates an expected call of DeleteGerrit. +func (mr *MockServiceMockRecorder) DeleteGerrit(ctx, gerritID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteGerrit", reflect.TypeOf((*MockService)(nil).DeleteGerrit), ctx, gerritID) +} + +// GetClaGroupGerrits mocks base method. +func (m *MockService) GetClaGroupGerrits(ctx context.Context, claGroupID string) (*models.GerritList, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetClaGroupGerrits", ctx, claGroupID) + ret0, _ := ret[0].(*models.GerritList) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetClaGroupGerrits indicates an expected call of GetClaGroupGerrits. +func (mr *MockServiceMockRecorder) GetClaGroupGerrits(ctx, claGroupID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetClaGroupGerrits", reflect.TypeOf((*MockService)(nil).GetClaGroupGerrits), ctx, claGroupID) +} + +// GetGerrit mocks base method. +func (m *MockService) GetGerrit(ctx context.Context, gerritID string) (*models.Gerrit, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGerrit", ctx, gerritID) + ret0, _ := ret[0].(*models.Gerrit) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGerrit indicates an expected call of GetGerrit. +func (mr *MockServiceMockRecorder) GetGerrit(ctx, gerritID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGerrit", reflect.TypeOf((*MockService)(nil).GetGerrit), ctx, gerritID) +} + +// GetGerritRepos mocks base method. +func (m *MockService) GetGerritRepos(ctx context.Context, gerritName string) (*models.GerritRepoList, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGerritRepos", ctx, gerritName) + ret0, _ := ret[0].(*models.GerritRepoList) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGerritRepos indicates an expected call of GetGerritRepos. +func (mr *MockServiceMockRecorder) GetGerritRepos(ctx, gerritName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGerritRepos", reflect.TypeOf((*MockService)(nil).GetGerritRepos), ctx, gerritName) +} + +// GetGerritsByProjectSFID mocks base method. +func (m *MockService) GetGerritsByProjectSFID(ctx context.Context, projectSFID string) (*models.GerritList, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGerritsByProjectSFID", ctx, projectSFID) + ret0, _ := ret[0].(*models.GerritList) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGerritsByProjectSFID indicates an expected call of GetGerritsByProjectSFID. +func (mr *MockServiceMockRecorder) GetGerritsByProjectSFID(ctx, projectSFID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGerritsByProjectSFID", reflect.TypeOf((*MockService)(nil).GetGerritsByProjectSFID), ctx, projectSFID) +} diff --git a/cla-backend-go/gerrits/service.go b/cla-backend-go/gerrits/service.go index dd1249515..01d0414a4 100644 --- a/cla-backend-go/gerrits/service.go +++ b/cla-backend-go/gerrits/service.go @@ -8,14 +8,19 @@ import ( "encoding/json" "errors" "fmt" + "io" + "net/http" "net/url" "strings" + "time" // "github.com/LF-Engineering/lfx-kit/auth" "github.com/go-openapi/strfmt" - "github.com/go-resty/resty/v2" + + // "github.com/go-resty/resty/v2" + apiclient "github.com/communitybridge/easycla/cla-backend-go/api_client" "github.com/sirupsen/logrus" "github.com/communitybridge/easycla/cla-backend-go/utils" @@ -84,6 +89,27 @@ func (s service) AddGerrit(ctx context.Context, claGroupID string, projectSFID s ProjectSFID: projectSFID, Version: params.Version, } + + // Get the gerrit repos + log.WithFields(f).Debugf("fetching gerrit repos for gerrit instance: %s", *params.GerritURL) + gerritHost, err := extractGerritHost(*params.GerritURL, f) + if err != nil { + return nil, err + } + gerritRepoList, getRepoErr := s.GetGerritRepos(ctx, gerritHost) + if getRepoErr != nil { + log.WithFields(f).WithError(getRepoErr).Warnf("problem fetching gerrit repos, error: %+v", getRepoErr) + return nil, getRepoErr + } + + log.WithFields(f).Debugf("discovered %d gerrit repos", len(gerritRepoList.Repos)) + log.WithFields(f).Debugf("gerrit repo list %+v", gerritRepoList) + // Set the connected flag - for now, we just set this value to true + for _, repo := range gerritRepoList.Repos { + repo.Connected = true + } + input.GerritRepoList = gerritRepoList + log.WithFields(f).Debugf("gerrit input %+v", input) return s.repo.AddGerrit(ctx, input) } @@ -216,6 +242,7 @@ func convertModel(responseModel map[string]GerritRepoInfo, serverInfo *ServerInf URL: strfmt.URI(weblink.URL), }) } + log.Debugf("Processing repo: %s, weblinks: %+v", name, weblinks) claEnabled := false if serverInfo != nil && serverInfo.Auth.UseContributorAgreements { @@ -260,7 +287,11 @@ func listGerritRepos(ctx context.Context, gerritHost string) (map[string]GerritR utils.XREQUESTID: ctx.Value(utils.XREQUESTID), "gerritHost": gerritHost, } - client := resty.New() + client := &apiclient.RestAPIClient{ + Client: &http.Client{ + Timeout: 10 * time.Second, + }, + } base := "https://" + gerritHost @@ -269,28 +300,40 @@ func listGerritRepos(ctx context.Context, gerritHost string) (map[string]GerritR return nil, gerritAPIPathErr } + log.WithFields(f).Debugf("gerrit API path using client: %s", gerritAPIPath) + if gerritAPIPath != "" { base = fmt.Sprintf("https://%s/%s", gerritHost, gerritAPIPath) } - resp, err := client.R(). - EnableTrace(). - Get(fmt.Sprintf("%s/projects/?d&pp=0", base)) + url := fmt.Sprintf("%s/projects/?d&pp=0", base) + resp, err := client.GetData(ctx, url) + if err != nil { - log.WithFields(f).Warnf("problem querying gerrit host: %s, error: %+v", gerritHost, err) return nil, err } - if resp.IsError() { - msg := fmt.Sprintf("non-success response from list gerrit host repos for gerrit %s, error code: %s", gerritHost, resp.Status()) - log.WithFields(f).Warn(msg) - return nil, errors.New(msg) + defer func() { + if err = resp.Body.Close(); err != nil { + log.WithFields(f).Debugf("Failed to close response body; %v", err) + } + }() + + log.WithFields(f).Debugf("response: %+v", resp.Body) + + // Get the response body + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err } - var result map[string]GerritRepoInfo // Need to strip off the leading "magic prefix line" from the response payload, which is: )]}' // See: https://gerrit.linuxfoundation.org/infra/Documentation/rest-api.html#output - err = json.Unmarshal(resp.Body()[4:], &result) + strippedBody := stripMagicPrefix(body) + + var result map[string]GerritRepoInfo + + err = json.Unmarshal(strippedBody, &result) if err != nil { log.WithFields(f).Warnf("problem unmarshalling response for gerrit host: %s, error: %+v", gerritHost, err) return nil, err @@ -299,6 +342,13 @@ func listGerritRepos(ctx context.Context, gerritHost string) (map[string]GerritR return result, nil } +func stripMagicPrefix(data []byte) []byte { + if len(data) > 4 { + return data[4:] + } + return data +} + // getGerritConfig returns the gerrit configuration for the specified host func getGerritConfig(ctx context.Context, gerritHost string) (*ServerInfo, error) { f := logrus.Fields{ diff --git a/cla-backend-go/gerrits/service_test.go b/cla-backend-go/gerrits/service_test.go index 457361f6a..5e9abdac4 100644 --- a/cla-backend-go/gerrits/service_test.go +++ b/cla-backend-go/gerrits/service_test.go @@ -4,44 +4,121 @@ package gerrits import ( + // "bytes" "context" + // "io" + // "net/http" "testing" + // mock_apiclient "github.com/communitybridge/easycla/cla-backend-go/api_client/mocks" "github.com/communitybridge/easycla/cla-backend-go/gen/v1/models" gerritsMock "github.com/communitybridge/easycla/cla-backend-go/gerrits/mocks" + "github.com/go-openapi/strfmt" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) func TestService_AddGerrit(t *testing.T) { - // AddGerrit test case - gerritName := "ONAP" - gerritURL := "https://gerrit.onap.org" + gerritName := "gerritName" + gerritURL := "https://mockapi.gerrit.dev.itx.linuxfoundation.org/" + // gerritHost := "mockapi.gerrit.dev.itx.linuxfoundation.org" + repos := []*models.GerritRepo{ + { + ClaEnabled: true, + Connected: true, + ContributorAgreements: []*models.GerritRepoContributorAgreementsItems0{ + { + Description: "CCLA (Corporate Contributor License Agreement) for SUN", + Name: "CCLA", + URL: "https://api.dev.lfcla.com/v2/gerrit/01af041c-fa69-4052-a23c-fb8c1d3bef24/corporate/agreementUrl.html", + }, + { + Description: "ICLA (Individual Contributor License Agreement) for SUN", + Name: "ICLA", + URL: "https://api.dev.lfcla.com/v2/gerrit/01af041c-fa69-4052-a23c-fb8c1d3bef24/individual/agreementUrl.html", + }, + }, + Description: "Access inherited by all other projects.", + ID: "All-Projects", + Name: "All-Projects", + State: "ACTIVE", + WebLinks: []*models.GerritRepoWebLinksItems0{ + { + Name: "browse", + URL: "/plugins/gitiles/All-Projects", + }, + }, + }, + } - ctrl := gomock.NewController(t) - defer ctrl.Finish() + testCases := []struct { + name string + claGroupID string + projectSFID string + params *models.AddGerritInput + gerritRepoList *models.GerritRepoList + ReposExist []*models.Gerrit + repoListErr error + claGroupModel *models.ClaGroup + expectedResult *models.Gerrit + expectedError error + }{ + { + name: "Valid input", + claGroupID: "claGroupID", + projectSFID: "projectSFID", + params: &models.AddGerritInput{ + GerritName: &gerritName, + GerritURL: &gerritURL, + Version: "version", + }, + ReposExist: []*models.Gerrit{}, + gerritRepoList: &models.GerritRepoList{ + Repos: repos, + }, + repoListErr: nil, + claGroupModel: &models.ClaGroup{}, + expectedResult: &models.Gerrit{ + GerritName: gerritName, + GerritURL: strfmt.URI(gerritURL), + ProjectID: "claGroupID", + ProjectSFID: "projectSFID", + Version: "version", + GerritRepoList: &models.GerritRepoList{ + Repos: repos, + }, + }, + expectedError: nil, + }, + } - mockRepo := gerritsMock.NewMockRepository(ctrl) - mockRepo.EXPECT().AddGerrit(gomock.Any(), gomock.Any()).Return(&models.Gerrit{ - GerritID: "e82c469a-55ea-492d-9722-fd30b31da2aa", - GerritName: "ONAP", - GerritURL: "https://gerrit.onap.org", - ProjectID: "projectID", - }, nil) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockRepo := gerritsMock.NewMockRepository(ctrl) + mockRepo.EXPECT().ExistsByName(context.Background(), gerritName).Return(tc.ReposExist, nil) + gerrit := &models.Gerrit{ + GerritName: gerritName, + GerritURL: strfmt.URI(gerritURL), + ProjectID: "claGroupID", + ProjectSFID: "projectSFID", + Version: "version", + GerritRepoList: tc.gerritRepoList, + } - //Gerrit repo by name does not exist - mockRepo.EXPECT().ExistsByName(context.TODO(), "ONAP").Return(nil, nil) + mockRepo.EXPECT().AddGerrit(gomock.Any(), gomock.Any()).Return(gerrit, nil) - service := NewService(mockRepo) - gerrit, err := service.AddGerrit(context.TODO(), "projectID", "projectSFID", &models.AddGerritInput{ - GerritName: &gerritName, - GerritURL: &gerritURL, - }, &models.ClaGroup{ - ProjectID: "projectID", - }) + service := NewService(mockRepo) - assert.NotNil(t, gerrit) - assert.NoError(t, err) + result, err := service.AddGerrit(context.Background(), tc.claGroupID, tc.projectSFID, tc.params, tc.claGroupModel) + if err != nil { + t.Fatalf("Add Gerrit returned an error: %v", err) + } + assert.Equal(t, tc.expectedResult, result) + assert.Equal(t, repos, result.GerritRepoList.Repos) + }) + } }