Fix exception handling on repository (#546)

* Fix constraint violation handling (400 instead of 500).

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Dont map constraintvioalation

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Added test in target repo.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Extended dialect handler.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Fix broken constraint handling. Added target tests and docs.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Further restricted aspect.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Add macro test.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Reduce duplicate code.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* No need to open a new transaction here.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Remove comment.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Remove flush from assign DS.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Remove commented line

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Fix exception handling for non-SQL cause.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Remove deprecated comment.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Documentation

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* More tests and documentation.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Private final.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Fix loop skip.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Fix test description.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Completed test coverage.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>
This commit is contained in:
Kai Zimmermann
2017-06-20 17:20:13 +02:00
committed by GitHub
parent 3b5f12b7a4
commit 2383aff5bf
25 changed files with 613 additions and 227 deletions

View File

@@ -9,14 +9,21 @@
package org.eclipse.hawkbit.repository.jpa;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Arrays;
import javax.validation.ConstraintViolationException;
import org.apache.commons.lang3.RandomStringUtils;
import org.eclipse.hawkbit.repository.DistributionSetManagement;
import org.eclipse.hawkbit.repository.event.remote.entity.DistributionSetCreatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.DistributionSetUpdatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.SoftwareModuleCreatedEvent;
import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException;
import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType;
import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.DistributionSetType;
import org.eclipse.hawkbit.repository.test.matcher.Expect;
import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
@@ -26,6 +33,7 @@ import com.google.common.collect.Sets;
import ru.yandex.qatools.allure.annotations.Description;
import ru.yandex.qatools.allure.annotations.Features;
import ru.yandex.qatools.allure.annotations.Step;
import ru.yandex.qatools.allure.annotations.Stories;
/**
@@ -75,6 +83,92 @@ public class DistributionSetTypeManagementTest extends AbstractJpaIntegrationTes
"DistributionSet");
}
@Test
@Description("Verify that a DistributionSet with invalid properties cannot be created or updated")
@ExpectEvents({ @Expect(type = DistributionSetCreatedEvent.class, count = 1),
@Expect(type = SoftwareModuleCreatedEvent.class, count = 3),
@Expect(type = DistributionSetUpdatedEvent.class, count = 0) })
public void createAndUpdateDistributionSetWithInvalidFields() {
final DistributionSet set = testdataFactory.createDistributionSet();
createAndUpdateDistributionSetWithInvalidDescription(set);
createAndUpdateDistributionSetWithInvalidName(set);
createAndUpdateDistributionSetWithInvalidVersion(set);
}
@Step
private void createAndUpdateDistributionSetWithInvalidDescription(final DistributionSet set) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.createDistributionSet(entityFactory.distributionSet()
.create().name("a").version("a").description(RandomStringUtils.randomAlphanumeric(513))))
.as("set with too long description should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.updateDistributionSet(entityFactory.distributionSet()
.update(set.getId()).description(RandomStringUtils.randomAlphanumeric(513))))
.as("set with too long description should not be updated");
}
@Step
private void createAndUpdateDistributionSetWithInvalidName(final DistributionSet set) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.createDistributionSet(entityFactory.distributionSet()
.create().version("a").name(RandomStringUtils.randomAlphanumeric(65))))
.as("set with too long name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.createDistributionSet(entityFactory.distributionSet().create().version("a").name("")))
.as("set with too short name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.createDistributionSet(entityFactory.distributionSet().create().version("a").name(null)))
.as("set with null name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.updateDistributionSet(entityFactory.distributionSet()
.update(set.getId()).name(RandomStringUtils.randomAlphanumeric(65))))
.as("set with too long name should not be updated");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.updateDistributionSet(entityFactory.distributionSet().update(set.getId()).name("")))
.as("set with too short name should not be updated");
}
@Step
private void createAndUpdateDistributionSetWithInvalidVersion(final DistributionSet set) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.createDistributionSet(entityFactory.distributionSet()
.create().name("a").version(RandomStringUtils.randomAlphanumeric(65))))
.as("set with too long name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.createDistributionSet(entityFactory.distributionSet().create().name("a").version("")))
.as("set with too short name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.createDistributionSet(entityFactory.distributionSet().create().name("a").version(null)))
.as("set with null name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.updateDistributionSet(entityFactory.distributionSet()
.update(set.getId()).version(RandomStringUtils.randomAlphanumeric(65))))
.as("set with too long name should not be updated");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement
.updateDistributionSet(entityFactory.distributionSet().update(set.getId()).version("")))
.as("set with too short name should not be updated");
}
@Test
@Description("Tests the successfull module update of unused distribution set type which is in fact allowed.")
public void updateUnassignedDistributionSetTypeModules() {

View File

@@ -0,0 +1,88 @@
/**
* 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.jpa;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.sql.SQLException;
import javax.persistence.OptimisticLockException;
import javax.persistence.PersistenceException;
import org.junit.Test;
import org.springframework.dao.ConcurrencyFailureException;
import org.springframework.dao.UncategorizedDataAccessException;
import ru.yandex.qatools.allure.annotations.Description;
import ru.yandex.qatools.allure.annotations.Features;
import ru.yandex.qatools.allure.annotations.Stories;
/**
* Mapping tests for {@link HawkBitEclipseLinkJpaDialect}.
*
*/
@Features("Unit Tests - Repository")
@Stories("Exception handling")
public class HawkBitEclipseLinkJpaDialectTest {
private final HawkBitEclipseLinkJpaDialect hawkBitEclipseLinkJpaDialectUnderTest = new HawkBitEclipseLinkJpaDialect();
@Test
@Description("Use Case: PersistenceException that can be mapped by EclipseLinkJpaDialect into corresponding DataAccessException.")
public void jpaOptimisticLockExceptionIsConcurrencyFailureException() {
assertThat(
hawkBitEclipseLinkJpaDialectUnderTest.translateExceptionIfPossible(mock(OptimisticLockException.class)))
.isInstanceOf(ConcurrencyFailureException.class);
}
@Test
@Description("Use Case: PersistenceException that could not be mapped by EclipseLinkJpaDialect directly but "
+ "instead is wrapped into JpaSystemException. Cause of PersistenceException is an SQLException.")
public void jpaSystemExceptionWithSqlDeadLockExceptionIsConcurrencyFailureException() {
final PersistenceException persEception = mock(PersistenceException.class);
when(persEception.getCause()).thenReturn(new SQLException("simulated transaction ER_LOCK_DEADLOCK", "40001"));
assertThat(hawkBitEclipseLinkJpaDialectUnderTest.translateExceptionIfPossible(persEception))
.isInstanceOf(ConcurrencyFailureException.class);
}
@Test
@Description("Use Case: PersistenceException that could not be mapped by EclipseLinkJpaDialect directly but instead is wrapped"
+ " into JpaSystemException. Cause of PersistenceException is not an SQLException.")
public void jpaSystemExceptionWithNumberFormatExceptionIsNull() {
final PersistenceException persEception = mock(PersistenceException.class);
when(persEception.getCause()).thenReturn(new NumberFormatException());
assertThat(hawkBitEclipseLinkJpaDialectUnderTest.translateExceptionIfPossible(persEception))
.isInstanceOf(UncategorizedDataAccessException.class);
}
@Test
@Description("Use Case: RuntimeException that could not be mapped by EclipseLinkJpaDialect directly. Cause of "
+ "RuntimeException is an SQLException.")
public void runtimeExceptionWithSqlDeadLockExceptionIsConcurrencyFailureException() {
final RuntimeException persEception = mock(RuntimeException.class);
when(persEception.getCause()).thenReturn(new SQLException("simulated transaction ER_LOCK_DEADLOCK", "40001"));
assertThat(hawkBitEclipseLinkJpaDialectUnderTest.translateExceptionIfPossible(persEception))
.isInstanceOf(ConcurrencyFailureException.class);
}
@Test
@Description("Use Case: RuntimeException that could not be mapped by EclipseLinkJpaDialect directly. Cause of "
+ "RuntimeException is not an SQLException.")
public void runtimeExceptionWithNumberFormatExceptionIsNull() {
final RuntimeException persEception = mock(RuntimeException.class);
when(persEception.getCause()).thenReturn(new NumberFormatException());
assertThat(hawkBitEclipseLinkJpaDialectUnderTest.translateExceptionIfPossible(persEception)).isNull();
}
}

View File

@@ -24,6 +24,7 @@ import java.util.stream.Collectors;
import javax.validation.ConstraintViolationException;
import org.apache.commons.lang3.RandomStringUtils;
import org.eclipse.hawkbit.im.authentication.SpPermission;
import org.eclipse.hawkbit.repository.event.remote.TargetAssignDistributionSetEvent;
import org.eclipse.hawkbit.repository.event.remote.TargetDeletedEvent;
@@ -55,12 +56,15 @@ import com.google.common.collect.Iterables;
import ru.yandex.qatools.allure.annotations.Description;
import ru.yandex.qatools.allure.annotations.Features;
import ru.yandex.qatools.allure.annotations.Step;
import ru.yandex.qatools.allure.annotations.Stories;
@Features("Component Tests - Repository")
@Stories("Target Management")
public class TargetManagementTest extends AbstractJpaIntegrationTest {
private static final String WHITESPACE_ERROR = "target with whitespaces in controller id should not be created";
@Test
@Description("Verifies that management get access react as specfied on calls for non existing entities by means "
+ "of Optional not present.")
@@ -174,25 +178,6 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest {
}
}
@Test
@Description("Verify that a target with empty controller id cannot be created")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) })
public void createTargetWithNoControllerId() {
try {
targetManagement.createTarget(entityFactory.target().create().controllerId(""));
fail("target with empty controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
try {
targetManagement.createTarget(entityFactory.target().create().controllerId(null));
fail("target with empty controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
}
@Test
@Description("Verify that a target with same controller ID than another device cannot be created.")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1) })
@@ -204,50 +189,125 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest {
}
@Test
@Description("Verify that a target with whitespaces in controller id cannot be created")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) })
public void createTargetWithWhitespaces() {
try {
targetManagement.createTarget(entityFactory.target().create().controllerId(" "));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
@Description("Verify that a target with with invalid properties cannot be created or updated")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = TargetUpdatedEvent.class, count = 0) })
public void createAndUpdateTargetWithInvalidFields() {
final Target target = testdataFactory.createTarget();
try {
targetManagement.createTarget(entityFactory.target().create().controllerId(" a"));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
createTargetWithInvalidControllerId();
createAndUpdateTargetWithInvalidDescription(target);
createAndUpdateTargetWithInvalidName(target);
createAndUpdateTargetWithInvalidSecurityToken(target);
createAndUpdateTargetWithInvalidAddress(target);
}
try {
targetManagement.createTarget(entityFactory.target().create().controllerId("a "));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
@Step
private void createAndUpdateTargetWithInvalidDescription(final Target target) {
try {
targetManagement.createTarget(entityFactory.target().create().controllerId("a b"));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a")
.description(RandomStringUtils.randomAlphanumeric(513))))
.as("target with too long description should not be created");
try {
targetManagement.createTarget(entityFactory.target().create().controllerId(" "));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.updateTarget(entityFactory.target().update(target.getControllerId())
.description(RandomStringUtils.randomAlphanumeric(513))))
.as("target with too long description should not be updated");
try {
targetManagement.createTarget(entityFactory.target().create().controllerId("aaa bbb"));
fail("target with whitespaces in controller id should not be created");
} catch (final ConstraintViolationException e) {
// ok
}
}
@Step
private void createAndUpdateTargetWithInvalidName(final Target target) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a")
.name(RandomStringUtils.randomAlphanumeric(65))))
.as("target with too long name should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.updateTarget(entityFactory.target().update(target.getControllerId())
.name(RandomStringUtils.randomAlphanumeric(65))))
.as("target with too long name should not be updated");
assertThatExceptionOfType(ConstraintViolationException.class).isThrownBy(
() -> targetManagement.updateTarget(entityFactory.target().update(target.getControllerId()).name("")))
.as("target with too short name should not be updated");
}
@Step
private void createAndUpdateTargetWithInvalidSecurityToken(final Target target) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a")
.securityToken(RandomStringUtils.randomAlphanumeric(129))))
.as("target with too long token should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.updateTarget(entityFactory.target().update(target.getControllerId())
.securityToken(RandomStringUtils.randomAlphanumeric(129))))
.as("target with too long token should not be updated");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement
.updateTarget(entityFactory.target().update(target.getControllerId()).securityToken("")))
.as("target with too short token should not be updated");
}
@Step
private void createAndUpdateTargetWithInvalidAddress(final Target target) {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a")
.address(RandomStringUtils.randomAlphanumeric(513))))
.as("target with too long address should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.updateTarget(entityFactory.target().update(target.getControllerId())
.address(RandomStringUtils.randomAlphanumeric(513))))
.as("target with too long address should not be updated");
}
@Step
private void createTargetWithInvalidControllerId() {
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("")))
.as("target with empty controller id should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId(null)))
.as("target with null controller id should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(
entityFactory.target().create().controllerId(RandomStringUtils.randomAlphanumeric(65))))
.as("target with too long controller id should not be created");
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId(" ")))
.as(WHITESPACE_ERROR);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId(" a")))
.as(WHITESPACE_ERROR);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a ")))
.as(WHITESPACE_ERROR);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId("a b")))
.as(WHITESPACE_ERROR);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.createTarget(entityFactory.target().create().controllerId(" ")))
.as(WHITESPACE_ERROR);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(
() -> targetManagement.createTarget(entityFactory.target().create().controllerId("aaa bbb")))
.as(WHITESPACE_ERROR);
}

View File

@@ -177,6 +177,8 @@ public class RSQLTargetFieldTest extends AbstractJpaIntegrationTest {
assertRSQLQuery(TargetFields.LASTCONTROLLERREQUESTAT.name() + "=lt=" + target2.getLastTargetQuery(), 1);
assertRSQLQuery(TargetFields.LASTCONTROLLERREQUESTAT.name() + "=gt=" + target.getLastTargetQuery(), 1);
assertRSQLQuery(TargetFields.LASTCONTROLLERREQUESTAT.name() + "=gt=" + target2.getLastTargetQuery(), 0);
assertRSQLQuery(TargetFields.LASTCONTROLLERREQUESTAT.name() + "=le=${NOW_TS}", 2);
assertRSQLQuery(TargetFields.LASTCONTROLLERREQUESTAT.name() + "=gt=${OVERDUE_TS}", 2);
}
private void assertRSQLQuery(final String rsqlParam, final long expcetedTargets) {