Fix sonar issues and add DMF tests for maintenance window feature (#655)

* Fix sonar issues. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * POM cleanup and more sonar issues fixed. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Remove unneeded JavaDocs. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * More sonar issues. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * More issues. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Adapt maintenance window to naming. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Add DMF tests. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Readibility. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Typos fixed. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>
2018-03-08 10:42:25 +01:00
parent f4278c45ef
commit b4414438b0
32 changed files with 411 additions and 364 deletions
--- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpConfiguration.java
+++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpConfiguration.java
@@ -312,12 +312,11 @@ public class AmqpConfiguration {
    AmqpMessageDispatcherService amqpMessageDispatcherService(final RabbitTemplate rabbitTemplate,
            final AmqpMessageSenderService amqpSenderService, final ArtifactUrlHandler artifactUrlHandler,
            final SystemSecurityContext systemSecurityContext, final SystemManagement systemManagement,
-            final ControllerManagement controllerManagement, final TargetManagement targetManagement,
-            final DistributionSetManagement distributionSetManagement,
+            final TargetManagement targetManagement, final DistributionSetManagement distributionSetManagement,
            final SoftwareModuleManagement softwareModuleManagement) {
        return new AmqpMessageDispatcherService(rabbitTemplate, amqpSenderService, artifactUrlHandler,
-                systemSecurityContext, systemManagement, controllerManagement, targetManagement, serviceMatcher,
-                distributionSetManagement, softwareModuleManagement);
+                systemSecurityContext, systemManagement, targetManagement, serviceMatcher, distributionSetManagement,
+                softwareModuleManagement);
    }

    private static Map<String, Object> getTTLMaxArgsAuthenticationQueue() {
--- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java
+++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java
@@ -13,7 +13,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Optional;
 import java.util.stream.Collectors;

 import org.eclipse.hawkbit.api.ApiType;
@@ -30,7 +29,6 @@ import org.eclipse.hawkbit.dmf.json.model.DmfArtifactHash;
 import org.eclipse.hawkbit.dmf.json.model.DmfDownloadAndUpdateRequest;
 import org.eclipse.hawkbit.dmf.json.model.DmfMetadata;
 import org.eclipse.hawkbit.dmf.json.model.DmfSoftwareModule;
-import org.eclipse.hawkbit.repository.ControllerManagement;
 import org.eclipse.hawkbit.repository.DistributionSetManagement;
 import org.eclipse.hawkbit.repository.RepositoryConstants;
 import org.eclipse.hawkbit.repository.SoftwareModuleManagement;
@@ -42,7 +40,6 @@ import org.eclipse.hawkbit.repository.event.remote.entity.CancelTargetAssignment
 import org.eclipse.hawkbit.repository.model.Artifact;
 import org.eclipse.hawkbit.repository.model.SoftwareModule;
 import org.eclipse.hawkbit.repository.model.SoftwareModuleMetadata;
-import org.eclipse.hawkbit.repository.model.Action;
 import org.eclipse.hawkbit.repository.model.Target;
 import org.eclipse.hawkbit.security.SystemSecurityContext;
 import org.eclipse.hawkbit.util.IpUtil;
@@ -76,7 +73,6 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
    private final AmqpMessageSenderService amqpSenderService;
    private final SystemSecurityContext systemSecurityContext;
    private final SystemManagement systemManagement;
-    private final ControllerManagement controllerManagement;
    private final TargetManagement targetManagement;
    private final ServiceMatcher serviceMatcher;
    private final DistributionSetManagement distributionSetManagement;
@@ -95,8 +91,6 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
     *            for execution with system permissions
     * @param systemManagement
     *            the systemManagement
-     * @param controllerManagement
-     *            for target repository access
     * @param targetManagement
     *            to access target information
     * @param serviceMatcher
@@ -108,15 +102,14 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
    protected AmqpMessageDispatcherService(final RabbitTemplate rabbitTemplate,
            final AmqpMessageSenderService amqpSenderService, final ArtifactUrlHandler artifactUrlHandler,
            final SystemSecurityContext systemSecurityContext, final SystemManagement systemManagement,
-            final ControllerManagement controllerManagement, final TargetManagement targetManagement,
-            final ServiceMatcher serviceMatcher, final DistributionSetManagement distributionSetManagement,
+            final TargetManagement targetManagement, final ServiceMatcher serviceMatcher,
+            final DistributionSetManagement distributionSetManagement,
            final SoftwareModuleManagement softwareModuleManagement) {
        super(rabbitTemplate);
        this.artifactUrlHandler = artifactUrlHandler;
        this.amqpSenderService = amqpSenderService;
        this.systemSecurityContext = systemSecurityContext;
        this.systemManagement = systemManagement;
-        this.controllerManagement = controllerManagement;
        this.targetManagement = targetManagement;
        this.serviceMatcher = serviceMatcher;
        this.distributionSetManagement = distributionSetManagement;
@@ -151,7 +144,8 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {

            targetManagement.getByControllerID(assignedEvent.getActions().keySet())
                    .forEach(target -> sendUpdateMessageToTarget(assignedEvent.getTenant(), target,
-                            assignedEvent.getActions().get(target.getControllerId()), modules));
+                            assignedEvent.getActions().get(target.getControllerId()), modules,
+                            assignedEvent.isMaintenanceWindowAvailable()));

        });
    }
