From d1bb184738a49b5c6632ad0e6a2ddd6d0c3e47f9 Mon Sep 17 00:00:00 2001 From: taoky Date: Wed, 24 Jul 2024 04:25:27 +0800 Subject: [PATCH] server: Give error if repo to be removed does not exist (#85) Closes: #68. --- pkg/server/repo_handlers.go | 8 ++++++-- pkg/server/repo_handlers_test.go | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/server/repo_handlers.go b/pkg/server/repo_handlers.go index 4a7efba..b369e6f 100644 --- a/pkg/server/repo_handlers.go +++ b/pkg/server/repo_handlers.go @@ -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) @@ -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) } diff --git a/pkg/server/repo_handlers_test.go b/pkg/server/repo_handlers_test.go index 45b76c7..eb9e5fd 100644 --- a/pkg/server/repo_handlers_test.go +++ b/pkg/server/repo_handlers_test.go @@ -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") }