diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java index 86baeba8f..46c320966 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java @@ -11,6 +11,7 @@ package org.eclipse.hawkbit.repository.jpa; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -31,6 +32,8 @@ import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetWithActionType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEventPublisher; import org.springframework.util.CollectionUtils; @@ -42,6 +45,8 @@ import org.springframework.util.CollectionUtils; */ public abstract class AbstractDsAssignmentStrategy { + private static final Logger LOG = LoggerFactory.getLogger(AbstractDsAssignmentStrategy.class); + protected final TargetRepository targetRepository; protected final AfterTransactionCommitExecutor afterCommit; protected final ApplicationEventPublisher eventPublisher; @@ -215,7 +220,6 @@ public abstract class AbstractDsAssignmentStrategy { .publishEvent(new CancelTargetAssignmentEvent(target, actionId, applicationContext.getId()))); } - @SuppressWarnings("squid:S2259") JpaAction createTargetAction(final Map targetsWithActionMap, final JpaTarget target, final JpaDistributionSet set) { @@ -223,17 +227,21 @@ public abstract class AbstractDsAssignmentStrategy { assertActionsPerTargetQuota(target, 1); // create the action - final JpaAction actionForTarget = new JpaAction(); - final TargetWithActionType targetWithActionType = targetsWithActionMap.get(target.getControllerId()); - actionForTarget.setActionType(targetWithActionType.getActionType()); - actionForTarget.setForcedTime(targetWithActionType.getForceTime()); - actionForTarget.setActive(true); - actionForTarget.setTarget(target); - actionForTarget.setDistributionSet(set); - actionForTarget.setMaintenanceWindowSchedule(targetWithActionType.getMaintenanceSchedule()); - actionForTarget.setMaintenanceWindowDuration(targetWithActionType.getMaintenanceWindowDuration()); - actionForTarget.setMaintenanceWindowTimeZone(targetWithActionType.getMaintenanceWindowTimeZone()); - return actionForTarget; + return getTargetWithActionType(targetsWithActionMap, target.getControllerId()).map(targetWithActionType -> { + final JpaAction actionForTarget = new JpaAction(); + actionForTarget.setActionType(targetWithActionType.getActionType()); + actionForTarget.setForcedTime(targetWithActionType.getForceTime()); + actionForTarget.setActive(true); + actionForTarget.setTarget(target); + actionForTarget.setDistributionSet(set); + actionForTarget.setMaintenanceWindowSchedule(targetWithActionType.getMaintenanceSchedule()); + actionForTarget.setMaintenanceWindowDuration(targetWithActionType.getMaintenanceWindowDuration()); + actionForTarget.setMaintenanceWindowTimeZone(targetWithActionType.getMaintenanceWindowTimeZone()); + return actionForTarget; + }).orElseGet(() -> { + LOG.warn("Cannot find targetWithActionType for target '{}'.", target.getControllerId()); + return null; + }); } JpaActionStatus createActionStatus(final JpaAction action, final String actionMessage) { @@ -254,4 +262,14 @@ public abstract class AbstractDsAssignmentStrategy { actionRepository::countByTargetId); } + private static Optional getTargetWithActionType( + final Map targetsWithActionMap, final String controllerId) { + if (targetsWithActionMap.containsKey(controllerId)) { + return Optional.of(targetsWithActionMap.get(controllerId)); + } else { + return targetsWithActionMap.values().stream() + .filter(t -> controllerId.equalsIgnoreCase(t.getControllerId())).findFirst(); + } + } + } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java index 83659dec3..2dad441cb 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java @@ -88,9 +88,6 @@ import org.springframework.transaction.annotation.Transactional; import org.springframework.util.CollectionUtils; import org.springframework.validation.annotation.Validated; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.ObjectWriter; import com.google.common.base.Joiner; import com.google.common.collect.Lists; @@ -288,7 +285,7 @@ public class JpaDeploymentManagement implements DeploymentManagement { targetEntitiesIdsChunks); final Map controllerIdsToActions = createActions(targetsWithActionType, targetEntities, - assignmentStrategy, controllerIDs, distributionSetEntity); + assignmentStrategy, distributionSetEntity); // create initial action status when action is created so we remember // the initial running status because we will change the status // of the action itself and with this action status we have a nicer @@ -376,14 +373,12 @@ public class JpaDeploymentManagement implements DeploymentManagement { private Map createActions(final Collection targetsWithActionType, final List targets, final AbstractDsAssignmentStrategy assignmentStrategy, - final List controllerIDs, final JpaDistributionSet set) { + final JpaDistributionSet set) { final Map targetsWithActionMap = targetsWithActionType.stream() .collect(Collectors.toMap(TargetWithActionType::getControllerId, Function.identity())); - return targets.stream() - .map(trg -> createTargetAction(assignmentStrategy, targetsWithActionType, controllerIDs, targets, - targetsWithActionMap, trg, set)) - .map(actionRepository::save) + return targets.stream().map(trg -> assignmentStrategy.createTargetAction(targetsWithActionMap, trg, set)) + .filter(Objects::nonNull).map(actionRepository::save) .collect(Collectors.toMap(action -> action.getTarget().getControllerId(), Function.identity())); } @@ -785,54 +780,6 @@ public class JpaDeploymentManagement implements DeploymentManagement { return "#" + Joiner.on(",#").join(elements); } - @SuppressWarnings({ "squid:S1695", "squid:S1696" }) - private JpaAction createTargetAction(final AbstractDsAssignmentStrategy assignmentStrategy, - final Collection targetsWithActionType, final List controllerIDs, - final List targets, final Map targetsWithActionMap, - final JpaTarget target, final JpaDistributionSet set) { - try { - return assignmentStrategy.createTargetAction(targetsWithActionMap, target, set); - } catch (final NullPointerException e) { - dumpDistributionSetAssignment(target, set, targetsWithActionType, controllerIDs, targets, - targetsWithActionMap); - throw e; - } - } - - private void dumpDistributionSetAssignment(final JpaTarget target, final JpaDistributionSet set, - final Collection targetsWithActionType, final List controllerIDs, - final List targets, final Map targetsWithActionMap) { - final ObjectWriter writer = new ObjectMapper().writer(); - final String correlationID = "DistributionSetAssignmentDump[" + Thread.currentThread().getId() + "]"; - dump(correlationID, "Target", Objects.toString(target)); - dump(correlationID, "DistributionSet", Objects.toString(set)); - if (target != null) { - dump(correlationID, "ControllerID", Objects.toString(target.getControllerId())); - if (set != null) { - dump(correlationID, "Actions", toString(writer, - actionRepository.findByTargetAndDistributionSet(new PageRequest(0, 500), target, set))); - } - } - dump(correlationID, "ControllerIDs", toString(writer, controllerIDs)); - dump(correlationID, "Targets", toString(writer, targets)); - dump(correlationID, "TargetsWithActionType", toString(writer, targetsWithActionType)); - dump(correlationID, "TargetsWithActionMap", toString(writer, targetsWithActionMap)); - } - - private static void dump(final String prefix, final String key, final String value) { - LOG.error("{} >>> {}: {}", prefix, key, value); - } - - @SuppressWarnings("squid:S1166") - private static String toString(final ObjectWriter writer, final Object o) { - try { - return writer.writeValueAsString(o); - } catch (final JsonProcessingException e) { - LOG.error("Object serialization failed", e); - return e.getMessage(); - } - } - protected ActionRepository getActionRepository() { return actionRepository; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java index 368841a90..6992c544f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java @@ -88,8 +88,10 @@ public class OfflineDsAssignmentStrategy extends AbstractDsAssignmentStrategy { protected JpaAction createTargetAction(final Map targetsWithActionMap, final JpaTarget target, final JpaDistributionSet set) { final JpaAction result = super.createTargetAction(targetsWithActionMap, target, set); - result.setStatus(Status.FINISHED); - result.setActive(Boolean.FALSE); + if (result != null) { + result.setStatus(Status.FINISHED); + result.setActive(Boolean.FALSE); + } return result; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java index 0c81e94e3..1e10223be 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java @@ -93,7 +93,9 @@ public class OnlineDsAssignmentStrategy extends AbstractDsAssignmentStrategy { JpaAction createTargetAction(final Map targetsWithActionMap, final JpaTarget target, final JpaDistributionSet set) { final JpaAction result = super.createTargetAction(targetsWithActionMap, target, set); - result.setStatus(Status.RUNNING); + if (result != null) { + result.setStatus(Status.RUNNING); + } return result; } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResourceTest.java index b4a300225..d3423b833 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResourceTest.java @@ -273,6 +273,29 @@ public class MgmtDistributionSetResourceTest extends AbstractManagementApiIntegr .as("Five targets in repository have DS assigned").hasSize(5); } + @Test + @Description("Ensures that targets can be assigned even if the specified controller IDs are in different case (e.g. 'TARGET1' instead of 'target1'.") + public void assignTargetsToDistributionSetIgnoreCase() throws Exception { + final DistributionSet createdDs = testdataFactory.createDistributionSet(); + + // prepare targets + final String[] knownTargetIds = new String[] { "64-da-a0-02-43-8b", "Trg1", "target2", "target4" }; + final String[] knownTargetIdDifferentCase = new String[] { "64-DA-A0-02-43-8b", "TRG1", "TarGET2", "target4" }; + for (final String targetId : knownTargetIds) { + testdataFactory.createTarget(targetId); + } + final JSONArray list = new JSONArray(); + for (final String targetId : knownTargetIdDifferentCase) { + list.put(new JSONObject().put("id", targetId).put("type", "forced")); + } + + mvc.perform(post( + MgmtRestConstants.DISTRIBUTIONSET_V1_REQUEST_MAPPING + "/" + createdDs.getId() + "/assignedTargets") + .contentType(MediaType.APPLICATION_JSON).content(list.toString())) + .andExpect(status().isOk()); + // we just need to make sure that no error 500 is returned + } + @Test @Description("Ensures that multi target assignment is protected by our 'max targets per manual assignment' quota.") public void assignMultipleTargetsToDistributionSetUntilQuotaIsExceeded() throws Exception {