Merge pull request 'fix: use buffered iterate for debian searchpackages' (#5702) from gusted/forgejo-buffered-iterate into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5702
Reviewed-by: Otto <otto@codeberg.org>

models/packages/debian/search.go

@@ -10,6 +10,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/packages"
 	debian_module "code.gitea.io/gitea/modules/packages/debian"
+	"code.gitea.io/gitea/modules/setting"
 
 	"xorm.io/builder"
 )
@@ -76,25 +77,41 @@ func ExistPackages(ctx context.Context, opts *PackageSearchOptions) (bool, error
 
 // SearchPackages gets the packages matching the search options
 func SearchPackages(ctx context.Context, opts *PackageSearchOptions, iter func(*packages.PackageFileDescriptor)) error {
-	return db.GetEngine(ctx).
+	var start int
-		Table("package_file").
+	batchSize := setting.Database.IterateBufferSize
-		Select("package_file.*").
+	for {
-		Join("INNER", "package_version", "package_version.id = package_file.version_id").
+		select {
-		Join("INNER", "package", "package.id = package_version.package_id").
+		case <-ctx.Done():
-		Where(opts.toCond()).
+			return ctx.Err()
-		Asc("package.lower_name", "package_version.created_unix").
+		default:
-		Iterate(new(packages.PackageFile), func(_ int, bean any) error {
+			beans := make([]*packages.PackageFile, 0, batchSize)
-			pf := bean.(*packages.PackageFile)
 
-			pfd, err := packages.GetPackageFileDescriptor(ctx, pf)
+			if err := db.GetEngine(ctx).
-			if err != nil {
+				Table("package_file").
+				Select("package_file.*").
+				Join("INNER", "package_version", "package_version.id = package_file.version_id").
+				Join("INNER", "package", "package.id = package_version.package_id").
+				Where(opts.toCond()).
+				Asc("package.lower_name", "package_version.created_unix").
+				Limit(batchSize, start).
+				Find(&beans); err != nil {
 				return err
 			}
+			if len(beans) == 0 {
+				return nil
+			}
+			start += len(beans)
 
-			iter(pfd)
+			for _, bean := range beans {
+				pfd, err := packages.GetPackageFileDescriptor(ctx, bean)
+				if err != nil {
+					return err
+				}
 
-			return nil
+				iter(pfd)
-		})
+			}
+		}
+	}
 }
 
 // GetDistributions gets all available distributions

models/packages/debian/search_test.go

@@ -0,0 +1,93 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package debian
+
+import (
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	packages_model "code.gitea.io/gitea/models/packages"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/packages"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+	packages_service "code.gitea.io/gitea/services/packages"
+
+	_ "code.gitea.io/gitea/models"
+	_ "code.gitea.io/gitea/models/actions"
+	_ "code.gitea.io/gitea/models/activities"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMain(m *testing.M) {
+	unittest.MainTest(m)
+}
+
+func preparePackage(t *testing.T, owner *user_model.User, name string) {
+	t.Helper()
+
+	data, err := packages.CreateHashedBufferFromReader(strings.NewReader("data"))
+	require.NoError(t, err)
+
+	_, _, err = packages_service.CreatePackageOrAddFileToExisting(
+		db.DefaultContext,
+		&packages_service.PackageCreationInfo{
+			PackageInfo: packages_service.PackageInfo{
+				Owner:       owner,
+				PackageType: packages_model.TypeDebian,
+				Name:        name,
+			},
+			Creator: owner,
+		},
+		&packages_service.PackageFileCreationInfo{
+			PackageFileInfo: packages_service.PackageFileInfo{
+				Filename: name,
+			},
+			Data:    data,
+			Creator: owner,
+			IsLead:  true,
+		},
+	)
+
+	require.NoError(t, err)
+}
+
+func TestSearchPackages(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	defer test.MockVariableValue(&setting.Database.IterateBufferSize, 1)()
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
+
+	preparePackage(t, user2, "debian-1")
+	preparePackage(t, user2, "debian-2")
+	preparePackage(t, user3, "debian-1")
+
+	packageFiles := []string{}
+	require.NoError(t, SearchPackages(db.DefaultContext, &PackageSearchOptions{
+		OwnerID: user2.ID,
+	}, func(pfd *packages_model.PackageFileDescriptor) {
+		assert.NotNil(t, pfd)
+		packageFiles = append(packageFiles, pfd.File.Name)
+	}))
+
+	assert.Len(t, packageFiles, 2)
+	assert.Contains(t, packageFiles, "debian-1")
+	assert.Contains(t, packageFiles, "debian-2")
+
+	packageFiles = []string{}
+	require.NoError(t, SearchPackages(db.DefaultContext, &PackageSearchOptions{
+		OwnerID: user3.ID,
+	}, func(pfd *packages_model.PackageFileDescriptor) {
+		assert.NotNil(t, pfd)
+		packageFiles = append(packageFiles, pfd.File.Name)
+	}))
+
+	assert.Len(t, packageFiles, 1)
+	assert.Contains(t, packageFiles, "debian-1")
+}