Consistent status on delete (#1703)

Closes #1675

server/api/cron.go

@@ -204,7 +204,7 @@ func DeleteCron(c *gin.Context) {
 		return
 	}
 	if err := store.FromContext(c).CronDelete(repo, id); err != nil {
-		c.String(http.StatusInternalServerError, "Error deleting cron %d. %s", id, err)
+		handleDbGetError(c, err)
 		return
 	}
 	c.String(http.StatusNoContent, "")

server/api/global_secret.go

@@ -118,7 +118,7 @@ func PatchGlobalSecret(c *gin.Context) {
 func DeleteGlobalSecret(c *gin.Context) {
 	name := c.Param("secret")
 	if err := server.Config.Services.Secrets.GlobalSecretDelete(name); err != nil {
-		c.String(http.StatusInternalServerError, "Error deleting global secret %q. %s", name, err)
+		handleDbGetError(c, err)
 		return
 	}
 	c.String(http.StatusNoContent, "")

server/api/org_secret.go

@@ -131,7 +131,7 @@ func DeleteOrgSecret(c *gin.Context) {
 		name  = c.Param("secret")
 	)
 	if err := server.Config.Services.Secrets.OrgSecretDelete(owner, name); err != nil {
-		c.String(http.StatusInternalServerError, "Error deleting org %q secret %q. %s", owner, name, err)
+		handleDbGetError(c, err)
 		return
 	}
 	c.String(http.StatusNoContent, "")

server/api/registry.go

@@ -135,7 +135,7 @@ func DeleteRegistry(c *gin.Context) {
 	)
 	err := server.Config.Services.Registries.RegistryDelete(repo, name)
 	if err != nil {
-		c.String(http.StatusInternalServerError, "Error deleting registry %q. %s", name, err)
+		handleDbGetError(c, err)
 		return
 	}
 	c.String(http.StatusNoContent, "")

server/api/repo_secret.go

@@ -133,7 +133,7 @@ func DeleteSecret(c *gin.Context) {
 		name = c.Param("secret")
 	)
 	if err := server.Config.Services.Secrets.SecretDelete(repo, name); err != nil {
-		c.String(http.StatusInternalServerError, "Error deleting secret %q. %s", name, err)
+		handleDbGetError(c, err)
 		return
 	}
 	c.String(http.StatusNoContent, "")

server/store/datastore/agent.go

@@ -47,6 +47,5 @@ func (s storage) AgentUpdate(agent *model.Agent) error {
 }
 
 func (s storage) AgentDelete(agent *model.Agent) error {
-	_, err := s.engine.ID(agent.ID).Delete(new(model.Agent))
+	return wrapDelete(s.engine.ID(agent.ID).Delete(new(model.Agent)))
-	return err
 }

server/store/datastore/cron.go

@@ -47,8 +47,7 @@ func (s storage) CronUpdate(_ *model.Repo, cron *model.Cron) error {
 }
 
 func (s storage) CronDelete(repo *model.Repo, id int64) error {
-	_, err := s.engine.ID(id).Where("repo_id = ?", repo.ID).Delete(new(model.Cron))
+	return wrapDelete(s.engine.ID(id).Where("repo_id = ?", repo.ID).Delete(new(model.Cron)))
-	return err
 }
 
 // CronListNextExecute returns limited number of jobs with NextExec being less or equal to the provided unix timestamp

server/store/datastore/helper.go

@@ -28,3 +28,14 @@ func wrapGet(exist bool, err error) error {
 	}
 	return nil
 }
+
+// wrapDelete return error if err not nil or if requested entry do not exist
+func wrapDelete(c int64, err error) error {
+	if err != nil {
+		return err
+	}
+	if c == 0 {
+		return types.RecordNotExist
+	}
+	return nil
+}

server/store/datastore/permission.go

@@ -76,10 +76,9 @@ func (s storage) permUpsert(sess *xorm.Session, perm *model.Perm) error {
 }
 
 func (s storage) PermDelete(perm *model.Perm) error {
-	_, err := s.engine.
+	return wrapDelete(s.engine.
 		Where("perm_user_id = ? AND perm_repo_id = ?", perm.UserID, perm.RepoID).
-		Delete(new(model.Perm))
+		Delete(new(model.Perm)))
-	return err
 }
 
 func (s storage) PermFlush(user *model.User, before int64) error {

server/store/datastore/secret.go

@@ -57,8 +57,7 @@ func (s storage) SecretUpdate(secret *model.Secret) error {
 }
 
 func (s storage) SecretDelete(secret *model.Secret) error {
-	_, err := s.engine.ID(secret.ID).Delete(new(model.Secret))
+	return wrapDelete(s.engine.ID(secret.ID).Delete(new(model.Secret)))
-	return err
 }
 
 func (s storage) OrgSecretFind(owner, name string) (*model.Secret, error) {

server/store/datastore/server_config.go

@@ -41,6 +41,5 @@ func (s storage) ServerConfigDelete(key string) error {
 		Key: key,
 	}
 
-	_, err := s.engine.Delete(config)
+	return wrapDelete(s.engine.Delete(config))
-	return err
 }

server/store/datastore/task.go

@@ -30,6 +30,5 @@ func (s storage) TaskInsert(task *model.Task) error {
 }
 
 func (s storage) TaskDelete(id string) error {
-	_, err := s.engine.Where("task_id = ?", id).Delete(new(model.Task))
+	return wrapDelete(s.engine.Where("task_id = ?", id).Delete(new(model.Task)))
-	return err
 }

server/store/datastore/user.go

@@ -55,7 +55,7 @@ func (s storage) DeleteUser(user *model.User) error {
 		return err
 	}
 
-	if _, err := sess.ID(user.ID).Delete(new(model.User)); err != nil {
+	if err := wrapDelete(sess.ID(user.ID).Delete(new(model.User))); err != nil {
 		return err
 	}