From f0514d271f5a36463f7d1cd58502a63c6c6bbfa1 Mon Sep 17 00:00:00 2001 From: Patrick Hobusch Date: Wed, 13 May 2026 16:11:59 +0800 Subject: [PATCH] Map GroupNotFoundException to 404 in setUserGroups and document groups semantics Under the new boolean-map membership model, group lifecycle is managed via the top-level groups map, so a non-existent group referenced from UserModel.groups is a client error (404), not an internal server error (500). Translate the Atlassian GroupNotFoundException thrown by addUserToGroup/removeUserFromGroup into the project's GroupNotFoundException. Add a @Schema description to UserModel.groups so the auto-generated REST API docs explain the true/false/null semantics and the requirement that referenced groups must already exist. --- .../bootstrapi/commons/model/UserModel.java | 4 ++++ confluence/Models/UserModel.md | 2 +- crowd/Models/UserModel.md | 2 +- .../crowd/service/UsersServiceImpl.java | 6 +++++- .../crowd/service/UsersServiceTest.java | 15 +++++++++++++++ jira/Models/UserModel.md | 2 +- 6 files changed, 27 insertions(+), 4 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 e763ed6e..f914217e 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 @@ -1,6 +1,7 @@ package com.deftdevs.bootstrapi.commons.model; import com.deftdevs.bootstrapi.commons.constants.BootstrAPI; +import io.swagger.v3.oas.annotations.media.Schema; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -44,6 +45,9 @@ public class UserModel { private String password; @XmlElement + @Schema(description = "Group memberships keyed by group name. true ensures the user is a member, " + + "false ensures the user is not a member, null is a no-op (leaves the membership untouched). " + + "Group lifecycle is managed via the top-level groups map; referenced groups must already exist.") private Map groups; // Example instances for documentation and tests diff --git a/confluence/Models/UserModel.md b/confluence/Models/UserModel.md index 46ac3215..20bf0c67 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** | | [optional] [default to null] | +| **groups** | **Map** | Group memberships keyed by group name. true ensures the user is a member, false ensures the user is not a member, null is a no-op (leaves the membership untouched). Group lifecycle is managed via the top-level groups map; referenced groups must already exist. | [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 46ac3215..20bf0c67 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** | | [optional] [default to null] | +| **groups** | **Map** | Group memberships keyed by group name. true ensures the user is a member, false ensures the user is not a member, null is a no-op (leaves the membership untouched). Group lifecycle is managed via the top-level groups map; referenced groups must already exist. | [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/service/UsersServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/UsersServiceImpl.java index 1090c83d..b37610e9 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 @@ -448,8 +448,12 @@ Map setUserGroups( } } catch (DirectoryPermissionException | ReadOnlyGroupException e) { throw new BadRequestException(e); + } catch (GroupNotFoundException e) { + // the referenced group does not exist - group lifecycle is managed via the top-level + // groups map, so a non-existent group here is a client error (404) + throw new com.deftdevs.bootstrapi.commons.exception.GroupNotFoundException(groupName); } catch (com.atlassian.crowd.exception.DirectoryNotFoundException | - com.atlassian.crowd.exception.UserNotFoundException | GroupNotFoundException | + com.atlassian.crowd.exception.UserNotFoundException | OperationFailedException e) { throw new InternalServerErrorException(e); } 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 40056379..e4ba6a61 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 @@ -283,6 +283,21 @@ public void testAddUserWithGroupsRemoveNotMember() throws CrowdException, Direct assertEquals(false, result.getGroups().get("some-group")); } + @Test + public void testAddUserWithGroupsUnknownGroupThrowsNotFound() 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 GroupNotFoundException("unknown-group")) + .when(directoryManager).addUserToGroup(anyLong(), anyString(), anyString()); + + final UserModel userModel = UserModelUtil.toUserModel(getTestUser()); + userModel.setPassword("12345"); + userModel.setGroups(Collections.singletonMap("unknown-group", true)); + + assertThrows(com.deftdevs.bootstrapi.commons.exception.GroupNotFoundException.class, + () -> usersService.addUser(1L, userModel)); + } + @Test public void testAddUserAlreadyExists() throws CrowdException { final User user = getTestUser(); diff --git a/jira/Models/UserModel.md b/jira/Models/UserModel.md index 46ac3215..20bf0c67 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** | | [optional] [default to null] | +| **groups** | **Map** | Group memberships keyed by group name. true ensures the user is a member, false ensures the user is not a member, null is a no-op (leaves the membership untouched). Group lifecycle is managed via the top-level groups map; referenced groups must already exist. | [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)