lingwei.kong/hawkbit
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Fix for endless loop when an exception of type EntityAlreadyExistsException is thrown in the context of the retryable findOrRegisterTargetIfItDoesNotExist method (#828)

Browse Source 
* fixed typo in method name

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

* Added recover method to handle EntityAlreadyExistsException

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

* Added tests

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

* Apply suggestions from code review

Use thenThrow(Exception.class) instead of (new Exception())

Co-Authored-By: a-sayyed <ahmed.sayed@bosch-si.com>

* Apply suggestions from code review

use final modifier

Co-Authored-By: a-sayyed <ahmed.sayed@bosch-si.com>

* Adapted review findings

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

* Adapted review findings

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

* added logs for EntityAlreadyExistsException case

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>
This commit is contained in:
Ahmed Sayed
2019-04-30 12:47:06 +02:00
committed by Dominic Schabel
parent ed95ae6398
commit 9884452ad4
10 changed files with 137 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java
View File

@@ -188,7 +188,7 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
}
final URI amqpUri = IpUtil.createAmqpUri(virtualHost, replyTo);
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(thingId, amqpUri);
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(thingId, amqpUri);
LOG.debug("Target {} reported online state.", thingId);
lookIfUpdateAvailable(target);

2
hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerServiceTest.java
View File

