Centralize API response handling
Currently, all API handler functions take care of response code and message on their own. This leads to a huge injection chain for HTTP related objects. This refactors the API to consistently return response code and message to the API main entrypoint where the response is created and sent. Now, SonarQube and Gitea will get a response at the very end of any bot action for one request. SonarQube has a timeout of 10 seconds, which may be reached due to network latency. We'll see. Signed-off-by: Steven Kriegler <sk.bunsenbrenner@gmail.com>
This commit is contained in:
parent
99fe6800b0
commit
525fa03065
|
@ -1,8 +1,6 @@
|
|||
package api
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"log"
|
||||
"net/http"
|
||||
|
@ -14,8 +12,8 @@ import (
|
|||
)
|
||||
|
||||
type GiteaWebhookHandlerInferface interface {
|
||||
HandleSynchronize(rw http.ResponseWriter, r *http.Request)
|
||||
HandleComment(rw http.ResponseWriter, r *http.Request)
|
||||
HandleSynchronize(r *http.Request) (int, string)
|
||||
HandleComment(r *http.Request) (int, string)
|
||||
}
|
||||
|
||||
type GiteaWebhookHandler struct {
|
||||
|
@ -23,7 +21,7 @@ type GiteaWebhookHandler struct {
|
|||
sqSdk sqSdk.SonarQubeSdkInterface
|
||||
}
|
||||
|
||||
func (h *GiteaWebhookHandler) parseBody(rw http.ResponseWriter, r *http.Request) ([]byte, error) {
|
||||
func (h *GiteaWebhookHandler) parseBody(r *http.Request) ([]byte, error) {
|
||||
if r.Body != nil {
|
||||
defer r.Body.Close()
|
||||
}
|
||||
|
@ -32,82 +30,62 @@ func (h *GiteaWebhookHandler) parseBody(rw http.ResponseWriter, r *http.Request)
|
|||
|
||||
if err != nil {
|
||||
log.Printf("Error reading request body %s", err.Error())
|
||||
rw.WriteHeader(http.StatusInternalServerError)
|
||||
io.WriteString(rw, fmt.Sprintf(`{"message": "%s"}`, err.Error()))
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
func (h *GiteaWebhookHandler) HandleSynchronize(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.Header().Set("Content-Type", "application/json")
|
||||
|
||||
raw, err := h.parseBody(rw, r)
|
||||
func (h *GiteaWebhookHandler) HandleSynchronize(r *http.Request) (int, string) {
|
||||
raw, err := h.parseBody(r)
|
||||
if err != nil {
|
||||
return
|
||||
return http.StatusInternalServerError, err.Error()
|
||||
}
|
||||
|
||||
ok, err := isValidWebhook(raw, settings.Gitea.Webhook.Secret, r.Header.Get("X-Gitea-Signature"), "Gitea")
|
||||
if !ok {
|
||||
log.Print(err.Error())
|
||||
rw.WriteHeader(http.StatusPreconditionFailed)
|
||||
io.WriteString(rw, fmt.Sprint(`{"message": "Webhook validation failed. Request rejected."}`))
|
||||
return
|
||||
return http.StatusPreconditionFailed, "Webhook validation failed. Request rejected."
|
||||
}
|
||||
|
||||
w, ok := webhook.NewPullWebhook(raw)
|
||||
if !ok {
|
||||
rw.WriteHeader(http.StatusUnprocessableEntity)
|
||||
io.WriteString(rw, `{"message": "Error parsing POST body."}`)
|
||||
return
|
||||
return http.StatusUnprocessableEntity, "Error parsing POST body."
|
||||
}
|
||||
|
||||
if err := w.Validate(); err != nil {
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
io.WriteString(rw, fmt.Sprintf(`{"message": "%s"}`, err.Error()))
|
||||
return
|
||||
return http.StatusOK, err.Error()
|
||||
}
|
||||
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
io.WriteString(rw, `{"message": "Processing data. See bot logs for details."}`)
|
||||
|
||||
w.ProcessData(h.giteaSdk, h.sqSdk)
|
||||
|
||||
return http.StatusOK, "Processing data. See bot logs for details."
|
||||
}
|
||||
|
||||
func (h *GiteaWebhookHandler) HandleComment(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.Header().Set("Content-Type", "application/json")
|
||||
|
||||
raw, err := h.parseBody(rw, r)
|
||||
func (h *GiteaWebhookHandler) HandleComment(r *http.Request) (int, string) {
|
||||
raw, err := h.parseBody(r)
|
||||
if err != nil {
|
||||
return
|
||||
return http.StatusInternalServerError, err.Error()
|
||||
}
|
||||
|
||||
ok, err := isValidWebhook(raw, settings.Gitea.Webhook.Secret, r.Header.Get("X-Gitea-Signature"), "Gitea")
|
||||
if !ok {
|
||||
log.Print(err.Error())
|
||||
rw.WriteHeader(http.StatusPreconditionFailed)
|
||||
io.WriteString(rw, `{"message": "Webhook validation failed. Request rejected."}`)
|
||||
return
|
||||
return http.StatusPreconditionFailed, "Webhook validation failed. Request rejected."
|
||||
}
|
||||
|
||||
w, ok := webhook.NewCommentWebhook(raw)
|
||||
if !ok {
|
||||
rw.WriteHeader(http.StatusUnprocessableEntity)
|
||||
io.WriteString(rw, `{"message": "Error parsing POST body."}`)
|
||||
return
|
||||
return http.StatusUnprocessableEntity, "Error parsing POST body."
|
||||
}
|
||||
|
||||
if err := w.Validate(); err != nil {
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
io.WriteString(rw, fmt.Sprintf(`{"message": "%s"}`, err.Error()))
|
||||
return
|
||||
return http.StatusOK, err.Error()
|
||||
}
|
||||
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
io.WriteString(rw, `{"message": "Processing data. See bot logs for details."}`)
|
||||
|
||||
w.ProcessData(h.giteaSdk, h.sqSdk)
|
||||
|
||||
return http.StatusOK, "Processing data. See bot logs for details."
|
||||
}
|
||||
|
||||
func NewGiteaWebhookHandler(g giteaSdk.GiteaSdkInterface, sq sqSdk.SonarQubeSdkInterface) GiteaWebhookHandlerInferface {
|
||||
|
|
|
@ -2,6 +2,8 @@ package api
|
|||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
@ -20,7 +22,12 @@ func withValidGiteaCommentRequestData(t *testing.T, jsonBody []byte) (*http.Requ
|
|||
}
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
handler := http.HandlerFunc(webhookHandler.HandleComment)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
status, response := webhookHandler.HandleComment(r)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
io.WriteString(w, fmt.Sprintf(`{"message": "%s"}`, response))
|
||||
})
|
||||
|
||||
return req, rr, handler
|
||||
}
|
||||
|
@ -34,7 +41,12 @@ func withValidGiteaSynchronizeRequestData(t *testing.T, jsonBody []byte) (*http.
|
|||
}
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
handler := http.HandlerFunc(webhookHandler.HandleSynchronize)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
status, response := webhookHandler.HandleSynchronize(r)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
io.WriteString(w, fmt.Sprintf(`{"message": "%s"}`, response))
|
||||
})
|
||||
|
||||
return req, rr, handler
|
||||
}
|
||||
|
|
|
@ -40,7 +40,10 @@ func (s *ApiServer) setup() {
|
|||
return
|
||||
}
|
||||
|
||||
s.sonarQubeWebhookHandler.Handle(c.Writer, c.Request)
|
||||
status, response := s.sonarQubeWebhookHandler.Handle(c.Request)
|
||||
c.JSON(status, gin.H{
|
||||
"message": response,
|
||||
})
|
||||
}).POST("/hooks/gitea", func(c *gin.Context) {
|
||||
h := validGiteaEndpointHeader{}
|
||||
|
||||
|
@ -49,16 +52,22 @@ func (s *ApiServer) setup() {
|
|||
return
|
||||
}
|
||||
|
||||
var status int
|
||||
var response string
|
||||
|
||||
switch h.GiteaEvent {
|
||||
case "pull_request":
|
||||
s.giteaWebhookHandler.HandleSynchronize(c.Writer, c.Request)
|
||||
status, response = s.giteaWebhookHandler.HandleSynchronize(c.Request)
|
||||
case "issue_comment":
|
||||
s.giteaWebhookHandler.HandleComment(c.Writer, c.Request)
|
||||
status, response = s.giteaWebhookHandler.HandleComment(c.Request)
|
||||
default:
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"message": "ignore unknown event",
|
||||
})
|
||||
status = http.StatusOK
|
||||
response = "ignore unknown event"
|
||||
}
|
||||
|
||||
c.JSON(status, gin.H{
|
||||
"message": response,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
@ -22,20 +22,23 @@ type SonarQubeHandlerMock struct {
|
|||
mock.Mock
|
||||
}
|
||||
|
||||
func (h *SonarQubeHandlerMock) Handle(rw http.ResponseWriter, r *http.Request) {
|
||||
h.Called(rw, r)
|
||||
func (h *SonarQubeHandlerMock) Handle(r *http.Request) (int, string) {
|
||||
h.Called(r)
|
||||
return http.StatusOK, "test-execution"
|
||||
}
|
||||
|
||||
type GiteaHandlerMock struct {
|
||||
mock.Mock
|
||||
}
|
||||
|
||||
func (h *GiteaHandlerMock) HandleSynchronize(rw http.ResponseWriter, r *http.Request) {
|
||||
h.Called(rw, r)
|
||||
func (h *GiteaHandlerMock) HandleSynchronize(r *http.Request) (int, string) {
|
||||
h.Called(r)
|
||||
return http.StatusOK, "test-execution"
|
||||
}
|
||||
|
||||
func (h *GiteaHandlerMock) HandleComment(rw http.ResponseWriter, r *http.Request) {
|
||||
h.Called(rw, r)
|
||||
func (h *GiteaHandlerMock) HandleComment(r *http.Request) (int, string) {
|
||||
h.Called(r)
|
||||
return http.StatusOK, "test-execution"
|
||||
}
|
||||
|
||||
type GiteaSdkMock struct {
|
||||
|
@ -83,6 +86,8 @@ func (h *SQSdkMock) ComposeGiteaComment(data *sqSdk.CommentComposeData) (string,
|
|||
// SETUP: mute logs
|
||||
func TestMain(m *testing.M) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
gin.DefaultWriter = ioutil.Discard
|
||||
gin.DefaultErrorWriter = ioutil.Discard
|
||||
log.SetOutput(ioutil.Discard)
|
||||
os.Exit(m.Run())
|
||||
}
|
||||
|
@ -113,7 +118,7 @@ func TestSonarQubeAPIRouteMissingProjectHeader(t *testing.T) {
|
|||
|
||||
func TestSonarQubeAPIRouteProcessing(t *testing.T) {
|
||||
sonarQubeHandlerMock := new(SonarQubeHandlerMock)
|
||||
sonarQubeHandlerMock.On("Handle", mock.Anything, mock.Anything).Return(nil)
|
||||
sonarQubeHandlerMock.On("Handle", mock.IsType(&http.Request{}))
|
||||
|
||||
router := New(new(GiteaHandlerMock), sonarQubeHandlerMock)
|
||||
|
||||
|
@ -124,6 +129,7 @@ func TestSonarQubeAPIRouteProcessing(t *testing.T) {
|
|||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
sonarQubeHandlerMock.AssertNumberOfCalls(t, "Handle", 1)
|
||||
sonarQubeHandlerMock.AssertExpectations(t)
|
||||
}
|
||||
|
||||
func TestGiteaAPIRouteMissingEventHeader(t *testing.T) {
|
||||
|
@ -139,7 +145,7 @@ func TestGiteaAPIRouteMissingEventHeader(t *testing.T) {
|
|||
func TestGiteaAPIRouteSynchronizeProcessing(t *testing.T) {
|
||||
giteaHandlerMock := new(GiteaHandlerMock)
|
||||
giteaHandlerMock.On("HandleSynchronize", mock.Anything, mock.Anything).Return(nil)
|
||||
giteaHandlerMock.On("HandleComment", mock.Anything, mock.Anything).Return(nil)
|
||||
giteaHandlerMock.On("HandleComment", mock.Anything, mock.Anything).Maybe()
|
||||
|
||||
router := New(giteaHandlerMock, new(SonarQubeHandlerMock))
|
||||
|
||||
|
@ -151,11 +157,12 @@ func TestGiteaAPIRouteSynchronizeProcessing(t *testing.T) {
|
|||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleSynchronize", 1)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleComment", 0)
|
||||
giteaHandlerMock.AssertExpectations(t)
|
||||
}
|
||||
|
||||
func TestGiteaAPIRouteCommentProcessing(t *testing.T) {
|
||||
giteaHandlerMock := new(GiteaHandlerMock)
|
||||
giteaHandlerMock.On("HandleSynchronize", mock.Anything, mock.Anything).Return(nil)
|
||||
giteaHandlerMock.On("HandleSynchronize", mock.Anything, mock.Anything).Maybe()
|
||||
giteaHandlerMock.On("HandleComment", mock.Anything, mock.Anything).Return(nil)
|
||||
|
||||
router := New(giteaHandlerMock, new(SonarQubeHandlerMock))
|
||||
|
@ -168,12 +175,13 @@ func TestGiteaAPIRouteCommentProcessing(t *testing.T) {
|
|||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleSynchronize", 0)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleComment", 1)
|
||||
giteaHandlerMock.AssertExpectations(t)
|
||||
}
|
||||
|
||||
func TestGiteaAPIRouteUnknownEvent(t *testing.T) {
|
||||
giteaHandlerMock := new(GiteaHandlerMock)
|
||||
giteaHandlerMock.On("HandleSynchronize", mock.Anything, mock.Anything).Return(nil)
|
||||
giteaHandlerMock.On("HandleComment", mock.Anything, mock.Anything).Return(nil)
|
||||
giteaHandlerMock.On("HandleSynchronize", mock.Anything, mock.Anything).Maybe()
|
||||
giteaHandlerMock.On("HandleComment", mock.Anything, mock.Anything).Maybe()
|
||||
|
||||
router := New(giteaHandlerMock, new(SonarQubeHandlerMock))
|
||||
|
||||
|
@ -185,4 +193,5 @@ func TestGiteaAPIRouteUnknownEvent(t *testing.T) {
|
|||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleSynchronize", 0)
|
||||
giteaHandlerMock.AssertNumberOfCalls(t, "HandleComment", 0)
|
||||
giteaHandlerMock.AssertExpectations(t)
|
||||
}
|
||||
|
|
|
@ -2,7 +2,6 @@ package api
|
|||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"log"
|
||||
"net/http"
|
||||
|
@ -15,7 +14,7 @@ import (
|
|||
)
|
||||
|
||||
type SonarQubeWebhookHandlerInferface interface {
|
||||
Handle(rw http.ResponseWriter, r *http.Request)
|
||||
Handle(r *http.Request) (int, string)
|
||||
}
|
||||
|
||||
type SonarQubeWebhookHandler struct {
|
||||
|
@ -56,17 +55,12 @@ func (h *SonarQubeWebhookHandler) processData(w *webhook.Webhook, repo settings.
|
|||
h.giteaSdk.PostComment(repo, w.PRIndex, comment)
|
||||
}
|
||||
|
||||
func (h *SonarQubeWebhookHandler) Handle(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.Header().Set("Content-Type", "application/json")
|
||||
|
||||
func (h *SonarQubeWebhookHandler) Handle(r *http.Request) (int, string) {
|
||||
projectName := r.Header.Get("X-SonarQube-Project")
|
||||
found, pIdx := h.inProjectsMapping(settings.Projects, projectName)
|
||||
if !found {
|
||||
log.Printf("Received hook for project '%s' which is not configured. Request ignored.", projectName)
|
||||
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
io.WriteString(rw, fmt.Sprintf(`{"message": "Project '%s' not in configured list. Request ignored."}`, projectName))
|
||||
return
|
||||
return http.StatusOK, fmt.Sprintf("Project '%s' not in configured list. Request ignored.", projectName)
|
||||
}
|
||||
|
||||
log.Printf("Received hook for project '%s'. Processing data.", projectName)
|
||||
|
@ -78,38 +72,28 @@ func (h *SonarQubeWebhookHandler) Handle(rw http.ResponseWriter, r *http.Request
|
|||
raw, err := ioutil.ReadAll(r.Body)
|
||||
if err != nil {
|
||||
log.Printf("Error reading request body %s", err.Error())
|
||||
rw.WriteHeader(http.StatusInternalServerError)
|
||||
io.WriteString(rw, fmt.Sprintf(`{"message": "%s"}`, err.Error()))
|
||||
return
|
||||
return http.StatusInternalServerError, err.Error()
|
||||
}
|
||||
|
||||
ok, err := isValidWebhook(raw, settings.SonarQube.Webhook.Secret, r.Header.Get("X-Sonar-Webhook-HMAC-SHA256"), "SonarQube")
|
||||
if !ok {
|
||||
log.Print(err.Error())
|
||||
rw.WriteHeader(http.StatusPreconditionFailed)
|
||||
io.WriteString(rw, `{"message": "Webhook validation failed. Request rejected."}`)
|
||||
return
|
||||
return http.StatusPreconditionFailed, "Webhook validation failed. Request rejected."
|
||||
}
|
||||
|
||||
w, ok := webhook.New(raw)
|
||||
if !ok {
|
||||
rw.WriteHeader(http.StatusUnprocessableEntity)
|
||||
io.WriteString(rw, `{"message": "Error parsing POST body."}`)
|
||||
return
|
||||
return http.StatusUnprocessableEntity, "Error parsing POST body."
|
||||
}
|
||||
|
||||
// Send response to SonarQube at this point to ensure being within 10 seconds limit of webhook response timeout
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
|
||||
if strings.ToLower(w.Branch.Type) != "pull_request" {
|
||||
io.WriteString(rw, `{"message": "Ignore Hook for non-PR analysis."}`)
|
||||
log.Println("Ignore Hook for non-PR analysis")
|
||||
return
|
||||
return http.StatusOK, "Ignore Hook for non-PR analysis."
|
||||
}
|
||||
|
||||
io.WriteString(rw, `{"message": "Processing data. See bot logs for details."}`)
|
||||
|
||||
h.processData(w, settings.Projects[pIdx].Gitea)
|
||||
|
||||
return http.StatusOK, "Processing data. See bot logs for details."
|
||||
}
|
||||
|
||||
func NewSonarQubeWebhookHandler(g giteaSdk.GiteaSdkInterface, sq sqSdk.SonarQubeSdkInterface) SonarQubeWebhookHandlerInferface {
|
||||
|
|
|
@ -2,6 +2,8 @@ package api
|
|||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"regexp"
|
||||
|
@ -22,7 +24,12 @@ func withValidSonarQubeRequestData(t *testing.T, jsonBody []byte) (*http.Request
|
|||
req.Header.Set("X-SonarQube-Project", "pr-bot")
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
handler := http.HandlerFunc(webhookHandler.Handle)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
status, response := webhookHandler.Handle(r)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
io.WriteString(w, fmt.Sprintf(`{"message": "%s"}`, response))
|
||||
})
|
||||
|
||||
return req, rr, handler
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue