From cb9a76eaafd37f921006822bcfe043655288bc63 Mon Sep 17 00:00:00 2001 From: Christof Rabensteiner Date: Mon, 22 Jul 2019 13:02:19 +0200 Subject: Test Flow of DeliveryRequest from "End-To-End" & Fix Bugs Schema Changes: - Remove mzs:DeliveryRequest/TnvzMetaData because all metadata fields can be collected from DeliveryRequest and redundancy is not needed. Fixes and Refactoring in preprocess: - MzsDeliveryRequestValidator: Instead of returning false, throw an exception when a condition is not met, and explain which condition is not met / why it is not met in the exception's message. - Integrate interface change in ConfigProfileGenerator and DeliveryRequestAugmenter. - Rewrite and simplify DeliveryRequestAugmenter's augmentation. - ConfigUtil Fixes: Ensure that we do not override the wrong parameters while merging. This error appeared in tnvz / msg client, connection / receive timeout, key / trust store, and lax hostname verification / trust all. Fix Bugs in Interceptor / SoapUtils: - Problem: DOM access and information extraction was implemented somewhat sloppy. - SolutioN: Change DOM access interface to access DOM more efficiently. Add boundary checks and handle edge cases while extracting information from SOAP Messages. - Test those changes properly. Testing: - Implement Delivery Request Flow in ITEndToEndTest. - Start application on random port instead of fixed port when running integration tests. - Add assertions to tests in ITMzsServiceTest suite. Others Bug Fixes: - ServicesConfig: Ensure that mzs service and msg service run on different endpoint addresses (/msg and /mzs). - DeliveryRequestBackend: Throw exception when binary message is missing. Don't wrap the exception. - SaveResponseToFileSink: Wrap Response in JAXB Element (otherwise, marshaller does not recognize it) --- .../moazs/preprocess/ConfigProfileGenerator.java | 8 +- .../at/gv/egiz/moazs/preprocess/ConfigUtil.java | 9 +- .../moazs/preprocess/DeliveryRequestAugmenter.java | 70 ++++----- .../preprocess/MzsDeliveryRequestValidator.java | 160 +++++++++++++++------ 4 files changed, 165 insertions(+), 82 deletions(-) (limited to 'src/main/java/at/gv/egiz/moazs/preprocess') diff --git a/src/main/java/at/gv/egiz/moazs/preprocess/ConfigProfileGenerator.java b/src/main/java/at/gv/egiz/moazs/preprocess/ConfigProfileGenerator.java index 5e81f0d..0637f98 100644 --- a/src/main/java/at/gv/egiz/moazs/preprocess/ConfigProfileGenerator.java +++ b/src/main/java/at/gv/egiz/moazs/preprocess/ConfigProfileGenerator.java @@ -11,6 +11,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import static at.gv.egiz.moazs.MoaZSException.moaZSException; +import static java.lang.String.format; import static java.util.stream.Collectors.*; public class ConfigProfileGenerator { @@ -70,9 +72,11 @@ public class ConfigProfileGenerator { var defaultProfile = profiles.get(defaultConfigKey); - if (!validator.isConfigProfileComplete(defaultProfile)) { + try { + validator.isConfigProfileComplete(defaultProfile); + } catch (MoaZSException ex) { if (verifyCompletenessOfDefaultConfiguration) - throw MoaZSException.moaZSException(PROFILE_NOT_COMPLETE_ERROR_MESSAGE); + throw moaZSException(format("%s Reason: %s", PROFILE_NOT_COMPLETE_ERROR_MESSAGE, ex.getMessage())); else { LOGGER.warn(PROFILE_NOT_COMPLETE_WARNING_MESSAGE); } diff --git a/src/main/java/at/gv/egiz/moazs/preprocess/ConfigUtil.java b/src/main/java/at/gv/egiz/moazs/preprocess/ConfigUtil.java index 056f6dc..f49132f 100644 --- a/src/main/java/at/gv/egiz/moazs/preprocess/ConfigUtil.java +++ b/src/main/java/at/gv/egiz/moazs/preprocess/ConfigUtil.java @@ -205,7 +205,7 @@ public class ConfigUtil { } if (primary.getTNVZClient() != null) { - builder.withMSGClient(merge(primary.getTNVZClient(), fallback.getTNVZClient())); + builder.withTNVZClient(merge(primary.getTNVZClient(), fallback.getTNVZClient())); } if (primary.getMsgResponseSinks() != null) { @@ -240,7 +240,7 @@ public class ConfigUtil { } if (primary.getReceiveTimeout() != null) { - builder.withConnectionTimeout(primary.getReceiveTimeout()); + builder.withReceiveTimeout(primary.getReceiveTimeout()); } return builder.build(); @@ -259,7 +259,7 @@ public class ConfigUtil { } if (primary.getTrustStore() != null) { - builder.withKeyStore(merge(primary.getTrustStore(), fallback.getTrustStore())); + builder.withTrustStore(merge(primary.getTrustStore(), fallback.getTrustStore())); } if (primary.isLaxHostNameVerification() != null) { @@ -267,8 +267,9 @@ public class ConfigUtil { } if (primary.isTrustAll() != null) { - builder.withLaxHostNameVerification(primary.isTrustAll()); + builder.withTrustAll(primary.isTrustAll()); } + return builder.build(); } diff --git a/src/main/java/at/gv/egiz/moazs/preprocess/DeliveryRequestAugmenter.java b/src/main/java/at/gv/egiz/moazs/preprocess/DeliveryRequestAugmenter.java index e7ee357..240a677 100644 --- a/src/main/java/at/gv/egiz/moazs/preprocess/DeliveryRequestAugmenter.java +++ b/src/main/java/at/gv/egiz/moazs/preprocess/DeliveryRequestAugmenter.java @@ -1,36 +1,41 @@ package at.gv.egiz.moazs.preprocess; +import at.gv.egiz.moazs.scheme.Marshaller; import at.gv.zustellung.app2mzs.xsd.ConfigType; import at.gv.zustellung.app2mzs.xsd.DeliveryRequestType; +import at.gv.zustellung.app2mzs.xsd.ObjectFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; import java.util.Map; import static at.gv.egiz.moazs.MoaZSException.moaZSException; +import static at.gv.egiz.moazs.util.NullCoalesce.coalesce; import static at.gv.zustellung.app2mzs.xsd.DeliveryRequestType.deliveryRequestTypeBuilder; -import static java.lang.String.format; @Component public class DeliveryRequestAugmenter { + private static final Logger log = LoggerFactory.getLogger(DeliveryRequestAugmenter.class); + private static final ObjectFactory FACTORY = new ObjectFactory(); + private final ConfigUtil util; private final Map configs; private final MzsDeliveryRequestValidator validator; + private final Marshaller mzsMarshaller; - private static final String INCOMPLETE_TNVZ_ERROR_MESSAGE = "mzs:DeliveryRequest is incomplete because mandatory " + - "fields for sending a tnvz:QueryPersonRequest are missing."; - private static final String INCOMPLETE_CONFIG_ERROR_MESSAGE = "Could not find a profile for " + - "the delivery request configuration, and the configuration attached to mzs:DeliveryRequest is incomplete."; - private static final String INCOMPLETE_MERGED_CONFIG_ERROR_MESSAGE = "I merged parameters from " + - "mzs:DeliveryRequest/Config with parameters from config profile with ProfileId='%s', but the " + - "configuration is incomplete."; + private static final String CONFIG_MISSING_ERROR_MSG = "Delivery request configuration is missing."; @Autowired - public DeliveryRequestAugmenter(Map deliveryRequestConfigs, ConfigUtil util, MzsDeliveryRequestValidator validator) { + public DeliveryRequestAugmenter(Map deliveryRequestConfigs, ConfigUtil util, + MzsDeliveryRequestValidator validator, Marshaller mzsMarshaller) { this.configs = deliveryRequestConfigs; this.util = util; this.validator = validator; + this.mzsMarshaller = mzsMarshaller; } /** @@ -46,41 +51,29 @@ public class DeliveryRequestAugmenter { var fallbackProfileId = determineProfileIdFrom(requestConfig); var fallbackConfig = configs.get(fallbackProfileId); - if (fallbackConfig == null) { - - if (!validator.isConfigProfileComplete(request.getConfig())) { - throw moaZSException(INCOMPLETE_CONFIG_ERROR_MESSAGE); - } else if (!validator.isTnvzComplete(request)) { - throw moaZSException(INCOMPLETE_TNVZ_ERROR_MESSAGE); - } else { - return request; - } + trace("request config", requestConfig); + trace("fallback config", fallbackConfig); - } else { + var augmentedConfig = (requestConfig != null && fallbackConfig != null) + ? util.merge(requestConfig, fallbackConfig) + : coalesce(requestConfig, fallbackConfig) + .orElseThrow(()-> moaZSException(CONFIG_MISSING_ERROR_MSG)); - var mergedConfig = (requestConfig == null) - ? fallbackConfig - : util.merge(requestConfig, fallbackConfig); + trace("augmented config", augmentedConfig); - if (!validator.isConfigProfileComplete(mergedConfig)) { - var message = format(INCOMPLETE_MERGED_CONFIG_ERROR_MESSAGE, fallbackProfileId); - throw moaZSException(message); - } + validator.isConfigProfileComplete(augmentedConfig); - var mergedRequest = deliveryRequestTypeBuilder(request) - .withConfig(mergedConfig) - .build(); + var augmentedRequest = deliveryRequestTypeBuilder(request) + .withConfig(augmentedConfig) + .build(); - if (!validator.isTnvzComplete(mergedRequest)) { - throw moaZSException(INCOMPLETE_TNVZ_ERROR_MESSAGE); - } + validator.isTnvzComplete(augmentedRequest); - return mergedRequest; + return augmentedRequest; - } } - private String determineProfileIdFrom(ConfigType requestConfig) { + private String determineProfileIdFrom(@Nullable ConfigType requestConfig) { return (requestConfig == null || requestConfig.getProfileID() == null || isProfileMissing(requestConfig.getProfileID())) @@ -92,4 +85,11 @@ public class DeliveryRequestAugmenter { return !configs.containsKey(id); } + private void trace(String description, ConfigType config) { + if (log.isTraceEnabled()) { + log.trace("{} : {}", description, mzsMarshaller.marshallXml(FACTORY.createConfig(config))); + } + } + + } diff --git a/src/main/java/at/gv/egiz/moazs/preprocess/MzsDeliveryRequestValidator.java b/src/main/java/at/gv/egiz/moazs/preprocess/MzsDeliveryRequestValidator.java index 2c2fc36..67086a2 100644 --- a/src/main/java/at/gv/egiz/moazs/preprocess/MzsDeliveryRequestValidator.java +++ b/src/main/java/at/gv/egiz/moazs/preprocess/MzsDeliveryRequestValidator.java @@ -1,90 +1,168 @@ package at.gv.egiz.moazs.preprocess; +import at.gv.egiz.moazs.MoaZSException; import at.gv.zustellung.app2mzs.xsd.*; import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import static at.gv.egiz.moazs.MoaZSException.moaZSException; +import static at.gv.egiz.moazs.scheme.NameSpace.*; +import static java.lang.String.format; + @Component public class MzsDeliveryRequestValidator { /** * Checks if the mandatory fields that are needed to send a tnvz:QueryPersonRequest are present. * @param request - * @return true if mandatory fields are present. + * @throws MoaZSException if a field is missing. */ - public boolean isTnvzComplete(DeliveryRequestType request) { - return !request.getConfig().isPerformQueryPersonRequest() || - (request.getTnvzMetaData() != null - && request.getSender().getCorporateBody() != null); + public void isTnvzComplete(DeliveryRequestType request) { + if (request.getConfig().isPerformQueryPersonRequest()) { + + if (request.getMetaData().getOrigin() == null) + throw mzse(MZS_DELIVERY_REQUEST, MZS_DELIVERY_REQUEST + "/MetaData/Origin is missing."); + + if (request.getMetaData().getDeliveryQuality() == null) + throw mzse(MZS_DELIVERY_REQUEST, MZS_DELIVERY_REQUEST + "/MetaData/DeliveryQuality missing."); + + if (request.getSender().getCorporateBody() == null) + throw mzse(MZS_DELIVERY_REQUEST, MZS_DELIVERY_REQUEST + "/Sender/CorporateBody is missing."); + } } /** * Check if all mandatory fields of configuration are present. * * @param profile - * @return true if all mandatory fields are present. + * @throws MoaZSException if a field is missing. */ - public boolean isConfigProfileComplete(@Nullable ConfigType profile) { - return profile != null - && profile.isPerformQueryPersonRequest() != null - && isTVNZClientConfigured(profile.getTNVZClient(), profile.isPerformQueryPersonRequest()) - && isClientConfigured(profile.getMSGClient()) - && areSinksConfigured(profile.getMsgResponseSinks()); + public void isConfigProfileComplete(@Nullable ConfigType profile) { + + if (profile == null) + throw mzse(MZS_DELIVERY_REQUEST + "/Config"); + + if (profile.isPerformQueryPersonRequest() == null) + throw mzse(MZS_DELIVERY_REQUEST + "/Config", "PerformQueryPersonRequest is missing."); + + isTNVZClientConfigured(profile.getTNVZClient(), profile.isPerformQueryPersonRequest()); + areSinksConfigured(profile.getMsgResponseSinks()); + + try { + isClientConfigured(profile.getMSGClient()); + } catch (MoaZSException ex) { + throw mzse(MZS_MSGCLIENT, ex.getMessage()); + } + } - private boolean isTVNZClientConfigured(ClientType tnvzClient, Boolean isPerformQueryPersonRequest) { - return !isPerformQueryPersonRequest || (tnvzClient != null + private void isTNVZClientConfigured(@Nullable ClientType tnvzClient, Boolean isPerformQueryPersonRequest) { + if (!isPerformQueryPersonRequest) return; + + var isConfigured = tnvzClient != null && tnvzClient.getURL() != null && tnvzClient.getReceiveTimeout() != null - && tnvzClient.getConnectionTimeout() != null - && isSSLConfigured(tnvzClient)); + && tnvzClient.getConnectionTimeout() != null; + + if (!isConfigured) { + if(tnvzClient == null) throw mzse(MZS_TNVZCLIENT); + + var reasons = new StringBuilder("The following elements in " + MZS_TNVZCLIENT + " are missing: "); + if(tnvzClient.getURL() == null) reasons.append("URL;"); + if(tnvzClient.getReceiveTimeout() == null) reasons.append("ReceiveTimeout;"); + if(tnvzClient.getConnectionTimeout() == null) reasons.append("ConnectionTimeout;"); + throw mzse(MZS_TNVZCLIENT, reasons.toString()); + } + + try { + isSSLConfigured(tnvzClient); + } catch (MoaZSException ex) { + throw mzse(MZS_TNVZCLIENT, ex.getMessage()); + } } - private boolean isClientConfigured(ClientType clientParams) { - return clientParams != null + private void isClientConfigured(@Nullable ClientType clientParams) { + var isConfigured = clientParams != null && clientParams.getURL() != null - && isSSLConfigured(clientParams) && clientParams.getReceiveTimeout() != null && clientParams.getConnectionTimeout() != null; + + if (!isConfigured) throw mzse("Client"); + + isSSLConfigured(clientParams); + } - private boolean isSSLConfigured(ClientType clientParams) { - return !clientParams.getURL().startsWith("https") || (clientParams.getSSL() != null - && clientParams.getSSL().isTrustAll() != null - && clientParams.getSSL().isLaxHostNameVerification() != null - && isKeyStoreConfigured(clientParams.getSSL().getKeyStore()) - && isTrustStoreConfigured(clientParams.getSSL().getTrustStore())); + private void isSSLConfigured(ClientType clientParams) { + if (!clientParams.getURL().startsWith("https")) return; + + var isConfigured = (clientParams.getSSL() != null + && clientParams.getSSL().isTrustAll() != null + && clientParams.getSSL().isLaxHostNameVerification() != null); + if (!isConfigured) throw mzse("SSL"); + + try { + isKeyStoreConfigured(clientParams.getSSL().getKeyStore()); + isTrustStoreConfigured(clientParams.getSSL().getTrustStore()); + } catch (MoaZSException ex) { + throw mzse("SSL", ex.getMessage()); + } } - private boolean isKeyStoreConfigured(KeyStoreType keyStore) { - return keyStore == null || (keyStore.getPassword() != null + private void isKeyStoreConfigured(@Nullable KeyStoreType keyStore) { + if (keyStore == null) return; + + var isConfigured = keyStore.getPassword() != null && keyStore.getFileType() != null - && keyStore.getFileName() != null); + && keyStore.getFileName() != null; + if (!isConfigured) throw mzse("KeyStore"); } - private boolean isTrustStoreConfigured(KeyStoreType trustStore) { - return trustStore == null || (trustStore.getPassword() != null + private void isTrustStoreConfigured(@Nullable KeyStoreType trustStore) { + if (trustStore == null) return; + + var isConfigured = trustStore.getPassword() != null && "JKS".equals(trustStore.getFileType()) - && trustStore.getFileName() != null); + && trustStore.getFileName() != null; + if (!isConfigured) throw mzse("TrustStore"); } - private boolean areSinksConfigured(MsgResponseSinksType sinks) { - return sinks != null - && sinks.isLogResponse() != null - && isSaveResponseToFileConfigured(sinks.getSaveResponseToFile()) - && isForwardResponseToServiceConfigured(sinks.getForwardResponseToService()); + private void areSinksConfigured(@Nullable MsgResponseSinksType sinks) { + var isConfigured = sinks != null && sinks.isLogResponse() != null; + if (!isConfigured) throw mzse("MsgResponseSinks"); + + isSaveResponseToFileConfigured(sinks.getSaveResponseToFile()); + isForwardResponseToServiceConfigured(sinks.getForwardResponseToService()); } - private boolean isSaveResponseToFileConfigured(SaveResponseToFileType fileSink) { - return fileSink != null + private void isSaveResponseToFileConfigured(@Nullable SaveResponseToFileType fileSink) { + var isConfigured = fileSink != null && (!fileSink.isActive() || fileSink.getPath() != null); + + if (!isConfigured) throw mzse("SaveResponseToFile"); + } + + private void isForwardResponseToServiceConfigured(@Nullable ForwardResponseToServiceType forwardSink) { + if (forwardSink == null) throw mzse("ForwardResponseToService"); + + if (forwardSink.isActive()) { + try { + isClientConfigured(forwardSink.getMzsClient()); + } catch (MoaZSException e) { + throw mzse("ForwardResponseToService", e.getMessage()); + } + } } - private boolean isForwardResponseToServiceConfigured(ForwardResponseToServiceType forwardSink) { - return forwardSink != null - && (!forwardSink.isActive() || isClientConfigured(forwardSink.getMzsClient())); + private MoaZSException mzse(String missing) { + return moaZSException(format("%s is not configured.", missing)); } + private MoaZSException mzse(String missing, String reason) { + return moaZSException(format("%s is not configured. Reason: %s", missing, reason)); + } + + } -- cgit v1.2.3