From 843ff7e5da4997f2525852ad60318ec7ed5cf7d4 Mon Sep 17 00:00:00 2001 From: Patrick Hobusch Date: Wed, 13 May 2026 00:21:52 +0800 Subject: [PATCH] Make null models declarative no-ops and use boolean map for user group membership Null model semantics across all bulk set services (setGroups, setUsers, setDirectories, setApplications, setApplicationLinks) are now declarative: null + existing entity returns it as-is, null + missing entity is a no-op. This makes multi-document YAML config merging work without forcing later documents to re-specify entire models just to reference an entry. UserModel.groups changes from Map to Map: true ensures membership, false ensures non-membership, null is a no-op. Avoids duplicating group definitions inside user definitions; group lifecycle is managed only via the top-level groups map. Idempotent in both directions (MembershipAlreadyExistsException and MembershipNotFoundException are swallowed). Single-entity create endpoints (createGroup, addUser) still reject null models since there is no map key to fall back on. Name fallback from map key remains when the model name field is null. --- .../bootstrapi/commons/model/UserModel.java | 2 +- .../service/AbstractDirectoriesService.java | 20 ++++- .../DefaultApplicationLinksServiceImpl.java | 9 ++ .../DefaultApplicationLinkServiceTest.java | 29 +++++++ confluence/Models/UserModel.md | 2 +- crowd/Models/UserModel.md | 2 +- .../crowd/config/ServiceConfig.java | 3 +- .../service/ApplicationsServiceImpl.java | 19 ++++- .../crowd/service/GroupsServiceImpl.java | 48 ++++++++--- .../crowd/service/UsersServiceImpl.java | 84 +++++++++++-------- .../service/ApplicationsServiceTest.java | 37 ++++++++ .../crowd/service/DirectoriesServiceTest.java | 48 +++++++++++ .../crowd/service/GroupsServiceTest.java | 40 +++++++-- .../crowd/service/UsersServiceTest.java | 76 +++++++++++------ jira/Models/UserModel.md | 2 +- 15 files changed, 335 insertions(+), 86 deletions(-) diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/model/UserModel.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/model/UserModel.java index 4f7437d8..e763ed6e 100644 --- a/commons/src/main/java/com/deftdevs/bootstrapi/commons/model/UserModel.java +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/model/UserModel.java @@ -44,7 +44,7 @@ public class UserModel { private String password; @XmlElement - private Map groups; + private Map groups; // Example instances for documentation and tests diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java index 49a07cf0..9b383378 100644 --- a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java @@ -22,20 +22,34 @@ public abstract class AbstractDirectoriesService implements DirectoriesService { final Map resultDirectories = new LinkedHashMap<>(); for (Map.Entry directoryModelEntry : directoryModels.entrySet()) { + final String directoryName = directoryModelEntry.getKey(); final AbstractDirectoryModel directoryModel = directoryModelEntry.getValue(); + final AbstractDirectoryModel existingDirectoryModel = existingDirectoriesByName.get(directoryName); + + if (directoryModel == null) { + // declarative no-op: null model + existing entity → return as-is; + // null model + missing entity → nothing to do + if (existingDirectoryModel != null) { + resultDirectories.put(existingDirectoryModel.getName(), existingDirectoryModel); + } + continue; + } + + if (directoryModel.getName() == null) { + directoryModel.setName(directoryName); + } // Check if directoryModel is not an instance of any supported class if (getSupportedClassesForUpdate().stream().noneMatch(clazz -> clazz.isInstance(directoryModel))) { throw new BadRequestException(String.format("Updating directory type '%s' is not supported (yet)", directoryModel.getClass())); } - final AbstractDirectoryModel existingDirectoryModel = existingDirectoriesByName.get(directoryModelEntry.getKey()); final AbstractDirectoryModel resultDirectoryModel; if (existingDirectoryModel != null) { - resultDirectoryModel = setDirectory(existingDirectoryModel.getId(), directoryModelEntry.getValue()); + resultDirectoryModel = setDirectory(existingDirectoryModel.getId(), directoryModel); } else { - resultDirectoryModel = addDirectory(directoryModelEntry.getValue()); + resultDirectoryModel = addDirectory(directoryModel); } resultDirectories.put(resultDirectoryModel.getName(), resultDirectoryModel); diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java index e5eb1e69..28a9b7ce 100644 --- a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java @@ -106,6 +106,15 @@ public Map setApplicationLinks( final String name = applicationLinkModelEntry.getKey(); final ApplicationLinkModel applicationLinkModel = applicationLinkModelEntry.getValue(); + // declarative no-op: null model means "leave this entry untouched" + if (applicationLinkModel == null) { + continue; + } + + if (applicationLinkModel.getName() == null) { + applicationLinkModel.setName(name); + } + if (linkModelMap.containsKey(name)) { setApplicationLink(linkModelMap.get(name).getUuid(), applicationLinkModel); } else { diff --git a/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java b/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java index 407dcc61..bb29264f 100644 --- a/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java +++ b/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java @@ -45,6 +45,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.doNothing; @@ -108,6 +109,34 @@ void testGetApplicationLink() throws URISyntaxException, NoAccessException, NoSu assertEquals(applicationLinkModel, appLinkResponse); } + @Test + void testSetApplicationLinksNullModelMissingLinkSkipsEntry() { + doReturn(Collections.emptyList()).when(mutatingApplicationLinkService).getApplicationLinks(); + final Map applicationLinkModels = Collections.singletonMap("missing-link", null); + assertTrue(applicationLinkService.setApplicationLinks(applicationLinkModels).isEmpty()); + } + + @Test + void testSetApplicationLinksNullNameUsesMapKey() + throws URISyntaxException, NoAccessException, NoSuchApplinkException, TypeNotInstalledException { + + final ApplicationLink applicationLink = createApplicationLink(); + final ApplicationLinkModel applicationLinkModel = createApplicationLinkModel(); + final String mapKey = applicationLinkModel.getName(); + applicationLinkModel.setName(null); + final Map applicationLinkModels = Collections.singletonMap(mapKey, applicationLinkModel); + doReturn(Collections.singletonList(applicationLink)).when(mutatingApplicationLinkService).getApplicationLinks(); + doReturn(applicationLink).when(mutatingApplicationLinkService).getApplicationLink(any()); + doReturn(applicationLink).when(mutatingApplicationLinkService).addApplicationLink(any(), any(), any()); + doReturn(new DefaultApplicationType()).when(typeAccessor).getApplicationType(any()); + doReturn(OAuthConfig.createDisabledConfig()).when(applicationLinksAuthConfigHelper).getOutgoingOAuthConfig(any()); + doReturn(OAuthConfig.createDisabledConfig()).when(applicationLinksAuthConfigHelper).getIncomingOAuthConfig(any()); + doReturn(createApplinkStatus(applicationLink, AVAILABLE)).when(applinkStatusService).getApplinkStatus(any()); + + applicationLinkService.setApplicationLinks(applicationLinkModels); + assertEquals(mapKey, applicationLinkModel.getName()); + } + @Test void testSetApplicationLinks() throws URISyntaxException, NoAccessException, NoSuchApplinkException, TypeNotInstalledException { diff --git a/confluence/Models/UserModel.md b/confluence/Models/UserModel.md index 45c99fb3..46ac3215 100644 --- a/confluence/Models/UserModel.md +++ b/confluence/Models/UserModel.md @@ -10,7 +10,7 @@ | **email** | **String** | | [optional] [default to null] | | **active** | **Boolean** | | [optional] [default to null] | | **password** | **String** | | [optional] [default to null] | -| **groups** | [**Map**](GroupModel.md) | | [optional] [default to null] | +| **groups** | **Map** | | [optional] [default to null] | [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/crowd/Models/UserModel.md b/crowd/Models/UserModel.md index 45c99fb3..46ac3215 100644 --- a/crowd/Models/UserModel.md +++ b/crowd/Models/UserModel.md @@ -10,7 +10,7 @@ | **email** | **String** | | [optional] [default to null] | | **active** | **Boolean** | | [optional] [default to null] | | **password** | **String** | | [optional] [default to null] | -| **groups** | [**Map**](GroupModel.md) | | [optional] [default to null] | +| **groups** | **Map** | | [optional] [default to null] | [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/config/ServiceConfig.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/config/ServiceConfig.java index b2fda085..8528fb6b 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/config/ServiceConfig.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/config/ServiceConfig.java @@ -93,8 +93,7 @@ public TrustedProxiesService trustedProxiesService() { public UsersService usersService() { return new UsersServiceImpl( atlassianConfig.crowdService(), - atlassianConfig.directoryManager(), - groupsService()); + atlassianConfig.directoryManager()); } } diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java index 44307f76..0e78c05a 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java @@ -64,10 +64,27 @@ public Map setApplications( final Map resultApplicationModels = new LinkedHashMap<>(); for (Map.Entry applicationModelEntry : applicationModels.entrySet()) { + final String applicationName = applicationModelEntry.getKey(); final ApplicationModel applicationModel = applicationModelEntry.getValue(); + if (applicationModel == null) { + // declarative no-op: null model + existing entity → return as-is; + // null model + missing entity → nothing to do + try { + final Application existingApplication = applicationManager.findByName(applicationName); + resultApplicationModels.put(applicationName, getApplication(existingApplication.getId())); + } catch (ApplicationNotFoundException ignored) { + // nothing to do + } + continue; + } + + if (applicationModel.getName() == null) { + applicationModel.setName(applicationName); + } + try { - final Application application = applicationManager.findByName(applicationModelEntry.getKey()); + final Application application = applicationManager.findByName(applicationName); final ApplicationModel updatedApplicationModel = setApplication(application.getId(), applicationModel); resultApplicationModels.put(updatedApplicationModel.getName(), updatedApplicationModel); } catch (ApplicationNotFoundException ignored) { diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java index 2212aff7..0a77f47d 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java @@ -49,17 +49,35 @@ public GroupModel createGroup( final long directoryId, final GroupModel groupModel) { - if (groupModel.getName() == null) { + if (groupModel == null) { + throw new BadRequestException("Cannot create group, a full group model is required"); + } + + return createGroup(directoryId, groupModel.getName(), groupModel); + } + + GroupModel createGroup( + final long directoryId, + final String groupName, + final GroupModel groupModel) { + + final String effectiveGroupName = groupModel.getName() != null ? groupModel.getName() : groupName; + + if (effectiveGroupName == null) { throw new BadRequestException("Cannot create group, group name is required"); } - final Group existingGroup = findGroup(directoryId, groupModel.getName()); + if (groupName != null && !effectiveGroupName.equals(groupName)) { + throw new BadRequestException("Cannot create group, two different group names provided"); + } + + final Group existingGroup = findGroup(directoryId, effectiveGroupName); if (existingGroup != null) { - throw new BadRequestException(String.format("Group '%s' already exists", groupModel.getName())); + throw new BadRequestException(String.format("Group '%s' already exists", effectiveGroupName)); } - final GroupTemplate groupTemplate = new GroupTemplate(groupModel.getName(), directoryId); + final GroupTemplate groupTemplate = new GroupTemplate(effectiveGroupName, directoryId); groupTemplate.setDescription(groupModel.getDescription()); groupTemplate.setActive(groupModel.getActive() == null || groupModel.getActive()); @@ -67,10 +85,10 @@ public GroupModel createGroup( return GroupModelUtil.toGroupModel(directoryManager.addGroup(directoryId, groupTemplate)); } catch (DirectoryPermissionException | InvalidGroupException e) { // A permission exception should only happen if we try adding the group - // a user in a read-only directory, so treat this as a bad request + // in a read-only directory, so treat this as a bad request throw new BadRequestException(e); } catch (com.atlassian.crowd.exception.DirectoryNotFoundException | OperationFailedException e) { - // At this point, we know the group exists, thus directory not found + // At this point, we know the directory exists, thus directory not found // should never happen, so if it does, treat it as an internal server error throw new InternalServerErrorException(e); } @@ -109,9 +127,10 @@ public GroupModel setGroup( if (group == null) { if (groupModel == null) { - throw new GroupNotFoundException(groupName); + // declarative no-op: null model + missing entity → nothing to do + return null; } - return createGroup(directoryId, groupModel); + return createGroup(directoryId, groupName, groupModel); } if (groupModel == null) { @@ -132,8 +151,17 @@ public Map setGroups( final Map resultGroupModels = new LinkedHashMap<>(); for (Map.Entry entry : groupModels.entrySet()) { - final GroupModel resultGroupModel = setGroup(directoryId, entry.getKey(), entry.getValue()); - resultGroupModels.put(resultGroupModel.getName(), resultGroupModel); + final String groupName = entry.getKey(); + final GroupModel groupModel = entry.getValue(); + + if (groupModel != null && groupModel.getName() == null) { + groupModel.setName(groupName); + } + + final GroupModel resultGroupModel = setGroup(directoryId, groupName, groupModel); + if (resultGroupModel != null) { + resultGroupModels.put(resultGroupModel.getName(), resultGroupModel); + } } return resultGroupModels; } diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceImpl.java index db0f8492..1090c83d 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceImpl.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceImpl.java @@ -16,18 +16,15 @@ import com.deftdevs.bootstrapi.commons.exception.UserNotFoundException; import com.deftdevs.bootstrapi.commons.exception.web.BadRequestException; import com.deftdevs.bootstrapi.commons.exception.web.InternalServerErrorException; -import com.deftdevs.bootstrapi.commons.model.GroupModel; import com.deftdevs.bootstrapi.commons.model.UserModel; import com.deftdevs.bootstrapi.commons.service.api.UsersService; import com.deftdevs.bootstrapi.crowd.model.util.UserModelUtil; -import com.deftdevs.bootstrapi.crowd.service.api.GroupsService; import javax.annotation.Nullable; import javax.validation.constraints.NotNull; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.*; -import java.util.function.Function; import java.util.stream.Collectors; import static com.atlassian.crowd.model.user.UserConstants.*; @@ -36,16 +33,13 @@ public class UsersServiceImpl implements UsersService { private final CrowdService crowdService; private final DirectoryManager directoryManager; - private final GroupsService groupsService; public UsersServiceImpl( final CrowdService crowdService, - final DirectoryManager directoryManager, - final GroupsService groupsService) { + final DirectoryManager directoryManager) { this.crowdService = crowdService; this.directoryManager = directoryManager; - this.groupsService = groupsService; } @Override @@ -82,7 +76,8 @@ UserModel setUser( if (user == null) { if (userModel == null) { - throw new UserNotFoundException(username); + // declarative no-op: null model + missing entity → nothing to do + return null; } return addUser(directoryId, username, userModel); } @@ -106,7 +101,9 @@ public Map setUsers( final Map resultUserModels = new LinkedHashMap<>(); for (Map.Entry entry : userModels.entrySet()) { final UserModel resultUserModel = setUser(directoryId, entry.getKey(), entry.getValue()); - resultUserModels.put(resultUserModel.getUsername(), resultUserModel); + if (resultUserModel != null) { + resultUserModels.put(resultUserModel.getUsername(), resultUserModel); + } } return resultUserModels; } @@ -158,8 +155,8 @@ UserModel addUser( final UserModel resultUserModel = UserModelUtil.toUserModel(user); if (userModel.getGroups() != null) { - final Map resultGroupModels = addUserToGroups(directoryId, effectiveUsername, userModel.getGroups()); - resultUserModel.setGroups(resultGroupModels); + final Map resultGroups = setUserGroups(directoryId, effectiveUsername, userModel.getGroups()); + resultUserModel.setGroups(resultGroups); } return resultUserModel; @@ -191,8 +188,8 @@ UserModel updateUser( final UserModel resultUserModel = UserModelUtil.toUserModel(user); if (userModel.getGroups() != null) { - final Map resultGroupModels = addUserToGroups(directoryId, user.getName(), userModel.getGroups()); - resultUserModel.setGroups(resultGroupModels); + final Map resultGroups = setUserGroups(directoryId, user.getName(), userModel.getGroups()); + resultUserModel.setGroups(resultGroups); } return resultUserModel; @@ -413,33 +410,52 @@ void resetUserPasswordAttributes( } } - Map addUserToGroups( + Map setUserGroups( final long directoryId, final String username, - final Map groupModels) { - - final List resultGroupModels = new ArrayList<>(); - - if (groupModels != null) { - for (Map.Entry groupModelEntry : groupModels.entrySet()) { - final GroupModel resultGroupModel = groupsService.setGroup(directoryId, groupModelEntry.getKey(), groupModelEntry.getValue()); - - try { - directoryManager.addUserToGroup(directoryId, username, resultGroupModel.getName()); - resultGroupModels.add(resultGroupModel); - } catch (DirectoryPermissionException | ReadOnlyGroupException e) { - throw new BadRequestException(e); - } catch (com.atlassian.crowd.exception.DirectoryNotFoundException | - com.atlassian.crowd.exception.UserNotFoundException | GroupNotFoundException | - OperationFailedException e) { - throw new InternalServerErrorException(e); - } catch (MembershipAlreadyExistsException e) { - resultGroupModels.add(resultGroupModel); + final Map groups) { + + final Map resultGroups = new LinkedHashMap<>(); + + if (groups == null) { + return resultGroups; + } + + for (Map.Entry entry : groups.entrySet()) { + final String groupName = entry.getKey(); + final Boolean assignUserToGroup = entry.getValue(); + + // null = no-op (declarative merge: don't touch this membership) + if (assignUserToGroup == null) { + continue; + } + + try { + if (assignUserToGroup) { + try { + directoryManager.addUserToGroup(directoryId, username, groupName); + } catch (MembershipAlreadyExistsException ignored) {} + + // if add successful or already a member, return that + resultGroups.put(groupName, true); + } else { + try { + directoryManager.removeUserFromGroup(directoryId, username, groupName); + } catch (MembershipNotFoundException ignored) {} + + // if remove successful or already not a member, return that + resultGroups.put(groupName, false); } + } catch (DirectoryPermissionException | ReadOnlyGroupException e) { + throw new BadRequestException(e); + } catch (com.atlassian.crowd.exception.DirectoryNotFoundException | + com.atlassian.crowd.exception.UserNotFoundException | GroupNotFoundException | + OperationFailedException e) { + throw new InternalServerErrorException(e); } } - return resultGroupModels.stream().collect(Collectors.toMap(GroupModel::getName, Function.identity())); + return resultGroups; } private static UserTemplate getUserTemplate( diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java index 3f57b836..2390599d 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java @@ -39,6 +39,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyLong; import static org.mockito.Mockito.doReturn; @@ -197,6 +198,42 @@ public void testSetApplicationEnsurePersistenceCalls() throws ApplicationNotFoun verify(spy).persistApplicationModelAutoAssignmentGroups(any(), any()); } + @Test + public void testSetApplicationsNullModelMissingApplicationSkipsEntry() throws ApplicationNotFoundException { + doThrow(new ApplicationNotFoundException("")).when(applicationManager).findByName("missing-app"); + final Map applicationModels = Collections.singletonMap("missing-app", null); + assertTrue(applicationsService.setApplications(applicationModels).isEmpty()); + } + + @Test + public void testSetApplicationsNullModelExistingApplicationReturnedAsIs() throws ApplicationNotFoundException { + final Application application = toApplication(EXAMPLE_1); + doReturn(application).when(applicationManager).findByName(EXAMPLE_1.getName()); + doReturn(application).when(applicationManager).findById(application.getId()); + + final Map applicationModels = Collections.singletonMap(EXAMPLE_1.getName(), null); + final Map result = applicationsService.setApplications(applicationModels); + assertEquals(applicationModels.size(), result.size()); + assertEquals(EXAMPLE_1.getName(), result.get(EXAMPLE_1.getName()).getName()); + } + + @Test + public void testSetApplicationsNullNameUsesMapKey() throws ApplicationNotFoundException { + final String mapKey = EXAMPLE_2.getName(); + final ApplicationModel modelWithoutName = ApplicationModel.builder() + .type(EXAMPLE_2.getType()) + .build(); + final Map applicationModels = Collections.singletonMap(mapKey, modelWithoutName); + doThrow(new ApplicationNotFoundException("")).when(applicationManager).findByName(mapKey); + + final ApplicationsServiceImpl spy = spy(applicationsService); + doReturn(EXAMPLE_2).when(spy).addApplication(modelWithoutName); + + spy.setApplications(applicationModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(spy).addApplication(modelWithoutName); + } + @Test public void testSetApplications() throws ApplicationNotFoundException { final Application applicationExample1 = ImmutableApplication.builder(toApplication(EXAMPLE_1)) diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java index 94fa7f75..4fcf66ca 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java @@ -82,6 +82,54 @@ public void testSetDirectoriesAddNew() { verify(spy).addDirectory(directoryModel); } + @Test + public void testSetDirectoriesNullModelMissingDirectorySkipsEntry() { + final Directory directoryInternal = getTestDirectoryInternal(); + final DirectoriesServiceImpl spy = spy(directoriesService); + doReturn(Collections.singletonList(directoryInternal)).when(spy).findAllDirectories(); + + final Map directoryModels = new LinkedHashMap<>(); + directoryModels.put("missing-directory", null); + assertTrue(spy.setDirectories(directoryModels).isEmpty()); + verify(spy, never()).addDirectory(any()); + verify(spy, never()).setDirectory(anyLong(), any()); + } + + @Test + public void testSetDirectoriesNullModelExistingDirectoryReturnedAsIs() { + final Directory directoryInternal = getTestDirectoryInternal(); + final DirectoriesServiceImpl spy = spy(directoriesService); + final List directories = Collections.singletonList(directoryInternal); + doReturn(directories).when(spy).findAllDirectories(); + + final Map directoryModels = new LinkedHashMap<>(); + directoryModels.put(directoryInternal.getName(), null); + + final Map result = spy.setDirectories(directoryModels); + assertEquals(directories.size(), result.size()); + assertEquals(directoryInternal.getName(), result.values().iterator().next().getName()); + verify(spy, never()).addDirectory(any()); + verify(spy, never()).setDirectory(anyLong(), any()); + } + + @Test + public void testSetDirectoriesNullNameUsesMapKey() { + final Directory directoryInternal = getTestDirectoryInternal(); + final Directory directoryInternalNew = getTestDirectoryInternalOther(); + final DirectoriesServiceImpl spy = spy(directoriesService); + doReturn(Collections.singletonList(directoryInternal)).when(spy).findAllDirectories(); + + final AbstractDirectoryModel directoryModel = DirectoryModelUtil.toDirectoryModel(directoryInternalNew); + final String mapKey = directoryModel.getName(); + directoryModel.setName(null); + final Map directoryModels = Collections.singletonMap(mapKey, directoryModel); + doReturn(directoryModel).when(spy).addDirectory(any()); + + spy.setDirectories(directoryModels); + assertEquals(mapKey, directoryModel.getName()); + verify(spy).addDirectory(directoryModel); + } + @Test public void testSetDirectoriesAddNewUnsupportedType() { final Directory directoryInternal = getTestDirectoryInternal(); diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java index dfddafb7..77bb9a94 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java @@ -107,6 +107,13 @@ public void testCreateGroupNoName() throws DirectoryNotFoundException, Operation }); } + @Test + public void testCreateGroupNullModel() { + assertThrows(BadRequestException.class, () -> { + groupsService.createGroup(0L, null); + }); + } + @Test public void testUpdateGroup() throws Exception { final Group group = getTestGroup(); @@ -157,10 +164,10 @@ public void testSetGroupAddNew() { final Group group = getTestGroup(); final GroupModel groupModel = GroupModel.EXAMPLE_1; final GroupsServiceImpl spy = spy(groupsService); - doReturn(groupModel).when(spy).createGroup(anyLong(), any()); + doReturn(groupModel).when(spy).createGroup(anyLong(), anyString(), any()); spy.setGroup(group.getDirectoryId(), groupModel.getName(), groupModel); - verify(spy).createGroup(anyLong(), any()); + verify(spy).createGroup(anyLong(), anyString(), any()); } @Test @@ -191,10 +198,33 @@ public void testSetGroups() { } @Test - public void testSetGroupNullModelAddNew() { + public void testSetGroupsNullModelMissingGroupSkipsEntry() { + final Map groupModels = new LinkedHashMap<>(); + groupModels.put("missing-group", null); + assertTrue(groupsService.setGroups(0L, groupModels).isEmpty()); + } + + @Test + public void testSetGroupsNullNameUsesMapKey() { + final String mapKey = GroupModel.EXAMPLE_1.getName(); + final GroupModel modelWithoutName = GroupModel.builder() + .active(GroupModel.EXAMPLE_1.getActive()) + .description(GroupModel.EXAMPLE_1.getDescription()) + .build(); + final Map groupModels = Collections.singletonMap(mapKey, modelWithoutName); + + final GroupsServiceImpl spy = spy(groupsService); + doReturn(modelWithoutName).when(spy).setGroup(anyLong(), anyString(), any()); + + spy.setGroups(0L, groupModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(spy).setGroup(anyLong(), eq(mapKey), eq(modelWithoutName)); + } + + @Test + public void testSetGroupNullModelMissingGroupIsNoOp() { final Group group = getTestGroup(); - assertThrows(GroupNotFoundException.class, () -> - groupsService.setGroup(group.getDirectoryId(), group.getName(), null)); + assertNull(groupsService.setGroup(group.getDirectoryId(), group.getName(), null)); } @Test diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceTest.java index 1794f4ee..40056379 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceTest.java @@ -12,11 +12,9 @@ import com.atlassian.crowd.model.user.UserTemplate; import com.atlassian.crowd.model.user.UserTemplateWithAttributes; import com.deftdevs.bootstrapi.commons.exception.web.BadRequestException; -import com.deftdevs.bootstrapi.commons.model.GroupModel; import com.deftdevs.bootstrapi.commons.model.UserModel; import com.deftdevs.bootstrapi.commons.exception.UserNotFoundException; import com.deftdevs.bootstrapi.crowd.model.util.UserModelUtil; -import com.deftdevs.bootstrapi.crowd.service.api.GroupsService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -44,14 +42,11 @@ public class UsersServiceTest { @Mock private DirectoryManager directoryManager; - @Mock - private GroupsService groupsService; - private UsersServiceImpl usersService; @BeforeEach public void setup() { - usersService = new UsersServiceImpl(crowdService, directoryManager, groupsService); + usersService = new UsersServiceImpl(crowdService, directoryManager); setupDirectoryManager(); } @@ -143,10 +138,16 @@ public void testSetUserWithMapKeyUsernameWhenModelUsernameIsNull() { } @Test - public void testSetUserNullModelAddNew() { + public void testSetUserNullModelMissingUserIsNoOp() { final User user = getTestUser(); - assertThrows(UserNotFoundException.class, () -> - usersService.setUser(user.getDirectoryId(), user.getName(), null)); + assertNull(usersService.setUser(user.getDirectoryId(), user.getName(), null)); + } + + @Test + public void testSetUsersWithNullModelMissingUserSkipsEntry() { + final Map userModels = new LinkedHashMap<>(); + userModels.put("missing-user", null); + assertTrue(usersService.setUsers(getTestDirectory().getId(), userModels).isEmpty()); } @Test @@ -228,33 +229,58 @@ public void testAddUserActiveByDefault() throws CrowdException, DirectoryPermiss public void testAddUserWithGroups() throws CrowdException, DirectoryPermissionException { // return the same user as the one we are adding doAnswer(invocation -> invocation.getArguments()[1]).when(directoryManager).addUser(anyLong(), any(), any()); - doAnswer(invocation -> invocation.getArguments()[2]).when(groupsService).setGroup(anyLong(), anyString(), any()); final UserModel userModel = UserModelUtil.toUserModel(getTestUser()); - final Map groupModels = Stream.of(GroupModel.EXAMPLE_1).collect(Collectors.toMap(GroupModel::getName, Function.identity())); + final Map groups = new LinkedHashMap<>(); + groups.put("group-to-add", true); + groups.put("group-to-remove", false); + groups.put("group-no-op", null); userModel.setPassword("12345"); - userModel.setGroups(groupModels); + userModel.setGroups(groups); - usersService.addUser(1L, userModel); - verify(groupsService, times(groupModels.size())).setGroup(anyLong(), anyString(), any()); + final UserModel result = usersService.addUser(1L, userModel); + verify(directoryManager).addUserToGroup(anyLong(), anyString(), eq("group-to-add")); + verify(directoryManager).removeUserFromGroup(anyLong(), anyString(), eq("group-to-remove")); + verify(directoryManager, never()).addUserToGroup(anyLong(), anyString(), eq("group-no-op")); + verify(directoryManager, never()).removeUserFromGroup(anyLong(), anyString(), eq("group-no-op")); + assertNotNull(result.getGroups()); + assertEquals(2, result.getGroups().size()); + assertEquals(true, result.getGroups().get("group-to-add")); + assertEquals(false, result.getGroups().get("group-to-remove")); } @Test public void testAddUserWithGroupsAlreadyMember() throws CrowdException, DirectoryPermissionException { // return the same user as the one we are adding doAnswer(invocation -> invocation.getArguments()[1]).when(directoryManager).addUser(anyLong(), any(), any()); - doAnswer(invocation -> invocation.getArguments()[2]).when(groupsService).setGroup(anyLong(), anyString(), any()); - doThrow(new MembershipAlreadyExistsException(1L, "test", GroupModel.EXAMPLE_1.getName())) + doThrow(new MembershipAlreadyExistsException(1L, "test", "some-group")) .when(directoryManager).addUserToGroup(anyLong(), anyString(), anyString()); final UserModel userModel = UserModelUtil.toUserModel(getTestUser()); - final Map groupModels = Stream.of(GroupModel.EXAMPLE_1).collect(Collectors.toMap(GroupModel::getName, Function.identity())); userModel.setPassword("12345"); - userModel.setGroups(groupModels); + userModel.setGroups(Collections.singletonMap("some-group", true)); + + final UserModel result = usersService.addUser(1L, userModel); + assertNotNull(result.getGroups()); + assertEquals(1, result.getGroups().size()); + assertEquals(true, result.getGroups().get("some-group")); + } + + @Test + public void testAddUserWithGroupsRemoveNotMember() throws CrowdException, DirectoryPermissionException { + // return the same user as the one we are adding + doAnswer(invocation -> invocation.getArguments()[1]).when(directoryManager).addUser(anyLong(), any(), any()); + doThrow(new MembershipNotFoundException("test", "some-group")) + .when(directoryManager).removeUserFromGroup(anyLong(), anyString(), anyString()); + + final UserModel userModel = UserModelUtil.toUserModel(getTestUser()); + userModel.setPassword("12345"); + userModel.setGroups(Collections.singletonMap("some-group", false)); final UserModel result = usersService.addUser(1L, userModel); assertNotNull(result.getGroups()); - assertEquals(groupModels.size(), result.getGroups().size()); + assertEquals(1, result.getGroups().size()); + assertEquals(false, result.getGroups().get("some-group")); } @Test @@ -345,15 +371,12 @@ public void updateUserWithGroups() throws CrowdException, DirectoryPermissionExc doReturn(user).when(directoryManager).findUserByName(user.getDirectoryId(), user.getName()); // return the same user as the one we are updating doAnswer(invocation -> invocation.getArguments()[1]).when(directoryManager).updateUser(anyLong(), any()); - doAnswer(invocation -> invocation.getArguments()[2]).when(groupsService).setGroup(anyLong(), anyString(), any()); final UserModel userModel = UserModelUtil.toUserModel(getTestUser()); - final Map groupModels = Stream.of(GroupModel.EXAMPLE_1) - .collect(Collectors.toMap(GroupModel::getName, Function.identity())); - userModel.setGroups(groupModels); + userModel.setGroups(Collections.singletonMap("group-to-add", true)); usersService.updateUser(1L, user.getName(), userModel); - verify(groupsService, times(groupModels.size())).setGroup(anyLong(), anyString(), any()); + verify(directoryManager).addUserToGroup(anyLong(), eq(user.getName()), eq("group-to-add")); } @Test @@ -362,15 +385,14 @@ public void updateUserWithGroupsAndNoUsernameInModel() throws CrowdException, Di doReturn(user).when(directoryManager).findUserByName(user.getDirectoryId(), user.getName()); // return the same user as the one we are updating doAnswer(invocation -> invocation.getArguments()[1]).when(directoryManager).updateUser(anyLong(), any()); - doAnswer(invocation -> invocation.getArguments()[2]).when(groupsService).setGroup(anyLong(), anyString(), any()); final UserModel userModel = UserModel.builder() .fullName("Other Full Name") - .groups(Stream.of(GroupModel.EXAMPLE_1).collect(Collectors.toMap(GroupModel::getName, Function.identity()))) + .groups(Collections.singletonMap("group-to-add", true)) .build(); usersService.updateUser(1L, user.getName(), userModel); - verify(directoryManager).addUserToGroup(anyLong(), eq(user.getName()), anyString()); + verify(directoryManager).addUserToGroup(anyLong(), eq(user.getName()), eq("group-to-add")); } @Test diff --git a/jira/Models/UserModel.md b/jira/Models/UserModel.md index 45c99fb3..46ac3215 100644 --- a/jira/Models/UserModel.md +++ b/jira/Models/UserModel.md @@ -10,7 +10,7 @@ | **email** | **String** | | [optional] [default to null] | | **active** | **Boolean** | | [optional] [default to null] | | **password** | **String** | | [optional] [default to null] | -| **groups** | [**Map**](GroupModel.md) | | [optional] [default to null] | +| **groups** | **Map** | | [optional] [default to null] | [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)