address some sonarqube findings

src/autoscaler/api/publicapiserver/public_api_handler.go

@@ -1,20 +1,19 @@
 package publicapiserver
 
 import (
-	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"os"
 	"reflect"
 
-	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/cred_helper"
-
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/config"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/policyvalidator"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/schedulerclient"
+	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/cred_helper"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/helpers"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/models"
@@ -37,6 +36,12 @@ type PublicApiHandler struct {
 	credentials          cred_helper.Credentials
 }
 
+const (
+	ActionWriteBody             = "write-body"
+	ActionCheckAppId            = "check-for-id-appid"
+	ErrorMessageAppidIsRequired = "AppId is required"
+)
+
 func NewPublicApiHandler(logger lager.Logger, conf *config.Config, policydb db.PolicyDB, bindingdb db.BindingDB, credentials cred_helper.Credentials) *PublicApiHandler {
 	seClient, err := helpers.CreateHTTPClient(&conf.ScalingEngine.TLSClientCerts, helpers.DefaultClientConfig(), logger.Session("scaling_client"))
 	if err != nil {
@@ -72,8 +77,8 @@ func writeErrorResponse(w http.ResponseWriter, statusCode int, message string) {
 func (h *PublicApiHandler) GetScalingPolicy(w http.ResponseWriter, r *http.Request, vars map[string]string) {
 	appId := vars["appId"]
 	if appId == "" {
-		h.logger.Error("AppId is missing", nil, nil)
-		writeErrorResponse(w, http.StatusBadRequest, "AppId is required")
+		h.logger.Error(ActionCheckAppId, errors.New(ErrorMessageAppidIsRequired), nil)
+		writeErrorResponse(w, http.StatusBadRequest, ErrorMessageAppidIsRequired)
 		return
 	}
 	logger := h.logger.Session("GetScalingPolicy", lager.Data{"appId": appId})
@@ -92,27 +97,14 @@ func (h *PublicApiHandler) GetScalingPolicy(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	bf := bytes.NewBuffer([]byte{})
-	jsonEncoder := json.NewEncoder(bf)
-	jsonEncoder.SetEscapeHTML(false)
-	err = jsonEncoder.Encode(scalingPolicy)
-	if err != nil {
-		logger.Error("Failed to json encode scaling policy", err, lager.Data{"policy": fmt.Sprintf("%+v", scalingPolicy)})
-		writeErrorResponse(w, http.StatusInternalServerError, "Error encode scaling policy")
-		return
-	}
-	w.Header().Set("Content-Type", "application/json")
-	_, err = w.Write(bf.Bytes())
-	if err != nil {
-		logger.Error("failed-to-write-body", err)
-	}
+	handlers.WriteJSONResponse(w, http.StatusOK, scalingPolicy)
 }
 
 func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Request, vars map[string]string) {
 	appId := vars["appId"]
 	if appId == "" {
-		h.logger.Error("AppId is missing", nil, nil)
-		writeErrorResponse(w, http.StatusBadRequest, "AppId is required")
+		h.logger.Error(ActionCheckAppId, errors.New(ErrorMessageAppidIsRequired), nil)
+		writeErrorResponse(w, http.StatusBadRequest, ErrorMessageAppidIsRequired)
 		return
 	}
 
@@ -169,8 +161,8 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
 func (h *PublicApiHandler) DetachScalingPolicy(w http.ResponseWriter, r *http.Request, vars map[string]string) {
 	appId := vars["appId"]
 	if appId == "" {
-		h.logger.Error("AppId is missing", nil, nil)
-		writeErrorResponse(w, http.StatusBadRequest, "AppId is required")
+		h.logger.Error(ActionCheckAppId, errors.New(ErrorMessageAppidIsRequired), nil)
+		writeErrorResponse(w, http.StatusBadRequest, ErrorMessageAppidIsRequired)
 		return
 	}
 	logger := h.logger.Session("DetachScalingPolicy", lager.Data{"appId": appId})
@@ -233,7 +225,7 @@ func (h *PublicApiHandler) DetachScalingPolicy(w http.ResponseWriter, r *http.Re
 	w.WriteHeader(http.StatusOK)
 	_, err = w.Write([]byte("{}"))
 	if err != nil {
-		logger.Error("failed-to-write-body", err)
+		logger.Error(ActionWriteBody, err)
 	}
 }
 
@@ -341,22 +333,22 @@ func (h *PublicApiHandler) GetApiInfo(w http.ResponseWriter, _ *http.Request, _
 
 	_, err = w.Write(info)
 	if err != nil {
-		h.logger.Error("failed-to-write-body", err)
+		h.logger.Error(ActionWriteBody, err)
 	}
 }
 
 func (h *PublicApiHandler) GetHealth(w http.ResponseWriter, _ *http.Request, _ map[string]string) {
 	_, err := w.Write([]byte(`{"alive":"true"}`))
 	if err != nil {
-		h.logger.Error("failed-to-write-body", err)
+		h.logger.Error(ActionWriteBody, err)
 	}
 }
 
 func (h *PublicApiHandler) CreateCredential(w http.ResponseWriter, r *http.Request, vars map[string]string) {
 	appId := vars["appId"]
 	if appId == "" {
-		h.logger.Error("AppId is missing", nil, nil)
-		writeErrorResponse(w, http.StatusBadRequest, "AppId is required")
+		h.logger.Error(ActionCheckAppId, errors.New(ErrorMessageAppidIsRequired), nil)
+		writeErrorResponse(w, http.StatusBadRequest, ErrorMessageAppidIsRequired)
 		return
 	}
 	logger := h.logger.Session("CreateCredential", lager.Data{"appId": appId})
@@ -403,8 +395,8 @@ func (h *PublicApiHandler) CreateCredential(w http.ResponseWriter, r *http.Reque
 func (h *PublicApiHandler) DeleteCredential(w http.ResponseWriter, r *http.Request, vars map[string]string) {
 	appId := vars["appId"]
 	if appId == "" {
-		h.logger.Error("AppId is missing", nil, nil)
-		writeErrorResponse(w, http.StatusBadRequest, "AppId is required")
+		h.logger.Error(ActionCheckAppId, errors.New(ErrorMessageAppidIsRequired), nil)
+		writeErrorResponse(w, http.StatusBadRequest, ErrorMessageAppidIsRequired)
 		return
 	}
 	logger := h.logger.Session("DeleteCredential", lager.Data{"appId": appId})

src/autoscaler/helpers/handlers/handlers.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"bytes"
 	"encoding/json"
 	"net/http"
 	"strconv"
@@ -13,16 +14,25 @@ var handlersLogger = helpers.InitLoggerFromConfig(&helpers.LoggingConfig{Level:
 
 func WriteJSONResponse(w http.ResponseWriter, statusCode int, jsonObj interface{}) {
 	logger := handlersLogger.Session("WriteJSONResponse", lager.Data{"json": jsonObj, "statusCode": statusCode})
-	jsonBytes, err := json.Marshal(jsonObj)
+
+	jsonBytes := new(bytes.Buffer)
+	jsonEncoder := json.NewEncoder(jsonBytes)
+	jsonEncoder.SetEscapeHTML(false)
+	err := jsonEncoder.Encode(jsonObj)
+
+	// json.Encoder adds a newline in the end as it is typically used to encode multiple objects
+	// however we only use it to be able to turn off escaping HTML characters
+	result := bytes.TrimRight(jsonBytes.Bytes(), "\n")
+
 	if err != nil {
 		logger.Error("marshall-json-response", err)
 		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
 		return
 	}
-	w.Header().Set("Content-Length", strconv.Itoa(len(jsonBytes)))
+	w.Header().Set("Content-Length", strconv.Itoa(len(result)))
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(statusCode)
-	_, err = w.Write(jsonBytes)
+	_, err = w.Write(result)
 	if err != nil {
 		logger.Error("write-json-response", err)
 	}