fix prefix enrichment issues from code-review

This commit is contained in:
Michael Hoennig 2024-06-14 14:46:32 +02:00
parent 24811661d4
commit 8163c1e338
8 changed files with 85 additions and 57 deletions

View File

@ -62,8 +62,7 @@ public class HsBookingItemController implements HsBookingItemsApi {
final var entityToSave = mapper.map(body, HsBookingItemEntity.class, RESOURCE_TO_ENTITY_POSTMAPPER); final var entityToSave = mapper.map(body, HsBookingItemEntity.class, RESOURCE_TO_ENTITY_POSTMAPPER);
final HsBookingItemEntity entityToSave1 = bookingItemRepo.save(entityToSave); final var saved = HsBookingItemEntityValidatorRegistry.validated(bookingItemRepo.save(entityToSave));
final var saved = HsBookingItemEntityValidatorRegistry.validated(entityToSave1);
final var uri = final var uri =
MvcUriComponentsBuilder.fromController(getClass()) MvcUriComponentsBuilder.fromController(getClass())
@ -84,7 +83,7 @@ public class HsBookingItemController implements HsBookingItemsApi {
context.define(currentUser, assumedRoles); context.define(currentUser, assumedRoles);
final var result = bookingItemRepo.findByUuid(bookingItemUuid); final var result = bookingItemRepo.findByUuid(bookingItemUuid);
result.ifPresent(entity -> em.detach(entity)); result.ifPresent(entity -> em.detach(entity)); // prevent further LAZY-loading
return result return result
.map(bookingItemEntity -> ResponseEntity.ok( .map(bookingItemEntity -> ResponseEntity.ok(
mapper.map(bookingItemEntity, HsBookingItemResource.class, ENTITY_TO_RESOURCE_POSTMAPPER))) mapper.map(bookingItemEntity, HsBookingItemResource.class, ENTITY_TO_RESOURCE_POSTMAPPER)))

View File

@ -21,25 +21,33 @@ public class HsBookingItemEntityValidator extends HsEntityValidator<HsBookingIte
public List<String> validate(final HsBookingItemEntity bookingItem) { public List<String> validate(final HsBookingItemEntity bookingItem) {
return sequentiallyValidate( return sequentiallyValidate(
() -> enrich(prefix(bookingItem.toShortString(), "resources"), validateProperties(bookingItem.getResources())), () -> validateProperties(bookingItem),
() -> enrich(prefix(bookingItem.toShortString(), "parentItem"), optionallyValidate(bookingItem.getParentItem())), () -> optionallyValidate(bookingItem.getParentItem()),
() -> validateAgainstSubEntities(bookingItem) () -> validateAgainstSubEntities(bookingItem)
); );
} }
private List<String> validateProperties(final HsBookingItemEntity bookingItem) {
return enrich(prefix(bookingItem.toShortString(), "resources"), validateProperties(bookingItem.getResources()));
}
private static List<String> optionallyValidate(final HsBookingItemEntity bookingItem) { private static List<String> optionallyValidate(final HsBookingItemEntity bookingItem) {
return bookingItem != null ? HsBookingItemEntityValidatorRegistry.doValidate(bookingItem) : emptyList(); return bookingItem != null
? enrich(prefix(bookingItem.toShortString(), ""),
HsBookingItemEntityValidatorRegistry.doValidate(bookingItem))
: emptyList();
} }
protected List<String> validateAgainstSubEntities(final HsBookingItemEntity bookingItem) { protected List<String> validateAgainstSubEntities(final HsBookingItemEntity bookingItem) {
return Stream.concat( return enrich(prefix(bookingItem.toShortString(), "resources"),
Stream.concat(
stream(propertyValidators) stream(propertyValidators)
.map(propDef -> propDef.validateTotals(bookingItem)) .map(propDef -> propDef.validateTotals(bookingItem))
.flatMap(Collection::stream), .flatMap(Collection::stream),
stream(propertyValidators) stream(propertyValidators)
.filter(ValidatableProperty::isTotalsValidator) .filter(ValidatableProperty::isTotalsValidator)
.map(prop -> validateMaxTotalValue(bookingItem, prop)) .map(prop -> validateMaxTotalValue(bookingItem, prop))
).filter(Objects::nonNull).toList(); ).filter(Objects::nonNull).toList());
} }
// TODO.refa: convert into generic shape like multi-options validator // TODO.refa: convert into generic shape like multi-options validator
@ -56,13 +64,13 @@ public class HsBookingItemEntityValidator extends HsEntityValidator<HsBookingIte
final var maxValue = getNonNullIntegerValue(propDef, bookingItem.getResources()); final var maxValue = getNonNullIntegerValue(propDef, bookingItem.getResources());
if (propDef.thresholdPercentage() != null ) { if (propDef.thresholdPercentage() != null ) {
return totalValue > (maxValue * propDef.thresholdPercentage() / 100) return totalValue > (maxValue * propDef.thresholdPercentage() / 100)
? "%s' total is %d%s, thus exceeds max total %s %d%s, which is above threshold of %d%%" ? "%s' maximum total is %d%s, but actual total %s %d%s, which exceeds threshold of %d%%"
.formatted(propName, totalValue, propUnit, propName, maxValue, propUnit, propDef.thresholdPercentage()) .formatted(propName, maxValue, propUnit, propName, totalValue, propUnit, propDef.thresholdPercentage())
: null; : null;
} else { } else {
return totalValue > maxValue return totalValue > maxValue
? "%s' total is %d%s, thus exceeds max total %s %d%s" ? "%s' maximum total is %d%s, but actual total %s %d%s"
.formatted(propName, totalValue, propUnit, propName, maxValue, propUnit) .formatted(propName, maxValue, propUnit, propName, totalValue, propUnit)
: null; : null;
} }
} }

