JAMES-2404 Add a new utility class in order to extract common parameters

server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/ParametersExtractor.java

@@ -0,0 +1,92 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you 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 org.apache.james.webadmin.utils;
+
+import java.util.Optional;
+
+import org.apache.james.util.streams.Limit;
+import org.apache.james.util.streams.Offset;
+import org.eclipse.jetty.http.HttpStatus;
+
+import com.google.common.base.Strings;
+
+import spark.Request;
+
+public class ParametersExtractor {
+
+    public static final String LIMIT_PARAMETER_NAME = "limit";
+    public static final String OFFSET_PARAMETER_NAME = "offset";
+
+    public static Limit extractLimit(Request request) {
+        return Limit.from(assertPositiveInteger(request, LIMIT_PARAMETER_NAME)
+                .map(value -> assertNotZero(value, LIMIT_PARAMETER_NAME)));
+    }
+
+    public static Offset extractOffset(Request request) {
+        return Offset.from(assertPositiveInteger(request, OFFSET_PARAMETER_NAME));
+    }
+
+    public static Optional<Double> extractPositiveDouble(Request request, String parameterName) {
+        return extractPositiveNumber(request, parameterName)
+                .map(Number::doubleValue);
+    }
+
+    private static Optional<Integer> assertPositiveInteger(Request request, String parameterName) {
+        return extractPositiveNumber(request, parameterName)
+                .map(Number::intValue);
+    }
+
+    private static Optional<Number> extractPositiveNumber(Request request, String parameterName) {
+        try {
+            return Optional.ofNullable(request.queryParams(parameterName))
+                .filter(s -> !Strings.isNullOrEmpty(s))
+                .map(Double::valueOf)
+                .map(value -> assertPositive(value, parameterName));
+        } catch (NumberFormatException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .cause(e)
+                .message("Can not parse " + parameterName)
+                .haltError();
+        }
+    }
+
+    private static Number assertPositive(Number value, String parameterName) {
+        if (value.doubleValue() < 0) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message(parameterName + " can not be negative")
+                .haltError();
+        }
+        return value;
+    }
+
+    private static int assertNotZero(int value, String parameterName) {
+        if (value == 0) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message(parameterName + " can not be equal to zero")
+                .haltError();
+        }
+        return value;
+    }
+}

server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/utils/ParametersExtractorTest.java

