From 86128f59944a52a619868b24d3b602fed30eb1ca Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 5 Aug 2022 15:39:23 +0200 Subject: [PATCH] test alignment and code cleanup --- doc/test-concept.md | 2 +- .../hs/hscustomer/CustomerRepository.java | 3 +- .../rbac/rbacuser/RbacUserRepository.java | 3 +- .../CustomerControllerRestTest.java | 55 ++++++- .../CustomerControllerUnitTest.java | 55 ------- .../PackageRepositoryIntegrationTest.java | 151 ++++++++++++++++++ 6 files changed, 201 insertions(+), 68 deletions(-) delete mode 100644 src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerUnitTest.java create mode 100644 src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java diff --git a/doc/test-concept.md b/doc/test-concept.md index 4682d30b..c8946342 100644 --- a/doc/test-concept.md +++ b/doc/test-concept.md @@ -43,7 +43,7 @@ These Tests are always named `...UnitTest` and can automatically run in the buil #### REST-Tests -At the level of REST-Controllers, *Spring's* `WebMvcTest`, a special kind of Unit-Test, are utilized. +At the level of REST-Controllers, *Spring's* `WebMvcTest`, a special kind of Unit-Test, are utilized to replace simple unit tests. Such tests issue REST-requests through a mocked REST-Layer and therefore use the controllers similar to a real client. Otherwise, the implementation technologies are like those of Unit-Tests. diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepository.java index b28dc8cf..0a182b73 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepository.java @@ -2,7 +2,6 @@ package net.hostsharing.hsadminng.hs.hscustomer; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.Repository; -import org.springframework.data.repository.query.Param; import java.util.List; import java.util.Optional; @@ -14,7 +13,7 @@ public interface CustomerRepository extends Repository { Optional findByUuid(UUID id); @Query("SELECT c FROM CustomerEntity c WHERE :prefix is null or c.prefix like concat(:prefix, '%')") - List findCustomerByOptionalPrefixLike(@Param("prefix") String prefix); + List findCustomerByOptionalPrefixLike(String prefix); CustomerEntity save(final CustomerEntity entity); diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java index 8e6f0342..51e7af86 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java @@ -2,7 +2,6 @@ package net.hostsharing.hsadminng.rbac.rbacuser; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.Repository; -import org.springframework.data.repository.query.Param; import java.util.List; import java.util.UUID; @@ -13,5 +12,5 @@ public interface RbacUserRepository extends Repository { List findByOptionalNameLike(final String userName); @Query(value = "SELECT * FROM grantedPermissions(:userName)", nativeQuery = true) - List findPermissionsOfUser(@Param("userName") String userName); + List findPermissionsOfUser(String userName); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerRestTest.java index 7bb58746..ed80dd01 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerRestTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerRestTest.java @@ -9,10 +9,12 @@ import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; -import static java.util.Arrays.asList; +import java.util.List; + import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -27,10 +29,12 @@ class CustomerControllerRestTest { CustomerRepository customerRepositoryMock; @Test - void apiCustomersWillReturnAllCustomersFromRepositoryIfNoCriteriaGiven() throws Exception { + void listCustomersWillReturnAllCustomersFromRepositoryIfNoCriteriaGiven() throws Exception { // given - when(customerRepositoryMock.findCustomerByOptionalPrefixLike(null)).thenReturn(asList(TestCustomer.xxx, TestCustomer.yyy)); + when(customerRepositoryMock.findCustomerByOptionalPrefixLike(null)).thenReturn(List.of( + TestCustomer.xxx, + TestCustomer.yyy)); // when mockMvc.perform(MockMvcRequestBuilders @@ -42,14 +46,19 @@ class CustomerControllerRestTest { .andExpect(status().isOk()) .andExpect(jsonPath("$", hasSize(2))) .andExpect(jsonPath("$[0].prefix", is(TestCustomer.xxx.getPrefix()))) - .andExpect(jsonPath("$[1].reference", is(TestCustomer.yyy.getReference()))); + .andExpect(jsonPath("$[1].reference", is(TestCustomer.yyy.getReference())) + ); + + // then + verify(contextMock).setCurrentUser("mike@hostsharing.net"); + verify(contextMock, never()).assumeRoles(anyString()); } @Test - void apiCustomersWillReturnMatchingCustomersFromRepositoryIfCriteriaGiven() throws Exception { + void listCustomersWillReturnMatchingCustomersFromRepositoryIfCriteriaGiven() throws Exception { // given - when(customerRepositoryMock.findCustomerByOptionalPrefixLike("x")).thenReturn(asList(TestCustomer.xxx)); + when(customerRepositoryMock.findCustomerByOptionalPrefixLike("x")).thenReturn(List.of(TestCustomer.xxx)); // when mockMvc.perform(MockMvcRequestBuilders @@ -61,6 +70,36 @@ class CustomerControllerRestTest { // then .andExpect(status().isOk()) .andExpect(jsonPath("$", hasSize(1))) - .andExpect(jsonPath("$[0].prefix", is(TestCustomer.xxx.getPrefix()))); + .andExpect(jsonPath("$[0].prefix", is(TestCustomer.xxx.getPrefix())) + ); + + // then + verify(contextMock).setCurrentUser("mike@hostsharing.net"); + verify(contextMock, never()).assumeRoles(anyString()); } + + @Test + void listCustomersWillReturnAllCustomersForGivenAssumedRoles() throws Exception { + + // given + when(customerRepositoryMock.findCustomerByOptionalPrefixLike(null)).thenReturn(List.of(TestCustomer.yyy)); + + // when + mockMvc.perform(MockMvcRequestBuilders + .get("/api/customers") + .header("current-user", "mike@hostsharing.net") + .header("assumed-roles", "admin@yyy.example.com") + .accept(MediaType.APPLICATION_JSON)) + + // then + .andExpect(status().isOk()) + .andExpect(jsonPath("$", hasSize(1))) + .andExpect(jsonPath("$[0].prefix", is(TestCustomer.yyy.getPrefix())) + ); + + // then + verify(contextMock).setCurrentUser("mike@hostsharing.net"); + verify(contextMock).assumeRoles("admin@yyy.example.com"); + } + } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerUnitTest.java deleted file mode 100644 index 3a5af0c7..00000000 --- a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerControllerUnitTest.java +++ /dev/null @@ -1,55 +0,0 @@ -package net.hostsharing.hsadminng.hs.hscustomer; - -import net.hostsharing.hsadminng.context.Context; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; - -@ExtendWith(MockitoExtension.class) -class CustomerControllerUnitTest { - - @Mock - Context contextMock; - @Mock - CustomerRepository customerRepositoryMock; - - @InjectMocks - CustomerController customerController; - - @Test - void apiCustomersWillReturnCustomersFromRepository() throws Exception { - - // given - when(customerRepositoryMock.findCustomerByOptionalPrefixLike(null)).thenReturn(asList(TestCustomer.xxx, TestCustomer.yyy)); - - // when - final var pacs = customerController.listCustomers("mike@hostsharing.net", null, null); - - // then - assertThat(pacs).hasSize(2); - verify(contextMock).setCurrentUser("mike@hostsharing.net"); - verify(contextMock, never()).assumeRoles(any()); - } - - @Test - void findAllWithAssumedCustomerAdminRole() throws Exception { - - // given - when(customerRepositoryMock.findCustomerByOptionalPrefixLike(null)).thenReturn(singletonList(TestCustomer.yyy)); - - // when - final var pacs = customerController.listCustomers("mike@hostsharing.net", "customer#yyy.admin", null); - - // then - assertThat(pacs).hasSize(1); - verify(contextMock).setCurrentUser("mike@hostsharing.net"); - verify(contextMock).assumeRoles("customer#yyy.admin"); - } -} diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java new file mode 100644 index 00000000..cd0fef2a --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java @@ -0,0 +1,151 @@ +package net.hostsharing.hsadminng.hs.hspackage; + +import net.hostsharing.hsadminng.context.Context; +import net.hostsharing.hsadminng.hs.hscustomer.CustomerRepository; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.orm.jpa.JpaSystemException; + +import javax.persistence.EntityManager; +import javax.transaction.Transactional; +import java.util.List; + +import static net.hostsharing.test.JpaAttempt.attempt; +import static org.assertj.core.api.Assertions.assertThat; + +@DataJpaTest +@ComponentScan(basePackageClasses = { Context.class, CustomerRepository.class }) +class PackageRepositoryIntegrationTest { + + @Autowired + Context context; + + @Autowired + PackageRepository packageRepository; + + @Autowired EntityManager em; + + @Nested + class FindAllByOptionalNameLike { + + @Test + public void hostsharingAdmin_withoutAssumedRole_canNotViewAnyPackages_becauseThoseGrantsAreNotFollowed() { + // given + currentUser("mike@hostsharing.net"); + + // when + final var result = packageRepository.findAllByOptionalNameLike(null); + + // then + noPackagesAreReturned(result); + } + + @Test + public void hostsharingAdmin_withAssumedHostsharingAdminRole__canNotViewAnyPackages_becauseThoseGrantsAreNotFollowed() { + given: + currentUser("mike@hostsharing.net"); + assumedRoles("global#hostsharing.admin"); + + // when + final var result = packageRepository.findAllByOptionalNameLike(null); + + then: + noPackagesAreReturned(result); + } + + @Test + public void customerAdmin_withoutAssumedRole_canViewOnlyItsOwnPackages() { + // given: + currentUser("admin@aaa.example.com"); + + // when: + final var result = packageRepository.findAllByOptionalNameLike(null); + + // then: + exactlyThesePackagesAreReturned(result, "aaa00", "aaa01", "aaa02"); + } + + @Test + public void customerAdmin_withAssumedOwnedPackageAdminRole_canViewOnlyItsOwnPackages() { + currentUser("admin@aaa.example.com"); + assumedRoles("package#aaa00.admin"); + + final var result = packageRepository.findAllByOptionalNameLike(null); + + exactlyThesePackagesAreReturned(result, "aaa00"); + } + + @Test + public void customerAdmin_withAssumedAlienPackageAdminRole_cannotViewAnyPackages() { + // given: + currentUser("admin@aaa.example.com"); + assumedRoles("package#aab00.admin"); + + // when + final var attempt = attempt( + em, + () -> packageRepository.findAllByOptionalNameLike(null)); + + // then + attempt.assertExceptionWithRootCauseMessage( + JpaSystemException.class, + "[403] user admin@aaa.example.com", "has no permission to assume role package#aab00#admin"); + } + + @Test + void unknownUser_withoutAssumedRole_cannotViewAnyPackages() { + currentUser("unknown@example.org"); + + final var attempt = attempt( + em, + () -> packageRepository.findAllByOptionalNameLike(null)); + + attempt.assertExceptionWithRootCauseMessage( + JpaSystemException.class, + "hsadminng.currentUser defined as unknown@example.org, but does not exists"); + } + + @Test + @Transactional + void unknownUser_withAssumedCustomerRole_cannotViewAnyPackages() { + currentUser("unknown@example.org"); + assumedRoles("customer#aaa.admin"); + + final var attempt = attempt( + em, + () -> packageRepository.findAllByOptionalNameLike(null)); + + attempt.assertExceptionWithRootCauseMessage( + JpaSystemException.class, + "hsadminng.currentUser defined as unknown@example.org, but does not exists"); + } + + } + + void currentUser(final String currentUser) { + context.setCurrentUser(currentUser); + assertThat(context.getCurrentUser()).as("precondition").isEqualTo(currentUser); + } + + void assumedRoles(final String assumedRoles) { + context.assumeRoles(assumedRoles); + assertThat(context.getAssumedRoles()).as("precondition").containsExactly(assumedRoles.split(";")); + } + + void noPackagesAreReturned(final List actualResult) { + assertThat(actualResult) + .extracting(PackageEntity::getName) + .isEmpty(); + } + + + void exactlyThesePackagesAreReturned(final List actualResult, final String... packageNames) { + assertThat(actualResult) + .extracting(PackageEntity::getName) + .containsExactlyInAnyOrder(packageNames); + } + +}