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)