Feature add optional name to thing created (#888)

* First implementation pushed because of debugging purpose Signed-off-by: Ammar Bikic <[email protected]> * Add name field and tests regarding name field functionality in THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * SonarQube realted changes in name field functionality in THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Add name field and tests regarding name field functionality in UPDATE_ATTRIBUTES Signed-off-by: Ammar Bikic <[email protected]> * Adapt documentation due to name field in THING_CREATED and UPDATE_ATTRIBUTES Signed-off-by: Ammar Bikic <[email protected]> * Add integration tests regarding name field functionality in THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Reformat after finding format bug regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Reformat after finding the real format bug regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Reformat regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Use constant in THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Format in THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Resolving peer review comments regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Resolving peer review comments (organize imports) regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Refactoring regarding THING_CREATED Signed-off-by: Ammar Bikic <[email protected]> * Refactoring due to peer review Signed-off-by: Ammar Bikic <[email protected]> * Refactoring due to peer review Signed-off-by: Ammar Bikic <[email protected]> * Excluding UPDATE_ATTRIBUTES changes and provide functionality of updating the name property in THING_CREATED message Signed-off-by: Ammar Bikic <[email protected]> * Refactoring due to peer review Signed-off-by: Ammar Bikic <[email protected]> * Refactoring due to peer review Signed-off-by: Ammar Bikic <[email protected]> * Fix SonarQube finding Signed-off-by: Ammar Bikic <[email protected]> * Merge master into current branch Signed-off-by: Ammar Bikic <[email protected]> * Fix peer review findings Signed-off-by: Ammar Bikic <[email protected]>
eclipse-hawkbit · Oct 24, 2019 · af0f7ce · af0f7ce
1 parent b50a82f
commit af0f7ce
Show file tree

Hide file tree

Showing 10 changed files with 525 additions and 388 deletions.
diff --git a/docs/content/apis/dmf_api.md b/docs/content/apis/dmf_api.md
@@ -52,12 +52,22 @@ Message Properties | Description
 content_type       | The content type of the payload                                                                      | String | true
 reply_to           | Exchange to reply to. The default is sp.direct.exchange which is bound to the sp_direct_queue | String | false
 
-Example headers
+Example headers and payload:
 
 Header                                                                             | MessageProperties
 ----------------------------------------------------------------------------------- | -------------------------------------------------------------------------------
 type=THING\_CREATED <br /> tenant=tenant123 <br /> thingId=abc  <br /> sender=Lwm2m | content\_type=application/json <br /> reply_to (optional) =sp.connector.replyTo
 
+Payload Template (optional):
+
+```json
+{
+    "name": "String"
+}
+```
+
+The "name" property specifies the name of the thing, which by default is the thing ID. This property is optional.
+
 
 ### THING_REMOVED
 

diff --git a/...mf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/...mf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java
@@ -10,6 +10,7 @@
 
 import static org.eclipse.hawkbit.repository.RepositoryConstants.MAX_ACTION_COUNT;
 import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.MULTI_ASSIGNMENTS_ENABLED;
+import static org.springframework.util.StringUtils.hasText;
 
 import java.io.Serializable;
 import java.net.URI;
@@ -26,6 +27,7 @@
 import org.eclipse.hawkbit.dmf.amqp.api.MessageType;
 import org.eclipse.hawkbit.dmf.json.model.DmfActionUpdateStatus;
 import org.eclipse.hawkbit.dmf.json.model.DmfAttributeUpdate;
+import org.eclipse.hawkbit.dmf.json.model.DmfCreateThing;
 import org.eclipse.hawkbit.dmf.json.model.DmfUpdateMode;
 import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions;
 import org.eclipse.hawkbit.im.authentication.TenantAwareAuthenticationDetails;
@@ -51,6 +53,7 @@
 import org.springframework.amqp.core.Message;
 import org.springframework.amqp.rabbit.annotation.RabbitListener;
 import org.springframework.amqp.rabbit.core.RabbitTemplate;
+import org.springframework.amqp.support.converter.MessageConversionException;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.messaging.handler.annotation.Header;
 import org.springframework.security.authentication.AnonymousAuthenticationToken;
@@ -84,6 +87,8 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
 
     private static final String THING_ID_NULL = "ThingId is null";
 
+    private static final String EMPTY_MESSAGE_BODY = "\"\"";
+
     /**
      * Constructor.
      * 
@@ -122,7 +127,6 @@ public AmqpMessageHandlerService(final RabbitTemplate rabbitTemplate,
      *            the message type
      * @param tenant
      *            the contentType of the message
-     * 
      * @return a message if <null> no message is send back to sender
      */
     @RabbitListener(queues = "${hawkbit.dmf.rabbitmq.receiverQueue:dmf_receiver}", containerFactory = "listenerContainerFactory")
@@ -198,12 +202,14 @@ private static void setTenantSecurityContext(final String tenantId) {
     }
 
     /**
-     * Method to create a new target or to find the target if it already exists.
+     * Method to create a new target or to find the target if it already exists and
+     * update its poll time, status and optionally its name.
      *
-     * @param targetID
-     *            the ID of the target/thing
-     * @param ip
-     *            the ip of the target/thing
+     * @param message
+     *            the message that contains replyTo property and optionally the name
+     *            in body
+     * @param virtualHost
+     *            the virtual host
      */
     private void registerTarget(final Message message, final String virtualHost) {
         final String thingId = getStringHeaderKey(message, MessageHeaderKey.THING_ID, THING_ID_NULL);
@@ -215,14 +221,29 @@ private void registerTarget(final Message message, final String virtualHost) {
 
         try {
             final URI amqpUri = IpUtil.createAmqpUri(virtualHost, replyTo);
-            final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(thingId, amqpUri);
+            final Target target;
+            if (isOptionalMessageBodyEmpty(message)) {
+                target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(thingId, amqpUri);
+            } else {
+                checkContentTypeJson(message);
+
+                    target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(thingId, amqpUri, convertMessage(message, DmfCreateThing.class).getName());
+
+            }
             LOG.debug("Target {} reported online state.", thingId);
             sendUpdateCommandToTarget(target);
-        } catch (EntityAlreadyExistsException e) {
-            throw new AmqpRejectAndDontRequeueException("Target already registered, message will be ignored!", e);
+        } catch (final EntityAlreadyExistsException e) {
+            throw new AmqpRejectAndDontRequeueException(
+                    "Tried to register previously registered target, message will be ignored!", e);
         }
     }
 
+    private static boolean isOptionalMessageBodyEmpty(final Message message) {
+        // empty byte array message body is serialized to double-quoted string
+        // by message converter and should also be considered as empty
+        return isMessageBodyEmpty(message) || EMPTY_MESSAGE_BODY.equals(new String(message.getBody()));
+    }
+
     private void sendUpdateCommandToTarget(final Target target) {
         if (isMultiAssignmentsEnabled()) {
             sendCurrentActionsAsMultiActionToTarget(target);

diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/BaseAmqpService.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/BaseAmqpService.java
@@ -69,7 +69,7 @@ protected MessageConverter getMessageConverter() {
         return rabbitTemplate.getMessageConverter();
     }
 
-    private static boolean isMessageBodyEmpty(final Message message) {
+    protected static boolean isMessageBodyEmpty(final Message message) {
         return message.getBody() == null || message.getBody().length == 0;
     }
 

diff --git a/...awkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java b/...awkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java
@@ -35,6 +35,7 @@
 import org.eclipse.hawkbit.dmf.json.model.DmfActionStatus;
 import org.eclipse.hawkbit.dmf.json.model.DmfActionUpdateStatus;
 import org.eclipse.hawkbit.dmf.json.model.DmfAttributeUpdate;
+import org.eclipse.hawkbit.dmf.json.model.DmfCreateThing;
 import org.eclipse.hawkbit.dmf.json.model.DmfDownloadResponse;
 import org.eclipse.hawkbit.dmf.json.model.DmfUpdateMode;
 import org.eclipse.hawkbit.repository.ArtifactManagement;
@@ -70,6 +71,7 @@
 import org.springframework.amqp.core.MessageProperties;
 import org.springframework.amqp.rabbit.core.RabbitTemplate;
 import org.springframework.amqp.support.converter.Jackson2JsonMessageConverter;
+import org.springframework.amqp.support.converter.MessageConversionException;
 import org.springframework.amqp.support.converter.MessageConverter;
 import org.springframework.http.HttpStatus;
 
@@ -82,8 +84,8 @@
 @Story("AmqpMessage Handler Service Test")
 public class AmqpMessageHandlerServiceTest {
 
-    private static final String FAIL_MESSAGE_AMQP_REJECT_REASON = AmqpRejectAndDontRequeueException.class.getSimpleName()
-            + " was expected, ";
+    private static final String FAIL_MESSAGE_AMQP_REJECT_REASON = AmqpRejectAndDontRequeueException.class
+            .getSimpleName() + " was expected, ";
 
     private static final String SHA1 = "12345";
     private static final String VIRTUAL_HOST = "vHost";
@@ -136,6 +138,9 @@ public class AmqpMessageHandlerServiceTest {
     @Captor
     private ArgumentCaptor<String> targetIdCaptor;
 
+    @Captor
+    private ArgumentCaptor<URI> uriCaptor;
+
     @Captor
     private ArgumentCaptor<UpdateMode> modeCaptor;
 
@@ -176,23 +181,60 @@ public void wrongContentType() {
     @Description("Tests the creation of a target/thing by calling the same method that incoming RabbitMQ messages would access.")
     public void createThing() {
         final String knownThingId = "1";
-        final MessageProperties messageProperties = createMessageProperties(MessageType.THING_CREATED);
-        messageProperties.setHeader(MessageHeaderKey.THING_ID, "1");
-        final Message message = messageConverter.toMessage(new byte[0], messageProperties);
 
         final Target targetMock = mock(Target.class);
 
-        final ArgumentCaptor<String> targetIdCaptor = ArgumentCaptor.forClass(String.class);
-        final ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+        targetIdCaptor = ArgumentCaptor.forClass(String.class);
+        uriCaptor = ArgumentCaptor.forClass(URI.class);
         when(controllerManagementMock.findOrRegisterTargetIfItDoesNotExist(targetIdCaptor.capture(),
                 uriCaptor.capture())).thenReturn(targetMock);
         when(controllerManagementMock.findOldestActiveActionByTarget(any())).thenReturn(Optional.empty());
 
-        amqpMessageHandlerService.onMessage(message, MessageType.THING_CREATED.name(), TENANT, VIRTUAL_HOST);
+        amqpMessageHandlerService.onMessage(createMessage(new byte[0], getThingCreatedMessageProperties(knownThingId)),
+                MessageType.THING_CREATED.name(), TENANT, VIRTUAL_HOST);
 
         // verify
         assertThat(targetIdCaptor.getValue()).as("Thing id is wrong").isEqualTo(knownThingId);
-        assertThat(uriCaptor.getValue().toString()).as("Uri is not right").isEqualTo("amqp://"+VIRTUAL_HOST+"/MyTest");
+        assertThat(uriCaptor.getValue().toString()).as("Uri is not right")
+                .isEqualTo("amqp://" + VIRTUAL_HOST + "/MyTest");
+    }
+
+    @Test
+    @Description("Tests the creation of a target/thing with specified name by calling the same method that incoming RabbitMQ messages would access.")
+    public void createThingWithName() {
+        final String knownThingId = "2";
+        final DmfCreateThing targetProperties = new DmfCreateThing();
+        targetProperties.setName("NonDefaultTargetName");
+
+        final Target targetMock = mock(Target.class);
+
+        targetIdCaptor = ArgumentCaptor.forClass(String.class);
+        uriCaptor = ArgumentCaptor.forClass(URI.class);
+        final ArgumentCaptor<String> targetNameCaptor = ArgumentCaptor.forClass(String.class);
+
+        when(controllerManagementMock.findOrRegisterTargetIfItDoesNotExist(targetIdCaptor.capture(),
+                uriCaptor.capture(), targetNameCaptor.capture())).thenReturn(targetMock);
+        when(controllerManagementMock.findOldestActiveActionByTarget(any())).thenReturn(Optional.empty());
+
+        amqpMessageHandlerService.onMessage(
+                createMessage(targetProperties, getThingCreatedMessageProperties(knownThingId)),
+                MessageType.THING_CREATED.name(), TENANT, "vHost");
+
+        assertThat(targetIdCaptor.getValue()).as("Thing id is wrong").isEqualTo(knownThingId);
+        assertThat(uriCaptor.getValue().toString()).as("Uri is not right").isEqualTo("amqp://vHost/MyTest");
+        assertThat(targetNameCaptor.getValue()).as("Thing name is not right").isEqualTo(targetProperties.getName());
+    }
+
+    @Test
+    @Description("Tests not allowed body in message")
+    public void createThingWithWrongBody() {
+        final String knownThingId = "3";
+
+        assertThatExceptionOfType(MessageConversionException.class)
+                .as("MessageConversionException was excepeted due to wrong body")
+                .isThrownBy(() -> amqpMessageHandlerService.onMessage(
+                        createMessage("Not allowed Body".getBytes(), getThingCreatedMessageProperties(knownThingId)),
+                        MessageType.THING_CREATED.name(), TENANT, VIRTUAL_HOST));
     }
 
     @Test
@@ -308,15 +350,13 @@ public void invalidEventTopic() {
         final Message message = new Message(new byte[0], messageProperties);
 
         assertThatExceptionOfType(AmqpRejectAndDontRequeueException.class)
-                .as(FAIL_MESSAGE_AMQP_REJECT_REASON + "due to unknown message type")
-                .isThrownBy(() -> amqpMessageHandlerService.onMessage(message, "unknownMessageType", TENANT,
-                        VIRTUAL_HOST));
+                .as(FAIL_MESSAGE_AMQP_REJECT_REASON + "due to unknown message type").isThrownBy(
+                        () -> amqpMessageHandlerService.onMessage(message, "unknownMessageType", TENANT, VIRTUAL_HOST));
 
         messageProperties.setHeader(MessageHeaderKey.TOPIC, "wrongTopic");
         assertThatExceptionOfType(AmqpRejectAndDontRequeueException.class)
-                .as(FAIL_MESSAGE_AMQP_REJECT_REASON + "due to unknown topic")
-                .isThrownBy(() -> amqpMessageHandlerService.onMessage(message, MessageType.EVENT.name(), TENANT,
-                        VIRTUAL_HOST));
+                .as(FAIL_MESSAGE_AMQP_REJECT_REASON + "due to unknown topic").isThrownBy(() -> amqpMessageHandlerService
+                        .onMessage(message, MessageType.EVENT.name(), TENANT, VIRTUAL_HOST));
 
         messageProperties.setHeader(MessageHeaderKey.TOPIC, EventTopic.CANCEL_DOWNLOAD.name());
         assertThatExceptionOfType(AmqpRejectAndDontRequeueException.class)
@@ -532,12 +572,12 @@ public void deleteThing() {
         final String knownThingId = "1";
         final MessageProperties messageProperties = createMessageProperties(MessageType.THING_REMOVED);
         messageProperties.setHeader(MessageHeaderKey.THING_ID, knownThingId);
-        final Message message = messageConverter.toMessage(new byte[0],messageProperties);
+        final Message message = messageConverter.toMessage(new byte[0], messageProperties);
 
         // test
         amqpMessageHandlerService.onMessage(message, MessageType.THING_REMOVED.name(), TENANT, VIRTUAL_HOST);
 
-        //verify
+        // verify
         verify(controllerManagementMock).deleteExistingTarget(knownThingId);
     }
 
@@ -547,11 +587,20 @@ public void deleteThingWithoutThingId() {
         // prepare invalid message
         final MessageProperties messageProperties = createMessageProperties(MessageType.THING_REMOVED);
         final Message message = messageConverter.toMessage(new byte[0], messageProperties);
-        
+
         assertThatExceptionOfType(AmqpRejectAndDontRequeueException.class)
                 .as(FAIL_MESSAGE_AMQP_REJECT_REASON + "since no thingId was set")
                 .isThrownBy(() -> amqpMessageHandlerService.onMessage(message, MessageType.THING_REMOVED.name(), TENANT,
                         VIRTUAL_HOST));
     }
 
+    private MessageProperties getThingCreatedMessageProperties(String thingId) {
+        final MessageProperties messageProperties = createMessageProperties(MessageType.THING_CREATED);
+        messageProperties.setHeader(MessageHeaderKey.THING_ID, thingId);
+        return messageProperties;
+    }
+
+    private Message createMessage(Object object, MessageProperties messageProperties) {
+        return messageConverter.toMessage(object, messageProperties);
+    }
 }
diff --git a/.../hawkbit-dmf-api/src/main/java/org/eclipse/hawkbit/dmf/json/model/DmfAttributeUpdate.java b/.../hawkbit-dmf-api/src/main/java/org/eclipse/hawkbit/dmf/json/model/DmfAttributeUpdate.java
@@ -1,6 +1,6 @@
 /**
  * Copyright (c) 2015 Bosch Software Innovations GmbH and others.
- *
+ * 
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at

diff --git a/...-dmf/hawkbit-dmf-api/src/main/java/org/eclipse/hawkbit/dmf/json/model/DmfCreateThing.java b/...-dmf/hawkbit-dmf-api/src/main/java/org/eclipse/hawkbit/dmf/json/model/DmfCreateThing.java
@@ -0,0 +1,34 @@
+/**
+ * Copyright (c) 2019 Bosch Software Innovations GmbH and others.
+ * 
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.hawkbit.dmf.json.model;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * JSON representation of the Attribute THING_CREATED message.
+ */
+@JsonInclude(Include.NON_NULL)
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class DmfCreateThing {
+
+    @JsonProperty
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(final String name) {
+        this.name = name;
+    }
+
+}