View File

@ -23,29 +23,38 @@ public class HsHostingAssetEntityValidator extends HsEntityValidator<HsHostingAs
@Override @Override
public List<String> validate(final HsHostingAssetEntity assetEntity) { public List<String> validate(final HsHostingAssetEntity assetEntity) {
return sequentiallyValidate( return sequentiallyValidate(
() -> enrich(prefix(assetEntity.toShortString(), "config"), validateProperties(assetEntity.getConfig())), () -> validateProperties(assetEntity),
() -> enrich(prefix(assetEntity.toShortString(), "bookingItem"), optionallyValidate(assetEntity.getBookingItem())), () -> optionallyValidate(assetEntity.getBookingItem()),
() -> enrich(prefix(assetEntity.toShortString(), "parentAsset"), optionallyValidate(assetEntity.getParentAsset())), () -> optionallyValidate(assetEntity.getParentAsset()),
() -> validateSubEntities(assetEntity) () -> validateAgainstSubEntities(assetEntity)
); );
} }
private List<String> validateProperties(final HsHostingAssetEntity assetEntity) {
return enrich(prefix(assetEntity.toShortString(), "config"), validateProperties(assetEntity.getConfig()));
}
private static List<String> optionallyValidate(final HsHostingAssetEntity assetEntity) { private static List<String> optionallyValidate(final HsHostingAssetEntity assetEntity) {
return assetEntity != null ? return assetEntity != null
HsHostingAssetEntityValidatorRegistry.forType(assetEntity.getType()).validate(assetEntity) : ? enrich(prefix(assetEntity.toShortString(), "parentAsset"),
emptyList(); HsHostingAssetEntityValidatorRegistry.forType(assetEntity.getType()).validate(assetEntity))
: emptyList();
} }
private static List<String> optionallyValidate(final HsBookingItemEntity bookingItem) { private static List<String> 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<String> validateSubEntities(final HsHostingAssetEntity assetEntity) { protected List<String> validateAgainstSubEntities(final HsHostingAssetEntity assetEntity) {
return stream(propertyValidators) return enrich(prefix(assetEntity.toShortString(), "config"),
stream(propertyValidators)
.filter(ValidatableProperty::isTotalsValidator) .filter(ValidatableProperty::isTotalsValidator)
.map(prop -> validateMaxTotalValue(assetEntity, prop)) .map(prop -> validateMaxTotalValue(assetEntity, prop))
.filter(Objects::nonNull) .filter(Objects::nonNull)
.toList(); .toList());
} }
private String validateMaxTotalValue( private String validateMaxTotalValue(
@ -60,8 +69,8 @@ public class HsHostingAssetEntityValidator extends HsEntityValidator<HsHostingAs
.reduce(0, Integer::sum); .reduce(0, Integer::sum);
final var maxValue = getNonNullIntegerValue(propDef, hostingAsset.getConfig()); final var maxValue = getNonNullIntegerValue(propDef, hostingAsset.getConfig());
return totalValue > maxValue return totalValue > maxValue
? "%s' total is %d%s, thus exceeds max total %s %d%s".formatted( ? "%s' maximum total is %d%s, but actual total is %s %d%s".formatted(
propName, totalValue, propUnit, propName, maxValue, propUnit) propName, maxValue, propUnit, propName, totalValue, propUnit)
: null; : null;
} }
} }

View File

@ -109,10 +109,10 @@ class HsCloudServerBookingItemValidatorUnitTest {
// then // then
assertThat(result).containsExactlyInAnyOrder( 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.resources.CPUs' maximum total is 4, but actual total CPUs 5",
"'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.resources.RAM' maximum total is 20 GB, but actual total RAM 30 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.resources.SSD' maximum total is 100 GB, but actual total SSD 150 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.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB"
); );
} }
} }

View File