@@ -161,25 +155,15 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
     * valid maintenance window available or not based on defined maintenance
     * schedule. In case of no maintenance schedule or if there is a valid
     * window available, the topic {@link EventTopic#DOWNLOAD_AND_INSTALL} is
-     * returned else {@link EventTopic#DOWNLOAD_AND_SKIP} is returned.
+     * returned else {@link EventTopic#DOWNLOAD} is returned.
     *
     * @param target
     *            for which to find the event type
     *
     * @return {@link EventTopic} to use for message.
     */
-    EventTopic getEventTypeForTarget(Target target) {
-        Optional<Action> action = controllerManagement.findOldestActiveActionByTarget(target.getControllerId());
-
-        if (action.isPresent()) {
-            if (action.get().isMaintenanceWindowAvailable()) {
-                return EventTopic.DOWNLOAD_AND_INSTALL;
-            } else {
-                return EventTopic.DOWNLOAD_AND_SKIP;
-            }
-        }
-
-        return EventTopic.DOWNLOAD_AND_INSTALL;
+    private static EventTopic getEventTypeForTarget(final boolean maintenanceWindowAvailable) {
+        return maintenanceWindowAvailable ? EventTopic.DOWNLOAD_AND_INSTALL : EventTopic.DOWNLOAD;
    }

    /**
@@ -216,7 +200,7 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
    }

    protected void sendUpdateMessageToTarget(final String tenant, final Target target, final Long actionId,
-            final Map<SoftwareModule, List<SoftwareModuleMetadata>> modules) {
+            final Map<SoftwareModule, List<SoftwareModuleMetadata>> modules, final boolean maintenanceWindowAvailable) {

        final URI targetAdress = target.getAddress();
        if (!IpUtil.isAmqpUri(targetAdress)) {
@@ -236,7 +220,8 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
        });

        final Message message = getMessageConverter().toMessage(downloadAndUpdateRequest,
-                createConnectorMessagePropertiesEvent(tenant, target.getControllerId(), getEventTypeForTarget(target)));
+                createConnectorMessagePropertiesEvent(tenant, target.getControllerId(),
+                        getEventTypeForTarget(maintenanceWindowAvailable)));
        amqpSenderService.sendMessage(message, targetAdress);
    }

--- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java
+++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java
@@ -102,8 +102,9 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
     * @return a message if <null> no message is send back to sender
     */
    @RabbitListener(queues = "${hawkbit.dmf.rabbitmq.receiverQueue:dmf_receiver}", containerFactory = "listenerContainerFactory")
-    public Message onMessage(final Message message, @Header(MessageHeaderKey.TYPE) final String type,
-            @Header(MessageHeaderKey.TENANT) final String tenant) {
+    public Message onMessage(final Message message,
+            @Header(name = MessageHeaderKey.TYPE, required = false) final String type,
+            @Header(name = MessageHeaderKey.TENANT, required = false) final String tenant) {
        return onMessage(message, type, tenant, getRabbitTemplate().getConnectionFactory().getVirtualHost());
    }

@@ -121,6 +122,9 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
     * @return the rpc message back to supplier.
     */
    public Message onMessage(final Message message, final String type, final String tenant, final String virtualHost) {
+        if (StringUtils.isEmpty(type) || StringUtils.isEmpty(tenant)) {
+            throw new AmqpRejectAndDontRequeueException("Invalid message! tenant and type header are mandatory!");
+        }

        final SecurityContext oldContext = SecurityContextHolder.getContext();
        try {
@@ -215,7 +219,7 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
        action.getDistributionSet().getModules().forEach(module -> modules.put(module, metadata.get(module.getId())));

        amqpMessageDispatcherService.sendUpdateMessageToTarget(action.getTenant(), action.getTarget(), action.getId(),
-                modules);
+                modules, action.isMaintenanceWindowAvailable());
    }

    /**
@@ -282,6 +286,9 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
                && message.getMessageProperties().getCorrelationId().length > 0;
    }

+    // Exception squid:MethodCyclomaticComplexity - false positive, is a simple
+    // mapping
+    @SuppressWarnings("squid:MethodCyclomaticComplexity")
    private Status mapStatus(final Message message, final DmfActionUpdateStatus actionUpdateStatus,
            final Action action) {
        Status status = null;
@@ -311,7 +318,7 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
            status = Status.DOWNLOADED;
            break;
        case CANCEL_REJECTED:
-            status = hanldeCancelRejectedState(message, action);
+            status = handleCancelRejectedState(message, action);
            break;
        default:
            logAndThrowMessageError(message, "Status for action does not exisit.");
@@ -320,14 +327,13 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
        return status;
    }

-    private Status hanldeCancelRejectedState(final Message message, final Action action) {
+    private Status handleCancelRejectedState(final Message message, final Action action) {
        if (action.isCancelingOrCanceled()) {
            return Status.CANCEL_REJECTED;
        }
        logAndThrowMessageError(message,
                "Cancel rejected message is not allowed, if action is on state: " + action.getStatus());
        return null;
-
    }

    private static String convertCorrelationId(final Message message) {