@@ -0,0 +1,206 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you 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 org.apache.james.webadmin.utils;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Optional;
+
+import org.apache.james.util.streams.Limit;
+import org.apache.james.util.streams.Offset;
+import org.junit.Test;
+
+import spark.HaltException;
+import spark.Request;
+
+public class ParametersExtractorTest {
+
+    @Test
+    public void extractLimitShouldReturnUnlimitedWhenNotInParameters() {
+        Request request = mock(Request.class);
+        when(request.queryParams("limit"))
+            .thenReturn(null);
+
+        Limit limit = ParametersExtractor.extractLimit(request);
+
+        assertThat(limit).isEqualTo(Limit.unlimited());
+    }
+
+    @Test
+    public void extractLimitShouldReturnUnlimitedWhenPresentInParametersButEmpty() {
+        Request request = mock(Request.class);
+        when(request.queryParams("limit"))
+            .thenReturn("");
+
+        Limit limit = ParametersExtractor.extractLimit(request);
+
+        assertThat(limit).isEqualTo(Limit.unlimited());
+    }
+
+    @Test
+    public void extractLimitShouldReturnTheLimitWhenPresentInParameters() {
+        Request request = mock(Request.class);
+        when(request.queryParams("limit"))
+            .thenReturn("123");
+
+        Limit limit = ParametersExtractor.extractLimit(request);
+
+        assertThat(limit).isEqualTo(Limit.from(123));
+    }
+
+    @Test
+    public void extractLimitShouldThrowWhenNegativeLimit() {
+        Request request = mock(Request.class);
+        when(request.queryParams("limit"))
+            .thenReturn("-123");
+
+        assertThatThrownBy(() -> ParametersExtractor.extractLimit(request))
+            .isInstanceOf(HaltException.class);
+    }
+
+    @Test
+    public void extractLimitShouldThrowWhenZeroLimit() {
+        Request request = mock(Request.class);
+        when(request.queryParams("limit"))
+            .thenReturn("0");
+
+        assertThatThrownBy(() -> ParametersExtractor.extractLimit(request))
+            .isInstanceOf(HaltException.class);
+    }
+
+    @Test
+    public void extractOffsetShouldReturnNoneWhenNotInParameters() {
+        Request request = mock(Request.class);
+        when(request.queryParams("offset"))
+            .thenReturn(null);
+
+        Offset offset = ParametersExtractor.extractOffset(request);
+
+        assertThat(offset).isEqualTo(Offset.none());
+    }
+
+    @Test
+    public void extractOffsetShouldReturnNoneWhenPresentInParametersButEmpty() {
+        Request request = mock(Request.class);
+        when(request.queryParams("offset"))
+            .thenReturn("");
+
+        Offset offset= ParametersExtractor.extractOffset(request);
+
+        assertThat(offset).isEqualTo(Offset.none());
+    }
+
+    @Test
+    public void extractOffsetShouldReturnTheOffsetWhenPresentInParameters() {
+        Request request = mock(Request.class);
+        when(request.queryParams("offset"))
+            .thenReturn("123");
+
+        Offset offset = ParametersExtractor.extractOffset(request);
+
+        assertThat(offset).isEqualTo(Offset.from(123));
+    }
+
+    @Test
+    public void extractOffsetShouldThrowWhenNegativeOffset() {
+        Request request = mock(Request.class);
+        when(request.queryParams("offset"))
+            .thenReturn("-123");
+
+        assertThatThrownBy(() -> ParametersExtractor.extractOffset(request))
+            .isInstanceOf(HaltException.class);
+    }
+
+    @Test
+    public void extractOffsetShouldReturnNoneWhenZeroLimit() {
+        Request request = mock(Request.class);
+        when(request.queryParams("offset"))
+            .thenReturn("0");
+
+        Offset offset = ParametersExtractor.extractOffset(request);
+
+        assertThat(offset).isEqualTo(Offset.none());
+    }
+
+
+
+    @Test
+    public void extractPositiveDoubleShouldReturnEmptyWhenNotInParameters() {
+        String parameterName = "param";
+        Request request = mock(Request.class);
+        when(request.queryParams(parameterName))
+            .thenReturn(null);
+
+        Optional<Double> result = ParametersExtractor.extractPositiveDouble(request, parameterName);
+
+        assertThat(result).isEmpty();
+    }
+
+    @Test
+    public void extractPositiveDoubleShouldReturnNoneWhenPresentInParametersButEmpty() {
+        String parameterName = "param";
+        Request request = mock(Request.class);
+        when(request.queryParams(parameterName))
+            .thenReturn("");
+
+        Optional<Double> result = ParametersExtractor.extractPositiveDouble(request, parameterName);
+
+        assertThat(result).isEmpty();
+    }
+
+    @Test
+    public void extractPositiveDoubleShouldReturnTheDoubleWhenPresentInParameters() {
+        String parameterName = "param";
+        Request request = mock(Request.class);
+        when(request.queryParams(parameterName))
+            .thenReturn("123");
+
+        Optional<Double> result = ParametersExtractor.extractPositiveDouble(request, parameterName);
+
+        assertThat(result).contains(Double.valueOf(123));
+    }
+
+    @Test
+    public void extractPositiveDoubleShouldThrowWhenNegativePositiveDouble() {
+        String parameterName = "param";
+        Request request = mock(Request.class);
+        when(request.queryParams(parameterName))
+            .thenReturn("-123");
+
+        assertThatThrownBy(() -> {
+            ParametersExtractor.extractPositiveDouble(request, parameterName);
+        })
+            .isInstanceOf(HaltException.class);
+    }
+
+    @Test
+    public void extractPositiveDoubleShouldReturnZeroWhenZeroLimit() {
+        String parameterName = "param";
+        Request request = mock(Request.class);
+        when(request.queryParams(parameterName))
+            .thenReturn("0");
+
+        Optional<Double> result = ParametersExtractor.extractPositiveDouble(request, parameterName);
+
+        assertThat(result).contains(Double.valueOf(0));
+    }
+}

