From 740e88b55aa7184d8184828b22fffcbfa22322ef Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 24 Jul 2018 11:10:52 +0530 Subject: [PATCH] Issue #255 Use functions for writing response Code for writing response(objects or errors) is repeated a lot. Utilize functions writeResponse and respondWithError for here. Move functions to their appropriate places. Make sure that while writing response following order is maintained ``` set the headers WriteHeader(status) call w.Write( ... ) ``` Added Tests to make sure of this order for writeResponse and respondWithError --- internal/api/api.go | 94 +++++++++++++++++----------------------- internal/api/api_test.go | 16 +++++++ 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index c248e56..fcf7401 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -88,9 +88,7 @@ func NewIdlerAPI(userIdlers *openshift.UserIdlerMap, clusterView cluster.View, t func (api *idler) Idle(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { openShiftAPI, openShiftBearerToken, err := api.getURLAndToken(r) if err != nil { - log.Error(err) - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(fmt.Sprintf("{\"error\": \"%s\"}", err))) + respondWithError(w, http.StatusBadRequest, err) return } @@ -100,10 +98,8 @@ func (api *idler) Idle(w http.ResponseWriter, r *http.Request, ps httprouter.Par elapsedTime := time.Since(startTime).Seconds() if err != nil { - log.Error(err) Recorder.RecordReqDuration(service, "Idle", http.StatusInternalServerError, elapsedTime) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("{\"error\": \"%s\"}", err))) + respondWithError(w, http.StatusInternalServerError, err) return } @@ -172,53 +168,61 @@ func (api *idler) UnIdle(w http.ResponseWriter, r *http.Request, ps httprouter.P func (api *idler) IsIdle(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { openShiftAPI, openShiftBearerToken, err := api.getURLAndToken(r) if err != nil { - log.Error(err) - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(fmt.Sprintf("{\"error\": \"%s\"}", err))) + respondWithError(w, http.StatusBadRequest, err) return } - w.Header().Set("Content-Type", "application/json") state, err := api.openShiftClient.State(openShiftAPI, openShiftBearerToken, ps.ByName("namespace"), "jenkins") if err != nil { - log.Error(err) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("{\"error\": \"%s\"}", err))) + respondWithError(w, http.StatusInternalServerError, err) return } s := status{} s.IsIdle = state < model.PodRunning - w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(s) + writeResponse(w, http.StatusOK, s) } -func (api *idler) ClusterDNSView(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - w.Header().Set("Content-Type", "application/json") +func (api *idler) Status(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + response := &statusResponse{} - clusterDNSView := api.clusterView.GetDNSView() - w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(clusterDNSView) + openshiftURL, openshiftToken, err := api.getURLAndToken(r) + if err != nil { + response.AppendError(tokenFetchFailed, "failed to obtain openshift token: "+err.Error()) + writeResponse(w, http.StatusBadRequest, *response) + return + } + + state, err := api.openShiftClient.State( + openshiftURL, openshiftToken, + ps.ByName("namespace"), + "jenkins", + ) + if err != nil { + response.AppendError(openShiftClientError, "openshift client error: "+err.Error()) + writeResponse(w, http.StatusInternalServerError, *response) + return + } + + response.SetState(state) + writeResponse(w, http.StatusOK, *response) } func (api *idler) Info(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - w.Header().Set("Content-Type", "application/json") namespace := ps.ByName("namespace") userIdler, ok := api.userIdlers.Load(namespace) if ok { - err := json.NewEncoder(w).Encode(userIdler.GetUser()) - - if err != nil { - log.Errorf("Could not serialize users: %s", err) - w.Write([]byte(fmt.Sprintf("{\"error\": \"Could not serialize users: %s\"}", err))) - w.WriteHeader(http.StatusInternalServerError) - } + writeResponse(w, http.StatusOK, userIdler.GetUser()) } else { - w.WriteHeader(http.StatusNotFound) + respondWithError(w, http.StatusNotFound, fmt.Errorf("Could not find queried namespace")) } } +func (api *idler) ClusterDNSView(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + writeResponse(w, http.StatusOK, api.clusterView.GetDNSView()) +} + func (api *idler) Reset(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { logger := log.WithFields(log.Fields{"component": "api", "function": "Reset"}) @@ -269,6 +273,7 @@ func (api idler) isJenkinsUnIdled(openshiftURL, openshiftToken, namespace string func respondWithError(w http.ResponseWriter, status int, err error) { log.Error(err) + w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) w.Write([]byte(fmt.Sprintf("{\"error\": \"%s\"}", err))) } @@ -308,35 +313,14 @@ func (s *statusResponse) SetState(state model.PodState) *statusResponse { return s } -func (api *idler) Status(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - response := statusResponse{} - - openshiftURL, openshiftToken, err := api.getURLAndToken(r) - if err != nil { - response.AppendError(tokenFetchFailed, "failed to obtain openshift token: "+err.Error()) - writeResponse(w, http.StatusBadRequest, response) - return - } - - state, err := api.openShiftClient.State( - openshiftURL, openshiftToken, - ps.ByName("namespace"), - "jenkins", - ) - if err != nil { - response.AppendError(openShiftClientError, "openshift client error: "+err.Error()) - writeResponse(w, http.StatusInternalServerError, response) - return - } - - response.SetState(state) - writeResponse(w, http.StatusOK, response) -} - type any interface{} func writeResponse(w http.ResponseWriter, status int, response any) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) - json.NewEncoder(w).Encode(response) + err := json.NewEncoder(w).Encode(response) + if err != nil { + respondWithError(w, http.StatusInternalServerError, fmt.Errorf("Could not serialize the response: %s", err)) + return + } } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index fc28e7a..28059a2 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -2,8 +2,10 @@ package api import ( "encoding/json" + "errors" "fmt" "net/http" + "net/http/httptest" "testing" "github.com/fabric8-services/fabric8-jenkins-idler/internal/testutils/mock" @@ -148,3 +150,17 @@ func Test_Status_BadRequest_fail(t *testing.T) { require.Contains(t, sr.Errors[0].Description, "failed to obtain openshift token", "Error must have a description") } + +func Test_writeFunctions(t *testing.T) { + w := httptest.NewRecorder() + testStatus := http.StatusBadRequest + writeResponse(w, testStatus, nil) + require.Equal(t, testStatus, w.Code, "in writeResponse, response was written before setting the HTTP status code") + + w = httptest.NewRecorder() + testStatus = http.StatusInternalServerError + err := errors.New("Mock error") + respondWithError(w, testStatus, err) + require.Equal(t, testStatus, w.Code, "in respondWithError, response was written before setting the HTTP status code") + +}