From 69458b4deefe5b9c2c33a3b51389face968ff52f Mon Sep 17 00:00:00 2001
From: Mattie Fu <mattiefu@google.com>
Date: Wed, 21 Apr 2021 10:40:17 -0400
Subject: [PATCH] fix: fix dynamic flow control setting checks (#1347)

* fix: fix dynamic flow control setting checks

* make checks more readable
---
 .../batching/DynamicFlowControlSettings.java  | 26 ++++++++++++-------
 .../DynamicFlowControlSettingsTest.java       | 16 ++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java b/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java
index 60783a6e2..95025183a 100644
--- a/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java
+++ b/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java
@@ -115,11 +115,14 @@ public DynamicFlowControlSettings build() {
     }
 
     private void verifyElementCountSettings(DynamicFlowControlSettings settings) {
-      boolean isEnabled =
-          settings.getInitialOutstandingElementCount() != null
-              || settings.getMinOutstandingElementCount() != null
-              || settings.getMaxOutstandingElementCount() != null;
-      if (!isEnabled) {
+      // If LimitExceededBehavior is Ignore, dynamic flow control is disabled, there's no need to
+      // check element count limit settings
+      if (settings.getLimitExceededBehavior() == LimitExceededBehavior.Ignore) {
+        return;
+      }
+      if (settings.getInitialOutstandingElementCount() == null
+          && settings.getMinOutstandingElementCount() == null
+          && settings.getMaxOutstandingElementCount() == null) {
         return;
       }
       Preconditions.checkState(
@@ -141,11 +144,14 @@ private void verifyElementCountSettings(DynamicFlowControlSettings settings) {
     }
 
     private void verifyRequestBytesSettings(DynamicFlowControlSettings settings) {
-      boolean isEnabled =
-          settings.getInitialOutstandingRequestBytes() != null
-              || settings.getMinOutstandingRequestBytes() != null
-              || settings.getMaxOutstandingRequestBytes() != null;
-      if (!isEnabled) {
+      // If LimitExceededBehavior is Ignore, dynamic flow control is disabled, there's no need to
+      // check request bytes limit settings
+      if (settings.getLimitExceededBehavior() == LimitExceededBehavior.Ignore) {
+        return;
+      }
+      if (settings.getInitialOutstandingRequestBytes() == null
+          && settings.getMinOutstandingRequestBytes() == null
+          && settings.getMaxOutstandingRequestBytes() == null) {
         return;
       }
       Preconditions.checkState(
diff --git a/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java b/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java
index c1f326f39..afbf8ce1e 100644
--- a/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java
+++ b/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java
@@ -55,6 +55,22 @@ public void testEmptyBuilder() {
     assertEquals(LimitExceededBehavior.Block, settings.getLimitExceededBehavior());
   }
 
+  @Test
+  public void testPartialSettingsIgnored() {
+    // If behavior is ignore, build shouldn't throw exceptions even when only one of the bytes or
+    // element limits is set
+    DynamicFlowControlSettings.Builder builder =
+        DynamicFlowControlSettings.newBuilder()
+            .setLimitExceededBehavior(LimitExceededBehavior.Ignore)
+            .setMaxOutstandingElementCount(1L);
+    builder.build();
+    builder =
+        DynamicFlowControlSettings.newBuilder()
+            .setLimitExceededBehavior(LimitExceededBehavior.Ignore)
+            .setMinOutstandingRequestBytes(1L);
+    builder.build();
+  }
+
   @Test
   public void testBuilder() {
     DynamicFlowControlSettings.Builder builder =