From 2b49f26017efc7f455cb44b525f2c9f3bca79acc Mon Sep 17 00:00:00 2001 From: Anirudh Oppiliappan Date: Tue, 11 Mar 2025 19:57:35 +0200 Subject: [PATCH] knotserver/git: shared types and idomatic go --- knotserver/git/merge.go | 16 ++++++++-------- knotserver/routes.go | 36 ++++++++++++++++++------------------ types/merge.go | 12 ++++++++++++ 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/knotserver/git/merge.go b/knotserver/git/merge.go index 6fa587fe..90fc462f 100644 --- a/knotserver/git/merge.go +++ b/knotserver/git/merge.go @@ -12,7 +12,7 @@ import ( "github.com/go-git/go-git/v5/plumbing" ) -type MergeError struct { +type ErrMerge struct { Message string Conflicts []ConflictInfo HasConflict bool @@ -24,7 +24,7 @@ type ConflictInfo struct { Reason string } -func (e MergeError) Error() string { +func (e ErrMerge) Error() string { if e.HasConflict { return fmt.Sprintf("merge failed due to conflicts: %s (%d conflicts)", e.Message, len(e.Conflicts)) } @@ -90,7 +90,7 @@ func (g *GitRepo) applyPatch(tmpDir, patchFile string, checkOnly bool) error { if err := cmd.Run(); err != nil { if checkOnly { conflicts := parseGitApplyErrors(stderr.String()) - return &MergeError{ + return &ErrMerge{ Message: "patch cannot be applied cleanly", Conflicts: conflicts, HasConflict: len(conflicts) > 0, @@ -106,7 +106,7 @@ func (g *GitRepo) applyPatch(tmpDir, patchFile string, checkOnly bool) error { func (g *GitRepo) MergeCheck(patchData []byte, targetBranch string) error { patchFile, err := g.createTempFileWithPatch(patchData) if err != nil { - return &MergeError{ + return &ErrMerge{ Message: err.Error(), OtherError: err, } @@ -115,7 +115,7 @@ func (g *GitRepo) MergeCheck(patchData []byte, targetBranch string) error { tmpDir, err := g.cloneRepository(targetBranch) if err != nil { - return &MergeError{ + return &ErrMerge{ Message: err.Error(), OtherError: err, } @@ -128,7 +128,7 @@ func (g *GitRepo) MergeCheck(patchData []byte, targetBranch string) error { func (g *GitRepo) Merge(patchData []byte, targetBranch string) error { patchFile, err := g.createTempFileWithPatch(patchData) if err != nil { - return &MergeError{ + return &ErrMerge{ Message: err.Error(), OtherError: err, } @@ -137,7 +137,7 @@ func (g *GitRepo) Merge(patchData []byte, targetBranch string) error { tmpDir, err := g.cloneRepository(targetBranch) if err != nil { - return &MergeError{ + return &ErrMerge{ Message: err.Error(), OtherError: err, } @@ -150,7 +150,7 @@ func (g *GitRepo) Merge(patchData []byte, targetBranch string) error { pushCmd := exec.Command("git", "-C", tmpDir, "push") if err := pushCmd.Run(); err != nil { - return &MergeError{ + return &ErrMerge{ Message: "failed to push changes to bare repository", OtherError: err, } diff --git a/knotserver/routes.go b/knotserver/routes.go index 5b075961..429163da 100644 --- a/knotserver/routes.go +++ b/knotserver/routes.go @@ -577,20 +577,20 @@ func (h *Handle) Merge(w http.ResponseWriter, r *http.Request) { notFound(w) return } - if err := gr.Merge([]byte(patch), branch); err != nil { - var mergeErr *git.MergeError + var mergeErr *git.ErrMerge if errors.As(err, &mergeErr) { - conflictDetails := make([]map[string]interface{}, len(mergeErr.Conflicts)) + conflicts := make([]types.ConflictInfo, len(mergeErr.Conflicts)) for i, conflict := range mergeErr.Conflicts { - conflictDetails[i] = map[string]interface{}{ - "filename": conflict.Filename, - "reason": conflict.Reason, + conflicts[i] = types.ConflictInfo{ + Filename: conflict.Filename, + Reason: conflict.Reason, } } - response := map[string]interface{}{ - "message": mergeErr.Message, - "conflicts": conflictDetails, + response := types.MergeCheckResponse{ + IsConflicted: true, + Conflicts: conflicts, + Message: mergeErr.Message, } writeConflict(w, response) h.l.Error("git: merge conflict", "handler", "Merge", "error", mergeErr) @@ -632,24 +632,24 @@ func (h *Handle) MergeCheck(w http.ResponseWriter, r *http.Request) { return } - var mergeErr *git.MergeError + var mergeErr *git.ErrMerge if errors.As(err, &mergeErr) { - conflictDetails := make([]map[string]interface{}, len(mergeErr.Conflicts)) + conflicts := make([]types.ConflictInfo, len(mergeErr.Conflicts)) for i, conflict := range mergeErr.Conflicts { - conflictDetails[i] = map[string]interface{}{ - "filename": conflict.Filename, - "reason": conflict.Reason, + conflicts[i] = types.ConflictInfo{ + Filename: conflict.Filename, + Reason: conflict.Reason, } } - response := map[string]interface{}{ - "message": mergeErr.Message, - "conflicts": conflictDetails, + response := types.MergeCheckResponse{ + IsConflicted: true, + Conflicts: conflicts, + Message: mergeErr.Message, } writeConflict(w, response) h.l.Error("git: merge conflict", "handler", "MergeCheck", "error", mergeErr.Error()) return } - writeError(w, err.Error(), http.StatusInternalServerError) h.l.Error("git: failed to check merge", "handler", "MergeCheck", "error", err.Error()) } diff --git a/types/merge.go b/types/merge.go index ab1254f4..73bccf0c 100644 --- a/types/merge.go +++ b/types/merge.go @@ -1 +1,13 @@ package types + +type ConflictInfo struct { + Filename string `json:"filename"` + Reason string `json:"reason"` +} + +type MergeCheckResponse struct { + IsConflicted bool `json:"is_conflicted"` + Conflicts []ConflictInfo `json:"conflicts"` + Message string `json:"message"` + Error string `json:"error"` +} -- 2.51.2