Implement suggestions by @lng2020 and myself regarding #28450

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
@@ -231,7 +231,7 @@ func DeleteByIDs[T any](ctx context.Context, ids ...int64) error {
 
 func Delete[T any](ctx context.Context, opts FindOptions) (int64, error) {
 	if opts == nil {
-		return 0, ErrConditionRequired{}
+		panic("opts are empty in db.Delete(ctx, opts). This should not be possible.")
 	}
 
 	var bean T

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
-}

models/system/notice_test.go

@@ -69,15 +69,6 @@ func TestNotices(t *testing.T) {
 	}
 }
 
-func TestDeleteNotice(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	unittest.AssertExistsAndLoadBean(t, &system.Notice{ID: 3})
-	_, err := db.DeleteByID[system.Notice](db.DefaultContext, 3)
-	assert.NoError(t, err)
-	unittest.AssertNotExistsBean(t, &system.Notice{ID: 3})
-}
-
 func TestDeleteNotices(t *testing.T) {
 	// delete a non-empty range
 	assert.NoError(t, unittest.PrepareTestDatabase())

routers/api/actions/artifacts.go

@@ -423,14 +423,14 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) {
 
 	artifactID := ctx.ParamsInt64("artifact_id")
 	artifact, exist, err := db.GetByID[actions.ActionArtifact](ctx, artifactID)
-	if !exist {
-		log.Error("artifact with %d does not exist", artifactID)
-		ctx.Error(http.StatusNotFound, fmt.Sprintf("artifact with %d does not exist", artifactID))
-		return
-	} else if err != nil {
+	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)

routers/api/v1/repo/mirror.go

@@ -232,7 +232,7 @@ func GetPushMirrorByName(ctx *context.APIContext) {
 		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)

services/mirror/mirror_push.go

@@ -94,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 := db.GetByID[repo_model.PushMirror](ctx, 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
 	}

services/user/delete.go

@@ -154,7 +154,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 
 	// ***** START: PublicKey *****
 	if _, err = db.DeleteByBean(ctx, &asymkey_model.PublicKey{OwnerID: u.ID}); err != nil {
-		return fmt.Errorf("DeleteByBean: %w", err)
+		return fmt.Errorf("deletePublicKeys: %w", err)
 	}
 	// ***** END: PublicKey *****