From d1bb184738a49b5c6632ad0e6a2ddd6d0c3e47f9 Mon Sep 17 00:00:00 2001 From: taoky Date: Wed, 24 Jul 2024 04:25:27 +0800 Subject: [PATCH 1/2] 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") } From 63bdc74d93b8b0af9a7eddd7e23e3dec33feb24f Mon Sep 17 00:00:00 2001 From: taoky Date: Wed, 24 Jul 2024 04:35:37 +0800 Subject: [PATCH 2/2] ci: add nilness check in govet (#86) * fix(server): check db error correctly in handlerGetRepo * ci: add nilness check in govet --- .golangci.yml | 3 +++ pkg/server/repo_handlers.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index cb221b7..c242661 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,6 +65,9 @@ linters-settings: - standard # Standard section: captures all standard packages. - default # Default section: contains all imports that could not be matched to another section type. - prefix(github.com/ustclug/Yuki) + govet: + enable: + - nilness exhaustive: # Only run exhaustive check on switches with "//exhaustive:enforce" comment. explicit-exhaustive-switch: true diff --git a/pkg/server/repo_handlers.go b/pkg/server/repo_handlers.go index b369e6f..ce269ac 100644 --- a/pkg/server/repo_handlers.go +++ b/pkg/server/repo_handlers.go @@ -62,7 +62,7 @@ func (s *Server) handlerGetRepo(c echo.Context) error { Where(model.Repo{Name: name}). Limit(1). Find(&repo) - if err != nil { + if res.Error != nil { const msg = "Fail to get Repo" l.Error(msg, slogErrAttr(err)) return newHTTPError(http.StatusInternalServerError, msg)