@@ -168,7 +168,7 @@ public class AmqpMessageHandlerServiceTest {
final ArgumentCaptor<String> targetIdCaptor = ArgumentCaptor.forClass(String.class);
final ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
when(controllerManagementMock.findOrRegisterTargetIfItDoesNotexist(targetIdCaptor.capture(),
when(controllerManagementMock.findOrRegisterTargetIfItDoesNotExist(targetIdCaptor.capture(),
uriCaptor.capture())).thenReturn(targetMock);
when(controllerManagementMock.findOldestActiveActionByTarget(any())).thenReturn(Optional.empty());

2
hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java
View File

@@ -192,7 +192,7 @@ public interface ControllerManagement {
* @return target reference
*/
@PreAuthorize(SpringEvalExpressions.IS_CONTROLLER)
Target findOrRegisterTargetIfItDoesNotexist(@NotEmpty String controllerId, @NotNull URI address);
Target findOrRegisterTargetIfItDoesNotExist(@NotEmpty String controllerId, @NotNull URI address);
/**
* Retrieves last {@link Action} for a download of an artifact of given

29
hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java
View File

@@ -50,6 +50,7 @@ import org.eclipse.hawkbit.repository.event.remote.TargetAttributesRequestedEven
import org.eclipse.hawkbit.repository.event.remote.TargetPollEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.CancelTargetAssignmentEvent;
import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException;
import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException;
import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException;
import org.eclipse.hawkbit.repository.jpa.builder.JpaActionStatusCreate;
@@ -364,16 +365,19 @@ public class JpaControllerManagement implements ControllerManagement {
}
@Override
@Transactional
@Retryable(include = {
ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Target findOrRegisterTargetIfItDoesNotexist(final String controllerId, final URI address) {
@Transactional(isolation = Isolation.READ_COMMITTED)
@Retryable(include = ConcurrencyFailureException.class, exclude = EntityAlreadyExistsException.class,
maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Target findOrRegisterTargetIfItDoesNotExist(final String controllerId, final URI address) {
final Specification<JpaTarget> spec = (targetRoot, query, cb) -> cb
.equal(targetRoot.get(JpaTarget_.controllerId), controllerId);
final Optional<JpaTarget> target = targetRepository.findOne(spec);
return targetRepository.findOne(spec).map(target -> updateTargetStatus(target, address))
.orElseGet(() -> createTarget(controllerId, address));
}
if (!target.isPresent()) {
private Target createTarget(final String controllerId, final URI address) {
try {
final Target result = targetRepository.save((JpaTarget) entityFactory.target().create()
.controllerId(controllerId).description("Plug and Play target: " + controllerId).name(controllerId)
.status(TargetUpdateStatus.REGISTERED).lastTargetQuery(System.currentTimeMillis())
@@ -382,9 +386,11 @@ public class JpaControllerManagement implements ControllerManagement {
afterCommit.afterCommit(() -> eventPublisher.publishEvent(new TargetPollEvent(result, bus.getId())));
return result;
} catch (final EntityAlreadyExistsException e){
LOG.warn("Caught an EntityAlreadyExistsException while creating non existing target " +
"[controllerId:{}, address:{}, tenant: {}]", controllerId, address, tenantAware.getCurrentTenant());
throw e;
}
return updateTargetStatus(target.get(), address);
}
/**
@@ -472,7 +478,7 @@ public class JpaControllerManagement implements ControllerManagement {
/**
* Stores target directly to DB in case either {@link Target#getAddress()}
* or {@link Target#getUpdateStatus()} changes or the buffer queue is full.
*
*
*/
private Target updateTargetStatus(final JpaTarget toUpdate, final URI address) {
boolean storeEager = isStoreEager(toUpdate, address);
@@ -1029,4 +1035,9 @@ public class JpaControllerManagement implements ControllerManagement {
afterCommit.afterCommit(
() -> eventPublisher.publishEvent(new CancelTargetAssignmentEvent(target, actionId, bus.getId())));
}
// for testing
void setTargetRepository(final TargetRepository targetRepositorySpy) {
this.targetRepository = targetRepositorySpy;
}
}

110
hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java
View File

@@ -11,9 +11,14 @@ 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.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.CONTROLLER_ROLE_ANONYMOUS;
import static org.eclipse.hawkbit.repository.jpa.configuration.Constants.TX_RT_MAX;
import static org.eclipse.hawkbit.repository.model.Action.ActionType.DOWNLOAD_ONLY;
import static org.eclipse.hawkbit.repository.test.util.TestdataFactory.DEFAULT_CONTROLLER_ID;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.ByteArrayInputStream;
import java.net.URISyntaxException;
@@ -43,6 +48,7 @@ import org.eclipse.hawkbit.repository.event.remote.entity.SoftwareModuleUpdatedE
import org.eclipse.hawkbit.repository.event.remote.entity.TargetCreatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent;
import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException;
import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException;
import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException;
import org.eclipse.hawkbit.repository.exception.QuotaExceededException;
import org.eclipse.hawkbit.repository.jpa.model.JpaTarget;
@@ -61,7 +67,9 @@ import org.eclipse.hawkbit.repository.test.matcher.Expect;
import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
import org.eclipse.hawkbit.repository.test.util.WithSpringAuthorityRule;
import org.junit.Test;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.ConcurrencyFailureException;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -497,19 +505,111 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest {
@Description("Register a controller which does not exist")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = TargetPollEvent.class, count = 2) })
public void findOrRegisterTargetIfItDoesNotexist() {
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist("AA", LOCALHOST);
public void findOrRegisterTargetIfItDoesNotExist() {
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
assertThat(target).as("target should not be null").isNotNull();
final Target sameTarget = controllerManagement.findOrRegisterTargetIfItDoesNotexist("AA", LOCALHOST);
final Target sameTarget = controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
assertThat(target.getId()).as("Target should be the equals").isEqualTo(sameTarget.getId());
assertThat(targetRepository.count()).as("Only 1 target should be registred").isEqualTo(1L);
assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> controllerManagement.findOrRegisterTargetIfItDoesNotexist("", LOCALHOST))
.isThrownBy(() -> controllerManagement.findOrRegisterTargetIfItDoesNotExist("", LOCALHOST))
.as("register target with empty controllerId should fail");
}
@Test
@Description("Register a controller which does not exist, when a ConcurrencyFailureException is raised, the " +
"exception is rethrown after max retries")
public void findOrRegisterTargetIfItDoesNotExistThrowsExceptionAfterMaxRetries() {
final TargetRepository mockTargetRepository = Mockito.mock(TargetRepository.class);
when(mockTargetRepository.findOne(any())).thenThrow(ConcurrencyFailureException.class);
((JpaControllerManagement) controllerManagement).setTargetRepository(mockTargetRepository);
try {
controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
fail("Expected an ConcurrencyFailureException to be thrown!");
} catch (final ConcurrencyFailureException e) {
verify(mockTargetRepository, times(TX_RT_MAX)).findOne(any());
} finally {
// revert
((JpaControllerManagement) controllerManagement).setTargetRepository(targetRepository);
}
}
@Test
@Description("Register a controller which does not exist, when a ConcurrencyFailureException is raised, the " +
"exception is not rethrown when the max retries are not yet reached")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = TargetPollEvent.class, count = 1) })
public void findOrRegisterTargetIfItDoesNotExistDoesNotThrowExceptionBeforeMaxRetries() {
TargetRepository mockTargetRepository = Mockito.mock(TargetRepository.class);
((JpaControllerManagement) controllerManagement).setTargetRepository(mockTargetRepository);
final Target target = testdataFactory.createTarget();
when(mockTargetRepository.findOne(any()))
.thenThrow(ConcurrencyFailureException.class)
.thenThrow(ConcurrencyFailureException.class)
.thenReturn(Optional.of((JpaTarget) target));
when(mockTargetRepository.save(any())).thenReturn(target);
try {
final Target targetFromControllerManagement = controllerManagement
.findOrRegisterTargetIfItDoesNotExist(target.getControllerId(), LOCALHOST);
verify(mockTargetRepository, times(3)).findOne(any());
verify(mockTargetRepository, times(1)).save(any());
assertThat(target).isEqualTo(targetFromControllerManagement);
} finally {
// revert
((JpaControllerManagement) controllerManagement).setTargetRepository(targetRepository);
}
}
@Test
@Description("Register a controller which does not exist, if a EntityAlreadyExistsException is raised, the " +
"exception is rethrown and no further retries will be attempted")
public void findOrRegisterTargetIfItDoesNotExistDoesntRetryWhenEntityAlreadyExistsException() {
TargetRepository mockTargetRepository = Mockito.mock(TargetRepository.class);
((JpaControllerManagement) controllerManagement).setTargetRepository(mockTargetRepository);
when(mockTargetRepository.findOne(any())).thenReturn(Optional.empty());
when(mockTargetRepository.save(any())).thenThrow(EntityAlreadyExistsException.class);
try {
controllerManagement.findOrRegisterTargetIfItDoesNotExist("1234", LOCALHOST);
fail("Expected an EntityAlreadyExistsException to be thrown!");
} catch (EntityAlreadyExistsException e) {
verify(mockTargetRepository, times(1)).findOne(any());
verify(mockTargetRepository, times(1)).save(any());
} finally {
// revert
((JpaControllerManagement) controllerManagement).setTargetRepository(targetRepository);
}
}
@Test
@Description("Retry is aborted when an unchecked exception is thrown and the exception should also be " +
"rethrown")
public void recoverFindOrRegisterTargetIfItDoesNotExistIsNotInvokedForOtherExceptions() {
TargetRepository mockTargetRepository = Mockito.mock(TargetRepository.class);
((JpaControllerManagement) controllerManagement).setTargetRepository(mockTargetRepository);
when(mockTargetRepository.findOne(any())).thenThrow(RuntimeException.class);
try {
controllerManagement.findOrRegisterTargetIfItDoesNotExist("aControllerId", LOCALHOST);
fail("Expected a RuntimeException to be thrown!");
} catch (RuntimeException e){
verify(mockTargetRepository, times(1)).findOne(any());
} finally {
// revert
((JpaControllerManagement) controllerManagement).setTargetRepository(targetRepository);
}
}
@Test
@Description("Verify that targetVisible metadata is returned from repository")
@ExpectEvents({ @Expect(type = DistributionSetCreatedEvent.class, count = 1),
@@ -533,7 +633,7 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest {
@Expect(type = TargetPollEvent.class, count = 0) })
public void targetPollEventNotSendIfDisabled() {
repositoryProperties.setPublishTargetPollEvent(false);
controllerManagement.findOrRegisterTargetIfItDoesNotexist("AA", LOCALHOST);
controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
repositoryProperties.setPublishTargetPollEvent(true);
}

4
hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/LazyControllerManagementTest.java
View File

@@ -41,11 +41,11 @@ public class LazyControllerManagementTest extends AbstractJpaIntegrationTest {
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = TargetPollEvent.class, count = 2) })
public void lazyFindOrRegisterTargetIfItDoesNotexist() throws InterruptedException {
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist("AA", LOCALHOST);
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
assertThat(target).as("target should not be null").isNotNull();
TimeUnit.MILLISECONDS.sleep(10);
controllerManagement.findOrRegisterTargetIfItDoesNotexist("AA", LOCALHOST);
controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST);
TimeUnit.MILLISECONDS.sleep(repositoryProperties.getPollPersistenceFlushTime() + 1);
final Target updated = targetManagement.get(target.getId()).get();

