diff --git a/pkg/cli/client/client.go b/pkg/cli/client/client.go index 2163dbdb..e5528f83 100644 --- a/pkg/cli/client/client.go +++ b/pkg/cli/client/client.go @@ -445,7 +445,8 @@ func fetchManifestStruct(ctx context.Context, repo, manifestReference string, se imageSize += manifestResp.Config.Size imageSize += manifestSize - layers := []common.LayerSummary{} + // Pre-allocate slice with known capacity + layers := make([]common.LayerSummary, 0, len(manifestResp.Layers)) for _, entry := range manifestResp.Layers { imageSize += entry.Size diff --git a/pkg/cli/client/search_functions.go b/pkg/cli/client/search_functions.go index 6e36fcdc..fc7a8a42 100644 --- a/pkg/cli/client/search_functions.go +++ b/pkg/cli/client/search_functions.go @@ -51,7 +51,8 @@ func SearchAllImagesGQL(config SearchConfig) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity + imageListData := make([]imageStruct, 0, len(imageList.Results)) for _, image := range imageList.Results { imageListData = append(imageListData, imageStruct(image)) @@ -103,7 +104,8 @@ func SearchImageByNameGQL(config SearchConfig, imageName string) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity (may be filtered, but worst case is all results) + imageListData := make([]imageStruct, 0, len(imageList.Results)) for _, image := range imageList.Results { if tag == "" || image.Tag == tag { @@ -152,7 +154,8 @@ func SearchDerivedImageListGQL(config SearchConfig, derivedImage string) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity + imageListData := make([]imageStruct, 0, len(imageList.DerivedImageList.Results)) for _, image := range imageList.DerivedImageList.Results { imageListData = append(imageListData, imageStruct(image)) @@ -173,7 +176,8 @@ func SearchBaseImageListGQL(config SearchConfig, baseImage string) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity + imageListData := make([]imageStruct, 0, len(imageList.BaseImageList.Results)) for _, image := range imageList.BaseImageList.Results { imageListData = append(imageListData, imageStruct(image)) @@ -193,7 +197,8 @@ func SearchImagesForDigestGQL(config SearchConfig, digest string) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity + imageListData := make([]imageStruct, 0, len(imageList.Results)) for _, image := range imageList.Results { imageListData = append(imageListData, imageStruct(image)) @@ -344,7 +349,8 @@ func SearchImagesByCVEIDGQL(config SearchConfig, repo, cveid string) error { return err } - imageListData := []imageStruct{} + // Pre-allocate slice with known capacity + imageListData := make([]imageStruct, 0, len(imageList.Results)) for _, image := range imageList.Results { imageListData = append(imageListData, imageStruct(image)) @@ -403,13 +409,14 @@ func GlobalSearchGQL(config SearchConfig, query string) error { return err } - imagesList := []imageStruct{} + // Pre-allocate slices with known capacity + imagesList := make([]imageStruct, 0, len(globalSearchResult.Images)) for _, image := range globalSearchResult.Images { imagesList = append(imagesList, imageStruct(image)) } - reposList := []repoStruct{} + reposList := make([]repoStruct, 0, len(globalSearchResult.Repos)) for _, repo := range globalSearchResult.Repos { reposList = append(reposList, repoStruct(repo)) diff --git a/pkg/cli/client/service.go b/pkg/cli/client/service.go index c8794b10..02d38d1a 100644 --- a/pkg/cli/client/service.go +++ b/pkg/cli/client/service.go @@ -403,7 +403,9 @@ func (service searchService) getTagsForCVEGQL(ctx context.Context, config Search return result, nil } + // Pre-allocate filtered results slice with estimated capacity filteredResults := &common.ImagesForCve{} + filteredResults.Results = make([]common.ImageSummary, 0, len(result.Results)) for _, image := range result.Results { if image.RepoName == repo { @@ -476,7 +478,8 @@ func (service searchService) getReferrers(ctx context.Context, config SearchConf return referrersResult{}, err } - referrersList := referrersResult{} + // Pre-allocate referrers list with known capacity + referrersList := make(referrersResult, 0, len(referrerResp.Manifests)) for _, referrer := range referrerResp.Manifests { referrersList = append(referrersList, common.Referrer{ @@ -954,7 +957,8 @@ func (ref referrersResult) stringPlainText(maxArtifactTypeLen int) (string, erro var builder strings.Builder maxDigestWidth := digestWidth - rows := [][]string{} + // Pre-allocate rows slice with known capacity + rows := make([][]string, 0, len(ref)) for _, referrer := range ref { artifactType := ellipsize(referrer.ArtifactType, maxArtifactTypeLen, ellipsis) @@ -1390,12 +1394,14 @@ func (service searchService) getRepos(ctx context.Context, config SearchConfig, fmt.Fprintln(config.ResultWriter, "\nREPOSITORY NAME") if config.SortBy == SortByAlphabeticAsc { - for i := 0; i < len(catalog.Repositories); i++ { - fmt.Fprintln(config.ResultWriter, catalog.Repositories[i]) + for _, repo := range catalog.Repositories { + fmt.Fprintln(config.ResultWriter, repo) } } else { - for i := len(catalog.Repositories) - 1; i >= 0; i-- { - fmt.Fprintln(config.ResultWriter, catalog.Repositories[i]) + // Iterate in reverse order + repos := catalog.Repositories + for i := len(repos) - 1; i >= 0; i-- { + fmt.Fprintln(config.ResultWriter, repos[i]) } } } diff --git a/pkg/cli/client/utils.go b/pkg/cli/client/utils.go index cdd6a346..d264c70f 100644 --- a/pkg/cli/client/utils.go +++ b/pkg/cli/client/utils.go @@ -93,9 +93,9 @@ func collectResults(config SearchConfig, wg *sync.WaitGroup, imageErr chan strin func getUsernameAndPassword(user string) (string, string) { if strings.Contains(user, ":") { - split := strings.Split(user, ":") + username, password, _ := strings.Cut(user, ":") - return split[0], split[1] + return username, password } return "", "" diff --git a/pkg/extensions/monitoring/minimal.go b/pkg/extensions/monitoring/minimal.go index 882d32f4..a973c38a 100644 --- a/pkg/extensions/monitoring/minimal.go +++ b/pkg/extensions/monitoring/minimal.go @@ -7,6 +7,7 @@ import ( "fmt" "math" "path" + "slices" "strconv" "sync" "time" @@ -236,9 +237,12 @@ func NewMetricsServer(enabled bool, log log.Logger) MetricServer { Histograms: make([]*HistogramValue, 0), } // convert to a map for returning easily the string corresponding to a bucket - bucketsFloat2String := map[float64]string{} + // Pre-allocate with known size: default buckets + storage latency buckets + defaultBuckets := GetDefaultBuckets() + storageBuckets := GetStorageLatencyBuckets() + bucketsFloat2String := make(map[float64]string, len(defaultBuckets)+len(storageBuckets)) - for _, fvalue := range append(GetDefaultBuckets(), GetStorageLatencyBuckets()...) { + for _, fvalue := range append(defaultBuckets, storageBuckets...) { if fvalue == math.MaxFloat64 { bucketsFloat2String[fvalue] = "+Inf" } else { @@ -314,54 +318,38 @@ func isMetricMatch(lValues, metricValues []string) bool { // returns {-1, false} in case metric was not found in the slice. func findCounterValueIndex(metricSlice []*CounterValue, name string, labelValues []string) (int, bool) { - for i, m := range metricSlice { - if m.Name == name { - if isMetricMatch(labelValues, m.LabelValues) { - return i, true - } - } - } + idx := slices.IndexFunc(metricSlice, func(m *CounterValue) bool { + return m.Name == name && isMetricMatch(labelValues, m.LabelValues) + }) - return -1, false + return idx, idx != -1 } // returns {-1, false} in case metric was not found in the slice. func findGaugeValueIndex(metricSlice []*GaugeValue, name string, labelValues []string) (int, bool) { - for i, m := range metricSlice { - if m.Name == name { - if isMetricMatch(labelValues, m.LabelValues) { - return i, true - } - } - } + idx := slices.IndexFunc(metricSlice, func(m *GaugeValue) bool { + return m.Name == name && isMetricMatch(labelValues, m.LabelValues) + }) - return -1, false + return idx, idx != -1 } // returns {-1, false} in case metric was not found in the slice. func findSummaryValueIndex(metricSlice []*SummaryValue, name string, labelValues []string) (int, bool) { - for i, m := range metricSlice { - if m.Name == name { - if isMetricMatch(labelValues, m.LabelValues) { - return i, true - } - } - } + idx := slices.IndexFunc(metricSlice, func(m *SummaryValue) bool { + return m.Name == name && isMetricMatch(labelValues, m.LabelValues) + }) - return -1, false + return idx, idx != -1 } // returns {-1, false} in case metric was not found in the slice. func findHistogramValueIndex(metricSlice []*HistogramValue, name string, labelValues []string) (int, bool) { - for i, m := range metricSlice { - if m.Name == name { - if isMetricMatch(labelValues, m.LabelValues) { - return i, true - } - } - } + idx := slices.IndexFunc(metricSlice, func(m *HistogramValue) bool { + return m.Name == name && isMetricMatch(labelValues, m.LabelValues) + }) - return -1, false + return idx, idx != -1 } func (ms *metricServer) CounterInc(cv *CounterValue) { diff --git a/pkg/extensions/search/convert/metadb.go b/pkg/extensions/search/convert/metadb.go index d3c0af2c..846cd5aa 100644 --- a/pkg/extensions/search/convert/metadb.go +++ b/pkg/extensions/search/convert/metadb.go @@ -73,7 +73,8 @@ func getAnnotationsFromMap(annotationsMap map[string]string) []*gql_generated.An func getImageBlobsInfo(manifestDigest string, manifestSize int64, configDigest string, configSize int64, layers []ispec.Descriptor, ) (int64, map[string]int64) { - imageBlobsMap := map[string]int64{} + // Pre-allocate map with known size: config + manifest + layers + imageBlobsMap := make(map[string]int64, 2+len(layers)) imageSize := int64(0) // add config size diff --git a/pkg/extensions/search/cve/cve.go b/pkg/extensions/search/cve/cve.go index 7231dca7..94fd7411 100644 --- a/pkg/extensions/search/cve/cve.go +++ b/pkg/extensions/search/cve/cve.go @@ -2,7 +2,7 @@ package cveinfo import ( "context" - "sort" + "slices" "strings" "time" @@ -528,8 +528,15 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImageMedia(ctx context.Context, repo, } func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo { - sort.Slice(allTags, func(i, j int) bool { - return allTags[i].Timestamp.Before(allTags[j].Timestamp) + slices.SortFunc(allTags, func(a, b cvemodel.TagInfo) int { + if a.Timestamp.Before(b.Timestamp) { + return -1 + } + if a.Timestamp.Equal(b.Timestamp) { + return 0 + } + + return 1 }) earliestVulnerable := vulnerableTags[0] @@ -597,7 +604,10 @@ func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo } // check if the current manifest doesn't have the vulnerability - if !indexHasVulnerableManifest || !containsDescriptorInfo(vulnTagInfo.Manifests, manifestDesc) { + if !indexHasVulnerableManifest || + !slices.ContainsFunc(vulnTagInfo.Manifests, func(di cvemodel.DescriptorInfo) bool { + return di.Digest == manifestDesc.Digest + }) { fixedManifests = append(fixedManifests, manifestDesc) } } @@ -616,16 +626,6 @@ func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo return fixedTags } -func containsDescriptorInfo(slice []cvemodel.DescriptorInfo, descriptorInfo cvemodel.DescriptorInfo) bool { - for _, di := range slice { - if di.Digest == descriptorInfo.Digest { - return true - } - } - - return false -} - func initCVESummaryFromCVEMap(cveMap map[string]cvemodel.CVE) cvemodel.ImageCVESummary { // Counters are initialized with 0 by default imageCVESummary := cvemodel.ImageCVESummary{ diff --git a/pkg/extensions/search/cve/pagination.go b/pkg/extensions/search/cve/pagination.go index 23dad926..fa9881e0 100644 --- a/pkg/extensions/search/cve/pagination.go +++ b/pkg/extensions/search/cve/pagination.go @@ -2,7 +2,8 @@ package cveinfo import ( "fmt" - "sort" + "slices" + "sync" zerr "zotregistry.dev/zot/v2/errors" "zotregistry.dev/zot/v2/pkg/common" @@ -15,34 +16,68 @@ const ( SeverityDsc = cvemodel.SortCriteria("SEVERITY") ) -func SortFunctions() map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE) func(i, j int) bool { - return map[cvemodel.SortCriteria]func(pageBuffer []cvemodel.CVE) func(i, j int) bool{ - AlphabeticAsc: SortByAlphabeticAsc, - AlphabeticDsc: SortByAlphabeticDsc, - SeverityDsc: SortBySeverity, - } -} +var ( + //nolint:gochecknoglobals // lazy initialization with sync.Once to avoid reallocation + sortFunctionsOnce sync.Once + //nolint:gochecknoglobals // cached map initialized once, effectively immutable + sortFunctions map[cvemodel.SortCriteria]func(a, b cvemodel.CVE) int +) -func SortByAlphabeticAsc(pageBuffer []cvemodel.CVE) func(i, j int) bool { - return func(i, j int) bool { - return pageBuffer[i].ID < pageBuffer[j].ID - } -} - -func SortByAlphabeticDsc(pageBuffer []cvemodel.CVE) func(i, j int) bool { - return func(i, j int) bool { - return pageBuffer[i].ID > pageBuffer[j].ID - } -} - -func SortBySeverity(pageBuffer []cvemodel.CVE) func(i, j int) bool { - return func(i, j int) bool { - if cvemodel.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) == 0 { - return pageBuffer[i].ID < pageBuffer[j].ID +// getSortFunctions returns a cached map of sort criteria to comparison functions. +// Using slices.SortFunc which expects func(a, b cvemodel.CVE) int. +// The map is initialized once using sync.Once to avoid reallocation. +func getSortFunctions() map[cvemodel.SortCriteria]func(a, b cvemodel.CVE) int { + sortFunctionsOnce.Do(func() { + sortFunctions = map[cvemodel.SortCriteria]func(a, b cvemodel.CVE) int{ + AlphabeticAsc: sortCVEByAlphabeticAsc, + AlphabeticDsc: sortCVEByAlphabeticDsc, + SeverityDsc: sortCVEBySeverityDsc, } + }) - return cvemodel.CompareSeverities(pageBuffer[i].Severity, pageBuffer[j].Severity) < 0 + return sortFunctions +} + +//nolint:varnamelen // standard comparison function signature +func sortCVEByAlphabeticAsc(a, b cvemodel.CVE) int { + if a.ID < b.ID { + return -1 } + if a.ID > b.ID { + return 1 + } + + return 0 +} + +//nolint:varnamelen // standard comparison function signature +func sortCVEByAlphabeticDsc(a, b cvemodel.CVE) int { + if a.ID > b.ID { + return -1 + } + if a.ID < b.ID { + return 1 + } + + return 0 +} + +//nolint:varnamelen // standard comparison function signature +func sortCVEBySeverityDsc(a, b cvemodel.CVE) int { + severityCmp := cvemodel.CompareSeverities(a.Severity, b.Severity) + if severityCmp != 0 { + return severityCmp + } + + // If severities are equal, sort by ID ascending + if a.ID < b.ID { + return -1 + } + if a.ID > b.ID { + return 1 + } + + return 0 } // PageFinder permits keeping a pool of objects using Add @@ -81,7 +116,8 @@ func NewCvePageFinder(limit, offset int, sortBy cvemodel.SortCriteria) (*CvePage return nil, zerr.ErrOffsetIsNegative } - if _, found := SortFunctions()[sortBy]; !found { + sortFuncs := getSortFunctions() + if _, found := sortFuncs[sortBy]; !found { return nil, fmt.Errorf("sorting CVEs by '%s' is not supported %w", sortBy, zerr.ErrSortCriteriaNotSupported) } @@ -89,12 +125,13 @@ func NewCvePageFinder(limit, offset int, sortBy cvemodel.SortCriteria) (*CvePage limit: limit, offset: offset, sortBy: sortBy, - pageBuffer: make([]cvemodel.CVE, 0, limit), + pageBuffer: make([]cvemodel.CVE, 0), }, nil } func (bpt *CvePageFinder) Reset() { - bpt.pageBuffer = []cvemodel.CVE{} + // Preserve capacity to avoid reallocation + bpt.pageBuffer = bpt.pageBuffer[:0] } func (bpt *CvePageFinder) Add(cve cvemodel.CVE) { @@ -108,7 +145,14 @@ func (bpt *CvePageFinder) Page() ([]cvemodel.CVE, common.PageInfo) { pageInfo := &common.PageInfo{} - sort.Slice(bpt.pageBuffer, SortFunctions()[bpt.sortBy](bpt.pageBuffer)) + // Use slices.SortFunc with cached comparison function + sortFuncs := getSortFunctions() + sortFn, ok := sortFuncs[bpt.sortBy] + if !ok { + // Fallback to default (should not happen due to validation in NewCvePageFinder) + sortFn = sortFuncs[SeverityDsc] + } + slices.SortFunc(bpt.pageBuffer, sortFn) // the offset and limit are calculated in terms of CVEs counted start := bpt.offset diff --git a/pkg/extensions/search/cve/pagination_test.go b/pkg/extensions/search/cve/pagination_test.go index a83e5774..7e2142a1 100644 --- a/pkg/extensions/search/cve/pagination_test.go +++ b/pkg/extensions/search/cve/pagination_test.go @@ -443,3 +443,63 @@ func TestCVEPagination(t *testing.T) { }) }) } + +func TestCvePageFinderUnit(t *testing.T) { + Convey("CvePageFinder unit tests", t, func() { + Convey("tie-breaking when IDs match", func() { + // Test tie-breaking for AlphabeticAsc - when IDs are equal, should return 0 + // and maintain stable order (first added stays first) + paginator, err := cveinfo.NewCvePageFinder(10, 0, cveinfo.AlphabeticAsc) + So(err, ShouldBeNil) + + // Add CVEs with same ID but different severities to verify stable sort + cve1 := cvemodel.CVE{ID: "CVE-2024-0001", Severity: "HIGH"} + cve2 := cvemodel.CVE{ID: "CVE-2024-0001", Severity: "MEDIUM"} + paginator.Add(cve1) + paginator.Add(cve2) + + result, _ := paginator.Page() + // When IDs are equal, sort is stable - first added (cve1) stays first + So(len(result), ShouldEqual, 2) + So(result[0].ID, ShouldEqual, "CVE-2024-0001") + So(result[0].Severity, ShouldEqual, "HIGH") // First added + So(result[1].ID, ShouldEqual, "CVE-2024-0001") + So(result[1].Severity, ShouldEqual, "MEDIUM") // Second added + + // Test tie-breaking for AlphabeticDsc - when IDs are equal, should return 0 + // and maintain stable order (first added stays first) + paginator, err = cveinfo.NewCvePageFinder(10, 0, cveinfo.AlphabeticDsc) + So(err, ShouldBeNil) + + paginator.Add(cve1) + paginator.Add(cve2) + + result, _ = paginator.Page() + // When IDs are equal, sort is stable - first added (cve1) stays first + So(len(result), ShouldEqual, 2) + So(result[0].ID, ShouldEqual, "CVE-2024-0001") + So(result[0].Severity, ShouldEqual, "HIGH") // First added + So(result[1].ID, ShouldEqual, "CVE-2024-0001") + So(result[1].Severity, ShouldEqual, "MEDIUM") // Second added + + // Test tie-breaking for SeverityDsc - when severities are equal, sort by ID ascending + paginator, err = cveinfo.NewCvePageFinder(10, 0, cveinfo.SeverityDsc) + So(err, ShouldBeNil) + + // Add CVEs with same severity but different IDs + cve3 := cvemodel.CVE{ID: "CVE-2024-0003", Severity: "HIGH"} + cve4 := cvemodel.CVE{ID: "CVE-2024-0001", Severity: "HIGH"} + cve5 := cvemodel.CVE{ID: "CVE-2024-0002", Severity: "HIGH"} + paginator.Add(cve3) + paginator.Add(cve4) + paginator.Add(cve5) + + result, _ = paginator.Page() + // All have same severity, so should be sorted by ID ascending + So(len(result), ShouldEqual, 3) + So(result[0].ID, ShouldEqual, "CVE-2024-0001") + So(result[1].ID, ShouldEqual, "CVE-2024-0002") + So(result[2].ID, ShouldEqual, "CVE-2024-0003") + }) + }) +} diff --git a/pkg/extensions/search/pagination/image_pagination.go b/pkg/extensions/search/pagination/image_pagination.go index e1febf39..b87615dd 100644 --- a/pkg/extensions/search/pagination/image_pagination.go +++ b/pkg/extensions/search/pagination/image_pagination.go @@ -2,14 +2,21 @@ package pagination import ( "fmt" - "sort" - "time" + "slices" + "sync" zerr "zotregistry.dev/zot/v2/errors" zcommon "zotregistry.dev/zot/v2/pkg/common" gql_gen "zotregistry.dev/zot/v2/pkg/extensions/search/gql_generated" ) +var ( + //nolint:gochecknoglobals // lazy initialization with sync.Once to avoid reallocation + imgSortFunctionsOnce sync.Once + //nolint:gochecknoglobals // cached map of static sort functions, effectively immutable + imgSortFunctions map[SortCriteria]func(a, b *gql_gen.ImageSummary) int +) + type ImageSummariesPageFinder struct { limit int offset int @@ -30,7 +37,8 @@ func NewImgSumPageFinder(limit, offset int, sortBy SortCriteria) (*ImageSummarie return nil, zerr.ErrOffsetIsNegative } - if _, found := ImgSumSortFuncs()[sortBy]; !found { + // Validate sortBy + if _, found := getImgSortFunctions()[sortBy]; !found { return nil, fmt.Errorf("sorting repos by '%s' is not supported %w", sortBy, zerr.ErrSortCriteriaNotSupported) } @@ -39,7 +47,7 @@ func NewImgSumPageFinder(limit, offset int, sortBy SortCriteria) (*ImageSummarie limit: limit, offset: offset, sortBy: sortBy, - pageBuffer: []*gql_gen.ImageSummary{}, + pageBuffer: make([]*gql_gen.ImageSummary, 0), }, nil } @@ -54,7 +62,7 @@ func (pf *ImageSummariesPageFinder) Page() ([]*gql_gen.ImageSummary, zcommon.Pag pageInfo := zcommon.PageInfo{} - sort.Slice(pf.pageBuffer, ImgSumSortFuncs()[pf.sortBy](pf.pageBuffer)) + slices.SortFunc(pf.pageBuffer, getImgSortFunctions()[pf.sortBy]) // the offset and limit are calculated in terms of repos counted start := pf.offset @@ -84,81 +92,109 @@ func (pf *ImageSummariesPageFinder) Page() ([]*gql_gen.ImageSummary, zcommon.Pag return page, pageInfo } -func ImgSumSortFuncs() map[SortCriteria]func(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - return map[SortCriteria]func(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool{ - AlphabeticAsc: ImgSortByAlphabeticAsc, - AlphabeticDsc: ImgSortByAlphabeticDsc, - UpdateTime: ImgSortByUpdateTime, - Relevance: ImgSortByRelevance, - Downloads: ImgSortByDownloads, - } +// getImgSortFunctions returns a cached map of sort functions. +func getImgSortFunctions() map[SortCriteria]func(a, b *gql_gen.ImageSummary) int { + imgSortFunctionsOnce.Do(func() { + imgSortFunctions = map[SortCriteria]func(a, b *gql_gen.ImageSummary) int{ + AlphabeticAsc: ImgSortByAlphabeticAsc, + AlphabeticDsc: ImgSortByAlphabeticDsc, + Relevance: ImgSortByRelevance, + UpdateTime: ImgSortByUpdateTime, + Downloads: ImgSortByDownloads, + } + }) + + return imgSortFunctions } -func ImgSortByAlphabeticAsc(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - return func(i, j int) bool { //nolint: varnamelen - if *pageBuffer[i].RepoName < *pageBuffer[j].RepoName { - return true - } - - if *pageBuffer[i].RepoName == *pageBuffer[j].RepoName { - return *pageBuffer[i].Tag < *pageBuffer[j].Tag - } - - return false +// ImgSortByAlphabeticAsc sorts alphabetically ascending. +func ImgSortByAlphabeticAsc(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // standard comparison func signature + if *a.RepoName < *b.RepoName { + return -1 } + + if *a.RepoName == *b.RepoName { + if *a.Tag < *b.Tag { + return -1 + } + if *a.Tag == *b.Tag { + return 0 + } + } + + return 1 } -func ImgSortByAlphabeticDsc(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - return func(i, j int) bool { //nolint: varnamelen - if *pageBuffer[i].RepoName > *pageBuffer[j].RepoName { - return true - } - - if *pageBuffer[i].RepoName == *pageBuffer[j].RepoName { - return *pageBuffer[i].Tag > *pageBuffer[j].Tag - } - - return false +// ImgSortByAlphabeticDsc sorts alphabetically descending. +func ImgSortByAlphabeticDsc(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // standard comparison func signature + if *a.RepoName > *b.RepoName { + return -1 } + + if *a.RepoName == *b.RepoName { + if *a.Tag > *b.Tag { + return -1 + } + if *a.Tag == *b.Tag { + return 0 + } + } + + return 1 } -func ImgSortByRelevance(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - return func(i, j int) bool { //nolint: varnamelen - if *pageBuffer[i].RepoName < *pageBuffer[j].RepoName { - return true - } - - if *pageBuffer[i].RepoName == *pageBuffer[j].RepoName { - return *pageBuffer[i].Tag < *pageBuffer[j].Tag - } - - return false +// ImgSortByRelevance sorts by relevance. +func ImgSortByRelevance(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // standard comparison func signature + if *a.RepoName < *b.RepoName { + return -1 } + + if *a.RepoName == *b.RepoName { + if *a.Tag < *b.Tag { + return -1 + } + if *a.Tag == *b.Tag { + return 0 + } + } + + return 1 } -// ImgSortByUpdateTime sorts descending by time. -func ImgSortByUpdateTime(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - repos2LastUpdated := map[string]time.Time{} - - for _, img := range pageBuffer { - lastUpdated, ok := repos2LastUpdated[*img.RepoName] - - if !ok || lastUpdated.Before(*img.LastUpdated) { - repos2LastUpdated[*img.RepoName] = *img.LastUpdated - } +// ImgSortByUpdateTime sorts descending by image update time. +func ImgSortByUpdateTime(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // standard comparison func signature + // Handle nil cases: nil values are treated as oldest (come last in descending sort) + if a.LastUpdated == nil && b.LastUpdated == nil { + return 0 } - return func(i, j int) bool { - iRepoTime, jRepoTime := repos2LastUpdated[*pageBuffer[i].RepoName], repos2LastUpdated[*pageBuffer[j].RepoName] - - return (iRepoTime.After(jRepoTime) || iRepoTime.Equal(jRepoTime)) && - pageBuffer[i].LastUpdated.After(*pageBuffer[j].LastUpdated) + if a.LastUpdated == nil { + return 1 // a is nil, b is not - a comes after b } + + if b.LastUpdated == nil { + return -1 // b is nil, a is not - a comes before b + } + + if a.LastUpdated.After(*b.LastUpdated) { + return -1 + } + + if a.LastUpdated.Equal(*b.LastUpdated) { + return 0 + } + + return 1 } // ImgSortByDownloads returns a comparison function for descendant sorting by downloads. -func ImgSortByDownloads(pageBuffer []*gql_gen.ImageSummary) func(i, j int) bool { - return func(i, j int) bool { - return *pageBuffer[i].DownloadCount > *pageBuffer[j].DownloadCount +func ImgSortByDownloads(a, b *gql_gen.ImageSummary) int { + if *a.DownloadCount > *b.DownloadCount { + return -1 } + if *a.DownloadCount == *b.DownloadCount { + return 0 + } + + return 1 } diff --git a/pkg/extensions/search/pagination/pagination_test.go b/pkg/extensions/search/pagination/pagination_test.go index 32b65fe2..78767112 100644 --- a/pkg/extensions/search/pagination/pagination_test.go +++ b/pkg/extensions/search/pagination/pagination_test.go @@ -27,35 +27,61 @@ func TestImgSumPagination(t *testing.T) { Convey("Sort Functions", t, func() { Convey("ImgSortByAlphabeticAsc", func() { // Case: repo1 is < repo2 - pageBuff := []*gql_generated.ImageSummary{ - {RepoName: ref("repo1:1")}, - {RepoName: ref("repo2:2")}, - } + image1 := &gql_generated.ImageSummary{RepoName: ref("repo1:1")} + image2 := &gql_generated.ImageSummary{RepoName: ref("repo2:2")} - sortFunc := pagination.ImgSortByAlphabeticAsc(pageBuff) - So(sortFunc(0, 1), ShouldBeTrue) + So(pagination.ImgSortByAlphabeticAsc(image1, image2), ShouldEqual, -1) }) Convey("ImgSortByAlphabeticDsc", func() { // Case: repo1 is < repo2 - pageBuff := []*gql_generated.ImageSummary{ - {RepoName: ref("repo1:1")}, - {RepoName: ref("repo2:2")}, - } + image1 := &gql_generated.ImageSummary{RepoName: ref("repo1:1")} + image2 := &gql_generated.ImageSummary{RepoName: ref("repo2:2")} - sortFunc := pagination.ImgSortByAlphabeticDsc(pageBuff) - So(sortFunc(0, 1), ShouldBeFalse) + So(pagination.ImgSortByAlphabeticDsc(image1, image2), ShouldEqual, 1) }) Convey("ImgSortByRelevance", func() { // Case: repo1 is < repo2 - pageBuff := []*gql_generated.ImageSummary{ - {RepoName: ref("repo1:1")}, - {RepoName: ref("repo2:2")}, - } + image1 := &gql_generated.ImageSummary{RepoName: ref("repo1:1")} + image2 := &gql_generated.ImageSummary{RepoName: ref("repo2:2")} - sortFunc := pagination.ImgSortByRelevance(pageBuff) - So(sortFunc(0, 1), ShouldBeTrue) + So(pagination.ImgSortByRelevance(image1, image2), ShouldEqual, -1) + }) + + Convey("ImgSortByUpdateTime with nil LastUpdated", func() { + time1 := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) + time2 := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + + // Both nil - should be equal + image1 := &gql_generated.ImageSummary{RepoName: ref("repo1"), Tag: ref("tag1"), LastUpdated: nil} + image2 := &gql_generated.ImageSummary{RepoName: ref("repo2"), Tag: ref("tag2"), LastUpdated: nil} + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 0) + + // a is nil, b is not - a should come after b (return 1) + image1.LastUpdated = nil + image2.LastUpdated = &time1 + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 1) + + // b is nil, a is not - a should come before b (return -1) + image1.LastUpdated = &time1 + image2.LastUpdated = nil + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, -1) + + // Both non-nil - normal comparison (a is newer, should come first in descending sort) + image1.LastUpdated = &time1 + image2.LastUpdated = &time2 + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, -1) + + // Both non-nil - normal comparison (b is newer, should come first in descending sort) + image1.LastUpdated = &time2 + image2.LastUpdated = &time1 + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 1) + + // Both non-nil and equal + image1.LastUpdated = &time1 + image2.LastUpdated = &time1 + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 0) }) }) } @@ -70,6 +96,8 @@ func TestRepoSumPagination(t *testing.T) { _, err = pagination.NewRepoSumPageFinder(0, 0, "unknown") So(err, ShouldNotBeNil) + // Verify it's the specific error for unsupported sort criteria + So(err.Error(), ShouldContainSubstring, "sorting repos by 'unknown' is not supported") }) } @@ -146,7 +174,7 @@ func TestPagination(t *testing.T) { &imgSum1, &imgSum2, &imgSum3, &imgSum4, }) - // ImgSortByUpdateTime + // ImgSortByUpdateTime (sorts by image LastUpdated descending) imagePageFinder, err = pagination.NewImgSumPageFinder(4, 0, pagination.UpdateTime) So(err, ShouldBeNil) imagePageFinder.Add(&imgSum1) @@ -154,8 +182,9 @@ func TestPagination(t *testing.T) { imagePageFinder.Add(&imgSum3) imagePageFinder.Add(&imgSum4) page, _ = imagePageFinder.Page() + // Expected order: imgSum2 (2020), imgSum4 (2012), imgSum3 (2011), imgSum1 (2010) So(page, ShouldEqual, []*gql_generated.ImageSummary{ - &imgSum2, &imgSum1, &imgSum4, &imgSum3, + &imgSum2, &imgSum4, &imgSum3, &imgSum1, }) // ImgSortByDownloads @@ -171,6 +200,148 @@ func TestPagination(t *testing.T) { }) }) + Convey("ImgSortByUpdateTime with nil LastUpdated in pagination", func() { + // Test pagination with images that have nil LastUpdated + imgSumWithTime := gql_generated.ImageSummary{ + RepoName: ref("repo1"), + Tag: ref("tag1"), + LastUpdated: ref(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), + } + imgSumNil1 := gql_generated.ImageSummary{ + RepoName: ref("repo2"), + Tag: ref("tag2"), + LastUpdated: nil, + } + imgSumNil2 := gql_generated.ImageSummary{ + RepoName: ref("repo3"), + Tag: ref("tag3"), + LastUpdated: nil, + } + imgSumOlder := gql_generated.ImageSummary{ + RepoName: ref("repo4"), + Tag: ref("tag4"), + LastUpdated: ref(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), + } + + imagePageFinder, err := pagination.NewImgSumPageFinder(4, 0, pagination.UpdateTime) + So(err, ShouldBeNil) + imagePageFinder.Add(&imgSumNil1) + imagePageFinder.Add(&imgSumWithTime) + imagePageFinder.Add(&imgSumNil2) + imagePageFinder.Add(&imgSumOlder) + page, _ := imagePageFinder.Page() + + // Expected order: imgSumWithTime (2020), imgSumOlder (2010), then nil values (imgSumNil1, imgSumNil2) + So(len(page), ShouldEqual, 4) + So(*page[0].RepoName, ShouldEqual, "repo1") // 2020 - newest + So(*page[1].RepoName, ShouldEqual, "repo4") // 2010 - older + // Nil values should come last + So(page[2].LastUpdated == nil || page[3].LastUpdated == nil, ShouldBeTrue) + }) + + Convey("Tie-breaking when values match", func() { + // Test tie-breaking for AlphabeticAsc - when RepoName and Tag are equal + imagePageFinder, err := pagination.NewImgSumPageFinder(10, 0, pagination.AlphabeticAsc) + So(err, ShouldBeNil) + + // Add images with same RepoName and Tag but different LastUpdated to verify stable sort + imgSum1 := gql_generated.ImageSummary{ + RepoName: ref("repo1"), + Tag: ref("tag1"), + LastUpdated: ref(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), + } + imgSum2 := gql_generated.ImageSummary{ + RepoName: ref("repo1"), + Tag: ref("tag1"), + LastUpdated: ref(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), + } + imagePageFinder.Add(&imgSum1) + imagePageFinder.Add(&imgSum2) + + page, _ := imagePageFinder.Page() + // When RepoName and Tag are equal, sort is stable - first added stays first + So(len(page), ShouldEqual, 2) + So(*page[0].RepoName, ShouldEqual, "repo1") + So(*page[0].Tag, ShouldEqual, "tag1") + So(page[0].LastUpdated.Equal(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // First added + So(*page[1].RepoName, ShouldEqual, "repo1") + So(*page[1].Tag, ShouldEqual, "tag1") + So(page[1].LastUpdated.Equal(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // Second added + + // Test tie-breaking for AlphabeticDsc + imagePageFinder, err = pagination.NewImgSumPageFinder(10, 0, pagination.AlphabeticDsc) + So(err, ShouldBeNil) + + imagePageFinder.Add(&imgSum1) + imagePageFinder.Add(&imgSum2) + + page, _ = imagePageFinder.Page() + // When RepoName and Tag are equal, sort is stable - first added stays first + So(len(page), ShouldEqual, 2) + So(page[0].LastUpdated.Equal(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // First added + So(page[1].LastUpdated.Equal(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // Second added + + // Test tie-breaking for Relevance + imagePageFinder, err = pagination.NewImgSumPageFinder(10, 0, pagination.Relevance) + So(err, ShouldBeNil) + + imagePageFinder.Add(&imgSum1) + imagePageFinder.Add(&imgSum2) + + page, _ = imagePageFinder.Page() + // When RepoName and Tag are equal, sort is stable - first added stays first + So(len(page), ShouldEqual, 2) + So(page[0].LastUpdated.Equal(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // First added + So(page[1].LastUpdated.Equal(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), ShouldBeTrue) // Second added + + // Test tie-breaking for UpdateTime - when LastUpdated times are equal + imagePageFinder, err = pagination.NewImgSumPageFinder(10, 0, pagination.UpdateTime) + So(err, ShouldBeNil) + + sameTime := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + imgSum3 := gql_generated.ImageSummary{ + RepoName: ref("repo1"), + Tag: ref("tag1"), + LastUpdated: ref(sameTime), + } + imgSum4 := gql_generated.ImageSummary{ + RepoName: ref("repo2"), + Tag: ref("tag2"), + LastUpdated: ref(sameTime), + } + imagePageFinder.Add(&imgSum3) + imagePageFinder.Add(&imgSum4) + + page, _ = imagePageFinder.Page() + // When LastUpdated times are equal, sort is stable - first added stays first + So(len(page), ShouldEqual, 2) + So(*page[0].RepoName, ShouldEqual, "repo1") // First added + So(*page[1].RepoName, ShouldEqual, "repo2") // Second added + + // Test tie-breaking for Downloads - when DownloadCount values are equal + imagePageFinder, err = pagination.NewImgSumPageFinder(10, 0, pagination.Downloads) + So(err, ShouldBeNil) + + imgSum5 := gql_generated.ImageSummary{ + RepoName: ref("repo1"), + Tag: ref("tag1"), + DownloadCount: ref(100), + } + imgSum6 := gql_generated.ImageSummary{ + RepoName: ref("repo2"), + Tag: ref("tag2"), + DownloadCount: ref(100), + } + imagePageFinder.Add(&imgSum5) + imagePageFinder.Add(&imgSum6) + + page, _ = imagePageFinder.Page() + // When DownloadCount values are equal, sort is stable - first added stays first + So(len(page), ShouldEqual, 2) + So(*page[0].RepoName, ShouldEqual, "repo1") // First added + So(*page[1].RepoName, ShouldEqual, "repo2") // Second added + }) + Convey("Errors", func() { imagePageFinder, err := pagination.NewImgSumPageFinder(2, 0, "") So(err, ShouldBeNil) @@ -182,13 +353,83 @@ func TestPagination(t *testing.T) { _, err = pagination.NewImgSumPageFinder(1, -1, "") So(err, ShouldNotBeNil) - _, err = pagination.NewImgSumPageFinder(1, -1, "bad sort func") + // Test invalid sortBy with valid limit and offset to ensure it reaches the sortBy validation + _, err = pagination.NewImgSumPageFinder(1, 0, "bad sort func") So(err, ShouldNotBeNil) + // Verify it's the specific error for unsupported sort criteria + So(err.Error(), ShouldContainSubstring, "sorting repos by 'bad sort func' is not supported") }) }) Convey("Repos Pagination", t, func() { - Convey("Sort functions", func() { + Convey("Sort Functions unit tests", func() { + Convey("RepoSortByAlphabeticAsc", func() { + // Case: repo1 is < repo2 + repo1 := &gql_generated.RepoSummary{Name: ref("repo1")} + repo2 := &gql_generated.RepoSummary{Name: ref("repo2")} + + So(pagination.RepoSortByAlphabeticAsc(repo1, repo2), ShouldEqual, -1) + So(pagination.RepoSortByAlphabeticAsc(repo2, repo1), ShouldEqual, 1) + So(pagination.RepoSortByAlphabeticAsc(repo1, repo1), ShouldEqual, 0) + }) + + Convey("RepoSortByAlphabeticDsc", func() { + // Case: repo1 is < repo2, so descending should return 1 + repo1 := &gql_generated.RepoSummary{Name: ref("repo1")} + repo2 := &gql_generated.RepoSummary{Name: ref("repo2")} + + So(pagination.RepoSortByAlphabeticDsc(repo1, repo2), ShouldEqual, 1) + So(pagination.RepoSortByAlphabeticDsc(repo2, repo1), ShouldEqual, -1) + So(pagination.RepoSortByAlphabeticDsc(repo1, repo1), ShouldEqual, 0) + }) + + Convey("RepoSortByDownloads", func() { + // Case: repo1 has more downloads than repo2, so should return -1 (descending) + repo1 := &gql_generated.RepoSummary{DownloadCount: ref(100)} + repo2 := &gql_generated.RepoSummary{DownloadCount: ref(50)} + + So(pagination.RepoSortByDownloads(repo1, repo2), ShouldEqual, -1) + So(pagination.RepoSortByDownloads(repo2, repo1), ShouldEqual, 1) + So(pagination.RepoSortByDownloads(repo1, repo1), ShouldEqual, 0) + }) + + Convey("RepoSortByUpdateTime with nil LastUpdated", func() { + time1 := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) + time2 := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + + // Both nil - should be equal + repo1 := &gql_generated.RepoSummary{Name: ref("repo1"), LastUpdated: nil} + repo2 := &gql_generated.RepoSummary{Name: ref("repo2"), LastUpdated: nil} + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 0) + + // a is nil, b is not - a should come after b (return 1) + repo1.LastUpdated = nil + repo2.LastUpdated = &time1 + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 1) + + // b is nil, a is not - a should come before b (return -1) + repo1.LastUpdated = &time1 + repo2.LastUpdated = nil + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, -1) + + // Both non-nil - normal comparison (a is newer, should come first in descending sort) + repo1.LastUpdated = &time1 + repo2.LastUpdated = &time2 + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, -1) + + // Both non-nil - normal comparison (b is newer, should come first in descending sort) + repo1.LastUpdated = &time2 + repo2.LastUpdated = &time1 + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 1) + + // Both non-nil and equal + repo1.LastUpdated = &time1 + repo2.LastUpdated = &time1 + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 0) + }) + }) + + Convey("Repo page finder tests", func() { repoSum1 := gql_generated.RepoSummary{ Name: ref("1"), LastUpdated: ref(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), @@ -278,7 +519,42 @@ func TestPagination(t *testing.T) { }) }) - Convey("Errors", func() { + Convey("RepoSortByUpdateTime with nil LastUpdated in pagination", func() { + // Test pagination with repos that have nil LastUpdated + repoSumWithTime := gql_generated.RepoSummary{ + Name: ref("repo1"), + LastUpdated: ref(time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)), + } + repoSumNil1 := gql_generated.RepoSummary{ + Name: ref("repo2"), + LastUpdated: nil, + } + repoSumNil2 := gql_generated.RepoSummary{ + Name: ref("repo3"), + LastUpdated: nil, + } + repoSumOlder := gql_generated.RepoSummary{ + Name: ref("repo4"), + LastUpdated: ref(time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)), + } + + repoPageFinder, err := pagination.NewRepoSumPageFinder(4, 0, pagination.UpdateTime) + So(err, ShouldBeNil) + repoPageFinder.Add(&repoSumNil1) + repoPageFinder.Add(&repoSumWithTime) + repoPageFinder.Add(&repoSumNil2) + repoPageFinder.Add(&repoSumOlder) + page, _ := repoPageFinder.Page() + + // Expected order: repoSumWithTime (2020), repoSumOlder (2010), then nil values (repoSumNil1, repoSumNil2) + So(len(page), ShouldEqual, 4) + So(*page[0].Name, ShouldEqual, "repo1") // 2020 - newest + So(*page[1].Name, ShouldEqual, "repo4") // 2010 - older + // Nil values should come last + So(page[2].LastUpdated == nil || page[3].LastUpdated == nil, ShouldBeTrue) + }) + + Convey("Repo page finder error tests", func() { repoPageFinder, err := pagination.NewRepoSumPageFinder(2, 0, "") So(err, ShouldBeNil) So(repoPageFinder, ShouldNotBeNil) @@ -289,8 +565,11 @@ func TestPagination(t *testing.T) { _, err = pagination.NewRepoSumPageFinder(1, -1, "") So(err, ShouldNotBeNil) - _, err = pagination.NewRepoSumPageFinder(1, -1, "bad sort func") + // Test invalid sortBy with valid limit and offset to ensure it reaches the sortBy validation + _, err = pagination.NewRepoSumPageFinder(1, 0, "bad sort func") So(err, ShouldNotBeNil) + // Verify it's the specific error for unsupported sort criteria + So(err.Error(), ShouldContainSubstring, "sorting repos by 'bad sort func' is not supported") }) }) } diff --git a/pkg/extensions/search/pagination/repo_pagination.go b/pkg/extensions/search/pagination/repo_pagination.go index 991ba5d8..5ec2ae5d 100644 --- a/pkg/extensions/search/pagination/repo_pagination.go +++ b/pkg/extensions/search/pagination/repo_pagination.go @@ -2,13 +2,21 @@ package pagination import ( "fmt" - "sort" + "slices" + "sync" zerr "zotregistry.dev/zot/v2/errors" zcommon "zotregistry.dev/zot/v2/pkg/common" gql_gen "zotregistry.dev/zot/v2/pkg/extensions/search/gql_generated" ) +var ( + //nolint:gochecknoglobals // lazy initialization with sync.Once to avoid reallocation + repoSortFunctionsOnce sync.Once + //nolint:gochecknoglobals // cached map initialized once, effectively immutable + repoSortFunctions map[SortCriteria]func(a, b *gql_gen.RepoSummary) int +) + type RepoSummariesPageFinder struct { limit int offset int @@ -29,7 +37,8 @@ func NewRepoSumPageFinder(limit, offset int, sortBy SortCriteria) (*RepoSummarie return nil, zerr.ErrOffsetIsNegative } - if _, found := RepoSumSortFuncs()[sortBy]; !found { + sortFuncs := getRepoSortFunctions() + if _, found := sortFuncs[sortBy]; !found { return nil, fmt.Errorf("sorting repos by '%s' is not supported %w", sortBy, zerr.ErrSortCriteriaNotSupported) } @@ -38,7 +47,7 @@ func NewRepoSumPageFinder(limit, offset int, sortBy SortCriteria) (*RepoSummarie limit: limit, offset: offset, sortBy: sortBy, - pageBuffer: []*gql_gen.RepoSummary{}, + pageBuffer: make([]*gql_gen.RepoSummary, 0), }, nil } @@ -53,7 +62,13 @@ func (pf *RepoSummariesPageFinder) Page() ([]*gql_gen.RepoSummary, zcommon.PageI pageInfo := zcommon.PageInfo{} - sort.Slice(pf.pageBuffer, RepoSumSortFuncs()[pf.sortBy](pf.pageBuffer)) + sortFuncs := getRepoSortFunctions() + sortFn, ok := sortFuncs[pf.sortBy] + if !ok { + // Fallback to default (should not happen due to validation in NewRepoSumPageFinder) + sortFn = sortFuncs[AlphabeticAsc] + } + slices.SortFunc(pf.pageBuffer, sortFn) // the offset and limit are calculated in terms of repos counted start := pf.offset @@ -83,44 +98,92 @@ func (pf *RepoSummariesPageFinder) Page() ([]*gql_gen.RepoSummary, zcommon.PageI return page, pageInfo } -func RepoSumSortFuncs() map[SortCriteria]func(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return map[SortCriteria]func(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool{ - AlphabeticAsc: RepoSortByAlphabeticAsc, - AlphabeticDsc: RepoSortByAlphabeticDsc, - Relevance: RepoSortByRelevance, - UpdateTime: RepoSortByUpdateTime, - Downloads: RepoSortByDownloads, - } +// getRepoSortFunctions returns a cached map of sort functions. +// The map is initialized once using sync.Once to avoid reallocation. +func getRepoSortFunctions() map[SortCriteria]func(a, b *gql_gen.RepoSummary) int { + repoSortFunctionsOnce.Do(func() { + repoSortFunctions = map[SortCriteria]func(a, b *gql_gen.RepoSummary) int{ + AlphabeticAsc: RepoSortByAlphabeticAsc, + AlphabeticDsc: RepoSortByAlphabeticDsc, + Relevance: RepoSortByRelevance, + UpdateTime: RepoSortByUpdateTime, + Downloads: RepoSortByDownloads, + } + }) + + return repoSortFunctions } -func RepoSortByAlphabeticAsc(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return func(i, j int) bool { - return *pageBuffer[i].Name < *pageBuffer[j].Name +// RepoSortByAlphabeticAsc sorts alphabetically ascending. +func RepoSortByAlphabeticAsc(a, b *gql_gen.RepoSummary) int { + if *a.Name < *b.Name { + return -1 } + if *a.Name == *b.Name { + return 0 + } + + return 1 } -func RepoSortByAlphabeticDsc(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return func(i, j int) bool { - return *pageBuffer[i].Name > *pageBuffer[j].Name +// RepoSortByAlphabeticDsc sorts alphabetically descending. +func RepoSortByAlphabeticDsc(a, b *gql_gen.RepoSummary) int { + if *a.Name > *b.Name { + return -1 } + if *a.Name == *b.Name { + return 0 + } + + return 1 } -func RepoSortByRelevance(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return func(i, j int) bool { - return *pageBuffer[i].Rank < *pageBuffer[j].Rank +// RepoSortByRelevance sorts by relevance. +func RepoSortByRelevance(a, b *gql_gen.RepoSummary) int { + if *a.Rank < *b.Rank { + return -1 } + if *a.Rank == *b.Rank { + return 0 + } + + return 1 } // RepoSortByUpdateTime sorts descending by time. -func RepoSortByUpdateTime(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return func(i, j int) bool { - return pageBuffer[i].LastUpdated.After(*pageBuffer[j].LastUpdated) +func RepoSortByUpdateTime(a, b *gql_gen.RepoSummary) int { //nolint:varnamelen // standard comparison func signature + // Handle nil cases: nil values are treated as oldest (come last in descending sort) + if a.LastUpdated == nil && b.LastUpdated == nil { + return 0 } + + if a.LastUpdated == nil { + return 1 // a is nil, b is not - a comes after b + } + + if b.LastUpdated == nil { + return -1 // b is nil, a is not - a comes before b + } + + if a.LastUpdated.After(*b.LastUpdated) { + return -1 + } + + if a.LastUpdated.Equal(*b.LastUpdated) { + return 0 + } + + return 1 } // RepoSortByDownloads returns a comparison function for descendant sorting by downloads. -func RepoSortByDownloads(pageBuffer []*gql_gen.RepoSummary) func(i, j int) bool { - return func(i, j int) bool { - return *pageBuffer[i].DownloadCount > *pageBuffer[j].DownloadCount +func RepoSortByDownloads(a, b *gql_gen.RepoSummary) int { + if *a.DownloadCount > *b.DownloadCount { + return -1 } + if *a.DownloadCount == *b.DownloadCount { + return 0 + } + + return 1 } diff --git a/pkg/extensions/search/resolver.go b/pkg/extensions/search/resolver.go index 22f077bf..4719007a 100644 --- a/pkg/extensions/search/resolver.go +++ b/pkg/extensions/search/resolver.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "slices" - "sort" "strings" godigest "github.com/opencontainers/go-digest" @@ -102,13 +101,9 @@ func FilterByDigest(digest string) mTypes.FilterFunc { // Check to see if the individual layers in the oci image manifest match the digest // These are blobs with mediaType application/vnd.oci.image.layer.v1.tar+gzip - for _, layer := range manifest.Manifest.Layers { - if strings.Contains(layer.Digest.String(), lookupDigest) { - return true - } - } - - return false + return slices.ContainsFunc(manifest.Manifest.Layers, func(layer ispec.Descriptor) bool { + return strings.Contains(layer.Digest.String(), lookupDigest) + }) } } @@ -1113,45 +1108,43 @@ func derivedImageList(ctx context.Context, image string, digest *string, metaDB }, nil } -func filterDerivedImages(image *gql_generated.ImageSummary) mTypes.FilterFunc { +func filterDerivedImages(baseImage *gql_generated.ImageSummary) mTypes.FilterFunc { return func(repoMeta mTypes.RepoMeta, imageMeta mTypes.ImageMeta) bool { - var addImageToList bool + if len(imageMeta.Manifests) == 0 { + return false + } - imageManifest := imageMeta.Manifests[0] + derivedManifest := imageMeta.Manifests[0] - for i := range image.Manifests { - manifestDigest := imageManifest.Digest.String() - if manifestDigest == *image.Manifests[i].Digest { + return slices.ContainsFunc(baseImage.Manifests, func(baseManifest *gql_generated.ManifestSummary) bool { + if baseManifest == nil || baseManifest.Digest == nil { return false } - imageLayers := image.Manifests[i].Layers + derivedDigest := derivedManifest.Digest.String() + if derivedDigest == *baseManifest.Digest { + return false + } - addImageToList = false - layers := imageManifest.Manifest.Layers + baseLayers := baseManifest.Layers + derivedLayers := derivedManifest.Manifest.Layers - sameLayer := 0 + // Check if 'baseLayers' is a prefix of 'derivedLayers'. + // This means ALL layers of 'baseLayers' must be present at the start of 'derivedLayers'. + if len(baseLayers) == 0 || len(baseLayers) > len(derivedLayers) { + return false + } - for _, l := range imageLayers { - for _, k := range layers { - if k.Digest.String() == *l.Digest { - sameLayer++ + return slices.EqualFunc(baseLayers, derivedLayers[:len(baseLayers)], + func(l *gql_generated.LayerSummary, k ispec.Descriptor) bool { + if l == nil || l.Digest == nil { + return false } - } - } - // if all layers are the same - if sameLayer == len(imageLayers) { - // it's a derived image - addImageToList = true - } - - if addImageToList { - return true - } - } - - return false + return *l.Digest == k.Digest.String() + }, + ) + }) } } @@ -1215,44 +1208,43 @@ func baseImageList(ctx context.Context, image string, digest *string, metaDB mTy }, nil } -func filterBaseImages(image *gql_generated.ImageSummary) mTypes.FilterFunc { +func filterBaseImages(derivedImage *gql_generated.ImageSummary) mTypes.FilterFunc { return func(repoMeta mTypes.RepoMeta, imageMeta mTypes.ImageMeta) bool { - var addImageToList bool + if len(imageMeta.Manifests) == 0 { + return false + } - manifest := imageMeta.Manifests[0] + baseManifest := imageMeta.Manifests[0] - for i := range image.Manifests { - manifestDigest := manifest.Digest.String() - if manifestDigest == *image.Manifests[i].Digest { + return slices.ContainsFunc(derivedImage.Manifests, func(derivedManifest *gql_generated.ManifestSummary) bool { + if derivedManifest == nil || derivedManifest.Digest == nil { return false } - addImageToList = true + baseDigest := baseManifest.Digest.String() + if baseDigest == *derivedManifest.Digest { + return false + } - for _, l := range manifest.Manifest.Layers { - foundLayer := false + // Check if 'baseManifest' is a base image for 'derivedManifest'. + // This means ALL layers of 'baseManifest' must be present in 'derivedManifest' as a prefix. + baseLayers := baseManifest.Manifest.Layers + derivedLayers := derivedManifest.Layers - for _, k := range image.Manifests[i].Layers { - if l.Digest.String() == *k.Digest { - foundLayer = true + if len(baseLayers) == 0 || len(baseLayers) > len(derivedLayers) { + return false + } - break + return slices.EqualFunc(baseLayers, derivedLayers[:len(baseLayers)], + func(l ispec.Descriptor, k *gql_generated.LayerSummary) bool { + if k == nil || k.Digest == nil { + return false } - } - if !foundLayer { - addImageToList = false - - break - } - } - - if addImageToList { - return true - } - } - - return false + return l.Digest.String() == *k.Digest + }, + ) + }) } } @@ -1377,7 +1369,9 @@ func cleanFilter(filter *gql_generated.Filter) *gql_generated.Filter { *filter.Arch[i] = strings.TrimSpace(*filter.Arch[i]) } - filter.Arch = deleteEmptyElements(filter.Arch) + filter.Arch = slices.DeleteFunc(filter.Arch, func(s *string) bool { + return *s == "" + }) } if filter.Os != nil { @@ -1386,36 +1380,14 @@ func cleanFilter(filter *gql_generated.Filter) *gql_generated.Filter { *filter.Os[i] = strings.TrimSpace(*filter.Os[i]) } - filter.Os = deleteEmptyElements(filter.Os) + filter.Os = slices.DeleteFunc(filter.Os, func(s *string) bool { + return *s == "" + }) } return filter } -func deleteEmptyElements(slice []*string) []*string { - i := 0 - for i < len(slice) { - if elementIsEmpty(*slice[i]) { - slice = deleteElementAt(slice, i) - } else { - i++ - } - } - - return slice -} - -func elementIsEmpty(s string) bool { - return s == "" -} - -func deleteElementAt(slice []*string, i int) []*string { - slice[i] = slice[len(slice)-1] - slice = slice[:len(slice)-1] - - return slice -} - func expandedRepoInfo(ctx context.Context, repo string, metaDB mTypes.MetaDB, cveInfo cveinfo.CveInfo, log log.Logger, ) (*gql_generated.RepoInfo, error) { if ok, err := reqCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { @@ -1463,30 +1435,40 @@ func expandedRepoInfo(ctx context.Context, repo string, metaDB mTypes.MetaDB, cv repoSummary, imageSummaries := convert.RepoMeta2ExpandedRepoInfo(ctx, repoMeta, imageMetaMap, skip, cveInfo, log) - dateSortedImages := make(timeSlice, 0, len(imageSummaries)) + dateSortedImages := make([]*gql_generated.ImageSummary, 0, len(imageSummaries)) for _, imgSummary := range imageSummaries { dateSortedImages = append(dateSortedImages, imgSummary) } - sort.Sort(dateSortedImages) + //nolint:varnamelen // standard comparison func signature + slices.SortFunc(dateSortedImages, func(a, b *gql_generated.ImageSummary) int { + // Handle nil cases: nil values are treated as oldest (come last in descending sort) + if a.LastUpdated == nil && b.LastUpdated == nil { + return 0 + } + + if a.LastUpdated == nil { + return 1 // a is nil, b is not - a comes after b + } + + if b.LastUpdated == nil { + return -1 // b is nil, a is not - a comes before b + } + + if a.LastUpdated.After(*b.LastUpdated) { + return -1 + } + + if a.LastUpdated.Equal(*b.LastUpdated) { + return 0 + } + + return 1 + }) return &gql_generated.RepoInfo{Summary: repoSummary, Images: dateSortedImages}, nil } -type timeSlice []*gql_generated.ImageSummary - -func (p timeSlice) Len() int { - return len(p) -} - -func (p timeSlice) Less(i, j int) bool { - return p[i].LastUpdated.After(*p[j].LastUpdated) -} - -func (p timeSlice) Swap(i, j int) { - p[i], p[j] = p[j], p[i] -} - // dderef is a default deref. func dderef[T any](pointer *T) T { var defValue T diff --git a/pkg/extensions/search/resolver_test.go b/pkg/extensions/search/resolver_test.go index 66eb6364..041a480e 100644 --- a/pkg/extensions/search/resolver_test.go +++ b/pkg/extensions/search/resolver_test.go @@ -6,7 +6,6 @@ import ( "context" "errors" "fmt" - "sort" "testing" "time" @@ -2396,6 +2395,144 @@ func TestMockedDerivedImageList(t *testing.T) { So(images.Results, ShouldNotBeEmpty) So(len(images.Results), ShouldEqual, 1) }) + + Convey("filterDerivedImages edge cases", func() { + // Create base image with layers + baseLayer1 := []byte{10, 11, 10, 11} + baseLayer2 := []byte{11, 11, 11, 11} + baseImage := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2}). + DefaultConfig().Build() + + baseImageMeta := baseImage.AsImageMeta() + + // Create base ImageSummary for filterDerivedImages + baseImageSummary := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(baseImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + }, + }, + }, + } + + // Test case 1: imageMeta with empty Manifests + emptyManifestsMeta := mTypes.ImageMeta{ + Manifests: []mTypes.ManifestMeta{}, + } + filterFunc := filterDerivedImages(baseImageSummary) + result := filterFunc(mTypes.RepoMeta{}, emptyManifestsMeta) + So(result, ShouldBeFalse) + + // Test case 2: baseImage with nil manifest + nilManifestBaseImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{nil}, + } + filterFunc = filterDerivedImages(nilManifestBaseImage) + derivedMeta := baseImageMeta + result = filterFunc(mTypes.RepoMeta{}, derivedMeta) + So(result, ShouldBeFalse) + + // Test case 3: baseImage with manifest but nil Digest + nilDigestBaseImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: nil, + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + }, + }, + }, + } + filterFunc = filterDerivedImages(nilDigestBaseImage) + result = filterFunc(mTypes.RepoMeta{}, derivedMeta) + So(result, ShouldBeFalse) + + // Test case 4: same image (derivedDigest == baseManifest.Digest) - not derived + sameImageMeta := baseImageMeta + filterFunc = filterDerivedImages(baseImageSummary) + result = filterFunc(mTypes.RepoMeta{}, sameImageMeta) + So(result, ShouldBeFalse) + + // Test case 5: baseImage with no layers (len(baseLayers) == 0) + noLayersBaseImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(baseImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{}, + }, + }, + } + derivedLayer := []byte{20, 21, 22, 23} + derivedImg := CreateImageWith(). + LayerBlobs([][]byte{derivedLayer}). + DefaultConfig().Build() + derivedImgMeta := derivedImg.AsImageMeta() + filterFunc = filterDerivedImages(noLayersBaseImage) + result = filterFunc(mTypes.RepoMeta{}, derivedImgMeta) + So(result, ShouldBeFalse) + + // Test case 6: baseLayers longer than derivedLayers + longBaseLayersImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(baseImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + {Digest: ref(godigest.FromBytes([]byte{99, 99, 99}).String())}, + }, + }, + }, + } + shortDerivedImg := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1}). + DefaultConfig().Build() + shortDerivedMeta := shortDerivedImg.AsImageMeta() + filterFunc = filterDerivedImages(longBaseLayersImage) + result = filterFunc(mTypes.RepoMeta{}, shortDerivedMeta) + So(result, ShouldBeFalse) + + // Test case 7: nil layer in baseLayers + nilLayerBaseImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(baseImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + nil, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + }, + }, + }, + } + // Create derived image that would match if layer wasn't nil + matchingDerivedImg := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2, {99}}). + DefaultConfig().Build() + matchingDerivedMeta := matchingDerivedImg.AsImageMeta() + filterFunc = filterDerivedImages(nilLayerBaseImage) + result = filterFunc(mTypes.RepoMeta{}, matchingDerivedMeta) + So(result, ShouldBeFalse) + + // Test case 8: layer with nil Digest in baseLayers + nilDigestLayerBaseImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(baseImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: nil}, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + }, + }, + }, + } + filterFunc = filterDerivedImages(nilDigestLayerBaseImage) + result = filterFunc(mTypes.RepoMeta{}, matchingDerivedMeta) + So(result, ShouldBeFalse) + }) }) } @@ -2646,6 +2783,139 @@ func TestMockedBaseImageList(t *testing.T) { So(err, ShouldBeNil) So(images.Results, ShouldBeEmpty) }) + + Convey("filterBaseImages edge cases", t, func() { + // Create base and derived images with layers + baseLayer1 := []byte{10, 11, 10, 11} + baseLayer2 := []byte{11, 11, 11, 11} + baseImage := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2}). + DefaultConfig().Build() + + derivedLayer3 := []byte{20, 21, 22, 23} + derivedImage := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2, derivedLayer3}). + DefaultConfig().Build() + + baseImageMeta := baseImage.AsImageMeta() + derivedImageMeta := derivedImage.AsImageMeta() + + // Create derived ImageSummary for filterBaseImages + derivedImageSummary := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(derivedImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + {Digest: ref(godigest.FromBytes(derivedLayer3).String())}, + }, + }, + }, + } + + // Test case 1: imageMeta with empty Manifests + emptyManifestsMeta := mTypes.ImageMeta{ + Manifests: []mTypes.ManifestMeta{}, + } + filterFunc := filterBaseImages(derivedImageSummary) + result := filterFunc(mTypes.RepoMeta{}, emptyManifestsMeta) + So(result, ShouldBeFalse) + + // Test case 2: derivedImage with nil manifest + nilManifestDerivedImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{nil}, + } + filterFunc = filterBaseImages(nilManifestDerivedImage) + result = filterFunc(mTypes.RepoMeta{}, baseImageMeta) + So(result, ShouldBeFalse) + + // Test case 3: derivedImage with manifest but nil Digest + nilDigestDerivedImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: nil, + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + }, + }, + }, + } + filterFunc = filterBaseImages(nilDigestDerivedImage) + result = filterFunc(mTypes.RepoMeta{}, baseImageMeta) + So(result, ShouldBeFalse) + + // Test case 4: same image (baseDigest == derivedManifest.Digest) - not a base + sameImageMeta := derivedImageMeta + filterFunc = filterBaseImages(derivedImageSummary) + result = filterFunc(mTypes.RepoMeta{}, sameImageMeta) + So(result, ShouldBeFalse) + + // Test case 5: baseImage with no layers (len(baseLayers) == 0) + noLayersBaseImg := CreateImageWith(). + LayerBlobs([][]byte{}). + DefaultConfig().Build() + noLayersBaseMeta := noLayersBaseImg.AsImageMeta() + filterFunc = filterBaseImages(derivedImageSummary) + result = filterFunc(mTypes.RepoMeta{}, noLayersBaseMeta) + So(result, ShouldBeFalse) + + // Test case 6: baseLayers longer than derivedLayers + longBaseImg := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2, {99, 99, 99}}). + DefaultConfig().Build() + longBaseMeta := longBaseImg.AsImageMeta() + shortDerivedImageSummary := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(derivedImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: ref(godigest.FromBytes(baseLayer1).String())}, + }, + }, + }, + } + filterFunc = filterBaseImages(shortDerivedImageSummary) + result = filterFunc(mTypes.RepoMeta{}, longBaseMeta) + So(result, ShouldBeFalse) + + // Test case 7: nil layer in derivedLayers + nilLayerDerivedImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(derivedImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + nil, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + }, + }, + }, + } + // Create base image that would match if layer wasn't nil + matchingBaseImg := CreateImageWith(). + LayerBlobs([][]byte{baseLayer1, baseLayer2}). + DefaultConfig().Build() + matchingBaseMeta := matchingBaseImg.AsImageMeta() + filterFunc = filterBaseImages(nilLayerDerivedImage) + result = filterFunc(mTypes.RepoMeta{}, matchingBaseMeta) + So(result, ShouldBeFalse) + + // Test case 8: layer with nil Digest in derivedLayers + nilDigestLayerDerivedImage := &gql_generated.ImageSummary{ + Manifests: []*gql_generated.ManifestSummary{ + { + Digest: ref(derivedImage.DigestStr()), + Layers: []*gql_generated.LayerSummary{ + {Digest: nil}, + {Digest: ref(godigest.FromBytes(baseLayer2).String())}, + }, + }, + }, + } + filterFunc = filterBaseImages(nilDigestLayerDerivedImage) + result = filterFunc(mTypes.RepoMeta{}, matchingBaseMeta) + So(result, ShouldBeFalse) + }) } func TestExpandedRepoInfoErrors(t *testing.T) { @@ -2668,135 +2938,6 @@ func TestExpandedRepoInfoErrors(t *testing.T) { }) } -func TestImageSummarySort(t *testing.T) { - Convey("Test sorting ImageSummary", t, func() { - Convey("Swap elements at valid indices", func() { - // Create test data with different timestamps - time1 := time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC) - time2 := time.Date(2023, 1, 2, 12, 0, 0, 0, time.UTC) - time3 := time.Date(2023, 1, 3, 12, 0, 0, 0, time.UTC) - - image1 := &gql_generated.ImageSummary{ - Tag: ref("tag1"), - LastUpdated: &time1, - } - image2 := &gql_generated.ImageSummary{ - Tag: ref("tag2"), - LastUpdated: &time2, - } - image3 := &gql_generated.ImageSummary{ - Tag: ref("tag3"), - LastUpdated: &time3, - } - - // Create timeSlice with test data - timeSliceData := timeSlice{image1, image2, image3} - - // Verify initial order - So(*timeSliceData[0].Tag, ShouldEqual, "tag1") - So(*timeSliceData[1].Tag, ShouldEqual, "tag2") - So(*timeSliceData[2].Tag, ShouldEqual, "tag3") - - // Swap elements at indices 0 and 2 - timeSliceData.Swap(0, 2) - - // Verify elements are swapped - So(*timeSliceData[0].Tag, ShouldEqual, "tag3") - So(*timeSliceData[1].Tag, ShouldEqual, "tag2") - So(*timeSliceData[2].Tag, ShouldEqual, "tag1") - - // Swap elements at indices 1 and 2 - timeSliceData.Swap(1, 2) - - // Verify elements are swapped again - So(*timeSliceData[0].Tag, ShouldEqual, "tag3") - So(*timeSliceData[1].Tag, ShouldEqual, "tag1") - So(*timeSliceData[2].Tag, ShouldEqual, "tag2") - }) - - Convey("Swap elements at same index (no-op)", func() { - time1 := time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC) - time2 := time.Date(2023, 1, 2, 12, 0, 0, 0, time.UTC) - - image1 := &gql_generated.ImageSummary{ - Tag: ref("tag1"), - LastUpdated: &time1, - } - image2 := &gql_generated.ImageSummary{ - Tag: ref("tag2"), - LastUpdated: &time2, - } - - timeSliceData := timeSlice{image1, image2} - - // Swap element with itself - timeSliceData.Swap(0, 0) - - // Verify no change - So(*timeSliceData[0].Tag, ShouldEqual, "tag1") - So(*timeSliceData[1].Tag, ShouldEqual, "tag2") - }) - - Convey("Swap with single element slice", func() { - time1 := time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC) - - image1 := &gql_generated.ImageSummary{ - Tag: ref("tag1"), - LastUpdated: &time1, - } - - timeSliceData := timeSlice{image1} - - // Swap single element with itself - timeSliceData.Swap(0, 0) - - // Verify no change - So(*timeSliceData[0].Tag, ShouldEqual, "tag1") - }) - - Convey("Swap with empty slice", func() { - timeSliceData := timeSlice{} - - // Verify slice is empty - // Note: Calling Swap on empty slice would panic, which is expected behavior - // The Swap method doesn't check bounds, so it's the caller's responsibility - // to ensure valid indices. This is consistent with Go's standard library behavior. - So(len(timeSliceData), ShouldEqual, 0) - }) - - Convey("Integration test with sort.Sort", func() { - // Create test data with unsorted timestamps - time1 := time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC) - time2 := time.Date(2023, 1, 3, 12, 0, 0, 0, time.UTC) // Latest - time3 := time.Date(2023, 1, 2, 12, 0, 0, 0, time.UTC) - - image1 := &gql_generated.ImageSummary{ - Tag: ref("tag1"), - LastUpdated: &time1, - } - image2 := &gql_generated.ImageSummary{ - Tag: ref("tag2"), - LastUpdated: &time2, - } - image3 := &gql_generated.ImageSummary{ - Tag: ref("tag3"), - LastUpdated: &time3, - } - - // Create timeSlice with unsorted data - timeSliceData := timeSlice{image1, image2, image3} - - // Sort using sort.Sort (which will call Swap internally) - sort.Sort(timeSliceData) - - // Verify sorted order (newest first due to Less implementation) - So(*timeSliceData[0].Tag, ShouldEqual, "tag2") // Latest time - So(*timeSliceData[1].Tag, ShouldEqual, "tag3") // Middle time - So(*timeSliceData[2].Tag, ShouldEqual, "tag1") // Oldest time - }) - }) -} - func TestUtils(t *testing.T) { Convey("utils", t, func() { Convey("", func() { diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index d3742bad..abbc1cfa 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -2169,8 +2169,8 @@ func TestDerivedImageList(t *testing.T) { layers = [][]byte{ {10, 11, 10, 11}, {11, 11, 11, 11}, - {10, 10, 10, 10}, {10, 10, 10, 11}, + {10, 10, 10, 10}, {11, 11, 10, 10}, {11, 10, 10, 10}, } @@ -2613,7 +2613,7 @@ func TestBaseImageList(t *testing.T) { // create image with less layers than the given image, but which are in the given image layers = [][]byte{ {10, 11, 10, 11}, - {10, 10, 10, 11}, + {11, 11, 11, 11}, } manifest = ispec.Manifest{ diff --git a/pkg/extensions/sync/utils.go b/pkg/extensions/sync/utils.go index 945a8881..70baa026 100644 --- a/pkg/extensions/sync/utils.go +++ b/pkg/extensions/sync/utils.go @@ -46,7 +46,7 @@ func parseReference(reference string) (digest.Digest, bool) { // Given a list of registry string URLs parse them and return *url.URLs slice. func parseRegistryURLs(rawURLs []string) ([]*url.URL, error) { - urls := make([]*url.URL, 0) + urls := make([]*url.URL, 0, len(rawURLs)) for _, rawURL := range rawURLs { u, err := url.Parse(rawURL) diff --git a/pkg/test/oci-utils/oci_layout.go b/pkg/test/oci-utils/oci_layout.go index 191e4020..cedc1dc8 100644 --- a/pkg/test/oci-utils/oci_layout.go +++ b/pkg/test/oci-utils/oci_layout.go @@ -1,4 +1,4 @@ -//go:build sync && scrub && metrics && search +//go:build search package ociutils