Translate negative Commons Pool limits to Integer.MAX_VALUE #1181

Lettuce now adapts negative pool limits for the async pool to Integer.MAX_VALUE when using GenericObjectPoolConfig to align with the behavior of Commons Pool 2.

src/main/java/io/lettuce/core/support/BoundedPoolConfig.java

@@ -149,8 +149,7 @@ public Builder testOnRelease(boolean testOnRelease) {
 
         /**
          * Configures the maximum number of objects that can be allocated by the pool (checked out to clients, or idle awaiting
-         * checkout) at a given time. When negative, there is no limit to the number of objects that can be managed by the pool
-         * at one time.
+         * checkout) at a given time.
          *
          * @param maxTotal maximum number of objects that can be allocated by the pool.
          * @return {@code this} {@link Builder}.
@@ -179,8 +178,7 @@ public Builder maxIdle(int maxIdle) {
 
         /**
          * Configures the maximum number of objects that can be allocated by the pool (checked out to clients, or idle awaiting
-         * checkout) at a given time. When negative, there is no limit to the number of objects that can be managed by the pool
-         * at one time.
+         * checkout) at a given time.
          *
          * @param minIdle maximum number of objects that can be allocated by the pool.
          * @return {@code this} {@link Builder}.

src/main/java/io/lettuce/core/support/CommonsPool2ConfigConverter.java

@@ -41,8 +41,13 @@ public static BoundedPoolConfig bounded(GenericObjectPoolConfig<?> config) {
 
         LettuceAssert.notNull(config, "GenericObjectPoolConfig must not be null");
 
-        return BoundedPoolConfig.builder().maxTotal(config.getMaxTotal()).maxIdle(config.getMaxIdle())
-                .minIdle(config.getMinIdle()).testOnAcquire(config.getTestOnBorrow()).testOnCreate(config.getTestOnCreate())
-                .testOnRelease(config.getTestOnReturn()).build();
+        return BoundedPoolConfig.builder() //
+                .maxTotal(config.getMaxTotal() > 0 ? config.getMaxTotal() : Integer.MAX_VALUE)
+                .maxIdle(config.getMaxIdle() > 0 ? config.getMaxIdle() : Integer.MAX_VALUE) //
+                .minIdle(config.getMinIdle()) //
+                .testOnAcquire(config.getTestOnBorrow()) //
+                .testOnCreate(config.getTestOnCreate()) //
+                .testOnRelease(config.getTestOnReturn()) //
+                .build();
     }
 }

src/test/java/io/lettuce/core/support/CommonsPool2ConfigConverterUnitTests.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.lettuce.core.support;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import org.apache.commons.pool2.impl.BaseObjectPoolConfig;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for {@link CommonsPool2ConfigConverter}.
+ *
+ * @author Mark Paluch
+ */
+class CommonsPool2ConfigConverterUnitTests {
+
+    @Test
+    void shouldAdaptConfiguration() {
+
+        GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
+        config.setMinIdle(2);
+        config.setMaxIdle(12);
+        config.setMaxTotal(13);
+        config.setTestOnBorrow(true);
+        config.setTestOnReturn(true);
+        config.setTestOnCreate(true);
+
+        BoundedPoolConfig result = CommonsPool2ConfigConverter.bounded(config);
+
+        assertThat(result.getMinIdle()).isEqualTo(2);
+        assertThat(result.getMaxIdle()).isEqualTo(12);
+        assertThat(result.getMaxTotal()).isEqualTo(13);
+        assertThat(result.isTestOnAcquire()).isTrue();
+        assertThat(result.isTestOnCreate()).isTrue();
+        assertThat(result.isTestOnRelease()).isTrue();
+    }
+
+    @Test
+    void shouldConvertNegativeValuesToMaxSize() {
+
+        GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
+        config.setMaxIdle(-1);
+        config.setMaxTotal(-1);
+
+        BoundedPoolConfig result = CommonsPool2ConfigConverter.bounded(config);
+
+        assertThat(result.getMaxIdle()).isEqualTo(Integer.MAX_VALUE);
+        assertThat(result.getMaxTotal()).isEqualTo(Integer.MAX_VALUE);
+    }
+
+    @Test
+    void shouldAdaptTestOnAcquire() {
+
+        booleanTester(true, BaseObjectPoolConfig::setTestOnBorrow, BasePoolConfig::isTestOnAcquire);
+        booleanTester(false, BaseObjectPoolConfig::setTestOnBorrow, BasePoolConfig::isTestOnAcquire);
+    }
+
+    @Test
+    void shouldAdaptTestOnCreate() {
+
+        booleanTester(true, BaseObjectPoolConfig::setTestOnCreate, BasePoolConfig::isTestOnCreate);
+        booleanTester(false, BaseObjectPoolConfig::setTestOnCreate, BasePoolConfig::isTestOnCreate);
+    }
+
+    @Test
+    void shouldAdaptTestOnRelease() {
+
+        booleanTester(true, BaseObjectPoolConfig::setTestOnReturn, BasePoolConfig::isTestOnRelease);
+        booleanTester(false, BaseObjectPoolConfig::setTestOnReturn, BasePoolConfig::isTestOnRelease);
+    }
+
+    static void booleanTester(boolean value, BiConsumer<GenericObjectPoolConfig<?>, Boolean> commonsConfigurer,
+            Function<BoundedPoolConfig, Boolean> targetExtractor) {
+
+        GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
+
+        commonsConfigurer.accept(config, value);
+        BoundedPoolConfig result = CommonsPool2ConfigConverter.bounded(config);
+
+        assertThat(targetExtractor.apply(result)).isEqualTo(value);
+    }
+}