From 778ad795fd4a19dc15723b59a846a250034c7c3a Mon Sep 17 00:00:00 2001 From: delvh Date: Mon, 25 Dec 2023 21:25:29 +0100 Subject: [PATCH] Refactor deletion (#28610) Introduce the new generic deletion methods - `func DeleteByID[T any](ctx context.Context, id int64) (int64, error)` - `func DeleteByIDs[T any](ctx context.Context, ids ...int64) error` - `func Delete[T any](ctx context.Context, opts FindOptions) (int64, error)` So, we no longer need any specific deletion method and can just use the generic ones instead. Replacement of #28450 Closes #28450 --------- Co-authored-by: Lunny Xiao --- models/actions/artifact.go | 13 --------- models/actions/runner.go | 2 +- models/asymkey/ssh_key.go | 14 ++-------- models/db/context.go | 34 +++++++++++++++++++----- models/db/error.go | 18 ------------- models/issues/issue_test.go | 4 +-- models/issues/label.go | 2 +- models/issues/milestone.go | 6 ++--- models/issues/review.go | 11 ++++---- models/org.go | 2 +- models/repo.go | 6 ++--- models/repo/archiver.go | 15 +---------- models/repo/pushmirror.go | 22 +++------------ models/repo/pushmirror_test.go | 6 ++--- models/repo/release.go | 8 +----- models/repo/upload.go | 4 +-- models/repo/watch.go | 10 +++---- models/system/notice.go | 17 ------------ models/system/notice_test.go | 11 ++------ models/webhook/webhook.go | 2 +- routers/api/actions/artifacts.go | 13 +++++---- routers/api/v1/repo/mirror.go | 13 ++++++--- routers/web/admin/notice.go | 3 ++- routers/web/repo/setting/setting.go | 2 +- services/asymkey/ssh_key.go | 2 +- services/mirror/mirror_push.go | 6 +++-- services/release/release.go | 2 +- services/repository/archiver/archiver.go | 4 +-- services/secrets/secrets.go | 2 +- services/user/delete.go | 2 +- tests/integration/repo_tag_test.go | 2 +- 31 files changed, 89 insertions(+), 169 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 42bd9c23cb4..5390f6288f1 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -90,19 +90,6 @@ func getArtifactByNameAndPath(ctx context.Context, runID int64, name, fpath stri return &art, nil } -// GetArtifactByID returns an artifact by id -func GetArtifactByID(ctx context.Context, id int64) (*ActionArtifact, error) { - var art ActionArtifact - has, err := db.GetEngine(ctx).ID(id).Get(&art) - if err != nil { - return nil, err - } else if !has { - return nil, util.ErrNotExist - } - - return &art, nil -} - // UpdateArtifactByID updates an artifact by id func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) error { art.ID = id diff --git a/models/actions/runner.go b/models/actions/runner.go index 5630741550d..4103ba44776 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -254,7 +254,7 @@ func DeleteRunner(ctx context.Context, id int64) error { return err } - _, err := db.GetEngine(ctx).Delete(&ActionRunner{ID: id}) + _, err := db.DeleteByID[ActionRunner](ctx, id) return err } diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index 552f2ffd69e..116d6351b0d 100644 --- a/models/asymkey/ssh_key.go +++ b/models/asymkey/ssh_key.go @@ -227,16 +227,6 @@ func UpdatePublicKeyUpdated(ctx context.Context, id int64) error { return nil } -// DeletePublicKeys does the actual key deletion but does not update authorized_keys file. -func DeletePublicKeys(ctx context.Context, keyIDs ...int64) error { - if len(keyIDs) == 0 { - return nil - } - - _, err := db.GetEngine(ctx).In("id", keyIDs).Delete(new(PublicKey)) - return err -} - // PublicKeysAreExternallyManaged returns whether the provided KeyID represents an externally managed Key func PublicKeysAreExternallyManaged(ctx context.Context, keys []*PublicKey) ([]bool, error) { sources := make([]*auth.Source, 0, 5) @@ -322,8 +312,8 @@ func deleteKeysMarkedForDeletion(ctx context.Context, keys []string) (bool, erro log.Error("SearchPublicKeyByContent: %v", err) continue } - if err = DeletePublicKeys(ctx, key.ID); err != nil { - log.Error("deletePublicKeys: %v", err) + if _, err = db.DeleteByID[PublicKey](ctx, key.ID); err != nil { + log.Error("DeleteByID[PublicKey]: %v", err) continue } sshKeysNeedUpdate = true diff --git a/models/db/context.go b/models/db/context.go index 7b739f7e9f3..cda608af196 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -175,7 +175,7 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) { func Get[T any](ctx context.Context, cond builder.Cond) (object *T, exist bool, err error) { if !cond.IsValid() { - return nil, false, ErrConditionRequired{} + panic("cond is invalid in db.Get(ctx, cond). This should not be possible.") } var bean T @@ -201,7 +201,7 @@ func GetByID[T any](ctx context.Context, id int64) (object *T, exist bool, err e func Exist[T any](ctx context.Context, cond builder.Cond) (bool, error) { if !cond.IsValid() { - return false, ErrConditionRequired{} + panic("cond is invalid in db.Exist(ctx, cond). This should not be possible.") } var bean T @@ -213,16 +213,36 @@ func ExistByID[T any](ctx context.Context, id int64) (bool, error) { return GetEngine(ctx).ID(id).NoAutoCondition().Exist(&bean) } +// DeleteByID deletes the given bean with the given ID +func DeleteByID[T any](ctx context.Context, id int64) (int64, error) { + var bean T + return GetEngine(ctx).ID(id).NoAutoCondition().NoAutoTime().Delete(&bean) +} + +func DeleteByIDs[T any](ctx context.Context, ids ...int64) error { + if len(ids) == 0 { + return nil + } + + var bean T + _, err := GetEngine(ctx).In("id", ids).NoAutoCondition().NoAutoTime().Delete(&bean) + return err +} + +func Delete[T any](ctx context.Context, opts FindOptions) (int64, error) { + if opts == nil || !opts.ToConds().IsValid() { + panic("opts are empty or invalid in db.Delete(ctx, opts). This should not be possible.") + } + + var bean T + return GetEngine(ctx).Where(opts.ToConds()).NoAutoCondition().NoAutoTime().Delete(&bean) +} + // DeleteByBean deletes all records according non-empty fields of the bean as conditions. func DeleteByBean(ctx context.Context, bean any) (int64, error) { return GetEngine(ctx).Delete(bean) } -// DeleteByID deletes the given bean with the given ID -func DeleteByID(ctx context.Context, id int64, bean any) (int64, error) { - return GetEngine(ctx).ID(id).NoAutoCondition().NoAutoTime().Delete(bean) -} - // FindIDs finds the IDs for the given table name satisfying the given condition // By passing a different value than "id" for "idCol", you can query for foreign IDs, i.e. the repo IDs which satisfy the condition func FindIDs(ctx context.Context, tableName, idCol string, cond builder.Cond) ([]int64, error) { diff --git a/models/db/error.go b/models/db/error.go index f601a15c01e..665e970e17c 100644 --- a/models/db/error.go +++ b/models/db/error.go @@ -72,21 +72,3 @@ func (err ErrNotExist) Error() string { func (err ErrNotExist) Unwrap() error { return util.ErrNotExist } - -// ErrConditionRequired represents an error which require condition. -type ErrConditionRequired struct{} - -// IsErrConditionRequired checks if an error is an ErrConditionRequired -func IsErrConditionRequired(err error) bool { - _, ok := err.(ErrConditionRequired) - return ok -} - -func (err ErrConditionRequired) Error() string { - return "condition is required" -} - -// Unwrap unwraps this as a ErrNotExist err -func (err ErrConditionRequired) Unwrap() error { - return util.ErrInvalidArgument -} diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 723fa27b1be..cc363d2fae7 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -249,11 +249,11 @@ func TestIssue_InsertIssue(t *testing.T) { // there are 5 issues and max index is 5 on repository 1, so this one should 6 issue := testInsertIssue(t, "my issue1", "special issue's comments?", 6) - _, err := db.GetEngine(db.DefaultContext).ID(issue.ID).Delete(new(issues_model.Issue)) + _, err := db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) issue = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 7) - _, err = db.GetEngine(db.DefaultContext).ID(issue.ID).Delete(new(issues_model.Issue)) + _, err = db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) } diff --git a/models/issues/label.go b/models/issues/label.go index 3b811c1529a..527d8d78532 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -256,7 +256,7 @@ func DeleteLabel(ctx context.Context, id, labelID int64) error { return nil } - if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { + if _, err = db.DeleteByID[Label](ctx, labelID); err != nil { return err } else if _, err = sess. Where("label_id = ?", labelID). diff --git a/models/issues/milestone.go b/models/issues/milestone.go index 9db7cb2b15e..eb42df8263b 100644 --- a/models/issues/milestone.go +++ b/models/issues/milestone.go @@ -289,9 +289,7 @@ func DeleteMilestoneByRepoID(ctx context.Context, repoID, id int64) error { } defer committer.Close() - sess := db.GetEngine(ctx) - - if _, err = sess.ID(m.ID).Delete(new(Milestone)); err != nil { + if _, err = db.DeleteByID[Milestone](ctx, m.ID); err != nil { return err } @@ -311,7 +309,7 @@ func DeleteMilestoneByRepoID(ctx context.Context, repoID, id int64) error { repo.NumMilestones = int(numMilestones) repo.NumClosedMilestones = int(numClosedMilestones) - if _, err = sess.ID(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { + if _, err = db.GetEngine(ctx).ID(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { return err } diff --git a/models/issues/review.go b/models/issues/review.go index e2f65e369f1..e06670c9a10 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -446,7 +446,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, issue *Issue, revi continue } - if _, err := sess.ID(teamReviewRequest.ID).NoAutoCondition().Delete(teamReviewRequest); err != nil { + if _, err := db.DeleteByID[Review](ctx, teamReviewRequest.ID); err != nil { return nil, nil, err } } @@ -869,7 +869,6 @@ func DeleteReview(ctx context.Context, r *Review) error { return err } defer committer.Close() - sess := db.GetEngine(ctx) if r.ID == 0 { return fmt.Errorf("review is not allowed to be 0") @@ -885,7 +884,7 @@ func DeleteReview(ctx context.Context, r *Review) error { ReviewID: r.ID, } - if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil { + if _, err := db.Delete[Comment](ctx, opts); err != nil { return err } @@ -895,7 +894,7 @@ func DeleteReview(ctx context.Context, r *Review) error { ReviewID: r.ID, } - if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil { + if _, err := db.Delete[Comment](ctx, opts); err != nil { return err } @@ -905,11 +904,11 @@ func DeleteReview(ctx context.Context, r *Review) error { ReviewID: r.ID, } - if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil { + if _, err := db.Delete[Comment](ctx, opts); err != nil { return err } - if _, err := sess.ID(r.ID).Delete(new(Review)); err != nil { + if _, err := db.DeleteByID[Review](ctx, r.ID); err != nil { return err } diff --git a/models/org.go b/models/org.go index 5e0deeb8de7..5f61f05b16a 100644 --- a/models/org.go +++ b/models/org.go @@ -57,7 +57,7 @@ func RemoveOrgUser(ctx context.Context, orgID, userID int64) error { } defer committer.Close() - if _, err := db.GetEngine(ctx).ID(ou.ID).Delete(ou); err != nil { + if _, err := db.DeleteByID[organization.OrgUser](ctx, ou.ID); err != nil { return err } else if _, err = db.Exec(ctx, "UPDATE `user` SET num_members=num_members-1 WHERE id=?", orgID); err != nil { return err diff --git a/models/repo.go b/models/repo.go index d525264b3b7..0dc8ee5df33 100644 --- a/models/repo.go +++ b/models/repo.go @@ -344,9 +344,7 @@ func DeleteDeployKey(ctx context.Context, doer *user_model.User, id int64) error } } - if _, err := db.DeleteByBean(ctx, &asymkey_model.DeployKey{ - ID: key.ID, - }); err != nil { + if _, err := db.DeleteByID[asymkey_model.DeployKey](ctx, key.ID); err != nil { return fmt.Errorf("delete deploy key [%d]: %w", key.ID, err) } @@ -355,7 +353,7 @@ func DeleteDeployKey(ctx context.Context, doer *user_model.User, id int64) error if err != nil { return err } else if !has { - if err = asymkey_model.DeletePublicKeys(ctx, key.KeyID); err != nil { + if _, err = db.DeleteByID[asymkey_model.PublicKey](ctx, key.KeyID); err != nil { return err } } diff --git a/models/repo/archiver.go b/models/repo/archiver.go index 6d0ed42877f..1fccb294994 100644 --- a/models/repo/archiver.go +++ b/models/repo/archiver.go @@ -68,14 +68,6 @@ func repoArchiverForRelativePath(relativePath string) (*RepoArchiver, error) { }, nil } -var delRepoArchiver = new(RepoArchiver) - -// DeleteRepoArchiver delete archiver -func DeleteRepoArchiver(ctx context.Context, archiver *RepoArchiver) error { - _, err := db.GetEngine(ctx).ID(archiver.ID).Delete(delRepoArchiver) - return err -} - // GetRepoArchiver get an archiver func GetRepoArchiver(ctx context.Context, repoID int64, tp git.ArchiveType, commitID string) (*RepoArchiver, error) { var archiver RepoArchiver @@ -100,12 +92,6 @@ func ExistsRepoArchiverWithStoragePath(ctx context.Context, storagePath string) return db.GetEngine(ctx).Exist(archiver) } -// AddRepoArchiver adds an archiver -func AddRepoArchiver(ctx context.Context, archiver *RepoArchiver) error { - _, err := db.GetEngine(ctx).Insert(archiver) - return err -} - // UpdateRepoArchiverStatus updates archiver's status func UpdateRepoArchiverStatus(ctx context.Context, archiver *RepoArchiver) error { _, err := db.GetEngine(ctx).ID(archiver.ID).Cols("status").Update(archiver) @@ -114,6 +100,7 @@ func UpdateRepoArchiverStatus(ctx context.Context, archiver *RepoArchiver) error // DeleteAllRepoArchives deletes all repo archives records func DeleteAllRepoArchives(ctx context.Context) error { + // 1=1 to enforce delete all data, otherwise it will delete nothing _, err := db.GetEngine(ctx).Where("1=1").Delete(new(RepoArchiver)) return err } diff --git a/models/repo/pushmirror.go b/models/repo/pushmirror.go index 61cf1849b00..24c58faf849 100644 --- a/models/repo/pushmirror.go +++ b/models/repo/pushmirror.go @@ -34,12 +34,13 @@ type PushMirror struct { } type PushMirrorOptions struct { + db.ListOptions ID int64 RepoID int64 RemoteName string } -func (opts *PushMirrorOptions) toConds() builder.Cond { +func (opts PushMirrorOptions) ToConds() builder.Cond { cond := builder.NewCond() if opts.RepoID > 0 { cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) @@ -75,12 +76,6 @@ func (m *PushMirror) GetRemoteName() string { return m.RemoteName } -// InsertPushMirror inserts a push-mirror to database -func InsertPushMirror(ctx context.Context, m *PushMirror) error { - _, err := db.GetEngine(ctx).Insert(m) - return err -} - // UpdatePushMirror updates the push-mirror func UpdatePushMirror(ctx context.Context, m *PushMirror) error { _, err := db.GetEngine(ctx).ID(m.ID).AllCols().Update(m) @@ -95,23 +90,12 @@ func UpdatePushMirrorInterval(ctx context.Context, m *PushMirror) error { func DeletePushMirrors(ctx context.Context, opts PushMirrorOptions) error { if opts.RepoID > 0 { - _, err := db.GetEngine(ctx).Where(opts.toConds()).Delete(&PushMirror{}) + _, err := db.Delete[PushMirror](ctx, opts) return err } return util.NewInvalidArgumentErrorf("repoID required and must be set") } -func GetPushMirror(ctx context.Context, opts PushMirrorOptions) (*PushMirror, error) { - mirror := &PushMirror{} - exist, err := db.GetEngine(ctx).Where(opts.toConds()).Get(mirror) - if err != nil { - return nil, err - } else if !exist { - return nil, ErrPushMirrorNotExist - } - return mirror, nil -} - // GetPushMirrorsByRepoID returns push-mirror information of a repository. func GetPushMirrorsByRepoID(ctx context.Context, repoID int64, listOptions db.ListOptions) ([]*PushMirror, int64, error) { sess := db.GetEngine(ctx).Where("repo_id = ?", repoID) diff --git a/models/repo/pushmirror_test.go b/models/repo/pushmirror_test.go index 9ab70235913..e19749d93a2 100644 --- a/models/repo/pushmirror_test.go +++ b/models/repo/pushmirror_test.go @@ -20,20 +20,20 @@ func TestPushMirrorsIterate(t *testing.T) { now := timeutil.TimeStampNow() - repo_model.InsertPushMirror(db.DefaultContext, &repo_model.PushMirror{ + db.Insert(db.DefaultContext, &repo_model.PushMirror{ RemoteName: "test-1", LastUpdateUnix: now, Interval: 1, }) long, _ := time.ParseDuration("24h") - repo_model.InsertPushMirror(db.DefaultContext, &repo_model.PushMirror{ + db.Insert(db.DefaultContext, &repo_model.PushMirror{ RemoteName: "test-2", LastUpdateUnix: now, Interval: long, }) - repo_model.InsertPushMirror(db.DefaultContext, &repo_model.PushMirror{ + db.Insert(db.DefaultContext, &repo_model.PushMirror{ RemoteName: "test-3", LastUpdateUnix: now, Interval: 0, diff --git a/models/repo/release.go b/models/repo/release.go index 223d3f25019..4514a034ed9 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -450,12 +450,6 @@ func SortReleases(rels []*Release) { sort.Sort(sorter) } -// DeleteReleaseByID deletes a release from database by given ID. -func DeleteReleaseByID(ctx context.Context, id int64) error { - _, err := db.GetEngine(ctx).ID(id).Delete(new(Release)) - return err -} - // UpdateReleasesMigrationsByType updates all migrated repositories' releases from gitServiceType to replace originalAuthorID to posterID func UpdateReleasesMigrationsByType(ctx context.Context, gitServiceType structs.GitServiceType, originalAuthorID string, posterID int64) error { _, err := db.GetEngine(ctx).Table("release"). @@ -509,7 +503,7 @@ func PushUpdateDeleteTag(ctx context.Context, repo *Repository, tagName string) return fmt.Errorf("GetRelease: %w", err) } if rel.IsTag { - if _, err = db.GetEngine(ctx).ID(rel.ID).Delete(new(Release)); err != nil { + if _, err = db.DeleteByID[Release](ctx, rel.ID); err != nil { return fmt.Errorf("Delete: %w", err) } } else { diff --git a/models/repo/upload.go b/models/repo/upload.go index d96ab21bcde..18834f6b83f 100644 --- a/models/repo/upload.go +++ b/models/repo/upload.go @@ -131,9 +131,7 @@ func DeleteUploads(ctx context.Context, uploads ...*Upload) (err error) { for i := 0; i < len(uploads); i++ { ids[i] = uploads[i].ID } - if _, err = db.GetEngine(ctx). - In("id", ids). - Delete(new(Upload)); err != nil { + if err = db.DeleteByIDs[Upload](ctx, ids...); err != nil { return fmt.Errorf("delete uploads: %w", err) } diff --git a/models/repo/watch.go b/models/repo/watch.go index fba66d6dcb5..80da4030cbc 100644 --- a/models/repo/watch.go +++ b/models/repo/watch.go @@ -85,23 +85,21 @@ func watchRepoMode(ctx context.Context, watch Watch, mode WatchMode) (err error) watch.Mode = mode - e := db.GetEngine(ctx) - if !hadrec && needsrec { watch.Mode = mode - if _, err = e.Insert(watch); err != nil { + if err = db.Insert(ctx, watch); err != nil { return err } } else if needsrec { watch.Mode = mode - if _, err := e.ID(watch.ID).AllCols().Update(watch); err != nil { + if _, err := db.GetEngine(ctx).ID(watch.ID).AllCols().Update(watch); err != nil { return err } - } else if _, err = e.Delete(Watch{ID: watch.ID}); err != nil { + } else if _, err = db.DeleteByID[Watch](ctx, watch.ID); err != nil { return err } if repodiff != 0 { - _, err = e.Exec("UPDATE `repository` SET num_watches = num_watches + ? WHERE id = ?", repodiff, watch.RepoID) + _, err = db.GetEngine(ctx).Exec("UPDATE `repository` SET num_watches = num_watches + ? WHERE id = ?", repodiff, watch.RepoID) } return err } diff --git a/models/system/notice.go b/models/system/notice.go index 058b78e6771..e7ec6a9693f 100644 --- a/models/system/notice.go +++ b/models/system/notice.go @@ -102,12 +102,6 @@ func Notices(ctx context.Context, page, pageSize int) ([]*Notice, error) { Find(¬ices) } -// DeleteNotice deletes a system notice by given ID. -func DeleteNotice(ctx context.Context, id int64) error { - _, err := db.GetEngine(ctx).ID(id).Delete(new(Notice)) - return err -} - // DeleteNotices deletes all notices with ID from start to end (inclusive). func DeleteNotices(ctx context.Context, start, end int64) error { if start == 0 && end == 0 { @@ -123,17 +117,6 @@ func DeleteNotices(ctx context.Context, start, end int64) error { return err } -// DeleteNoticesByIDs deletes notices by given IDs. -func DeleteNoticesByIDs(ctx context.Context, ids []int64) error { - if len(ids) == 0 { - return nil - } - _, err := db.GetEngine(ctx). - In("id", ids). - Delete(new(Notice)) - return err -} - // DeleteOldSystemNotices deletes all old system notices from database. func DeleteOldSystemNotices(ctx context.Context, olderThan time.Duration) (err error) { if olderThan <= 0 { diff --git a/models/system/notice_test.go b/models/system/notice_test.go index e8ce05d3324..599b2fb65c3 100644 --- a/models/system/notice_test.go +++ b/models/system/notice_test.go @@ -69,14 +69,6 @@ func TestNotices(t *testing.T) { } } -func TestDeleteNotice(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 3}) - assert.NoError(t, system.DeleteNotice(db.DefaultContext, 3)) - unittest.AssertNotExistsBean(t, &system.Notice{ID: 3}) -} - func TestDeleteNotices(t *testing.T) { // delete a non-empty range assert.NoError(t, unittest.PrepareTestDatabase()) @@ -109,7 +101,8 @@ func TestDeleteNoticesByIDs(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 1}) unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 2}) unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 3}) - assert.NoError(t, system.DeleteNoticesByIDs(db.DefaultContext, []int64{1, 3})) + err := db.DeleteByIDs[system.Notice](db.DefaultContext, 1, 3) + assert.NoError(t, err) unittest.AssertNotExistsBean(t, &system.Notice{ID: 1}) unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 2}) unittest.AssertNotExistsBean(t, &system.Notice{ID: 3}) diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index a72bd938aac..4a84a3d4111 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -471,7 +471,7 @@ func DeleteWebhookByID(ctx context.Context, id int64) (err error) { } defer committer.Close() - if count, err := db.DeleteByID(ctx, id, new(Webhook)); err != nil { + if count, err := db.DeleteByID[Webhook](ctx, id); err != nil { return err } else if count == 0 { return ErrWebhookNotExist{ID: id} diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 5411237103a..5bfcc9dfcd7 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -63,7 +63,6 @@ package actions import ( "crypto/md5" - "errors" "fmt" "net/http" "strconv" @@ -423,15 +422,15 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) { } artifactID := ctx.ParamsInt64("artifact_id") - artifact, err := actions.GetArtifactByID(ctx, artifactID) - if errors.Is(err, util.ErrNotExist) { - log.Error("Error getting artifact: %v", err) - ctx.Error(http.StatusNotFound, err.Error()) - return - } else if err != nil { + artifact, exist, err := db.GetByID[actions.ActionArtifact](ctx, artifactID) + if err != nil { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return + } else if !exist { + log.Error("artifact with ID %d does not exist", artifactID) + ctx.Error(http.StatusNotFound, fmt.Sprintf("artifact with ID %d does not exist", artifactID)) + return } if artifact.RunID != runID { log.Error("Error dismatch runID and artifactID, task: %v, artifact: %v", runID, artifactID) diff --git a/routers/api/v1/repo/mirror.go b/routers/api/v1/repo/mirror.go index 72ddc758dce..26e0be301c1 100644 --- a/routers/api/v1/repo/mirror.go +++ b/routers/api/v1/repo/mirror.go @@ -227,11 +227,18 @@ func GetPushMirrorByName(ctx *context.APIContext) { mirrorName := ctx.Params(":name") // Get push mirror of a specific repo by remoteName - pushMirror, err := repo_model.GetPushMirror(ctx, repo_model.PushMirrorOptions{RepoID: ctx.Repo.Repository.ID, RemoteName: mirrorName}) + pushMirror, exist, err := db.Get[repo_model.PushMirror](ctx, repo_model.PushMirrorOptions{ + RepoID: ctx.Repo.Repository.ID, + RemoteName: mirrorName, + }.ToConds()) if err != nil { - ctx.Error(http.StatusNotFound, "GetPushMirrors", err) + ctx.Error(http.StatusInternalServerError, "GetPushMirrors", err) + return + } else if !exist { + ctx.Error(http.StatusNotFound, "GetPushMirrors", nil) return } + m, err := convert.ToPushMirror(ctx, pushMirror) if err != nil { ctx.ServerError("GetPushMirrorByRemoteName", err) @@ -368,7 +375,7 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro RemoteAddress: remoteAddress, } - if err = repo_model.InsertPushMirror(ctx, pushMirror); err != nil { + if err = db.Insert(ctx, pushMirror); err != nil { ctx.ServerError("InsertPushMirror", err) return } diff --git a/routers/web/admin/notice.go b/routers/web/admin/notice.go index 99039a2a9f9..e1cb578d052 100644 --- a/routers/web/admin/notice.go +++ b/routers/web/admin/notice.go @@ -8,6 +8,7 @@ import ( "net/http" "strconv" + "code.gitea.io/gitea/models/db" system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" @@ -55,7 +56,7 @@ func DeleteNotices(ctx *context.Context) { } } - if err := system_model.DeleteNoticesByIDs(ctx, ids); err != nil { + if err := db.DeleteByIDs[system_model.Notice](ctx, ids...); err != nil { ctx.Flash.Error("DeleteNoticesByIDs: " + err.Error()) ctx.Status(http.StatusInternalServerError) } else { diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 0864b1c911a..289cf5c9aea 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -418,7 +418,7 @@ func SettingsPost(ctx *context.Context) { Interval: interval, RemoteAddress: remoteAddress, } - if err := repo_model.InsertPushMirror(ctx, m); err != nil { + if err := db.Insert(ctx, m); err != nil { ctx.ServerError("InsertPushMirror", err) return } diff --git a/services/asymkey/ssh_key.go b/services/asymkey/ssh_key.go index 1c3bf09b083..83d7edafa3b 100644 --- a/services/asymkey/ssh_key.go +++ b/services/asymkey/ssh_key.go @@ -33,7 +33,7 @@ func DeletePublicKey(ctx context.Context, doer *user_model.User, id int64) (err } defer committer.Close() - if err = asymkey_model.DeletePublicKeys(dbCtx, id); err != nil { + if _, err = db.DeleteByID[asymkey_model.PublicKey](dbCtx, id); err != nil { return err } diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 6eaca71495e..b117e79faca 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" @@ -93,8 +94,9 @@ func SyncPushMirror(ctx context.Context, mirrorID int64) bool { log.Error("PANIC whilst syncPushMirror[%d] Panic: %v\nStacktrace: %s", mirrorID, err, log.Stack(2)) }() - m, err := repo_model.GetPushMirror(ctx, repo_model.PushMirrorOptions{ID: mirrorID}) - if err != nil { + // TODO: Handle "!exist" better + m, exist, err := db.GetByID[repo_model.PushMirror](ctx, mirrorID) + if err != nil || !exist { log.Error("GetPushMirrorByID [%d]: %v", mirrorID, err) return false } diff --git a/services/release/release.go b/services/release/release.go index fc91171fba6..ddfb11fa476 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -339,7 +339,7 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re }, repository.NewPushCommits()) notify_service.DeleteRef(ctx, doer, repo, refName) - if err := repo_model.DeleteReleaseByID(ctx, rel.ID); err != nil { + if _, err := db.DeleteByID[repo_model.Release](ctx, rel.ID); err != nil { return fmt.Errorf("DeleteReleaseByID: %w", err) } } else { diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index f700e3af5de..b73d0eed480 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -177,7 +177,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver CommitID: r.CommitID, Status: repo_model.ArchiverGenerating, } - if err := repo_model.AddRepoArchiver(ctx, archiver); err != nil { + if err := db.Insert(ctx, archiver); err != nil { return nil, err } } @@ -309,7 +309,7 @@ func StartArchive(request *ArchiveRequest) error { } func deleteOldRepoArchiver(ctx context.Context, archiver *repo_model.RepoArchiver) error { - if err := repo_model.DeleteRepoArchiver(ctx, archiver); err != nil { + if _, err := db.DeleteByID[repo_model.RepoArchiver](ctx, archiver.ID); err != nil { return err } p := archiver.RelativePath() diff --git a/services/secrets/secrets.go b/services/secrets/secrets.go index 97e15ba6c7f..031c474dd72 100644 --- a/services/secrets/secrets.go +++ b/services/secrets/secrets.go @@ -76,7 +76,7 @@ func DeleteSecretByName(ctx context.Context, ownerID, repoID int64, name string) } func deleteSecret(ctx context.Context, s *secret_model.Secret) error { - if _, err := db.DeleteByID(ctx, s.ID, s); err != nil { + if _, err := db.DeleteByID[secret_model.Secret](ctx, s.ID); err != nil { return err } return nil diff --git a/services/user/delete.go b/services/user/delete.go index c4617e064e7..748f5c2d1e9 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -185,7 +185,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) } // ***** END: ExternalLoginUser ***** - if _, err = db.DeleteByID(ctx, u.ID, new(user_model.User)); err != nil { + if _, err = db.DeleteByID[user_model.User](ctx, u.ID); err != nil { return fmt.Errorf("delete: %w", err) } diff --git a/tests/integration/repo_tag_test.go b/tests/integration/repo_tag_test.go index 8667a6d6e9f..e588097994f 100644 --- a/tests/integration/repo_tag_test.go +++ b/tests/integration/repo_tag_test.go @@ -81,7 +81,7 @@ func TestCreateNewTagProtected(t *testing.T) { assert.NoError(t, err) for _, release := range releases { - err = repo_model.DeleteReleaseByID(db.DefaultContext, release.ID) + _, err = db.DeleteByID[repo_model.Release](db.DefaultContext, release.ID) assert.NoError(t, err) }