From aa16cb0887be54e04bdf6fbb95fc71b97ddfefbe Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 16 Oct 2024 10:00:48 +0200 Subject: [PATCH 01/24] tests for PersistenceBase class --- .../persistence/PersistenceBaseTestIT.java | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 backend/src/test/java/ch/puzzle/okr/service/persistence/PersistenceBaseTestIT.java diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/PersistenceBaseTestIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/PersistenceBaseTestIT.java new file mode 100644 index 0000000000..7f5a60a5e5 --- /dev/null +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/PersistenceBaseTestIT.java @@ -0,0 +1,182 @@ +package ch.puzzle.okr.service.persistence; + +import ch.puzzle.okr.dto.ErrorDto; +import ch.puzzle.okr.models.User; +import ch.puzzle.okr.multitenancy.TenantContext; +import ch.puzzle.okr.repository.UserRepository; +import ch.puzzle.okr.test.SpringIntegrationTest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.web.server.ResponseStatusException; + +import java.util.List; + +import static ch.puzzle.okr.test.TestHelper.getAllErrorKeys; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.springframework.http.HttpStatus.*; + +/** + * Testing the functionality of the abstract PersistenceBase and use UserRepository as example of a CrudRepository + * implementation. + *

+ * Tests depending on data from V100_0_0__TestData.sql + */ +@SpringIntegrationTest +public class PersistenceBaseTestIT { + + private User createdUser; + + private static final long NON_EXISTING_USER_ID = 321L; + private static final long USER_PACO_ID = 1L; + private static final User USER_WITHOUT_CONSTRAINTS = User.Builder.builder() // + .withFirstname("Hans") // + .withLastname("Muster") // + .withEmail("hans.muster@puzzle.ch") // + .build(); + + @Autowired + private PersistenceBase persistenceBase; + + @BeforeEach + void setUp() { + TenantContext.setCurrentTenant("pitc"); + } + + @AfterEach + void tearDown() { + if (createdUser != null) { + persistenceBase.deleteById(createdUser.getId()); + createdUser = null; + } + TenantContext.setCurrentTenant(null); + } + + @DisplayName("findById() should return single entity if entity with id exists") + @Test + void findByIdShouldReturnSingleEntityIfEntityWithIdExists() { + var foundUser = persistenceBase.findById(USER_PACO_ID); + + assertEquals(USER_PACO_ID, foundUser.getId()); + assertUser("Paco", "Eggimann", "peggimann@puzzle.ch", foundUser); + } + + @DisplayName("findById() should throw exception if entity with id does not exist") + @Test + void findByIdShouldThrowExceptionIfEntityWithIdDoesNotExist() { + var exception = assertThrows(ResponseStatusException.class, + () -> persistenceBase.findById(NON_EXISTING_USER_ID)); + + assertEquals(NOT_FOUND, exception.getStatusCode()); + assertErrorKey("MODEL_WITH_ID_NOT_FOUND", exception); + } + + @DisplayName("findById() should throw exception if id is null") + @Test + void findByIdShouldThrowExceptionIfIdIsNull() { + var exception = assertThrows(ResponseStatusException.class, () -> persistenceBase.findById(null)); + + assertEquals(BAD_REQUEST, exception.getStatusCode()); + assertErrorKey("ATTRIBUTE_NULL", exception); + } + + @DisplayName("findAll() should return all entities as list") + @Test + void findAllShouldReturnAllEntitiesAsList() throws ResponseStatusException { + var userList = persistenceBase.findAll(); + + assertThat(userList.size()).isGreaterThanOrEqualTo(7); + } + + @DisplayName("save() should add new entity") + @Test + void saveShouldAddNewEntity() throws ResponseStatusException { + createdUser = persistenceBase.save(USER_WITHOUT_CONSTRAINTS); + + assertNotNull(createdUser); + assertUser("Hans", "Muster", "hans.muster@puzzle.ch", createdUser); + } + + @DisplayName("save() should throw exception in the case of optimistic locking failure") + @Test + void saveShouldThrowExceptionInTheCaseOfOptimisticLockingFailure() throws ResponseStatusException { + // arrange + var testRepository = mock(UserRepository.class); + when(testRepository.save(any())).thenThrow(OptimisticLockingFailureException.class); + + var persistenceBaseForTest = new PersistenceBase<>(testRepository) { + @Override + public String getModelName() { + return "for_test"; + } + }; + + // act + assert + var exception = assertThrows(ResponseStatusException.class, () -> persistenceBaseForTest.save(createdUser)); + + // assert + assertEquals(UNPROCESSABLE_ENTITY, exception.getStatusCode()); + assertErrorKey("DATA_HAS_BEEN_UPDATED", exception); + } + + @DisplayName("save() existing entity with different data should update existing entity") + @Test + void saveExistingEntityWithDifferentDataShouldUpdateExistingEntity() throws ResponseStatusException { + // arrange + createdUser = persistenceBase.save(USER_WITHOUT_CONSTRAINTS); + var createdUserId = createdUser.getId(); + var foundUser = persistenceBase.findById(createdUserId); + + // pro-condition + assertEquals("Hans", createdUser.getFirstname()); + + // act + foundUser.setFirstname("Pekka"); + persistenceBase.save(foundUser); + foundUser = persistenceBase.findById(createdUserId); + + // assert + assertEquals(createdUserId, foundUser.getId()); + assertEquals("Pekka", foundUser.getFirstname()); + } + + @DisplayName("deleteById() should delete entity") + @Test + void deleteByIdShouldDeleteEntity() throws ResponseStatusException { + // arrange + createdUser = persistenceBase.save(USER_WITHOUT_CONSTRAINTS); + var createdUserId = createdUser.getId(); + assertNotNull(persistenceBase.findById(createdUserId)); + + // act + persistenceBase.deleteById(createdUserId); + + // assert + assertEntityNotFound(createdUserId); + } + + private static void assertUser(String expectedFirstName, String expectedLastName, String expectedEmail, + User currentUser) { + assertEquals(expectedFirstName, currentUser.getFirstname()); + assertEquals(expectedLastName, currentUser.getLastname()); + assertEquals(expectedEmail, currentUser.getEmail()); + } + + private void assertErrorKey(String errorKey, ResponseStatusException exception) { + var errorKeys = getAllErrorKeys(List.of(new ErrorDto(errorKey, List.of("User")))); + assertTrue(errorKeys.contains(exception.getReason())); + } + + private void assertEntityNotFound(long entityId) { + var exception = assertThrows(ResponseStatusException.class, () -> persistenceBase.findById(entityId)); + assertEquals(NOT_FOUND, exception.getStatusCode()); + assertErrorKey("MODEL_WITH_ID_NOT_FOUND", exception); + } +} \ No newline at end of file From 66dd93907da0ae4af4fce2123774b3f24a191a2e Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 16 Oct 2024 10:17:06 +0200 Subject: [PATCH 02/24] remove tests from QuarterPersistenceServiceIT which are already covered in PersistenceBaseTestIT --- .../QuarterPersistenceServiceIT.java | 46 +++---------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/QuarterPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/QuarterPersistenceServiceIT.java index 23a1ce5faf..1eee877cc1 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/QuarterPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/QuarterPersistenceServiceIT.java @@ -1,7 +1,5 @@ package ch.puzzle.okr.service.persistence; -import ch.puzzle.okr.dto.ErrorDto; -import ch.puzzle.okr.exception.OkrResponseStatusException; import ch.puzzle.okr.models.Quarter; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; @@ -16,12 +14,10 @@ import java.time.LocalDate; import java.util.List; +import static ch.puzzle.okr.Constants.QUARTER; import static ch.puzzle.okr.test.TestConstants.GJ_FOR_TESTS_QUARTER_ID; import static ch.puzzle.okr.test.TestConstants.GJ_FOR_TEST_QUARTER_LABEL; -import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; -import static org.springframework.http.HttpStatus.BAD_REQUEST; -import static org.springframework.http.HttpStatus.NOT_FOUND; @SpringIntegrationTest class QuarterPersistenceServiceIT { @@ -39,40 +35,6 @@ void tearDown() { TenantContext.setCurrentTenant(null); } - @Test - void shouldReturnSingleQuarterWhenFindingByValidId() { - Quarter returnedQuarter = quarterPersistenceService.findById(GJ_FOR_TESTS_QUARTER_ID); - - assertEquals(GJ_FOR_TESTS_QUARTER_ID, returnedQuarter.getId()); - assertEquals(GJ_FOR_TEST_QUARTER_LABEL, returnedQuarter.getLabel()); - assertEquals(LocalDate.of(2000, 7, 1), returnedQuarter.getStartDate()); - assertEquals(LocalDate.of(2000, 9, 30), returnedQuarter.getEndDate()); - } - - @Test - void shouldThrowExceptionWhenFindingQuarterNotFound() { - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> quarterPersistenceService.findById(321L)); - - List expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of("Quarter", "321"))); - - assertEquals(NOT_FOUND, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); - } - - @Test - void shouldThrowExceptionWhenFindingQuarterWithIdNull() { - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> quarterPersistenceService.findById(null)); - - List expectedErrors = List.of(new ErrorDto("ATTRIBUTE_NULL", List.of("ID", "Quarter"))); - - assertEquals(BAD_REQUEST, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); - } - @DisplayName("getMostCurrentQuarters() should return current quarter and future quarter and GJForTests quarter") @Test void getMostCurrentQuartersShouldReturnCurrentQuarterAndFutureQuarterAndGJForTestsQuarter() { @@ -141,4 +103,10 @@ void findByLabelShouldReturnNullWhenLabelIsNull() { // assert assertNull(returnedQuarter); } + + @DisplayName("getModelName() should return Quarter") + @Test + void getModelNameShouldReturnQuarter() { + assertEquals(QUARTER, quarterPersistenceService.getModelName()); + } } From 6582e3b7dc1bfc99ef8870a79584945ebab444dc Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 16 Oct 2024 10:52:17 +0200 Subject: [PATCH 03/24] remove no longer used OrganisationXXX() methods --- .../java/ch/puzzle/okr/repository/TeamRepository.java | 9 --------- .../okr/service/persistence/TeamPersistenceService.java | 8 -------- 2 files changed, 17 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/repository/TeamRepository.java b/backend/src/main/java/ch/puzzle/okr/repository/TeamRepository.java index 6a792921c5..14e48978ed 100644 --- a/backend/src/main/java/ch/puzzle/okr/repository/TeamRepository.java +++ b/backend/src/main/java/ch/puzzle/okr/repository/TeamRepository.java @@ -1,20 +1,11 @@ package ch.puzzle.okr.repository; import ch.puzzle.okr.models.Team; -import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.CrudRepository; -import org.springframework.data.repository.query.Param; import java.util.List; public interface TeamRepository extends CrudRepository { - @Query(value = """ - select distinct teamOrg.team_id from team_organisation teamOrg - inner join organisation o on o.id = teamOrg.organisation_id - where o.org_name in (:organisationNames) - """, nativeQuery = true) - List findTeamIdsByOrganisationNames(@Param("organisationNames") List organisationNames); - List findTeamsByName(String name); } diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/TeamPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/TeamPersistenceService.java index 45d3eba4ae..533949d848 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/TeamPersistenceService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/persistence/TeamPersistenceService.java @@ -20,14 +20,6 @@ public String getModelName() { return TEAM; } - public List findTeamIdsByOrganisationName(String organisationName) { - return findTeamIdsByOrganisationNames(List.of(organisationName)); - } - - public List findTeamIdsByOrganisationNames(List organisationNames) { - return getRepository().findTeamIdsByOrganisationNames(organisationNames); - } - public List findTeamsByName(String name) { return getRepository().findTeamsByName(name); } From 0b856edb287cd79c45e97e4ee0a8843766daef64 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 16 Oct 2024 10:53:59 +0200 Subject: [PATCH 04/24] remove tests from TeamPersistenceServiceIT which are already covered in PersistenceBaseTestIT --- .../persistence/TeamPersistenceServiceIT.java | 127 ++---------------- 1 file changed, 14 insertions(+), 113 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/TeamPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/TeamPersistenceServiceIT.java index f71c9daa79..f37812cb21 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/TeamPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/TeamPersistenceServiceIT.java @@ -1,29 +1,22 @@ package ch.puzzle.okr.service.persistence; -import ch.puzzle.okr.test.TestHelper; -import ch.puzzle.okr.dto.ErrorDto; -import ch.puzzle.okr.exception.OkrResponseStatusException; import ch.puzzle.okr.models.Team; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; -import org.junit.jupiter.api.AfterEach; +import ch.puzzle.okr.test.TestHelper; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.web.server.ResponseStatusException; import java.util.List; -import static ch.puzzle.okr.test.TestConstants.TEAM_PUZZLE; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; -import static org.springframework.http.HttpStatus.*; +import static ch.puzzle.okr.Constants.TEAM; +import static org.junit.jupiter.api.Assertions.assertEquals; @SpringIntegrationTest class TeamPersistenceServiceIT { - private static final String NEW_TEAM = "New Team"; - private Team createdTeam; @Autowired private TeamPersistenceService teamPersistenceService; @@ -32,112 +25,20 @@ void setUp() { TenantContext.setCurrentTenant(TestHelper.SCHEMA_PITC); } - @AfterEach - void tearDown() { - try { - if (createdTeam != null) { - teamPersistenceService.findById(createdTeam.getId()); - teamPersistenceService.deleteById(createdTeam.getId()); - } - } catch (ResponseStatusException ex) { - // created team already deleted - } finally { - createdTeam = null; - } - TenantContext.setCurrentTenant(null); - } - - @Test - void getTeamByIdShouldReturnTeam() throws ResponseStatusException { - Team team = teamPersistenceService.findById(5L); - - assertEquals(5L, team.getId()); - assertEquals(TEAM_PUZZLE, team.getName()); - } - - @Test - void getTeamByIdShouldThrowExceptionWhenTeamNotFound() { - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> teamPersistenceService.findById(321L)); - - List expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of("Team", "321"))); - - assertEquals(NOT_FOUND, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); - } - - @Test - void getTeamByIdShouldThrowExceptionWhenTeamIdIsNull() { - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> teamPersistenceService.findById(null)); - - List expectedErrors = List.of(new ErrorDto("ATTRIBUTE_NULL", List.of("ID", "Team"))); - - assertEquals(BAD_REQUEST, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); - } - - @Test - void shouldSaveANewTeam() { - Team team = Team.Builder.builder().withName("TestTeam").build(); - - createdTeam = teamPersistenceService.save(team); - assertNotNull(createdTeam.getId()); - assertEquals("TestTeam", createdTeam.getName()); - } - + // uses data from V100_0_0__TestData.sql + @DisplayName("findTeamsByName() should return teams with matching name") @Test - void shouldUpdateTeamProperly() { - Team team = Team.Builder.builder().withName(NEW_TEAM).build(); - createdTeam = teamPersistenceService.save(team); - createdTeam.setName("Updated Team"); - - Team returnedTeam = teamPersistenceService.save(createdTeam); - - assertEquals(createdTeam.getId(), returnedTeam.getId()); - assertEquals("Updated Team", returnedTeam.getName()); - } - - @Test - void updateTeamShouldThrowExceptionWhenAlreadyUpdated() { - Team team = Team.Builder.builder().withVersion(1).withName(NEW_TEAM).build(); - createdTeam = teamPersistenceService.save(team); - Team changedTeam = Team.Builder.builder().withId(createdTeam.getId()).withVersion(0).withName("Changed Team") - .build(); - - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> teamPersistenceService.save(changedTeam)); - List expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of("Team"))); - - assertEquals(UNPROCESSABLE_ENTITY, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); - } - - @Test - void shouldDeleteTeam() { - Team team = Team.Builder.builder().withName(NEW_TEAM).build(); - createdTeam = teamPersistenceService.save(team); - teamPersistenceService.deleteById(createdTeam.getId()); - - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> teamPersistenceService.findById(createdTeam.getId())); - - List expectedErrors = List - .of(ErrorDto.of("MODEL_WITH_ID_NOT_FOUND", List.of("Team", createdTeam.getId()))); + void findTeamsByNameShouldReturnTeamsWithMatchingName() { + List teams = teamPersistenceService.findTeamsByName("LoremIpsum"); - assertEquals(NOT_FOUND, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); + assertEquals(1, teams.size()); + assertEquals(6, teams.get(0).getId()); + assertEquals("LoremIpsum", teams.get(0).getName()); } + @DisplayName("getModelName() should return Team") @Test - void shouldFindTeamsByName() { - Team team = Team.Builder.builder().withName(NEW_TEAM).build(); - createdTeam = teamPersistenceService.save(team); - List teams = teamPersistenceService.findTeamsByName(NEW_TEAM); - assertThat(teams).contains(createdTeam); + void getModelNameShouldReturnTeam() { + assertEquals(TEAM, teamPersistenceService.getModelName()); } } \ No newline at end of file From 940e2ac58b9b2976f40620bcaa67b8ce73fb267e Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 16 Oct 2024 14:24:46 +0200 Subject: [PATCH 05/24] remove tests from UserPersistenceServiceIT which are already covered in PersistenceBaseTestIT and add additional tests --- .../ch/puzzle/okr/util/CollectionUtils.java | 14 ++ .../persistence/UserPersistenceServiceIT.java | 159 +++++++++++++----- 2 files changed, 133 insertions(+), 40 deletions(-) create mode 100644 backend/src/main/java/ch/puzzle/okr/util/CollectionUtils.java diff --git a/backend/src/main/java/ch/puzzle/okr/util/CollectionUtils.java b/backend/src/main/java/ch/puzzle/okr/util/CollectionUtils.java new file mode 100644 index 0000000000..5c3a002a28 --- /dev/null +++ b/backend/src/main/java/ch/puzzle/okr/util/CollectionUtils.java @@ -0,0 +1,14 @@ +package ch.puzzle.okr.util; + +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +public class CollectionUtils { + + public static List iterableToList(Iterable iterable) { + return StreamSupport // + .stream(iterable.spliterator(), false) // + .collect(Collectors.toList()); + } +} diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java index ca1dc10624..2377f4b772 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java @@ -3,24 +3,23 @@ import ch.puzzle.okr.models.User; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpStatus; -import org.springframework.web.server.ResponseStatusException; import java.util.List; +import java.util.Optional; +import static ch.puzzle.okr.Constants.USER; +import static ch.puzzle.okr.util.CollectionUtils.iterableToList; import static org.junit.jupiter.api.Assertions.*; @SpringIntegrationTest class UserPersistenceServiceIT { - private static final String EMAIL_ALICE = "wunderland@puzzle.ch"; - - User createdUser; + private User createdUser; @Autowired private UserPersistenceService userPersistenceService; @@ -39,63 +38,143 @@ void tearDown() { TenantContext.setCurrentTenant(null); } + @DisplayName("save() should save user with empty user team list") @Test - void shouldReturnAllUsersCorrect() throws ResponseStatusException { - List userList = userPersistenceService.findAll(); + void saveShouldSaveUserWithEmptyUserTeamList() { + // arrange + var newUser = User.Builder.builder() // + .withFirstname("Hans") // + .withLastname("Muster") // + .withEmail("muster@puzzle.ch") // + .withUserTeamList(List.of()).build(); + + // act + createdUser = userPersistenceService.save(newUser); + + // assert + assertNotNull(createdUser.getId()); + assertUser("Hans", "Muster", "muster@puzzle.ch", createdUser); + } - Assertions.assertThat(userList.size()).isGreaterThanOrEqualTo(7); + @DisplayName("save() should save user with null value for user team list") + @Test + void saveShouldSaveUserWithNullUserTeamList() { + // arrange + var newUser = User.Builder.builder() // + .withFirstname("Hans") // + .withLastname("Muster") // + .withEmail("muster@puzzle.ch") // + .withUserTeamList(null).build(); + + // act + createdUser = userPersistenceService.save(newUser); + + // assert + assertNotNull(createdUser.getId()); + assertUser("Hans", "Muster", "muster@puzzle.ch", createdUser); } + @DisplayName("saveAll() should save all users in the input list") @Test - void shouldReturnSingleUserWhenFindingOwnerByValidId() { - User returnedUser = userPersistenceService.findById(1L); + void saveAllShouldSaveAllUsersInTheInputList() { + // arrange + var newUser = User.Builder.builder() // + .withFirstname("Hans") // + .withLastname("Muster") // + .withEmail("muster@puzzle.ch") // + .build(); + + // act + var createdUsers = iterableToList(userPersistenceService.saveAll(List.of(newUser))); + + // assert + assertEquals(1, createdUsers.size()); + createdUser = createdUsers.get(0); - assertEquals(1L, returnedUser.getId()); - assertEquals("Paco", returnedUser.getFirstname()); - assertEquals("Eggimann", returnedUser.getLastname()); - assertEquals("peggimann@puzzle.ch", returnedUser.getEmail()); + assertNotNull(createdUser.getId()); + assertUser("Hans", "Muster", "muster@puzzle.ch", createdUser); } + @DisplayName("getOrCreateUser() should return single user when user found") @Test - void shouldThrowExceptionWhenFindingOwnerNotFound() { - ResponseStatusException exception = assertThrows(ResponseStatusException.class, - () -> userPersistenceService.findById(321L)); + void getOrCreateUserShouldReturnSingleUserWhenUserFound() { + // arrange + var existingUser = User.Builder.builder().withEmail("wunderland@puzzle.ch").build(); - assertEquals(HttpStatus.NOT_FOUND, exception.getStatusCode()); - assertEquals("MODEL_WITH_ID_NOT_FOUND", exception.getReason()); + // act + var returnedUser = userPersistenceService.getOrCreateUser(existingUser); + + // assert + assertUser(11L, "Alice", "Wunderland", "wunderland@puzzle.ch", returnedUser); } + @DisplayName("getOrCreateUser() should return saved user when user not found") @Test - void shouldThrowExceptionWhenFindingOwnerWithNullId() { - ResponseStatusException exception = assertThrows(ResponseStatusException.class, - () -> userPersistenceService.findById(null)); + void getOrCreateUserShouldReturnSavedUserWhenUserNotFound() { + // arrange + var newUser = User.Builder.builder() // + .withId(null) // + .withFirstname("firstname") // + .withLastname("lastname") // + .withEmail("lastname@puzzle.ch") // + .build(); + + // act + createdUser = userPersistenceService.getOrCreateUser(newUser); - assertEquals(HttpStatus.BAD_REQUEST, exception.getStatusCode()); - assertEquals("ATTRIBUTE_NULL", exception.getReason()); + // assert + assertNotNull(createdUser.getId()); + assertUser("firstname", "lastname", "lastname@puzzle.ch", createdUser); } + // uses data from V100_0_0__TestData.sql + @DisplayName("findByEmail() should return user if email is found") @Test - void getOrCreateUserShouldReturnSingleUserWhenUserFound() { - User existingUser = User.Builder.builder().withEmail(EMAIL_ALICE).build(); + void findByEmailShouldReturnUserIfEmailIsFound() { + Optional user = userPersistenceService.findByEmail("gl@gl.com"); - User returnedUser = userPersistenceService.getOrCreateUser(existingUser); + assertTrue(user.isPresent()); + assertEquals("Jaya", user.get().getFirstname()); + assertEquals("Norris", user.get().getLastname()); + } - assertEquals(11L, returnedUser.getId()); - assertEquals("Alice", returnedUser.getFirstname()); - assertEquals("Wunderland", returnedUser.getLastname()); - assertEquals("wunderland@puzzle.ch", returnedUser.getEmail()); + @DisplayName("findByEmail() should return empty optional if email is not found") + @Test + void findByEmailShouldReturnEmptyOptionalIfEmailIsNotFound() { + assertTrue(userPersistenceService.findByEmail("not_valid@gl.com").isEmpty()); } + @DisplayName("findByEmail() should return empty optional if email is null") @Test - void getOrCreateUserShouldReturnSavedUserWhenUserNotFound() { - User newUser = User.Builder.builder().withId(null).withFirstname("firstname").withLastname("lastname") - .withEmail("lastname@puzzle.ch").build(); + void findByEmailShouldReturnEmptyOptionalIfEmailIsNull() { + assertTrue(userPersistenceService.findByEmail(null).isEmpty()); + } - createdUser = userPersistenceService.getOrCreateUser(newUser); + // uses data from V100_0_0__TestData.sql + @DisplayName("findAllOkrChampions() should return all okr champions") + @Test + void findAllOkrChampionsShouldReturnAllOkrChampions() { + var allOkrChampions = userPersistenceService.findAllOkrChampions(); - assertNotNull(createdUser.getId()); - assertEquals("firstname", createdUser.getFirstname()); - assertEquals("lastname", createdUser.getLastname()); - assertEquals("lastname@puzzle.ch", createdUser.getEmail()); + assertEquals(1, allOkrChampions.size()); + assertEquals("Jaya", allOkrChampions.get(0).getFirstname()); + assertEquals("Norris", allOkrChampions.get(0).getLastname()); + } + + @DisplayName("getModelName() should return user") + @Test + void getModelNameShouldReturnUser() { + assertEquals(USER, userPersistenceService.getModelName()); + } + + private void assertUser(Long id, String firstName, String lastName, String email, User currentUser) { + assertEquals(id, currentUser.getId()); + assertUser(firstName, lastName, email, currentUser); + } + + private void assertUser(String firstName, String lastName, String email, User currentUser) { + assertEquals(firstName, currentUser.getFirstname()); + assertEquals(lastName, currentUser.getLastname()); + assertEquals(email, currentUser.getEmail()); } } \ No newline at end of file From 61e044a493fb6f793f4035bca569bdb2ffa1a281 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 17 Oct 2024 06:49:45 +0200 Subject: [PATCH 06/24] remove tests from CheckInPersistenceServiceIT which are already covered in PersistenceBaseTestIT and add additional test --- .../CheckInPersistenceServiceIT.java | 144 +++++------------- 1 file changed, 40 insertions(+), 104 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/CheckInPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/CheckInPersistenceServiceIT.java index 419971e392..0837b5f996 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/CheckInPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/CheckInPersistenceServiceIT.java @@ -1,52 +1,32 @@ package ch.puzzle.okr.service.persistence; -import ch.puzzle.okr.test.TestHelper; -import ch.puzzle.okr.dto.ErrorDto; -import ch.puzzle.okr.exception.OkrResponseStatusException; -import ch.puzzle.okr.models.Objective; -import ch.puzzle.okr.models.User; import ch.puzzle.okr.models.checkin.CheckIn; -import ch.puzzle.okr.models.checkin.CheckInMetric; -import ch.puzzle.okr.models.keyresult.KeyResultMetric; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; +import ch.puzzle.okr.test.TestHelper; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.web.server.ResponseStatusException; -import java.time.LocalDateTime; import java.util.List; import java.util.Objects; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; -import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; +import static ch.puzzle.okr.Constants.CHECK_IN; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; @SpringIntegrationTest class CheckInPersistenceServiceIT { - CheckIn createdCheckIn; + + private static final long KEY_RESULT_ID = 7L; @Autowired private CheckInPersistenceService checkInPersistenceService; - private static CheckIn createCheckIn(Long id) { - return createCheckIn(id, 1); - } - - private static final String UPDATED_CHECKIN = "Updated CheckIn"; - - private static CheckIn createCheckIn(Long id, int version) { - return CheckInMetric.Builder.builder().withValue(30D).withId(id).withVersion(version) - .withCreatedBy(User.Builder.builder().withId(1L).withFirstname("Frank").build()) - .withCreatedOn(LocalDateTime.MAX) - .withKeyResult(KeyResultMetric.Builder.builder().withBaseline(1.0).withStretchGoal(13.0).withId(8L) - .withObjective(Objective.Builder.builder().withId(1L).build()).build()) - .withChangeInfo("ChangeInfo").withInitiatives("Initiatives").withModifiedOn(LocalDateTime.MAX) - .withConfidence(5).build(); - } - @BeforeEach void setUp() { TenantContext.setCurrentTenant(TestHelper.SCHEMA_PITC); @@ -54,97 +34,53 @@ void setUp() { @AfterEach void tearDown() { - try { - if (createdCheckIn != null) { - checkInPersistenceService.findById(createdCheckIn.getId()); - checkInPersistenceService.deleteById(createdCheckIn.getId()); - } - } catch (ResponseStatusException ex) { - // created CheckIn already deleted - } finally { - createdCheckIn = null; - } TenantContext.setCurrentTenant(null); } + // uses data from V100_0_0__TestData.sql + @DisplayName("getCheckInsByKeyResultIdOrderByCheckInDate() should get checkIns by keyResultId and order them by date desc") @Test - void saveCheckInShouldSaveNewCheckIn() { - CheckIn checkIn = createCheckIn(null); - - createdCheckIn = checkInPersistenceService.save(checkIn); - - assertNotNull(createdCheckIn.getId()); - assertEquals(checkIn.getModifiedOn(), createdCheckIn.getModifiedOn()); - assertEquals(((CheckInMetric) checkIn).getValue(), ((CheckInMetric) createdCheckIn).getValue()); - assertEquals(checkIn.getCreatedBy(), createdCheckIn.getCreatedBy()); - assertEquals(checkIn.getCreatedOn(), createdCheckIn.getCreatedOn()); - assertEquals(checkIn.getInitiatives(), createdCheckIn.getInitiatives()); - assertEquals(checkIn.getChangeInfo(), createdCheckIn.getChangeInfo()); + void getCheckInsByKeyResultIdOrderByCheckInDateShouldGetCheckInsByKeyResultIdAndOrderThemByDateDesc() { + // act + List checkIns = checkInPersistenceService + .getCheckInsByKeyResultIdOrderByCheckInDateDesc(KEY_RESULT_ID); + + // assert + assertThat(2, greaterThanOrEqualTo(checkIns.size())); + CheckIn firstCheckIn = checkIns.get(0); + CheckIn lastCheckIn = checkIns.get(checkIns.size() - 1); + assertFirstIsCreatedAfterSecond(firstCheckIn, lastCheckIn); } - @Test - void updateKeyResultShouldUpdateKeyResult() { - createdCheckIn = checkInPersistenceService.save(createCheckIn(null)); - CheckIn updateCheckIn = createCheckIn(createdCheckIn.getId(), createdCheckIn.getVersion()); - updateCheckIn.setChangeInfo(UPDATED_CHECKIN); - - CheckIn updatedCheckIn = checkInPersistenceService.save(updateCheckIn); - - assertEquals(createdCheckIn.getId(), updatedCheckIn.getId()); - assertEquals(createdCheckIn.getVersion() + 1, updatedCheckIn.getVersion()); - assertEquals(UPDATED_CHECKIN, updatedCheckIn.getChangeInfo()); + private void assertFirstIsCreatedAfterSecond(CheckIn first, CheckIn second) { + assertTrue(first.getCreatedOn().isAfter(second.getCreatedOn())); } + // uses data from V100_0_0__TestData.sql + @DisplayName("getLastCheckInOfKeyResult() should get last checkIn of keyResult") @Test - void updateKeyResultShouldThrowExceptionWhenAlreadyUpdated() { - createdCheckIn = checkInPersistenceService.save(createCheckIn(null)); - CheckIn updateCheckIn = createCheckIn(createdCheckIn.getId(), 0); - updateCheckIn.setChangeInfo(UPDATED_CHECKIN); + void getLastCheckInOfKeyResultShouldGetLastCheckInOfKeyResult() { + // act + var lastCheckIn = checkInPersistenceService.getLastCheckInOfKeyResult(KEY_RESULT_ID); - OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, - () -> checkInPersistenceService.save(updateCheckIn)); - - List expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of("Check-in"))); - - assertEquals(UNPROCESSABLE_ENTITY, exception.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(exception.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason())); + // assert + var allCheckins = checkInPersistenceService.getCheckInsByKeyResultIdOrderByCheckInDateDesc(KEY_RESULT_ID); + assertLastIsCreatedAfterAllOtherCheckIns(lastCheckIn, allCheckins); } - @Test - void getAllCheckInShouldReturnListOfAllCheckIns() { - List checkIns = checkInPersistenceService.findAll(); - - assertEquals(19, checkIns.size()); - } - - @Test - void getCheckInByIdShouldReturnCheckInProperly() { - CheckIn checkIn = checkInPersistenceService.findById(20L); - - assertEquals(20L, checkIn.getId()); - assertEquals(0.5, ((CheckInMetric) checkIn).getValue(), 0.01); - assertEquals( - "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore " - + "magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores ", - checkIn.getChangeInfo()); + private void assertLastIsCreatedAfterAllOtherCheckIns(CheckIn last, List allCheckIns) { + for (CheckIn checkInLoop : allCheckIns) { + if (!Objects.equals(checkInLoop.getId(), last.getId())) { + assertTrue(last.getCreatedOn().isAfter(checkInLoop.getCreatedOn())); + } + } } + @DisplayName("getModelName() should return checkIn") @Test - void shouldGetCheckInsByKeyResultIdAndOrderThemByDateDesc() { - List checkIns = checkInPersistenceService.getCheckInsByKeyResultIdOrderByCheckInDateDesc(7L); - assertTrue(checkIns.get(0).getCreatedOn().isAfter(checkIns.get(checkIns.size() - 1).getCreatedOn())); + void getModelNameShouldReturnCheckIn() { + assertEquals(CHECK_IN, checkInPersistenceService.getModelName()); } - @Test - void shouldGetLastCheckInOfKeyResult() { - CheckIn checkIn = checkInPersistenceService.getLastCheckInOfKeyResult(7L); - List checkInList = checkInPersistenceService.getCheckInsByKeyResultIdOrderByCheckInDateDesc(7L); - for (CheckIn checkInLoop : checkInList) { - if (!Objects.equals(checkInLoop.getId(), checkIn.getId())) { - assertTrue(checkIn.getCreatedOn().isAfter(checkInLoop.getCreatedOn())); - } - } - } } From 481a33bb7625f3d273b18e6867a2196ba4be8a7e Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 17 Oct 2024 13:54:54 +0200 Subject: [PATCH 07/24] umbau ObjectivePersistenceServiceIT: remove tests which are already covered in PersistenceBaseTestIT, add additional test, make the tests readable --- .../ObjectivePersistenceService.java | 11 + .../ObjectivePersistenceServiceIT.java | 333 +++++++++--------- 2 files changed, 185 insertions(+), 159 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceService.java index 675d8ec249..9e7a2bcd34 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceService.java @@ -40,6 +40,17 @@ public String getModelName() { return OBJECTIVE; } + /** + * Get the number of Objectives of a Team in a Quarter. The underling sql looks like "select count(*) from objective + * where teamId = team.id and quarterId = quarter.id." + * + * @param team + * Team + * @param quarter + * Quarter + * + * @return number of Objectives of team in quarter + */ public Integer countByTeamAndQuarter(Team team, Quarter quarter) { return getRepository().countByTeamAndQuarter(team, quarter); } diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceServiceIT.java index ccc6c3a8a0..9708e9d779 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/ObjectivePersistenceServiceIT.java @@ -2,56 +2,68 @@ import ch.puzzle.okr.dto.ErrorDto; import ch.puzzle.okr.exception.OkrResponseStatusException; -import ch.puzzle.okr.models.*; +import ch.puzzle.okr.models.Objective; +import ch.puzzle.okr.models.Quarter; +import ch.puzzle.okr.models.Team; import ch.puzzle.okr.models.authorization.AuthorizationUser; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; import ch.puzzle.okr.test.TestHelper; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.web.server.ResponseStatusException; +import org.springframework.http.HttpStatus; -import java.time.LocalDateTime; import java.util.List; +import java.util.stream.Stream; -import static ch.puzzle.okr.test.TestConstants.GJ_FOR_TESTS_QUARTER_ID; +import static ch.puzzle.okr.exception.OkrResponseStatusException.of; import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; -import static org.springframework.http.HttpStatus.*; +import static org.junit.jupiter.params.provider.Arguments.arguments; +import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static org.springframework.http.HttpStatus.UNAUTHORIZED; +// tests are using data from V100_0_0__TestData.sql @SpringIntegrationTest class ObjectivePersistenceServiceIT { - private static final String REASON = "not authorized to read objective"; - private static final OkrResponseStatusException exception = OkrResponseStatusException.of(REASON); - private static final String HIGHER_CUSTOMER_HAPPINESS = "Wir wollen die Kundenzufriedenheit steigern"; - private static final String MODEL_WITH_ID_NOT_FOUND = "MODEL_WITH_ID_NOT_FOUND"; + private static final long INVALID_OBJECTIVE_ID = 321L; + private static final long INVALID_KEY_RESULT_ID = 321L; + private static final long INVALID_CHECK_IN_ID = 321L; + private static final long INVALID_TEAM_ID = 321L; + private static final long INVALID_QUARTER_ID = 12L; + + private static final long ID_OF_OBJECTIVE_3 = 3L; + private static final long ID_OF_OBJECTIVE_8 = 8L; + private static final long ID_OF_OBJECTIVE_9 = 9L; + private static final long ID_OF_OBJECTIVE_10 = 10L; + + private static final String TITLE_OF_OBJECTIVE_3 = "Wir wollen die Kundenzufriedenheit steigern"; + private static final String TITLE_OF_OBJECTIVE_8 = "consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua"; + private static final String TITLE_OF_OBJECTIVE_9 = "At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet."; + private static final String TITLE_OF_OBJECTIVE_10 = "should not appear on staging, no sea takimata sanctus est Lorem ipsum dolor sit amet."; + + private static final long ID_OF_KEY_RESULT_5 = 5L; + private static final long ID_OF_CHECK_IN_7 = 7L; + private static final long ID_OF_TEAM_6 = 6L; + + private static final String REASON_UNAUTHORIZED = "not authorized to read objective"; + private static final OkrResponseStatusException NO_RESULT_EXCEPTION = of(REASON_UNAUTHORIZED); + private static final String OBJECTIVE = "Objective"; private static final String ATTRIBUTE_NULL = "ATTRIBUTE_NULL"; + private static final long CURRENT_QUARTER_ID = 2L; + private final AuthorizationUser authorizationUser = defaultAuthorizationUser(); - private Objective createdObjective; @Autowired private ObjectivePersistenceService objectivePersistenceService; - @Autowired - private TeamPersistenceService teamPersistenceService; - @Autowired - private QuarterPersistenceService quarterPersistenceService; - - private static Objective createObjective(Long id) { - return createObjective(id, 1); - } - - private static Objective createObjective(Long id, int version) { - return Objective.Builder.builder().withId(id).withVersion(version).withTitle("title") - .withCreatedBy(User.Builder.builder().withId(1L).build()) - .withTeam(Team.Builder.builder().withId(5L).build()) - .withQuarter(Quarter.Builder.builder().withId(GJ_FOR_TESTS_QUARTER_ID).build()) - .withDescription("This is our description").withState(State.DRAFT).withCreatedOn(LocalDateTime.MAX) - .withModifiedOn(LocalDateTime.MAX).withModifiedBy(User.Builder.builder().withId(1L).build()).build(); - } @BeforeEach void setUp() { @@ -60,206 +72,209 @@ void setUp() { @AfterEach void tearDown() { - try { - if (createdObjective != null) { - objectivePersistenceService.findById(createdObjective.getId()); - objectivePersistenceService.deleteById(createdObjective.getId()); - } - } catch (ResponseStatusException ex) { - // created key result already deleted - } finally { - createdObjective = null; - } TenantContext.setCurrentTenant(null); } - @Test - void findAllShouldReturnListOfObjectives() { - List objectives = objectivePersistenceService.findAll(); - - assertEquals(7, objectives.size()); - } - + @DisplayName("findObjectiveById() should return objective properly") @Test void findObjectiveByIdShouldReturnObjectiveProperly() { - Objective objective = objectivePersistenceService.findObjectiveById(3L, authorizationUser, exception); + // act + var objective = objectivePersistenceService.findObjectiveById(ID_OF_OBJECTIVE_3, authorizationUser, + NO_RESULT_EXCEPTION); - assertEquals(3L, objective.getId()); - assertEquals(HIGHER_CUSTOMER_HAPPINESS, objective.getTitle()); + // assert + assertObjective(ID_OF_OBJECTIVE_3, TITLE_OF_OBJECTIVE_3, objective); } + @DisplayName("findObjectiveById() should throw exception when objective not found") @Test void findObjectiveByIdShouldThrowExceptionWhenObjectiveNotFound() { - ResponseStatusException findObjectiveException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveById(321L, authorizationUser, - ObjectivePersistenceServiceIT.exception)); + // act + var exception = assertThrows(OkrResponseStatusException.class, () -> objectivePersistenceService + .findObjectiveById(INVALID_OBJECTIVE_ID, authorizationUser, NO_RESULT_EXCEPTION)); - assertEquals(UNAUTHORIZED, findObjectiveException.getStatusCode()); - assertEquals(REASON, findObjectiveException.getReason()); + // assert + var expectedErrors = List.of(new ErrorDto(REASON_UNAUTHORIZED, List.of())); + assertResponseStatusException(UNAUTHORIZED, expectedErrors, exception); } + @DisplayName("findObjectiveById() should throw exception when objective id is null") @Test void findObjectiveByIdShouldThrowExceptionWhenObjectiveIdIsNull() { - OkrResponseStatusException findObjectiveException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveById(null, authorizationUser, - ObjectivePersistenceServiceIT.exception)); - - List expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); + // act + var exception = assertThrows(OkrResponseStatusException.class, + () -> objectivePersistenceService.findObjectiveById(null, authorizationUser, NO_RESULT_EXCEPTION)); - assertEquals(BAD_REQUEST, findObjectiveException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(findObjectiveException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(findObjectiveException.getReason())); + // assert + var expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); + assertResponseStatusException(BAD_REQUEST, expectedErrors, exception); } + @DisplayName("findObjectiveByKeyResultId() should return objective properly") @Test void findObjectiveByKeyResultIdShouldReturnObjectiveProperly() { - Objective objective = objectivePersistenceService.findObjectiveByKeyResultId(5L, authorizationUser, exception); + // act + var objective = objectivePersistenceService.findObjectiveByKeyResultId(ID_OF_KEY_RESULT_5, authorizationUser, + NO_RESULT_EXCEPTION); - assertEquals(3L, objective.getId()); - assertEquals(HIGHER_CUSTOMER_HAPPINESS, objective.getTitle()); + // assert + assertObjective(ID_OF_OBJECTIVE_3, TITLE_OF_OBJECTIVE_3, objective); } + @DisplayName("findObjectiveByKeyResultId() should throw exception when objective not found") @Test void findObjectiveByKeyResultIdShouldThrowExceptionWhenObjectiveNotFound() { - ResponseStatusException objectiveByKeyResultException = assertThrows(ResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveByKeyResultId(321L, authorizationUser, - ObjectivePersistenceServiceIT.exception)); + // act + var exception = assertThrows(OkrResponseStatusException.class, () -> objectivePersistenceService + .findObjectiveByKeyResultId(INVALID_KEY_RESULT_ID, authorizationUser, NO_RESULT_EXCEPTION)); - assertEquals(UNAUTHORIZED, objectiveByKeyResultException.getStatusCode()); - assertEquals(REASON, objectiveByKeyResultException.getReason()); + // assert + var expectedErrors = List.of(new ErrorDto(REASON_UNAUTHORIZED, List.of())); + assertResponseStatusException(UNAUTHORIZED, expectedErrors, exception); } + @DisplayName("findObjectiveByKeyResultId() should throw exception when objective id is null") @Test void findObjectiveByKeyResultIdShouldThrowExceptionWhenObjectiveIdIsNull() { - OkrResponseStatusException objectiveByKeyResultException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveByKeyResultId(null, authorizationUser, - ObjectivePersistenceServiceIT.exception)); + // act + var exception = assertThrows(OkrResponseStatusException.class, () -> objectivePersistenceService + .findObjectiveByKeyResultId(null, authorizationUser, NO_RESULT_EXCEPTION)); - List expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); - - assertEquals(BAD_REQUEST, objectiveByKeyResultException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(objectiveByKeyResultException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(objectiveByKeyResultException.getReason())); + // assert + var expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); + assertResponseStatusException(BAD_REQUEST, expectedErrors, exception); } + @DisplayName("findObjectiveByCheckInId() should return objective properly") @Test void findObjectiveByCheckInIdShouldReturnObjectiveProperly() { - Objective objective = objectivePersistenceService.findObjectiveByCheckInId(7L, authorizationUser, exception); + // act + var objective = objectivePersistenceService.findObjectiveByCheckInId(ID_OF_CHECK_IN_7, authorizationUser, + NO_RESULT_EXCEPTION); - assertEquals(3L, objective.getId()); - assertEquals(HIGHER_CUSTOMER_HAPPINESS, objective.getTitle()); + // assert + assertObjective(ID_OF_OBJECTIVE_3, TITLE_OF_OBJECTIVE_3, objective); } + @DisplayName("findObjectiveByCheckInId() should throw exception when objective not found") @Test void findObjectiveByCheckInIdShouldThrowExceptionWhenObjectiveNotFound() { - ResponseStatusException objectiveByCheckInException = assertThrows(ResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveByCheckInId(321L, authorizationUser, - ObjectivePersistenceServiceIT.exception)); + // act + var exception = assertThrows(OkrResponseStatusException.class, () -> objectivePersistenceService + .findObjectiveByCheckInId(INVALID_CHECK_IN_ID, authorizationUser, NO_RESULT_EXCEPTION)); - assertEquals(UNAUTHORIZED, objectiveByCheckInException.getStatusCode()); - assertEquals(REASON, objectiveByCheckInException.getReason()); + // assert + var expectedErrors = List.of(new ErrorDto(REASON_UNAUTHORIZED, List.of())); + assertResponseStatusException(UNAUTHORIZED, expectedErrors, exception); } + @DisplayName("findObjectiveByCheckInId() should throw exception when objective id is null") @Test void findObjectiveByCheckInIdShouldThrowExceptionWhenObjectiveIdIsNull() { - OkrResponseStatusException objectiveByCheckInException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.findObjectiveByCheckInId(null, authorizationUser, - ObjectivePersistenceServiceIT.exception)); - - List expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); + // act + var exception = assertThrows(OkrResponseStatusException.class, () -> objectivePersistenceService + .findObjectiveByCheckInId(null, authorizationUser, ObjectivePersistenceServiceIT.NO_RESULT_EXCEPTION)); - assertEquals(BAD_REQUEST, objectiveByCheckInException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(objectiveByCheckInException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(objectiveByCheckInException.getReason())); + // assert + var expectedErrors = List.of(new ErrorDto(ATTRIBUTE_NULL, List.of("ID", OBJECTIVE))); + assertResponseStatusException(BAD_REQUEST, expectedErrors, exception); } + @DisplayName("findObjectiveByTeamId() should return objectives of team properly") @Test - void saveObjectiveShouldSaveNewObjective() { - Objective objective = createObjective(null); - - createdObjective = objectivePersistenceService.save(objective); - - assertNotNull(createdObjective.getId()); - assertEquals(objective.getDescription(), createdObjective.getDescription()); - assertEquals(objective.getDescription(), createdObjective.getDescription()); - assertEquals(objective.getModifiedOn(), createdObjective.getModifiedOn()); + void findObjectiveByTeamIdShouldReturnObjectivesOfTeamProperly() { + // act + var objectives = objectivePersistenceService.findObjectiveByTeamId(ID_OF_TEAM_6); + + // assert + assertEquals(3, objectives.size()); + assertObjective(ID_OF_OBJECTIVE_8, TITLE_OF_OBJECTIVE_8, objectives.get(0)); + assertObjective(ID_OF_OBJECTIVE_9, TITLE_OF_OBJECTIVE_9, objectives.get(1)); + assertObjective(ID_OF_OBJECTIVE_10, TITLE_OF_OBJECTIVE_10, objectives.get(2)); } + @DisplayName("findObjectiveByTeamId() should return empty list when objective not found") @Test - void updateObjectiveShouldUpdateObjective() { - createdObjective = objectivePersistenceService.save(createObjective(null)); - Objective updateObjective = createObjective(createdObjective.getId(), createdObjective.getVersion()); - updateObjective.setState(State.ONGOING); - - Objective updatedObjective = objectivePersistenceService.save(updateObjective); + void findObjectiveByTeamIdShouldReturnEmptyListWhenObjectiveNotFound() { + // act + var objectives = objectivePersistenceService.findObjectiveByTeamId(INVALID_TEAM_ID); - assertEquals(createdObjective.getId(), updatedObjective.getId()); - assertEquals(State.ONGOING, updatedObjective.getState()); + // assert + assertTrue(objectives.isEmpty()); } + @DisplayName("findObjectiveByTeamId() should return empty list when objective id is null") @Test - void updateObjectiveShouldThrowExceptionWhenAlreadyUpdated() { - createdObjective = objectivePersistenceService.save(createObjective(null)); - Objective updateObjective = createObjective(createdObjective.getId(), 0); - updateObjective.setState(State.ONGOING); - - OkrResponseStatusException objectiveSaveException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.save(updateObjective)); - List expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of(OBJECTIVE))); - - assertEquals(UNPROCESSABLE_ENTITY, objectiveSaveException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(objectiveSaveException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(objectiveSaveException.getReason())); - } - - @Test - void deleteObjectiveShouldThrowExceptionWhenKeyResultNotFound() { - Objective objective = createObjective(321L); - createdObjective = objectivePersistenceService.save(objective); - objectivePersistenceService.deleteById(createdObjective.getId()); - - Long objectiveId = createdObjective.getId(); - OkrResponseStatusException findObjectiveException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.findById(objectiveId)); + void findObjectiveByTeamIdShouldReturnEmptyListWhenObjectiveIdIsNull() { + // act + var objectives = objectivePersistenceService.findObjectiveByTeamId(null); - List expectedErrors = List.of(new ErrorDto(MODEL_WITH_ID_NOT_FOUND, List.of(OBJECTIVE, "200"))); - - assertEquals(NOT_FOUND, findObjectiveException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(findObjectiveException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(findObjectiveException.getReason())); + // assert + assertTrue(objectives.isEmpty()); } + @DisplayName("countByTeamAndQuarter() should return number of objectives for current quarter") @Test - void countByTeamAndQuarterShouldThrowErrorIfQuarterDoesNotExist() { - Team teamId5 = teamPersistenceService.findById(5L); - OkrResponseStatusException countByTeamException = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.countByTeamAndQuarter(teamId5, - quarterPersistenceService.findById(12L))); - - List expectedErrors = List.of(new ErrorDto(MODEL_WITH_ID_NOT_FOUND, List.of("Quarter", "12"))); + void countByTeamAndQuarterShouldReturnNumberOfObjectivesForCurrentQuarter() { + // arrange: there are 3 objectives for the current quarter (id 2) for team with id 6 + var team = Team.Builder.builder().withId(ID_OF_TEAM_6).build(); + var quarter = Quarter.Builder.builder().withId(CURRENT_QUARTER_ID).build(); - assertEquals(NOT_FOUND, countByTeamException.getStatusCode()); - assertThat(expectedErrors).hasSameElementsAs(countByTeamException.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(countByTeamException.getReason())); + // act + var count = objectivePersistenceService.countByTeamAndQuarter(team, quarter); - Quarter quarterId2 = quarterPersistenceService.findById(2L); - OkrResponseStatusException exceptionTeam = assertThrows(OkrResponseStatusException.class, - () -> objectivePersistenceService.countByTeamAndQuarter(teamPersistenceService.findById(500L), - quarterId2)); + // assert + assertEquals(3, count); + } - List expectedErrorsTeam = List.of(new ErrorDto(MODEL_WITH_ID_NOT_FOUND, List.of("Team", "500"))); + @DisplayName("countByTeamAndQuarter() should return zero when team or quarter is not valid or null") + @ParameterizedTest + @MethodSource("invalidTeamsAndQuarters") + void countByTeamAndQuarterShouldReturnZeroWhenTeamOrQuarterIsNotValidOrNull(Team team, Quarter quarter) { + // act + var count = objectivePersistenceService.countByTeamAndQuarter(team, quarter); - assertEquals(NOT_FOUND, exceptionTeam.getStatusCode()); - assertThat(expectedErrorsTeam).hasSameElementsAs(exceptionTeam.getErrors()); - assertTrue(TestHelper.getAllErrorKeys(expectedErrorsTeam).contains(exceptionTeam.getReason())); + // assert + assertEquals(0, count); + } + private static Stream invalidTeamsAndQuarters() { + var validTeam = Team.Builder.builder().withId(ID_OF_TEAM_6).build(); + var invalidTeam = Team.Builder.builder().withId(INVALID_TEAM_ID).build(); + var validQuarter = Quarter.Builder.builder().withId(CURRENT_QUARTER_ID).build(); + var invalidQuarter = Quarter.Builder.builder().withId(INVALID_QUARTER_ID).build(); + + return Stream.of( + // valid team + invalid quarter + arguments(validTeam, invalidQuarter), + // valid team + null quarter + arguments(validTeam, null), + // invalid team + valid quarter + arguments(invalidTeam, validQuarter), + // invalid team + null quarter + arguments(null, validQuarter), + // invalid team + invalid quarter + arguments(invalidTeam, invalidQuarter), + // null team + null quarter + arguments(null, null)); } + @DisplayName("getModelName() should return Objective") @Test - void countByTeamAndQuarterShouldReturnCountValue() { - Integer count = objectivePersistenceService.countByTeamAndQuarter(Team.Builder.builder().withId(5L).build(), - Quarter.Builder.builder().withId(2L).build()); + void getModelNameShouldReturnObjective() { + assertEquals(OBJECTIVE, objectivePersistenceService.getModelName()); + } + + private void assertResponseStatusException(HttpStatus expectedStatus, List expectedErrors, + OkrResponseStatusException currentException) { + assertEquals(expectedStatus, currentException.getStatusCode()); + assertThat(expectedErrors).hasSameElementsAs(currentException.getErrors()); + assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(currentException.getReason())); + } - assertEquals(2, count); + private void assertObjective(Long expectedId, String expectedTitle, Objective currentObjective) { + assertEquals(expectedId, currentObjective.getId()); + assertEquals(expectedTitle, currentObjective.getTitle()); } + } From c9c73497d71a16057644c097ecaaa79539518d5c Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 21 Oct 2024 12:25:11 +0200 Subject: [PATCH 08/24] tests for AuthorizationCriteria --- .../TeamAuthorizationServiceTest.java | 2 +- .../AuthorizationCriteriaTest.java | 104 ++++++++++++++++++ .../java/ch/puzzle/okr/test/TestHelper.java | 19 ++-- 3 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/TeamAuthorizationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/TeamAuthorizationServiceTest.java index 09243f4aac..6fb9ec4f34 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/TeamAuthorizationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/TeamAuthorizationServiceTest.java @@ -130,7 +130,7 @@ void getAllTeamsShouldReturnAllTeams(boolean isWriteable) { if (isWriteable) { when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(okrChampionUser); } else { - when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(userWithoutWriteAllRole()); + when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(defaultAuthorizationUser()); } when(teamBusinessService.getAllTeams(any())).thenReturn(teamList); diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java new file mode 100644 index 0000000000..d7e550b543 --- /dev/null +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java @@ -0,0 +1,104 @@ +package ch.puzzle.okr.service.persistence; + +import ch.puzzle.okr.models.Objective; +import ch.puzzle.okr.models.User; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.stream.Stream; + +import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser; +import static ch.puzzle.okr.test.TestHelper.mockAuthorizationUser; +import static org.junit.jupiter.api.Assertions.assertFalse; + +public class AuthorizationCriteriaTest { + + @DisplayName("appendObjective() should be successful with default authorization user") + @Test + void appendObjectiveShouldBeSuccessfulWithDefaultAuthorizationUser() { + // arrange + var criteria = new AuthorizationCriteria(); + + // act + var current = criteria.appendObjective(defaultAuthorizationUser()); + + // assert + var expected = " and ((o.state=:teamDraftState and o.team.id IN (:userTeamIds)) or o.state IN (:publishedStates))"; + Assertions.assertEquals(expected, current); + } + + @DisplayName("appendObjective() should be successful when user is okrChampion") + @Test + void appendObjectiveShouldBeSuccessfulWhenUserIsOkrChampion() { + // arrange + var user = User.Builder.builder() // + .withId(23L) // + .withFirstname("Hanna") // + .withLastname("muster") // + .withEmail("hanna.muster@example.com") // + .withOkrChampion(true) // + .build(); + var criteria = new AuthorizationCriteria(); + + // act + var current = criteria.appendObjective(mockAuthorizationUser(user)); + + // assert + var expected = " and (o.state=:allDraftState or o.state IN (:publishedStates))"; + Assertions.assertEquals(expected, current); + } + + @DisplayName("appendOverview() should be successful when team ids or objective query are empty") + @ParameterizedTest + @MethodSource("provideListAndString") + void appendOverviewShouldBeSuccessfulWhenTeamIdsOrObjectiveQueryAreEmpty(List teamIds, + String objectiveQuery) { + // arrange + var criteria = new AuthorizationCriteria(); + + // act + var current = criteria.appendOverview(teamIds, objectiveQuery, defaultAuthorizationUser()); + + // assert + var expected = "\n and ((o.objectiveState=:teamDraftState and o.overviewId.teamId IN (:userTeamIds)) or o.objectiveState IN (:publishedStates) or o.overviewId.objectiveId = -1)"; + Assertions.assertEquals(expected, current); + } + + private static Stream provideListAndString() { + return Stream.of( // + Arguments.of(List.of(), null), // + Arguments.of(List.of(), ""), // + Arguments.of(null, null), // + Arguments.of(null, "")); + } + + @DisplayName("appendOverview() should be successful when team ids and objective query are not empty") + @Test + void appendOverviewShouldBeSuccessfulWhenTeamIdsAndObjectiveQueryAreNotEmpty() { + // arrange + var criteria = new AuthorizationCriteria(); + var anyTeamIds = List.of(99L); + var anyNonEmptyString = "OBJECTIVEQUERY"; + var startingNewLine = "\n"; + var singleSpace = " "; + + // act + var current = criteria.appendOverview(anyTeamIds, anyNonEmptyString, defaultAuthorizationUser()); + + // assert + var expected = startingNewLine + singleSpace + + """ + and o.overviewId.teamId in (:teamIds) + and lower(coalesce(o.objectiveTitle, '')) like lower(concat('%',:objectiveQuery,'%')) + and ((o.objectiveState=:teamDraftState and o.overviewId.teamId IN (:userTeamIds)) or o.objectiveState IN (:publishedStates) or o.overviewId.objectiveId = -1)"""; + + Assertions.assertEquals(expected, current); + assertFalse(current.contains(anyNonEmptyString)); + } + +} diff --git a/backend/src/test/java/ch/puzzle/okr/test/TestHelper.java b/backend/src/test/java/ch/puzzle/okr/test/TestHelper.java index 75d53c1db4..86bb30b3f7 100644 --- a/backend/src/test/java/ch/puzzle/okr/test/TestHelper.java +++ b/backend/src/test/java/ch/puzzle/okr/test/TestHelper.java @@ -56,19 +56,22 @@ public static UserTeam defaultUserTeam(Long id, User user) { } public static AuthorizationUser defaultAuthorizationUser() { - return mockAuthorizationUser(1L, FIRSTNAME, LASTNAME, EMAIL); - } - - public static AuthorizationUser userWithoutWriteAllRole() { - return mockAuthorizationUser(1L, FIRSTNAME, LASTNAME, EMAIL); + return mockAuthorizationUser(1L, FIRSTNAME, LASTNAME, EMAIL, false); } public static AuthorizationUser mockAuthorizationUser(User user) { - return mockAuthorizationUser(user.getId(), user.getFirstname(), user.getLastname(), user.getEmail()); + return mockAuthorizationUser(user.getId(), user.getFirstname(), user.getLastname(), user.getEmail(), + user.isOkrChampion()); } - public static AuthorizationUser mockAuthorizationUser(Long id, String firstname, String lastname, String email) { - User user = User.Builder.builder().withId(id).withFirstname(firstname).withLastname(lastname).withEmail(email) + public static AuthorizationUser mockAuthorizationUser(Long id, String firstname, String lastname, String email, + boolean isOkrChampion) { + User user = User.Builder.builder() // + .withId(id) // + .withFirstname(firstname) // + .withLastname(lastname) // + .withEmail(email) // + .withOkrChampion(isOkrChampion) // .build(); user.setUserTeamList(List.of(defaultUserTeam(1L, user))); return new AuthorizationUser(user); From df612b35010a56697e0be5e27ef6f9b4f201738c Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 21 Oct 2024 12:40:47 +0200 Subject: [PATCH 09/24] tests for AuthorizationCriteria --- .../AuthorizationCriteriaParametersTest.java | 327 ++++++++++++++++++ .../AuthorizationCriteriaTest.java | 9 +- 2 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaParametersTest.java diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaParametersTest.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaParametersTest.java new file mode 100644 index 0000000000..7cff83afd8 --- /dev/null +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaParametersTest.java @@ -0,0 +1,327 @@ +package ch.puzzle.okr.service.persistence; + +import ch.puzzle.okr.models.Objective; +import ch.puzzle.okr.models.User; +import jakarta.persistence.*; +import org.apache.commons.lang3.NotImplementedException; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.*; +import java.util.stream.Stream; + +import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser; +import static ch.puzzle.okr.test.TestHelper.mockAuthorizationUser; +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class AuthorizationCriteriaParametersTest { + + @DisplayName("setParameters() should be successful with default authorization user") + @Test + void setParametersShouldBeSuccessfulWithDefaultAuthorizationUser() { + // arrange + var criteria = new AuthorizationCriteria(); + TypedQueryMock typedQueryMock = new TypedQueryMock<>(); + + // act + criteria.setParameters(typedQueryMock, defaultAuthorizationUser()); + + // assert + var expected = """ + teamDraftState, State=DRAFT + userTeamIds, ListN=[1] + publishedStates, ListN=[ONGOING, SUCCESSFUL, NOTSUCCESSFUL] + """; + + assertEquals(expected, typedQueryMock.getLog()); + } + + @DisplayName("setParameters() should be successful when user is okr champion") + @Test + void setParametersShouldBeSuccessfulWhenUserIsOkrChampion() { + // arrange + var user = User.Builder.builder() // + .withId(23L) // + .withFirstname("Hanna") // + .withLastname("muster") // + .withEmail("hanna.muster@example.com") // + .withOkrChampion(true) // + .build(); + var criteria = new AuthorizationCriteria(); + TypedQueryMock typedQueryMock = new TypedQueryMock<>(); + + // act + criteria.setParameters(typedQueryMock, mockAuthorizationUser(user)); + + // assert + var expected = """ + allDraftState, State=DRAFT + publishedStates, ListN=[ONGOING, SUCCESSFUL, NOTSUCCESSFUL] + """; + + assertEquals(expected, typedQueryMock.getLog()); + } + + @DisplayName("setParameters() should be successful when team ids or objective query are empty") + @ParameterizedTest + @MethodSource("provideListAndString") + void setParametersShouldBeSuccessfulWhenTeamIdsOrObjectiveQueryAreEmpty(List teamIds, String objectiveQuery) { + // arrange + var criteria = new AuthorizationCriteria(); + TypedQueryMock typedQueryMock = new TypedQueryMock<>(); + + // act + criteria.setParameters(typedQueryMock, teamIds, objectiveQuery, defaultAuthorizationUser()); + + // assert + var expected = """ + teamDraftState, State=DRAFT + userTeamIds, ListN=[1] + publishedStates, ListN=[ONGOING, SUCCESSFUL, NOTSUCCESSFUL] + """; + + assertEquals(expected, typedQueryMock.getLog()); + } + + private static Stream provideListAndString() { + return Stream.of( // + Arguments.of(List.of(), null), // + Arguments.of(List.of(), ""), // + Arguments.of(null, null), // + Arguments.of(null, "")); + } + + @DisplayName("setParameters() should be successful when team ids and objective query are not empty") + @Test + void setParametersShouldBeSuccessfulWhenTeamIdsAndObjectiveQueryAreNotEmpty() { + // arrange + TypedQueryMock typedQueryMock = new TypedQueryMock<>(); + var criteria = new AuthorizationCriteria(); + var anyTeamIds = List.of(99L); + var anyNonEmptyString = "OBJECTIVEQUERY"; + + // act + criteria.setParameters(typedQueryMock, anyTeamIds, anyNonEmptyString, defaultAuthorizationUser()); + + // assert + var expected = """ + teamIds, List12=[99] + objectiveQuery, String=OBJECTIVEQUERY + teamDraftState, State=DRAFT + userTeamIds, ListN=[1] + publishedStates, ListN=[ONGOING, SUCCESSFUL, NOTSUCCESSFUL] + """; + + assertEquals(expected, typedQueryMock.getLog()); + } + + // TypedQuery implementation for testing. The setParameterX() methods calls are logged in an internal StringBuilder + // which is return by getLog(). This log can be used for checking the internal state of the TypedQuery. All other + // methods are not implemented. + private static class TypedQueryMock implements TypedQuery { + + private final StringBuilder log = new StringBuilder(); + + public String getLog() { + return log.toString(); + } + + @Override + public TypedQuery setParameter(Parameter parameter, T t) { + log.append(parameter.getName()).append(", ") // + .append(t.getClass().getSimpleName()).append("=").append(t) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(Parameter parameter, Calendar calendar, + TemporalType temporalType) { + log.append(parameter.getName()).append(", ") // + .append(calendar.getTime()).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(Parameter parameter, Date date, TemporalType temporalType) { + log.append(parameter.getName()).append(", ") // + .append(date).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(String s, Object o) { + log.append(s).append(", ") // + .append(o.getClass().getSimpleName()).append("=").append(o) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(String s, Calendar calendar, TemporalType temporalType) { + log.append(s).append(", ") // + .append(calendar.getTime()).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(String s, Date date, TemporalType temporalType) { + log.append(s).append(", ") // + .append(date).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(int i, Object o) { + log.append(i).append(", ") // + .append(o.getClass().getSimpleName()).append("=").append(o) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(int i, Calendar calendar, TemporalType temporalType) { + log.append(i).append(", ") // + .append(calendar.getTime()).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public TypedQuery setParameter(int i, Date date, TemporalType temporalType) { + log.append(i).append(", ") // + .append(date).append(", ") // + .append(temporalType.name()) // + .append("\n"); + return null; + } + + @Override + public List getResultList() { + throw new NotImplementedException(); + } + + @Override + public Objective getSingleResult() { + throw new NotImplementedException(); + } + + @Override + public int executeUpdate() { + throw new NotImplementedException(); + } + + @Override + public TypedQuery setMaxResults(int i) { + throw new NotImplementedException(); + } + + @Override + public int getMaxResults() { + throw new NotImplementedException(); + } + + @Override + public TypedQuery setFirstResult(int i) { + throw new NotImplementedException(); + } + + @Override + public int getFirstResult() { + throw new NotImplementedException(); + } + + @Override + public TypedQuery setHint(String s, Object o) { + throw new NotImplementedException(); + } + + @Override + public Map getHints() { + throw new NotImplementedException(); + } + + @Override + public Set> getParameters() { + throw new NotImplementedException(); + } + + @Override + public Parameter getParameter(String s) { + throw new NotImplementedException(); + } + + @Override + public Parameter getParameter(String s, Class aClass) { + throw new NotImplementedException(); + } + + @Override + public Parameter getParameter(int i) { + throw new NotImplementedException(); + } + + @Override + public Parameter getParameter(int i, Class aClass) { + throw new NotImplementedException(); + } + + @Override + public boolean isBound(Parameter parameter) { + throw new NotImplementedException(); + } + + @Override + public T getParameterValue(Parameter parameter) { + throw new NotImplementedException(); + } + + @Override + public Object getParameterValue(String s) { + throw new NotImplementedException(); + } + + @Override + public Object getParameterValue(int i) { + throw new NotImplementedException(); + } + + @Override + public TypedQuery setFlushMode(FlushModeType flushModeType) { + throw new NotImplementedException(); + } + + @Override + public FlushModeType getFlushMode() { + throw new NotImplementedException(); + } + + @Override + public TypedQuery setLockMode(LockModeType lockModeType) { + throw new NotImplementedException(); + } + + @Override + public LockModeType getLockMode() { + throw new NotImplementedException(); + } + + @Override + public T unwrap(Class aClass) { + throw new NotImplementedException(); + } + } + +} diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java index d7e550b543..bf75fc9ab5 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java @@ -14,6 +14,7 @@ import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser; import static ch.puzzle.okr.test.TestHelper.mockAuthorizationUser; +import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertFalse; public class AuthorizationCriteriaTest { @@ -29,7 +30,7 @@ void appendObjectiveShouldBeSuccessfulWithDefaultAuthorizationUser() { // assert var expected = " and ((o.state=:teamDraftState and o.team.id IN (:userTeamIds)) or o.state IN (:publishedStates))"; - Assertions.assertEquals(expected, current); + assertEquals(expected, current); } @DisplayName("appendObjective() should be successful when user is okrChampion") @@ -50,7 +51,7 @@ void appendObjectiveShouldBeSuccessfulWhenUserIsOkrChampion() { // assert var expected = " and (o.state=:allDraftState or o.state IN (:publishedStates))"; - Assertions.assertEquals(expected, current); + assertEquals(expected, current); } @DisplayName("appendOverview() should be successful when team ids or objective query are empty") @@ -66,7 +67,7 @@ void appendOverviewShouldBeSuccessfulWhenTeamIdsOrObjectiveQueryAreEmpty(List provideListAndString() { @@ -97,7 +98,7 @@ and o.overviewId.teamId in (:teamIds) and lower(coalesce(o.objectiveTitle, '')) like lower(concat('%',:objectiveQuery,'%')) and ((o.objectiveState=:teamDraftState and o.overviewId.teamId IN (:userTeamIds)) or o.objectiveState IN (:publishedStates) or o.overviewId.objectiveId = -1)"""; - Assertions.assertEquals(expected, current); + assertEquals(expected, current); assertFalse(current.contains(anyNonEmptyString)); } From f856d2aac14a82656f3dc516bc344b191d4cb9cd Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 21 Oct 2024 12:41:49 +0200 Subject: [PATCH 10/24] tests for AuthorizationCriteria --- .../okr/service/persistence/AuthorizationCriteriaTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java index bf75fc9ab5..bdb382edaf 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaTest.java @@ -2,7 +2,6 @@ import ch.puzzle.okr.models.Objective; import ch.puzzle.okr.models.User; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -14,7 +13,7 @@ import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser; import static ch.puzzle.okr.test.TestHelper.mockAuthorizationUser; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; public class AuthorizationCriteriaTest { From 508481ecd6e091855e9f2d7d62ffe0ab9ca9aa77 Mon Sep 17 00:00:00 2001 From: peggimann Date: Wed, 23 Oct 2024 10:24:47 +0200 Subject: [PATCH 11/24] enable debug trace --- .github/workflows/backend-test-action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/backend-test-action.yml b/.github/workflows/backend-test-action.yml index 50f40c9fb2..428368d7ff 100644 --- a/.github/workflows/backend-test-action.yml +++ b/.github/workflows/backend-test-action.yml @@ -17,4 +17,4 @@ jobs: distribution: 'adopt' - name: Use Maven to run unittests and integration tests - run: mvn clean verify \ No newline at end of file + run: mvn clean verify -X \ No newline at end of file From fdb7ece182791eb30abc060ce9b8d93b0edca4d6 Mon Sep 17 00:00:00 2001 From: peggimann Date: Wed, 23 Oct 2024 10:44:56 +0200 Subject: [PATCH 12/24] debug found okr-champions in test --- .../okr/service/persistence/UserPersistenceServiceIT.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java index 2377f4b772..5c904a3b3d 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java @@ -1,5 +1,6 @@ package ch.puzzle.okr.service.persistence; +import ch.puzzle.okr.OkrApplicationContextInitializer; import ch.puzzle.okr.models.User; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; @@ -7,6 +8,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import java.util.List; @@ -19,6 +22,8 @@ @SpringIntegrationTest class UserPersistenceServiceIT { + private static final Logger logger = LoggerFactory.getLogger(UserPersistenceServiceIT.class); + private User createdUser; @Autowired @@ -155,6 +160,7 @@ void findByEmailShouldReturnEmptyOptionalIfEmailIsNull() { @Test void findAllOkrChampionsShouldReturnAllOkrChampions() { var allOkrChampions = userPersistenceService.findAllOkrChampions(); + allOkrChampions.forEach(user -> logger.warn(user.toString())); assertEquals(1, allOkrChampions.size()); assertEquals("Jaya", allOkrChampions.get(0).getFirstname()); From 3275487b3f67aebc74a6f0b2f8d1e630b47cd522 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 13:34:14 +0200 Subject: [PATCH 13/24] add logging in findAllOkrChampionsShouldReturnAllOkrChampions() test --- .../persistence/UserPersistenceServiceIT.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java index 5c904a3b3d..55ac22a909 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java @@ -159,7 +159,19 @@ void findByEmailShouldReturnEmptyOptionalIfEmailIsNull() { @DisplayName("findAllOkrChampions() should return all okr champions") @Test void findAllOkrChampionsShouldReturnAllOkrChampions() { + var allUsers = userPersistenceService.findAll(); + logger.warn("*** ALL USERS"); + allUsers.forEach(user -> { + logger.warn(user.toString()); + }); + + logger.warn("*** ALL OKR CHAMPIONS V1"); + allUsers.stream().filter(user -> user.isOkrChampion()).toList().forEach(champion -> { + logger.warn(champion.toString()); + }); + var allOkrChampions = userPersistenceService.findAllOkrChampions(); + logger.warn("*** ALL OKR CHAMPIONS V2"); allOkrChampions.forEach(user -> logger.warn(user.toString())); assertEquals(1, allOkrChampions.size()); From 5cfed89aad22d824ee2b4b46760963375bbfa5db Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 14:11:34 +0200 Subject: [PATCH 14/24] disable tests which call setOkrChampion() --- .../authorization/AuthorizationRegistrationService.java | 6 ++++++ .../test/java/ch/puzzle/okr/SpringCachingConfigTest.java | 1 + .../AuthorizationRegistrationServiceIT.java | 9 +++++---- .../service/authorization/AuthorizationServiceTest.java | 2 ++ .../okr/service/authorization/UserUpdateHelperTest.java | 3 +++ 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java b/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java index fc1213b565..ec2da167bd 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java @@ -6,6 +6,8 @@ import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.service.business.UserBusinessService; import jakarta.persistence.EntityNotFoundException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; @@ -19,6 +21,8 @@ public class AuthorizationRegistrationService { private final UserUpdateHelper helper = new UserUpdateHelper(); + private static final Logger logger = LoggerFactory.getLogger(AuthorizationRegistrationService.class); + public AuthorizationRegistrationService(UserBusinessService userBusinessService, TenantConfigProvider tenantConfigProvider) { this.userBusinessService = userBusinessService; @@ -41,6 +45,7 @@ private User setFirstLastNameFromToken(User userFromDB, User userFromToken) { // okr champion is set in application properties private User setOkrChampionFromProperties(User user) { + logger.warn("SHOULD NEVER BE CALLED 4 USER (outer): " + user); TenantConfigProvider.TenantConfig tenantConfig = this.tenantConfigProvider .getTenantConfigById(TenantContext.getCurrentTenant()) .orElseThrow(() -> new EntityNotFoundException("Cannot find tenant")); @@ -51,6 +56,7 @@ private User setOkrChampionFromProperties(User user) { public static class UserUpdateHelper { public User setOkrChampionFromProperties(User user, TenantConfigProvider.TenantConfig tenantConfig) { + logger.warn("SHOULD NEVER BE CALLED 4 USER (inner): " + user); for (var mail : tenantConfig.okrChampionEmails()) { if (mail.trim().equals(user.getEmail())) { user.setOkrChampion(true); diff --git a/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java b/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java index aa87d01441..12e048f9e4 100644 --- a/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java +++ b/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java @@ -48,6 +48,7 @@ void testUserIsNotCached() { assertNull(cache.get(key, AuthorizationUser.class)); } + @Disabled @DisplayName("updateOrAddAuthorizationUser puts the User in the cache with key composed by Tenant and User Email") @Test void testUserIsCached() { diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 8ebcaa9351..5e44a7fb4a 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -6,10 +6,7 @@ import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.service.persistence.UserPersistenceService; import ch.puzzle.okr.test.SpringIntegrationTest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.*; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; @@ -47,6 +44,7 @@ void tearDown() { TenantContext.setCurrentTenant(null); } + @Disabled @Test void registerAuthorizationUserShouldAddAuthorizationUserToCache() { // arrange @@ -63,6 +61,7 @@ void registerAuthorizationUserShouldAddAuthorizationUserToCache() { userPersistenceService.deleteById(user.getId()); } + @Disabled @DisplayName("registerAuthorizationUser for a user with an email not defined in the application-integration-test.properties should set OkrChampions to false") @Test void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { @@ -94,6 +93,7 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { * false) is after calling updateOrAddAuthorizationUser() a user champion. - because the user wunderland@puzzle.ch * exists before the test, we make no clean in db (we don't remove it) */ + @Disabled @Test @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { @@ -115,6 +115,7 @@ void registerAuthorizationUserShouldSetOkrChampionsToTrue() { assertTrue(userFromDB.get().isOkrChampion()); } + @Disabled @Test void registerAuthorizationUser_shouldSetFirstnameAndLastnameFromToken() { // arrange diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java index eb4e88823f..a86d8e8723 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java @@ -13,6 +13,7 @@ import ch.puzzle.okr.models.keyresult.KeyResultMetric; import ch.puzzle.okr.security.JwtHelper; import ch.puzzle.okr.service.persistence.ObjectivePersistenceService; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -89,6 +90,7 @@ void hasRoleWriteForTeam_shouldReturnFalseWhenUserIsNotAdmin() { assertFalse(hasRoleWriteForTeam(authorizationUser, 3L)); } + @Disabled @Test void getAuthorizationUserShouldReturnAuthorizationUser() { User user = defaultUser(null); diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java index 958eaac311..365e41c066 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java @@ -3,6 +3,7 @@ import ch.puzzle.okr.models.User; import ch.puzzle.okr.multitenancy.TenantConfigProvider; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -42,6 +43,7 @@ void updateUserFromWithTokenData() { assertEquals("lastname_from_token", updatedUser.getLastname()); } + @Disabled @Test void updateUserAsNoChampion() { // arrange @@ -56,6 +58,7 @@ void updateUserAsNoChampion() { assertFalse(updatedUser.isOkrChampion()); } + @Disabled @Test void updateUserAsChampion() { // arrange From e01c8c177a0f777227e80912a0c414514cb1b514 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 14:28:00 +0200 Subject: [PATCH 15/24] enable some disabled tests --- .../test/java/ch/puzzle/okr/SpringCachingConfigTest.java | 7 ++++--- .../service/authorization/AuthorizationServiceTest.java | 6 ++---- .../okr/service/authorization/UserUpdateHelperTest.java | 3 --- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java b/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java index 12e048f9e4..1ca5d0825a 100644 --- a/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java +++ b/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java @@ -6,7 +6,10 @@ import ch.puzzle.okr.service.authorization.AuthorizationRegistrationService; import ch.puzzle.okr.test.SpringIntegrationTest; import ch.puzzle.okr.test.TestHelper; -import org.junit.jupiter.api.*; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; @@ -14,7 +17,6 @@ import static ch.puzzle.okr.SpringCachingConfig.AUTHORIZATION_USER_CACHE; import static ch.puzzle.okr.test.TestHelper.defaultUser; import static org.junit.jupiter.api.Assertions.*; -import static org.junit.jupiter.api.Assertions.assertNotNull; @SpringIntegrationTest class SpringCachingConfigTest { @@ -48,7 +50,6 @@ void testUserIsNotCached() { assertNull(cache.get(key, AuthorizationUser.class)); } - @Disabled @DisplayName("updateOrAddAuthorizationUser puts the User in the cache with key composed by Tenant and User Email") @Test void testUserIsCached() { diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java index a86d8e8723..957b27540f 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java @@ -1,6 +1,5 @@ package ch.puzzle.okr.service.authorization; -import ch.puzzle.okr.test.TestHelper; import ch.puzzle.okr.dto.ErrorDto; import ch.puzzle.okr.exception.OkrResponseStatusException; import ch.puzzle.okr.models.Objective; @@ -13,7 +12,7 @@ import ch.puzzle.okr.models.keyresult.KeyResultMetric; import ch.puzzle.okr.security.JwtHelper; import ch.puzzle.okr.service.persistence.ObjectivePersistenceService; -import org.junit.jupiter.api.Disabled; +import ch.puzzle.okr.test.TestHelper; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -29,9 +28,9 @@ import java.util.List; import static ch.puzzle.okr.ErrorKey.*; -import static ch.puzzle.okr.test.TestHelper.*; import static ch.puzzle.okr.service.authorization.AuthorizationService.hasRoleWriteAndReadAll; import static ch.puzzle.okr.service.authorization.AuthorizationService.hasRoleWriteForTeam; +import static ch.puzzle.okr.test.TestHelper.*; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -90,7 +89,6 @@ void hasRoleWriteForTeam_shouldReturnFalseWhenUserIsNotAdmin() { assertFalse(hasRoleWriteForTeam(authorizationUser, 3L)); } - @Disabled @Test void getAuthorizationUserShouldReturnAuthorizationUser() { User user = defaultUser(null); diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java index 365e41c066..958eaac311 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java @@ -3,7 +3,6 @@ import ch.puzzle.okr.models.User; import ch.puzzle.okr.multitenancy.TenantConfigProvider; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -43,7 +42,6 @@ void updateUserFromWithTokenData() { assertEquals("lastname_from_token", updatedUser.getLastname()); } - @Disabled @Test void updateUserAsNoChampion() { // arrange @@ -58,7 +56,6 @@ void updateUserAsNoChampion() { assertFalse(updatedUser.isOkrChampion()); } - @Disabled @Test void updateUserAsChampion() { // arrange From 1bf0e80e8d72779c4f84f3222bf1641a851b4b2b Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 14:36:45 +0200 Subject: [PATCH 16/24] enable some disabled tests --- .../authorization/AuthorizationRegistrationServiceIT.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 5e44a7fb4a..783c96a0fc 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -44,7 +44,6 @@ void tearDown() { TenantContext.setCurrentTenant(null); } - @Disabled @Test void registerAuthorizationUserShouldAddAuthorizationUserToCache() { // arrange @@ -61,7 +60,6 @@ void registerAuthorizationUserShouldAddAuthorizationUserToCache() { userPersistenceService.deleteById(user.getId()); } - @Disabled @DisplayName("registerAuthorizationUser for a user with an email not defined in the application-integration-test.properties should set OkrChampions to false") @Test void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { @@ -115,7 +113,6 @@ void registerAuthorizationUserShouldSetOkrChampionsToTrue() { assertTrue(userFromDB.get().isOkrChampion()); } - @Disabled @Test void registerAuthorizationUser_shouldSetFirstnameAndLastnameFromToken() { // arrange From 82900c609bb7ed2f0e37458e79e28e58f7987acb Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 14:39:36 +0200 Subject: [PATCH 17/24] enable last disabled test .. should fail --- .../authorization/AuthorizationRegistrationServiceIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 783c96a0fc..8113a6e8ec 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -91,7 +91,7 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { * false) is after calling updateOrAddAuthorizationUser() a user champion. - because the user wunderland@puzzle.ch * exists before the test, we make no clean in db (we don't remove it) */ - @Disabled + // @Disabled @Test @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { From 5fb338f7bf490b33cedae421f1230ca530eedb27 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Wed, 23 Oct 2024 15:53:04 +0200 Subject: [PATCH 18/24] ugly temp fix --- .../AuthorizationRegistrationServiceIT.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 8113a6e8ec..0c043d2f8d 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -31,6 +31,12 @@ class AuthorizationRegistrationServiceIT { private final String tenant = TestHelper.SCHEMA_PITC; private final String key = tenant + "_" + user.getEmail(); + private User userX = User.Builder.builder() // + .withFirstname("Alice") // + .withLastname("Wunderland") // + .withEmail("wunderland@puzzle.ch") // user.champion.emails from application-integration-test.properties + .build(); + @BeforeEach void setUp() { TenantContext.setCurrentTenant(tenant); @@ -38,12 +44,28 @@ void setUp() { @AfterEach void tearDown() { + + cleanup(); + Cache cache = cacheManager.getCache(AUTHORIZATION_USER_CACHE); assertNotNull(cache); cache.clear(); TenantContext.setCurrentTenant(null); } + private void cleanup() { + var aa = userPersistenceService.findByEmail(userX.getEmail()); + System.out.println("aa=" + aa.get().isOkrChampion()); + + userX.setOkrChampion(false); + var xxx = userPersistenceService.getOrCreateUser(userX); + xxx.setOkrChampion(false); + userPersistenceService.save(xxx); + + var zz = userPersistenceService.findByEmail(userX.getEmail()); + System.out.println("zz=" + zz.get().isOkrChampion()); + } + @Test void registerAuthorizationUserShouldAddAuthorizationUserToCache() { // arrange @@ -96,20 +118,15 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { // arrange - User user = User.Builder.builder() // - .withFirstname("Alice") // - .withLastname("Wunderland") // - .withEmail("wunderland@puzzle.ch") // user.champion.emails from application-integration-test.properties - .build(); - userPersistenceService.getOrCreateUser(user); // updates input user with id from DB !!! + userPersistenceService.getOrCreateUser(userX); // updates input user with id from DB !!! // act - AuthorizationUser processedUser = authorizationRegistrationService.updateOrAddAuthorizationUser(user); + AuthorizationUser processedUser = authorizationRegistrationService.updateOrAddAuthorizationUser(userX); // assert assertTrue(processedUser.user().isOkrChampion()); - Optional userFromDB = userPersistenceService.findByEmail(user.getEmail()); + Optional userFromDB = userPersistenceService.findByEmail(userX.getEmail()); assertTrue(userFromDB.get().isOkrChampion()); } From 9fd331d9e72b21dfc92cdce81a2bd1e07005826b Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 07:25:18 +0200 Subject: [PATCH 19/24] cleanup --- .../AuthorizationRegistrationService.java | 6 --- .../AuthorizationRegistrationServiceIT.java | 52 ++++++++++++------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java b/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java index ec2da167bd..fc1213b565 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationService.java @@ -6,8 +6,6 @@ import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.service.business.UserBusinessService; import jakarta.persistence.EntityNotFoundException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; @@ -21,8 +19,6 @@ public class AuthorizationRegistrationService { private final UserUpdateHelper helper = new UserUpdateHelper(); - private static final Logger logger = LoggerFactory.getLogger(AuthorizationRegistrationService.class); - public AuthorizationRegistrationService(UserBusinessService userBusinessService, TenantConfigProvider tenantConfigProvider) { this.userBusinessService = userBusinessService; @@ -45,7 +41,6 @@ private User setFirstLastNameFromToken(User userFromDB, User userFromToken) { // okr champion is set in application properties private User setOkrChampionFromProperties(User user) { - logger.warn("SHOULD NEVER BE CALLED 4 USER (outer): " + user); TenantConfigProvider.TenantConfig tenantConfig = this.tenantConfigProvider .getTenantConfigById(TenantContext.getCurrentTenant()) .orElseThrow(() -> new EntityNotFoundException("Cannot find tenant")); @@ -56,7 +51,6 @@ private User setOkrChampionFromProperties(User user) { public static class UserUpdateHelper { public User setOkrChampionFromProperties(User user, TenantConfigProvider.TenantConfig tenantConfig) { - logger.warn("SHOULD NEVER BE CALLED 4 USER (inner): " + user); for (var mail : tenantConfig.okrChampionEmails()) { if (mail.trim().equals(user.getEmail())) { user.setOkrChampion(true); diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 0c043d2f8d..ffbafb9abf 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -31,10 +31,12 @@ class AuthorizationRegistrationServiceIT { private final String tenant = TestHelper.SCHEMA_PITC; private final String key = tenant + "_" + user.getEmail(); - private User userX = User.Builder.builder() // + private static final String EMAIL_WUNDERLAND = "wunderland@puzzle.ch"; + + private final User userWithNoOkrChampionInfo = User.Builder.builder() // .withFirstname("Alice") // .withLastname("Wunderland") // - .withEmail("wunderland@puzzle.ch") // user.champion.emails from application-integration-test.properties + .withEmail(EMAIL_WUNDERLAND) // user.champion.emails from application-integration-test.properties .build(); @BeforeEach @@ -44,26 +46,29 @@ void setUp() { @AfterEach void tearDown() { + resetOkrChamptionStatus(userWithNoOkrChampionInfo); + clearCache(); + TenantContext.setCurrentTenant(null); + } - cleanup(); - + private void clearCache() { Cache cache = cacheManager.getCache(AUTHORIZATION_USER_CACHE); assertNotNull(cache); cache.clear(); - TenantContext.setCurrentTenant(null); } - private void cleanup() { - var aa = userPersistenceService.findByEmail(userX.getEmail()); - System.out.println("aa=" + aa.get().isOkrChampion()); - - userX.setOkrChampion(false); - var xxx = userPersistenceService.getOrCreateUser(userX); - xxx.setOkrChampion(false); - userPersistenceService.save(xxx); + private void resetOkrChamptionStatus(User user) { + // user.setOkrChampion(false); + var userFromDb = userPersistenceService.getOrCreateUser(user); + userFromDb.setOkrChampion(false); + userPersistenceService.save(userFromDb); + assertOkrChampionStatusInDb(user.getEmail(), false); + } - var zz = userPersistenceService.findByEmail(userX.getEmail()); - System.out.println("zz=" + zz.get().isOkrChampion()); + private void assertOkrChampionStatusInDb(String email, boolean expectedOkrChampionStatus) { + var userInDb = userPersistenceService.findByEmail(email); + assertTrue(userInDb.isPresent()); + assertEquals(expectedOkrChampionStatus, userInDb.get().isOkrChampion()); } @Test @@ -118,16 +123,23 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { // arrange - - userPersistenceService.getOrCreateUser(userX); // updates input user with id from DB !!! + var userFromDb = userPersistenceService.getOrCreateUser(userWithNoOkrChampionInfo); // ensure user exists + assertFalse(userFromDb.isOkrChampion()); // pre-condition // act - AuthorizationUser processedUser = authorizationRegistrationService.updateOrAddAuthorizationUser(userX); + // load user from db and set OkrChampion status based on property "okr.tenants.pitc.user.champion.emails" + // from application-integration-test.properties file + AuthorizationUser processedUser = authorizationRegistrationService + .updateOrAddAuthorizationUser(userWithNoOkrChampionInfo); // assert assertTrue(processedUser.user().isOkrChampion()); - Optional userFromDB = userPersistenceService.findByEmail(userX.getEmail()); - assertTrue(userFromDB.get().isOkrChampion()); + assertOkrChampionStatusInDb(processedUser.user().getEmail(), true); + + /* + * Optional updatedUserFromDB = userPersistenceService.findByEmail(userWithNoOkrChampionInfo.getEmail()); + * assertTrue(updatedUserFromDB.isPresent()); assertTrue(updatedUserFromDB.get().isOkrChampion()); + */ } @Test From 24cc486e0d251b6084665883c89b711dbbd52b9a Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 07:36:00 +0200 Subject: [PATCH 20/24] cleanup --- .../AuthorizationRegistrationServiceIT.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index ffbafb9abf..cb74cf05af 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -46,25 +46,27 @@ void setUp() { @AfterEach void tearDown() { - resetOkrChamptionStatus(userWithNoOkrChampionInfo); + resetOkrChampionStatus(userWithNoOkrChampionInfo); clearCache(); TenantContext.setCurrentTenant(null); } + private void resetOkrChampionStatus(User user) { + // var userFromDb = userPersistenceService.getOrCreateUser(user); + Optional userFromDb = userPersistenceService.findByEmail(user.getEmail()); + assertTrue(userFromDb.isPresent()); + + userFromDb.get().setOkrChampion(false); + userPersistenceService.save(userFromDb.get()); + assertOkrChampionStatusInDb(user.getEmail(), false); + } + private void clearCache() { Cache cache = cacheManager.getCache(AUTHORIZATION_USER_CACHE); assertNotNull(cache); cache.clear(); } - private void resetOkrChamptionStatus(User user) { - // user.setOkrChampion(false); - var userFromDb = userPersistenceService.getOrCreateUser(user); - userFromDb.setOkrChampion(false); - userPersistenceService.save(userFromDb); - assertOkrChampionStatusInDb(user.getEmail(), false); - } - private void assertOkrChampionStatusInDb(String email, boolean expectedOkrChampionStatus) { var userInDb = userPersistenceService.findByEmail(email); assertTrue(userInDb.isPresent()); From 648befe5cecc7943a5aef940e1f170661f8348eb Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 07:46:07 +0200 Subject: [PATCH 21/24] cleanup --- .../AuthorizationRegistrationServiceIT.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index cb74cf05af..37618080e3 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -46,19 +46,18 @@ void setUp() { @AfterEach void tearDown() { - resetOkrChampionStatus(userWithNoOkrChampionInfo); + resetOkrChampionStatus(userWithNoOkrChampionInfo.getEmail()); clearCache(); TenantContext.setCurrentTenant(null); } - private void resetOkrChampionStatus(User user) { - // var userFromDb = userPersistenceService.getOrCreateUser(user); - Optional userFromDb = userPersistenceService.findByEmail(user.getEmail()); + private void resetOkrChampionStatus(String email) { + Optional userFromDb = userPersistenceService.findByEmail(email); assertTrue(userFromDb.isPresent()); userFromDb.get().setOkrChampion(false); userPersistenceService.save(userFromDb.get()); - assertOkrChampionStatusInDb(user.getEmail(), false); + assertOkrChampionStatusInDb(userFromDb.get().getEmail(), false); } private void clearCache() { From 87cfa3bfe5eefc6ed0ec766e5f97adae8b0f5588 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 07:59:23 +0200 Subject: [PATCH 22/24] cleanup --- .../AuthorizationRegistrationServiceIT.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 37618080e3..3650a9c3b0 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -106,6 +106,8 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { // assert assertFalse(processedUser.user().isOkrChampion()); Optional userFromDB = userPersistenceService.findByEmail(user.getEmail()); + + assertTrue(userFromDB.isPresent()); assertFalse(userFromDB.get().isOkrChampion()); // cleanup @@ -119,16 +121,15 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { * false) is after calling updateOrAddAuthorizationUser() a user champion. - because the user wunderland@puzzle.ch * exists before the test, we make no clean in db (we don't remove it) */ - // @Disabled @Test @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { // arrange - var userFromDb = userPersistenceService.getOrCreateUser(userWithNoOkrChampionInfo); // ensure user exists - assertFalse(userFromDb.isOkrChampion()); // pre-condition + assertOkrChampionStatusInDb(userWithNoOkrChampionInfo.getEmail(), false); // pre-condition // act - // load user from db and set OkrChampion status based on property "okr.tenants.pitc.user.champion.emails" + // load user from db (by email) and set OkrChampion status based on property + // "okr.tenants.pitc.user.champion.emails" // from application-integration-test.properties file AuthorizationUser processedUser = authorizationRegistrationService .updateOrAddAuthorizationUser(userWithNoOkrChampionInfo); @@ -136,11 +137,6 @@ void registerAuthorizationUserShouldSetOkrChampionsToTrue() { // assert assertTrue(processedUser.user().isOkrChampion()); assertOkrChampionStatusInDb(processedUser.user().getEmail(), true); - - /* - * Optional updatedUserFromDB = userPersistenceService.findByEmail(userWithNoOkrChampionInfo.getEmail()); - * assertTrue(updatedUserFromDB.isPresent()); assertTrue(updatedUserFromDB.get().isOkrChampion()); - */ } @Test @@ -166,6 +162,7 @@ void registerAuthorizationUser_shouldSetFirstnameAndLastnameFromToken() { // assert Optional userFromDB = userPersistenceService.findByEmail(user.getEmail()); + assertTrue(userFromDB.isPresent()); assertEquals(userFromDB.get().getFirstname(), firstNameFromToken); assertEquals(userFromDB.get().getLastname(), lastNameFromToken); From 30e7f7f360aa447f051a7858a427297b0c57cace Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 08:23:54 +0200 Subject: [PATCH 23/24] cleanup --- .../AuthorizationRegistrationServiceIT.java | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java index 3650a9c3b0..4f0e3c77e4 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java @@ -33,12 +33,6 @@ class AuthorizationRegistrationServiceIT { private static final String EMAIL_WUNDERLAND = "wunderland@puzzle.ch"; - private final User userWithNoOkrChampionInfo = User.Builder.builder() // - .withFirstname("Alice") // - .withLastname("Wunderland") // - .withEmail(EMAIL_WUNDERLAND) // user.champion.emails from application-integration-test.properties - .build(); - @BeforeEach void setUp() { TenantContext.setCurrentTenant(tenant); @@ -46,13 +40,13 @@ void setUp() { @AfterEach void tearDown() { - resetOkrChampionStatus(userWithNoOkrChampionInfo.getEmail()); + resetOkrChampionStatus(); clearCache(); TenantContext.setCurrentTenant(null); } - private void resetOkrChampionStatus(String email) { - Optional userFromDb = userPersistenceService.findByEmail(email); + private void resetOkrChampionStatus() { + Optional userFromDb = userPersistenceService.findByEmail(EMAIL_WUNDERLAND); assertTrue(userFromDb.isPresent()); userFromDb.get().setOkrChampion(false); @@ -116,23 +110,27 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() { /* * Special test setup. 

 - the user wunderland@puzzle.ch is an existing user in the H2 db (created via
-     * X_TestData.sql) - the user wunderland@puzzle.ch is also defined in application-integration-test.properties as
-     * user champion - with this combination we can test, that the user in the db (which has initial isOkrChampion ==
-     * false) is after calling updateOrAddAuthorizationUser() a user champion. - because the user wunderland@puzzle.ch
-     * exists before the test, we make no clean in db (we don't remove it)
+ * V100_0_0__TestData.sql) - the user wunderland@puzzle.ch is also defined in + * application-integration-test.properties as user champion - with this combination we can test, that the user in + * the db (which has initial isOkrChampion == false) is after calling updateOrAddAuthorizationUser() a user + * champion. - the OkrChampion status must manually be reset (in the tearDown method) */ @Test @DisplayName("registerAuthorizationUser for a user with an email defined in the application-integration-test.properties should set OkrChampions to true") void registerAuthorizationUserShouldSetOkrChampionsToTrue() { // arrange - assertOkrChampionStatusInDb(userWithNoOkrChampionInfo.getEmail(), false); // pre-condition + assertOkrChampionStatusInDb(EMAIL_WUNDERLAND, false); // pre-condition // act // load user from db (by email) and set OkrChampion status based on property - // "okr.tenants.pitc.user.champion.emails" - // from application-integration-test.properties file + // "okr.tenants.pitc.user.champion.emails" from application-integration-test.properties file AuthorizationUser processedUser = authorizationRegistrationService - .updateOrAddAuthorizationUser(userWithNoOkrChampionInfo); + .updateOrAddAuthorizationUser(User.Builder.builder() // + .withFirstname("Alice") // + .withLastname("Wunderland") // + .withEmail(EMAIL_WUNDERLAND) // user.champion.emails from + // application-integration-test.properties + .build()); // assert assertTrue(processedUser.user().isOkrChampion()); From 4ee83499c7bb05eb6df4f02e3329561edd024804 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Thu, 24 Oct 2024 08:52:03 +0200 Subject: [PATCH 24/24] cleanup --- .../persistence/UserPersistenceServiceIT.java | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java index 55ac22a909..0104060b48 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/UserPersistenceServiceIT.java @@ -1,6 +1,5 @@ package ch.puzzle.okr.service.persistence; -import ch.puzzle.okr.OkrApplicationContextInitializer; import ch.puzzle.okr.models.User; import ch.puzzle.okr.multitenancy.TenantContext; import ch.puzzle.okr.test.SpringIntegrationTest; @@ -8,8 +7,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import java.util.List; @@ -22,8 +19,6 @@ @SpringIntegrationTest class UserPersistenceServiceIT { - private static final Logger logger = LoggerFactory.getLogger(UserPersistenceServiceIT.class); - private User createdUser; @Autowired @@ -159,24 +154,12 @@ void findByEmailShouldReturnEmptyOptionalIfEmailIsNull() { @DisplayName("findAllOkrChampions() should return all okr champions") @Test void findAllOkrChampionsShouldReturnAllOkrChampions() { - var allUsers = userPersistenceService.findAll(); - logger.warn("*** ALL USERS"); - allUsers.forEach(user -> { - logger.warn(user.toString()); - }); - - logger.warn("*** ALL OKR CHAMPIONS V1"); - allUsers.stream().filter(user -> user.isOkrChampion()).toList().forEach(champion -> { - logger.warn(champion.toString()); - }); - + // act var allOkrChampions = userPersistenceService.findAllOkrChampions(); - logger.warn("*** ALL OKR CHAMPIONS V2"); - allOkrChampions.forEach(user -> logger.warn(user.toString())); + // assert assertEquals(1, allOkrChampions.size()); - assertEquals("Jaya", allOkrChampions.get(0).getFirstname()); - assertEquals("Norris", allOkrChampions.get(0).getLastname()); + assertUser(61L, "Jaya", "Norris", "gl@gl.com", allOkrChampions.get(0)); } @DisplayName("getModelName() should return user")