Merge pull request 'feat: optimize the FindUnreferencedPackages package query' (#4711) from earl-warren/forgejo:wip-container-simplify into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4711
Reviewed-by: thefox <thefox@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2024-07-27 15:33:13 +00:00
commit e7a12286dd
3 changed files with 18 additions and 16 deletions

View file

@ -280,19 +280,17 @@ func GetPackagesByType(ctx context.Context, ownerID int64, packageType Type) ([]
} }
// FindUnreferencedPackages gets all packages without associated versions // FindUnreferencedPackages gets all packages without associated versions
func FindUnreferencedPackages(ctx context.Context) ([]*Package, error) { func FindUnreferencedPackages(ctx context.Context) ([]int64, error) {
in := builder. var pIDs []int64
if err := db.GetEngine(ctx).
Select("package.id"). Select("package.id").
From("package"). Table("package").
LeftJoin("package_version", "package_version.package_id = package.id"). Join("LEFT", "package_version", "package_version.package_id = package.id").
Where(builder.Expr("package_version.id IS NULL")) Where("package_version.id IS NULL").
Find(&pIDs); err != nil {
ps := make([]*Package, 0, 10) return nil, err
return ps, db.GetEngine(ctx). }
// double select workaround for MySQL return pIDs, nil
// https://stackoverflow.com/questions/4471277/mysql-delete-from-with-subquery-as-condition
Where(builder.In("package.id", builder.Select("id").From(in, "temp"))).
Find(&ps)
} }
// HasOwnerPackages tests if a user/org has accessible packages // HasOwnerPackages tests if a user/org has accessible packages

View file

@ -154,15 +154,15 @@ func CleanupExpiredData(outerCtx context.Context, olderThan time.Duration) error
return err return err
} }
ps, err := packages_model.FindUnreferencedPackages(ctx) pIDs, err := packages_model.FindUnreferencedPackages(ctx)
if err != nil { if err != nil {
return err return err
} }
for _, p := range ps { for _, pID := range pIDs {
if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypePackage, p.ID); err != nil { if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypePackage, pID); err != nil {
return err return err
} }
if err := packages_model.DeletePackageByID(ctx, p.ID); err != nil { if err := packages_model.DeletePackageByID(ctx, pID); err != nil {
return err return err
} }
} }

View file

@ -479,6 +479,8 @@ func TestPackageCleanup(t *testing.T) {
AddBasicAuth(user.Name) AddBasicAuth(user.Name)
MakeRequest(t, req, http.StatusCreated) MakeRequest(t, req, http.StatusCreated)
unittest.AssertExistsAndLoadBean(t, &packages_model.Package{Name: "cleanup-test"})
pbs, err := packages_model.FindExpiredUnreferencedBlobs(db.DefaultContext, duration) pbs, err := packages_model.FindExpiredUnreferencedBlobs(db.DefaultContext, duration)
assert.NoError(t, err) assert.NoError(t, err)
assert.NotEmpty(t, pbs) assert.NotEmpty(t, pbs)
@ -493,6 +495,8 @@ func TestPackageCleanup(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.Empty(t, pbs) assert.Empty(t, pbs)
unittest.AssertNotExistsBean(t, &packages_model.Package{Name: "cleanup-test"})
_, err = packages_model.GetInternalVersionByNameAndVersion(db.DefaultContext, user.ID, packages_model.TypeContainer, "cleanup-test", container_model.UploadVersion) _, err = packages_model.GetInternalVersionByNameAndVersion(db.DefaultContext, user.ID, packages_model.TypeContainer, "cleanup-test", container_model.UploadVersion)
assert.ErrorIs(t, err, packages_model.ErrPackageNotExist) assert.ErrorIs(t, err, packages_model.ErrPackageNotExist)
}) })