From 4defbc2f6a8183edfbd3505527bd02e2652d23ad Mon Sep 17 00:00:00 2001
From: Alex Morel <amorel@codelutin.com>
Date: Fri, 19 Jul 2024 10:36:21 +0200
Subject: [PATCH] Refactoring to lower methods complexity

---
 .../core/service/AbstractScimService.java     | 62 +++++++++++--------
 .../libre/scim/core/service/ScimClient.java   |  4 +-
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/src/main/java/sh/libre/scim/core/service/AbstractScimService.java b/src/main/java/sh/libre/scim/core/service/AbstractScimService.java
index 8397464a01..af2e938695 100644
--- a/src/main/java/sh/libre/scim/core/service/AbstractScimService.java
+++ b/src/main/java/sh/libre/scim/core/service/AbstractScimService.java
@@ -139,41 +139,19 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
             EntityOnRemoteScimId externalId = resource.getId()
                     .map(EntityOnRemoteScimId::new)
                     .orElseThrow(() -> new UnexpectedScimDataException("Remote SCIM resource doesn't have an id, cannot import it in Keycloak"));
-            Optional<ScimResourceMapping> optionalMapping = getScimResourceDao().findByExternalId(externalId, type);
-
-            // If an existing mapping exists, delete potential dangling references
-            if (optionalMapping.isPresent()) {
-                ScimResourceMapping mapping = optionalMapping.get();
-                if (entityExists(mapping.getIdAsKeycloakId())) {
-                    LOGGER.debug("[SCIM] Valid mapping found, skipping");
-                    return;
-                } else {
-                    LOGGER.info("[SCIM] Delete a dangling mapping");
-                    getScimResourceDao().delete(mapping);
-                }
-            }
+            if (validMappingAlreadyExists(externalId)) return;
 
             // Here no keycloak user/group matching the SCIM external id exists
             // Try to match existing keycloak resource by properties (username, email, name)
             Optional<KeycloakId> mapped = matchKeycloakMappingByScimProperties(resource);
             if (mapped.isPresent()) {
+                // If found a mapped, update
                 LOGGER.info("[SCIM] Matched SCIM resource " + externalId + " from properties with keycloak entity " + mapped.get());
                 createMapping(mapped.get(), externalId);
                 syncRes.increaseUpdated();
             } else {
-                switch (scimProviderConfiguration.getImportAction()) {
-                    case CREATE_LOCAL -> {
-                        LOGGER.info("[SCIM] Create local resource for SCIM resource " + externalId);
-                        KeycloakId id = createEntity(resource);
-                        createMapping(id, externalId);
-                        syncRes.increaseAdded();
-                    }
-                    case DELETE_REMOTE -> {
-                        LOGGER.info("[SCIM] Delete remote resource " + externalId);
-                        scimClient.delete(externalId);
-                    }
-                    case NOTHING -> LOGGER.info("[SCIM] Import action set to NOTHING");
-                }
+                // If not, create it locally or deleting it remotely (according to the configured Import Action)
+                createLocalOrDeleteRemote(syncRes, resource, externalId);
             }
         } catch (UnexpectedScimDataException e) {
             if (skipOrStopStrategy.skipInvalidDataFromScimEndpoint(getConfiguration())) {
@@ -198,6 +176,38 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
 
     }
 
+    private boolean validMappingAlreadyExists(EntityOnRemoteScimId externalId) {
+        Optional<ScimResourceMapping> optionalMapping = getScimResourceDao().findByExternalId(externalId, type);
+        // If an existing mapping exists, delete potential dangling references
+        if (optionalMapping.isPresent()) {
+            ScimResourceMapping mapping = optionalMapping.get();
+            if (entityExists(mapping.getIdAsKeycloakId())) {
+                LOGGER.debug("[SCIM] Valid mapping found, skipping");
+                return true;
+            } else {
+                LOGGER.info("[SCIM] Delete a dangling mapping");
+                getScimResourceDao().delete(mapping);
+            }
+        }
+        return false;
+    }
+
+    private void createLocalOrDeleteRemote(SynchronizationResult syncRes, S resource, EntityOnRemoteScimId externalId) throws UnexpectedScimDataException, InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
+        switch (scimProviderConfiguration.getImportAction()) {
+            case CREATE_LOCAL -> {
+                LOGGER.info("[SCIM] Create local resource for SCIM resource " + externalId);
+                KeycloakId id = createEntity(resource);
+                createMapping(id, externalId);
+                syncRes.increaseAdded();
+            }
+            case DELETE_REMOTE -> {
+                LOGGER.info("[SCIM] Delete remote resource " + externalId);
+                scimClient.delete(externalId);
+            }
+            case NOTHING -> LOGGER.info("[SCIM] Import action set to NOTHING");
+        }
+    }
+
 
     protected abstract S scimRequestBodyForCreate(K roleMapperModel) throws InconsistentScimMappingException;
 
diff --git a/src/main/java/sh/libre/scim/core/service/ScimClient.java b/src/main/java/sh/libre/scim/core/service/ScimClient.java
index 8a5cd738f5..64b2474d02 100644
--- a/src/main/java/sh/libre/scim/core/service/ScimClient.java
+++ b/src/main/java/sh/libre/scim/core/service/ScimClient.java
@@ -42,7 +42,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
         this.logAllRequests = detailedLogs;
     }
 
-    public static ScimClient open(ScrimEndPointConfiguration scimProviderConfiguration, ScimResourceType scimResourceType) {
+    public static <T extends ResourceNode> ScimClient<T> open(ScrimEndPointConfiguration scimProviderConfiguration, ScimResourceType scimResourceType) {
         String scimApplicationBaseUrl = scimProviderConfiguration.getEndPoint();
         Map<String, String> httpHeaders = new HashMap<>();
         httpHeaders.put(HttpHeaders.AUTHORIZATION, scimProviderConfiguration.getAuthorizationHeaderValue());
@@ -58,7 +58,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
                         scimApplicationBaseUrl,
                         scimClientConfig
                 );
-        return new ScimClient(scimRequestBuilder, scimResourceType, scimProviderConfiguration.isLogAllScimRequests());
+        return new ScimClient<>(scimRequestBuilder, scimResourceType, scimProviderConfiguration.isLogAllScimRequests());
     }
 
     public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws InvalidResponseFromScimEndpointException {