From c4fd082098b01228cd0e719cd94743ac87b594bc Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 09:39:07 +0200 Subject: [PATCH 01/12] fix the lookup of artifacts with the same lookup key, e.g. SHA1 hash if multiple artifacts with key exists for different software modules Signed-off-by: Michael Hirsch --- .../amqp/AmqpMessageHandlerService.java | 14 +++-- .../repository/ControllerManagement.java | 27 +++++++++ .../specifications/ActionSpecifications.java | 60 +++++++++++++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java diff --git a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 41e70f91f..e05ddf71d 100644 --- a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -166,6 +166,8 @@ public class AmqpMessageHandlerService extends BaseAmqpService { final LocalArtifact localArtifact = findLocalArtifactByFileResource(fileResource); if (localArtifact == null) { + LOG.info("target {} requested file resource which does not exists to download", + secruityToken.getControllerId(), fileResource); throw new EntityNotFoundException(); } @@ -175,10 +177,14 @@ public class AmqpMessageHandlerService extends BaseAmqpService { // assigned to this controller. Otherwise no controllerId is set = // anonymous download if (secruityToken.getControllerId() != null) { - final Action action = controllerManagement.getActionForDownloadByTargetAndSoftwareModule( - secruityToken.getControllerId(), localArtifact.getSoftwareModule()); - LOG.info("Found action for download authentication request action: {}, resource: {}", action, - secruityToken.getFileResource()); + LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", + secruityToken.getControllerId(), localArtifact); + if (!controllerManagement.hasTargetArtifactAssigned(secruityToken.getControllerId(), localArtifact)) { + LOG.info("target {} tried to download artifact {} which is not assigned to the target"); + throw new EntityNotFoundException(); + } + LOG.info("download security check for target {} and artifact {} granted", + secruityToken.getControllerId(), localArtifact); } final Artifact artifact = convertDbArtifact(artifactManagement.loadLocalArtifactBinary(localArtifact)); diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java index 0bb94bd72..9510ae899 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java @@ -28,12 +28,14 @@ import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.ActionStatus; import org.eclipse.hawkbit.repository.model.ActionStatus_; import org.eclipse.hawkbit.repository.model.DistributionSet; +import org.eclipse.hawkbit.repository.model.LocalArtifact; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetInfo; import org.eclipse.hawkbit.repository.model.TargetUpdateStatus; import org.eclipse.hawkbit.repository.model.Target_; import org.eclipse.hawkbit.repository.model.TenantConfiguration; +import org.eclipse.hawkbit.repository.specifications.ActionSpecifications; import org.eclipse.hawkbit.security.HawkbitSecurityProperties; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey; import org.hibernate.validator.constraints.NotEmpty; @@ -166,6 +168,31 @@ public class ControllerManagement { return action.get(0); } + /** + * Checks if a given target has currently or has even been assigned to the + * given artifact through the action history list. This can e.g. indicate if + * a target is allowed to download a given artifact because it has currently + * assigned or had ever been assigned to the target and so it's visible to a + * specific target e.g. for downloading. + * + * @param targetId + * the ID of the target to check + * @param localArtifact + * the artifact to verify if the given target had even been + * assigned to + * @return {@code true} if the given target has currently or had ever a + * relation to the given artifact through the action history, + * otherwise {@code false} + */ + public boolean hasTargetArtifactAssigned(@NotNull final String targetId, + @NotNull final LocalArtifact localArtifact) { + final Target target = targetRepository.findByControllerId(targetId); + if (target == null) { + return false; + } + return actionRepository.count(ActionSpecifications.hasTargetArtifactAssigned(target, localArtifact)) > 0; + } + /** * Refreshes the time of the last time the controller has been connected to * the server. diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java new file mode 100644 index 000000000..ef8c97e22 --- /dev/null +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java @@ -0,0 +1,60 @@ +/** + * 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 + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.specifications; + +import javax.persistence.criteria.Join; +import javax.persistence.criteria.ListJoin; +import javax.persistence.criteria.SetJoin; + +import org.eclipse.hawkbit.repository.model.Action; +import org.eclipse.hawkbit.repository.model.Action_; +import org.eclipse.hawkbit.repository.model.DistributionSet; +import org.eclipse.hawkbit.repository.model.DistributionSet_; +import org.eclipse.hawkbit.repository.model.LocalArtifact; +import org.eclipse.hawkbit.repository.model.LocalArtifact_; +import org.eclipse.hawkbit.repository.model.SoftwareModule; +import org.eclipse.hawkbit.repository.model.SoftwareModule_; +import org.eclipse.hawkbit.repository.model.Target; +import org.springframework.data.jpa.domain.Specification; + +/** + * Utility class for {@link Action}s {@link Specification}s. The class provides + * Spring Data JPQL Specifications. + * + */ +public class ActionSpecifications { + + private ActionSpecifications() { + // utility class + } + + /** + * Specification which joins all necessary tables to retrieve the dependency + * between a target and a local file assignment through the assigen action + * of the target. All actions are included, not only active actions. + * + * @param target + * the target to verfiy if the given artifact is currently + * assigned or had been assigned + * @param localArtifact + * the local artifact to check wherever the target had ever been + * assigned + * @return a specification to use with spring JPA + */ + public static Specification hasTargetArtifactAssigned(final Target target, + final LocalArtifact localArtifact) { + return (actionRoot, query, cb) -> { + final Join dsJoin = actionRoot.join(Action_.distributionSet); + final SetJoin modulesJoin = dsJoin.join(DistributionSet_.modules); + final ListJoin artifactsJoin = modulesJoin.join(SoftwareModule_.artifacts); + return cb.and(cb.equal(artifactsJoin.get(LocalArtifact_.id), localArtifact.getId()), + cb.equal(actionRoot.get(Action_.target), target)); + }; + } +} From 0b5286299c7a2453a635f8a7d64eac33cb99d46d Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 10:03:19 +0200 Subject: [PATCH 02/12] fix unit tests after changing the lookup way Signed-off-by: Michael Hirsch --- .../eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java b/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java index 930ca02ad..0faa0a8a6 100644 --- a/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java +++ b/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java @@ -313,11 +313,9 @@ public class AmqpMessageHandlerServiceTest { // mock final LocalArtifact localArtifactMock = mock(LocalArtifact.class); - final Action actionMock = mock(Action.class); final DbArtifact dbArtifactMock = mock(DbArtifact.class); when(artifactManagementMock.findFirstLocalArtifactsBySHA1(anyString())).thenReturn(localArtifactMock); - when(controllerManagementMock.getActionForDownloadByTargetAndSoftwareModule(anyObject(), anyObject())) - .thenReturn(actionMock); + when(controllerManagementMock.hasTargetArtifactAssigned(anyObject(), anyObject())).thenReturn(true); when(artifactManagementMock.loadLocalArtifactBinary(localArtifactMock)).thenReturn(dbArtifactMock); when(dbArtifactMock.getArtifactId()).thenReturn("artifactId"); when(dbArtifactMock.getSize()).thenReturn(1L); From b11a0bac5db51e38685d77ca15669e4042069ba4 Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 12:18:59 +0200 Subject: [PATCH 03/12] reduce method cyclomatic complexity by extracting security check into own method Signed-off-by: Michael Hirsch --- .../amqp/AmqpMessageHandlerService.java | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index e05ddf71d..47807cd4d 100644 --- a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -171,21 +171,7 @@ public class AmqpMessageHandlerService extends BaseAmqpService { throw new EntityNotFoundException(); } - // check action for this download purposes, the method will throw an - // EntityNotFoundException in case the controller is not allowed to - // download this file because it's not assigned to an action and not - // assigned to this controller. Otherwise no controllerId is set = - // anonymous download - if (secruityToken.getControllerId() != null) { - LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", - secruityToken.getControllerId(), localArtifact); - if (!controllerManagement.hasTargetArtifactAssigned(secruityToken.getControllerId(), localArtifact)) { - LOG.info("target {} tried to download artifact {} which is not assigned to the target"); - throw new EntityNotFoundException(); - } - LOG.info("download security check for target {} and artifact {} granted", - secruityToken.getControllerId(), localArtifact); - } + checkIfArtifactIsAssignedToTarget(secruityToken, localArtifact); final Artifact artifact = convertDbArtifact(artifactManagement.loadLocalArtifactBinary(localArtifact)); if (artifact == null) { @@ -219,6 +205,25 @@ public class AmqpMessageHandlerService extends BaseAmqpService { return getMessageConverter().toMessage(authentificationResponse, messageProperties); } + private void checkIfArtifactIsAssignedToTarget(final TenantSecurityToken secruityToken, + final LocalArtifact localArtifact) { + // check action for this download purposes, the method will throw an + // EntityNotFoundException in case the controller is not allowed to + // download this file because it's not assigned to an action and not + // assigned to this controller. Otherwise no controllerId is set = + // anonymous download + if (secruityToken.getControllerId() != null) { + LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", + secruityToken.getControllerId(), localArtifact); + if (!controllerManagement.hasTargetArtifactAssigned(secruityToken.getControllerId(), localArtifact)) { + LOG.info("target {} tried to download artifact {} which is not assigned to the target"); + throw new EntityNotFoundException(); + } + LOG.info("download security check for target {} and artifact {} granted", secruityToken.getControllerId(), + localArtifact); + } + } + private LocalArtifact findLocalArtifactByFileResource(final FileResource fileResource) { if (fileResource.getSha1() != null) { return artifactManagement.findFirstLocalArtifactsBySHA1(fileResource.getSha1()); From d91922dc048cb3119561135ff60f0e9da8f24997 Mon Sep 17 00:00:00 2001 From: SirWayne Date: Tue, 12 Apr 2016 13:37:07 +0200 Subject: [PATCH 04/12] Update the entity before call the super method Signed-off-by: SirWayne --- .../smtable/SoftwareModuleTable.java | 59 ++++++++----------- .../table/AbstractNamedVersionTable.java | 2 +- .../dstable/DistributionSetTable.java | 12 ++-- .../distributions/smtable/SwModuleTable.java | 20 ++++--- 4 files changed, 47 insertions(+), 46 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java index f101c94a7..237634398 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java @@ -9,7 +9,6 @@ package org.eclipse.hawkbit.ui.artifacts.smtable; import java.util.HashMap; -import java.util.List; import java.util.Map; import org.eclipse.hawkbit.repository.SoftwareManagement; @@ -25,7 +24,6 @@ import org.eclipse.hawkbit.ui.utils.HawkbitCommonUtil; import org.eclipse.hawkbit.ui.utils.SPUIComponetIdProvider; import org.eclipse.hawkbit.ui.utils.SPUIDefinitions; import org.eclipse.hawkbit.ui.utils.SPUILabelDefinitions; -import org.eclipse.hawkbit.ui.utils.TableColumn; import org.springframework.beans.factory.annotation.Autowired; import org.vaadin.addons.lazyquerycontainer.BeanQueryFactory; import org.vaadin.addons.lazyquerycontainer.LazyQueryContainer; @@ -44,6 +42,8 @@ import com.vaadin.ui.UI; /** * Header of Software module table. + * + * * */ @SpringComponent @@ -129,22 +129,6 @@ public class SoftwareModuleTable extends AbstractNamedVersionTable getTableVisibleColumns() { - final List columnList = super.getTableVisibleColumns(); - if (!isMaximized()) { - return columnList; - } - columnList.add(new TableColumn(SPUILabelDefinitions.VAR_VENDOR, i18n.get("header.vendor"), 0.1F)); - return columnList; + protected void updateEntity(final SoftwareModule baseEntity, final Item item) { + final String swNameVersion = HawkbitCommonUtil.concatStrings(":", baseEntity.getName(), + baseEntity.getVersion()); + item.getItemProperty(SPUILabelDefinitions.NAME_VERSION).setValue(swNameVersion); + item.getItemProperty("swId").setValue(baseEntity.getId()); + item.getItemProperty(SPUILabelDefinitions.VAR_VENDOR).setValue(baseEntity.getVendor()); + super.updateEntity(baseEntity, item); } @Override @@ -203,6 +180,22 @@ public class SoftwareModuleTable extends AbstractNamedVersionTable Date: Tue, 12 Apr 2016 13:42:57 +0200 Subject: [PATCH 05/12] Update the entity before call the super method Signed-off-by: SirWayne --- .../smtable/SoftwareModuleTable.java | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java index 237634398..478438891 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtable/SoftwareModuleTable.java @@ -9,6 +9,7 @@ package org.eclipse.hawkbit.ui.artifacts.smtable; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.eclipse.hawkbit.repository.SoftwareManagement; @@ -24,6 +25,7 @@ import org.eclipse.hawkbit.ui.utils.HawkbitCommonUtil; import org.eclipse.hawkbit.ui.utils.SPUIComponetIdProvider; import org.eclipse.hawkbit.ui.utils.SPUIDefinitions; import org.eclipse.hawkbit.ui.utils.SPUILabelDefinitions; +import org.eclipse.hawkbit.ui.utils.TableColumn; import org.springframework.beans.factory.annotation.Autowired; import org.vaadin.addons.lazyquerycontainer.BeanQueryFactory; import org.vaadin.addons.lazyquerycontainer.LazyQueryContainer; @@ -42,8 +44,6 @@ import com.vaadin.ui.UI; /** * Header of Software module table. - * - * * */ @SpringComponent @@ -129,6 +129,22 @@ public class SoftwareModuleTable extends AbstractNamedVersionTable getTableVisibleColumns() { + final List columnList = super.getTableVisibleColumns(); + if (!isMaximized()) { + return columnList; + } + columnList.add(new TableColumn(SPUILabelDefinitions.VAR_VENDOR, i18n.get("header.vendor"), 0.1F)); + return columnList; + } + @Override protected DropHandler getTableDropHandler() { return new DropHandler() { @@ -180,22 +207,6 @@ public class SoftwareModuleTable extends AbstractNamedVersionTable Date: Tue, 12 Apr 2016 14:35:59 +0200 Subject: [PATCH 06/12] source code improvements Signed-off-by: Michael Hirsch --- .../amqp/AmqpMessageHandlerService.java | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 47807cd4d..994ef244a 100644 --- a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -205,23 +205,32 @@ public class AmqpMessageHandlerService extends BaseAmqpService { return getMessageConverter().toMessage(authentificationResponse, messageProperties); } + /** + * check action for this download purposes, the method will throw an + * EntityNotFoundException in case the controller is not allowed to download + * this file because it's not assigned to an action and not assigned to this + * controller. Otherwise no controllerId is set = anonymous download + * + * @param secruityToken + * the security token which holds the target ID to check on + * @param localArtifact + * the local artifact to verify if the given target is allowed to + * download this artifact + */ private void checkIfArtifactIsAssignedToTarget(final TenantSecurityToken secruityToken, final LocalArtifact localArtifact) { - // check action for this download purposes, the method will throw an - // EntityNotFoundException in case the controller is not allowed to - // download this file because it's not assigned to an action and not - // assigned to this controller. Otherwise no controllerId is set = - // anonymous download - if (secruityToken.getControllerId() != null) { - LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", - secruityToken.getControllerId(), localArtifact); - if (!controllerManagement.hasTargetArtifactAssigned(secruityToken.getControllerId(), localArtifact)) { - LOG.info("target {} tried to download artifact {} which is not assigned to the target"); - throw new EntityNotFoundException(); - } - LOG.info("download security check for target {} and artifact {} granted", secruityToken.getControllerId(), - localArtifact); + final String controllerId = secruityToken.getControllerId(); + if (controllerId == null) { + LOG.info("anonymous download no authentication check for artifact {}", localArtifact); + return; } + LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", + controllerId, localArtifact); + if (!controllerManagement.hasTargetArtifactAssigned(controllerId, localArtifact)) { + LOG.info("target {} tried to download artifact {} which is not assigned to the target"); + throw new EntityNotFoundException(); + } + LOG.info("download security check for target {} and artifact {} granted", controllerId, localArtifact); } private LocalArtifact findLocalArtifactByFileResource(final FileResource fileResource) { From cc8c6743415be5b01cd80cffd8da669611acd547 Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 15:47:21 +0200 Subject: [PATCH 07/12] correct usage of logging, missing parameters Signed-off-by: Michael Hirsch --- .../org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 994ef244a..2ee0a0f45 100644 --- a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -227,7 +227,8 @@ public class AmqpMessageHandlerService extends BaseAmqpService { LOG.debug("no anonymous download request, doing authentication check for target {} and artifact {}", controllerId, localArtifact); if (!controllerManagement.hasTargetArtifactAssigned(controllerId, localArtifact)) { - LOG.info("target {} tried to download artifact {} which is not assigned to the target"); + LOG.info("target {} tried to download artifact {} which is not assigned to the target", controllerId, + localArtifact); throw new EntityNotFoundException(); } LOG.info("download security check for target {} and artifact {} granted", controllerId, localArtifact); From 30e8230b8aaa55e1e3834c1d9eeed019454d5f99 Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 15:48:19 +0200 Subject: [PATCH 08/12] correct usage of logging, missing parameters Signed-off-by: Michael Hirsch --- .../org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 2ee0a0f45..447a8ffca 100644 --- a/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -166,7 +166,7 @@ public class AmqpMessageHandlerService extends BaseAmqpService { final LocalArtifact localArtifact = findLocalArtifactByFileResource(fileResource); if (localArtifact == null) { - LOG.info("target {} requested file resource which does not exists to download", + LOG.info("target {} requested file resource {} which does not exists to download", secruityToken.getControllerId(), fileResource); throw new EntityNotFoundException(); } From d903be57b7a9913c29db8e8729efa88f5fab4666 Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 15:50:48 +0200 Subject: [PATCH 09/12] use correct mock parameters to ensure test Signed-off-by: Michael Hirsch --- .../eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java b/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java index 0faa0a8a6..19a7be3bc 100644 --- a/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java +++ b/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java @@ -315,7 +315,8 @@ public class AmqpMessageHandlerServiceTest { final LocalArtifact localArtifactMock = mock(LocalArtifact.class); final DbArtifact dbArtifactMock = mock(DbArtifact.class); when(artifactManagementMock.findFirstLocalArtifactsBySHA1(anyString())).thenReturn(localArtifactMock); - when(controllerManagementMock.hasTargetArtifactAssigned(anyObject(), anyObject())).thenReturn(true); + when(controllerManagementMock.hasTargetArtifactAssigned(securityToken.getControllerId(), localArtifactMock)) + .thenReturn(true); when(artifactManagementMock.loadLocalArtifactBinary(localArtifactMock)).thenReturn(dbArtifactMock); when(dbArtifactMock.getArtifactId()).thenReturn("artifactId"); when(dbArtifactMock.getSize()).thenReturn(1L); From d704ef99fd1a3ee0f8152f933ee7c9f8ed610901 Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Tue, 12 Apr 2016 16:22:43 +0200 Subject: [PATCH 10/12] source code hygiene, better method naming and variable naming Signed-off-by: Michael Hirsch --- .../eclipse/hawkbit/repository/ControllerManagement.java | 2 +- .../repository/specifications/ActionSpecifications.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java index 9510ae899..dc097a7e3 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java @@ -190,7 +190,7 @@ public class ControllerManagement { if (target == null) { return false; } - return actionRepository.count(ActionSpecifications.hasTargetArtifactAssigned(target, localArtifact)) > 0; + return actionRepository.count(ActionSpecifications.hasTargetAssignedArtifact(target, localArtifact)) > 0; } /** diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java index ef8c97e22..1ac3afb67 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/specifications/ActionSpecifications.java @@ -47,14 +47,14 @@ public class ActionSpecifications { * assigned * @return a specification to use with spring JPA */ - public static Specification hasTargetArtifactAssigned(final Target target, + public static Specification hasTargetAssignedArtifact(final Target target, final LocalArtifact localArtifact) { - return (actionRoot, query, cb) -> { + return (actionRoot, query, criteriaBuilder) -> { final Join dsJoin = actionRoot.join(Action_.distributionSet); final SetJoin modulesJoin = dsJoin.join(DistributionSet_.modules); final ListJoin artifactsJoin = modulesJoin.join(SoftwareModule_.artifacts); - return cb.and(cb.equal(artifactsJoin.get(LocalArtifact_.id), localArtifact.getId()), - cb.equal(actionRoot.get(Action_.target), target)); + return criteriaBuilder.and(criteriaBuilder.equal(artifactsJoin.get(LocalArtifact_.id), localArtifact.getId()), + criteriaBuilder.equal(actionRoot.get(Action_.target), target)); }; } } From f70120d5461959e0fa7abc0bb4382e17885341f1 Mon Sep 17 00:00:00 2001 From: SirWayne Date: Wed, 13 Apr 2016 15:36:14 +0200 Subject: [PATCH 11/12] Set the id after the update Signed-off-by: SirWayne --- .../hawkbit/ui/distributions/dstable/DistributionSetTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java index 4e6be369d..998cd1de2 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java @@ -471,9 +471,9 @@ public class DistributionSetTable extends AbstractNamedVersionTable Date: Wed, 13 Apr 2016 15:47:08 +0200 Subject: [PATCH 12/12] Version should be set after id Signed-off-by: SirWayne --- .../hawkbit/ui/common/table/AbstractNamedVersionTable.java | 2 +- .../hawkbit/ui/distributions/dstable/DistributionSetTable.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/table/AbstractNamedVersionTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/table/AbstractNamedVersionTable.java index b14c69a51..3e215f77c 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/table/AbstractNamedVersionTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/table/AbstractNamedVersionTable.java @@ -40,8 +40,8 @@ public abstract class AbstractNamedVersionTable