simplfy test

cloudfoundry · Nov 28, 2024 · 44af620 · 44af620
1 parent 23e84f2
commit 44af620
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 54 deletions.
diff --git a/src/autoscaler/api/publicapiserver/public_api_handler.go b/src/autoscaler/api/publicapiserver/public_api_handler.go
@@ -10,8 +10,6 @@ import (
 	"os"
 	"reflect"
 
-	"github.com/pivotal-cf/brokerapi/v11/domain/apiresponses"
-
 	"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"
@@ -136,13 +134,6 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
 		writeErrorResponse(w, http.StatusInternalServerError, "Failed to read request body")
 		return
 	}
-	bindingConfiguration, err := h.getBindingConfigurationFromRequest(policyBytes, logger)
-	if err != nil {
-		errMessage := "Failed to read binding configuration request body"
-		logger.Error(errMessage, err)
-		writeErrorResponse(w, http.StatusInternalServerError, errMessage)
-		return
-	}
 
 	policy, errResults := h.policyValidator.ParseAndValidatePolicy(policyBytes)
 	if errResults != nil {
@@ -151,6 +142,14 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
 		return
 	}
 
+	bindingConfiguration, err := h.getBindingConfigurationFromRequest(policyBytes, logger)
+	if err != nil {
+		errMessage := "Failed to read binding configuration request body"
+		logger.Error(errMessage, err)
+		writeErrorResponse(w, http.StatusInternalServerError, errMessage)
+		return
+	}
+
 	policyGuid := uuid.NewString()
 	err = h.policydb.SaveAppPolicy(r.Context(), appId, policy, policyGuid)
 	if err != nil {
@@ -356,12 +355,8 @@ func (h *PublicApiHandler) getBindingConfigurationFromRequest(rawJson json.RawMe
 	if rawJson != nil {
 		err = json.Unmarshal(rawJson, &bindingConfiguration)
 		if err != nil {
-			actionReadBindingConfiguration := "read-binding-configurations"
 			logger.Error("unmarshal-binding-configuration", err)
-			return bindingConfiguration, apiresponses.NewFailureResponseBuilder(
-				ErrInvalidConfigurations, http.StatusBadRequest, actionReadBindingConfiguration).
-				WithErrorKey(actionReadBindingConfiguration).
-				Build()
+			return bindingConfiguration, err
 		}
 	}
 	return bindingConfiguration, err

diff --git a/src/autoscaler/api/publicapiserver/public_api_handler_test.go b/src/autoscaler/api/publicapiserver/public_api_handler_test.go
@@ -382,61 +382,56 @@ var _ = Describe("PublicApiHandler", func() {
 		Context("Binding Configuration", func() {
 			When("reading binding configuration from request fails", func() {
 				BeforeEach(func() {
-					pathVariables["appId"] = TEST_APP_ID
-					req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString("incorrect.json"))
+					req = setupRequest("incorrect.json", TEST_APP_ID, req, pathVariables)
 				})
 				It("should not succeed and fail with 400", func() {
-					Expect(resp.Code).To(Equal(http.StatusInternalServerError))
-					Expect(resp.Body.String()).To(ContainSubstring("Failed to read binding configuration request body"))
+					Expect(resp.Body.String()).To(ContainSubstring("invalid character"))
+					Expect(resp.Code).To(Equal(http.StatusBadRequest))
 				})
 			})
 
 			When("invalid configuration is provided with the policy", func() {
 				BeforeEach(func() {
-					pathVariables["appId"] = TEST_APP_ID
-					req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(InvalidCustomMetricsConfigurationStr))
+					req = setupRequest(InvalidCustomMetricsConfigurationStr, TEST_APP_ID, req, pathVariables)
 					schedulerStatus = 200
 				})
 				It("should not succeed and fail with 400", func() {
+					Expect(resp.Body.String()).To(MatchJSON(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
 					Expect(resp.Code).To(Equal(http.StatusBadRequest))
-					Expect(resp.Body.String()).To(Equal(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
 				})
 			})
+		})
 
-			When("save configuration returned error", func() {
-				BeforeEach(func() {
-					pathVariables["appId"] = TEST_APP_ID
-					req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(validCustomMetricsConfigurationStr))
-					schedulerStatus = 200
-					bindingdb.SetOrUpdateCustomMetricStrategyReturns(fmt.Errorf("failed to save custom metrics configuration"))
-				})
-				It("should not succeed and fail with internal server error", func() {
-					Expect(resp.Code).To(Equal(http.StatusInternalServerError))
-					Expect(resp.Body.String()).To(MatchJSON(`{"code":"Internal Server Error","message":"failed to save custom metric submission strategy in the database"}`))
-				})
+		When("save configuration returned error", func() {
+			BeforeEach(func() {
+				req = setupRequest(validCustomMetricsConfigurationStr, TEST_APP_ID, req, pathVariables)
+				schedulerStatus = 200
+				bindingdb.SetOrUpdateCustomMetricStrategyReturns(fmt.Errorf("failed to save custom metrics configuration"))
+			})
+			It("should not succeed and fail with internal server error", func() {
+				Expect(resp.Code).To(Equal(http.StatusInternalServerError))
+				Expect(resp.Body.String()).To(MatchJSON(`{"code":"Internal Server Error","message":"failed to save custom metric submission strategy in the database"}`))
 			})
+		})
 
-			When("valid configuration is provided with the policy", func() {
-				BeforeEach(func() {
-					pathVariables["appId"] = TEST_APP_ID
-					req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(validCustomMetricsConfigurationStr))
-					schedulerStatus = 200
-				})
-				It("returns the policy and configuration with 200", func() {
-					Expect(resp.Code).To(Equal(http.StatusOK))
-					Expect(resp.Body).To(MatchJSON(validCustomMetricsConfigurationStr))
-				})
+		When("valid configuration is provided with the policy", func() {
+			BeforeEach(func() {
+				req = setupRequest(validCustomMetricsConfigurationStr, TEST_APP_ID, req, pathVariables)
+				schedulerStatus = 200
 			})
-			When("configuration is removed but only policy is provided", func() {
-				BeforeEach(func() {
-					pathVariables["appId"] = TEST_APP_ID
-					req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(ValidPolicyStr))
-					schedulerStatus = 200
-				})
-				It("returns the policy 200", func() {
-					Expect(resp.Body).To(MatchJSON(ValidPolicyStr))
-					Expect(resp.Code).To(Equal(http.StatusOK))
-				})
+			It("returns the policy and configuration with 200", func() {
+				Expect(resp.Code).To(Equal(http.StatusOK))
+				Expect(resp.Body).To(MatchJSON(validCustomMetricsConfigurationStr))
+			})
+		})
+		When("configuration is removed but only policy is provided", func() {
+			BeforeEach(func() {
+				req = setupRequest(ValidPolicyStr, TEST_APP_ID, req, pathVariables)
+				schedulerStatus = 200
+			})
+			It("returns the policy 200", func() {
+				Expect(resp.Body).To(MatchJSON(ValidPolicyStr))
+				Expect(resp.Code).To(Equal(http.StatusOK))
 			})
 		})
 	})
@@ -1018,8 +1013,13 @@ var _ = Describe("PublicApiHandler", func() {
 	})
 })
 
-func setupPolicy(policydb *fakes.FakePolicyDB) {
-	policydb.GetAppPolicyReturns(&models.ScalingPolicy{
+func setupRequest(requestBody string, appId string, req *http.Request, pathVariables map[string]string) *http.Request {
+	pathVariables["appId"] = appId
+	req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(requestBody))
+	return req
+}
+func setupPolicy(policyDb *fakes.FakePolicyDB) {
+	policyDb.GetAppPolicyReturns(&models.ScalingPolicy{
 		InstanceMax: 5,
 		InstanceMin: 1,
 		ScalingRules: []*models.ScalingRule{