4
hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TargetManagementTest.java
View File

@@ -472,7 +472,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest {
Target target = createTargetWithAttributes("4711");
final long current = System.currentTimeMillis();
controllerManagement.findOrRegisterTargetIfItDoesNotexist("4711", LOCALHOST);
controllerManagement.findOrRegisterTargetIfItDoesNotExist("4711", LOCALHOST);
final DistributionSetAssignmentResult result = assignDistributionSet(set.getId(), "4711");
@@ -858,7 +858,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest {
@Expect(type = TargetPollEvent.class, count = 1) })
public void targetCanBeReadWithOnlyReadTargetPermission() throws Exception {
final String knownTargetControllerId = "readTarget";
controllerManagement.findOrRegisterTargetIfItDoesNotexist(knownTargetControllerId, new URI("http://127.0.0.1"));
controllerManagement.findOrRegisterTargetIfItDoesNotExist(knownTargetControllerId, new URI("http://127.0.0.1"));
securityRule.runAs(WithSpringAuthorityRule.withUser("bumlux", "READ_TARGET"), () -> {
final Target findTargetByControllerID = targetManagement.getByControllerID(knownTargetControllerId).get();

4
hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java
View File

@@ -48,7 +48,7 @@ public class RSQLTargetFieldTest extends AbstractJpaIntegrationTest {
.name("targetName123").description("targetDesc123"));
attributes.put("revision", "1.1");
target = controllerManagement.updateControllerAttributes(target.getControllerId(), attributes, null);
target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(target.getControllerId(), LOCALHOST);
target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(target.getControllerId(), LOCALHOST);
createTargetMetadata(target.getControllerId(), entityFactory.generateTargetMetadata("metaKey", "metaValue"));
target2 = targetManagement
@@ -56,7 +56,7 @@ public class RSQLTargetFieldTest extends AbstractJpaIntegrationTest {
attributes.put("revision", "1.2");
Thread.sleep(1);
target2 = controllerManagement.updateControllerAttributes(target2.getControllerId(), attributes, null);
target2 = controllerManagement.findOrRegisterTargetIfItDoesNotexist(target2.getControllerId(), LOCALHOST);
target2 = controllerManagement.findOrRegisterTargetIfItDoesNotExist(target2.getControllerId(), LOCALHOST);
createTargetMetadata(target2.getControllerId(), entityFactory.generateTargetMetadata("metaKey", "value"));
final Target target3 = testdataFactory.createTarget("targetId1235");

2
hawkbit-rest/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java
View File

@@ -143,7 +143,7 @@ public class DdiRootController implements DdiRootControllerRestApi {
@PathVariable("controllerId") final String controllerId) {
LOG.debug("getControllerBase({})", controllerId);
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(controllerId, IpUtil
final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotExist(controllerId, IpUtil
.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
final Action action = controllerManagement.findOldestActiveActionByTarget(controllerId).orElse(null);

4
hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetResourceTest.java
View File

@@ -1640,7 +1640,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
private Target createSingleTarget(final String controllerId, final String name) {
targetManagement.create(entityFactory.target().create().controllerId(controllerId).name(name)
.description(TARGET_DESCRIPTION_TEST));
return controllerManagement.findOrRegisterTargetIfItDoesNotexist(controllerId, LOCALHOST);
return controllerManagement.findOrRegisterTargetIfItDoesNotExist(controllerId, LOCALHOST);
}
/**
@@ -1656,7 +1656,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
for (int index = 0; index < amount; index++) {
final String str = String.valueOf(character);
targetManagement.create(entityFactory.target().create().controllerId(str).name(str).description(str));
controllerManagement.findOrRegisterTargetIfItDoesNotexist(str, LOCALHOST);
controllerManagement.findOrRegisterTargetIfItDoesNotExist(str, LOCALHOST);
character++;
}
}