diff --git a/cmd/sippy/load.go b/cmd/sippy/load.go index 49e8346f69..04a3ab8ed5 100644 --- a/cmd/sippy/load.go +++ b/cmd/sippy/load.go @@ -39,6 +39,7 @@ import ( "github.com/openshift/sippy/pkg/dataloader/prowloader/github" "github.com/openshift/sippy/pkg/dataloader/releaseloader" "github.com/openshift/sippy/pkg/dataloader/testownershiploader" + "github.com/openshift/sippy/pkg/dataloader/testreportcacheloader" "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/flags" "github.com/openshift/sippy/pkg/github/commenter" @@ -318,6 +319,16 @@ func NewLoadCommand() *cobra.Command { loaders = append(loaders, fgLoader) } + if l == "test-report-cache" { + if dbErr != nil { + return dbErr + } + if cacheErr != nil { + return errors.Wrap(cacheErr, "couldn't get cache client") + } + loaders = append(loaders, testreportcacheloader.New(dbc, cacheClient, releaseConfigs)) + } + } // Run loaders with the metrics wrapper diff --git a/pkg/api/README.md b/pkg/api/README.md index 6482f1bb18..36d2cdbd54 100644 --- a/pkg/api/README.md +++ b/pkg/api/README.md @@ -408,10 +408,25 @@ Endpoint: `/api/tests` | filter | Filter | Filters the results by the specified value. | See filtering | | sortField| Field name | Sort by this field | | | sort | asc / desc | Sort type, ascending or descending | "asc" or "desc" | -| limit | Integer | The maximum amount of results to return | N/A | +| limit | Integer | The maximum amount of results to return (legacy, use perPage instead) | N/A | +| perPage | Integer | Number of results per page (enables server-side pagination) | 1-1000 | +| page | Integer | Zero-indexed page number (requires perPage) | 0+ | + +When `perPage` is provided, the response is wrapped in a pagination envelope: + +```json +{ + "rows": [ ... ], + "total_rows": 2847, + "page_size": 25, + "page": 0 +} +``` + +When `perPage` is omitted, the legacy array response format is returned for backward compatibility.
-Example response +Example response (legacy, without pagination) ```json [ diff --git a/pkg/api/cache.go b/pkg/api/cache.go index 20cf2ab5c7..1c9d177b0b 100644 --- a/pkg/api/cache.go +++ b/pkg/api/cache.go @@ -1,9 +1,12 @@ package api import ( + "bytes" + "compress/gzip" "context" "encoding/json" "fmt" + "io" "reflect" "strings" "time" @@ -15,6 +18,17 @@ import ( var defaultCacheDuration = 8 * time.Hour +func formatBytes(b int) string { + switch { + case b >= 1<<20: + return fmt.Sprintf("%.1f MB", float64(b)/float64(1<<20)) + case b >= 1<<10: + return fmt.Sprintf("%.1f KB", float64(b)/float64(1<<10)) + default: + return fmt.Sprintf("%d B", b) + } +} + // CacheSpec specifies caching parameters for an individual query type CacheSpec struct { // function to turn the specified key struct into cacheable bytes @@ -94,8 +108,12 @@ func GetDataFromCacheOrGenerate[T any]( "key": string(cacheKey), "type": reflect.TypeOf(defaultVal).String(), }).Infof("cache hit") + decompressed, err := cacheDecompress(res) + if err != nil { + return defaultVal, []error{errors.WithMessagef(err, "failed to decompress cached item. cacheKey=%+v", cacheKey)} + } var cr T - if err := json.Unmarshal(res, &cr); err != nil { + if err := json.Unmarshal(decompressed, &cr); err != nil { return defaultVal, []error{errors.WithMessagef(err, "failed to unmarshal cached item. cacheKey=%+v", cacheKey)} } return cr, nil @@ -118,20 +136,73 @@ func GetDataFromCacheOrGenerate[T any]( return generateFn(ctx) } -func CacheSet[T any](ctx context.Context, c cache.Cache, result T, cacheKey []byte, cacheDuration time.Duration) { +// gzipMagic is prepended to compressed cache entries so reads can detect them. +var gzipMagic = []byte{0x1f, 0x8b} + +func CacheSet[T any](ctx context.Context, c cache.Cache, result T, cacheKey []byte, cacheDuration time.Duration, opts ...CacheSetOption) { + o := cacheSetOptions{} + for _, opt := range opts { + opt(&o) + } + cr, err := json.Marshal(result) - if err == nil { - if err := c.Set(ctx, string(cacheKey), cr, cacheDuration); err != nil { - if strings.Contains(err.Error(), "connection refused") { - logrus.WithError(err).Fatalf("redis URL specified but got connection refused, exiting due to cost issues in this configuration") - } - logrus.WithError(err).Warningf("couldn't persist new item to cache") - } else { - logrus.Debugf("cache set for cache key: %s", string(cacheKey)) + if err != nil { + logrus.WithError(err).Errorf("Failed to marshall cache item") + return + } + + stored := cr + if o.compress { + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + if _, err := gz.Write(cr); err != nil { + logrus.WithError(err).Errorf("Failed to gzip cache item") + return + } + if err := gz.Close(); err != nil { + logrus.WithError(err).Errorf("Failed to close gzip writer") + return } + stored = buf.Bytes() + logrus.Infof("cache set: key=%s raw=%s compressed=%s", string(cacheKey), formatBytes(len(cr)), formatBytes(len(stored))) } else { - logrus.WithError(err).Errorf("Failed to marshall cache item: %v", result) + logrus.Infof("cache set: key=%s size=%s", string(cacheKey), formatBytes(len(cr))) + } + + if err := c.Set(ctx, string(cacheKey), stored, cacheDuration); err != nil { + if strings.Contains(err.Error(), "connection refused") { + logrus.WithError(err).Fatalf("redis URL specified but got connection refused, exiting due to cost issues in this configuration") + } + logrus.WithError(err).Warningf("couldn't persist new item to cache, size=%s", formatBytes(len(stored))) + } +} + +type cacheSetOptions struct { + compress bool +} + +// CacheSetOption configures optional CacheSet behavior. +type CacheSetOption func(*cacheSetOptions) + +// WithCompression enables gzip compression for the cache entry. +func WithCompression() CacheSetOption { + return func(o *cacheSetOptions) { + o.compress = true + } +} + +// cacheDecompress returns decompressed bytes if the input is gzip-compressed, +// otherwise returns the input unchanged. +func cacheDecompress(data []byte) ([]byte, error) { + if len(data) < 2 || data[0] != gzipMagic[0] || data[1] != gzipMagic[1] { + return data, nil + } + gz, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("failed to create gzip reader: %w", err) } + defer gz.Close() + return io.ReadAll(gz) } func CalculateRoundedCacheDuration(cacheOptions cache.RequestOptions) time.Duration { @@ -198,7 +269,10 @@ func GetDataFromCacheOrMatview[T any](ctx context.Context, "type": reflect.TypeOf(defaultVal).String(), }).Debugf("cache hit") - if err := json.Unmarshal(cached, &cacheVal); err != nil { + decompressed, decompErr := cacheDecompress(cached) + if decompErr != nil { + logrus.WithError(decompErr).Warnf("failed to decompress cached item. cacheKey=%+v", cacheKey) + } else if err := json.Unmarshal(decompressed, &cacheVal); err != nil { logrus.WithError(err).Warnf("failed to unmarshal cached item. cacheKey=%+v", cacheKey) // fall through to generate the data instead } else { diff --git a/pkg/api/tests.go b/pkg/api/tests.go index 415ee9e917..7e9c64e21c 100644 --- a/pkg/api/tests.go +++ b/pkg/api/tests.go @@ -14,6 +14,7 @@ import ( "github.com/pkg/errors" log "github.com/sirupsen/logrus" "google.golang.org/api/iterator" + "gorm.io/gorm" apitype "github.com/openshift/sippy/pkg/apis/api" "github.com/openshift/sippy/pkg/apis/cache" @@ -226,6 +227,19 @@ func GetTestDurationsFromDB(dbc *db.DB, release, test string, filters *filter.Fi type TestsAPIResult []apitype.Test +func (tests TestsAPIResult) filter(f *filter.Filter) TestsAPIResult { + if f == nil || len(f.Items) == 0 { + return tests + } + var result TestsAPIResult + for _, t := range tests { + if match, err := f.Filter(t); err == nil && match { + result = append(result, t) + } + } + return result +} + func (tests TestsAPIResult) sort(req *http.Request) TestsAPIResult { sortField := param.SafeRead(req, "sortField") sort := param.SafeRead(req, "sort") @@ -339,19 +353,47 @@ func PrintTestsJSONFromDB( w http.ResponseWriter, req *http.Request, dbc *db.DB, cacheClient cache.Cache, release string, + pagination *apitype.Pagination, ) { spec, ok := makeTestsResultsSpec(w, req, release) if !ok { return } + // Cache the unfiltered result set so that filter/sort/page changes + // are served from cache without hitting the database again. + requestFilter := spec.Filter + spec.Filter = nil + result, err := spec.buildTestsResultsFromPostgres(req.Context(), dbc, cacheClient) if err != nil { RespondWithJSON(http.StatusInternalServerError, w, map[string]interface{}{"code": http.StatusInternalServerError, "message": "Error building job report:" + err.Error()}) return } - testsResult := result.TestsAPIResult.sort(req).limit(req) + filtered := result.TestsAPIResult.filter(requestFilter) + sorted := filtered.sort(req) + + if pagination != nil { + totalRows := int64(len(sorted)) + start := pagination.Page * pagination.PerPage + end := start + pagination.PerPage + if start > int(totalRows) { + start = int(totalRows) + } + if end > int(totalRows) { + end = int(totalRows) + } + RespondWithJSON(http.StatusOK, w, apitype.PaginationResult{ + Rows: sorted[start:end], + TotalRows: totalRows, + PageSize: pagination.PerPage, + Page: pagination.Page, + }) + return + } + + testsResult := sorted.limit(req) if result.Test != nil { testsResult = append([]apitype.Test{*result.Test}, testsResult...) } @@ -359,7 +401,7 @@ func PrintTestsJSONFromDB( RespondWithJSON(http.StatusOK, w, testsResult) } -func PrintTestsJSONFromBigQuery(release string, w http.ResponseWriter, req *http.Request, bqc *bq.Client) { +func PrintTestsJSONFromBigQuery(release string, w http.ResponseWriter, req *http.Request, bqc *bq.Client, pagination *apitype.Pagination) { spec, ok := makeTestsResultsSpec(w, req, release) if !ok { return @@ -371,11 +413,32 @@ func PrintTestsJSONFromBigQuery(release string, w http.ResponseWriter, req *http return } - testsResult := result.TestsAPIResultBQ.sort(req).limit(req) + sorted := result.TestsAPIResultBQ.sort(req) if result.Test != nil { - testsResult = append([]apitype.TestBQ{*result.Test}, testsResult...) + sorted = append([]apitype.TestBQ{*result.Test}, sorted...) } + if pagination != nil { + totalRows := int64(len(sorted)) + start := pagination.Page * pagination.PerPage + end := start + pagination.PerPage + if start > int(totalRows) { + start = int(totalRows) + } + if end > int(totalRows) { + end = int(totalRows) + } + RespondWithJSON(http.StatusOK, w, apitype.PaginationResult{ + Rows: sorted[start:end], + TotalRows: totalRows, + PageSize: pagination.PerPage, + Page: pagination.Page, + }) + return + } + + testsResult := sorted.limit(req) + RespondWithJSON(http.StatusOK, w, testsResult) } @@ -418,25 +481,63 @@ type TestResultsSpec struct { Collapse, IncludeOverall bool Filter *filter.Filter } + +func (spec *TestResultsSpec) matview() string { + if spec.Period == "twoDay" { + return testReport2dMatView + } + return testReport7dMatView +} + type testResults struct { TestsAPIResult Test *apitype.Test } -const testResultsCacheDuration = time.Hour +const TestResultsCacheDuration = 5 * time.Hour -func (spec *TestResultsSpec) buildTestsResultsFromPostgres(ctx context.Context, dbc *db.DB, cacheClient cache.Cache) (testResults, error) { - matview := testReport7dMatView - if spec.Period == "twoDay" { - matview = testReport2dMatView +// PrimeTestResultsCache unconditionally regenerates and caches results for the +// given release/period/collapse combination, replacing any existing entry. +func PrimeTestResultsCache(ctx context.Context, dbc *db.DB, cacheClient cache.Cache, release, period string, collapse bool) error { + spec := TestResultsSpec{ + Release: release, + Period: period, + Collapse: collapse, + IncludeOverall: !collapse, + } + if cacheClient == nil { + return fmt.Errorf("cache client is required for priming") + } + matview := spec.matview() + result, errs := spec.buildTestsResultsPGGenerator(ctx, dbc, matview) + if len(errs) > 0 { + return fmt.Errorf("error(s) generating test results: %v", errs) + } + cacheSpec := NewCacheSpec(spec, "PostgresTestsResults~", nil) + cacheKey, err := cacheSpec.GetCacheKey() + if err != nil { + return fmt.Errorf("error getting cache key: %v", err) } + cacheVal := struct { + Val testResults + Timestamp time.Time + }{ + Val: result, + Timestamp: time.Now().UTC(), + } + CacheSet(ctx, cacheClient, cacheVal, cacheKey, TestResultsCacheDuration, WithCompression()) + return nil +} + +func (spec *TestResultsSpec) buildTestsResultsFromPostgres(ctx context.Context, dbc *db.DB, cacheClient cache.Cache) (testResults, error) { + matview := spec.matview() generator := func(ctx context.Context) (testResults, []error) { return spec.buildTestsResultsPGGenerator(ctx, dbc, matview) } result, errs := GetDataFromCacheOrMatview(ctx, cacheClient, NewCacheSpec(spec, "PostgresTestsResults~", nil), - matview, testResultsCacheDuration, + matview, TestResultsCacheDuration, generator, testResults{}, ) @@ -466,7 +567,26 @@ func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, d if spec.Collapse { rawQuery = rawQuery.Select(`suite_name,name,jira_component,jira_component_id,` + query.QueryTestSummer).Group("suite_name,name,jira_component,jira_component_id") } else { - rawQuery = query.TestsByNURPAndStandardDeviation(dbc, spec.Release, matview) + var subqueryFilters []query.SubqueryFilter + if rawFilter != nil { + variantFilter, nameFilter := rawFilter.Split([]string{"variants"}) + if len(nameFilter.Items) > 0 { + subqueryFilters = append(subqueryFilters, query.SubqueryFilter{ + Apply: func(db *gorm.DB) *gorm.DB { + return nameFilter.ToSQL(db, apitype.Test{}) + }, + }) + } + if len(variantFilter.Items) > 0 { + subqueryFilters = append(subqueryFilters, query.SubqueryFilter{ + Apply: func(db *gorm.DB) *gorm.DB { + return variantFilter.ToSQL(db, apitype.Test{}) + }, + VariantOnly: true, + }) + } + } + rawQuery = query.TestsByNURPAndStandardDeviation(dbc, spec.Release, matview, subqueryFilters...) variantSelect = "suite_name, variants," + "delta_from_working_average, working_average, working_standard_deviation, " + "delta_from_passing_average, passing_average, passing_standard_deviation, " + @@ -496,7 +616,6 @@ func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, d return } - // Produce a special "overall" test that has a summary of all the selected tests. var overallTest *apitype.Test if spec.IncludeOverall { finalResults := dbc.DB.Table("(?) as final_results", finalResults) @@ -533,7 +652,7 @@ func (spec *TestResultsSpec) buildTestsResultsFromBigQuery(ctx context.Context, result, errs := GetDataFromCacheOrGenerate[testResultsBQ]( ctx, bqc.Cache, - cache.RequestOptions{Expiry: testResultsCacheDuration}, + cache.RequestOptions{Expiry: TestResultsCacheDuration}, NewCacheSpec(spec, "BigQueryTestsResults~", nil), generator, testResultsBQ{}) diff --git a/pkg/dataloader/testreportcacheloader/testreportcacheloader.go b/pkg/dataloader/testreportcacheloader/testreportcacheloader.go new file mode 100644 index 0000000000..630e3a250d --- /dev/null +++ b/pkg/dataloader/testreportcacheloader/testreportcacheloader.go @@ -0,0 +1,81 @@ +package testreportcacheloader + +import ( + "context" + + log "github.com/sirupsen/logrus" + + "github.com/openshift/sippy/pkg/api" + "github.com/openshift/sippy/pkg/apis/cache" + v1 "github.com/openshift/sippy/pkg/apis/sippy/v1" + "github.com/openshift/sippy/pkg/db" +) + +type testReportCacheLoader struct { + dbc *db.DB + cacheClient cache.Cache + releases []v1.Release + errs []error +} + +func New(dbc *db.DB, cacheClient cache.Cache, releases []v1.Release) *testReportCacheLoader { + return &testReportCacheLoader{ + dbc: dbc, + cacheClient: cacheClient, + releases: releases, + } +} + +func (l *testReportCacheLoader) Name() string { + return "test-report-cache" +} + +func (l *testReportCacheLoader) Load() { + ctx := context.Background() + + var releases []string + if err := l.dbc.DB.Raw("SELECT DISTINCT release FROM prow_test_report_7d_matview").Scan(&releases).Error; err != nil { + l.errs = append(l.errs, err) + return + } + + devReleases := l.developmentReleases() + for _, release := range releases { + if !devReleases[release] { + log.Infof("Skipping test report cache for non-development release=%s", release) + continue + } + for _, period := range []string{"default", "twoDay"} { + for _, collapse := range []bool{true, false} { + log.Infof("Priming test report cache for release=%s period=%s collapse=%v", release, period, collapse) + if err := api.PrimeTestResultsCache(ctx, l.dbc, l.cacheClient, release, period, collapse); err != nil { + log.WithError(err).Errorf("Failed to prime test report cache for release=%s period=%s collapse=%v", release, period, collapse) + l.errs = append(l.errs, err) + } + } + } + } +} + +// developmentReleases returns a set of release names that are OCP development +// releases (have payloadTags capability and no GA date). +func (l *testReportCacheLoader) developmentReleases() map[string]bool { + devReleases := make(map[string]bool) + for _, r := range l.releases { + if r.Product != "OCP" { + continue + } + if r.GADate != nil { + continue + } + if !r.Capabilities[v1.PayloadTagsCap] { + continue + } + devReleases[r.Release] = true + } + return devReleases +} + +func (l *testReportCacheLoader) Errors() []error { + return l.errs +} diff --git a/pkg/dataloader/testreportcacheloader/testreportcacheloader_test.go b/pkg/dataloader/testreportcacheloader/testreportcacheloader_test.go new file mode 100644 index 0000000000..b66734b873 --- /dev/null +++ b/pkg/dataloader/testreportcacheloader/testreportcacheloader_test.go @@ -0,0 +1,93 @@ +package testreportcacheloader + +import ( + "testing" + "time" + + v1 "github.com/openshift/sippy/pkg/apis/sippy/v1" + "github.com/stretchr/testify/assert" +) + +func TestDevelopmentReleases(t *testing.T) { + gaDate := time.Date(2024, 10, 1, 0, 0, 0, 0, time.UTC) + + tests := []struct { + name string + releases []v1.Release + want map[string]bool + }{ + { + name: "filters to dev OCP releases only", + releases: []v1.Release{ + { + Release: "4.18", + Product: "OCP", + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true}, + }, + { + Release: "4.17", + Product: "OCP", + GADate: &gaDate, + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true}, + }, + { + Release: "Presubmits", + Product: "OCP", + Capabilities: map[v1.ReleaseCapability]bool{v1.SippyClassicCap: true}, + }, + { + Release: "4.19", + Product: "OCP", + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true, v1.SippyClassicCap: true}, + }, + }, + want: map[string]bool{ + "4.18": true, + "4.19": true, + }, + }, + { + name: "excludes OKD releases", + releases: []v1.Release{ + { + Release: "4.18", + Product: "OCP", + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true}, + }, + { + Release: "4.18-okd", + Product: "OKD", + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true}, + }, + }, + want: map[string]bool{ + "4.18": true, + }, + }, + { + name: "empty releases", + releases: nil, + want: map[string]bool{}, + }, + { + name: "all GA releases returns empty", + releases: []v1.Release{ + { + Release: "4.17", + Product: "OCP", + GADate: &gaDate, + Capabilities: map[v1.ReleaseCapability]bool{v1.PayloadTagsCap: true}, + }, + }, + want: map[string]bool{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := &testReportCacheLoader{releases: tt.releases} + got := l.developmentReleases() + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/db/query/test_queries.go b/pkg/db/query/test_queries.go index 83c1acd89e..89a39e4776 100644 --- a/pkg/db/query/test_queries.go +++ b/pkg/db/query/test_queries.go @@ -244,7 +244,17 @@ func LoadBugsForTest(dbc *db.DB, testName string, filterClosed bool) ([]models.B // flake_average shows the average flake percentage among all variants. // flake_standard_deviation shows the standard deviation of the flake percentage among variants. The number reflects how much flake percentage differs among variants. // delta_from_flake_average shows how much each variant differs from the flake_average. This can be used to identify outliers. -func TestsByNURPAndStandardDeviation(dbc *db.DB, release, table string) *gorm.DB { + +// SubqueryFilter wraps a filter function with metadata about what it targets. +// Variant-only filters are applied only to passRates, not to stats, because +// the stats subquery computes cross-variant averages and standard deviations +// that should not be skewed by variant exclusions. +type SubqueryFilter struct { + Apply func(*gorm.DB) *gorm.DB + VariantOnly bool +} + +func TestsByNURPAndStandardDeviation(dbc *db.DB, release, table string, subqueryFilters ...SubqueryFilter) *gorm.DB { // 1. Create a virtual stats table. There is a single row for each test. stats := dbc.DB.Table(table). Select(` @@ -264,6 +274,18 @@ func TestsByNURPAndStandardDeviation(dbc *db.DB, release, table string) *gorm.DB Select(`id as test_id, suite_name as pass_rate_suite_name, variants as pass_rate_variants, `+QueryTestPercentages). Where(`release = ?`, release) + // Apply filters to the subqueries so they can leverage indexes instead of + // scanning the entire matview for the release. Variant-specific filters + // only apply to passRates to preserve cross-variant stats semantics. + for _, f := range subqueryFilters { + if f.VariantOnly { + passRates = f.Apply(passRates) + } else { + stats = f.Apply(stats) + passRates = f.Apply(passRates) + } + } + // 3. Join the tables to produce test report. Each row represent one variant of a test and contains all stats, both unique to the specific variant and average across all variants. return dbc.DB. Table(table). diff --git a/pkg/sippyserver/parameters.go b/pkg/sippyserver/parameters.go index 6f420c7a74..1e66713420 100644 --- a/pkg/sippyserver/parameters.go +++ b/pkg/sippyserver/parameters.go @@ -1,6 +1,7 @@ package sippyserver import ( + "fmt" "net/http" "strconv" "time" @@ -74,6 +75,8 @@ func getLimitParam(req *http.Request) int { return limit } +const maxPerPage = 1000 + func getPaginationParams(req *http.Request) (*apitype.Pagination, error) { perPage := req.URL.Query().Get("perPage") page := req.URL.Query().Get("page") @@ -82,6 +85,9 @@ func getPaginationParams(req *http.Request) (*apitype.Pagination, error) { if err != nil { return nil, err } + if perPageInt <= 0 || perPageInt > maxPerPage { + return nil, fmt.Errorf("perPage must be between 1 and %d", maxPerPage) + } pageNo := 0 if page != "" { @@ -89,6 +95,9 @@ func getPaginationParams(req *http.Request) (*apitype.Pagination, error) { if err != nil { return nil, err } + if pageNo < 0 { + return nil, fmt.Errorf("page must be non-negative") + } } return &apitype.Pagination{ diff --git a/pkg/sippyserver/parameters_test.go b/pkg/sippyserver/parameters_test.go new file mode 100644 index 0000000000..1f708a90be --- /dev/null +++ b/pkg/sippyserver/parameters_test.go @@ -0,0 +1,97 @@ +package sippyserver + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetPaginationParams(t *testing.T) { + tests := []struct { + name string + query url.Values + wantNil bool + wantErr bool + wantPage int + wantPer int + }{ + { + name: "no params returns nil", + query: url.Values{}, + wantNil: true, + }, + { + name: "valid perPage and page", + query: url.Values{"perPage": {"25"}, "page": {"2"}}, + wantPer: 25, + wantPage: 2, + }, + { + name: "perPage without page defaults to page 0", + query: url.Values{"perPage": {"50"}}, + wantPer: 50, + wantPage: 0, + }, + { + name: "perPage=0 returns error", + query: url.Values{"perPage": {"0"}}, + wantErr: true, + }, + { + name: "negative perPage returns error", + query: url.Values{"perPage": {"-1"}}, + wantErr: true, + }, + { + name: "negative page returns error", + query: url.Values{"perPage": {"25"}, "page": {"-1"}}, + wantErr: true, + }, + { + name: "perPage exceeds max returns error", + query: url.Values{"perPage": {"10000"}}, + wantErr: true, + }, + { + name: "non-numeric perPage returns error", + query: url.Values{"perPage": {"abc"}}, + wantErr: true, + }, + { + name: "non-numeric page returns error", + query: url.Values{"perPage": {"25"}, "page": {"abc"}}, + wantErr: true, + }, + { + name: "max perPage is accepted", + query: url.Values{"perPage": {"1000"}}, + wantPer: 1000, + wantPage: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := &http.Request{URL: &url.URL{RawQuery: tc.query.Encode()}} + result, err := getPaginationParams(req) + + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + + if tc.wantNil { + assert.Nil(t, result) + return + } + + require.NotNil(t, result) + assert.Equal(t, tc.wantPer, result.PerPage) + assert.Equal(t, tc.wantPage, result.Page) + }) + } +} diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index 3165f605c3..50cf333126 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -1133,9 +1133,15 @@ func (s *Server) jsonJobBugsFromDB(w http.ResponseWriter, req *http.Request) { func (s *Server) jsonTestsReportFromDB(w http.ResponseWriter, req *http.Request) { release := s.getParamOrFail(w, req, "release") - if release != "" { - api.PrintTestsJSONFromDB(w, req, s.db, s.cache, release) + if release == "" { + return + } + pagination, err := getPaginationParams(req) + if err != nil { + api.RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{"code": http.StatusBadRequest, "message": "invalid pagination params: " + err.Error()}) + return } + api.PrintTestsJSONFromDB(w, req, s.db, s.cache, release, pagination) } func (s *Server) jsonTestsReportFromBigQuery(w http.ResponseWriter, req *http.Request) { @@ -1145,9 +1151,15 @@ func (s *Server) jsonTestsReportFromBigQuery(w http.ResponseWriter, req *http.Re return } release := s.getParamOrFail(w, req, "release") - if release != "" { - api.PrintTestsJSONFromBigQuery(release, w, req, s.bigQueryClient) + if release == "" { + return + } + pagination, err := getPaginationParams(req) + if err != nil { + api.RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{"code": http.StatusBadRequest, "message": "invalid pagination params: " + err.Error()}) + return } + api.PrintTestsJSONFromBigQuery(release, w, req, s.bigQueryClient, pagination) } func (s *Server) jsonTestDetailsReportFromDB(w http.ResponseWriter, req *http.Request) { diff --git a/sippy-ng/src/tests/TestTable.js b/sippy-ng/src/tests/TestTable.js index 2455f853a7..9b7a531ee2 100644 --- a/sippy-ng/src/tests/TestTable.js +++ b/sippy-ng/src/tests/TestTable.js @@ -128,7 +128,9 @@ function TestTable(props) { const [fetchError, setFetchError] = React.useState('') const [isLoaded, setLoaded] = React.useState(false) const [isSearching, setSearching] = React.useState(false) - const [rows, setRows] = React.useState([]) + const [apiResult, setApiResult] = React.useState({ rows: [], total_rows: 0 }) + const [page = 0, setPage] = useQueryParam('page', NumberParam) + const [pageFlip, setPageFlip] = React.useState(false) const [period = props.period, setPeriod] = useQueryParam( 'period', @@ -917,6 +919,7 @@ function TestTable(props) { gridView.setView(v) setSort(gridView.view.sort) setSortField(gridView.view.sortField) + setPage(0) } const fetchData = () => { @@ -940,6 +943,8 @@ function TestTable(props) { queryString += '&sortField=' + safeEncodeURIComponent(sortField) queryString += '&sort=' + safeEncodeURIComponent(sort) + queryString += '&perPage=' + safeEncodeURIComponent(pageSize) + queryString += '&page=' + safeEncodeURIComponent(page) queryString += '&collapse=' + safeEncodeURIComponent(props.collapse) @@ -957,12 +962,13 @@ function TestTable(props) { }) .then((json) => { if (json != null) { - setRows(json) + setApiResult(json) } else { - setRows([]) + setApiResult({ rows: [], total_rows: 0 }) } setLoaded(true) setSearching(false) + setPageFlip(false) }) .catch((error) => { setFetchError( @@ -976,7 +982,7 @@ function TestTable(props) { useEffect(() => { if (prevLocation.current !== location) { - setRows([]) + setApiResult({ rows: [], total_rows: 0 }) setLoaded(false) } @@ -986,7 +992,7 @@ function TestTable(props) { fetchData() } else { // Mark as loaded so we don't show loading spinner, and clear any stale data - setRows([]) + setApiResult({ rows: [], total_rows: 0 }) setLoaded(true) setSearching(false) } @@ -997,6 +1003,8 @@ function TestTable(props) { filterModel, sort, sortField, + page, + pageSize, props.collapse, props.briefTable, view, @@ -1004,6 +1012,7 @@ function TestTable(props) { const requestSearch = (searchValue) => { setSearching(true) + setPage(0) const currentFilters = filterModel currentFilters.items = currentFilters.items.filter( (f) => f.columnField !== 'name' @@ -1041,6 +1050,7 @@ function TestTable(props) { currentFilters.push(item) } }) + setPage(0) setFilterModel({ items: currentFilters, linkOperator: filterModel.linkOperator || 'and', @@ -1059,6 +1069,13 @@ function TestTable(props) { if (sortField !== model[0].field) { setSortField(model[0].field) } + + setPage(0) + } + + const changePage = (newPage) => { + setPageFlip(true) + setPage(newPage) } return ( @@ -1071,15 +1088,23 @@ function TestTable(props) { )} (props.collapse ? 100 : 'auto')} disableColumnFilter={props.briefTable} + pagination + paginationMode="server" + page={page} pageSize={pageSize} - onPageSizeChange={(newPageSize) => setPageSize(newPageSize)} + onPageChange={(newPage) => changePage(newPage)} + onPageSizeChange={(newPageSize) => { + setPageSize(newPageSize) + setPage(0) + }} rowsPerPageOptions={props.rowsPerPageOptions} checkboxSelection={false} filterMode="server" @@ -1129,7 +1154,7 @@ function TestTable(props) { filterModel: filterModel, setFilterModel: setFilterModel, downloadDataFunc: () => { - return rows + return apiResult.rows || [] }, downloadFilePrefix: 'tests', },