refactor: remove unneccessary getters

Kuadrant · Apr 8, 2024 · 953ce0a · 953ce0a
1 parent 8ae6cac
commit 953ce0a
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 20 deletions.
diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go
@@ -250,7 +250,7 @@ func (r *RateLimitPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
 
 func (r *RateLimitPolicy) GetRulesHostnames() (ruleHosts []string) {
 	ruleHosts = make([]string, 0)
-	for _, limit := range r.GetLimits() {
+	for _, limit := range r.Spec.CommonSpec().Limits {
 		for _, routeSelector := range limit.RouteSelectors {
 			convertHostnamesToString := func(gwHostnames []gatewayapiv1.Hostname) []string {
 				hostnames := make([]string, 0, len(gwHostnames))
@@ -277,18 +277,16 @@ func (r *RateLimitPolicy) DirectReferenceAnnotationName() string {
 	return RateLimitPolicyDirectReferenceAnnotationName
 }
 
-func (r *RateLimitPolicy) GetRateLimitPolicyCommonSpec() *RateLimitPolicyCommonSpec {
-	if r.Spec.Defaults != nil {
-		return r.Spec.Defaults
+// CommonSpec returns the Default RateLimitPolicyCommonSpec if it is defined.
+// Otherwise, it returns the RateLimitPolicyCommonSpec from the spec.
+// This function should be used instead of accessing the fields directly, so that either the explicit or implicit default
+// is returned.
+func (r *RateLimitPolicySpec) CommonSpec() *RateLimitPolicyCommonSpec {
+	if r.Defaults != nil {
+		return r.Defaults
 	}
 
-	return &r.Spec.RateLimitPolicyCommonSpec
-}
-
-// GetLimits returns the limits based on whether default or implicit rules are defined.
-// Default rules takes precedence
-func (r *RateLimitPolicy) GetLimits() map[string]Limit {
-	return r.GetRateLimitPolicyCommonSpec().Limits
+	return &r.RateLimitPolicyCommonSpec
 }
 
 func init() {

diff --git a/api/v1beta2/ratelimitpolicy_types_test.go b/api/v1beta2/ratelimitpolicy_types_test.go
@@ -98,21 +98,21 @@ func TestRateLimitPolicy_GetLimits(t *testing.T) {
 
 	t.Run("No limits defined", func(subT *testing.T) {
 		r := testBuildBasicHTTPRouteRLP(name, nil)
-		assert.DeepEqual(subT, r.GetLimits(), map[string]Limit(nil))
+		assert.DeepEqual(subT, r.Spec.CommonSpec().Limits, map[string]Limit(nil))
 	})
 	t.Run("Defaults defined", func(subT *testing.T) {
 		r := testBuildBasicHTTPRouteRLP(name, func(policy *RateLimitPolicy) {
 			policy.Spec.Defaults = &RateLimitPolicyCommonSpec{
 				Limits: defaultLimits,
 			}
 		})
-		assert.DeepEqual(subT, r.GetLimits(), defaultLimits)
+		assert.DeepEqual(subT, r.Spec.CommonSpec().Limits, defaultLimits)
 	})
 	t.Run("Implicit rules defined", func(subT *testing.T) {
 		r := testBuildBasicHTTPRouteRLP(name, func(policy *RateLimitPolicy) {
 			policy.Spec.Limits = implicitLimits
 		})
-		assert.DeepEqual(subT, r.GetLimits(), implicitLimits)
+		assert.DeepEqual(subT, r.Spec.CommonSpec().Limits, implicitLimits)
 	})
 	t.Run("Default rules takes precedence over implicit rules if validation is somehow bypassed", func(subT *testing.T) {
 		r := testBuildBasicHTTPRouteRLP(name, func(policy *RateLimitPolicy) {
@@ -121,6 +121,6 @@ func TestRateLimitPolicy_GetLimits(t *testing.T) {
 			}
 			policy.Spec.Limits = implicitLimits
 		})
-		assert.DeepEqual(subT, r.GetLimits(), defaultLimits)
+		assert.DeepEqual(subT, r.Spec.CommonSpec().Limits, defaultLimits)
 	})
 }
diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go
@@ -281,7 +281,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 			// Create HTTPRoute RLP with new default limits
 			routeRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
 				policy.Name = "httproute-rlp"
-				policy.Spec.Defaults.Limits = map[string]kuadrantv1beta2.Limit{
+				policy.Spec.CommonSpec().Limits = map[string]kuadrantv1beta2.Limit{
 					"l1": {
 						Rates: []kuadrantv1beta2.Rate{
 							{
@@ -322,7 +322,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 			Expect(existingGateway.GetAnnotations()).To(HaveKey(routeRLP.BackReferenceAnnotationName()))
 			Expect(existingGateway.GetAnnotations()[routeRLP.BackReferenceAnnotationName()]).To(ContainSubstring(string(serialized)))
 
-		}, SpecTimeout(1*time.Minute))
+		}, SpecTimeout(time.Minute))
 	})
 
 	Context("RLP accepted condition reasons", func() {

diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go
@@ -41,7 +41,7 @@ func LimitadorRateLimitsFromRLP(rlp *kuadrantv1beta2.RateLimitPolicy) []limitado
 	limitsNamespace := LimitsNamespaceFromRLP(rlp)
 
 	rateLimits := make([]limitadorv1alpha1.RateLimit, 0)
-	for limitKey, limit := range rlp.GetLimits() {
+	for limitKey, limit := range rlp.Spec.CommonSpec().Limits {
 		limitIdentifier := LimitNameToLimitadorIdentifier(limitKey)
 		for _, rate := range limit.Rates {
 			maxValue, seconds := rateToSeconds(rate)

diff --git a/pkg/rlptools/wasm_utils.go b/pkg/rlptools/wasm_utils.go
@@ -33,7 +33,8 @@ func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1.HTTPRou
 
 	// Sort RLP limits for consistent comparison with existing wasmplugin objects
 	limitNames := make([]string, 0, len(rlp.Spec.Limits))
-	for name := range rlp.GetLimits() {
+	limits := rlp.Spec.CommonSpec().Limits
+	for name := range limits {
 		limitNames = append(limitNames, name)
 	}
 
@@ -42,7 +43,7 @@ func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1.HTTPRou
 
 	for _, limitName := range limitNames {
 		// 1 RLP limit <---> 1 WASM rule
-		limit := rlp.GetLimits()[limitName]
+		limit := limits[limitName]
 		limitIdentifier := LimitNameToLimitadorIdentifier(limitName)
 		rule, err := ruleFromLimit(limitIdentifier, &limit, route)
 		if err == nil {