server/protocols/webadmin/webadmin-mailqueue/src/main/java/org/apache/james/webadmin/routes/MailQueueRoutes.java

@@ -52,6 +52,7 @@
 import org.apache.james.webadmin.utils.JsonExtractException;
 import org.apache.james.webadmin.utils.JsonExtractor;
 import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.ParametersExtractor;
 import org.eclipse.jetty.http.HttpStatus;
 
 import com.github.fge.lambdas.Throwing;
@@ -217,7 +218,7 @@ public void listMails(Service service) {
     private List<MailQueueItemDTO> listMails(Request request) {
         String mailQueueName = request.params(MAIL_QUEUE_NAME);
         return mailQueueFactory.getQueue(mailQueueName)
-                .map(name -> listMails(name, isDelayed(request.queryParams(DELAYED_QUERY_PARAM)), limit(request.queryParams(LIMIT_QUERY_PARAM))))
+                .map(name -> listMails(name, isDelayed(request.queryParams(DELAYED_QUERY_PARAM)), ParametersExtractor.extractLimit(request)))
                 .orElseThrow(
                     () -> ErrorResponder.builder()
                         .message(String.format("%s can not be found", mailQueueName))
@@ -231,21 +232,6 @@ private List<MailQueueItemDTO> listMails(Request request) {
                 .map(Boolean::parseBoolean);
     }
 
-    @VisibleForTesting Limit limit(String limitAsString) throws HaltException {
-        try {
-            return Optional.ofNullable(limitAsString)
-                    .map(Integer::parseInt)
-                    .map(Limit::limit)
-                    .orElseGet(() -> Limit.from(DEFAULT_LIMIT_VALUE));
-        } catch (IllegalArgumentException e) {
-            throw ErrorResponder.builder()
-                .message(String.format("limit can't be less or equals to zero"))
-                .statusCode(HttpStatus.BAD_REQUEST_400)
-                .type(ErrorResponder.ErrorType.NOT_FOUND)
-                .haltError();
-        }
-    }
-
     private List<MailQueueItemDTO> listMails(ManageableMailQueue queue, Optional<Boolean> isDelayed, Limit limit) {
         try {
             return limit.applyOnStream(Iterators.toStream(queue.browse()))

server/protocols/webadmin/webadmin-mailqueue/src/test/java/org/apache/james/webadmin/routes/MailQueueRoutesUnitTest.java

@@ -20,20 +20,16 @@
 package org.apache.james.webadmin.routes;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.util.Optional;
 
 import org.apache.james.queue.api.MailQueueFactory;
 import org.apache.james.queue.api.ManageableMailQueue;
 import org.apache.james.task.MemoryTaskManager;
-import org.apache.james.util.streams.Limit;
 import org.apache.james.webadmin.utils.JsonTransformer;
 import org.junit.Before;
 import org.junit.Test;
 
-import spark.HaltException;
-
 public class MailQueueRoutesUnitTest {
 
     MailQueueRoutes testee;
@@ -68,34 +64,4 @@ public void isDelayedShouldBeEqualsToFalseWhenOtherValue() {
         Optional<Boolean> delayed = testee.isDelayed("abc");
         assertThat(delayed).contains(false);
     }
-
-    @Test
-    public void limitShouldBeEqualsToDefaultValueWhenNull() {
-        Limit limit = testee.limit(null);
-        assertThat(limit.getLimit()).contains(MailQueueRoutes.DEFAULT_LIMIT_VALUE);
-    }
-
-    @Test
-    public void limitShouldBeEqualsToTheValueWhenOne() {
-        Limit limit = testee.limit("123");
-        assertThat(limit.getLimit()).contains(123);
-    }
-
-    @Test
-    public void limitShouldThrowWhenOtherValue() {
-        assertThatThrownBy(() -> testee.limit("abc"))
-            .isInstanceOf(HaltException.class);
-    }
-
-    @Test
-    public void limitShouldThrowWhenEqualsToZero() {
-        assertThatThrownBy(() -> testee.limit("0"))
-            .isInstanceOf(HaltException.class);
-    }
-
-    @Test
-    public void limitShouldThrowWhenLessThanZero() {
-        assertThatThrownBy(() -> testee.limit("-1"))
-            .isInstanceOf(HaltException.class);
-    }
 }