From 4a4fea0b813e8eef3869df77592f0ae9d9820fb8 Mon Sep 17 00:00:00 2001 From: ewezy Date: Wed, 26 Oct 2022 01:32:20 +0800 Subject: [PATCH] Refactor pagination to only take place when fields are not specified --- .../services/experiment_service.go | 2 +- .../services/experiment_service_test.go | 54 ++++++++++++------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/management-service/services/experiment_service.go b/management-service/services/experiment_service.go index ba835c56..16b7c286 100644 --- a/management-service/services/experiment_service.go +++ b/management-service/services/experiment_service.go @@ -166,7 +166,7 @@ func (svc *experimentService) ListExperiments( // Pagination var pagingResponse *pagination.Paging var count int64 - if params.Fields == nil || params.Page != nil || params.PageSize != nil { + if params.Fields == nil { err = pagination.ValidatePaginationParams(params.Page, params.PageSize) if err != nil { return nil, nil, err diff --git a/management-service/services/experiment_service_test.go b/management-service/services/experiment_service_test.go index 848c1e50..db126b7d 100644 --- a/management-service/services/experiment_service_test.go +++ b/management-service/services/experiment_service_test.go @@ -163,28 +163,30 @@ func testListExperiments(s *ExperimentServiceTestSuite) { stringSegmenter := []string{"seg-1"} boolSegmenter := []string{"true"} + var nilPagingResponse *pagination.Paging + // All experiments under a settings - expResponsesList, pagingResponse, err := svc.ListExperiments(1, services.ListExperimentsParams{}) + actualResponsesList, pagingResponse, err := svc.ListExperiments(1, services.ListExperimentsParams{}) s.Suite.Require().NoError(err) tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 1, Total: 3}, pagingResponse) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[1], s.Experiments[2]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[1], s.Experiments[2]}, actualResponsesList) // No experiments filtered - expResponsesList, pagingResponse, err = svc.ListExperiments(3, services.ListExperimentsParams{}) + actualResponsesList, pagingResponse, err = svc.ListExperiments(3, services.ListExperimentsParams{}) s.Suite.Require().NoError(err) tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 0, Total: 0}, pagingResponse) - tu.AssertEqualValues(t, []*models.Experiment{}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{}, actualResponsesList) // Filter by a single parameter - expResponsesList, pagingResponse, err = svc.ListExperiments(1, + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{Status: &testStatus}, ) s.Suite.Require().NoError(err) tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 1, Total: 2}, pagingResponse) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[2]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[2]}, actualResponsesList) // Filter by all parameters - expResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{ + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{ Type: &testExpType, Status: &testStatus, StartTime: &testStartTime, @@ -208,11 +210,11 @@ func testListExperiments(s *ExperimentServiceTestSuite) { Pages: 1, Total: 1, }, pagingResponse) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0]}, actualResponsesList) // Use the same start and end times testExactTimestamp := time.Date(2021, 2, 2, 3, 5, 7, 0, time.UTC) - expResponsesList, pagingResponse, err = svc.ListExperiments(1, + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{ StartTime: &testExactTimestamp, EndTime: &testExactTimestamp, @@ -220,10 +222,10 @@ func testListExperiments(s *ExperimentServiceTestSuite) { ) s.Suite.Require().NoError(err) tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 1, Total: 1}, pagingResponse) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[2]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[2]}, actualResponsesList) // Partial match of segmenter on multiple experiments - expResponsesList, pagingResponse, err = svc.ListExperiments(1, + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{ Segment: models.ExperimentSegment{ "float_segmenter": float2Segmenter, @@ -234,11 +236,11 @@ func testListExperiments(s *ExperimentServiceTestSuite) { tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 1, Total: 2}, pagingResponse) tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[1]}, - expResponsesList, + actualResponsesList, ) // Weak match of segmenters - expResponsesList, pagingResponse, err = svc.ListExperiments(1, + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, services.ListExperimentsParams{ Segment: models.ExperimentSegment{ "float_segmenter": floatSegmenter, @@ -250,21 +252,21 @@ func testListExperiments(s *ExperimentServiceTestSuite) { tu.AssertEqualValues(t, &pagination.Paging{Page: 1, Pages: 1, Total: 2}, pagingResponse) tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[2]}, - expResponsesList, + actualResponsesList, ) // Match name or description testDesc := "-1" - expResponsesList, _, err = svc.ListExperiments(1, + actualResponsesList, _, err = svc.ListExperiments(1, services.ListExperimentsParams{ Search: &testDesc, }, ) s.Suite.Require().NoError(err) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[2]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[2]}, actualResponsesList) // Match friendly status + start time - expResponsesList, _, err = svc.ListExperiments(1, + actualResponsesList, _, err = svc.ListExperiments(1, services.ListExperimentsParams{ StatusFriendly: []services.ExperimentStatusFriendly{ services.ExperimentStatusFriendlyCompleted, @@ -275,7 +277,23 @@ func testListExperiments(s *ExperimentServiceTestSuite) { }, ) s.Suite.Require().NoError(err) - tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[1]}, expResponsesList) + tu.AssertEqualValues(t, []*models.Experiment{s.Experiments[0], s.Experiments[1]}, actualResponsesList) + + // Specify selected fields in ListExperimentsParams + actualResponsesList, pagingResponse, err = svc.ListExperiments(1, + services.ListExperimentsParams{ + Fields: &[]models.ExperimentField{ + models.ExperimentFieldName, + }, + }, + ) + s.Suite.Require().NoError(err) + tu.AssertEqualValues(t, nilPagingResponse, pagingResponse) + tu.AssertEqualValues(t, []*models.Experiment{ + {Name: s.Experiments[0].Name}, + {Name: s.Experiments[1].Name}, + {Name: s.Experiments[2].Name}, + }, actualResponsesList) } func testCreateUpdateExperiment(s *ExperimentServiceTestSuite) {