Adds override for individual fields on activity retry policy #628

temporalio · Jul 29, 2020 · 1f880f6 · 1f880f6
1 parent 02c8fac
commit 1f880f6
Show file tree

Hide file tree

Showing 4 changed files with 145 additions and 1 deletion.
diff --git a/common/util.go b/common/util.go
@@ -367,6 +367,38 @@ func SortInt64Slice(slice []int64) {
 	})
 }
 
+// EnsureRetryPolicyDefaults ensures the policy subfields, if not explicitly set, are set to the specified defaults
+func EnsureRetryPolicyDefaults(originalPolicy *commonpb.RetryPolicy, defaultPolicy *commonpb.RetryPolicy) *commonpb.RetryPolicy {
+	if originalPolicy == nil {
+		return defaultPolicy
+	}
+
+	merged := &commonpb.RetryPolicy{
+		BackoffCoefficient:       originalPolicy.GetBackoffCoefficient(),
+		InitialIntervalInSeconds: originalPolicy.GetInitialIntervalInSeconds(),
+		MaximumIntervalInSeconds: originalPolicy.GetMaximumIntervalInSeconds(),
+		MaximumAttempts:          originalPolicy.GetMaximumAttempts(),
+	}
+
+	if merged.GetMaximumAttempts() == 0 {
+		merged.MaximumAttempts = defaultPolicy.GetMaximumAttempts()
+	}
+
+	if merged.GetInitialIntervalInSeconds() == 0 {
+		merged.InitialIntervalInSeconds = defaultPolicy.GetInitialIntervalInSeconds()
+	}
+
+	if merged.GetMaximumIntervalInSeconds() == 0 {
+		merged.MaximumIntervalInSeconds = defaultPolicy.GetMaximumIntervalInSeconds()
+	}
+
+	if merged.GetBackoffCoefficient() == 0 {
+		merged.BackoffCoefficient = defaultPolicy.GetBackoffCoefficient()
+	}
+
+	return merged
+}
+
 // ValidateRetryPolicy validates a retry policy
 func ValidateRetryPolicy(policy *commonpb.RetryPolicy) error {
 	if policy == nil {

diff --git a/common/util_test.go b/common/util_test.go
@@ -87,3 +87,84 @@ func TestValidateRetryPolicy(t *testing.T) {
 		})
 	}
 }
+
+func TestEnsureRetryPolicyDefaults(t *testing.T) {
+	defaultRetryPolicy := &commonpb.RetryPolicy{
+		InitialIntervalInSeconds: 1,
+		MaximumIntervalInSeconds: 100,
+		BackoffCoefficient:       2,
+		MaximumAttempts:          120,
+	}
+
+	testCases := []struct {
+		name  string
+		input *commonpb.RetryPolicy
+		want  *commonpb.RetryPolicy
+	}{
+		{
+			name:  "nil policy is okay",
+			input: nil,
+			want:  defaultRetryPolicy,
+		},
+		{
+			name:  "default fields are set ",
+			input: &commonpb.RetryPolicy{},
+			want:  defaultRetryPolicy,
+		},
+		{
+			name: "non-default InitialIntervalInSeconds is not set",
+			input: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 2,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 2,
+				MaximumIntervalInSeconds: 100,
+				BackoffCoefficient:       2,
+				MaximumAttempts:          120,
+			},
+		},
+		{
+			name: "non-default MaximumIntervalInSeconds is not set",
+			input: &commonpb.RetryPolicy{
+				MaximumIntervalInSeconds: 1000,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 1,
+				MaximumIntervalInSeconds: 1000,
+				BackoffCoefficient:       2,
+				MaximumAttempts:          120,
+			},
+		},
+		{
+			name: "non-default BackoffCoefficient is not set",
+			input: &commonpb.RetryPolicy{
+				BackoffCoefficient: 1.5,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 1,
+				MaximumIntervalInSeconds: 100,
+				BackoffCoefficient:       1.5,
+				MaximumAttempts:          120,
+			},
+		},
+		{
+			name: "non-default Maximum attempts is not set",
+			input: &commonpb.RetryPolicy{
+				MaximumAttempts: 49,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 1,
+				MaximumIntervalInSeconds: 100,
+				BackoffCoefficient:       2,
+				MaximumAttempts:          49,
+			},
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := EnsureRetryPolicyDefaults(tt.input, defaultRetryPolicy)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/service/history/commandChecker.go b/service/history/commandChecker.go
@@ -657,6 +657,7 @@ func (v *commandAttrValidator) validateActivityRetryPolicy(attributes *commandpb
 		return nil
 	}
 
+	attributes.RetryPolicy = common.EnsureRetryPolicyDefaults(attributes.RetryPolicy, v.defaultActivityRetryPolicy)
 	return common.ValidateRetryPolicy(attributes.RetryPolicy)
 }
 

diff --git a/service/history/commandChecker_test.go b/service/history/commandChecker_test.go
@@ -572,7 +572,7 @@ func (s *commandAttrValidatorSuite) TestValidateActivityRetryPolicy() {
 			},
 		},
 		{
-			name: "do not override set policy",
+			name: "do not override fully set policy",
 			input: &commonpb.RetryPolicy{
 				InitialIntervalInSeconds: 5,
 				BackoffCoefficient:       10,
@@ -586,6 +586,36 @@ func (s *commandAttrValidatorSuite) TestValidateActivityRetryPolicy() {
 				MaximumAttempts:          8,
 			},
 		},
+		{
+			name: "partial override set policy",
+			input: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 0,
+				BackoffCoefficient:       1.2,
+				MaximumIntervalInSeconds: 0,
+				MaximumAttempts:          7,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 1,
+				BackoffCoefficient:       1.2,
+				MaximumIntervalInSeconds: 100,
+				MaximumAttempts:          7,
+			},
+		},
+		{
+			name: "override all defaults",
+			input: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 0,
+				BackoffCoefficient:       0,
+				MaximumIntervalInSeconds: 0,
+				MaximumAttempts:          0,
+			},
+			want: &commonpb.RetryPolicy{
+				InitialIntervalInSeconds: 1,
+				BackoffCoefficient:       2,
+				MaximumIntervalInSeconds: 100,
+				MaximumAttempts:          0,
+			},
+		},
 	}
 
 	for _, tt := range testCases {