Test/1012 service (#1050)

* tests for PersistenceBase class * remove tests from QuarterPersistenceServiceIT which are already covered in PersistenceBaseTestIT * remove no longer used OrganisationXXX() methods * remove tests from TeamPersistenceServiceIT which are already covered in PersistenceBaseTestIT * remove tests from UserPersistenceServiceIT which are already covered in PersistenceBaseTestIT and add additional tests * remove tests from CheckInPersistenceServiceIT which are already covered in PersistenceBaseTestIT and add additional test * umbau ObjectivePersistenceServiceIT: remove tests which are already covered in PersistenceBaseTestIT, add additional test, make the tests readable * tests for AuthorizationCriteria * tests for AuthorizationCriteria * tests for AuthorizationCriteria * enable debug trace * debug found okr-champions in test * add logging in findAllOkrChampionsShouldReturnAllOkrChampions() test * disable tests which call setOkrChampion() * enable some disabled tests * enable some disabled tests * enable last disabled test .. should fail * ugly temp fix * cleanup * cleanup * cleanup * cleanup * cleanup * cleanup --------- Co-authored-by: peggimann <peggimann@puzzle.com>
puzzle · Nov 1, 2024 · c9c738b · c9c738b
1 parent 4f48c07
commit c9c738b
Show file tree

Hide file tree

Showing 18 changed files with 1,054 additions and 505 deletions.
diff --git 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
+        run: mvn clean verify -X
diff --git 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<Team, Long> {
 
-    @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<Long> findTeamIdsByOrganisationNames(@Param("organisationNames") List<String> organisationNames);
-
     List<Team> findTeamsByName(String name);
 }
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
@@ -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/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<Long> findTeamIdsByOrganisationName(String organisationName) {
-        return findTeamIdsByOrganisationNames(List.of(organisationName));
-    }
-
-    public List<Long> findTeamIdsByOrganisationNames(List<String> organisationNames) {
-        return getRepository().findTeamIdsByOrganisationNames(organisationNames);
-    }
-
     public List<Team> findTeamsByName(String name) {
         return getRepository().findTeamsByName(name);
     }

diff --git a/backend/src/main/java/ch/puzzle/okr/util/CollectionUtils.java 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 <T> List<T> iterableToList(Iterable<T> iterable) {
+        return StreamSupport //
+                .stream(iterable.spliterator(), false) //
+                .collect(Collectors.toList());
+    }
+}
diff --git a/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java b/backend/src/test/java/ch/puzzle/okr/SpringCachingConfigTest.java
@@ -6,15 +6,17 @@
 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;
 
 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 {

diff --git a/...src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java b/...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;
@@ -34,17 +31,39 @@ class AuthorizationRegistrationServiceIT {
     private final String tenant = TestHelper.SCHEMA_PITC;
     private final String key = tenant + "_" + user.getEmail();
 
+    private static final String EMAIL_WUNDERLAND = "wunderland@puzzle.ch";
+
     @BeforeEach
     void setUp() {
         TenantContext.setCurrentTenant(tenant);
     }
 
     @AfterEach
     void tearDown() {
+        resetOkrChampionStatus();
+        clearCache();
+        TenantContext.setCurrentTenant(null);
+    }
+
+    private void resetOkrChampionStatus() {
+        Optional<User> userFromDb = userPersistenceService.findByEmail(EMAIL_WUNDERLAND);
+        assertTrue(userFromDb.isPresent());
+
+        userFromDb.get().setOkrChampion(false);
+        userPersistenceService.save(userFromDb.get());
+        assertOkrChampionStatusInDb(userFromDb.get().getEmail(), false);
+    }
+
+    private void clearCache() {
         Cache cache = cacheManager.getCache(AUTHORIZATION_USER_CACHE);
         assertNotNull(cache);
         cache.clear();
-        TenantContext.setCurrentTenant(null);
+    }
+
+    private void assertOkrChampionStatusInDb(String email, boolean expectedOkrChampionStatus) {
+        var userInDb = userPersistenceService.findByEmail(email);
+        assertTrue(userInDb.isPresent());
+        assertEquals(expectedOkrChampionStatus, userInDb.get().isOkrChampion());
     }
 
     @Test
@@ -81,6 +100,8 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() {
         // assert
         assertFalse(processedUser.user().isOkrChampion());
         Optional<User> userFromDB = userPersistenceService.findByEmail(user.getEmail());
+
+        assertTrue(userFromDB.isPresent());
         assertFalse(userFromDB.get().isOkrChampion());
 
         // cleanup
@@ -89,30 +110,31 @@ void registerAuthorizationUser_shouldSetOkrChampionsToFalse() {
 
     /*
      * Special test setup. <pre> - 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) </pre>
+     * 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) </pre>
      */
     @Test
     @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 !!!
+        assertOkrChampionStatusInDb(EMAIL_WUNDERLAND, false); // pre-condition
 
         // act
-        AuthorizationUser processedUser = authorizationRegistrationService.updateOrAddAuthorizationUser(user);
+        // 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(User.Builder.builder() //
+                        .withFirstname("Alice") //
+                        .withLastname("Wunderland") //
+                        .withEmail(EMAIL_WUNDERLAND) // user.champion.emails from
+                                                     // application-integration-test.properties
+                        .build());
 
         // assert
         assertTrue(processedUser.user().isOkrChampion());
-        Optional<User> userFromDB = userPersistenceService.findByEmail(user.getEmail());
-        assertTrue(userFromDB.get().isOkrChampion());
+        assertOkrChampionStatusInDb(processedUser.user().getEmail(), true);
     }
 
     @Test
@@ -138,6 +160,7 @@ void registerAuthorizationUser_shouldSetFirstnameAndLastnameFromToken() {
 
         // assert
         Optional<User> userFromDB = userPersistenceService.findByEmail(user.getEmail());
+        assertTrue(userFromDB.isPresent());
         assertEquals(userFromDB.get().getFirstname(), firstNameFromToken);
         assertEquals(userFromDB.get().getLastname(), lastNameFromToken);
 

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
@@ -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,6 +12,7 @@
 import ch.puzzle.okr.models.keyresult.KeyResultMetric;
 import ch.puzzle.okr.security.JwtHelper;
 import ch.puzzle.okr.service.persistence.ObjectivePersistenceService;
+import ch.puzzle.okr.test.TestHelper;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
@@ -28,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;

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
@@ -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);