Skip to content

Commit

Permalink
server: Give error if repo to be removed does not exist
Browse files Browse the repository at this point in the history
Closes: #68.
  • Loading branch information
taoky committed Jul 23, 2024
1 parent ebc72a3 commit 4d696b0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
8 changes: 6 additions & 2 deletions pkg/server/repo_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ func (s *Server) handlerRemoveRepo(c echo.Context) error {
}

db := s.getDB(c)
err = db.Where(model.Repo{Name: name}).Delete(&model.Repo{}).Error
if err != nil {
res := db.Where(model.Repo{Name: name}).Delete(&model.Repo{})
if res.Error != nil {
const msg = "Fail to delete Repo"
l.Error(msg, slogErrAttr(err), slog.String("repo", name))
return newHTTPError(http.StatusInternalServerError, msg)
Expand All @@ -95,6 +95,10 @@ func (s *Server) handlerRemoveRepo(c echo.Context) error {
l.Error("Fail to delete RepoMeta", slogErrAttr(err), slog.String("repo", name))
}
s.repoSchedules.Remove(name)
// Check repo existence after RepoMeta and schedule removal, to prevent inconsistency
if res.RowsAffected == 0 {
return newHTTPError(http.StatusNotFound, "Repo not found")
}
return c.NoContent(http.StatusNoContent)
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/server/repo_handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,8 @@ func TestHandlerRemoveRepo(t *testing.T) {
require.False(t, te.server.repoSchedules.Has(name))
require.ErrorContains(t, te.server.db.First(&model.Repo{Name: name}).Error, "record not found")
require.ErrorContains(t, te.server.db.First(&model.RepoMeta{Name: name}).Error, "record not found")

resp, err = cli.R().Delete("/repos/nonexist")
require.NoError(t, err)
require.Equal(t, 404, resp.StatusCode(), "Removing non-exist repo does not return 404")
}

0 comments on commit 4d696b0

Please sign in to comment.