From 22f493be73b923fbf9a5418565ef24cb2547de8d Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 9 Aug 2024 15:18:12 +0200 Subject: [PATCH] implement ValidatableProperty.writeOnce --- .../hs/booking/item/HsBookingItemEntity.java | 4 +- .../hosting/asset/HsHostingAssetEntity.java | 4 +- .../asset/HsHostingAssetEntityPatcher.java | 2 +- .../hs/validation/PropertiesProvider.java | 8 +- .../hs/validation/ValidatableProperty.java | 7 +- .../hsadminng/mapper/PatchableMapWrapper.java | 22 +++++ ...sHostingAssetControllerAcceptanceTest.java | 4 +- ...HostingAssetRepositoryIntegrationTest.java | 3 +- ...lAddressHostingAssetValidatorUnitTest.java | 90 ++++++++++++++++--- .../migration/HsHostingAssetRealEntity.java | 4 +- .../validation/PasswordPropertyUnitTest.java | 7 +- .../rbac/test/PatchUnitTestBase.java | 4 +- 12 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java index a7b9db66..58a5d4b8 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java @@ -158,8 +158,8 @@ public class HsBookingItemEntity implements Stringifyable, BaseEntity directProps() { - return resources; + public PatchableMapWrapper directProps() { + return getResources(); } @Override diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java index 4083ab36..46b315ff 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java @@ -128,8 +128,8 @@ public class HsHostingAssetEntity implements HsHostingAsset { } @Override - public Map directProps() { - return config; + public PatchableMapWrapper directProps() { + return getConfig(); } @Override diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java index 856b4243..c16c22e0 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntityPatcher.java @@ -14,7 +14,7 @@ public class HsHostingAssetEntityPatcher implements EntityPatcher directProps(); + PatchableMapWrapper directProps(); Object getContextValue(final String propName); default T getDirectValue(final String propName, final Class clazz) { @@ -16,6 +16,10 @@ public interface PropertiesProvider { return cast(propName, directProps().get(propName), clazz, defaultValue); } + default boolean isPatched(String propertyName) { + return directProps().isPatched(propertyName); + } + default T getContextValue(final String propName, final Class clazz) { return cast(propName, getContextValue(propName), clazz, null); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/validation/ValidatableProperty.java b/src/main/java/net/hostsharing/hsadminng/hs/validation/ValidatableProperty.java index 72fc444f..3f5011e9 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/validation/ValidatableProperty.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/validation/ValidatableProperty.java @@ -206,6 +206,9 @@ public abstract class ValidatableProperty

, T if (required == TRUE) { result.add(propertyName + "' is required but missing"); } + if (isWriteOnce() && propsProvider.isLoaded() && propsProvider.isPatched(propertyName) ) { + result.add(propertyName + "' is write-once but got removed"); + } validateRequiresAtLeastOneOf(result, propsProvider); } if (propValue != null){ @@ -248,10 +251,12 @@ public abstract class ValidatableProperty

, T } protected void validate(final List result, final T propValue, final PropertiesProvider propProvider) { - if (isReadOnly() && propValue != null) { result.add(propertyName + "' is readonly but given as " + display(propValue)); } + if (isWriteOnce() && propProvider.isLoaded() && propValue != null && propProvider.isPatched(propertyName) ) { + result.add(propertyName + "' is write-once but given as " + display(propValue)); + } } public void verifyConsistency(final Map.Entry, ?> typeDef) { diff --git a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java index 01b71ead..6f08b923 100644 --- a/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java +++ b/src/main/java/net/hostsharing/hsadminng/mapper/PatchableMapWrapper.java @@ -7,7 +7,9 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import jakarta.validation.constraints.NotNull; import java.util.Collection; +import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Consumer; @@ -23,6 +25,7 @@ public class PatchableMapWrapper implements Map { .configure(SerializationFeature.INDENT_OUTPUT, true); private final Map delegate; + private final Set patched = new HashSet<>(); private PatchableMapWrapper(final Map map) { delegate = map; @@ -36,6 +39,10 @@ public class PatchableMapWrapper implements Map { }); } + public static PatchableMapWrapper of(final Map delegate) { + return new PatchableMapWrapper(delegate); + } + @NotNull public static ImmutablePair entry(final String key, final E value) { return new ImmutablePair<>(key, value); @@ -45,6 +52,7 @@ public class PatchableMapWrapper implements Map { if (entries != null ) { delegate.clear(); delegate.putAll(entries); + patched.clear(); } } @@ -58,6 +66,10 @@ public class PatchableMapWrapper implements Map { }); } + public boolean isPatched(final String propertyName) { + return patched.contains(propertyName); + } + @SneakyThrows public String toString() { return jsonWriter.writeValueAsString(delegate); @@ -92,11 +104,17 @@ public class PatchableMapWrapper implements Map { @Override public T put(final String key, final T value) { + if (!Objects.equals(value, delegate.get(key))) { + patched.add(key); + } return delegate.put(key, value); } @Override public T remove(final Object key) { + if (delegate.containsKey(key.toString())) { + patched.add(key.toString()); + } return delegate.remove(key); } @@ -107,20 +125,24 @@ public class PatchableMapWrapper implements Map { @Override public void clear() { + patched.addAll(delegate.keySet()); delegate.clear(); } @Override + @NotNull public Set keySet() { return delegate.keySet(); } @Override + @NotNull public Collection values() { return delegate.values(); } @Override + @NotNull public Set> entrySet() { return delegate.entrySet(); } 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 1e822604..476b6bb0 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 @@ -546,7 +546,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup .contentType(ContentType.JSON) .body(""" { - "caption": "some patched test-unix-user", + "caption" : "some patched test-unix-user", "config": { "shell": "/bin/bash", "totpKey": "0x1234567890abcdef0123456789abcdef", @@ -588,7 +588,7 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup }).returnedValue()).isPresent().get() .matches(asset -> { assertThat(asset.getCaption()).isEqualTo("some patched test-unix-user"); - assertThat(asset.getConfig().toString()).isEqualTo(""" + assertThat(asset.getConfig().toString()).isEqualToIgnoringWhitespace(""" { "password": "$6$Jr5w/Y8zo8pCkqg7$/rePRbvey3R6Sz/02YTlTQcRt5qdBPTj2h5.hz.rB8NfIoND8pFOjeB7orYcPs9JNf3JDxPP2V.6MQlE5BwAY/", "shell": "/bin/bash", diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java index 99f0efd6..ae733e54 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java @@ -227,7 +227,7 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu exactlyTheseAssetsAreReturned( result, "HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)", - "HsHostingAssetEntity(MANAGED_SERVER, vm1011, some ManagedServer, D-1000111:D-1000111 default project:separate ManagedServer, { monit_max_cpu_usage: 90, monit_max_ram_usage: 80, monit_max_ssd_usage: 70 } )"); + "HsHostingAssetEntity(MANAGED_SERVER, vm1011, some ManagedServer, D-1000111:D-1000111 default project:separate ManagedServer, { monit_max_cpu_usage : 90, monit_max_ram_usage : 80, monit_max_ssd_usage : 70 })"); } @Test @@ -444,6 +444,7 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu .extracting(HsHostingAssetEntity::toString) .extracting(input -> input.replaceAll("\\s+", " ")) .extracting(input -> input.replaceAll("\"", "")) + .extracting(input -> input.replaceAll("\" : ", "\": ")) .containsExactlyInAnyOrder(serverNames); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsEMailAddressHostingAssetValidatorUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsEMailAddressHostingAssetValidatorUnitTest.java index b635a493..441200c6 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsEMailAddressHostingAssetValidatorUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsEMailAddressHostingAssetValidatorUnitTest.java @@ -4,13 +4,15 @@ import net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAssetEntity; import net.hostsharing.hsadminng.mapper.Array; import org.junit.jupiter.api.Test; +import java.util.HashMap; import java.util.Map; -import static java.util.Map.entry; +import static java.util.Map.ofEntries; import static net.hostsharing.hsadminng.hs.booking.item.TestHsBookingItem.TEST_MANAGED_SERVER_BOOKING_ITEM; import static net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAssetType.DOMAIN_MBOX_SETUP; import static net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAssetType.EMAIL_ADDRESS; import static net.hostsharing.hsadminng.hs.hosting.asset.TestHsHostingAssetEntities.TEST_MANAGED_SERVER_HOSTING_ASSET; +import static net.hostsharing.hsadminng.mapper.PatchableMapWrapper.entry; import static org.assertj.core.api.Assertions.assertThat; class HsEMailAddressHostingAssetValidatorUnitTest { @@ -28,11 +30,11 @@ class HsEMailAddressHostingAssetValidatorUnitTest { return HsHostingAssetEntity.builder() .type(EMAIL_ADDRESS) .parentAsset(domainMboxSetup) - .identifier("test@example.org") - .config(Map.ofEntries( - entry("local-part", "test"), + .identifier("old-local-part@example.org") + .config(new HashMap<>(ofEntries( + entry("local-part", "old-local-part"), entry("target", Array.of("xyz00", "xyz00-abc", "office@example.com")) - )); + ))); } @Test @@ -64,10 +66,10 @@ class HsEMailAddressHostingAssetValidatorUnitTest { void rejectsInvalidProperties() { // given final var emailAddressHostingAssetEntity = validEntityBuilder() - .config(Map.ofEntries( + .config(new HashMap<>(ofEntries( entry("local-part", "no@allowed"), entry("sub-domain", "no@allowedeither"), - entry("target", Array.of("xyz00", "xyz00-abc", "garbage", "office@example.com")))) + entry("target", Array.of("xyz00", "xyz00-abc", "garbage", "office@example.com"))))) .build(); final var validator = HostingAssetEntityValidatorRegistry.forType(emailAddressHostingAssetEntity.getType()); @@ -76,9 +78,69 @@ class HsEMailAddressHostingAssetValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "'EMAIL_ADDRESS:test@example.org.config.local-part' is expected to match any of [^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+$] but 'no@allowed' does not match", - "'EMAIL_ADDRESS:test@example.org.config.sub-domain' is expected to match any of [^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+$] but 'no@allowedeither' does not match", - "'EMAIL_ADDRESS:test@example.org.config.target' is expected to match any of [^[a-z][a-z0-9]{2}[0-9]{2}(-[a-z0-9][a-z0-9\\._-]*)?$, ^([a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+)?@[a-zA-Z0-9.-]+$, ^nobody$] but 'garbage' does not match any"); + "'EMAIL_ADDRESS:old-local-part@example.org.config.local-part' is expected to match any of [^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+$] but 'no@allowed' does not match", + "'EMAIL_ADDRESS:old-local-part@example.org.config.sub-domain' is expected to match any of [^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+$] but 'no@allowedeither' does not match", + "'EMAIL_ADDRESS:old-local-part@example.org.config.target' is expected to match any of [^[a-z][a-z0-9]{2}[0-9]{2}(-[a-z0-9][a-z0-9\\._-]*)?$, ^([a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+)?@[a-zA-Z0-9.-]+$, ^nobody$] but 'garbage' does not match any"); + } + + @Test + void rejectsOverwritingWriteOnceProperties() { + // given + final var emailAddressHostingAssetEntity = validEntityBuilder() + .isLoaded(true) + .build(); + final var validator = HostingAssetEntityValidatorRegistry.forType(emailAddressHostingAssetEntity.getType()); + + // when + emailAddressHostingAssetEntity.getConfig().put("local-part", "new-local-part"); + emailAddressHostingAssetEntity.getConfig().put("sub-domain", "new-sub-domain"); + final var result = validator.validateEntity(emailAddressHostingAssetEntity); + + // then + assertThat(result).containsExactlyInAnyOrder( + "'EMAIL_ADDRESS:old-local-part@example.org.config.local-part' is write-once but given as 'new-local-part'", + "'EMAIL_ADDRESS:old-local-part@example.org.config.sub-domain' is write-once but given as 'new-sub-domain'"); + } + + @Test + void rejectsRemovingWriteOnceProperties() { + // given + final var emailAddressHostingAssetEntity = validEntityBuilder() + .config(new HashMap<>(ofEntries( + entry("local-part", "old-local-part"), + entry("sub-domain", "old-sub-domain"), + entry("target", Array.of("xyz00", "xyz00-abc", "office@example.com")) + ))) + .isLoaded(true) + .build(); + final var validator = HostingAssetEntityValidatorRegistry.forType(emailAddressHostingAssetEntity.getType()); + + // when + emailAddressHostingAssetEntity.getConfig().remove("local-part"); + emailAddressHostingAssetEntity.getConfig().remove("sub-domain"); + final var result = validator.validateEntity(emailAddressHostingAssetEntity); + + // then + assertThat(result).containsExactlyInAnyOrder( + "'EMAIL_ADDRESS:old-local-part@example.org.config.local-part' is write-once but got removed", + "'EMAIL_ADDRESS:old-local-part@example.org.config.sub-domain' is write-once but got removed"); + } + + @Test + void acceptsOverwritingWriteOncePropertiesWithSameValues() { + // given + final var emailAddressHostingAssetEntity = validEntityBuilder() + .isLoaded(true) + .build(); + final var validator = HostingAssetEntityValidatorRegistry.forType(emailAddressHostingAssetEntity.getType()); + + // when + emailAddressHostingAssetEntity.getConfig().put("local-part", "old-local-part"); + emailAddressHostingAssetEntity.getConfig().remove("sub-domain"); // is not there anyway + final var result = validator.validateEntity(emailAddressHostingAssetEntity); + + // then + assertThat(result).isEmpty(); } @Test @@ -94,7 +156,7 @@ class HsEMailAddressHostingAssetValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "'identifier' expected to match '^\\Qtest@example.org\\E$', but is 'abc00-office'"); + "'identifier' expected to match '^\\Qold-local-part@example.org\\E$', but is 'abc00-office'"); } @Test @@ -112,8 +174,8 @@ class HsEMailAddressHostingAssetValidatorUnitTest { // then assertThat(result).containsExactlyInAnyOrder( - "'EMAIL_ADDRESS:test@example.org.bookingItem' must be null but is of type MANAGED_SERVER", - "'EMAIL_ADDRESS:test@example.org.parentAsset' must be of type DOMAIN_MBOX_SETUP but is of type MANAGED_SERVER", - "'EMAIL_ADDRESS:test@example.org.assignedToAsset' must be null but is of type MANAGED_SERVER"); + "'EMAIL_ADDRESS:old-local-part@example.org.bookingItem' must be null but is of type MANAGED_SERVER", + "'EMAIL_ADDRESS:old-local-part@example.org.parentAsset' must be of type DOMAIN_MBOX_SETUP but is of type MANAGED_SERVER", + "'EMAIL_ADDRESS:old-local-part@example.org.assignedToAsset' must be null but is of type MANAGED_SERVER"); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/HsHostingAssetRealEntity.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/HsHostingAssetRealEntity.java index 51665b9c..b83f97ee 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/HsHostingAssetRealEntity.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/HsHostingAssetRealEntity.java @@ -103,8 +103,8 @@ public class HsHostingAssetRealEntity implements HsHostingAsset { } @Override - public Map directProps() { - return config; + public PatchableMapWrapper directProps() { + return getConfig(); } @Override diff --git a/src/test/java/net/hostsharing/hsadminng/hs/validation/PasswordPropertyUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/validation/PasswordPropertyUnitTest.java index aea913e5..64ca8236 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/validation/PasswordPropertyUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/validation/PasswordPropertyUnitTest.java @@ -1,6 +1,7 @@ package net.hostsharing.hsadminng.hs.validation; import net.hostsharing.hsadminng.hash.LinuxEtcShadowHashGenerator; +import net.hostsharing.hsadminng.mapper.PatchableMapWrapper; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -109,10 +110,10 @@ class PasswordPropertyUnitTest { } @Override - public Map directProps() { - return Map.ofEntries( + public PatchableMapWrapper directProps() { + return PatchableMapWrapper.of(Map.ofEntries( entry(passwordProp.propertyName, "some password") - ); + )); } @Override diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/test/PatchUnitTestBase.java b/src/test/java/net/hostsharing/hsadminng/rbac/test/PatchUnitTestBase.java index d446258b..97fa53ec 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/test/PatchUnitTestBase.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/test/PatchUnitTestBase.java @@ -34,7 +34,7 @@ public abstract class PatchUnitTestBase { @Test @SuppressWarnings("unchecked") - void willPatchAllProperties() { + protected void willPatchAllProperties() { // given final var givenEntity = newInitialEntity(); final var patchResource = newPatchResource(); @@ -55,7 +55,7 @@ public abstract class PatchUnitTestBase { @ParameterizedTest @MethodSource("propertyTestCases") - void willPatchOnlyGivenProperty(final Property testCase) { + protected void willPatchOnlyGivenProperty(final Property testCase) { // given final var givenEntity = newInitialEntity();