From a613a92adb71edbf24cc0d99a79c3344929c669e Mon Sep 17 00:00:00 2001
From: Manuel Friedli <manuel@fritteli.ch>
Date: Fri, 13 Dec 2024 22:35:10 +0100
Subject: [PATCH] Make dimension limits Option<Integer> instead of using the
 magic value of 0 for "no limit", and add more tests for the ServerConfig.

---
 pom.xml                                       |  13 +-
 .../ch/fritteli/maze/server/ServerConfig.java |  42 ++++---
 .../maze/server/handler/CreateHandler.java    |   9 +-
 .../handler/ParametersToMazeExtractor.java    |  20 ++-
 .../maze/server/ServerConfigTest.java         | 114 ++++++++++++++++--
 5 files changed, 158 insertions(+), 40 deletions(-)

diff --git a/pom.xml b/pom.xml
index 3ec91a4..aed0ac3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
@@ -57,7 +58,7 @@
     <properties>
         <maze-generator.version>0.2.1</maze-generator.version>
         <maven-site-plugin.version>4.0.0-M8</maven-site-plugin.version>
-        <undertow.version>2.3.13.Final</undertow.version>
+        <undertow.version>2.3.18.Final</undertow.version>
     </properties>
 
     <dependencies>
@@ -95,6 +96,11 @@
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter-api</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
@@ -104,7 +110,8 @@
                 <artifactId>maven-shade-plugin</artifactId>
                 <configuration>
                     <transformers>
-                        <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
+                        <transformer
+                                implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
                             <mainClass>ch.fritteli.maze.server.Main</mainClass>
                         </transformer>
                     </transformers>
diff --git a/src/main/java/ch/fritteli/maze/server/ServerConfig.java b/src/main/java/ch/fritteli/maze/server/ServerConfig.java
index cd85ab9..401fd6d 100644
--- a/src/main/java/ch/fritteli/maze/server/ServerConfig.java
+++ b/src/main/java/ch/fritteli/maze/server/ServerConfig.java
@@ -1,5 +1,6 @@
 package ch.fritteli.maze.server;
 
+import io.vavr.control.Option;
 import io.vavr.control.Try;
 import lombok.extern.slf4j.Slf4j;
 import org.jetbrains.annotations.NotNull;
