From b49b10ec15f6fb22585640753a3f2c52454bfb25 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 8 Aug 2024 09:14:54 +0200 Subject: [PATCH] handle expectedErrors in test-data --- ...HsDomainDnsSetupHostingAssetValidator.java | 17 ++++++-- .../hsadminng/system/SystemProcess.java | 5 +++ .../hsadminng/hs/migration/CsvDataImport.java | 15 +++++-- .../hs/migration/ImportHostingAssets.java | 41 ++++++++++++------- .../hs/migration/ImportOfficeData.java | 4 +- 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidator.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidator.java index 4fd1b2ac..4f9f393b 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidator.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidator.java @@ -15,7 +15,8 @@ import static net.hostsharing.hsadminng.hs.validation.BooleanProperty.booleanPro import static net.hostsharing.hsadminng.hs.validation.IntegerProperty.integerProperty; import static net.hostsharing.hsadminng.hs.validation.StringProperty.stringProperty; -class HsDomainDnsSetupHostingAssetValidator extends HostingAssetEntityValidator { +// TODO.impl: make package private once we've migrated the legacy data +public class HsDomainDnsSetupHostingAssetValidator extends HostingAssetEntityValidator { // according to RFC 1035 (section 5) and RFC 1034 static final String RR_REGEX_NAME = "(\\*\\.)?([a-zA-Z0-9\\._-]+|@)[ \t]+"; @@ -32,6 +33,8 @@ class HsDomainDnsSetupHostingAssetValidator extends HostingAssetEntityValidator RR_REGEX_NAME + RR_REGEX_IN + RR_REGEX_TTL + RR_RECORD_TYPE + RR_RECORD_DATA + RR_COMMENT; public static final String IDENTIFIER_SUFFIX = "|DNS"; + private static List zoneFileErrors = null; // TODO.impl: remove once legacy data is migrated + HsDomainDnsSetupHostingAssetValidator() { super( DOMAIN_DNS_SETUP, @@ -78,12 +81,16 @@ class HsDomainDnsSetupHostingAssetValidator extends HostingAssetEntityValidator // TODO.spec: define which checks should get raised to error level final var namedCheckZone = new SystemProcess("named-checkzone", fqdn(assetEntity)); final var zonefileString = toZonefileString(assetEntity); + final var zoneFileErrorResult = zoneFileErrors != null ? zoneFileErrors : result; if (namedCheckZone.execute(zonefileString) != 0) { - // yes, named-checkzone writes error messages to stdout + // yes, named-checkzone writes error messages to stdout, not stderr stream(namedCheckZone.getStdOut().split("\n")) .map(line -> line.replaceAll(" stream-0x[0-9a-f]+:", "line ")) .map(line -> "[" + assetEntity.getIdentifier() + "] " + line) - .forEach(result::add); + .forEach(zoneFileErrorResult::add); + if (!namedCheckZone.getStdErr().isEmpty()) { + result.add("unexpected stderr output for " + namedCheckZone.getCommand() + ": " + namedCheckZone.getStdErr()); + } } return result; } @@ -170,4 +177,8 @@ class HsDomainDnsSetupHostingAssetValidator extends HostingAssetEntityValidator private String fqdn(final HsHostingAsset assetEntity) { return assetEntity.getIdentifier().substring(0, assetEntity.getIdentifier().length() - IDENTIFIER_SUFFIX.length()); } + + public static void addZonefileErrorsTo(final List zoneFileErrors) { + HsDomainDnsSetupHostingAssetValidator.zoneFileErrors = zoneFileErrors; + } } diff --git a/src/main/java/net/hostsharing/hsadminng/system/SystemProcess.java b/src/main/java/net/hostsharing/hsadminng/system/SystemProcess.java index 149c6019..8b302098 100644 --- a/src/main/java/net/hostsharing/hsadminng/system/SystemProcess.java +++ b/src/main/java/net/hostsharing/hsadminng/system/SystemProcess.java @@ -21,6 +21,11 @@ public class SystemProcess { this.processBuilder = new ProcessBuilder(command); } + + public String getCommand() { + return processBuilder.command().toString(); + } + public int execute() throws IOException, InterruptedException { final var process = processBuilder.start(); stdOut = fetchOutput(process.getInputStream()); // yeah, twisted ProcessBuilder API diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/CsvDataImport.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/CsvDataImport.java index 03593bc8..9112c000 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/CsvDataImport.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/CsvDataImport.java @@ -33,7 +33,7 @@ import java.lang.annotation.RetentionPolicy; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.time.LocalDate; -import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -77,7 +77,7 @@ public class CsvDataImport extends ContextBasedTest { @MockBean HttpServletRequest request; - static final List errors = new ArrayList<>(); + static final LinkedHashSet errors = new LinkedHashSet<>(); public List readAllLines(Reader reader) throws Exception { @@ -318,8 +318,15 @@ public class CsvDataImport extends ContextBasedTest { errors.add(error); } - protected final void logErrors() { - final var errorsToLog = new ArrayList<>(errors); + protected static void expectError(final String expectedError) { + final var found = errors.remove(expectedError); + if (!found) { + logError("expected but not found: " + expectedError); + } + } + + protected final void assertNoErrors() { + final var errorsToLog = new LinkedHashSet<>(errors); errors.clear(); assertThat(errorsToLog).isEmpty(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportHostingAssets.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportHostingAssets.java index e0ba0610..d7e8f77c 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportHostingAssets.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportHostingAssets.java @@ -13,6 +13,7 @@ import net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectEntity; import net.hostsharing.hsadminng.hs.hosting.asset.HsHostingAssetType; import net.hostsharing.hsadminng.hs.hosting.asset.validators.HostingAssetEntitySaveProcessor; import net.hostsharing.hsadminng.hs.hosting.asset.validators.HostingAssetEntityValidatorRegistry; +import net.hostsharing.hsadminng.hs.hosting.asset.validators.HsDomainDnsSetupHostingAssetValidator; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.MethodOrderer; @@ -130,6 +131,7 @@ public class ImportHostingAssets extends ImportOfficeData { static final Integer DOMAIN_HTTP_SETUP_OFFSET = 12000000; static final Integer DOMAIN_MBOX_SETUP_OFFSET = 13000000; static final Integer DOMAIN_SMTP_SETUP_OFFSET = 14000000; + static List zonefileErrors = new ArrayList<>(); record Hive(int hive_id, String hive_name, int inet_addr_id, AtomicReference serverRef) {} @@ -578,7 +580,11 @@ public class ImportHostingAssets extends ImportOfficeData { @Order(18999) @ContinueOnFailure void logValidationErrors() { - this.logErrors(); + if (isImportingControlledTestData()) { + expectError("zonedata dom_owner of mellis.de is old00 but expected to be mim00"); + expectError("\nexpected: \"vm1068\"\n but was: \"vm1093\""); + } + this.assertNoErrors(); } // -------------------------------------------------------------------------------------------- @@ -769,7 +775,8 @@ public class ImportHostingAssets extends ImportOfficeData { @Test @Order(19999) void logErrorsAfterPersistingHostingAssets() { - logErrors(); + errors.addAll(zonefileErrors); + assertNoErrors(); } private void persistRecursively(final Integer key, final HsBookingItemEntity bi) { @@ -784,21 +791,25 @@ public class ImportHostingAssets extends ImportOfficeData { private void persistHostingAssetsOfType(final HsHostingAssetType... hsHostingAssetTypes) { final var hsHostingAssetTypeSet = stream(hsHostingAssetTypes).collect(toSet()); - hostingAssets.forEach((key, ha) -> { - logError(() -> - jpaAttempt.transacted(() -> { - context(rbacSuperuser); + if (hsHostingAssetTypeSet.contains(DOMAIN_DNS_SETUP)) { + HsDomainDnsSetupHostingAssetValidator.addZonefileErrorsTo(zonefileErrors); + } + + jpaAttempt.transacted(() -> + hostingAssets.forEach((key, ha) -> { + context(rbacSuperuser); // if put only outside the loop, it seems to get lost after a while, no idea why if (hsHostingAssetTypeSet.contains(ha.getType())) { - new HostingAssetEntitySaveProcessor(em, ha) - .preprocessEntity() - .validateEntityIgnoring("'EMAIL_ALIAS:.*\\.config\\.target' .*") - .prepareForSave() - .saveUsing(entity -> persist(key, entity)) - .validateContext(); + logError(() -> + new HostingAssetEntitySaveProcessor(em, ha) + .preprocessEntity() + .validateEntityIgnoring("'EMAIL_ALIAS:.*\\.config\\.target' .*") + .prepareForSave() + .saveUsing(entity -> persist(key, entity)) + .validateContext() + ); } - }).assertSuccessful() - ); - }); + }) + ).assertSuccessful(); } private void importIpNumbers(final String[] header, final List records) { diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java index b94320d6..5bdacaab 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java @@ -611,7 +611,7 @@ public class ImportOfficeData extends CsvDataImport { @Order(9000) @ContinueOnFailure void logCollectedErrorsBeforePersist() { - logErrors(); + assertNoErrors(); } @Test @@ -732,7 +732,7 @@ public class ImportOfficeData extends CsvDataImport { @Order(9999) @ContinueOnFailure void logCollectedErrors() { - this.logErrors(); + this.assertNoErrors(); } private void importBusinessPartners(final String[] header, final List records) {