#36 Introduce checks and cosntants for config

To make the code base more robust did some refactorings:

- Enumerte all the env var names and decouple internal from external names.
- Added some sane checks for config values.
- Add some basic tests.

Signed-off-by: Sven Strittmatter <sven.strittmatter@iteratec.com>

Author: Weltraumschaf

build.gradle

@@ -18,8 +18,9 @@ repositories {
 }
 
 dependencies {
-    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.6.0'
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
+    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.6.0'
+    testImplementation("org.junit.jupiter:junit-jupiter-params:5.9.2")
     testImplementation 'org.springframework:spring-test:5.3.23'
     testImplementation 'org.hamcrest:java-hamcrest:2.0.0.0'
     testImplementation 'org.mockito:mockito-core:3.+'

src/main/java/io/securecodebox/persistence/defectdojo/config/Config.java

@@ -5,10 +5,7 @@
 
 package io.securecodebox.persistence.defectdojo.config;
 
-import lombok.AllArgsConstructor;
-import lombok.Getter;
-import lombok.NonNull;
-import lombok.ToString;
+import lombok.*;
 
 import java.util.Optional;
 
@@ -17,7 +14,7 @@
  */
 @Getter
 @ToString
-@AllArgsConstructor
+@EqualsAndHashCode
 public final class Config {
     /**
      * Default for {@link #maxPageCountForGets}
@@ -30,10 +27,12 @@ public final class Config {
      * any path. The path to the concrete API endpoints are maintained by this client library itself.
      * </p>
      */
+    @NonNull
     private final String url;
     /**
      * API key to authorize against the DefectDojo API.
      */
+    @NonNull
     private final String apiKey;
     /**
      * This name is used to set the creator of entities created in DefectDojo (findings etc.).
@@ -45,6 +44,7 @@ public final class Config {
      *
      * @deprecated Must not be used anymore because we determine the userid via user_profile API endpoint.
      */
+    @NonNull
     @Deprecated
     private final String username;
 
@@ -73,6 +73,32 @@ public final class Config {
     @Deprecated
     private final Long userId;
 
+    /**
+     * Dedicated constructor
+     *
+     * @param url                 not {@code null}
+     * @param apiKey              not {@code null}
+     * @param username            not {@code null}
+     * @param maxPageCountForGets not less than 1
+     * @param userId              may be {@code null} (see {@link #userId})
+     */
+    public Config(final @NonNull String url, final @NonNull String apiKey, final @NonNull String username, final int maxPageCountForGets, final Long userId) {
+        super();
+        this.url = url;
+        this.apiKey = apiKey;
+        this.username = username;
+        this.maxPageCountForGets = validateIsGreaterZero(maxPageCountForGets, "maxPageCountForGets");
+        this.userId = userId;
+    }
+
+    private static int validateIsGreaterZero(final int number, final String name) {
+        if (number < 1) {
+            throw new IllegalArgumentException(String.format("%s must be greater than 0!", name));
+        }
+
+        return number;
+    }
+
     /**
      * Default constructor which sets {@link #userId} to {@code null}
      *
@@ -81,8 +107,7 @@ public final class Config {
      * @param username            not {@code null}
      * @param maxPageCountForGets not less than 1
      */
-    public Config(final @NonNull String url, final @NonNull String apiKey, final @NonNull String username, final int maxPageCountForGets) {
-        // FIXME: Implement check that maxPageCountForGets is not less than 1
+    public Config(final String url, final String apiKey, final String username, final int maxPageCountForGets) {
         this(url, apiKey, username, maxPageCountForGets, null);
     }
 
@@ -92,17 +117,43 @@ public Config(final @NonNull String url, final @NonNull String apiKey, final @No
      * @return never {@code null}
      */
     public static Config fromEnv() {
-        final var url = System.getenv("DEFECTDOJO_URL");
-        final var username = System.getenv("DEFECTDOJO_USERNAME");
-        final var apiKey = System.getenv("DEFECTDOJO_APIKEY");
-        final var userId = Optional.ofNullable(System.getenv("DEFECTDOJO_USER_ID")).map(Long::parseLong).orElse(null);
+        final var url = System.getenv(EnvVars.DEFECTDOJO_URL.literal);
+        final var username = System.getenv(EnvVars.DEFECTDOJO_USERNAME.literal);
+        final var apiKey = System.getenv(EnvVars.DEFECTDOJO_APIKEY.literal);
+        final var userId = Optional.ofNullable(System.getenv(EnvVars.DEFECTDOJO_USER_ID.literal))
+            .map(Long::parseLong).orElse(null);
 
-        int maxPageCountForGets = DEFAULT_MAX_PAGE_COUNT_FOR_GETS;
+        final int maxPageCountForGets;
 
-        if (System.getenv("DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS") != null) {
-            maxPageCountForGets = Integer.parseInt(System.getenv("DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS"));
+        if (System.getenv(EnvVars.DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS.literal) != null) {
+            maxPageCountForGets = Integer.parseInt(System.getenv(EnvVars.DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS.literal));
+        } else {
+            maxPageCountForGets = DEFAULT_MAX_PAGE_COUNT_FOR_GETS;
         }
 
         return new Config(url, apiKey, username, maxPageCountForGets, userId);
     }
+
+    /**
+     * Enumerates the available environment variables to configure the client
+     */
+    public enum EnvVars {
+        DEFECTDOJO_URL("DEFECTDOJO_URL"),
+        DEFECTDOJO_USERNAME("DEFECTDOJO_USERNAME"),
+        DEFECTDOJO_APIKEY("DEFECTDOJO_APIKEY"),
+        DEFECTDOJO_USER_ID("DEFECTDOJO_USER_ID"),
+        DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS("DEFECTDOJO_MAX_PAGE_COUNT_FOR_GETS");
+        /**
+         * Literal name of configuration environment name
+         * <p>
+         * We use an own value to hold the actual value, so that refactoring the enum name does
+         * not break the published API of env var names.
+         * </p>
+         */
+        private final String literal;
+
+        EnvVars(final String literal) {
+            this.literal = literal;
+        }
+    }
 }

src/test/java/io/securecodebox/persistence/defectdojo/config/ConfigTest.java

@@ -0,0 +1,53 @@
+package io.securecodebox.persistence.defectdojo.config;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+
+/**
+ * Tests for {@link Config}
+ */
+class ConfigTest {
+
+    @Test
+    void constructor_urlMustNotBeNull() {
+        final var thrown = assertThrows(NullPointerException.class, () ->{
+            new Config(null, "apiKey", "username", 1,null);
+        });
+
+        assertThat(thrown.getMessage(), startsWith("url "));
+    }
+
+    @Test
+    void constructor_apiKeyMustNotBeNull() {
+        final var thrown = assertThrows(NullPointerException.class, () ->{
+            new Config("url", null, "username", 1,null);
+        });
+
+        assertThat(thrown.getMessage(), startsWith("apiKey "));
+    }
+
+    @Test
+    void constructor_usernameMustNotBeNull() {
+        final var thrown = assertThrows(NullPointerException.class, () ->{
+            new Config("url", "apiKey", null, 1,null);
+        });
+
+        assertThat(thrown.getMessage(), startsWith("username "));
+    }
+
+    @ParameterizedTest
+    @ValueSource(ints = {0, -1, -2, -23, -42, Integer.MIN_VALUE})
+    void constructor_maxPageCountForGetsMustNotBeLessThanOne(final int number) {
+        final var thrown = assertThrows(IllegalArgumentException.class, () ->{
+            new Config("url", "apiKey", "username", number,null);
+        });
+
+        assertThat(thrown.getMessage(), startsWith("maxPageCountForGets "));
+    }
+
+}