@ -120,10 +120,10 @@ class HsManagedServerBookingItemValidatorUnitTest {
// then // then
assertThat(result).containsExactlyInAnyOrder( assertThat(result).containsExactlyInAnyOrder(
"'D-12345:Test-Project:null.parentItem.CPUs' total is 5, thus exceeds max total CPUs 4", "'D-12345:Test-Project:null.resources.CPUs' maximum total is 4, but actual total CPUs 5",
"'D-12345:Test-Project:null.parentItem.RAM' total is 30 GB, thus exceeds max total RAM 20 GB", "'D-12345:Test-Project:null.resources.RAM' maximum total is 20 GB, but actual total RAM 30 GB",
"'D-12345:Test-Project:null.parentItem.SSD' total is 150 GB, thus exceeds max total SSD 100 GB", "'D-12345:Test-Project:null.resources.SSD' maximum total is 100 GB, but actual total SSD 150 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.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB"
); );
} }
@ -132,6 +132,7 @@ class HsManagedServerBookingItemValidatorUnitTest {
// given // given
final var managedWebspaceBookingItem = HsBookingItemEntity.builder() final var managedWebspaceBookingItem = HsBookingItemEntity.builder()
.type(MANAGED_WEBSPACE) .type(MANAGED_WEBSPACE)
.project(project)
.caption("test Managed-Webspace") .caption("test Managed-Webspace")
.resources(ofEntries( .resources(ofEntries(
entry("SSD", 100), entry("SSD", 100),
@ -166,10 +167,10 @@ class HsManagedServerBookingItemValidatorUnitTest {
// then // then
assertThat(result).containsExactlyInAnyOrder( assertThat(result).containsExactlyInAnyOrder(
"Multi=1 allows at maximum 25 unix users, but 26 found", "'D-12345:Test-Project:test Managed-Webspace.resources.Multi=1 allows at maximum 25 unix users, but 26 found",
"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 database users, but 6 found",
"Multi=1 allows at maximum 5 databases, but 9 found", "'D-12345:Test-Project:test Managed-Webspace.resources.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 250 databases, but 260 found"
); );
} }

View File

@ -1,6 +1,8 @@
package net.hostsharing.hsadminng.hs.booking.item.validators; 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.item.HsBookingItemEntity;
import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectEntity;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import static java.util.List.of; import static java.util.List.of;
@ -13,6 +15,14 @@ import static org.assertj.core.api.Assertions.assertThat;
class HsPrivateCloudBookingItemValidatorTest { class HsPrivateCloudBookingItemValidatorTest {
final HsBookingDebitorEntity debitor = HsBookingDebitorEntity.builder()
.debitorNumber(12345)
.build();
final HsBookingProjectEntity project = HsBookingProjectEntity.builder()
.debitor(debitor)
.caption("Test-Project")
.build();
@Test @Test
void validatesPropertyTotals() { void validatesPropertyTotals() {
// given // given
@ -57,6 +67,7 @@ class HsPrivateCloudBookingItemValidatorTest {
void validatesExceedingPropertyTotals() { void validatesExceedingPropertyTotals() {
// given // given
final var privateCloudBookingItemEntity = HsBookingItemEntity.builder() final var privateCloudBookingItemEntity = HsBookingItemEntity.builder()
.project(project)
.type(PRIVATE_CLOUD) .type(PRIVATE_CLOUD)
.resources(ofEntries( .resources(ofEntries(
entry("CPUs", 4), entry("CPUs", 4),
@ -91,10 +102,10 @@ class HsPrivateCloudBookingItemValidatorTest {
// then // then
assertThat(result).containsExactlyInAnyOrder( assertThat(result).containsExactlyInAnyOrder(
"CPUs' total is 5, thus exceeds max total CPUs 4", "'D-12345:Test-Project:null.resources.CPUs' maximum total is 4, but actual total CPUs 5",
"RAM' total is 30 GB, thus exceeds max total RAM 20 GB", "'D-12345:Test-Project:null.resources.RAM' maximum total is 20 GB, but actual total RAM 30 GB",
"SSD' total is 150 GB, thus exceeds max total SSD 100 GB", "'D-12345:Test-Project:null.resources.SSD' maximum total is 100 GB, but actual total SSD 150 GB",
"Traffic' total is 5500 GB, thus exceeds max total Traffic 5000 GB" "'D-12345:Test-Project:null.resources.Traffic' maximum total is 5000 GB, but actual total Traffic 5500 GB"
); );
} }

View File

@ -199,10 +199,10 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
.extract().header("Location"); // @formatter:on .extract().header("Location"); // @formatter:on
// finally, the new asset can be accessed under the generated UUID // 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)); location.substring(location.lastIndexOf('/') + 1));
assertThat(newUserUuid).isNotNull(); assertThat(newWebspace).isNotNull();
toCleanup(HsHostingAssetEntity.class, newUserUuid); toCleanup(HsHostingAssetEntity.class, newWebspace);
} }
@Test @Test
@ -334,7 +334,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
{ {
"statusPhrase": "Bad Request", "statusPhrase": "Bad Request",
"message": "[ "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 """.replaceAll(" +<<<", ""))); // @formatter:on

View File

@ -25,8 +25,8 @@ class HsHostingAssetEntityValidatorUnitTest {
// then // then
assertThat(result).isInstanceOf(ValidationException.class) assertThat(result).isInstanceOf(ValidationException.class)
.hasMessageContaining( .hasMessageContaining(
"MANAGED_SERVER:vm1234.config.monit_max_ssd_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_cpu_usage' is required but missing",
"MANAGED_SERVER:vm1234.config.monit_max_ram_usage is required but missing"); "'MANAGED_SERVER:vm1234.config.monit_max_ram_usage' is required but missing");
} }
} }