Feature add optional name to thing created (#888)

* First implementation pushed because of debugging purpose

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Add name field and tests regarding name field functionality in THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* SonarQube realted changes in name field functionality in THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Add name field and tests regarding name field functionality in UPDATE_ATTRIBUTES

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Adapt documentation due to name field in THING_CREATED and UPDATE_ATTRIBUTES

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Add integration tests regarding name field functionality in THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Reformat after finding format bug regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Reformat after finding the real format bug regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Reformat regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Use constant in THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Format in THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Resolving peer review comments regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Resolving peer review comments (organize imports) regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Refactoring regarding THING_CREATED

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Refactoring due to peer review

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Refactoring due to peer review

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Excluding UPDATE_ATTRIBUTES changes and provide functionality of updating the name property in THING_CREATED message

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Refactoring due to peer review

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Refactoring due to peer review

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Fix SonarQube finding

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Merge master into current branch

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>

* Fix peer review findings

Signed-off-by: Ammar Bikic <ammar.bikic@bosch-si.com>
This commit is contained in:
Ammar Bikic
2019-10-24 12:13:35 +02:00
committed by Dominic Schabel
parent b50a82fc0f
commit af0f7ceb5a
10 changed files with 525 additions and 388 deletions

View File

@@ -10,6 +10,7 @@ package org.eclipse.hawkbit.amqp;
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.MessageHeaderKey;
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.AmqpRejectAndDontRequeueException;
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 class AmqpMessageHandlerService extends BaseAmqpService {
* 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 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
}
/**
* 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 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
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);

View File

@@ -69,7 +69,7 @@ public class BaseAmqpService {
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;
}

View File

@@ -35,6 +35,7 @@ import org.eclipse.hawkbit.dmf.amqp.api.MessageType;
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.Message;
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 @@ import io.qameta.allure.Story;
@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 class AmqpMessageHandlerServiceTest {
@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 class AmqpMessageHandlerServiceTest {
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 class AmqpMessageHandlerServiceTest {
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 class AmqpMessageHandlerServiceTest {
// 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);
}
}

View File

@@ -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

View File

@@ -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;
}
}