diff --git a/src/main/helm/templates/network_policy.yaml b/src/main/helm/templates/network_policy.yaml index 1c8c51e0be101348d8e999960f0e0e98124d85c8..a86717297700f48a3f27349e17b24fbb4638fa8d 100644 --- a/src/main/helm/templates/network_policy.yaml +++ b/src/main/helm/templates/network_policy.yaml @@ -28,6 +28,13 @@ spec: {{ toYaml . | indent 2 }} {{- end }} egress: + - to: + - podSelector: + matchLabels: + component: zufi-server + ports: + - port: 9090 + protocol: TCP - to: - podSelector: matchLabels: diff --git a/src/main/helm/templates/service_account.yaml b/src/main/helm/templates/service_account.yaml index 0e13e6bcabf1933117c29487473453b63265922a..3bac8e223d1fd108b386d1f06ed4e9fb2284a67c 100644 --- a/src/main/helm/templates/service_account.yaml +++ b/src/main/helm/templates/service_account.yaml @@ -1,5 +1,5 @@ # -# Copyright (C) 2022 Das Land Schleswig-Holstein vertreten durch den +# Copyright (C) 2024 Das Land Schleswig-Holstein vertreten durch den # Ministerpräsidenten des Landes Schleswig-Holstein # Staatskanzlei # Abteilung Digitalisierung und zentrales IT-Management der Landesregierung diff --git a/src/main/java/de/ozgcloud/admin/keycloak/AddGroupData.java b/src/main/java/de/ozgcloud/admin/keycloak/AddGroupData.java index 2845e10babe0bed5eaf22a1b5d5bb043d0532a6b..bfef1afa5a70b06618372fcb6e7c1eb035f3e872 100644 --- a/src/main/java/de/ozgcloud/admin/keycloak/AddGroupData.java +++ b/src/main/java/de/ozgcloud/admin/keycloak/AddGroupData.java @@ -9,7 +9,6 @@ import lombok.ToString; @ToString public class AddGroupData { - private String parentId; private String name; private String organisationsEinheitId; } diff --git a/src/main/java/de/ozgcloud/admin/keycloak/GroupMapper.java b/src/main/java/de/ozgcloud/admin/keycloak/GroupMapper.java index 4af927fa8481a073970825559f759b9234b72c90..a18a572526e3e4da1ca1bbc66433fe17266acedc 100644 --- a/src/main/java/de/ozgcloud/admin/keycloak/GroupMapper.java +++ b/src/main/java/de/ozgcloud/admin/keycloak/GroupMapper.java @@ -23,15 +23,6 @@ abstract class GroupMapper { public abstract List<Group> fromGroupRepresentations(List<GroupRepresentation> groupRepresentations); - @AfterMapping - protected void deleteGroupsWithoutOrganisationsEinheitId(@MappingTarget List<Group> groups) { - groups.removeIf(this::isMissingOrganisationsEinheitId); - } - - protected boolean isMissingOrganisationsEinheitId(Group group) { - return StringUtils.isBlank(group.getOrganisationsEinheitId()); - } - @Mapping(target = "organisationsEinheitId", source = "attributes") public abstract Group fromGroupRepresentation(GroupRepresentation groupRepresentation); @@ -44,15 +35,24 @@ abstract class GroupMapper { return null; } if (values.size() > 1 && values.stream().distinct().count() > 1) { - LOG.warn("Group contains multiple values for {}. The first one is taken.", keycloakApiProperties.getOrganisationsEinheitIdKey()); + throw new GroupRepresentationMappingException("Group contains multiple values for organisationsEinheitId: %s".formatted(values)); } return values.getFirst(); } - @Mapping(target = "attributes", expression = "java(getAttributes(groupToAdd))") + @AfterMapping + protected void deleteGroupsWithoutOrganisationsEinheitId(@MappingTarget List<Group> groups) { + groups.removeIf(this::isMissingOrganisationsEinheitId); + } + + protected boolean isMissingOrganisationsEinheitId(Group group) { + return StringUtils.isBlank(group.getOrganisationsEinheitId()); + } + + @Mapping(target = "attributes", expression = "java(buildGroupAttributes(groupToAdd))") public abstract GroupRepresentation toGroupRepresentation(AddGroupData groupToAdd); - Map<String, List<String>> getAttributes(AddGroupData groupToAdd) { + Map<String, List<String>> buildGroupAttributes(AddGroupData groupToAdd) { var organisationsEinheitId = groupToAdd.getOrganisationsEinheitId(); return StringUtils.isNotBlank(organisationsEinheitId) ? Map.of(keycloakApiProperties.getOrganisationsEinheitIdKey(), List.of(organisationsEinheitId)) : diff --git a/src/main/java/de/ozgcloud/admin/keycloak/GroupRepresentationMappingException.java b/src/main/java/de/ozgcloud/admin/keycloak/GroupRepresentationMappingException.java new file mode 100644 index 0000000000000000000000000000000000000000..c07743ddf481a64e3d296309fbaa556fbab31f14 --- /dev/null +++ b/src/main/java/de/ozgcloud/admin/keycloak/GroupRepresentationMappingException.java @@ -0,0 +1,8 @@ +package de.ozgcloud.admin.keycloak; + +public class GroupRepresentationMappingException extends RuntimeException { + + public GroupRepresentationMappingException(String message) { + super(message); + } +} diff --git a/src/main/java/de/ozgcloud/admin/keycloak/KeycloakApiService.java b/src/main/java/de/ozgcloud/admin/keycloak/KeycloakApiService.java index 09b865543a58b34c7a89233f6f75ee0253bc8d6c..888f98c8b9acbecc3eaecf16cfbacd9dca176587 100644 --- a/src/main/java/de/ozgcloud/admin/keycloak/KeycloakApiService.java +++ b/src/main/java/de/ozgcloud/admin/keycloak/KeycloakApiService.java @@ -20,18 +20,12 @@ class KeycloakApiService { } public String addGroup(GroupRepresentation groupRepresentation) { - try (var response = addParentOrChildGroup(groupRepresentation)) { - return getAddedResourceIdFromResponse(response); + try (var response = groupsResource.add(groupRepresentation)) { + return getCreatedResourceIdFromResponse(response); } } - Response addParentOrChildGroup(GroupRepresentation groupRepresentation) { - return groupRepresentation.getParentId() == null ? - groupsResource.add(groupRepresentation) : - groupsResource.group(groupRepresentation.getParentId()).subGroup(groupRepresentation); - } - - String getAddedResourceIdFromResponse(Response response) { + String getCreatedResourceIdFromResponse(Response response) { if (response.getStatus() == Response.Status.CREATED.getStatusCode()) { return extractResourceIdFromLocationHeader(response.getHeaderString("Location")); } diff --git a/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapper.java b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapper.java index 193a6f05d8410e8733aad684fc8a4fb6761825f5..da1634ec80bf9a5169022184c50bba014dd735a4 100644 --- a/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapper.java +++ b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapper.java @@ -13,6 +13,5 @@ interface OrganisationsEinheitMapper { @Mapping(target = "zufiId", source = "id") OrganisationsEinheit fromGrpc(GrpcOrganisationsEinheit grpcOrganisationsEinheit); - @Mapping(target = "parentId", source = "parentKeycloakId") - AddGroupData toAddGroupData(OrganisationsEinheit organisationsEinheit, String parentKeycloakId); + AddGroupData toAddGroupData(OrganisationsEinheit organisationsEinheit); } diff --git a/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitRepository.java b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitRepository.java index 3f8e3c38963297d397cb05542479f4a8a1d99984..f32cbadfe751ab671151de99142112f22ff02671 100644 --- a/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitRepository.java +++ b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitRepository.java @@ -25,6 +25,6 @@ interface OrganisationsEinheitRepository extends MongoRepository<OrganisationsEi @Update("{'$set': {'syncResult': 'DELETED'}}") void setUnsyncedAsDeleted(long lastSyncTimestamp); - @Query("{'syncResult': { $eq: null }}") + @Query("{'syncResult': null }") Stream<OrganisationsEinheit> findAllWithoutSyncResult(); } diff --git a/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitSynchronizationException.java b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitSynchronizationException.java new file mode 100644 index 0000000000000000000000000000000000000000..42944f0a41b1d5eb7cd1a724ab8e1792bc7da7de --- /dev/null +++ b/src/main/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitSynchronizationException.java @@ -0,0 +1,12 @@ +package de.ozgcloud.admin.organisationseinheit; + +public class OrganisationsEinheitSynchronizationException extends RuntimeException { + + public OrganisationsEinheitSynchronizationException(String message) { + super(message); + } + + public OrganisationsEinheitSynchronizationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/de/ozgcloud/admin/organisationseinheit/SyncService.java b/src/main/java/de/ozgcloud/admin/organisationseinheit/SyncService.java index 2b3949a5436739f9fe8633a0373a803d060eb06d..a1c911a20d16d2cb4fdbbd707417180521c0bd9f 100644 --- a/src/main/java/de/ozgcloud/admin/organisationseinheit/SyncService.java +++ b/src/main/java/de/ozgcloud/admin/organisationseinheit/SyncService.java @@ -2,7 +2,6 @@ package de.ozgcloud.admin.organisationseinheit; import java.util.List; import java.util.Optional; -import java.util.stream.Stream; import org.springframework.stereotype.Service; @@ -90,50 +89,25 @@ class SyncService { } public void syncAddedOrganisationsEinheiten(long syncTimestamp) { - sortInAdditionOrder(repository.findAllWithoutSyncResult()).forEach( - organisationsEinheit -> syncAddedOrganisationsEinheit(syncTimestamp, organisationsEinheit)); + repository.findAllWithoutSyncResult().forEach( + organisationsEinheit -> syncAddedOrganisationsEinheit(organisationsEinheit, syncTimestamp)); } - Stream<OrganisationsEinheit> sortInAdditionOrder(Stream<OrganisationsEinheit> organisationsEinheiten) { - return organisationsEinheiten.sorted((org1, org2) -> { - if (org1.getParentId() == null && org2.getParentId() == null) { - return 0; - } - if (org1.getParentId() == null) { - return -1; - } - if (org2.getParentId() == null) { - return 1; - } - if (org1.getId().equals(org2.getParentId())) { - return -1; - } - if (org1.getParentId().equals(org2.getId())) { - return 1; - } - return 0; - }); + void syncAddedOrganisationsEinheit(OrganisationsEinheit organisationsEinheit, long syncTimestamp) { + addAsGroupInKeycloak(organisationsEinheit).ifPresent( + addedGroupId -> updateAfterSuccessfulGroupCreation(organisationsEinheit, addedGroupId, syncTimestamp)); } - void syncAddedOrganisationsEinheit(long syncTimestamp, OrganisationsEinheit organisationsEinheit) { - var parent = findParent(organisationsEinheit); - if (parent.isEmpty() || parent.get().getKeycloakId() != null) { - var parentKeycloakId = parent.map(OrganisationsEinheit::getKeycloakId).orElse(null); - addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId).ifPresent( - addedGroupId -> updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, addedGroupId)); + Optional<String> addAsGroupInKeycloak(OrganisationsEinheit organisationsEinheit) { + if (organisationsEinheit.getParentId() != null) { + throw new OrganisationsEinheitSynchronizationException( + "Organisationseinheit %s has parent".formatted(organisationsEinheit.getOrganisationsEinheitId())); } - } - - Optional<OrganisationsEinheit> findParent(OrganisationsEinheit organisationsEinheit) { - return Optional.ofNullable(organisationsEinheit.getParentId()).flatMap(repository::findById); - } - - Optional<String> addAsGroupInKeycloak(OrganisationsEinheit organisationsEinheit, String parentKeycloakId) { - var addGroupData = organisationsEinheitMapper.toAddGroupData(organisationsEinheit, parentKeycloakId); + var addGroupData = organisationsEinheitMapper.toAddGroupData(organisationsEinheit); return addGroupInKeycloak(addGroupData); } - void updateAfterSuccessfulGroupCreation(OrganisationsEinheit organisationsEinheit, long syncTimestamp, String keycloakId) { + void updateAfterSuccessfulGroupCreation(OrganisationsEinheit organisationsEinheit, String keycloakId, long syncTimestamp) { var updatedOrganisationsEinheit = organisationsEinheit.toBuilder() .keycloakId(keycloakId) .syncResult(SyncResult.OK) @@ -146,8 +120,7 @@ class SyncService { try { return Optional.of(keycloakRemoteService.addGroup(addGroupData)); } catch (ResourceCreationException e) { - LOG.error("Error adding group %s in Keycloak".formatted(addGroupData), e); - return Optional.empty(); + throw new OrganisationsEinheitSynchronizationException("Error creating group %s in Keycloak".formatted(addGroupData), e); } } } diff --git a/src/main/resources/application-remotekc.yaml b/src/main/resources/application-remotekc.yaml index a4cd02efb0e33fb73ccaf95e1478973185b12261..e64923222403088b888536380a901d46bb3fc442 100644 --- a/src/main/resources/application-remotekc.yaml +++ b/src/main/resources/application-remotekc.yaml @@ -6,4 +6,3 @@ ozgcloud: keycloak: api: user: administrationApiUser - password: 4hVe-qWRM4ZwMdQ diff --git a/src/test/helm/network_policy_test.yaml b/src/test/helm/network_policy_test.yaml index 427fafd41265488ffe1002a9746373156b7a9718..ccd95833d7420217c7cbefde549ebf7683b6c731 100644 --- a/src/test/helm/network_policy_test.yaml +++ b/src/test/helm/network_policy_test.yaml @@ -83,6 +83,13 @@ tests: - protocol: TCP port: 8081 egress: + - to: + - podSelector: + matchLabels: + component: zufi-server + ports: + - port: 9090 + protocol: TCP - to: - podSelector: matchLabels: diff --git a/src/test/java/de/ozgcloud/admin/keycloak/AddGroupDataTestFactory.java b/src/test/java/de/ozgcloud/admin/keycloak/AddGroupDataTestFactory.java index b53d872617b553a2c708e6b653905231889af445..cea95944ffe15491d5a60ead097db8a8a29c4989 100644 --- a/src/test/java/de/ozgcloud/admin/keycloak/AddGroupDataTestFactory.java +++ b/src/test/java/de/ozgcloud/admin/keycloak/AddGroupDataTestFactory.java @@ -2,7 +2,6 @@ package de.ozgcloud.admin.keycloak; public class AddGroupDataTestFactory { - public static final String PARENT_ID = GroupRepresentationTestFactory.ID; public static final String NAME = GroupRepresentationTestFactory.NAME; public static final String ORGANISATIONS_EINHEIT_ID = GroupRepresentationTestFactory.ORGANISATIONS_EINHEIT_ID; @@ -12,7 +11,6 @@ public class AddGroupDataTestFactory { public static AddGroupData.AddGroupDataBuilder createBuilder() { return new AddGroupData.AddGroupDataBuilder() - .parentId(PARENT_ID) .name(NAME) .organisationsEinheitId(ORGANISATIONS_EINHEIT_ID); } diff --git a/src/test/java/de/ozgcloud/admin/keycloak/GroupMapperTest.java b/src/test/java/de/ozgcloud/admin/keycloak/GroupMapperTest.java index adaddc58f2a88a674a6e66e2fba85c698ba81570..82b47bca310517e47c6ac5b812c4abc9aef66db9 100644 --- a/src/test/java/de/ozgcloud/admin/keycloak/GroupMapperTest.java +++ b/src/test/java/de/ozgcloud/admin/keycloak/GroupMapperTest.java @@ -4,6 +4,7 @@ import static de.ozgcloud.admin.keycloak.GroupRepresentationTestFactory.*; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; @@ -13,8 +14,11 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; import org.keycloak.representations.idm.GroupRepresentation; import org.mapstruct.factory.Mappers; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; @@ -28,42 +32,64 @@ class GroupMapperTest { private GroupMapper mapper = Mappers.getMapper(GroupMapper.class); @Nested - class TestGetOrganisationsEinheitId { + class TestFromGroupRepresentations { + + private final GroupRepresentation groupRepresentation = GroupRepresentationTestFactory.create(); + + @Captor + private ArgumentCaptor<List<Group>> groupsCaptor; @Test - void shouldReturnNullIfAttributesAreNull() { - var result = mapper.getOrganisationsEinheitId(null); + void shouldMapFromGroupRepresentation() { + givenMappedGroupWithOrganisationsEinheitId(); - assertThat(result).isNull(); + callMapper(); + + verify(mapper).fromGroupRepresentation(groupRepresentation); } @Test - void shouldReturnNullIfAttributeIsAbsent() { - givenOrganisationsEinheitIdProperty(); + void shouldReturnListWithMappedGroupRepresentation() { + var group = givenMappedGroupWithOrganisationsEinheitId(); - var result = mapper.getOrganisationsEinheitId(Map.of("dummy-attribute", List.of("123"))); + var groups = callMapper(); - assertThat(result).isNull(); + assertThat(groups).containsExactly(group); } - @Test - void shouldReturnOrganisationsEinheitId() { - givenOrganisationsEinheitIdProperty(); - var value = GroupRepresentationTestFactory.ORGANISATIONS_EINHEIT_ID; - var result = mapper.getOrganisationsEinheitId(Map.of(ORGANIZATIONS_EINHEIT_ID_ATTRIBUTE, List.of(value))); + @ParameterizedTest + @NullAndEmptySource + void shouldReturnEmptyList(String organisationsEinheitId) { + givenMappedGroupWithOrganisationsEinheitId(organisationsEinheitId); - assertThat(result).isEqualTo(value); + var groups = callMapper(); + + assertThat(groups).isEmpty(); } @Test - void shouldReturnFirstValueIfMultipleAreAvailable() { - givenOrganisationsEinheitIdProperty(); - var value = GroupRepresentationTestFactory.ORGANISATIONS_EINHEIT_ID; - var value2 = UUID.randomUUID().toString(); + void shouldDeleteGroupsWithoutOrganisationsEinheitId() { + var group = givenMappedGroupWithOrganisationsEinheitId(); - var result = mapper.getOrganisationsEinheitId(Map.of(ORGANIZATIONS_EINHEIT_ID_ATTRIBUTE, List.of(value, value2))); + callMapper(); - assertThat(result).isEqualTo(value); + verify(mapper).deleteGroupsWithoutOrganisationsEinheitId(groupsCaptor.capture()); + assertThat(groupsCaptor.getValue()).containsExactly(group); + } + + private Group givenMappedGroupWithOrganisationsEinheitId() { + var group = GroupTestFactory.create(); + doReturn(group).when(mapper).fromGroupRepresentation(groupRepresentation); + return group; + } + + private void givenMappedGroupWithOrganisationsEinheitId(String id) { + var group = GroupTestFactory.createBuilder().organisationsEinheitId(id).build(); + doReturn(group).when(mapper).fromGroupRepresentation(groupRepresentation); + } + + private List<Group> callMapper() { + return mapper.fromGroupRepresentations(List.of(groupRepresentation)); } } @@ -122,51 +148,101 @@ class GroupMapperTest { } @Nested - class TestFromGroupRepresentations { + class TestGetOrganisationsEinheitId { - private final GroupRepresentation groupRepresentation = GroupRepresentationTestFactory.create(); + @Test + void shouldReturnNullIfAttributesAreNull() { + var result = mapper.getOrganisationsEinheitId(null); + + assertThat(result).isNull(); + } @Test - void shouldMapFromGroupRepresentation() { - givenMappedGroupWithOrganisationsEinheitId(); + void shouldReturnNullIfAttributeIsAbsent() { + givenOrganisationsEinheitIdProperty(); - callMapper(); + var result = mapper.getOrganisationsEinheitId(Map.of("dummy-attribute", List.of("123"))); - verify(mapper).fromGroupRepresentation(groupRepresentation); + assertThat(result).isNull(); } @Test - void shouldReturnListWithMappedGroupRepresentation() { - var group = givenMappedGroupWithOrganisationsEinheitId(); + void shouldReturnOrganisationsEinheitId() { + givenOrganisationsEinheitIdProperty(); + var value = GroupRepresentationTestFactory.ORGANISATIONS_EINHEIT_ID; + var result = mapper.getOrganisationsEinheitId(Map.of(ORGANIZATIONS_EINHEIT_ID_ATTRIBUTE, List.of(value))); - var groups = callMapper(); + assertThat(result).isEqualTo(value); + } + + @Test + void shouldThrowExceptionIfMultipleValuesAreAvailable() { + givenOrganisationsEinheitIdProperty(); + var value = GroupRepresentationTestFactory.ORGANISATIONS_EINHEIT_ID; + var value2 = UUID.randomUUID().toString(); + + assertThatExceptionOfType(GroupRepresentationMappingException.class) + .isThrownBy(() -> mapper.getOrganisationsEinheitId(Map.of(ORGANIZATIONS_EINHEIT_ID_ATTRIBUTE, List.of(value, value2)))) + .withMessage("Group contains multiple values for organisationsEinheitId: %s", List.of(value, value2)); + } + } + + @Nested + class TestDeleteGroupsWithoutOrganisationsEinheitId { + + private final Group group = GroupTestFactory.create(); + private final List<Group> groups = new ArrayList<>(); + + @BeforeEach + void init() { + groups.add(group); + } + + @Test + void shouldCheckIfGroupHasOrganisationsEinheitId() { + mapper.deleteGroupsWithoutOrganisationsEinheitId(groups); + + verify(mapper).isMissingOrganisationsEinheitId(group); + } + + @Test + void shouldKeepGroupsWithOrganisationsEinheitId() { + doReturn(false).when(mapper).isMissingOrganisationsEinheitId(group); + + mapper.deleteGroupsWithoutOrganisationsEinheitId(groups); assertThat(groups).containsExactly(group); } - @ParameterizedTest - @NullAndEmptySource - void shouldReturnEmptyList(String organisationsEinheitId) { - givenMappedGroupWithOrganisationsEinheitId(organisationsEinheitId); + @Test + void shouldRemoveGroupsWithoutOrganisationsEinheitId() { + doReturn(true).when(mapper).isMissingOrganisationsEinheitId(group); - var groups = callMapper(); + mapper.deleteGroupsWithoutOrganisationsEinheitId(groups); assertThat(groups).isEmpty(); } + } - private Group givenMappedGroupWithOrganisationsEinheitId() { - var group = GroupTestFactory.create(); - doReturn(group).when(mapper).fromGroupRepresentation(groupRepresentation); - return group; - } + @Nested + class TestIsMissingOrganisationsEinheitId { - private void givenMappedGroupWithOrganisationsEinheitId(String id) { - var group = GroupTestFactory.createBuilder().organisationsEinheitId(id).build(); - doReturn(group).when(mapper).fromGroupRepresentation(groupRepresentation); + @ParameterizedTest + @ValueSource(strings = " ") + @NullAndEmptySource + void shouldReturnTrueIfIdIsBlank(String organisationsEinheitId) { + var group = GroupTestFactory.createBuilder().organisationsEinheitId(organisationsEinheitId).build(); + + var isMissing = mapper.isMissingOrganisationsEinheitId(group); + + assertThat(isMissing).isTrue(); } - private List<Group> callMapper() { - return mapper.fromGroupRepresentations(List.of(groupRepresentation)); + @Test + void shouldReturnFalseIfIdIsSet() { + var isMissing = mapper.isMissingOrganisationsEinheitId(GroupTestFactory.create()); + + assertThat(isMissing).isFalse(); } } @@ -177,14 +253,7 @@ class GroupMapperTest { @BeforeEach void init() { - doReturn(ATTRIBUTES).when(mapper).getAttributes(addGroupData); - } - - @Test - void shouldSetParentId() { - var groupRepresentation = callMapper(); - - assertThat(groupRepresentation.getParentId()).isEqualTo(addGroupData.getParentId()); + doReturn(ATTRIBUTES).when(mapper).buildGroupAttributes(addGroupData); } @Test @@ -202,10 +271,10 @@ class GroupMapperTest { } @Test - void shouldGetAttributes() { + void shouldBuildGroupAttributes() { callMapper(); - verify(mapper).getAttributes(addGroupData); + verify(mapper).buildGroupAttributes(addGroupData); } @Test @@ -221,14 +290,14 @@ class GroupMapperTest { } @Nested - class TestGetAttributes { + class TestBuildGroupAttributes { @ParameterizedTest @NullAndEmptySource void shouldReturnEmptyMap(String organisationsEinheitId) { var addGroupData = AddGroupDataTestFactory.createBuilder().organisationsEinheitId(organisationsEinheitId).build(); - var attributes = mapper.getAttributes(addGroupData); + var attributes = mapper.buildGroupAttributes(addGroupData); assertThat(attributes).isEmpty(); } @@ -237,7 +306,7 @@ class GroupMapperTest { void shouldAddOrganisationsEinheitIdToAttributes() { givenOrganisationsEinheitIdProperty(); - var attributes = mapper.getAttributes(AddGroupDataTestFactory.create()); + var attributes = mapper.buildGroupAttributes(AddGroupDataTestFactory.create()); assertThat(attributes).hasSize(1).containsEntry(ORGANIZATIONS_EINHEIT_ID_ATTRIBUTE, List.of(GroupTestFactory.ORGANISATIONS_EINHEIT_ID)); } diff --git a/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceITCase.java b/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceITCase.java index f4bbd5036da9a34a15704c19209e2d4571eb9705..f2c7dd6c83ee9651cca09c17ad7eda8927ac94eb 100644 --- a/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceITCase.java +++ b/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceITCase.java @@ -8,7 +8,6 @@ import java.util.Optional; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.keycloak.admin.client.resource.GroupsResource; import org.keycloak.representations.idm.GroupRepresentation; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.context.ContextConfiguration; @@ -26,8 +25,6 @@ class KeycloakApiServiceITCase { private KeycloakApiService service; @Autowired private KeycloakApiProperties properties; - @Autowired - private GroupsResource groupsResource; @Nested class TestGetAllGroups { @@ -158,20 +155,6 @@ class KeycloakApiServiceITCase { .asList().isEmpty(); } - @Test - void shouldAddSubgroupToParent() { - var parentToAdd = createUniqueGroupRepresentation("shouldAddSubgroupToParent-parent"); - var parentId = service.addGroup(parentToAdd); - var childToAdd = createUniqueGroupRepresentation("shouldAddSubgroupToParent-child"); - childToAdd.setParentId(parentId); - - var childId = service.addGroup(childToAdd); - - var subgroupsInKc = groupsResource.group(parentId).getSubGroups(0, Integer.MAX_VALUE, true); - assertThat(subgroupsInKc).hasSize(1).first().extracting(GroupRepresentation::getId, GroupRepresentation::getParentId) - .contains(childId, parentId); - } - private GroupRepresentation createUniqueGroupRepresentation(String nameSuffix) { // LoremIpsum does not guarantee unique results when called repeatedly var groupName = "%s (%s)".formatted(LoremIpsum.getInstance().getName(), nameSuffix); diff --git a/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceTest.java b/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceTest.java index f04a47581dc3c2d7c67a79edb6f4ec3f1c69572b..ce9ed1256d94d81e5e04e820fe2436bfdc34b168 100644 --- a/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceTest.java +++ b/src/test/java/de/ozgcloud/admin/keycloak/KeycloakApiServiceTest.java @@ -3,10 +3,11 @@ package de.ozgcloud.admin.keycloak; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import java.util.List; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.keycloak.admin.client.resource.GroupResource; import org.keycloak.admin.client.resource.GroupsResource; import org.keycloak.representations.idm.GroupRepresentation; import org.mockito.InjectMocks; @@ -27,37 +28,58 @@ class KeycloakApiServiceTest { @Mock private Response response; + @Nested + class TestGetAllGroups { + + @Test + void shouldCallGroupsResource() { + service.getAllGroups(); + + verify(groupsResource).groups("", 0, Integer.MAX_VALUE, false); + } + + @Test + void shouldReturnGroupRepresentations() { + var groupRepresentation = GroupRepresentationTestFactory.create(); + when(groupsResource.groups("", 0, Integer.MAX_VALUE, false)).thenReturn(List.of(groupRepresentation)); + + var gotGroupRepresentations = service.getAllGroups(); + + assertThat(gotGroupRepresentations).containsExactly(groupRepresentation); + } + } + @Nested class TestAddGroup { private final GroupRepresentation groupRepresentation = GroupRepresentationTestFactory.create(); - private final String RESOURCE_ID = GroupRepresentationTestFactory.create().getId(); + private final String resourceId = GroupRepresentationTestFactory.create().getId(); @BeforeEach void init() { - doReturn(response).when(service).addParentOrChildGroup(groupRepresentation); - doReturn(RESOURCE_ID).when(service).getAddedResourceIdFromResponse(response); + when(groupsResource.add(groupRepresentation)).thenReturn(response); + doReturn(resourceId).when(service).getCreatedResourceIdFromResponse(response); } @Test - void shouldAddParentOrChildGroup() { + void shouldCallGroupsResource() { callService(); - verify(service).addParentOrChildGroup(groupRepresentation); + verify(groupsResource).add(groupRepresentation); } @Test void shouldGetAddedResourceIdFromResponse() { callService(); - verify(service).getAddedResourceIdFromResponse(response); + verify(service).getCreatedResourceIdFromResponse(response); } @Test void shouldReturnAddedResourceId() { var id = callService(); - assertThat(id).isEqualTo(RESOURCE_ID); + assertThat(id).isEqualTo(resourceId); } private String callService() { @@ -66,73 +88,10 @@ class KeycloakApiServiceTest { } @Nested - class TestAddParentOrChildGroup { - - private GroupRepresentation groupRepresentation; - - @Nested - class OnParentGroup { - - @BeforeEach - void init() { - groupRepresentation = GroupRepresentationTestFactory.create(); - groupRepresentation.setParentId(null); - when(groupsResource.add(groupRepresentation)).thenReturn(response); - } - - @Test - void shouldCallGroupsResource() { - callService(); - - verify(groupsResource).add(groupRepresentation); - } - - @Test - void shouldReturnResponse() { - var serviceResponse = callService(); - - assertThat(serviceResponse).isEqualTo(response); - } - } - - @Nested - class OnChildGroup { - - @Mock - private GroupResource groupResource; - - @BeforeEach - void init() { - groupRepresentation = GroupRepresentationTestFactory.create(); - when(groupsResource.group(GroupRepresentationTestFactory.PARENT_ID)).thenReturn(groupResource); - when(groupResource.subGroup(groupRepresentation)).thenReturn(response); - } - - @Test - void shouldCallGroupResource() { - callService(); - - verify(groupResource).subGroup(groupRepresentation); - } - - @Test - void shouldReturnResponse() { - var serviceResponse = callService(); - - assertThat(serviceResponse).isEqualTo(response); - } - } - - private Response callService() { - return service.addParentOrChildGroup(groupRepresentation); - } - } - - @Nested - class TestGetAddedResourceIdFromResponse { + class TestGetCreatedResourceIdFromResponse { - private final String RESOURCE_ID = GroupRepresentationTestFactory.create().getId(); - private final String LOCATION = LoremIpsum.getInstance().getUrl(); + private final String resourceId = GroupRepresentationTestFactory.create().getId(); + private final String location = LoremIpsum.getInstance().getUrl(); @Test void shouldExtractResourceIdFromLocationHeader() { @@ -140,7 +99,7 @@ class KeycloakApiServiceTest { callService(); - verify(service).extractResourceIdFromLocationHeader(LOCATION); + verify(service).extractResourceIdFromLocationHeader(location); } @Test @@ -149,7 +108,7 @@ class KeycloakApiServiceTest { var id = callService(); - assertThat(id).isEqualTo(RESOURCE_ID); + assertThat(id).isEqualTo(resourceId); } @Test @@ -162,8 +121,8 @@ class KeycloakApiServiceTest { private void givenResponseCreated() { when(response.getStatus()).thenReturn(201); - when(response.getHeaderString("Location")).thenReturn(LOCATION); - doReturn(RESOURCE_ID).when(service).extractResourceIdFromLocationHeader(LOCATION); + when(response.getHeaderString("Location")).thenReturn(location); + doReturn(resourceId).when(service).extractResourceIdFromLocationHeader(location); } private void givenResponseUnauthorized() { @@ -172,21 +131,21 @@ class KeycloakApiServiceTest { } private String callService() { - return service.getAddedResourceIdFromResponse(response); + return service.getCreatedResourceIdFromResponse(response); } } @Nested class TestExtractResourceIdFromLocationHeader { - private final String RESOURCE_ID = GroupRepresentationTestFactory.create().getId(); - private final String LOCATION = "https://keycloak-url.test/admin/realms/test/groups/" + RESOURCE_ID; + private final String resourceId = GroupRepresentationTestFactory.create().getId(); + private final String location = "https://keycloak-url.test/admin/realms/test/groups/" + resourceId; @Test void shouldReturnId() { - var id = service.extractResourceIdFromLocationHeader(LOCATION); + var id = service.extractResourceIdFromLocationHeader(location); - assertThat(id).isEqualTo(RESOURCE_ID); + assertThat(id).isEqualTo(resourceId); } } } diff --git a/src/test/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapperTest.java b/src/test/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapperTest.java index 9192df6b2e1ac6545099bec61c84006758dadde3..53ec27998c68c86ce3dcdb7b91a1a8ba77fb8c20 100644 --- a/src/test/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapperTest.java +++ b/src/test/java/de/ozgcloud/admin/organisationseinheit/OrganisationsEinheitMapperTest.java @@ -4,7 +4,6 @@ import static org.assertj.core.api.Assertions.*; import java.util.Collections; import java.util.Objects; -import java.util.UUID; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -46,18 +45,14 @@ class OrganisationsEinheitMapperTest { @Nested class TestToAddGroupData { - public static final String PARENT_KEYCLOAK_ID = UUID.randomUUID().toString(); - @Test void shouldMap() { - var mapped = mapper.toAddGroupData(OrganisationsEinheitTestFactory.create(), PARENT_KEYCLOAK_ID); + var mapped = mapper.toAddGroupData(OrganisationsEinheitTestFactory.create()); assertThat(mapped).extracting( - AddGroupData::getParentId, AddGroupData::getName, AddGroupData::getOrganisationsEinheitId ).containsExactly( - PARENT_KEYCLOAK_ID, OrganisationsEinheitTestFactory.NAME, OrganisationsEinheitTestFactory.ORGANISATIONS_EINHEIT_ID ); diff --git a/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncAddedOrganisationsEinheitenITCase.java b/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncAddedOrganisationsEinheitenITCase.java index ca1b0366500f5ea25e29cef7ebdaee5deff22484..1af0cf1617c485ebb64936aba8e653cdafa83ed1 100644 --- a/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncAddedOrganisationsEinheitenITCase.java +++ b/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncAddedOrganisationsEinheitenITCase.java @@ -2,11 +2,9 @@ package de.ozgcloud.admin.organisationseinheit; import static org.assertj.core.api.Assertions.*; import static org.assertj.core.groups.Tuple.tuple; -import static org.mockito.Mockito.*; import java.time.Instant; import java.util.List; -import java.util.Optional; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; @@ -19,7 +17,6 @@ import org.springframework.test.context.ContextConfiguration; import de.ozgcloud.admin.common.KeycloakInitializer; import de.ozgcloud.admin.keycloak.Group; import de.ozgcloud.admin.keycloak.KeycloakRemoteService; -import de.ozgcloud.admin.keycloak.ResourceCreationException; import de.ozgcloud.common.test.DbInitializer; import de.ozgcloud.common.test.ITCase; @@ -53,48 +50,6 @@ class SyncAddedOrganisationsEinheitenITCase { .contains(tuple(topLevel.getName(), topLevel.getOrganisationsEinheitId(), List.of())); } - @Test - void shouldSynchronizeAddedTopLevelGroupWithChild() { - var topLevel = topLevel("shouldSynchronizeAddedTopLevelGroupWithChild"); - var childLevel1 = childLevel("shouldSynchronizeAddedTopLevelGroupWithChild", topLevel); - operations.save(topLevel); - operations.save(childLevel1); - - service.syncAddedOrganisationsEinheiten(syncTimestamp); - - assertThat(findGroupInKeycloak(topLevel.getName())).isPresent().get() - .extracting(Group::getSubGroups).asList().extracting("name") - .containsExactly(childLevel1.getName()); - } - - @Test - void shouldSynchronizeChildAddedToAlreadySynchronizedParent() { - var parent = topLevel("shouldSynchronizeChildAddedToAlreadySynchronizedParent"); - syncOrganisationsEinheitToKeycloak(parent); - var childLevel1 = childLevel("shouldSynchronizeChildAddedToAlreadySynchronizedParent", parent); - operations.save(childLevel1); - - service.syncAddedOrganisationsEinheiten(syncTimestamp); - - assertThat(findGroupInKeycloak(parent.getName())).isPresent().get() - .extracting(Group::getSubGroups).asList().extracting("name") - .containsExactly(childLevel1.getName()); - } - - @Test - void shouldNotAddChildIfParentWasNotSynchronized() { - var topLevel = topLevel("shouldNotAddChildIfParentWasNotSynchronized"); - var childLevel1 = childLevel("shouldNotAddChildIfParentWasNotSynchronized", topLevel); - operations.save(topLevel); - operations.save(childLevel1); - doThrow(new ResourceCreationException("OMG!")).when(keycloakRemoteService) - .addGroup(argThat(addGroupData -> addGroupData.getName().equals(topLevel.getName()))); - - service.syncAddedOrganisationsEinheiten(syncTimestamp); - - assertThat(findGroupInKeycloak(childLevel1.getName())).isEmpty(); - } - private static OrganisationsEinheit topLevel(String nameSuffix) { return OrganisationsEinheitTestFactory.createBuilder() .id(UUID.randomUUID().toString()) @@ -104,25 +59,4 @@ class SyncAddedOrganisationsEinheitenITCase { .name("topLevel (%s)".formatted(nameSuffix)) .build(); } - - private static OrganisationsEinheit childLevel(String nameSuffix, OrganisationsEinheit parent) { - return OrganisationsEinheitTestFactory.createBuilder() - .id(UUID.randomUUID().toString()) - .parentId(parent.getId()) - .keycloakId(null) - .syncResult(null) - .name("childLevel1 (%s)".formatted(nameSuffix)) - .build(); - } - - private Optional<Group> findGroupInKeycloak(String groupName) { - return keycloakRemoteService.getGroupsWithOrganisationsEinheitId() - .filter(group -> groupName.equals(group.getName())) - .findFirst(); - } - - private void syncOrganisationsEinheitToKeycloak(OrganisationsEinheit organisationsEinheit) { - operations.save(organisationsEinheit); - service.syncAddedOrganisationsEinheiten(syncTimestamp); - } } diff --git a/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncServiceTest.java b/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncServiceTest.java index af78396ac96a80283352eb82a1aedd302b0f64ec..f9b3a0011fc4767b653a49ea3d1c3b85f65034e8 100644 --- a/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncServiceTest.java +++ b/src/test/java/de/ozgcloud/admin/organisationseinheit/SyncServiceTest.java @@ -5,7 +5,6 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import java.time.Instant; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -443,17 +442,15 @@ class SyncServiceTest { private final long syncTimestamp = Instant.now().toEpochMilli(); private final OrganisationsEinheit withoutSyncResult1 = OrganisationsEinheitTestFactory.createBuilder().id("A").build(); private final OrganisationsEinheit withoutSyncResult2 = OrganisationsEinheitTestFactory.createBuilder().id("B").build(); - private final OrganisationsEinheit[] unsortedOrganisationsEinheiten = new OrganisationsEinheit[] { withoutSyncResult2, withoutSyncResult1 }; - private final OrganisationsEinheit[] sortedOrganisationsEinheiten = new OrganisationsEinheit[] { withoutSyncResult1, withoutSyncResult2 }; + private final OrganisationsEinheit[] organisationsEinheiten = new OrganisationsEinheit[] { withoutSyncResult2, withoutSyncResult1 }; @Captor private ArgumentCaptor<Stream<OrganisationsEinheit>> streamArgumentCaptor; @BeforeEach void setUp() { - when(repository.findAllWithoutSyncResult()).thenReturn(Stream.of(unsortedOrganisationsEinheiten)); - doReturn(Stream.of(sortedOrganisationsEinheiten)).when(service).sortInAdditionOrder(any()); - doNothing().when(service).syncAddedOrganisationsEinheit(anyLong(), any()); + when(repository.findAllWithoutSyncResult()).thenReturn(Stream.of(organisationsEinheiten)); + doNothing().when(service).syncAddedOrganisationsEinheit(any(), anyLong()); } @Test @@ -463,36 +460,18 @@ class SyncServiceTest { verify(repository).findAllWithoutSyncResult(); } - @Test - void shouldSortInAdditionOrder() { - callService(); - - verify(service).sortInAdditionOrder(streamArgumentCaptor.capture()); - assertThat(streamArgumentCaptor.getValue()).containsExactly(unsortedOrganisationsEinheiten); - } - @Test void shouldSynchronizeFirstOrganisationsEinheit() { callService(); - verify(service).syncAddedOrganisationsEinheit(syncTimestamp, withoutSyncResult1); + verify(service).syncAddedOrganisationsEinheit(withoutSyncResult1, syncTimestamp); } @Test void shouldSynchronizeSecondOrganisationsEinheit() { callService(); - verify(service).syncAddedOrganisationsEinheit(syncTimestamp, withoutSyncResult2); - } - - @Test - void shouldSynchronizeInOrder() { - var inOrder = inOrder(service); - - callService(); - - Arrays.stream(sortedOrganisationsEinheiten).forEach(organisationsEinheit -> - inOrder.verify(service).syncAddedOrganisationsEinheit(syncTimestamp, organisationsEinheit)); + verify(service).syncAddedOrganisationsEinheit(withoutSyncResult2, syncTimestamp); } private void callService() { @@ -500,240 +479,101 @@ class SyncServiceTest { } } - @Nested - class TestSortInAdditionOrder { - - @Test - void shouldTopLevelOrgComeFirst() { - var orgWithParent = OrganisationsEinheitTestFactory.create(); - var orgWithoutParent = OrganisationsEinheitTestFactory.createBuilder().parentId(null).build(); - - var sorted = service.sortInAdditionOrder(Stream.of(orgWithParent, orgWithoutParent)); - - assertThat(sorted).containsExactly(orgWithoutParent, orgWithParent); - } - - @Test - void shouldParentComeBeforeChild() { - var parent = OrganisationsEinheitTestFactory.create(); - var child = OrganisationsEinheitTestFactory.createBuilder().parentId(parent.getId()).build(); - - var sorted = service.sortInAdditionOrder(Stream.of(child, parent)); - - assertThat(sorted).containsExactly(parent, child); - } - - @Test - void shouldPreserveOrderForTopLevelGroups() { - var orgWithoutParent1 = OrganisationsEinheitTestFactory.createBuilder().parentId(null).build(); - var orgWithoutParent2 = OrganisationsEinheitTestFactory.createBuilder().parentId(null).build(); - - var sorted = service.sortInAdditionOrder(Stream.of(orgWithoutParent1, orgWithoutParent2)); - - assertThat(sorted).containsExactly(orgWithoutParent1, orgWithoutParent2); - } - } - @Nested class TestSyncAddedOrganisationsEinheit { private final long syncTimestamp = Instant.now().toEpochMilli(); private final String addedGroupId = UUID.randomUUID().toString(); - private final String parentKeycloakId = UUID.randomUUID().toString(); private final OrganisationsEinheit organisationsEinheit = OrganisationsEinheitTestFactory.create(); - private final OrganisationsEinheit parentWithoutKeycloakId = OrganisationsEinheitTestFactory.createBuilder().keycloakId(null).build(); - private final OrganisationsEinheit parentWithKeycloakId = OrganisationsEinheitTestFactory.createBuilder().keycloakId(parentKeycloakId) - .build(); @Test - void shouldFindParentKeycloakId() { - doReturn(Optional.of(parentWithoutKeycloakId)).when(service).findParent(organisationsEinheit); + void shouldAddAsGroupInKeycloak() { + doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit); callService(); - verify(service).findParent(organisationsEinheit); + verify(service).addAsGroupInKeycloak(organisationsEinheit); } - @Nested - class OnParentHasNoKeycloakId { - - @BeforeEach - void init() { - doReturn(Optional.of(parentWithoutKeycloakId)).when(service).findParent(organisationsEinheit); - } + @Test + void shouldUpdateAfterSuccessfulGroupCreation() { + doReturn(Optional.of(addedGroupId)).when(service).addAsGroupInKeycloak(organisationsEinheit); + doNothing().when(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, addedGroupId, syncTimestamp); - @Test - void shouldNotAddAsGroupInKeycloak() { - callService(); + callService(); - verify(service, never()).addAsGroupInKeycloak(any(), any()); - } + verify(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, addedGroupId, syncTimestamp); } - @Nested - class OnOrganisationsEinheitIsTopLevel { - - @BeforeEach - void init() { - doReturn(Optional.empty()).when(service).findParent(organisationsEinheit); - } - - @Test - void shouldAddAsGroupInKeycloakWithNullParent() { - doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit, null); - - callService(); - - verify(service).addAsGroupInKeycloak(organisationsEinheit, null); - } - - @Test - void shouldUpdateAfterSuccessfulGroupCreation() { - doReturn(Optional.of(addedGroupId)).when(service).addAsGroupInKeycloak(organisationsEinheit, null); - doNothing().when(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, addedGroupId); - - callService(); - - verify(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, addedGroupId); - } + @Test + void shouldNotUpdate() { + doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit); - @Test - void shouldNotUpdate() { - doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit, null); + callService(); - callService(); + verify(service, never()).updateAfterSuccessfulGroupCreation(any(), any(), anyLong()); + } - verify(service, never()).updateAfterSuccessfulGroupCreation(any(), anyLong(), any()); - } + private void callService() { + service.syncAddedOrganisationsEinheit(organisationsEinheit, syncTimestamp); } + } + + @Nested + class TestAddAsGroupInKeycloak { @Nested - class OnParentHasKeycloakId { + class OnParentIdIsNotSet { + + private final OrganisationsEinheit organisationsEinheit = OrganisationsEinheitTestFactory.createBuilder().parentId(null).build(); + private final AddGroupData addGroupData = AddGroupDataTestFactory.create(); + private final String addedGroupId = UUID.randomUUID().toString(); @BeforeEach void init() { - doReturn(Optional.of(parentWithKeycloakId)).when(service).findParent(organisationsEinheit); + when(organisationsEinheitMapper.toAddGroupData(organisationsEinheit)).thenReturn(addGroupData); + doReturn(Optional.of(addedGroupId)).when(service).addGroupInKeycloak(addGroupData); } @Test - void shouldAddAsGroupInKeycloakWithNullParent() { - doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId); - + void shouldCreateAddGroupData() { callService(); - verify(service).addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId); + verify(organisationsEinheitMapper).toAddGroupData(organisationsEinheit); } @Test - void shouldUpdateAfterSuccessfulGroupCreation() { - doReturn(Optional.of(addedGroupId)).when(service).addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId); - doNothing().when(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, addedGroupId); - + void shouldAddGroupInKeycloak() { callService(); - verify(service).updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, addedGroupId); + verify(service).addGroupInKeycloak(addGroupData); } @Test - void shouldNotUpdate() { - doReturn(Optional.empty()).when(service).addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId); - - callService(); + void shouldReturnAddedGroupId() { + var returnedGroupId = callService(); - verify(service, never()).updateAfterSuccessfulGroupCreation(any(), anyLong(), any()); + assertThat(returnedGroupId).get().isEqualTo(addedGroupId); } - } - - private void callService() { - service.syncAddedOrganisationsEinheit(syncTimestamp, organisationsEinheit); - } - } - - @Nested - class TestFindParent { - @Nested - class OnParentIdIsNull { - @Test - void shouldReturnEmpty() { - var parent = service.findParent(OrganisationsEinheitTestFactory.createBuilder().parentId(null).build()); - - assertThat(parent).isEmpty(); + private Optional<String> callService() { + return service.addAsGroupInKeycloak(organisationsEinheit); } } @Nested - class OnParentIdIsNotNull { - - private final String PARENT_KEYCLOAK_ID = UUID.randomUUID().toString(); - private final OrganisationsEinheit organisationsEinheit = OrganisationsEinheitTestFactory.create(); - private final OrganisationsEinheit parent = OrganisationsEinheitTestFactory.createBuilder().keycloakId(PARENT_KEYCLOAK_ID).build(); - - @BeforeEach - void init() { - when(repository.findById(OrganisationsEinheitTestFactory.PARENT_ID)).thenReturn(Optional.of(parent)); - } + class OnParentIdIsSet { @Test - void shouldFindById() { - callService(); - - verify(repository).findById(OrganisationsEinheitTestFactory.PARENT_ID); - } - - @Test - void shouldReturnParent() { - var found = callService(); - - assertThat(found).isPresent().get().isEqualTo(parent); - } - - private Optional<OrganisationsEinheit> callService() { - return service.findParent(organisationsEinheit); + void shouldThrowException() { + var withParentId = OrganisationsEinheitTestFactory.create(); + assertThatExceptionOfType(OrganisationsEinheitSynchronizationException.class) + .isThrownBy(() -> service.addAsGroupInKeycloak(withParentId)) + .withMessage("Organisationseinheit " + withParentId.getOrganisationsEinheitId() + " has parent"); } } } - @Nested - class TestAddAsGroupInKeycloak { - - private final OrganisationsEinheit organisationsEinheit = OrganisationsEinheitTestFactory.create(); - private final String parentKeycloakId = UUID.randomUUID().toString(); - private final AddGroupData addGroupData = AddGroupDataTestFactory.create(); - private final String addedGroupId = UUID.randomUUID().toString(); - - @BeforeEach - void init() { - when(organisationsEinheitMapper.toAddGroupData(organisationsEinheit, parentKeycloakId)).thenReturn(addGroupData); - doReturn(Optional.of(addedGroupId)).when(service).addGroupInKeycloak(addGroupData); - } - - @Test - void shouldCreateAddGroupData() { - callService(); - - verify(organisationsEinheitMapper).toAddGroupData(organisationsEinheit, parentKeycloakId); - } - - @Test - void shouldAddGroupInKeycloak() { - callService(); - - verify(service).addGroupInKeycloak(addGroupData); - } - - @Test - void shouldReturnAddedGroupId() { - var returnedGroupId = callService(); - - assertThat(returnedGroupId).get().isEqualTo(addedGroupId); - } - - private Optional<String> callService() { - return service.addAsGroupInKeycloak(organisationsEinheit, parentKeycloakId); - } - } - @Nested class TestUpdateAfterSuccessfulGroupCreation { @@ -750,7 +590,7 @@ class SyncServiceTest { @Test void shouldSaveUpdatedOrganisationsEinheit() { - service.updateAfterSuccessfulGroupCreation(organisationsEinheit, syncTimestamp, keycloakId); + service.updateAfterSuccessfulGroupCreation(organisationsEinheit, keycloakId, syncTimestamp); verify(repository).save(organisationsEinheitArgumentCaptor.capture()); assertThat(organisationsEinheitArgumentCaptor.getValue()) @@ -762,6 +602,9 @@ class SyncServiceTest { @Nested class TestAddGroupInKeycloak { + private static final String FAILURE_MESSAGE = LoremIpsum.getInstance().getWords(5); + private final Throwable resourceCreationException = new ResourceCreationException(FAILURE_MESSAGE); + private final String keycloakId = GroupTestFactory.ID; private final AddGroupData addGroupData = AddGroupDataTestFactory.create(); @@ -784,12 +627,11 @@ class SyncServiceTest { } @Test - void shouldReturnEmptyInCaseOfException() { + void shouldThrowExceptionInCaseOfResourceCreationException() { givenAddGroupFailed(); - var keycloakId = callService(); - - assertThat(keycloakId).isEmpty(); + assertThatExceptionOfType(OrganisationsEinheitSynchronizationException.class).isThrownBy(this::callService) + .withMessage("Error creating group %s in Keycloak", addGroupData.toString()).withCause(resourceCreationException); } private void givenAddGroupSuccessful() { @@ -797,7 +639,7 @@ class SyncServiceTest { } private void givenAddGroupFailed() { - when(keycloakRemoteService.addGroup(addGroupData)).thenThrow(new ResourceCreationException("Failure")); + when(keycloakRemoteService.addGroup(addGroupData)).thenThrow(resourceCreationException); } private Optional<String> callService() {