From 3a1448f67e0afe425eb61c894d7525aa9bf692c8 Mon Sep 17 00:00:00 2001 From: Patrick Hobusch Date: Wed, 13 May 2026 21:49:11 +0800 Subject: [PATCH] Extract AbstractAuthenticationService to share logic between jira and confluence The shared service plumbing (bulk set with declarative null-model and map-key fallback semantics, single-entity set, find-by-name, get/set SSO) lives in a new AbstractAuthenticationService in commons. Host-specific subclasses provide abstract hooks that delegate to their existing per-host model utils. Add tests for the null-model and null-name semantics in both AuthenticationServiceTests. --- .../AbstractAuthenticationService.java | 126 ++++++++++++++++++ .../service/AuthenticationServiceImpl.java | 74 +++------- .../service/AuthenticationServiceTest.java | 40 ++++++ .../service/AuthenticationServiceImpl.java | 73 +++------- .../service/AuthenticationServiceTest.java | 40 ++++++ 5 files changed, 238 insertions(+), 115 deletions(-) create mode 100644 commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractAuthenticationService.java diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractAuthenticationService.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractAuthenticationService.java new file mode 100644 index 00000000..07409622 --- /dev/null +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractAuthenticationService.java @@ -0,0 +1,126 @@ +package com.deftdevs.bootstrapi.commons.service; + +import com.atlassian.plugins.authentication.api.config.IdpConfig; +import com.atlassian.plugins.authentication.api.config.IdpConfigService; +import com.atlassian.plugins.authentication.api.config.SsoConfig; +import com.atlassian.plugins.authentication.api.config.SsoConfigService; +import com.deftdevs.bootstrapi.commons.exception.web.BadRequestException; +import com.deftdevs.bootstrapi.commons.model.AbstractAuthenticationIdpModel; +import com.deftdevs.bootstrapi.commons.model.AuthenticationSsoModel; +import com.deftdevs.bootstrapi.commons.service.api.AuthenticationService; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; + +public abstract class AbstractAuthenticationService + implements AuthenticationService { + + protected final IdpConfigService idpConfigService; + protected final SsoConfigService ssoConfigService; + + protected AbstractAuthenticationService( + final IdpConfigService idpConfigService, + final SsoConfigService ssoConfigService) { + + this.idpConfigService = idpConfigService; + this.ssoConfigService = ssoConfigService; + } + + @Override + public Map getAuthenticationIdps() { + return idpConfigService.getIdpConfigs().stream() + .map(this::toAuthenticationIdpModel) + .collect(Collectors.toMap(AbstractAuthenticationIdpModel::getName, Function.identity())); + } + + @Override + public Map setAuthenticationIdps( + final Map authenticationIdpModels) { + + final Map resultIdpModels = new LinkedHashMap<>(); + + for (Map.Entry entry : authenticationIdpModels.entrySet()) { + final String idpName = entry.getKey(); + final IB idpModel = entry.getValue(); + + if (idpModel == null) { + // declarative no-op: null model + existing entity → return as-is; + // null model + missing entity → nothing to do + final IdpConfig existingIdpConfig = findIdpConfigByName(idpName); + if (existingIdpConfig != null) { + resultIdpModels.put(idpName, toAuthenticationIdpModel(existingIdpConfig)); + } + continue; + } + + if (idpModel.getName() == null) { + idpModel.setName(idpName); + } + + final IB resultIdpModel = setAuthenticationIdp(idpModel); + resultIdpModels.put(resultIdpModel.getName(), resultIdpModel); + } + + return resultIdpModels; + } + + @Override + public IB setAuthenticationIdp( + final IB authenticationIdpModel) { + + if (authenticationIdpModel.getName() == null || authenticationIdpModel.getName().trim().isEmpty()) { + throw new BadRequestException("The name cannot be empty"); + } + + final IdpConfig existingIdpConfig = findIdpConfigByName(authenticationIdpModel.getName()); + + if (existingIdpConfig == null) { + final IdpConfig idpConfig = toIdpConfig(authenticationIdpModel); + final IdpConfig addedIdpConfig = idpConfigService.addIdpConfig(idpConfig); + return toAuthenticationIdpModel(addedIdpConfig); + } + + final IdpConfig idpConfig = toIdpConfig(authenticationIdpModel, existingIdpConfig); + final IdpConfig updatedIdpConfig = idpConfigService.updateIdpConfig(idpConfig); + return toAuthenticationIdpModel(updatedIdpConfig); + } + + @Override + public SB getAuthenticationSso() { + return toAuthenticationSsoModel(ssoConfigService.getSsoConfig()); + } + + @Override + public SB setAuthenticationSso( + final SB authenticationSsoModel) { + + final SsoConfig existingSsoConfig = ssoConfigService.getSsoConfig(); + final SsoConfig ssoConfig = toSsoConfig(authenticationSsoModel, existingSsoConfig); + return toAuthenticationSsoModel(ssoConfigService.updateSsoConfig(ssoConfig)); + } + + IdpConfig findIdpConfigByName( + final String name) { + + final Map idpConfigsByName = idpConfigService.getIdpConfigs().stream().collect(Collectors.toMap( + IdpConfig::getName, Function.identity(), (existing, replacement) -> { + throw new IllegalStateException("Duplicate name key found: " + existing.getName()); + } + )); + + return idpConfigsByName.get(name); + } + + protected abstract IB toAuthenticationIdpModel(IdpConfig idpConfig); + + protected abstract IdpConfig toIdpConfig(IB authenticationIdpModel); + + protected abstract IdpConfig toIdpConfig(IB authenticationIdpModel, IdpConfig existingIdpConfig); + + protected abstract SB toAuthenticationSsoModel(SsoConfig ssoConfig); + + protected abstract SsoConfig toSsoConfig(SB authenticationSsoModel, SsoConfig existingSsoConfig); + +} diff --git a/confluence/src/main/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceImpl.java b/confluence/src/main/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceImpl.java index 470a4909..66462a14 100644 --- a/confluence/src/main/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceImpl.java +++ b/confluence/src/main/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceImpl.java @@ -4,89 +4,47 @@ import com.atlassian.plugins.authentication.api.config.IdpConfigService; import com.atlassian.plugins.authentication.api.config.SsoConfig; import com.atlassian.plugins.authentication.api.config.SsoConfigService; -import com.deftdevs.bootstrapi.commons.exception.web.BadRequestException; import com.deftdevs.bootstrapi.commons.model.AbstractAuthenticationIdpModel; import com.deftdevs.bootstrapi.commons.model.AuthenticationSsoModel; +import com.deftdevs.bootstrapi.commons.service.AbstractAuthenticationService; import com.deftdevs.bootstrapi.confluence.model.util.AuthenticationIdpModelUtil; import com.deftdevs.bootstrapi.confluence.model.util.AuthenticationSsoModelUtil; import com.deftdevs.bootstrapi.confluence.service.api.ConfluenceAuthenticationService; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - -public class AuthenticationServiceImpl implements ConfluenceAuthenticationService { - - private final IdpConfigService idpConfigService; - - private final SsoConfigService ssoConfigService; +public class AuthenticationServiceImpl + extends AbstractAuthenticationService + implements ConfluenceAuthenticationService { public AuthenticationServiceImpl( final IdpConfigService idpConfigService, final SsoConfigService ssoConfigService) { - this.idpConfigService = idpConfigService; - this.ssoConfigService = ssoConfigService; + super(idpConfigService, ssoConfigService); } @Override - public Map getAuthenticationIdps() { - return idpConfigService.getIdpConfigs().stream() - .map(AuthenticationIdpModelUtil::toAuthenticationIdpModel) - .collect(Collectors.toMap(AbstractAuthenticationIdpModel::getName, Function.identity())); + protected AbstractAuthenticationIdpModel toAuthenticationIdpModel(final IdpConfig idpConfig) { + return AuthenticationIdpModelUtil.toAuthenticationIdpModel(idpConfig); } @Override - public Map setAuthenticationIdps( - final Map authenticationIdpModels) { - - return authenticationIdpModels.values().stream() - .map(this::setAuthenticationIdp) - .collect(Collectors.toMap(AbstractAuthenticationIdpModel::getName, Function.identity())); - } - - public AbstractAuthenticationIdpModel setAuthenticationIdp( - final AbstractAuthenticationIdpModel authenticationIdpModel) { - - if (authenticationIdpModel.getName() == null || authenticationIdpModel.getName().trim().isEmpty()) { - throw new BadRequestException("The name cannot be empty"); - } - - final IdpConfig existingIdpConfig = findIdpConfigByName(authenticationIdpModel.getName()); - - if (existingIdpConfig == null) { - final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel); - final IdpConfig addedIdpConfig = idpConfigService.addIdpConfig(idpConfig); - return AuthenticationIdpModelUtil.toAuthenticationIdpModel(addedIdpConfig); - } - - final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel, existingIdpConfig); - final IdpConfig updatedIdpConfig = idpConfigService.updateIdpConfig(idpConfig); - return AuthenticationIdpModelUtil.toAuthenticationIdpModel(updatedIdpConfig); + protected IdpConfig toIdpConfig(final AbstractAuthenticationIdpModel authenticationIdpModel) { + return AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel); } @Override - public AuthenticationSsoModel getAuthenticationSso() { - return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfigService.getSsoConfig()); + protected IdpConfig toIdpConfig(final AbstractAuthenticationIdpModel authenticationIdpModel, final IdpConfig existingIdpConfig) { + return AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel, existingIdpConfig); } @Override - public AuthenticationSsoModel setAuthenticationSso(AuthenticationSsoModel authenticationSsoModel) { - final SsoConfig existingSsoConfig = ssoConfigService.getSsoConfig(); - final SsoConfig ssoConfig = AuthenticationSsoModelUtil.toSsoConfig(authenticationSsoModel, existingSsoConfig); - return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfigService.updateSsoConfig(ssoConfig)); + protected AuthenticationSsoModel toAuthenticationSsoModel(final SsoConfig ssoConfig) { + return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfig); } - IdpConfig findIdpConfigByName( - final String name) { - - final Map idpConfigsByName = idpConfigService.getIdpConfigs().stream().collect(Collectors.toMap( - IdpConfig::getName, Function.identity(), (existing, replacement) -> { - throw new IllegalStateException("Duplicate name key found: " + existing.getName()); - } - )); - - return idpConfigsByName.get(name); + @Override + protected SsoConfig toSsoConfig(final AuthenticationSsoModel authenticationSsoModel, final SsoConfig existingSsoConfig) { + return AuthenticationSsoModelUtil.toSsoConfig(authenticationSsoModel, existingSsoConfig); } } diff --git a/confluence/src/test/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceTest.java b/confluence/src/test/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceTest.java index 69b02766..808b8735 100644 --- a/confluence/src/test/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceTest.java +++ b/confluence/src/test/java/com/deftdevs/bootstrapi/confluence/service/AuthenticationServiceTest.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -117,6 +118,45 @@ void testSetAuthenticationIdpsNameEmpty() { }); } + @Test + void testSetAuthenticationIdpsNullNameUsesMapKey() { + final String mapKey = "from-map-key"; + final AuthenticationIdpOidcModel modelWithoutName = AuthenticationIdpOidcModel.builder().build(); + final Map authenticationIdpModels = Collections.singletonMap(mapKey, modelWithoutName); + doAnswer(invocation -> invocation.getArgument(0)).when(idpConfigService).addIdpConfig(any()); + + authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(idpConfigService, times(1)).addIdpConfig(any()); + } + + @Test + void testSetAuthenticationIdpsNullModelMissingIdpSkipsEntry() { + final Map authenticationIdpModels = new LinkedHashMap<>(); + authenticationIdpModels.put("missing-idp", null); + + final Map result = authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertTrue(result.isEmpty()); + verify(idpConfigService, times(0)).addIdpConfig(any()); + verify(idpConfigService, times(0)).updateIdpConfig(any()); + } + + @Test + void testSetAuthenticationIdpsNullModelExistingIdpReturnedAsIs() { + final AuthenticationIdpOidcModel idpModel = AuthenticationIdpOidcModel.EXAMPLE_1; + final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(idpModel); + doReturn(Collections.singletonList(idpConfig)).when(idpConfigService).getIdpConfigs(); + + final Map authenticationIdpModels = new LinkedHashMap<>(); + authenticationIdpModels.put(idpModel.getName(), null); + + final Map result = authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertEquals(1, result.size()); + assertEquals(idpModel.getName(), result.values().iterator().next().getName()); + verify(idpConfigService, times(0)).addIdpConfig(any()); + verify(idpConfigService, times(0)).updateIdpConfig(any()); + } + @Test void testGetAuthenticationSso() { final SsoConfig ssoConfig = ImmutableSsoConfig.builder() diff --git a/jira/src/main/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceImpl.java b/jira/src/main/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceImpl.java index 9208108a..9ce75727 100644 --- a/jira/src/main/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceImpl.java +++ b/jira/src/main/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceImpl.java @@ -4,88 +4,47 @@ import com.atlassian.plugins.authentication.api.config.IdpConfigService; import com.atlassian.plugins.authentication.api.config.SsoConfig; import com.atlassian.plugins.authentication.api.config.SsoConfigService; -import com.deftdevs.bootstrapi.commons.exception.web.BadRequestException; import com.deftdevs.bootstrapi.commons.model.AbstractAuthenticationIdpModel; import com.deftdevs.bootstrapi.commons.model.AuthenticationSsoModel; +import com.deftdevs.bootstrapi.commons.service.AbstractAuthenticationService; import com.deftdevs.bootstrapi.jira.model.util.AuthenticationIdpModelUtil; import com.deftdevs.bootstrapi.jira.model.util.AuthenticationSsoModelUtil; import com.deftdevs.bootstrapi.jira.service.api.JiraAuthenticationService; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - -public class AuthenticationServiceImpl implements JiraAuthenticationService { - - private final IdpConfigService idpConfigService; - private final SsoConfigService ssoConfigService; +public class AuthenticationServiceImpl + extends AbstractAuthenticationService + implements JiraAuthenticationService { public AuthenticationServiceImpl( final IdpConfigService idpConfigService, final SsoConfigService ssoConfigService) { - this.idpConfigService = idpConfigService; - this.ssoConfigService = ssoConfigService; + super(idpConfigService, ssoConfigService); } @Override - public Map getAuthenticationIdps() { - return idpConfigService.getIdpConfigs().stream() - .map(AuthenticationIdpModelUtil::toAuthenticationIdpModel) - .collect(Collectors.toMap(AbstractAuthenticationIdpModel::getName, Function.identity())); + protected AbstractAuthenticationIdpModel toAuthenticationIdpModel(final IdpConfig idpConfig) { + return AuthenticationIdpModelUtil.toAuthenticationIdpModel(idpConfig); } @Override - public Map setAuthenticationIdps( - final Map authenticationIdpModels) { - - return authenticationIdpModels.values().stream() - .map(this::setAuthenticationIdp) - .collect(Collectors.toMap(AbstractAuthenticationIdpModel::getName, Function.identity())); - } - - public AbstractAuthenticationIdpModel setAuthenticationIdp( - final AbstractAuthenticationIdpModel authenticationIdpModel) { - - if (authenticationIdpModel.getName() == null || authenticationIdpModel.getName().trim().isEmpty()) { - throw new BadRequestException("The name cannot be empty"); - } - - final IdpConfig existingIdpConfig = findIdpConfigByName(authenticationIdpModel.getName()); - - if (existingIdpConfig == null) { - final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel); - final IdpConfig addedIdpConfig = idpConfigService.addIdpConfig(idpConfig); - return AuthenticationIdpModelUtil.toAuthenticationIdpModel(addedIdpConfig); - } - - final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel, existingIdpConfig); - final IdpConfig updatedIdpConfig = idpConfigService.updateIdpConfig(idpConfig); - return AuthenticationIdpModelUtil.toAuthenticationIdpModel(updatedIdpConfig); + protected IdpConfig toIdpConfig(final AbstractAuthenticationIdpModel authenticationIdpModel) { + return AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel); } @Override - public AuthenticationSsoModel getAuthenticationSso() { - return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfigService.getSsoConfig()); + protected IdpConfig toIdpConfig(final AbstractAuthenticationIdpModel authenticationIdpModel, final IdpConfig existingIdpConfig) { + return AuthenticationIdpModelUtil.toIdpConfig(authenticationIdpModel, existingIdpConfig); } @Override - public AuthenticationSsoModel setAuthenticationSso(AuthenticationSsoModel authenticationSsoModel) { - final SsoConfig existingSsoConfig = ssoConfigService.getSsoConfig(); - final SsoConfig ssoConfig = AuthenticationSsoModelUtil.toSsoConfig(authenticationSsoModel, existingSsoConfig); - return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfigService.updateSsoConfig(ssoConfig)); + protected AuthenticationSsoModel toAuthenticationSsoModel(final SsoConfig ssoConfig) { + return AuthenticationSsoModelUtil.toAuthenticationSsoModel(ssoConfig); } - IdpConfig findIdpConfigByName( - final String name) { - - final Map idpConfigsByName = idpConfigService.getIdpConfigs().stream().collect(Collectors.toMap( - IdpConfig::getName, Function.identity(), (existing, replacement) -> { - throw new IllegalStateException("Duplicate name key found: " + existing.getName()); - } - )); - - return idpConfigsByName.get(name); + @Override + protected SsoConfig toSsoConfig(final AuthenticationSsoModel authenticationSsoModel, final SsoConfig existingSsoConfig) { + return AuthenticationSsoModelUtil.toSsoConfig(authenticationSsoModel, existingSsoConfig); } } diff --git a/jira/src/test/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceTest.java b/jira/src/test/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceTest.java index c93b5372..33c1e9f5 100644 --- a/jira/src/test/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceTest.java +++ b/jira/src/test/java/com/deftdevs/bootstrapi/jira/service/AuthenticationServiceTest.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -117,6 +118,45 @@ void testSetAuthenticationIdpsNameEmpty() { }); } + @Test + void testSetAuthenticationIdpsNullNameUsesMapKey() { + final String mapKey = "from-map-key"; + final AuthenticationIdpOidcModel modelWithoutName = AuthenticationIdpOidcModel.builder().build(); + final Map authenticationIdpModels = Collections.singletonMap(mapKey, modelWithoutName); + doAnswer(invocation -> invocation.getArgument(0)).when(idpConfigService).addIdpConfig(any()); + + authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(idpConfigService, times(1)).addIdpConfig(any()); + } + + @Test + void testSetAuthenticationIdpsNullModelMissingIdpSkipsEntry() { + final Map authenticationIdpModels = new LinkedHashMap<>(); + authenticationIdpModels.put("missing-idp", null); + + final Map result = authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertTrue(result.isEmpty()); + verify(idpConfigService, times(0)).addIdpConfig(any()); + verify(idpConfigService, times(0)).updateIdpConfig(any()); + } + + @Test + void testSetAuthenticationIdpsNullModelExistingIdpReturnedAsIs() { + final AuthenticationIdpOidcModel idpModel = AuthenticationIdpOidcModel.EXAMPLE_1; + final IdpConfig idpConfig = AuthenticationIdpModelUtil.toIdpConfig(idpModel); + doReturn(Collections.singletonList(idpConfig)).when(idpConfigService).getIdpConfigs(); + + final Map authenticationIdpModels = new LinkedHashMap<>(); + authenticationIdpModels.put(idpModel.getName(), null); + + final Map result = authenticationService.setAuthenticationIdps(authenticationIdpModels); + assertEquals(1, result.size()); + assertEquals(idpModel.getName(), result.values().iterator().next().getName()); + verify(idpConfigService, times(0)).addIdpConfig(any()); + verify(idpConfigService, times(0)).updateIdpConfig(any()); + } + @Test void testGetAuthenticationSso() { final SsoConfig ssoConfig = ImmutableSsoConfig.builder()