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 844bbd097..e4a1e5e6b 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 @@ -15,7 +15,6 @@ import java.util.Map; import javax.persistence.EntityManager; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; -import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import javax.validation.constraints.NotNull; @@ -212,14 +211,9 @@ public class ControllerManagement implements EnvironmentAware { @Modifying @Transactional @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER) - public Target findOrRegisterTargetIfItDoesNotexist(@NotNull final String targetid, final URI address) { - final Specification spec = new Specification() { - @Override - public Predicate toPredicate(final Root targetRoot, final CriteriaQuery query, - final CriteriaBuilder cb) { - return cb.equal(targetRoot.get(Target_.controllerId), targetid); - } - }; + public Target findOrRegisterTargetIfItDoesNotexist(@NotEmpty final String targetid, final URI address) { + final Specification spec = (targetRoot, query, cb) -> cb.equal(targetRoot.get(Target_.controllerId), + targetid); Target target = targetRepository.findOne(spec); @@ -229,9 +223,9 @@ public class ControllerManagement implements EnvironmentAware { target.setName(targetid); return targetManagement.createTarget(target, TargetUpdateStatus.REGISTERED, System.currentTimeMillis(), address); - } else { - return updateLastTargetQuery(target.getTargetInfo(), address).getTarget(); } + + return updateLastTargetQuery(target.getTargetInfo(), address).getTarget(); } /** diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java index e790d0a5e..fb824dfae 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java @@ -27,8 +27,10 @@ import javax.persistence.criteria.JoinType; import javax.persistence.criteria.Order; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; +import javax.validation.ConstraintViolationException; import javax.validation.constraints.NotNull; +import org.apache.commons.lang3.StringUtils; import org.eclipse.hawkbit.Constants; import org.eclipse.hawkbit.eventbus.event.TargetTagAssigmentResultEvent; import org.eclipse.hawkbit.executor.AfterTransactionCommitExecutor; @@ -931,6 +933,11 @@ public class TargetManagement { public Target createTarget(@NotNull final Target target, @NotNull final TargetUpdateStatus status, final Long lastTargetQuery, final URI address) { + if (StringUtils.isEmpty(target.getControllerId())) { + throw new ConstraintViolationException("Empty string for controller id not allowed", + Collections.emptySet()); + } + if (targetRepository.findByControllerId(target.getControllerId()) != null) { throw new EntityAlreadyExistsException(target.getControllerId()); } diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java index 8289b8173..41a7c7848 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java @@ -9,10 +9,13 @@ package org.eclipse.hawkbit.repository; import static org.fest.assertions.api.Assertions.assertThat; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.List; +import javax.validation.ConstraintViolationException; + import org.apache.commons.lang3.RandomStringUtils; import org.eclipse.hawkbit.AbstractIntegrationTest; import org.eclipse.hawkbit.TestDataUtil; @@ -71,6 +74,24 @@ public class ControllerManagementTest extends AbstractIntegrationTest { .getNumberOfElements()).isEqualTo(3); } + @Test + @Description("Register a controller which not exist") + public void testfindOrRegisterTargetIfItDoesNotexist() { + final Target target = controllerManagament.findOrRegisterTargetIfItDoesNotexist("AA", null); + assertThat(target).as("target should not be null").isNotNull(); + + final Target sameTarget = controllerManagament.findOrRegisterTargetIfItDoesNotexist("AA", null); + assertThat(target).as("Target should be the equals").isEqualTo(sameTarget); + assertThat(targetRepository.count()).as("Only 1 target should be registred").isEqualTo(1L); + + try { + controllerManagament.findOrRegisterTargetIfItDoesNotexist("", null); + fail("target with empty controller id should not be registred"); + } catch (final ConstraintViolationException e) { + // ok + } + } + @Test @Description("Controller trys to finish an update process after it has been finished by an error action status.") public void tryToFinishUpdateProcessMoreThenOnce() { diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/TargetManagementTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/TargetManagementTest.java index a7b8e673f..72f1951c6 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/TargetManagementTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/TargetManagementTest.java @@ -26,6 +26,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.persistence.Query; +import javax.validation.ConstraintViolationException; import org.eclipse.hawkbit.AbstractIntegrationTest; import org.eclipse.hawkbit.TestDataUtil; @@ -65,6 +66,17 @@ public class TargetManagementTest extends AbstractIntegrationTest { } } + @Test + @Description("Verify that a target with empty controller id cannot be created") + public void createTarget() { + try { + targetManagement.createTarget(new Target("")); + fail("target with empty controller id should not be created"); + } catch (final ConstraintViolationException e) { + // ok + } + } + @Test @Description("Ensures that targets can assigned and unassigned to a target tag. Not exists target will be ignored for the assignment.") public void assignAndUnassignTargetsToTag() {