further improved exception handling for mapping

* removed almost all IllegalStateExceptions
* made content-type required
* validate configuration

Signed-off-by: Thomas Jaeckle <thomas.jaeckle@bosch-si.com>

model/amqp-bridge/src/main/java/org/eclipse/ditto/model/amqpbridge/ExternalMessage.java

@@ -25,6 +25,11 @@ public interface ExternalMessage {
      */
     String CONTENT_TYPE_HEADER = "content-type";
 
+    /**
+     * TODO TJ doc
+     */
+    String ACCEPT_HEADER = "accept";
+
     /**
      * @return the headers of the ExternalMessage
      */

model/amqp-bridge/src/main/java/org/eclipse/ditto/model/amqpbridge/ImmutableMappingContext.java

@@ -46,7 +46,11 @@ private ImmutableMappingContext(final String contentType, final String mappingEn
 
         this.contentType = contentType;
         this.mappingEngine = mappingEngine;
-        this.options = Collections.unmodifiableMap(new HashMap<>(options));
+        final HashMap<String, String> adjustedMap = new HashMap<>(options);
+        if (!adjustedMap.containsKey(ExternalMessage.CONTENT_TYPE_HEADER)) {
+            adjustedMap.put(ExternalMessage.CONTENT_TYPE_HEADER, contentType);
+        }
+        this.options = Collections.unmodifiableMap(adjustedMap);
     }
 
     /**

model/amqp-bridge/src/main/java/org/eclipse/ditto/model/amqpbridge/MappingContext.java

@@ -36,9 +36,11 @@ public interface MappingContext extends Jsonifiable.WithFieldSelectorAndPredicat
     String getContentType();
 
     /**
-     * Fully qualified classname of a mapping engine which can map messages of the contexts content-type.
+     * Fully qualified classname of a mapping engine which can map messages of the contexts content-type or one of the
+     * supported built-in mapping-engines aliases.
      * E.g.:
      * <ul>
+     *     <li>JavaScript</li>
      *     <li><pre>org.eclipse.ditto.services.amqpbridge.mapping.mapper.javascript.JavaScriptMessageMapperRhino</pre></li>
      * </ul>
      * @return the mapping engine name

model/amqp-bridge/src/main/java/org/eclipse/ditto/model/amqpbridge/MessageMapperConfigurationInvalidException.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2017 Bosch Software Innovations GmbH.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/org/documents/epl-2.0/index.php
+ *
+ * Contributors:
+ *    Bosch Software Innovations GmbH - initial contribution
+ */
+package org.eclipse.ditto.model.amqpbridge;
+
+import java.net.URI;
+import java.text.MessageFormat;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.NotThreadSafe;
+
+import org.eclipse.ditto.json.JsonObject;
+import org.eclipse.ditto.model.base.common.HttpStatusCode;
+import org.eclipse.ditto.model.base.exceptions.DittoRuntimeException;
+import org.eclipse.ditto.model.base.exceptions.DittoRuntimeExceptionBuilder;
+import org.eclipse.ditto.model.base.headers.DittoHeaders;
+
+/**
+ * Thrown if the the configuration of a Message Mapper was invalid.
+ */
+@Immutable
+public final class MessageMapperConfigurationInvalidException extends DittoRuntimeException implements AmqpBridgeException {
+
+    /**
+     * Error code of this exception.
+     */
+    public static final String ERROR_CODE = ERROR_CODE_PREFIX + "message.mapper.config.invalid";
+
+    private static final String MESSAGE_TEMPLATE = "The message mapper was not configured correctly as property ''{0}'' was missing from the mapper's configuration.";
+
+    private static final String DEFAULT_DESCRIPTION = "Make sure to add the missing property to the mapper's configuration.";
+
+    private static final long serialVersionUID = -2538489434734124572L;
+
+    private MessageMapperConfigurationInvalidException(final DittoHeaders dittoHeaders, @Nullable final String message,
+            @Nullable final String description,
+            @Nullable final Throwable cause, @Nullable final URI href) {
+        super(ERROR_CODE, HttpStatusCode.BAD_REQUEST, dittoHeaders, message, description, cause, href);
+    }
+
+    /**
+     * A mutable builder for a {@code MessageMapperConfigurationInvalidException}.
+     *
+     * @param missingConfigurationProperty the missingConfigurationProperty which was not present in the config.
+     * @return the builder.
+     */
+    public static Builder newBuilder(final String missingConfigurationProperty) {
+        return new Builder(missingConfigurationProperty);
+    }
+
+    /**
+     * Constructs a new {@code MessageMapperConfigurationInvalidException} object with given message.
+     *
+     * @param message detail message. This message can be later retrieved by the {@link #getMessage()} method.
+     * @param dittoHeaders the headers of the command which resulted in this exception.
+     * @return the new MessageMapperConfigurationInvalidException.
+     */
+    public static MessageMapperConfigurationInvalidException fromMessage(final String message, final DittoHeaders dittoHeaders) {
+        return new Builder()
+                .dittoHeaders(dittoHeaders)
+                .message(message)
+                .build();
+    }
+
+    /**
+     * Constructs a new {@code MessageMapperConfigurationInvalidException} object with the exception message extracted from the given
+     * JSON object.
+     *
+     * @param jsonObject the JSON to read the {@link JsonFields#MESSAGE} field from.
+     * @param dittoHeaders the headers of the command which resulted in this exception.
+     * @return the new MessageMapperConfigurationInvalidException.
+     * @throws org.eclipse.ditto.json.JsonMissingFieldException if the {@code jsonObject} does not have the {@link
+     * JsonFields#MESSAGE} field.
+     */
+    public static MessageMapperConfigurationInvalidException fromJson(final JsonObject jsonObject, final DittoHeaders dittoHeaders) {
+        return fromMessage(readMessage(jsonObject), dittoHeaders);
+    }
+
+    /**
+     * A mutable builder with a fluent API for a {@link MessageMapperConfigurationInvalidException}.
+     */
+    @NotThreadSafe
+    public static final class Builder extends DittoRuntimeExceptionBuilder<MessageMapperConfigurationInvalidException> {
+
+        private Builder() {
+            description(DEFAULT_DESCRIPTION);
+        }
+
+        private Builder(final String missingConfigurationProperty) {
+            this();
+            message(MessageFormat.format(MESSAGE_TEMPLATE, missingConfigurationProperty));
+        }
+
+        @Override
+        protected MessageMapperConfigurationInvalidException doBuild(final DittoHeaders dittoHeaders, @Nullable final String message,
+                @Nullable final String description, @Nullable final Throwable cause, @Nullable final URI href) {
+            return new MessageMapperConfigurationInvalidException(dittoHeaders, message, description, cause, href);
+        }
+    }
+
+}

model/amqp-bridge/src/main/java/org/eclipse/ditto/model/amqpbridge/MessageMappingFailedException.java

@@ -41,6 +41,9 @@ public final class MessageMappingFailedException extends DittoRuntimeException i
             "Check if you are sending the correct content-type/payload combination an that you have registered a mapper " +
                     "which transforms your payload to Ditto Protocol";
 
+    private static final String CONTENT_TYPE_MISSING_DESCRIPTION =
+            "Make sure you specify the 'Content-Type' when sending your message";
+
     private static final long serialVersionUID = -6312489434534126579L;
 
     private MessageMappingFailedException(final DittoHeaders dittoHeaders, @Nullable final String message,
@@ -52,10 +55,11 @@ private MessageMappingFailedException(final DittoHeaders dittoHeaders, @Nullable
     /**
      * A mutable builder for a {@code MessageMappingFailedException}.
      *
-     * @param contentType the contentType of the ExternalMessage which could not be mapped.
+     * @param contentType the contentType of the ExternalMessage which could not be mapped or {@code null} if there was
+     * not a contentType present.
      * @return the builder.
      */
-    public static Builder newBuilder(final String contentType) {
+    public static Builder newBuilder(@Nullable final String contentType) {
         return new Builder(contentType);
     }
 
@@ -97,9 +101,13 @@ private Builder() {
             description(DEFAULT_DESCRIPTION);
         }
 
-        private Builder(final String contentType) {
+        private Builder(@Nullable final String contentType) {
             this();
-            message(MessageFormat.format(MESSAGE_TEMPLATE, contentType));
+            final boolean contentTypeAvailable = contentType == null || contentType.isEmpty();
+            message(MessageFormat.format(MESSAGE_TEMPLATE, contentTypeAvailable ? contentType : "<unspecified>"));
+            if (contentTypeAvailable) {
+                description(CONTENT_TYPE_MISSING_DESCRIPTION);
+            }
         }
 
         @Override

services/amqp-bridge/mapping/src/main/java/org/eclipse/ditto/services/amqpbridge/mapping/mapper/ContentTypeRestrictedMessageMapper.java

@@ -63,23 +63,19 @@ public static MessageMapper wrap(final MessageMapper mapper, final String conten
     }
 
     @Override
-    public Optional<String> getContentType() {
-        return Objects.nonNull(contentTypeOverride) ? Optional.of(contentTypeOverride) : delegate.getContentType();
+    public String getContentType() {
+        return Objects.nonNull(contentTypeOverride) ? contentTypeOverride : delegate.getContentType();
     }
 
     @Override
     public void configure(final MessageMapperConfiguration configuration) {
         delegate.configure(configuration);
-        requireConfiguredContentType();
     }
 
     @Override
     public Adaptable map(final ExternalMessage message) {
         final String actualContentType = findContentType(message)
-                .orElseThrow(() ->
-                        MessageMappingFailedException.newBuilder("?")
-                                .description("Make sure you specify the 'Content-Type' when sending your message")
-                                .build());
+                .orElseThrow(() -> MessageMappingFailedException.newBuilder("").build());
 
         requireMatchingContentType(actualContentType);
         return delegate.map(message);
@@ -88,32 +84,17 @@ public Adaptable map(final ExternalMessage message) {
     @Override
     public ExternalMessage map(final Adaptable adaptable) {
         final String actualContentType = findContentType(adaptable)
-                .orElseThrow(() -> new IllegalArgumentException(
-                        String.format("Adaptable headers do not contain a value for %s", CONTENT_TYPE_KEY)));
+                .orElseThrow(() -> MessageMappingFailedException.newBuilder("").build());
 
         requireMatchingContentType(actualContentType);
         return delegate.map(adaptable);
     }
 
-    private void requireConfiguredContentType() {
-        if (!getContentType().isPresent()) {
-            throw new IllegalConfigurationException("Required configuration property missing: '%s'" +
-                    MessageMapperConfigurationProperties.CONTENT_TYPE);
-        }
-    }
-
 
     private void requireMatchingContentType(final String actualContentType) {
-        final String contentType = getContentType().filter(s -> !s.isEmpty()).orElseThrow(
-                () -> new NotYetConfiguredException(String.format(
-                        "A matching Content-Type is required, but none configured. Set a Content-Type with the following key in configuration: %s",
-                        MessageMapperConfigurationProperties.CONTENT_TYPE))
-        );
-
-        if (!contentType.equalsIgnoreCase(actualContentType)) {
-            throw new IllegalArgumentException(
-                    String.format("Unsupported value for %s: actual='%s', expected='%s'",
-                            CONTENT_TYPE_KEY, actualContentType, contentType));
+
+        if (!getContentType().equalsIgnoreCase(actualContentType)) {
+            throw MessageMappingFailedException.newBuilder(actualContentType).build();
         }
     }
 

services/amqp-bridge/mapping/src/main/java/org/eclipse/ditto/services/amqpbridge/mapping/mapper/DefaultMessageMapperFactory.java

@@ -25,6 +25,7 @@
 import javax.annotation.concurrent.Immutable;
 
 import org.eclipse.ditto.model.amqpbridge.MappingContext;
+import org.eclipse.ditto.model.amqpbridge.MessageMapperConfigurationInvalidException;
 
 import akka.actor.DynamicAccess;
 import akka.event.DiagnosticLoggingAdapter;
@@ -134,7 +135,7 @@ public List<MessageMapper> mappersOf(final List<MappingContext> contexts) {
     @Override
     public MessageMapperRegistry registryOf(final MappingContext defaultContext, final List<MappingContext> contexts) {
         final MessageMapper defaultMapper = mapperOf(defaultContext)
-                .orElseThrow(() ->
+                .orElseThrow(() -> // TODO TJ other exception - also in JavaDoc
                         new IllegalArgumentException("No mapper found for default context: " + defaultContext));
 
         final List<MessageMapper> mappers = contexts.stream()
@@ -190,9 +191,9 @@ Optional<MessageMapper> findClassAndCreateInstance(final MappingContext mappingC
 
     private boolean configureInstance(final MessageMapper mapper, final DefaultMessageMapperConfiguration options) {
         try {
-            mapper.configure(options);
+            mapper.configureWithValidation(options);
             return true;
-        } catch (final IllegalArgumentException e) {
+        } catch (final MessageMapperConfigurationInvalidException e) {
             log.warning("Failed to apply configuration <{}> to mapper instance <{}>: {}", options, mapper,
                     e.getMessage());
             return false;

services/amqp-bridge/mapping/src/main/java/org/eclipse/ditto/services/amqpbridge/mapping/mapper/DefaultMessageMapperRegistry.java

@@ -19,6 +19,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -33,11 +34,7 @@ private DefaultMessageMapperRegistry(final MessageMapper defaultMapper, final Co
         this.defaultMapper = checkNotNull(defaultMapper);
         this.mappers = Collections.unmodifiableMap(
                 mappers.stream()
-                        .filter(m -> m.getContentType().isPresent())
-                        .collect(Collectors.toMap(m -> m.getContentType().orElseThrow(
-                                () -> new IllegalArgumentException(
-                                        "Mappers contains a mapper without content type: " + m)),
-                                m -> m))
+                        .collect(Collectors.toMap(MessageMapper::getContentType, Function.identity()))
         );
     }
 
@@ -69,8 +66,12 @@ public static DefaultMessageMapperRegistry of(final MessageMapper defaultMapper,
 
     @Override
     public boolean equals(final Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
         final DefaultMessageMapperRegistry that = (DefaultMessageMapperRegistry) o;
         return Objects.equals(mappers, that.mappers) &&
                 Objects.equals(defaultMapper, that.defaultMapper);

services/amqp-bridge/mapping/src/main/java/org/eclipse/ditto/services/amqpbridge/mapping/mapper/DittoMessageMapper.java

@@ -27,6 +27,7 @@
 import org.eclipse.ditto.model.amqpbridge.AmqpBridgeModelFactory;
 import org.eclipse.ditto.model.amqpbridge.ExternalMessage;
 import org.eclipse.ditto.model.amqpbridge.MappingContext;
+import org.eclipse.ditto.model.amqpbridge.MessageMappingFailedException;
 import org.eclipse.ditto.model.base.common.DittoConstants;
 import org.eclipse.ditto.model.base.exceptions.DittoJsonException;
 import org.eclipse.ditto.model.base.headers.DittoHeaders;
@@ -51,8 +52,8 @@ public final class DittoMessageMapper implements MessageMapper {
     );
 
     @Override
-    public Optional<String> getContentType() {
-        return Optional.ofNullable(DittoConstants.DITTO_PROTOCOL_CONTENT_TYPE);
+    public String getContentType() {
+        return DittoConstants.DITTO_PROTOCOL_CONTENT_TYPE;
     }
 
 
@@ -78,7 +79,7 @@ public Adaptable map(final ExternalMessage message) {
     public ExternalMessage map(final Adaptable adaptable) {
         final ExternalMessage.MessageType messageType = determineMessageType(adaptable);
         final Map<String, String> headers = new LinkedHashMap<>(adaptable.getHeaders().orElse(DittoHeaders.empty()));
-        getContentType().ifPresent(value -> headers.put(CONTENT_TYPE_KEY, value));
+        headers.put(CONTENT_TYPE_KEY, getContentType());
 
         final String jsonString = ProtocolFactory.wrapAsJsonifiableAdaptable(adaptable).toJsonString();
 
@@ -92,13 +93,16 @@ private static String extractPayloadAsString(final ExternalMessage message) {
         if (message.isTextMessage()) {
             payload = message.getTextPayload();
         } else if (message.isBytesMessage()) {
+            // TODO TJ Handle charset correctly
             payload = message.getBytePayload().map(ByteBuffer::array).map(ba -> new String(ba, StandardCharsets.UTF_8));
         } else {
             payload = Optional.empty();
         }
 
-        return payload.filter(s -> !s.isEmpty()).orElseThrow(
-                () -> new IllegalArgumentException("Failed to extract string payload of message: " + message));
+        return payload.filter(s -> !s.isEmpty()).orElseThrow(() ->
+                MessageMappingFailedException.newBuilder(message.findContentType().orElse(""))
+                    .description("As payload was absent or empty, please make sure to send payload in your messages.")
+                    .build());
     }
 
     /**
@@ -132,7 +136,7 @@ private ExternalMessage.MessageType determineMessageType(final Adaptable adaptab
             default:
                 final String errorMessage = MessageFormat.format("Cannot map '{0}' message. Only [{1}, {2}] allowed.",
                         criterion.getName(), TopicPath.Criterion.COMMANDS, TopicPath.Criterion.EVENTS);
-                throw new IllegalArgumentException(errorMessage);
+                throw new IllegalArgumentException(errorMessage); // TODO TJ other exception
         }
     }