From e1f605c1a461aeb1f8e4ca9395ace6c246e31ca7 Mon Sep 17 00:00:00 2001 From: Harold Wanyama Date: Tue, 13 Aug 2024 10:09:46 +0300 Subject: [PATCH] Revert "Feature/Gerrit Post response update" This reverts commit 257c6bca56b593994f4733991c8a9c8aad311c37. 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, 35 insertions(+), 386 deletions(-) delete mode 100644 cla-backend-go/api_client/api_client.go delete mode 100644 cla-backend-go/api_client/mocks/mock_client.go delete 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 deleted file mode 100644 index 42c0d1999..000000000 --- a/cla-backend-go/api_client/api_client.go +++ /dev/null @@ -1,27 +0,0 @@ -// 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 deleted file mode 100644 index 08dd5ceba..000000000 --- a/cla-backend-go/api_client/mocks/mock_client.go +++ /dev/null @@ -1,54 +0,0 @@ -// 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 deleted file mode 100644 index 64ba64d65..000000000 --- a/cla-backend-go/gerrits/mocks/mock_service.go +++ /dev/null @@ -1,143 +0,0 @@ -// 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 01d0414a4..dd1249515 100644 --- a/cla-backend-go/gerrits/service.go +++ b/cla-backend-go/gerrits/service.go @@ -8,19 +8,14 @@ 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/go-resty/resty/v2" "github.com/sirupsen/logrus" "github.com/communitybridge/easycla/cla-backend-go/utils" @@ -89,27 +84,6 @@ 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) } @@ -242,7 +216,6 @@ 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 { @@ -287,11 +260,7 @@ func listGerritRepos(ctx context.Context, gerritHost string) (map[string]GerritR utils.XREQUESTID: ctx.Value(utils.XREQUESTID), "gerritHost": gerritHost, } - client := &apiclient.RestAPIClient{ - Client: &http.Client{ - Timeout: 10 * time.Second, - }, - } + client := resty.New() base := "https://" + gerritHost @@ -300,40 +269,28 @@ 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) } - url := fmt.Sprintf("%s/projects/?d&pp=0", base) - resp, err := client.GetData(ctx, url) - + resp, err := client.R(). + EnableTrace(). + Get(fmt.Sprintf("%s/projects/?d&pp=0", base)) if err != nil { + log.WithFields(f).Warnf("problem querying gerrit host: %s, error: %+v", gerritHost, err) return nil, err } - 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 + 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) } + 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 - strippedBody := stripMagicPrefix(body) - - var result map[string]GerritRepoInfo - - err = json.Unmarshal(strippedBody, &result) + err = json.Unmarshal(resp.Body()[4:], &result) if err != nil { log.WithFields(f).Warnf("problem unmarshalling response for gerrit host: %s, error: %+v", gerritHost, err) return nil, err @@ -342,13 +299,6 @@ 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 5e9abdac4..457361f6a 100644 --- a/cla-backend-go/gerrits/service_test.go +++ b/cla-backend-go/gerrits/service_test.go @@ -4,121 +4,44 @@ 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 := "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", - }, - }, - }, - } + gerritName := "ONAP" + gerritURL := "https://gerrit.onap.org" - 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, - }, - } + ctrl := gomock.NewController(t) + defer ctrl.Finish() - 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, - } + 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) - mockRepo.EXPECT().AddGerrit(gomock.Any(), gomock.Any()).Return(gerrit, nil) + //Gerrit repo by name does not exist + mockRepo.EXPECT().ExistsByName(context.TODO(), "ONAP").Return(nil, nil) - service := NewService(mockRepo) + service := NewService(mockRepo) + gerrit, err := service.AddGerrit(context.TODO(), "projectID", "projectSFID", &models.AddGerritInput{ + GerritName: &gerritName, + GerritURL: &gerritURL, + }, &models.ClaGroup{ + ProjectID: "projectID", + }) - result, err := service.AddGerrit(context.Background(), tc.claGroupID, tc.projectSFID, tc.params, tc.claGroupModel) + assert.NotNil(t, gerrit) + assert.NoError(t, err) - 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) - }) - } }