simplify, clean, refactor

src/acceptance/api/api_test.go

@@ -112,12 +112,6 @@ var _ = Describe("AutoScaler Public API", func() {
 			Expect(string(response)).Should(ContainSubstring(`[{"context":"(root).instance_min_count","description":"Must be greater than or equal to 1"}]`))
 		})
 
-		// custom metrics strategy - FIXME This test can be removed as it is covered by "should fail with 404 when retrieve policy"
-		It("should fail with 404 when trying to create custom metrics submission without policy", func() {
-			_, status := getPolicy()
-			Expect(status).To(Equal(404))
-		})
-
 		It("should fail to create an invalid custom metrics submission", func() {
 			response, status := createPolicy(GenerateBindingsWithScalingPolicy("invalid-value", 1, 2, "memoryused", 30, 100))
 			Expect(string(response)).Should(ContainSubstring(`configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\", \"same_app\""}`))

src/autoscaler/api/broker/broker.go

@@ -508,7 +508,7 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 		logger.Error("get-default-policy", err)
 		return result, err
 	}
-	bindingConfiguration, err = bindingConfiguration.ValidateOrGetDefaultCustomMetricsStrategy(bindingConfiguration)
+	bindingConfiguration, err = bindingConfiguration.ValidateOrGetDefaultCustomMetricsStrategy()
 	if err != nil {
 		actionName := "validate-or-get-default-custom-metric-strategy"
 		logger.Error(actionName, err)
@@ -728,11 +728,11 @@ func (b *Broker) GetBinding(ctx context.Context, instanceID string, bindingID st
 		return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(errors.New("failed to retrieve scaling policy"), http.StatusInternalServerError, "get-policy")
 	}
 	if policy != nil {
-		andPolicy, err := models.DetermineBindingConfigAndPolicy(policy, serviceBinding.CustomMetricsStrategy)
+		policyAndBinding, err := models.GetBindingConfigAndPolicy(policy, serviceBinding.CustomMetricsStrategy)
 		if err != nil {
-			return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(errors.New("failed to build policy and config response object"), http.StatusInternalServerError, "determine-BindingConfig-and-Policy")
+			return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(errors.New("failed to build policy and config object"), http.StatusInternalServerError, "determine-BindingConfig-and-Policy")
 		}
-		result.Parameters = andPolicy
+		result.Parameters = policyAndBinding
 	}
 	return result, nil
 }

src/autoscaler/api/broker/broker_test.go

@@ -26,7 +26,7 @@ var _ = Describe("Broker", func() {
 		fakePolicyDB             *fakes.FakePolicyDB
 		fakeCredentials          *fakes.FakeCredentials
 		testLogger               = lagertest.NewTestLogger("test")
-		bindingConfigWithScaling *models.BindingConfigWithPolicy
+		bindingConfigWithScaling *models.ScalingPolicyWithBindingConfig
 	)
 
 	BeforeEach(func() {
@@ -187,10 +187,6 @@ var _ = Describe("Broker", func() {
 			})
 			Context("with configuration and policy", func() {
 				BeforeEach(func() {
-					_ = &models.BindingConfig{Configuration: models.Configuration{CustomMetrics: models.CustomMetricsConfig{
-						MetricSubmissionStrategy: models.MetricsSubmissionStrategy{AllowFrom: "bound_app"},
-					},
-					}}
 					fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId,
 						ServiceInstanceID: testInstanceId, AppID: testAppId, CustomMetricsStrategy: "bound_app"}, nil)
 					bindingBytes, err := os.ReadFile("testdata/policy-with-configs.json")
@@ -216,7 +212,7 @@ var _ = Describe("Broker", func() {
 					Expect(err).ShouldNot(HaveOccurred())
 					fakePolicyDB.GetAppPolicyReturns(nil, nil)
 				})
-				It("returns the bindings with configs in parameters", func() {
+				It("returns no binding configs in parameters", func() {
 					Expect(err).To(BeNil())
 					Expect(Binding).To(Equal(domain.GetBindingSpec{Parameters: nil}))
 				})

src/autoscaler/api/publicapiserver/public_api_handler.go

@@ -110,13 +110,13 @@ func (h *PublicApiHandler) GetScalingPolicy(w http.ResponseWriter, r *http.Reque
 		writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy")
 		return
 	}
-	scalingPolicyResult, err := models.DetermineBindingConfigAndPolicy(scalingPolicy, customMetricStrategy)
+	scalingPolicyWithCustomMetricStrategy, err := models.GetBindingConfigAndPolicy(scalingPolicy, customMetricStrategy)
 	if err != nil {
 		logger.Error("Failed to build policy and config response object", err)
 		writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy")
 		return
 	}
-	handlers.WriteJSONResponse(w, http.StatusOK, scalingPolicyResult)
+	handlers.WriteJSONResponse(w, http.StatusOK, scalingPolicyWithCustomMetricStrategy)
 }
 
 func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Request, vars map[string]string) {
@@ -167,28 +167,28 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	validatedBindingConfiguration, err := bindingConfiguration.ValidateOrGetDefaultCustomMetricsStrategy(bindingConfiguration)
+	validatedBindingConfiguration, err := bindingConfiguration.ValidateOrGetDefaultCustomMetricsStrategy()
 	if err != nil {
 		logger.Error(ErrInvalidConfigurations.Error(), err)
 		writeErrorResponse(w, http.StatusBadRequest, err.Error())
 		return
 	}
-	strategy := validatedBindingConfiguration.GetCustomMetricsStrategy()
-	logger.Info("saving custom metric submission strategy", lager.Data{"strategy": strategy, "appId": appId})
+	customMetricStrategy := validatedBindingConfiguration.GetCustomMetricsStrategy()
+	logger.Info("saving custom metric submission strategy", lager.Data{"customMetricStrategy": customMetricStrategy, "appId": appId})
 	err = h.bindingdb.SetOrUpdateCustomMetricStrategy(r.Context(), appId, validatedBindingConfiguration.GetCustomMetricsStrategy(), "update")
 	if err != nil {
 		actionName := "failed to save custom metric submission strategy in the database"
 		logger.Error(actionName, err)
 		writeErrorResponse(w, http.StatusInternalServerError, actionName)
 		return
 	}
-	response, err := h.buildResponse(strategy, validatedBindingConfiguration, policy)
+	responseJson, err := buildResponse(policy, customMetricStrategy, logger)
 	if err != nil {
-		logger.Error("Failed to marshal policy", err)
+		logger.Error("Failed to to build response", err)
 		writeErrorResponse(w, http.StatusInternalServerError, "Error building response")
 		return
 	}
-	_, err = w.Write(response)
+	_, err = w.Write(responseJson)
 	if err != nil {
 		logger.Error("Failed to write body", err)
 	}
@@ -350,11 +350,11 @@ func (h *PublicApiHandler) GetHealth(w http.ResponseWriter, _ *http.Request, _ m
 	}
 }
 
-func (h *PublicApiHandler) getBindingConfigurationFromRequest(policyJson json.RawMessage, logger lager.Logger) (*models.BindingConfig, error) {
+func (h *PublicApiHandler) getBindingConfigurationFromRequest(rawJson json.RawMessage, logger lager.Logger) (*models.BindingConfig, error) {
 	bindingConfiguration := &models.BindingConfig{}
 	var err error
-	if policyJson != nil {
-		err = json.Unmarshal(policyJson, &bindingConfiguration)
+	if rawJson != nil {
+		err = json.Unmarshal(rawJson, &bindingConfiguration)
 		if err != nil {
 			actionReadBindingConfiguration := "read-binding-configurations"
 			logger.Error("unmarshal-binding-configuration", err)
@@ -367,13 +367,17 @@ func (h *PublicApiHandler) getBindingConfigurationFromRequest(policyJson json.Ra
 	return bindingConfiguration, err
 }
 
-func (h *PublicApiHandler) buildResponse(strategy string, bindingConfiguration *models.BindingConfig, policy *models.ScalingPolicy) ([]byte, error) {
-	if strategy != "" && strategy != models.CustomMetricsSameApp {
-		bindingConfigWithPolicy := &models.BindingConfigWithPolicy{
-			BindingConfig: *bindingConfiguration,
-			ScalingPolicy: *policy,
-		}
-		return json.Marshal(bindingConfigWithPolicy)
+func buildResponse(policy *models.ScalingPolicy, customMetricStrategy string, logger lager.Logger) ([]byte, error) {
+	scalingPolicyWithCustomMetricStrategy, err := models.GetBindingConfigAndPolicy(policy, customMetricStrategy)
+	if err != nil {
+		logger.Error("Failed to build policy and config response object", err)
+		//writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy")
+		return nil, errors.New("error: building binding and policy")
+	}
+	responseJson, err := json.Marshal(scalingPolicyWithCustomMetricStrategy)
+	if err != nil {
+		logger.Error("Failed to marshal policy", err)
+		return nil, errors.New("error: marshalling policy")
 	}
-	return json.Marshal(policy)
+	return responseJson, nil
 }

src/autoscaler/models/api.go

@@ -55,9 +55,9 @@ type ServiceBinding struct {
 	CustomMetricsStrategy string `db:"custom_metrics_strategy"`
 }
 
-type BindingConfigWithPolicy struct {
-	BindingConfig
+type ScalingPolicyWithBindingConfig struct {
 	ScalingPolicy
+	*BindingConfig
 }
 
 type BindingRequestBody struct {

src/autoscaler/models/binding_configs.go

@@ -10,11 +10,8 @@ import (
 {
   "configuration": {
     "custom_metrics": {
-      "auth": {
-        "credential_type": "binding_secret"
-      },
       "metric_submission_strategy": {
-        "allow_from": "bound_app or same_app"
+        "allow_from": "bound_app"
       }
     }
   }
@@ -50,7 +47,7 @@ func (b *BindingConfig) SetCustomMetricsStrategy(allowFrom string) {
 }
 
 /**
- * DetermineBindingConfigAndPolicy determines the binding configuration and policy based on the given parameters.
+ * GetBindingConfigAndPolicy combines the binding configuration and policy based on the given parameters.
  * It establishes the relationship between the scaling policy and the custom metrics strategy.
  * @param scalingPolicy the scaling policy
  * @param customMetricStrategy the custom metric strategy
@@ -59,39 +56,32 @@ func (b *BindingConfig) SetCustomMetricsStrategy(allowFrom string) {
  * @throws an error if no policy or custom metrics strategy is found
 */
 
-func DetermineBindingConfigAndPolicy(scalingPolicy *ScalingPolicy, customMetricStrategy string) (interface{}, error) {
+func GetBindingConfigAndPolicy(scalingPolicy *ScalingPolicy, customMetricStrategy string) (interface{}, error) {
 	if scalingPolicy == nil {
 		return nil, fmt.Errorf("policy not found")
 	}
-
-	combinedConfig, bindingConfig := buildConfigurationIfPresent(customMetricStrategy)
-	if combinedConfig != nil { //both are present
-		combinedConfig.ScalingPolicy = *scalingPolicy
-		combinedConfig.BindingConfig = *bindingConfig
-		return combinedConfig, nil
+	if customMetricStrategy != "" && customMetricStrategy != CustomMetricsSameApp { //if customMetricStrategy found
+		return buildCombinedConfig(scalingPolicy, customMetricStrategy), nil
 	}
 	return scalingPolicy, nil
 }
 
-func buildConfigurationIfPresent(customMetricsStrategy string) (*BindingConfigWithPolicy, *BindingConfig) {
-	var combinedConfig *BindingConfigWithPolicy
-	var bindingConfig *BindingConfig
+func buildCombinedConfig(scalingPolicy *ScalingPolicy, customMetricStrategy string) *ScalingPolicyWithBindingConfig {
+	bindingConfig := &BindingConfig{}
+	bindingConfig.SetCustomMetricsStrategy(customMetricStrategy)
 
-	if customMetricsStrategy != "" && customMetricsStrategy != CustomMetricsSameApp { //if custom metric was given in the binding process
-		combinedConfig = &BindingConfigWithPolicy{}
-		bindingConfig = &BindingConfig{}
-		bindingConfig.SetCustomMetricsStrategy(customMetricsStrategy)
-		combinedConfig.BindingConfig = *bindingConfig
+	return &ScalingPolicyWithBindingConfig{
+		BindingConfig: bindingConfig,
+		ScalingPolicy: *scalingPolicy,
 	}
-	return combinedConfig, bindingConfig
 }
 
-func (b *BindingConfig) ValidateOrGetDefaultCustomMetricsStrategy(bindingConfiguration *BindingConfig) (*BindingConfig, error) {
-	strategy := bindingConfiguration.GetCustomMetricsStrategy()
+func (b *BindingConfig) ValidateOrGetDefaultCustomMetricsStrategy() (*BindingConfig, error) {
+	strategy := b.GetCustomMetricsStrategy()
 	if strategy == "" {
-		bindingConfiguration.SetCustomMetricsStrategy(CustomMetricsSameApp)
+		b.SetCustomMetricsStrategy(CustomMetricsSameApp)
 	} else if strategy != CustomMetricsBoundApp {
 		return nil, errors.New("error: custom metrics strategy not supported")
 	}
-	return bindingConfiguration, nil
+	return b, nil
 }

src/autoscaler/models/binding_configs_test.go

@@ -8,7 +8,7 @@ import (
 
 var _ = Describe("BindingConfigs", func() {
 
-	Describe("DetermineBindingConfigAndPolicy", func() {
+	Describe("GetBindingConfigAndPolicy", func() {
 		var (
 			scalingPolicy        *ScalingPolicy
 			customMetricStrategy string
@@ -17,7 +17,7 @@ var _ = Describe("BindingConfigs", func() {
 		)
 
 		JustBeforeEach(func() {
-			result, err = DetermineBindingConfigAndPolicy(scalingPolicy, customMetricStrategy)
+			result, err = GetBindingConfigAndPolicy(scalingPolicy, customMetricStrategy)
 		})
 
 		Context("when both scaling policy and custom metric strategy are present", func() {
@@ -41,8 +41,8 @@ var _ = Describe("BindingConfigs", func() {
 
 			It("should return combined configuration", func() {
 				Expect(err).NotTo(HaveOccurred())
-				Expect(result).To(BeAssignableToTypeOf(&BindingConfigWithPolicy{}))
-				combinedConfig := result.(*BindingConfigWithPolicy)
+				Expect(result).To(BeAssignableToTypeOf(&ScalingPolicyWithBindingConfig{}))
+				combinedConfig := result.(*ScalingPolicyWithBindingConfig)
 				Expect(combinedConfig.ScalingPolicy).To(Equal(*scalingPolicy))
 				Expect(combinedConfig.BindingConfig.GetCustomMetricsStrategy()).To(Equal(customMetricStrategy))
 			})
@@ -116,17 +116,15 @@ var _ = Describe("BindingConfigs", func() {
 	Context("ValidateOrGetDefaultCustomMetricsStrategy", func() {
 		var (
 			validatedBindingConfig *BindingConfig
-			incomingBindingConfig  *BindingConfig
 			err                    error
 		)
 		JustBeforeEach(func() {
-			validatedBindingConfig, err = bindingConfig.ValidateOrGetDefaultCustomMetricsStrategy(incomingBindingConfig)
+			validatedBindingConfig, err = bindingConfig.ValidateOrGetDefaultCustomMetricsStrategy()
 		})
 		When("custom metrics strategy is empty", func() {
 
 			BeforeEach(func() {
 				bindingConfig = &BindingConfig{}
-				incomingBindingConfig = &BindingConfig{}
 			})
 			It("should set the default custom metrics strategy", func() {
 				Expect(err).NotTo(HaveOccurred())
@@ -136,8 +134,7 @@ var _ = Describe("BindingConfigs", func() {
 
 		When("custom metrics strategy is unsupported", func() {
 			BeforeEach(func() {
-				bindingConfig = &BindingConfig{}
-				incomingBindingConfig = &BindingConfig{
+				bindingConfig = &BindingConfig{
 					Configuration: Configuration{
 						CustomMetrics: CustomMetricsConfig{
 							MetricSubmissionStrategy: MetricsSubmissionStrategy{

src/autoscaler/models/policy.go

@@ -39,8 +39,8 @@ func (p *PolicyJson) GetAppPolicy() (*AppPolicy, error) {
 }
 
 // ScalingPolicy is a customer facing entity and represents the scaling policy for an application.
-// It can be manipulated by the user via the binding process and public API. If a change is required in the policy,
-// think of other interaction points e.g., public api server.
+// It can be created/deleted/retrieved by the user via the binding process and public api. If a change is required in the policy,
+// the corresponding endpoints should be also be updated in the public api server.
 type ScalingPolicy struct {
 	InstanceMin  int               `json:"instance_min_count"`
 	InstanceMax  int               `json:"instance_max_count"`