Extracted clamp to helper class with tests and fixed negative config bug

asegal-hs · asegal-hs · commit 10b4ff64b628 · 2023-11-21T16:10:40.000-05:00
diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java
@@ -50,6 +50,7 @@
 import com.hubspot.jinjava.tree.output.OutputNode;
 import com.hubspot.jinjava.tree.output.RenderedOutputNode;
 import com.hubspot.jinjava.util.EagerReconstructionUtils;
+import com.hubspot.jinjava.util.RenderLimitUtils;
 import com.hubspot.jinjava.util.Variable;
 import com.hubspot.jinjava.util.WhitespaceUtils;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -301,7 +302,9 @@ public String render(Node root, boolean processExtendRoots) {
    * @return rendered result
    */
   private String render(Node root, boolean processExtendRoots, long renderLimit) {
-    OutputList output = new OutputList(clampOutputSizeSafely(renderLimit));
+    OutputList output = new OutputList(
+      RenderLimitUtils.clampProvidedRenderLimitToConfig(renderLimit, config)
+    );
     for (Node node : root.getChildren()) {
       lineNumber = node.getLineNumber();
       position = node.getStartPosition();
@@ -924,20 +927,6 @@ private String getWrappedErrorMessage(
     }
   }
 
-  private long clampOutputSizeSafely(long providedLimit) {
-    long configMaxOutput = config.getMaxOutputSize();
-
-    if (configMaxOutput == 0) {
-      return providedLimit;
-    }
-
-    if (providedLimit <= 0) {
-      return configMaxOutput;
-    }
-
-    return Math.min(providedLimit, configMaxOutput);
-  }
-
   @Override
   @SuppressWarnings("unchecked")
   public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
diff --git a/src/main/java/com/hubspot/jinjava/util/RenderLimitUtils.java b/src/main/java/com/hubspot/jinjava/util/RenderLimitUtils.java
@@ -0,0 +1,23 @@
+package com.hubspot.jinjava.util;
+
+import com.hubspot.jinjava.JinjavaConfig;
+
+public class RenderLimitUtils {
+
+  public static long clampProvidedRenderLimitToConfig(
+    long providedLimit,
+    JinjavaConfig jinjavaConfig
+  ) {
+    long configMaxOutput = jinjavaConfig.getMaxOutputSize();
+
+    if (configMaxOutput <= 0) {
+      return providedLimit;
+    }
+
+    if (providedLimit <= 0) {
+      return configMaxOutput;
+    }
+
+    return Math.min(providedLimit, configMaxOutput);
+  }
+}
diff --git a/src/test/java/com/hubspot/jinjava/util/RenderLimitUtilsTest.java b/src/test/java/com/hubspot/jinjava/util/RenderLimitUtilsTest.java
@@ -0,0 +1,46 @@
+package com.hubspot.jinjava.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.hubspot.jinjava.JinjavaConfig;
+import org.junit.Test;
+
+public class RenderLimitUtilsTest {
+
+  @Test
+  public void itPicksLowerLimitWhenConfigIsSet() {
+    assertThat(
+        RenderLimitUtils.clampProvidedRenderLimitToConfig(100, configWithOutputSize(10))
+      )
+      .isEqualTo(10);
+  }
+
+  @Test
+  public void itKeepsConfigLimitWhenConfigSetAndUnlimitedProvided() {
+    assertThat(
+        RenderLimitUtils.clampProvidedRenderLimitToConfig(0, configWithOutputSize(10))
+      )
+      .isEqualTo(10);
+    assertThat(
+        RenderLimitUtils.clampProvidedRenderLimitToConfig(-10, configWithOutputSize(10))
+      )
+      .isEqualTo(10);
+  }
+
+  @Test
+  public void itUsesProvidedLimitWhenConfigIsUnlimited() {
+    assertThat(
+        RenderLimitUtils.clampProvidedRenderLimitToConfig(10, configWithOutputSize(0))
+      )
+      .isEqualTo(10);
+
+    assertThat(
+        RenderLimitUtils.clampProvidedRenderLimitToConfig(10, configWithOutputSize(-10))
+      )
+      .isEqualTo(10);
+  }
+
+  private JinjavaConfig configWithOutputSize(long size) {
+    return JinjavaConfig.newBuilder().withMaxOutputSize(size).build();
+  }
+}
