Skip to content

Commit b217458

Browse files
lukaszgryglickilukaszgryglicki
committed
Final fix that allows 'compat' behaviour of V4 API that fallbacks to 'true' on 2 missing fields + test coverage
Signed-off-by: Łukasz Gryglicki <[email protected]>
1 parent 3229554 commit b217458

File tree

8 files changed

+90
-12
lines changed

8 files changed

+90
-12
lines changed

‎cla-backend-go/project/mocks/mock_repo.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cla-backend-go/project/mocks/mock_service.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cla-backend-go/project/repository/repository.go‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const (
5050
type ProjectRepository interface { //nolint
5151
CreateCLAGroup(ctx context.Context, claGroupModel *models.ClaGroup) (*models.ClaGroup, error)
5252
GetCLAGroupByID(ctx context.Context, claGroupID string, loadRepoDetails bool) (*models.ClaGroup, error)
53+
GetCLAGroupByIDCompat(ctx context.Context, claGroupID string, loadRepoDetails bool) (*models.ClaGroup, error)
5354
GetCLAGroupsByExternalID(ctx context.Context, params *project.GetProjectsByExternalIDParams, loadRepoDetails bool) (*models.ClaGroups, error)
5455
GetCLAGroupByName(ctx context.Context, claGroupName string) (*models.ClaGroup, error)
5556
GetExternalCLAGroup(ctx context.Context, claGroupExternalID string) (*models.ClaGroup, error)
@@ -149,7 +150,7 @@ func (repo *repo) CreateCLAGroup(ctx context.Context, claGroupModel *models.ClaG
149150
return claGroupModel, nil
150151
}
151152

152-
func (repo *repo) getCLAGroupByID(ctx context.Context, claGroupID string, loadCLAGroupDetails bool) (*models.ClaGroup, error) {
153+
func (repo *repo) getCLAGroupByID(ctx context.Context, claGroupID string, loadCLAGroupDetails bool, claEnabledDefaultIsTrue bool) (*models.ClaGroup, error) {
153154
f := logrus.Fields{
154155
"functionName": "project.repository.getCLAGroupByID",
155156
utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
@@ -188,19 +189,36 @@ func (repo *repo) getCLAGroupByID(ctx context.Context, claGroupID string, loadCL
188189
return nil, &utils.CLAGroupNotFound{CLAGroupID: claGroupID}
189190
}
190191
var dbModel models2.DBProjectModel
191-
err = dynamodbattribute.UnmarshalMap(results.Items[0], &dbModel)
192+
rawItem := results.Items[0]
193+
err = dynamodbattribute.UnmarshalMap(rawItem, &dbModel)
192194
if err != nil {
193195
log.WithFields(f).Warnf("error unmarshalling db cla group model, error: %+v", err)
194196
return nil, err
195197
}
198+
if claEnabledDefaultIsTrue {
199+
// If missing, assume true like Pynamo default=True
200+
if _, ok := rawItem["project_icla_enabled"]; !ok {
201+
dbModel.ProjectIclaEnabled = true
202+
}
203+
if _, ok := rawItem["project_ccla_enabled"]; !ok {
204+
dbModel.ProjectCclaEnabled = true
205+
}
206+
}
196207

197208
// Convert the database model to an API response model
198209
return repo.buildCLAGroupModel(ctx, dbModel, loadCLAGroupDetails), nil
199210
}
200211

201212
// GetCLAGroupByID returns the cla group model associated for the specified claGroupID
202213
func (repo *repo) GetCLAGroupByID(ctx context.Context, claGroupID string, loadRepoDetails bool) (*models.ClaGroup, error) {
203-
return repo.getCLAGroupByID(ctx, claGroupID, loadRepoDetails)
214+
return repo.getCLAGroupByID(ctx, claGroupID, loadRepoDetails, false)
215+
}
216+
217+
// GetCLAGroupByIDCompat returns the cla group model associated for the specified claGroupID
218+
func (repo *repo) GetCLAGroupByIDCompat(ctx context.Context, claGroupID string, loadRepoDetails bool) (*models.ClaGroup, error) {
219+
// Uses compatible mode (with python v2): claEnabledDefaultIsTrue - means if project_ccla_enabled or project_icla_enabled
220+
// aren't set on dynamoDB item - they will default to true as in Py V2 API
221+
return repo.getCLAGroupByID(ctx, claGroupID, loadRepoDetails, true)
204222
}
205223

206224
// GetCLAGroupsByExternalID queries the database and returns a list of the cla groups
@@ -383,7 +401,7 @@ func (repo *repo) GetClaGroupByProjectSFID(ctx context.Context, projectSFID stri
383401

384402
log.WithFields(f).Debugf("found CLA Group ID: %s for project SFID: %s", claGroupProject.ClaGroupID, projectSFID)
385403

386-
return repo.getCLAGroupByID(ctx, claGroupProject.ClaGroupID, loadRepoDetails)
404+
return repo.getCLAGroupByID(ctx, claGroupProject.ClaGroupID, loadRepoDetails, false)
387405
}
388406

389407
// GetCLAGroupByName returns the project model associated for the specified project name

‎cla-backend-go/project/service/service.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type Service interface {
3030
CreateCLAGroup(ctx context.Context, project *models.ClaGroup) (*models.ClaGroup, error)
3131
GetCLAGroups(ctx context.Context, params *project.GetProjectsParams) (*models.ClaGroups, error)
3232
GetCLAGroupByID(ctx context.Context, claGroupID string) (*models.ClaGroup, error)
33+
GetCLAGroupByIDCompat(ctx context.Context, claGroupID string) (*models.ClaGroup, error)
3334
GetCLAGroupsByExternalSFID(ctx context.Context, projectSFID string) (*models.ClaGroups, error)
3435
GetCLAGroupsByExternalID(ctx context.Context, params *project.GetProjectsByExternalIDParams) (*models.ClaGroups, error)
3536
GetCLAGroupByName(ctx context.Context, projectName string) (*models.ClaGroup, error)
@@ -75,6 +76,16 @@ func (s ProjectService) GetCLAGroups(ctx context.Context, params *project.GetPro
7576

7677
// GetCLAGroupByID service method
7778
func (s ProjectService) GetCLAGroupByID(ctx context.Context, claGroupID string) (*models.ClaGroup, error) {
79+
return s.getCLAGroupByID(ctx, claGroupID, false)
80+
}
81+
82+
// GetCLAGroupByIDCompat service method
83+
func (s ProjectService) GetCLAGroupByIDCompat(ctx context.Context, claGroupID string) (*models.ClaGroup, error) {
84+
return s.getCLAGroupByID(ctx, claGroupID, true)
85+
}
86+
87+
// getCLAGroupByID service method
88+
func (s ProjectService) getCLAGroupByID(ctx context.Context, claGroupID string, claEnabledDefaultIsTrue bool) (*models.ClaGroup, error) {
7889
f := logrus.Fields{
7990
"functionName": "GetCLAGroupByID",
8091
utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
@@ -83,7 +94,15 @@ func (s ProjectService) GetCLAGroupByID(ctx context.Context, claGroupID string)
8394
}
8495

8596
log.WithFields(f).Debug("locating CLA Group by ID...")
86-
project, err := s.repo.GetCLAGroupByID(ctx, claGroupID, repository.LoadRepoDetails)
97+
var (
98+
project *models.ClaGroup
99+
err error
100+
)
101+
if claEnabledDefaultIsTrue {
102+
project, err = s.repo.GetCLAGroupByIDCompat(ctx, claGroupID, repository.LoadRepoDetails)
103+
} else {
104+
project, err = s.repo.GetCLAGroupByID(ctx, claGroupID, repository.LoadRepoDetails)
105+
}
87106
if err != nil {
88107
return nil, err
89108
}

‎cla-backend-go/v2/project/handlers.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ func Configure(api *operations.EasyclaAPI, service v1Project.Service, v2Service
349349
"projectID": params.ProjectID,
350350
}
351351

352-
proj, err := service.GetCLAGroupByID(ctx, params.ProjectID)
352+
proj, err := service.GetCLAGroupByIDCompat(ctx, params.ProjectID)
353353
if err != nil {
354354
if err.Error() == projectDoesNotExist {
355355
return project.NewGetProjectCompatNotFound().WithXRequestID(reqID).WithPayload(errorResponse(reqID, err))
@@ -448,8 +448,6 @@ func buildCompatProject(project *v1Models.ClaGroup, projectClaGroups []*projects
448448
GitlabRepos: gitlabRepos,
449449
})
450450
}
451-
// fmt.Printf("GERRITS: %+v\n", *project.Gerrits[0])
452-
// fmt.Printf("GITHUBS: %+v\n", *project.GithubRepositories[0])
453451
return &models.ProjectCompat{
454452
FoundationSfid: project.FoundationSFID,
455453
ProjectName: project.ProjectName,

‎tests/py2go/README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
2) Start `golang` API backend:
77
- `` source setenv.sh; cd cla-backend-go; make swagger; make build-linux ``.
88
- `` PORT=5001 AUTH0_USERNAME_CLAIM_CLI='http://lfx.dev/claims/username' AUTH0_EMAIL_CLAIM_CLI='http://lfx.dev/claims/email' AUTH0_NAME_CLAIM_CLI='http://lfx.dev/claims/username' ./bin/cla ``.
9+
- Or: `` ../utils/run_go_api_server.sh ``.
910

1011
3) Get `auth0` token from browser session (login using `LFID`):
1112
- `` ./get_oauth_token.sh ``. Copy the token value.
@@ -18,3 +19,5 @@
1819
- `` MAX_PARALLEL=8 PY_API_URL=https://api.lfcla.dev.platform.linuxfoundation.org go test -v -run '^TestAllProjectsCompatAPI$' ``.
1920
- To run a specific test case(s): `` DEBUG=1 PROJECT_UUID=88ee12de-122b-4c46-9046-19422054ed8d PY_API_URL=https://api.lfcla.dev.platform.linuxfoundation.org go test -v -run '^TestProjectCompatAPI$' ``.
2021
- Manually via `cURL`: `` curl -s -XGET http://127.0.0.1:5001/v4/project-compat/01af041c-fa69-4052-a23c-fb8c1d3bef24 | jq . ``.
22+
- To manually see given project values if APIs differ (to dewbug): `` aws --region us-east-1 --profile lfproduct-dev dynamodb get-item --table-name cla-dev-projects --key '{"project_id": {"S": "4a855799-0aea-4e01-98b7-ef3da09df478"}}' | jq '.Item' ``.
23+
- And `` aws --region us-east-1 --profile lfproduct-dev dynamodb query --table-name cla-dev-projects-cla-groups --index-name cla-group-id-index --key-condition-expression "cla_group_id = :project_id" --expression-attribute-values '{":project_id":{"S":"4a855799-0aea-4e01-98b7-ef3da09df478"}}' | jq '.Items' ``.

‎tests/py2go/api_test.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,17 @@ func compareMappedFields(t *testing.T, pyData, goData map[string]interface{}, ke
146146

147147
if okPyTime && okGoTime {
148148
if !pyTime.Equal(goTime) {
149-
t.Errorf("Datetime mismatch for key '%s' (Go: '%s'): %s != %s", pyKey, goKey, pyTime, goTime)
149+
t.Errorf("Datetime mismatch for key '%s' (Go: '%s'): py:%s != go:%s", pyKey, goKey, pyTime, goTime)
150150
}
151151
continue
152152
}
153153

154+
if (pyVal == nil && goVal == "") || (goVal == nil && pyVal == "") {
155+
continue
156+
}
157+
154158
if fmt.Sprint(pyVal) != fmt.Sprint(goVal) {
155-
t.Errorf("Mismatch for key '%s' (Go: '%s'): %v != %v", pyKey, goKey, pyVal, goVal)
159+
t.Errorf("Mismatch for key '%s' (Go: '%s'): py:%+v != go:%+v", pyKey, goKey, pyVal, goVal)
156160
}
157161
}
158162
}
@@ -256,13 +260,17 @@ func compareNestedFields(t *testing.T, pyData, goData, keyMapping map[string]int
256260

257261
if okPyTime && okGoTime {
258262
if !pyTime.Equal(goTime) {
259-
t.Errorf("Datetime mismatch for key '%s': %s != %s", k, pyTime, goTime)
263+
t.Errorf("Datetime mismatch for key '%s': py:%s != go:%s", k, pyTime, goTime)
260264
}
261265
continue
262266
}
263267

268+
if (pyVal == nil && goVal == "") || (goVal == nil && pyVal == "") {
269+
continue
270+
}
271+
264272
if fmt.Sprint(pyVal) != fmt.Sprint(goVal) {
265-
t.Errorf("Mismatch for key '%s': %v != %v", k, pyVal, goVal)
273+
t.Errorf("Mismatch for key '%s': py:%+v != go:%+v", k, pyVal, goVal)
266274
}
267275
}
268276
}

‎utils/run_go_api_server.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/bin/bash
2+
make build-linux && PORT=5001 AUTH0_USERNAME_CLAIM_CLI='http://lfx.dev/claims/username' AUTH0_EMAIL_CLAIM_CLI='http://lfx.dev/claims/email' AUTH0_NAME_CLAIM_CLI='http://lfx.dev/claims/username' ./bin/cla

0 commit comments

Comments
 (0)