From 048a976a1abfb709fc9b38ffbc6336a5df8e11da Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Mon, 19 Aug 2024 15:48:09 +0200 Subject: [PATCH] fix issues from code-review --- .../asset/HsHostingAssetController.java | 4 +- .../asset/HsHostingAssetRbacRepository.java | 2 +- .../asset/HsHostingAssetRealRepository.java | 4 +- .../asset/HsHostingAssetRepository.java | 24 ++++++++++ .../hs/validation/HsEntityValidator.java | 3 +- ...ostingAssetRepositoryIntegrationTest.java} | 47 ++++++++++++++++--- .../test/ContextBasedTestWithCleanup.java | 9 ++++ 7 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepository.java rename src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/{HsHostingAssetRbacRepositoryIntegrationTest.java => HsHostingAssetRepositoryIntegrationTest.java} (92%) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java index b988d510..4ae94c00 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetController.java @@ -45,7 +45,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi { private HsHostingAssetRealRepository realAssetRepo; @Autowired - private HsBookingItemRealRepository relBookingItemRepo; + private HsBookingItemRealRepository realBookingItemRepo; @Override @Transactional(readOnly = true) @@ -151,7 +151,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi { final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { entity.putConfig(KeyValueMap.from(resource.getConfig())); if (resource.getBookingItemUuid() != null) { - entity.setBookingItem(relBookingItemRepo.findByUuid(resource.getBookingItemUuid()) + entity.setBookingItem(realBookingItemRepo.findByUuid(resource.getBookingItemUuid()) .orElseThrow(() -> new EntityNotFoundException("ERROR: [400] bookingItemUuid %s not found".formatted( resource.getBookingItemUuid())))); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepository.java index af9908cc..c7f5aa34 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepository.java @@ -8,7 +8,7 @@ import java.util.Optional; import java.util.UUID; -public interface HsHostingAssetRbacRepository extends Repository { +public interface HsHostingAssetRbacRepository extends HsHostingAssetRepository, Repository { Optional findByUuid(final UUID serverUuid); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRealRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRealRepository.java index 2b40c96a..15a7de84 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRealRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRealRepository.java @@ -7,7 +7,7 @@ import java.util.List; import java.util.Optional; import java.util.UUID; -public interface HsHostingAssetRealRepository extends Repository { +public interface HsHostingAssetRealRepository extends HsHostingAssetRepository, Repository { Optional findByUuid(final UUID serverUuid); @@ -38,7 +38,7 @@ public interface HsHostingAssetRealRepository extends Repository { + + Optional findByUuid(final UUID serverUuid); + + List findByIdentifier(String assetIdentifier); + + List findAllByCriteriaImpl(UUID projectUuid, UUID parentAssetUuid, String type); + + default List findAllByCriteria(final UUID projectUuid, final UUID parentAssetUuid, final HsHostingAssetType type) { + return findAllByCriteriaImpl(projectUuid, parentAssetUuid, HsHostingAssetType.asString(type)); + } + + E save(HsHostingAsset current); + + int deleteByUuid(final UUID uuid); + + long count(); +} diff --git a/src/main/java/net/hostsharing/hsadminng/hs/validation/HsEntityValidator.java b/src/main/java/net/hostsharing/hsadminng/hs/validation/HsEntityValidator.java index 2b9c8c54..ce353a7d 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/validation/HsEntityValidator.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/validation/HsEntityValidator.java @@ -45,9 +45,8 @@ public abstract class HsEntityValidator { localEntityManager.set(em); try { return code.get(); - } catch (final RuntimeException e) { + } finally { localEntityManager.remove(); - throw e; } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java similarity index 92% rename from src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepositoryIntegrationTest.java rename to src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java index f4a2bb4c..469fbdf1 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRbacRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java @@ -12,6 +12,8 @@ import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.boot.test.mock.mockito.MockBean; @@ -39,7 +41,7 @@ import static org.assertj.core.api.Assertions.assertThat; @DataJpaTest @Import({ Context.class, JpaAttempt.class }) -class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCleanup { +class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanup { @Autowired HsHostingAssetRealRepository realAssetRepo; @@ -203,13 +205,14 @@ class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCl @Nested class FindAssets { - @Test - public void globalAdmin_withoutAssumedRole_canViewArbitraryAssetsOfAllDebitors() { + @ParameterizedTest + @EnumSource(TestCase.class) + public void globalAdmin_withoutAssumedRole_canViewArbitraryAssetsOfAllDebitors(final TestCase testCase) { // given context("superuser-alex@hostsharing.net"); // when - final var result = rbacAssetRepo.findAllByCriteria(null, null, MANAGED_WEBSPACE); + final var result = repoUnderTest(testCase).findAllByCriteria(null, null, MANAGED_WEBSPACE); // then exactlyTheseAssetsAreReturned( @@ -451,13 +454,45 @@ class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCl } void exactlyTheseAssetsAreReturned( - final List actualResult, + final List actualResult, final String... serverNames) { assertThat(actualResult) - .extracting(HsHostingAssetRbacEntity::toString) + .extracting(HsHostingAsset::toString) .extracting(input -> input.replaceAll("\\s+", " ")) .extracting(input -> input.replaceAll("\"", "")) .extracting(input -> input.replaceAll("\" : ", "\": ")) .containsExactlyInAnyOrder(serverNames); } + + void allTheseBookingProjectsAreReturned( + final List actualResult, + final String... serverNames) { + assertThat(actualResult) + .extracting(HsHostingAsset::toString) + .extracting(input -> input.replaceAll("\\s+", " ")) + .extracting(input -> input.replaceAll("\"", "")) + .extracting(input -> input.replaceAll("\" : ", "\": ")) + .contains(serverNames); + } + + private HsHostingAssetRepository repoUnderTest(final HsHostingAssetRepositoryIntegrationTest.TestCase testCase) { + return testCase.repo(HsHostingAssetRepositoryIntegrationTest.this); + } + + enum TestCase { + REAL { + @Override + HsHostingAssetRepository repo(final HsHostingAssetRepositoryIntegrationTest test) { + return test.realAssetRepo; + } + }, + RBAC { + @Override + HsHostingAssetRepository repo(final HsHostingAssetRepositoryIntegrationTest test) { + return test.rbacAssetRepo; + } + }; + + abstract HsHostingAssetRepository repo(final HsHostingAssetRepositoryIntegrationTest test); + } } diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/test/ContextBasedTestWithCleanup.java b/src/test/java/net/hostsharing/hsadminng/rbac/test/ContextBasedTestWithCleanup.java index 173ea031..5fb1c4e6 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/test/ContextBasedTestWithCleanup.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/test/ContextBasedTestWithCleanup.java @@ -293,7 +293,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest { /** * @return an array of all RBAC roles matching the given pattern + * + * Usually unused, but for temporary debugging purposes of findind role names for new tests. */ + @SuppressWarnings("unused") protected String[] roleNames(final String sqlLikeExpression) { final var pattern = Pattern.compile(sqlLikeExpression); //noinspection unchecked @@ -307,7 +310,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest { /** * Generates a diagram of the RBAC-Grants to the current subjects (user or assumed roles). + * + * Usually unused, but for temporary use for debugging and other analysis. */ + @SuppressWarnings("unused") protected void generateRbacDiagramForCurrentSubjects(final EnumSet include, final String name) { RbacGrantsDiagramService.writeToFile( name, @@ -318,7 +324,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest { /** * Generates a diagram of the RBAC-Grants for the given object and permission. + * + * Usually unused, but for temporary use for debugging and other analysis. */ + @SuppressWarnings("unused") protected void generateRbacDiagramForObjectPermission(final UUID targetObject, final String rbacOp, final String name) { RbacGrantsDiagramService.writeToFile( name,