#943: adopt functional programming style for helper classes

puzzle · Jun 14, 2024 · 2a92f6d · 2a92f6d
1 parent ddd504d
commit 2a92f6d
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 63 deletions.
diff --git a/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java b/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java
@@ -15,8 +15,11 @@
 import org.springframework.stereotype.Component;
 
 import java.text.MessageFormat;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Function;
 
 import static ch.puzzle.okr.Constants.USER;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
@@ -25,6 +28,8 @@
 public class JwtHelper {
     public static final String CLAIM_TENANT = "tenant";
     public static final String CLAIM_ISS = "iss";
+    public static final String ERROR_MESSAGE = "Missing `" + CLAIM_TENANT + "` and '" + CLAIM_ISS
+            + "' claims in JWT token!";
 
     private static final Logger logger = LoggerFactory.getLogger(JwtHelper.class);
 
@@ -61,36 +66,45 @@ public User getUserFromJwt(Jwt token) {
 
     public String getTenantFromToken(Jwt token) {
         TokenHelper helper = new TokenHelper();
+        List<Function<Jwt, Optional<String>>> getTenantFromTokenFunctions = Arrays.asList( //
+                helper::getTenantFromTokenUsingClaimIss, //
+                helper::getTenantFromTokenUsingClaimTenant //
+        );
 
-        Optional<String> tenantUsingClaimIss = helper.getTenantFromTokenUsingClaimIss(token);
-        if (tenantUsingClaimIss.isPresent()) {
-            return getMatchingTenantFromConfigOrThrow(tenantUsingClaimIss.get());
-        }
+        return getFirstMatchingTenantUsingListOfHelperFunctions(token, getTenantFromTokenFunctions);
+    }
 
-        Optional<String> tenantUsingClaimTenant = helper.getTenantFromTokenUsingClaimTenant(token);
-        if (tenantUsingClaimTenant.isPresent()) {
-            return getMatchingTenantFromConfigOrThrow(tenantUsingClaimTenant.get());
-        }
+    private String getFirstMatchingTenantUsingListOfHelperFunctions(Jwt token,
+            List<Function<Jwt, Optional<String>>> getTenantFunctions) {
 
-        logErrorAndThrowException(CLAIM_TENANT, CLAIM_ISS);
-        return null; // only to make the compiler happy
+        return getTenantFunctions.stream() //
+                .map(func -> func.apply(token)) //
+                .filter(Optional::isPresent) //
+                .map(Optional::get) //
+                .map(this::getMatchingTenantFromConfigOrThrow) //
+                .findFirst() //
+                .orElseThrow(() -> new RuntimeException(ERROR_MESSAGE));
     }
 
     public String getTenantFromJWTClaimsSet(JWTClaimsSet claimSet) {
         ClaimHelper helper = new ClaimHelper();
+        List<Function<JWTClaimsSet, Optional<String>>> getTenantFromClaimsSetFunctions = Arrays.asList( //
+                helper::getTenantFromClaimsSetUsingClaimIss, //
+                helper::getTenantFromClaimsSetUsingClaimTenant //
+        );
 
-        Optional<String> tenantUsingClaimIss = helper.getTenantFromClaimsSetUsingClaimIss(claimSet);
-        if (tenantUsingClaimIss.isPresent()) {
-            return getMatchingTenantFromConfigOrThrow(tenantUsingClaimIss.get());
-        }
+        return getFirstMatchingTenantUsingListOfHelperFunctions(claimSet, getTenantFromClaimsSetFunctions);
+    }
 
-        Optional<String> tenantUsingClaimTenant = helper.getTenantFromClaimsSetUsingClaimTenant(claimSet);
-        if (tenantUsingClaimTenant.isPresent()) {
-            return getMatchingTenantFromConfigOrThrow(tenantUsingClaimTenant.get());
-        }
+    private String getFirstMatchingTenantUsingListOfHelperFunctions(JWTClaimsSet claimSet,
+            List<Function<JWTClaimsSet, Optional<String>>> getTenantFunctions) {
 
-        logErrorAndThrowException(CLAIM_TENANT, CLAIM_ISS);
-        return null; // only to make the compiler happy
+        return getTenantFunctions.stream() //
+                .map(func -> func.apply(claimSet)) //
+                .filter(Optional::isPresent) //
+                .map(Optional::get) //
+                .map(this::getMatchingTenantFromConfigOrThrow).findFirst() //
+                .orElseThrow(() -> new RuntimeException(ERROR_MESSAGE));
     }
 
     private String getMatchingTenantFromConfigOrThrow(String tenant) {
@@ -100,10 +114,4 @@ private String getMatchingTenantFromConfigOrThrow(String tenant) {
                 .tenantId();
     }
 
-    private void logErrorAndThrowException(String tenant, String iss) throws RuntimeException {
-        String errorInfo = "* Missing `" + tenant + "` and '" + iss + "' claims in JWT token!";
-        logger.error(errorInfo);
-        throw new RuntimeException(errorInfo);
-    }
-
 }
diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java
@@ -14,43 +14,38 @@ public class ClaimHelper {
 
     public Optional<String> getTenantFromClaimsSetUsingClaimTenant(JWTClaimsSet claimSet) {
         try {
-            String tenant = getTenant(claimSet);
-            return Optional.ofNullable(tenant);
+            return getTenant(claimSet);
         } catch (ParseException e) {
             logStatus(CLAIM_TENANT, claimSet, e);
             return Optional.empty();
         }
     }
 
-    private static String getTenant(JWTClaimsSet claimSet) throws ParseException {
+    private Optional<String> getTenant(JWTClaimsSet claimSet) throws ParseException {
         String tenant = claimSet.getStringClaim(CLAIM_TENANT);
         logStatus(CLAIM_TENANT, claimSet, tenant);
-        return tenant;
+        return Optional.ofNullable(tenant);
     }
 
     public Optional<String> getTenantFromClaimsSetUsingClaimIss(JWTClaimsSet claimSet) {
         try {
-            String issUrl = getIssUrl(claimSet);
-            if (issUrl == null) {
-                return Optional.empty();
-            }
-            return getTenant(claimSet, issUrl);
+            return getIssUrl(claimSet).flatMap(url -> getTenant(claimSet, url));
         } catch (ParseException e) {
             logStatus(CLAIM_ISS, claimSet, e);
             return Optional.empty();
         }
     }
 
-    private static String getIssUrl(JWTClaimsSet claimSet) throws ParseException {
+    private Optional<String> getIssUrl(JWTClaimsSet claimSet) throws ParseException {
         String issUrl = claimSet.getStringClaim(CLAIM_ISS);
         logStatus(CLAIM_ISS, claimSet, issUrl);
-        return issUrl;
+        return Optional.ofNullable(issUrl);
     }
 
-    private static Optional<String> getTenant(JWTClaimsSet claimSet, String issUrl) {
-        String tenant = extractTenantFromIssUrl(issUrl);
-        logStatus(CLAIM_ISS, claimSet, tenant);
-        return Optional.ofNullable(tenant);
+    private Optional<String> getTenant(JWTClaimsSet claimSet, String issUrl) {
+        Optional<String> tenant = extractTenantFromIssUrl(issUrl);
+        logStatus(CLAIM_ISS, claimSet, tenant.isPresent());
+        return tenant;
     }
 
 }
diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java b/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java
@@ -10,7 +10,10 @@ public class JwtStatusLogger {
     private static final Logger logger = LoggerFactory.getLogger(ClaimHelper.class);
 
     public static void logStatus(String claim, Object context, String result) {
-        boolean isOk = result != null;
+        logStatus(claim, context, result != null);
+    }
+
+    public static void logStatus(String claim, Object context, boolean isOk) {
         if (isOk) {
             logger.info("Tenant: get claim '{}' from {}{}", claim, context.getClass().getSimpleName(),
                     statusToSymbol(isOk));

diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java
@@ -12,33 +12,28 @@
 public class TokenHelper {
 
     public Optional<String> getTenantFromTokenUsingClaimTenant(Jwt token) {
-        String tenant = getTenant(token);
-        return Optional.ofNullable(tenant);
+        return getTenant(token);
     }
 
-    private static String getTenant(Jwt token) {
+    private Optional<String> getTenant(Jwt token) {
         String tenant = token.getClaimAsString(CLAIM_TENANT); // can return null
         logStatus(CLAIM_TENANT, token, tenant);
-        return tenant;
+        return Optional.ofNullable(tenant);
     }
 
     public Optional<String> getTenantFromTokenUsingClaimIss(Jwt token) {
-        String issUrl = getIssUrl(token);
-        if (issUrl == null) {
-            return Optional.empty();
-        }
-        return getTenant(token, issUrl);
+        return getIssUrl(token).flatMap(url -> getTenant(token, url));
     }
 
-    private String getIssUrl(Jwt token) {
+    private Optional<String> getIssUrl(Jwt token) {
         String issUrl = token.getClaimAsString(CLAIM_ISS); // can return null
         logStatus(CLAIM_ISS, token, issUrl);
-        return issUrl;
+        return Optional.ofNullable(issUrl);
     }
 
     private Optional<String> getTenant(Jwt token, String issUrl) {
-        String tenant = extractTenantFromIssUrl(issUrl);
-        logStatus(CLAIM_ISS, token, tenant);
-        return Optional.ofNullable(tenant);
+        Optional<String> tenant = extractTenantFromIssUrl(issUrl);
+        logStatus(CLAIM_ISS, token, tenant.isPresent());
+        return tenant;
     }
 }
diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java
@@ -1,9 +1,14 @@
 package ch.puzzle.okr.security.helper;
 
+import java.util.Optional;
+
 public class UrlHelper {
 
-    public static String extractTenantFromIssUrl(String issUrl) {
+    public static Optional<String> extractTenantFromIssUrl(String issUrl) {
+        if (issUrl == null)
+            return Optional.empty();
         String[] issUrlParts = issUrl.split("/");
-        return issUrlParts[issUrlParts.length - 1];
+        String tenant = issUrlParts[issUrlParts.length - 1];
+        return Optional.of(tenant);
     }
 }
diff --git a/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java b/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java
@@ -5,7 +5,9 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.*;
 
 public class UrlHelperTest {
 
@@ -18,10 +20,11 @@ void extractTenantFromIssUrlReturnTenantIfUrlContainSlash(String issUrl) {
         // arrange
 
         // act
-        String tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl);
+        Optional<String> tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl);
 
         // assert
-        assertEquals(PITC, tenantFromIssUrl);
+        assertTrue(tenantFromIssUrl.isPresent());
+        assertEquals(PITC, tenantFromIssUrl.get());
     }
 
     @DisplayName("extractTenantFromIssUrl() return input url if url not contains slash")
@@ -31,9 +34,23 @@ void extractTenantFromIssUrlReturnInputIfUrlNotContainSlash() {
         String issUrl = "this_is_not_a_valid_url";
 
         // act
-        String tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl);
+        Optional<String> tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl);
+
+        // assert
+        assertTrue(tenantFromIssUrl.isPresent());
+        assertEquals(issUrl, tenantFromIssUrl.get());
+    }
+
+    @DisplayName("extractTenantFromIssUrl() return empty if url is null")
+    @Test
+    void extractTenantFromIssUrlReturnEmptyIfUrlIsNull() {
+        // arrange
+        String issUrl = null;
+
+        // act
+        Optional<String> tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl);
 
         // assert
-        assertEquals(issUrl, tenantFromIssUrl);
+        assertTrue(tenantFromIssUrl.isEmpty());
     }
 }