fix issues from code review

src/main/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsDomainSetupBookingItemValidator.java

@@ -20,6 +20,7 @@ class HsDomainSetupBookingItemValidator extends HsBookingItemEntityValidator {
     HsDomainSetupBookingItemValidator() {
         super(
                 stringProperty(DOMAIN_NAME_PROPERTY_NAME).writeOnce()
+                        .maxLength(253)
                         .matchesRegEx(FQDN_REGEX).describedAs("is not a (non-top-level) fully qualified domain name")
                         .notMatchesRegEx(REGISTRAR_LEVEL_DOMAINS).describedAs("is a forbidden registrar-level domain name")
                         .required(),
@@ -34,7 +35,7 @@ class HsDomainSetupBookingItemValidator extends HsBookingItemEntityValidator {
         final var violations = new ArrayList<String>();
         final var domainName = bookingItem.getDirectValue(DOMAIN_NAME_PROPERTY_NAME, String.class);
         if (!bookingItem.isLoaded() &&
-                domainName.matches("hostsharing.(com|net|org|coop)")) {
+                domainName.matches("hostsharing.(com|net|org|coop|de)")) {
             violations.add("'" + bookingItem.toShortString() + ".resources." + DOMAIN_NAME_PROPERTY_NAME + "' = '" + domainName
                     + "' is a forbidden Hostsharing domain name");
         }

src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/Dns.java

@@ -49,16 +49,25 @@ public class Dns {
         return Optional.empty();
     }
 
-    public static boolean isRegistrarLevel(final String domainName) {
+    public static boolean isRegistrarLevelDomain(final String domainName) {
         return stream(REGISTRAR_LEVEL_DOMAIN_PATTERN)
                 .anyMatch(p -> p.matcher(domainName).matches());
     }
 
+    /**
+     * @param domainName a fully qualified domain name
+     * @return true if `domainName` can be registered at a registrar, false if it's a subdomain of such or a registrar-level domain itself
+     */
+    public static boolean isRegistrableDomain(final String domainName) {
+        return !isRegistrarLevelDomain(domainName) &&
+                superDomain(domainName).map(Dns::isRegistrarLevelDomain).orElse(false);
+    }
+
     public static void fakeResultForDomain(final String domainName, final Result fakeResult) {
         fakeResults.put(domainName, fakeResult);
     }
 
-    static void resetFakeResults() {
+    public static void resetFakeResults() {
         fakeResults.clear();
     }
 

src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainSetupHostingAssetValidator.java

@@ -2,7 +2,6 @@ package net.hostsharing.hsadminng.hs.hosting.asset.validators;
 
 import net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAsset;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -27,8 +26,12 @@ class HsDomainSetupHostingAssetValidator extends HostingAssetEntityValidator {
 
     @Override
     public List<String> validateEntity(final HsHostingAsset assetEntity) {
+        final var violations = // new ArrayList<String>();
+            super.validateEntity(assetEntity);
+        if (!violations.isEmpty()) {
+            return violations;
+        }
 
-        final var violations = new ArrayList<String>();
         final var domainName = assetEntity.getIdentifier();
         final var dnsResult = new Dns(domainName).fetchRecordsOfType("TXT");
         final Supplier<String> getCode = () -> assetEntity.getBookingItem().getDirectValue("verificationCode", String.class);
@@ -50,10 +53,8 @@ class HsDomainSetupHostingAssetValidator extends HostingAssetEntityValidator {
             }
 
             case Dns.Status.NAME_NOT_FOUND: {
+                if (isDnsVerificationRequiredForUnregisteredDomain(assetEntity)) {
                     final var superDomain = superDomain(domainName);
-                final var verificationRequired = !superDomain.map(Dns::isRegistrarLevel).orElse(false)
-                        && assetEntity.getBookingItem() != null; // FIXME: or getParentAsset() == nuĺl? or extract method
-                if (verificationRequired) {
                     final var expectedTxtRecordValue = "Hostsharing-domain-setup-verification-code=" + getCode.get();
                     final var verificationFoundInSuperDomain = superDomain.flatMap(superDomainName -> findTxtRecord(
                             new Dns(superDomainName).fetchRecordsOfType("TXT"),
@@ -77,17 +78,9 @@ class HsDomainSetupHostingAssetValidator extends HostingAssetEntityValidator {
                 violations.add("[DNS] lookup failed for domain name '" + assetEntity.getIdentifier() + "': " + dnsResult.exception());
                 break;
         }
-
-        violations.addAll(super.validateEntity(assetEntity));
         return violations;
     }
 
-    private static Optional<String> findTxtRecord(final Dns.Result result, final String expectedTxtRecordValue) {
-        return result.records().stream()
-                .filter(r -> r.contains(expectedTxtRecordValue))
-                .findAny();
-    }
-
     @Override
     protected Pattern identifierPattern(final HsHostingAsset assetEntity) {
         if (assetEntity.getBookingItem() != null) {
@@ -98,4 +91,16 @@ class HsDomainSetupHostingAssetValidator extends HostingAssetEntityValidator {
         final var parentDomainName = assetEntity.getParentAsset().getIdentifier();
         return Pattern.compile(SUBDOMAIN_NAME_REGEX + "\\." + parentDomainName.replace(".", "\\."), Pattern.CASE_INSENSITIVE);
     }
+
+    private static boolean isDnsVerificationRequiredForUnregisteredDomain(final HsHostingAsset assetEntity) {
+        return !Dns.isRegistrableDomain(assetEntity.getIdentifier())
+                && assetEntity.getParentAsset() == null;
+    }
+
+
+    private static Optional<String> findTxtRecord(final Dns.Result result, final String expectedTxtRecordValue) {
+        return result.records().stream()
+                .filter(r -> r.contains(expectedTxtRecordValue))
+                .findAny();
+    }
 }

src/test/java/net/hostsharing/hsadminng/hs/booking/item/validators/HsDomainSetupBookingItemValidatorUnitTest.java

@@ -12,10 +12,12 @@ import java.util.Map;
 
 import static java.util.Map.entry;
 import static net.hostsharing.hsadminng.hs.booking.item.HsBookingItemType.DOMAIN_SETUP;
+import static org.apache.commons.lang3.StringUtils.right;
 import static org.assertj.core.api.Assertions.assertThat;
 
 class HsDomainSetupBookingItemValidatorUnitTest {
 
+    public static final String TOO_LONG_DOMAIN_NAME = "asdfghijklmnopqrstuvwxyz0123456789.".repeat(8) + "example.org";
     final HsBookingDebitorEntity debitor = HsBookingDebitorEntity.builder()
             .debitorNumber(12345)
             .build();
@@ -44,6 +46,42 @@ class HsDomainSetupBookingItemValidatorUnitTest {
         assertThat(result).isEmpty();
     }
 
+    @Test
+    void acceptsMaximumDomainNameLength() {
+        final var domainSetupBookingItemEntity = HsBookingItemRealEntity.builder()
+                .type(DOMAIN_SETUP)
+                .project(project)
+                .caption("Test-Domain")
+                .resources(Map.ofEntries(
+                        entry("domainName", right(TOO_LONG_DOMAIN_NAME, 253))
+                ))
+                .build();
+
+        // when
+        final var result = HsBookingItemEntityValidatorRegistry.doValidate(em, domainSetupBookingItemEntity);
+
+        // then
+        assertThat(result).isEmpty();
+    }
+
+    @Test
+    void rejectsTooLongTotalName() {
+        final var domainSetupBookingItemEntity = HsBookingItemRealEntity.builder()
+                .type(DOMAIN_SETUP)
+                .project(project)
+                .caption("Test-Domain")
+                .resources(Map.ofEntries(
+                        entry("domainName", right(TOO_LONG_DOMAIN_NAME, 254))
+                ))
+                .build();
+
+        // when
+        final var result = HsBookingItemEntityValidatorRegistry.doValidate(em, domainSetupBookingItemEntity);
+
+        // then
+        assertThat(result).contains("'D-12345:Test-Project:Test-Domain.resources.domainName' length is expected to be at max 253 but length of 'dfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.asdfghijklmnopqrstuvwxyz0123456789.example.org' is 254");
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {
             "de", "com", "net", "org", "actually-any-top-level-domain",
@@ -81,7 +119,7 @@ class HsDomainSetupBookingItemValidatorUnitTest {
 
     @ParameterizedTest
     @ValueSource(strings = {
-            "hostsharing.net", "hostsharing.org", "hostsharing.com", "hostsharing.coop"
+            "hostsharing.net", "hostsharing.org", "hostsharing.com", "hostsharing.coop", "hostsharing.de"
     })
     void rejectHostsharingDomain(final String secondLevelRegistrarDomain) {
         // given
@@ -111,7 +149,7 @@ class HsDomainSetupBookingItemValidatorUnitTest {
 
         // then
         assertThat(validator.properties()).map(Map::toString).containsExactlyInAnyOrder(
-                "{type=string, propertyName=domainName, matchesRegEx=[^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{2,12}], matchesRegExDescription=is not a (non-top-level) fully qualified domain name, notMatchesRegEx=[[^.]+, (co|org|gov|ac|sch)\\.uk, (com|net|org|edu|gov|asn|id)\\.au, (co|ne|or|ac|go)\\.jp, (com|net|org|gov|edu|ac)\\.cn, (com|net|org|gov|edu|mil|art)\\.br, (co|net|org|gen|firm|ind)\\.in, (com|net|org|gob|edu)\\.mx, (gov|edu)\\.it, (co|net|org|govt|ac|school|geek|kiwi)\\.nz, (co|ne|or|go|re|pe)\\.kr], notMatchesRegExDescription=is a forbidden registrar-level domain name, required=true, writeOnce=true}",
+                "{type=string, propertyName=domainName, matchesRegEx=[^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{2,12}], matchesRegExDescription=is not a (non-top-level) fully qualified domain name, notMatchesRegEx=[[^.]+, (co|org|gov|ac|sch)\\.uk, (com|net|org|edu|gov|asn|id)\\.au, (co|ne|or|ac|go)\\.jp, (com|net|org|gov|edu|ac)\\.cn, (com|net|org|gov|edu|mil|art)\\.br, (co|net|org|gen|firm|ind)\\.in, (com|net|org|gob|edu)\\.mx, (gov|edu)\\.it, (co|net|org|govt|ac|school|geek|kiwi)\\.nz, (co|ne|or|go|re|pe)\\.kr], notMatchesRegExDescription=is a forbidden registrar-level domain name, maxLength=253, required=true, writeOnce=true}",
                 "{type=string, propertyName=verificationCode, readOnly=true, computed=IN_INIT}");
     }
 }

src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetControllerAcceptanceTest.java

@@ -14,6 +14,7 @@ import net.hostsharing.hsadminng.hs.office.contact.HsOfficeContactRealEntity;
 import net.hostsharing.hsadminng.hs.office.contact.HsOfficeContactRealRepository;
 import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup;
 import net.hostsharing.hsadminng.rbac.test.JpaAttempt;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.ClassOrderer;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Order;
@@ -65,6 +66,11 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
     @Autowired
     JpaAttempt jpaAttempt;
 
+    @AfterEach
+    void cleanup() {
+        Dns.resetFakeResults();
+    }
+
     @Nested
     @Order(2)
     class ListAssets {

src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/DnsUnitTest.java

@@ -7,11 +7,23 @@ import static org.assertj.core.api.Assertions.assertThat;
 class DnsUnitTest {
 
     @Test
-    void isRegistrarLevel() {
+    void isRegistrarLevelDomain() {
-        assertThat(Dns.isRegistrarLevel("de")).isTrue();
+        assertThat(Dns.isRegistrarLevelDomain("de")).isTrue();
-        assertThat(Dns.isRegistrarLevel("example.de")).isFalse();
+        assertThat(Dns.isRegistrarLevelDomain("example.de")).isFalse();
 
-        assertThat(Dns.isRegistrarLevel("co.uk")).isTrue();
+        assertThat(Dns.isRegistrarLevelDomain("co.uk")).isTrue();
-        assertThat(Dns.isRegistrarLevel("example.co.uk")).isFalse();
+        assertThat(Dns.isRegistrarLevelDomain("example.co.uk")).isFalse();
+        assertThat(Dns.isRegistrarLevelDomain("co.uk.com")).isFalse();
+    }
+
+    @Test
+    void isRegistrableDomain() {
+        assertThat(Dns.isRegistrableDomain("de")).isFalse();
+        assertThat(Dns.isRegistrableDomain("example.de")).isTrue();
+        assertThat(Dns.isRegistrableDomain("sub.example.de")).isFalse();
+
+        assertThat(Dns.isRegistrableDomain("co.uk")).isFalse();
+        assertThat(Dns.isRegistrableDomain("example.co.uk")).isTrue();
+        assertThat(Dns.isRegistrableDomain("sub.example.co.uk")).isFalse();
     }
 }

src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainSetupHostingAssetValidatorUnitTest.java

@@ -59,30 +59,30 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
 
     //=====================================================================================================================
 
-    enum InvalidDomainNameIdentifier {
+    enum InvalidSubDomainNameIdentifierForExampleOrg {
-        EMPTY(""),
+        IDENTICAL("example.org"),
-        TOO_LONG("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz0123456890123456789.de"),
+        TOO_LONG("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz0123456890123456789.example.org"),
-        DASH_AT_BEGINNING("-example.com"),
+        DASH_AT_BEGINNING("-sub.example.org"),
-        DOT_AT_BEGINNING(".example.com"),
+        DOT(".example.org"),
-        DOT_AT_END("example.com.");
+        DOT_AT_BEGINNING(".sub.example.org"),
+        DOUBLE_DOT("sub..example.com.");
 
         final String domainName;
 
-        InvalidDomainNameIdentifier(final String domainName) {
+        InvalidSubDomainNameIdentifierForExampleOrg(final String domainName) {
             this.domainName = domainName;
         }
     }
 
     @ParameterizedTest
-    @EnumSource(InvalidDomainNameIdentifier.class)
+    @EnumSource(InvalidSubDomainNameIdentifierForExampleOrg.class)
-    void rejectsInvalidIdentifier(final InvalidDomainNameIdentifier testCase) {
+    void rejectsInvalidIdentifier(final InvalidSubDomainNameIdentifierForExampleOrg testCase) {
         // given
-        final var givenEntity = validEntityBuilder(testCase.domainName,
+        final var givenEntity = validEntityBuilder(testCase.domainName)
-                    bib -> bib.resources(new HashMap<>(ofEntries(
+                .bookingItem(null)
-                            entry("domainName", "example.org")
+                .parentAsset(HsHostingAssetRealEntity.builder().type(DOMAIN_SETUP).identifier("example.org").build())
-                    ))).build()
+                .build();
-                ).build();
+        // fakeValidDnsVerification(givenEntity);
-        fakeValidDnsVerification(givenEntity);
         final var validator = HostingAssetEntityValidatorRegistry.forType(givenEntity.getType());
 
         // when
@@ -90,26 +90,26 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
 
         // then
         assertThat(result).contains(
-                "'identifier' expected to match 'example.org', but is '" + testCase.domainName + "'"
+                "'identifier' expected to match '(\\*|(?!-)[A-Za-z0-9-]{1,63}(?<!-))\\.example\\.org', but is '" + testCase.domainName + "'"
         );
     }
 
-    enum ValidDomainNameIdentifier {
+    enum ValidSubDomainNameIdentifier {
-        SIMPLE("example.org"),
+        SIMPLE("sub.example.org"),
-        MAX_LENGTH("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz01234568901.de"),
+        MAX_LENGTH("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz01234568901.example.org"),
-        WITH_DASH("example-test.com"),
+        MIN_LENGTH("x.example.org"),
-        SUBDOMAIN("test.example.com");
+        WITH_DASH("example-test.example.org");
 
         final String domainName;
 
-        ValidDomainNameIdentifier(final String domainName) {
+        ValidSubDomainNameIdentifier(final String domainName) {
             this.domainName = domainName;
         }
     }
 
     @ParameterizedTest
-    @EnumSource(ValidDomainNameIdentifier.class)
+    @EnumSource(ValidSubDomainNameIdentifier.class)
-    void acceptsValidIdentifier(final ValidDomainNameIdentifier testCase) {
+    void acceptsValidIdentifier(final ValidSubDomainNameIdentifier testCase) {
         // given
         final var givenEntity = validEntityBuilder(testCase.domainName).identifier(testCase.domainName).build();
         fakeValidDnsVerification(givenEntity);
@@ -317,11 +317,17 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
     }
 
     @Test
-    void rejectSetupOfUnregisteredSubdomainWithoutDnsVerificationInSuperDomain() {
+    void rejectSetupOfUnregisteredSubdomainWithoutParentAssetAndWithoutDnsVerificationInSuperDomain() {
         domainSetupFor("sub.example.org").notRegistered()
                 .isRejectedWithCauseMissingVerificationIn("example.org");
     }
 
+    @Test
+    void acceptSetupOfUnregisteredSubdomainWithParentAssetEvenWithoutDnsVerificationInSuperDomain() {
+        domainSetupWithParentAssetFor("sub.example.org").notRegistered()
+                .isAccepted();
+    }
+
     @Test
     void allowSetupOfExistingSubdomainWithValidDnsVerificationInSuperDomain() {
         domainSetupFor("sub.example.org").registered()
@@ -361,6 +367,14 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
             expectedHash = domainAsset.getBookingItem().getDirectValue("verificationCode", String.class);
         }
 
+        public DomainSetupBuilder(final HsHostingAssetRealEntity parentAsset, final String domainName) {
+            domainAsset = validEntityBuilder(domainName)
+                    .bookingItem(null)
+                    .parentAsset(parentAsset)
+                    .build();
+            expectedHash = null;
+        }
+
         DomainSetupBuilder notRegistered() {
             Dns.fakeResultForDomain(domainAsset.getIdentifier(), DOMAIN_NOT_REGISTERED);
             return this;
@@ -386,6 +400,7 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
         }
 
         DomainSetupBuilder withVerificationIn(final String domainName) {
+            assertThat(expectedHash).as("no expectedHash available").isNotNull();
             Dns.fakeResultForDomain(
                     domainName,
                     Dns.Result.fromRecords("Hostsharing-domain-setup-verification-code=" + expectedHash));
@@ -393,6 +408,7 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
         }
 
         void isRejectedWithCauseMissingVerificationIn(final String domainName) {
+            assertThat(expectedHash).as("no expectedHash available").isNotNull();
             assertThat(validate()).containsAnyOf(
                     "[DNS] no TXT record 'Hostsharing-domain-setup-verification-code=" + expectedHash
                             + "' found for domain name '" + domainName + "' (nor in its super-domain)",
@@ -413,4 +429,10 @@ class HsDomainSetupHostingAssetValidatorUnitTest {
     private DomainSetupBuilder domainSetupFor(final String domainName) {
         return new DomainSetupBuilder(domainName);
     }
+
+    private DomainSetupBuilder domainSetupWithParentAssetFor(final String domainName) {
+        return new DomainSetupBuilder(
+                HsHostingAssetRealEntity.builder().type(DOMAIN_SETUP).identifier(Dns.superDomain(domainName).orElseThrow()).build(),
+                domainName);
+    }
 }