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/model/Target.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/model/Target.java index 6de84ccaa..08e4fcecb 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/model/Target.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/model/Target.java @@ -32,6 +32,8 @@ import javax.persistence.PrimaryKeyJoinColumn; import javax.persistence.Table; import javax.persistence.Transient; import javax.persistence.UniqueConstraint; +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Size; import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.repository.model.helper.SecurityChecker; @@ -74,6 +76,8 @@ public class Target extends NamedEntity implements Persistable { private static final long serialVersionUID = 1L; @Column(name = "controller_id", length = 64) + @Size(min = 1) + @NotNull private String controllerId; @Transient 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 555d654c5..3a70441ca 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 @@ -25,6 +25,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; @@ -64,6 +65,24 @@ public class TargetManagementTest extends AbstractIntegrationTest { } } + @Test + @Description("Verify that a target with empty controller id cannot be created") + public void createTargetWithNoControllerId() { + try { + targetManagement.createTarget(new Target("")); + fail("target with empty controller id should not be created"); + } catch (final ConstraintViolationException e) { + // ok + } + + try { + targetManagement.createTarget(new Target(null)); + 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() {