MockWebServerExtension argument checking, Javadocs, etc. (#560)

* Add more Javadocs to MockWebServerExtension
* Make uri() a "real" method (not Lombok generated) and check that the
URI has been assigned. Throw IllegalStateException if it has not been
assigned yet.
* Add argument checking to uri(path)
* Change uri(path) to check that the URI is assigned, and throw
IllegalStateException like uri() does when it is not yet assigned.
* Add a dedicated test. Previously, this extension was tested indirectly
via other tests, which did not include argument and state checking, etc.

Related to #547, #555, #556

src/main/java/org/kiwiproject/test/okhttp3/mockwebserver/MockWebServerExtension.java

@@ -1,12 +1,17 @@
 package org.kiwiproject.test.okhttp3.mockwebserver;
 
+import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.isNull;
+import static java.util.Objects.nonNull;
+import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
+import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotNull;
 import static org.kiwiproject.base.KiwiPreconditions.requireNotNull;
 
 import lombok.Builder;
 import lombok.Getter;
 import lombok.experimental.Accessors;
 import okhttp3.mockwebserver.MockWebServer;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -20,6 +25,13 @@
 /**
  * A simple JUnit Jupiter extension that creates and starts a {@link MockWebServer}
  * before <em>each</em> test, and shuts it down after <em>each</em> test.
+ * <p>
+ * You can create an instance using the constructors or builder.
+ * <p>
+ * If you need to perform customization of the {@code MockWebServer}, you can
+ * provide a "customizer" as a {@link Consumer} that accepts a {@code MockWebServer}.
+ * This allows you to configure the server to use TLS, to specify which protocols
+ * are supported, etc.
  */
 public class MockWebServerExtension implements BeforeEachCallback, AfterEachCallback {
 
@@ -30,17 +42,11 @@ public class MockWebServerExtension implements BeforeEachCallback, AfterEachCall
     @Accessors(fluent = true)
     private final MockWebServer server;
 
-    /**
-     * The base {@link URI} of the {@link MockWebServer}.
-     * <p>
-     * This is available after the {@link MockWebServer} has been started.
-     */
-    @Getter
-    @Accessors(fluent = true)
-    private URI uri;
-
     private final Consumer<MockWebServer> serverCustomizer;
 
+    // Assigned after the server is started
+    private URI uri;
+
     /**
      * Create a new instance.
      * <p>
@@ -58,36 +64,67 @@ public MockWebServerExtension() {
      *
      * @param server           the server
      * @param serverCustomizer allows a test to configure the server, e.g., to customize the protocols
-     *                         it supports or to serve requests via HTTPS over TLS.
+     *                         it supports or to serve requests via HTTPS over TLS. If this is
+     *                         {@code null}, it is ignored.
      */
     @Builder
-    MockWebServerExtension(MockWebServer server, Consumer<MockWebServer> serverCustomizer) {
+    MockWebServerExtension(MockWebServer server, @Nullable Consumer<MockWebServer> serverCustomizer) {
         this.server = requireNotNull(server, "server must not be null");
         this.serverCustomizer = isNull(serverCustomizer) ? KiwiConsumers.noOp() : serverCustomizer;
     }
 
+    /**
+     * Calls the server customizer {@link Consumer} if present, then starts the server
+     * and assigns the base {@link #uri()}.
+     *
+     * @param context the current extension context; never {@code null}
+     * @throws IOException if an error occurs starting the {@link MockWebServer}
+     */
     @Override
     public void beforeEach(ExtensionContext context) throws IOException {
         serverCustomizer.accept(server);
         server.start();
         uri = MockWebServers.uri(server);
     }
 
+    /**
+     * Closes the {@link MockWebServer}, ignoring any exceptions that are thrown.
+     *
+     * @param context the current extension context; never {@code null}
+     */
     @Override
     public void afterEach(ExtensionContext context) {
         KiwiIO.closeQuietly(server);
     }
 
     /**
-     * Get a {@link URI} with the give {@code path} that can be used to
+     * The base {@link URI} of the {@link MockWebServer}.
+     * <p>
+     * This is available after the {@link MockWebServer} has been started.
+     *
+     * @throws IllegalStateException if called before the server is started
+     */
+    public URI uri() {
+        checkState(nonNull(uri),
+                "server has not been started; only call this after beforeEach executes");
+        return uri;
+    }
+
+    /**
+     * Get a {@link URI} with the given {@code path} that can be used to
      * make calls to the {@link MockWebServer}.
      * <p>
      * This can be called after the {@link MockWebServer} has been started.
      *
-     * @param path the path
+     * @param path the path, not null. If the path is blank (whitespace only),
+     *             then it is normalized to an empty string before resolving
+     *             the relative URI.
      * @return a new {@link URI}
+     * @throws IllegalStateException if called before the server is started
      */
     public URI uri(String path) {
-        return uri.resolve(path);
+        checkArgumentNotNull(path, "path must not be null");
+        var normalizedPath = defaultIfBlank(path, "");
+        return uri().resolve(normalizedPath);
     }
 }

src/test/java/org/kiwiproject/test/okhttp3/mockwebserver/MockWebServerExtensionTest.java

@@ -0,0 +1,255 @@
+package org.kiwiproject.test.okhttp3.mockwebserver;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.only;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.common.base.Strings;
+import okhttp3.HttpUrl;
+import okhttp3.mockwebserver.MockWebServer;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.kiwiproject.test.junit.jupiter.params.provider.MinimalBlankStringSource;
+import org.kiwiproject.util.function.KiwiConsumers;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.function.Consumer;
+
+@DisplayName("MockWebServerExtension")
+class MockWebServerExtensionTest {
+
+    @Nested
+    class Constructors {
+
+        @Test
+        void shouldRequireNonNullServer() {
+            assertThatIllegalArgumentException()
+                    .isThrownBy(() -> new MockWebServerExtension(null, KiwiConsumers.noOp()))
+                    .withMessage("server must not be null");
+        }
+
+        @SuppressWarnings("resource")
+        @Test
+        void shouldAllowNullServerCustomizer() {
+            var server = new MockWebServer();
+            assertThatCode(() -> new MockWebServerExtension(server, null))
+                    .doesNotThrowAnyException();
+        }
+
+        @Test
+        void shouldCreateServer() {
+            var extension = new MockWebServerExtension();
+
+            assertThat(extension.server()).isNotNull();
+        }
+
+        @Test
+        void shouldSetSpecificServer() {
+            var server = new MockWebServer();
+            var extension = new MockWebServerExtension(server, null);
+
+            assertThat(extension.server()).isSameAs(server);
+        }
+    }
+
+    @Nested
+    class Builder {
+
+        @Test
+        void shouldRequireNonNullServer() {
+            assertThatIllegalArgumentException()
+                    .isThrownBy(() -> MockWebServerExtension.builder().build())
+                    .withMessage("server must not be null");
+        }
+
+        @SuppressWarnings("resource")
+        @Test
+        void shouldAllowNullServerCustomizer() {
+            var server = new MockWebServer();
+            assertThatCode(() -> MockWebServerExtension.builder()
+                    .server(server)
+                    .serverCustomizer(null)
+                    .build())
+                    .doesNotThrowAnyException();
+        }
+
+        @Test
+        void shouldSetSpecificServer() {
+            var server = new MockWebServer();
+            var extension = MockWebServerExtension.builder()
+                    .server(server)
+                    .build();
+
+            assertThat(extension.server()).isSameAs(server);
+        }
+    }
+
+    @Nested
+    class BeforeEachMethod {
+
+        @Test
+        void shouldStartServer() throws IOException {
+            var server = mock(MockWebServer.class);
+            when(server.url(anyString())).thenReturn(mock(HttpUrl.class));
+
+            var extension = MockWebServerExtension.builder().server(server).build();
+            extension.beforeEach(null);
+
+            verify(server).start();
+        }
+
+        @Test
+        void shouldCallCustomizer() throws IOException {
+            var server = mock(MockWebServer.class);
+            when(server.url(anyString())).thenReturn(mock(HttpUrl.class));
+
+            Consumer<MockWebServer> customizer = mock();
+
+            var extension = MockWebServerExtension.builder()
+                    .server(server)
+                    .serverCustomizer(customizer)
+                    .build();
+            extension.beforeEach(null);
+
+            verify(customizer, only()).accept(server);
+        }
+
+        @Test
+        void shouldAssignURI() throws IOException {
+            var server = mock(MockWebServer.class);
+            var httpUrl = mock(HttpUrl.class);
+            var uri = URI.create("/path");
+            when(httpUrl.uri()).thenReturn(uri);
+            when(server.url(anyString())).thenReturn(httpUrl);
+
+            var extension = MockWebServerExtension.builder().server(server).build();
+            extension.beforeEach(null);
+
+            assertThat(extension.uri()).isSameAs(uri);
+        }
+    }
+
+    @Nested
+    class AfterEachMethod {
+
+        @Test
+        void shouldCloseServer() throws IOException {
+            var server = mock(MockWebServer.class);
+
+            var extension = MockWebServerExtension.builder().server(server).build();
+            extension.afterEach(null);
+
+            verify(server, only()).close();
+        }
+
+        @Test
+        void shouldIgnoreExceptionsThrownByServerClose() throws IOException {
+            var server = mock(MockWebServer.class);
+            doThrow(new IOException("i/o error closing server"))
+                    .when(server)
+                    .close();
+
+            var extension = MockWebServerExtension.builder().server(server).build();
+
+            assertThatCode(() -> extension.afterEach(null))
+                    .doesNotThrowAnyException();
+
+            verify(server, only()).close();
+        }
+    }
+
+    @Nested
+    class UriMethod {
+
+        @Test
+        void shouldThrowIllegalStateException_WhenCalledBeforeServerIsStarted() {
+            var extension = new MockWebServerExtension();
+
+            assertThatIllegalStateException()
+                    .isThrownBy(extension::uri)
+                    .withMessage("server has not been started; only call this after beforeEach executes");
+        }
+    }
+
+    @Nested
+    class UriWithPath {
+
+        @Test
+        void shouldThrowIllegalStateException_WhenCalledBeforeServerIsStarted() {
+            var extension = new MockWebServerExtension();
+
+            assertThatIllegalStateException()
+                    .isThrownBy(() -> extension.uri("/path"))
+                    .withMessage("server has not been started; only call this after beforeEach executes");
+        }
+
+        @Test
+        void shouldNotAllowNullPath() throws IOException {
+            var server = new MockWebServer();
+            var extension = MockWebServerExtension.builder()
+                    .server(server)
+                    .build();
+
+            extension.beforeEach(null);
+
+            assertThatIllegalArgumentException()
+                    .isThrownBy(() -> extension.uri(null))
+                    .withMessage("path must not be null");
+        }
+
+        @ParameterizedTest
+        @MinimalBlankStringSource
+        void shouldNormalizeBlankPaths(String path) throws IOException {
+            var server = new MockWebServer();
+            var extension = MockWebServerExtension.builder()
+                    .server(server)
+                    .build();
+
+            extension.beforeEach(null);
+
+            // MinimalBlankStringSource gives us a null, which we need to ignore,
+            // so convert it to an empty string.
+            var nonNullPath = Strings.nullToEmpty(path);
+
+            assertThat(extension.uri(nonNullPath))
+                    .extracting(URI::getPath)
+                    .isEqualTo("/");
+        }
+
+        @ParameterizedTest
+        @ValueSource(strings = {
+                "",
+                "/",
+                "/status",
+                "/foo/bar"
+        })
+        void shouldResolvePaths(String path) throws IOException {
+            var server = new MockWebServer();
+            var extension = MockWebServerExtension.builder()
+                    .server(server)
+                    .build();
+
+            extension.beforeEach(null);
+
+            // handle the special case of an empty string; the resulting
+            // path should end with a slash
+            var expectedPath = path.isEmpty() ? "/" : path;
+
+            assertThat(extension.uri(path))
+                    .isEqualTo(extension.uri().resolve(path))
+                    .extracting(URI::getPath)
+                    .isEqualTo(expectedPath);
+        }
+    }
+}