From 8163c1e33874b9039de42d583eab1a482e34d42b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 14 Jun 2024 14:46:32 +0200 Subject: [PATCH] fix prefix enrichment issues from code-review --- .../booking/item/HsBookingItemController.java | 5 +-- .../HsBookingItemEntityValidator.java | 38 ++++++++++------- .../HsHostingAssetEntityValidator.java | 41 +++++++++++-------- ...oudServerBookingItemValidatorUnitTest.java | 8 ++-- ...gedServerBookingItemValidatorUnitTest.java | 17 ++++---- ...sPrivateCloudBookingItemValidatorTest.java | 19 +++++++-- ...sHostingAssetControllerAcceptanceTest.java | 8 ++-- ...HsHostingAssetEntityValidatorUnitTest.java | 6 +-- 8 files changed, 85 insertions(+), 57 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java index 6c66e032..1343378c 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemController.java @@ -62,8 +62,7 @@ public class HsBookingItemController implements HsBookingItemsApi { final var entityToSave = mapper.map(body, HsBookingItemEntity.class, RESOURCE_TO_ENTITY_POSTMAPPER); - final HsBookingItemEntity entityToSave1 = bookingItemRepo.save(entityToSave); - final var saved = HsBookingItemEntityValidatorRegistry.validated(entityToSave1); + final var saved = HsBookingItemEntityValidatorRegistry.validated(bookingItemRepo.save(entityToSave)); final var uri = MvcUriComponentsBuilder.fromController(getClass()) @@ -84,7 +83,7 @@ public class HsBookingItemController implements HsBookingItemsApi { context.define(currentUser, assumedRoles); final var result = bookingItemRepo.findByUuid(bookingItemUuid); - result.ifPresent(entity -> em.detach(entity)); + result.ifPresent(entity -> em.detach(entity)); // prevent further LAZY-loading return result .map(bookingItemEntity -> ResponseEntity.ok( mapper.map(bookingItemEntity, HsBookingItemResource.class, ENTITY_TO_RESOURCE_POSTMAPPER))) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsBookingItemEntityValidator.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsBookingItemEntityValidator.java index 85468088..7d002bac 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsBookingItemEntityValidator.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsBookingItemEntityValidator.java @@ -21,25 +21,33 @@ public class HsBookingItemEntityValidator extends HsEntityValidator validate(final HsBookingItemEntity bookingItem) { return sequentiallyValidate( - () -> enrich(prefix(bookingItem.toShortString(), "resources"), validateProperties(bookingItem.getResources())), - () -> enrich(prefix(bookingItem.toShortString(), "parentItem"), optionallyValidate(bookingItem.getParentItem())), + () -> validateProperties(bookingItem), + () -> optionallyValidate(bookingItem.getParentItem()), () -> validateAgainstSubEntities(bookingItem) ); } + private List validateProperties(final HsBookingItemEntity bookingItem) { + return enrich(prefix(bookingItem.toShortString(), "resources"), validateProperties(bookingItem.getResources())); + } + private static List optionallyValidate(final HsBookingItemEntity bookingItem) { - return bookingItem != null ? HsBookingItemEntityValidatorRegistry.doValidate(bookingItem) : emptyList(); + return bookingItem != null + ? enrich(prefix(bookingItem.toShortString(), ""), + HsBookingItemEntityValidatorRegistry.doValidate(bookingItem)) + : emptyList(); } protected List validateAgainstSubEntities(final HsBookingItemEntity bookingItem) { - return Stream.concat( - stream(propertyValidators) - .map(propDef -> propDef.validateTotals(bookingItem)) - .flatMap(Collection::stream), - stream(propertyValidators) - .filter(ValidatableProperty::isTotalsValidator) - .map(prop -> validateMaxTotalValue(bookingItem, prop)) - ).filter(Objects::nonNull).toList(); + return enrich(prefix(bookingItem.toShortString(), "resources"), + Stream.concat( + stream(propertyValidators) + .map(propDef -> propDef.validateTotals(bookingItem)) + .flatMap(Collection::stream), + stream(propertyValidators) + .filter(ValidatableProperty::isTotalsValidator) + .map(prop -> validateMaxTotalValue(bookingItem, prop)) + ).filter(Objects::nonNull).toList()); } // TODO.refa: convert into generic shape like multi-options validator @@ -56,13 +64,13 @@ public class HsBookingItemEntityValidator extends HsEntityValidator (maxValue * propDef.thresholdPercentage() / 100) - ? "%s' total is %d%s, thus exceeds max total %s %d%s, which is above threshold of %d%%" - .formatted(propName, totalValue, propUnit, propName, maxValue, propUnit, propDef.thresholdPercentage()) + ? "%s' maximum total is %d%s, but actual total %s %d%s, which exceeds threshold of %d%%" + .formatted(propName, maxValue, propUnit, propName, totalValue, propUnit, propDef.thresholdPercentage()) : null; } else { return totalValue > maxValue - ? "%s' total is %d%s, thus exceeds max total %s %d%s" - .formatted(propName, totalValue, propUnit, propName, maxValue, propUnit) + ? "%s' maximum total is %d%s, but actual total %s %d%s" + .formatted(propName, maxValue, propUnit, propName, totalValue, propUnit) : null; } } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidator.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidator.java index 0757a3e3..3a0438ee 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidator.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidator.java @@ -23,29 +23,38 @@ public class HsHostingAssetEntityValidator extends HsEntityValidator validate(final HsHostingAssetEntity assetEntity) { return sequentiallyValidate( - () -> enrich(prefix(assetEntity.toShortString(), "config"), validateProperties(assetEntity.getConfig())), - () -> enrich(prefix(assetEntity.toShortString(), "bookingItem"), optionallyValidate(assetEntity.getBookingItem())), - () -> enrich(prefix(assetEntity.toShortString(), "parentAsset"), optionallyValidate(assetEntity.getParentAsset())), - () -> validateSubEntities(assetEntity) + () -> validateProperties(assetEntity), + () -> optionallyValidate(assetEntity.getBookingItem()), + () -> optionallyValidate(assetEntity.getParentAsset()), + () -> validateAgainstSubEntities(assetEntity) ); } + private List validateProperties(final HsHostingAssetEntity assetEntity) { + return enrich(prefix(assetEntity.toShortString(), "config"), validateProperties(assetEntity.getConfig())); + } + private static List optionallyValidate(final HsHostingAssetEntity assetEntity) { - return assetEntity != null ? - HsHostingAssetEntityValidatorRegistry.forType(assetEntity.getType()).validate(assetEntity) : - emptyList(); + return assetEntity != null + ? enrich(prefix(assetEntity.toShortString(), "parentAsset"), + HsHostingAssetEntityValidatorRegistry.forType(assetEntity.getType()).validate(assetEntity)) + : emptyList(); } private static List optionallyValidate(final HsBookingItemEntity bookingItem) { - return bookingItem != null ? HsBookingItemEntityValidatorRegistry.doValidate(bookingItem) : emptyList(); + return bookingItem != null + ? enrich(prefix(bookingItem.toShortString(), "bookingItem"), + HsBookingItemEntityValidatorRegistry.doValidate(bookingItem)) + : emptyList(); } - protected List validateSubEntities(final HsHostingAssetEntity assetEntity) { - return stream(propertyValidators) - .filter(ValidatableProperty::isTotalsValidator) - .map(prop -> validateMaxTotalValue(assetEntity, prop)) - .filter(Objects::nonNull) - .toList(); + protected List validateAgainstSubEntities(final HsHostingAssetEntity assetEntity) { + return enrich(prefix(assetEntity.toShortString(), "config"), + stream(propertyValidators) + .filter(ValidatableProperty::isTotalsValidator) + .map(prop -> validateMaxTotalValue(assetEntity, prop)) + .filter(Objects::nonNull) + .toList()); } private String validateMaxTotalValue( @@ -60,8 +69,8 @@ public class HsHostingAssetEntityValidator extends HsEntityValidator maxValue - ? "%s' total is %d%s, thus exceeds max total %s %d%s".formatted( - propName, totalValue, propUnit, propName, maxValue, propUnit) + ? "%s' maximum total is %d%s, but actual total is %s %d%s".formatted( + propName, maxValue, propUnit, propName, totalValue, propUnit) : null; } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsCloudServerBookingItemValidatorUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsCloudServerBookingItemValidatorUnitTest.java index cf10e3f9..787b4c08 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsCloudServerBookingItemValidatorUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsCloudServerBookingItemValidatorUnitTest.java @@ -109,10 +109,10 @@ class HsCloudServerBookingItemValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "'D-12345:Test-Project:Test Cloud-Server.parentItem.CPUs' total is 5, thus exceeds max total CPUs 4", - "'D-12345:Test-Project:Test Cloud-Server.parentItem.RAM' total is 30 GB, thus exceeds max total RAM 20 GB", - "'D-12345:Test-Project:Test Cloud-Server.parentItem.SSD' total is 150 GB, thus exceeds max total SSD 100 GB", - "'D-12345:Test-Project:Test Cloud-Server.parentItem.Traffic' total is 5500 GB, thus exceeds max total Traffic 5000 GB" + "'D-12345:Test-Project:Test Cloud.resources.CPUs' maximum total is 4, but actual total CPUs 5", + "'D-12345:Test-Project:Test Cloud.resources.RAM' maximum total is 20 GB, but actual total RAM 30 GB", + "'D-12345:Test-Project:Test Cloud.resources.SSD' maximum total is 100 GB, but actual total SSD 150 GB", + "'D-12345:Test-Project:Test Cloud.resources.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB" ); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsManagedServerBookingItemValidatorUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsManagedServerBookingItemValidatorUnitTest.java index 994f4ec9..1fe54a82 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsManagedServerBookingItemValidatorUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsManagedServerBookingItemValidatorUnitTest.java @@ -120,10 +120,10 @@ class HsManagedServerBookingItemValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "'D-12345:Test-Project:null.parentItem.CPUs' total is 5, thus exceeds max total CPUs 4", - "'D-12345:Test-Project:null.parentItem.RAM' total is 30 GB, thus exceeds max total RAM 20 GB", - "'D-12345:Test-Project:null.parentItem.SSD' total is 150 GB, thus exceeds max total SSD 100 GB", - "'D-12345:Test-Project:null.parentItem.Traffic' total is 5500 GB, thus exceeds max total Traffic 5000 GB" + "'D-12345:Test-Project:null.resources.CPUs' maximum total is 4, but actual total CPUs 5", + "'D-12345:Test-Project:null.resources.RAM' maximum total is 20 GB, but actual total RAM 30 GB", + "'D-12345:Test-Project:null.resources.SSD' maximum total is 100 GB, but actual total SSD 150 GB", + "'D-12345:Test-Project:null.resources.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB" ); } @@ -132,6 +132,7 @@ class HsManagedServerBookingItemValidatorUnitTest { // given final var managedWebspaceBookingItem = HsBookingItemEntity.builder() .type(MANAGED_WEBSPACE) + .project(project) .caption("test Managed-Webspace") .resources(ofEntries( entry("SSD", 100), @@ -166,10 +167,10 @@ class HsManagedServerBookingItemValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "Multi=1 allows at maximum 25 unix users, but 26 found", - "Multi=1 allows at maximum 5 database users, but 6 found", - "Multi=1 allows at maximum 5 databases, but 9 found", - "Multi=1 allows at maximum 250 databases, but 260 found" + "'D-12345:Test-Project:test Managed-Webspace.resources.Multi=1 allows at maximum 25 unix users, but 26 found", + "'D-12345:Test-Project:test Managed-Webspace.resources.Multi=1 allows at maximum 5 database users, but 6 found", + "'D-12345:Test-Project:test Managed-Webspace.resources.Multi=1 allows at maximum 5 databases, but 9 found", + "'D-12345:Test-Project:test Managed-Webspace.resources.Multi=1 allows at maximum 250 databases, but 260 found" ); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsPrivateCloudBookingItemValidatorTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsPrivateCloudBookingItemValidatorTest.java index 974f9d42..e54a0670 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsPrivateCloudBookingItemValidatorTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsPrivateCloudBookingItemValidatorTest.java @@ -1,6 +1,8 @@ package net.hostsharing.hsadminng.hs.booking.item.validators; +import net.hostsharing.hsadminng.hs.booking.debitor.HsBookingDebitorEntity; import net.hostsharing.hsadminng.hs.booking.item.HsBookingItemEntity; +import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectEntity; import org.junit.jupiter.api.Test; import static java.util.List.of; @@ -13,6 +15,14 @@ import static org.assertj.core.api.Assertions.assertThat; class HsPrivateCloudBookingItemValidatorTest { + final HsBookingDebitorEntity debitor = HsBookingDebitorEntity.builder() + .debitorNumber(12345) + .build(); + final HsBookingProjectEntity project = HsBookingProjectEntity.builder() + .debitor(debitor) + .caption("Test-Project") + .build(); + @Test void validatesPropertyTotals() { // given @@ -57,6 +67,7 @@ class HsPrivateCloudBookingItemValidatorTest { void validatesExceedingPropertyTotals() { // given final var privateCloudBookingItemEntity = HsBookingItemEntity.builder() + .project(project) .type(PRIVATE_CLOUD) .resources(ofEntries( entry("CPUs", 4), @@ -91,10 +102,10 @@ class HsPrivateCloudBookingItemValidatorTest { // then assertThat(result).containsExactlyInAnyOrder( - "CPUs' total is 5, thus exceeds max total CPUs 4", - "RAM' total is 30 GB, thus exceeds max total RAM 20 GB", - "SSD' total is 150 GB, thus exceeds max total SSD 100 GB", - "Traffic' total is 5500 GB, thus exceeds max total Traffic 5000 GB" + "'D-12345:Test-Project:null.resources.CPUs' maximum total is 4, but actual total CPUs 5", + "'D-12345:Test-Project:null.resources.RAM' maximum total is 20 GB, but actual total RAM 30 GB", + "'D-12345:Test-Project:null.resources.SSD' maximum total is 100 GB, but actual total SSD 150 GB", + "'D-12345:Test-Project:null.resources.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB" ); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerAcceptanceTest.java index a2f1650c..e9f8180d 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerAcceptanceTest.java @@ -199,10 +199,10 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup .extract().header("Location"); // @formatter:on // finally, the new asset can be accessed under the generated UUID - final var newUserUuid = UUID.fromString( + final var newWebspace = UUID.fromString( location.substring(location.lastIndexOf('/') + 1)); - assertThat(newUserUuid).isNotNull(); - toCleanup(HsHostingAssetEntity.class, newUserUuid); + assertThat(newWebspace).isNotNull(); + toCleanup(HsHostingAssetEntity.class, newWebspace); } @Test @@ -334,7 +334,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup { "statusPhrase": "Bad Request", "message": "[ - <<<'MANAGED_WEBSPACE:fir01.bookingItem.Multi=1 allows at maximum 25 unix users, but 26 found + <<<'D-1000111:D-1000111 default project:separate ManagedWebspace.resources.Multi=1 allows at maximum 25 unix users, but 26 found <<<]" } """.replaceAll(" +<<<", ""))); // @formatter:on diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidatorUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidatorUnitTest.java index 82371eaa..b92e5dc9 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidatorUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsHostingAssetEntityValidatorUnitTest.java @@ -25,8 +25,8 @@ class HsHostingAssetEntityValidatorUnitTest { // then assertThat(result).isInstanceOf(ValidationException.class) .hasMessageContaining( - "MANAGED_SERVER:vm1234.config.monit_max_ssd_usage is required but missing", - "MANAGED_SERVER:vm1234.config.monit_max_cpu_usage is required but missing", - "MANAGED_SERVER:vm1234.config.monit_max_ram_usage is required but missing"); + "'MANAGED_SERVER:vm1234.config.monit_max_ssd_usage' is required but missing", + "'MANAGED_SERVER:vm1234.config.monit_max_cpu_usage' is required but missing", + "'MANAGED_SERVER:vm1234.config.monit_max_ram_usage' is required but missing"); } }