@@ -10,8 +11,8 @@ import java.net.InetAddress;
 @Slf4j
 public record ServerConfig(@NotNull InetAddress address,
                            int port,
-                           int maxMazeHeight,
-                           int maxMazeWidth) {
+                           @NotNull Option<Integer> maxMazeHeight,
+                           @NotNull Option<Integer> maxMazeWidth) {
     public static final String SYSPROP_HOST = "fritteli.maze.server.host";
     public static final String SYSPROP_PORT = "fritteli.maze.server.port";
     public static final String SYSPROP_MAX_MAZE_HEIGHT = "fritteli.maze.maxheight";
@@ -19,16 +20,19 @@ public record ServerConfig(@NotNull InetAddress address,
 
     public ServerConfig(@NotNull final InetAddress address,
                         final int port,
-                        final int maxMazeHeight,
-                        final int maxMazeWidth) {
+                        @NotNull final Option<Integer> maxMazeHeight,
+                        @NotNull final Option<Integer> maxMazeWidth) {
         this.address = address;
         this.port = validatePort(port);
         this.maxMazeHeight = validateDimension(maxMazeHeight, "height");
         this.maxMazeWidth = validateDimension(maxMazeWidth, "width");
-        log.debug("host={}, port={}", this.address, this.port);
+        log.debug("host={}, port={}, maxHeight={}, maxWidth={}", this.address, this.port, this.maxMazeHeight, this.maxMazeWidth);
     }
 
-    public ServerConfig(@Nullable final String address, final int port, final int maxMazeHeight, final int maxMazeWidth)
+    public ServerConfig(@Nullable final String address,
+                        final int port,
+                        @NotNull final Option<Integer> maxMazeHeight,
+                        @NotNull final Option<Integer> maxMazeWidth)
             throws ConfigurationException {
         this(validateAddress(address), port, maxMazeHeight, maxMazeWidth);
     }
@@ -40,8 +44,8 @@ public record ServerConfig(@NotNull InetAddress address,
         final String maxMazeHeightString = System.getProperty(SYSPROP_MAX_MAZE_HEIGHT);
         final String maxMazeWidthString = System.getProperty(SYSPROP_MAX_MAZE_WIDTH);
         final int port = validatePort(portString);
-        final int maxMazeHeight = validateDimension(maxMazeHeightString, "height", SYSPROP_MAX_MAZE_HEIGHT);
-        final int maxMazeWidth = validateDimension(maxMazeWidthString, "width", SYSPROP_MAX_MAZE_WIDTH);
+        final Option<Integer> maxMazeHeight = validateDimension(maxMazeHeightString, "height", SYSPROP_MAX_MAZE_HEIGHT);
+        final Option<Integer> maxMazeWidth = validateDimension(maxMazeWidthString, "width", SYSPROP_MAX_MAZE_WIDTH);
         return new ServerConfig(host, port, maxMazeHeight, maxMazeWidth);
     }
 
@@ -66,32 +70,36 @@ public record ServerConfig(@NotNull InetAddress address,
             log.info("No port configured; using default.");
             return 0;
         }
-        return Try.of(() -> Integer.valueOf(portString))
+        return Try.of(() -> Integer.parseInt(portString))
                 .getOrElseThrow(cause -> new ConfigurationException(
                         "Failed to parse port specified in system property '%s': %s".formatted(SYSPROP_PORT, portString),
                         cause
                 ));
     }
 
-    private static int validateDimension(final int dimension, @NotNull final String identifier) {
-        if (dimension < 0) {
-            throw new ConfigurationException("Maximum %s is negative : %s".formatted(dimension, identifier));
+    @NotNull
+    private static Option<Integer> validateDimension(@NotNull final Option<Integer> dimension,
+                                                     @NotNull final String identifier) {
+        if (dimension.exists(d -> d <= 1)) {
+            throw new ConfigurationException("Maximum %s must be greater than 1: %s"
+                    .formatted(identifier, dimension.get()));
         }
         return dimension;
     }
 
-    private static int validateDimension(@Nullable final String dimensionString,
-                                         @NotNull final String identifier,
-                                         @NotNull final String syspropName) {
+    private static Option<Integer> validateDimension(@Nullable final String dimensionString,
+                                                     @NotNull final String identifier,
+                                                     @NotNull final String syspropName) {
         if (dimensionString == null) {
             log.info("No maximum {} configured; using default (unlimited).", identifier);
-            return 0;
+            return Option.none();
         }
-        return Try.of(() -> Integer.valueOf(dimensionString))
+        final int desiredDimension = Try.of(() -> Integer.parseInt(dimensionString))
                 .getOrElseThrow(cause -> new ConfigurationException(
                         "Failed to parse maximum %s specified in system property '%s': %s"
                                 .formatted(identifier, syspropName, dimensionString),
                         cause
                 ));
+        return Option.some(desiredDimension);
     }
 }
diff --git a/src/main/java/ch/fritteli/maze/server/handler/CreateHandler.java b/src/main/java/ch/fritteli/maze/server/handler/CreateHandler.java
index 7ad7892..4d44ab2 100644
--- a/src/main/java/ch/fritteli/maze/server/handler/CreateHandler.java
+++ b/src/main/java/ch/fritteli/maze/server/handler/CreateHandler.java
@@ -7,6 +7,7 @@ import io.undertow.server.HttpServerExchange;
 import io.undertow.util.Headers;
 import io.undertow.util.HttpString;
 import io.undertow.util.StatusCodes;
+import io.vavr.control.Option;
 import io.vavr.control.Try;
 import lombok.extern.slf4j.Slf4j;
 import org.jetbrains.annotations.NotNull;
@@ -22,10 +23,12 @@ import java.util.Map;
 public class CreateHandler extends AbstractHttpHandler {
 
     public static final String PATH_TEMPLATE = "/create/{output}";
-    private final int maxHeight;
-    private final int maxWidth;
+    @NotNull
+    private final Option<Integer> maxHeight;
+    @NotNull
+    private final Option<Integer> maxWidth;
 
-    public CreateHandler(final int maxHeight, final int maxWidth) {
+    public CreateHandler(@NotNull final Option<Integer> maxHeight, @NotNull final Option<Integer> maxWidth) {
         this.maxHeight = maxHeight;
         this.maxWidth = maxWidth;
     }
diff --git a/src/main/java/ch/fritteli/maze/server/handler/ParametersToMazeExtractor.java b/src/main/java/ch/fritteli/maze/server/handler/ParametersToMazeExtractor.java
index 1b93e38..6c747e5 100644
--- a/src/main/java/ch/fritteli/maze/server/handler/ParametersToMazeExtractor.java
+++ b/src/main/java/ch/fritteli/maze/server/handler/ParametersToMazeExtractor.java
@@ -20,8 +20,10 @@ class ParametersToMazeExtractor {
 
     @NotNull
     private final Map<String, Deque<String>> queryParameters;
-    private final int maxHeight;
-    private final int maxWidth;
+    @NotNull
+    private final Option<Integer> maxHeight;
+    @NotNull
+    private final Option<Integer> maxWidth;
 
     @NotNull
     Try<GeneratedMaze> createMaze() {
@@ -54,12 +56,18 @@ class ParametersToMazeExtractor {
         final int desiredHeight = height.get();
         final int desiredWidth = width.get();
 
-        if (this.maxHeight != 0 && desiredHeight > this.maxHeight) {
-            return Try.failure(new InvalidRequestParameterException("Specified height (%s) is greater than allowed maximum of %s".formatted(desiredHeight, this.maxHeight)));
+        if (desiredHeight <= 1) {
+            return Try.failure(new InvalidRequestParameterException("Specified height (%s) must be > 1".formatted(desiredHeight)));
+        }
+        if (desiredWidth <= 1) {
+            return Try.failure(new InvalidRequestParameterException("Specified width (%s) must be > 1".formatted(desiredHeight)));
+        }
+        if (this.maxHeight.exists(max -> desiredHeight > max)) {
+            return Try.failure(new InvalidRequestParameterException("Specified height (%s) is greater than allowed maximum of %s".formatted(desiredHeight, this.maxHeight.get())));
         }
 
-        if (this.maxWidth != 0 && desiredWidth > this.maxWidth) {
-            return Try.failure(new InvalidRequestParameterException("Specified width (%s) is greater than allowed maximum of %s".formatted(desiredWidth, this.maxWidth)));
+        if (this.maxWidth.exists(max -> desiredWidth > max)) {
+            return Try.failure(new InvalidRequestParameterException("Specified width (%s) is greater than allowed maximum of %s".formatted(desiredWidth, this.maxWidth.get())));
         }
 
         return Try.of(() -> {
diff --git a/src/test/java/ch/fritteli/maze/server/ServerConfigTest.java b/src/test/java/ch/fritteli/maze/server/ServerConfigTest.java
index 999bb0f..68aae74 100644
--- a/src/test/java/ch/fritteli/maze/server/ServerConfigTest.java
+++ b/src/test/java/ch/fritteli/maze/server/ServerConfigTest.java
@@ -3,14 +3,16 @@ package ch.fritteli.maze.server;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 class ServerConfigTest {
     @BeforeEach
     void clearSysProperties() {
         System.clearProperty(ServerConfig.SYSPROP_HOST);
         System.clearProperty(ServerConfig.SYSPROP_PORT);
+        System.clearProperty(ServerConfig.SYSPROP_MAX_MAZE_HEIGHT);
+        System.clearProperty(ServerConfig.SYSPROP_MAX_MAZE_WIDTH);
     }
 
     @Test
@@ -20,9 +22,27 @@ class ServerConfigTest {
         final ServerConfig sut = ServerConfig.init();
 
         // assert
-        assertEquals("127.0.0.1", sut.address().getHostAddress());
-        assertEquals("localhost", sut.address().getHostName());
-        assertEquals(0, sut.port());
+        assertThat(sut.address())
+                .satisfies(
+                        address -> assertThat(address.getHostAddress()).isEqualTo("127.0.0.1"),
+                        address -> assertThat(address.getHostName()).isEqualTo("localhost")
+                );
+        assertThat(sut.port()).isZero();
+        assertThat(sut.maxMazeHeight()).isEmpty();
+        assertThat(sut.maxMazeWidth()).isEmpty();
+    }
+
+
+    @Test
+    void testInit_invalidAddress() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_HOST, "256.2515.19.0");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Invalid hostname/address")
+                .withMessageContaining("256.2515.19.0");
     }
 
     @Test
@@ -31,29 +51,101 @@ class ServerConfigTest {
         System.setProperty(ServerConfig.SYSPROP_PORT, "Hello World!");
 
         // act / assert
-        assertThrows(ConfigurationException.class, ServerConfig::init);
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Failed to parse port")
+                .withMessageContaining("Hello World!");
     }
 
     @Test
-    void testInit_invalidPort() {
+    void testInit_invalidPortTooSmall() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_PORT, "-5");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Port out of range")
+                .withMessageContaining("-5");
+    }
+
+    @Test
+    void testInit_invalidPortTooLarge() {
         // arrange
         System.setProperty(ServerConfig.SYSPROP_PORT, "99999");
 
         // act / assert
-        assertThrows(ConfigurationException.class, ServerConfig::init);
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Port out of range")
+                .withMessageContaining("99999");
+    }
+
+    @Test
+    void testInit_unparseableMaxMazeHeight() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_HEIGHT, "Hello World!");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Failed to parse maximum height")
+                .withMessageContaining("Hello World!");
+
+    }
+
+    @Test
+    void testInit_invalidMaxMazeHeight() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_HEIGHT, "1");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Maximum height must be greater than 1:")
+                .withMessageEndingWith("1");
+    }
+
+    @Test
+    void testInit_unparseableMaxMazeWidth() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_WIDTH, "Hello World!");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Failed to parse maximum width")
+                .withMessageContaining("Hello World!");
+
+    }
+
+    @Test
+    void testInit_invalidMaxMazeWidth() {
+        // arrange
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_WIDTH, "-24");
+
+        // act / assert
+        assertThatExceptionOfType(ConfigurationException.class)
+                .isThrownBy(ServerConfig::init)
+                .withMessageContaining("Maximum width must be greater than 1:")
+                .withMessageContaining("-24");
     }
 
     @Test
     void testInit() throws ConfigurationException {
         // arrange
-        System.setProperty(ServerConfig.SYSPROP_HOST, "127.0.0.1");
+        System.setProperty(ServerConfig.SYSPROP_HOST, "10.0.9.12");
         System.setProperty(ServerConfig.SYSPROP_PORT, "12345");
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_HEIGHT, "100");
+        System.setProperty(ServerConfig.SYSPROP_MAX_MAZE_WIDTH, "42");
 
         // act
         final ServerConfig sut = ServerConfig.init();
 
         // assert
-        assertEquals("127.0.0.1", sut.address().getHostAddress());
-        assertEquals(12345, sut.port());
+        assertThat(sut.address().getHostAddress()).isEqualTo("10.0.9.12");
+        assertThat(sut.port()).isEqualTo(12345);
+        assertThat(sut.maxMazeHeight()).singleElement().isEqualTo(100);
+        assertThat(sut.maxMazeWidth()).singleElement().isEqualTo(42);
     }
 }