hawkBit repository uses Optional on single entity find/get requests (#435)
* Repo returns optionals. * Improved exception handling for collection usage in repo queries. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>
This commit is contained in:
@@ -155,21 +155,26 @@ public class AmqpAuthenticationMessageHandler extends BaseAmqpService {
|
||||
LOG.info("download security check for target {} and artifact {} granted", controllerId, sha1Hash);
|
||||
}
|
||||
|
||||
private Optional<String> findArtifactByFileResource(final FileResource fileResource) {
|
||||
private Optional<org.eclipse.hawkbit.repository.model.Artifact> findArtifactByFileResource(
|
||||
final FileResource fileResource) {
|
||||
if (fileResource.getSha1() != null) {
|
||||
return Optional.ofNullable(fileResource.getSha1());
|
||||
} else if (fileResource.getFilename() != null) {
|
||||
return artifactManagement.findArtifactByFilename(fileResource.getFilename()).stream().findFirst()
|
||||
.map(org.eclipse.hawkbit.repository.model.Artifact::getSha1Hash);
|
||||
} else if (fileResource.getArtifactId() != null) {
|
||||
return Optional.ofNullable(artifactManagement.findArtifact(fileResource.getArtifactId()))
|
||||
.map(org.eclipse.hawkbit.repository.model.Artifact::getSha1Hash);
|
||||
} else if (fileResource.getSoftwareModuleFilenameResource() != null) {
|
||||
return artifactManagement
|
||||
.findByFilenameAndSoftwareModule(fileResource.getSoftwareModuleFilenameResource().getFilename(),
|
||||
fileResource.getSoftwareModuleFilenameResource().getSoftwareModuleId())
|
||||
.stream().findFirst().map(org.eclipse.hawkbit.repository.model.Artifact::getSha1Hash);
|
||||
return artifactManagement.findFirstArtifactBySHA1(fileResource.getSha1());
|
||||
}
|
||||
|
||||
if (fileResource.getFilename() != null) {
|
||||
return artifactManagement.findArtifactByFilename(fileResource.getFilename());
|
||||
}
|
||||
|
||||
if (fileResource.getArtifactId() != null) {
|
||||
return artifactManagement.findArtifact(fileResource.getArtifactId());
|
||||
}
|
||||
|
||||
if (fileResource.getSoftwareModuleFilenameResource() != null) {
|
||||
return artifactManagement.findByFilenameAndSoftwareModule(
|
||||
fileResource.getSoftwareModuleFilenameResource().getFilename(),
|
||||
fileResource.getSoftwareModuleFilenameResource().getSoftwareModuleId());
|
||||
}
|
||||
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
@@ -190,24 +195,20 @@ public class AmqpAuthenticationMessageHandler extends BaseAmqpService {
|
||||
try {
|
||||
SecurityContextHolder.getContext().setAuthentication(authenticationManager.doAuthenticate(secruityToken));
|
||||
|
||||
final Optional<String> sha1Hash = findArtifactByFileResource(fileResource);
|
||||
final String sha1Hash = findArtifactByFileResource(fileResource)
|
||||
.map(org.eclipse.hawkbit.repository.model.Artifact::getSha1Hash)
|
||||
.orElseThrow(() -> new EntityNotFoundException());
|
||||
|
||||
if (!sha1Hash.isPresent()) {
|
||||
LOG.info("target {} requested file resource {} which does not exists to download",
|
||||
secruityToken.getControllerId(), fileResource);
|
||||
throw new EntityNotFoundException();
|
||||
}
|
||||
checkIfArtifactIsAssignedToTarget(secruityToken, sha1Hash);
|
||||
|
||||
checkIfArtifactIsAssignedToTarget(secruityToken, sha1Hash.get());
|
||||
|
||||
final Artifact artifact = convertDbArtifact(artifactManagement.loadArtifactBinary(sha1Hash.get()));
|
||||
final Artifact artifact = convertDbArtifact(artifactManagement.loadArtifactBinary(sha1Hash));
|
||||
if (artifact == null) {
|
||||
throw new EntityNotFoundException();
|
||||
}
|
||||
authentificationResponse.setArtifact(artifact);
|
||||
final String downloadId = UUID.randomUUID().toString();
|
||||
// SHA1 key is set, download by SHA1
|
||||
final DownloadArtifactCache downloadCache = new DownloadArtifactCache(DownloadType.BY_SHA1, sha1Hash.get());
|
||||
final DownloadArtifactCache downloadCache = new DownloadArtifactCache(DownloadType.BY_SHA1, sha1Hash);
|
||||
cache.put(downloadId, downloadCache);
|
||||
authentificationResponse
|
||||
.setDownloadUrl(UriComponentsBuilder.fromUri(hostnameResolver.resolveHostname().toURI())
|
||||
|
||||
@@ -103,8 +103,8 @@ public class AmqpMessageDispatcherService extends BaseAmqpService {
|
||||
}
|
||||
|
||||
sendUpdateMessageToTarget(assignedEvent.getTenant(),
|
||||
targetManagement.findTargetByControllerID(assignedEvent.getControllerId()), assignedEvent.getActionId(),
|
||||
assignedEvent.getModules());
|
||||
targetManagement.findTargetByControllerID(assignedEvent.getControllerId()).get(),
|
||||
assignedEvent.getActionId(), assignedEvent.getModules());
|
||||
}
|
||||
|
||||
void sendUpdateMessageToTarget(final String tenant, final Target target, final Long actionId,
|
||||
|
||||
@@ -28,7 +28,6 @@ import org.eclipse.hawkbit.repository.ControllerManagement;
|
||||
import org.eclipse.hawkbit.repository.EntityFactory;
|
||||
import org.eclipse.hawkbit.repository.RepositoryConstants;
|
||||
import org.eclipse.hawkbit.repository.builder.ActionStatusCreate;
|
||||
import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
|
||||
import org.eclipse.hawkbit.repository.model.Action;
|
||||
import org.eclipse.hawkbit.repository.model.Action.Status;
|
||||
import org.eclipse.hawkbit.repository.model.Target;
|
||||
@@ -317,6 +316,9 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
|
||||
return controllerManagement.addUpdateActionStatus(actionStatus);
|
||||
}
|
||||
|
||||
// Exception squid:S3655 - logAndThrowMessageError throws exception, i.e.
|
||||
// get will not be called
|
||||
@SuppressWarnings("squid:S3655")
|
||||
private Action checkActionExist(final Message message, final ActionUpdateStatus actionUpdateStatus) {
|
||||
final Long actionId = actionUpdateStatus.getActionId();
|
||||
LOG.debug("Target notifies intermediate about action {} with status {}.", actionId,
|
||||
@@ -326,17 +328,12 @@ public class AmqpMessageHandlerService extends BaseAmqpService {
|
||||
logAndThrowMessageError(message, "Invalid message no action id");
|
||||
}
|
||||
|
||||
try {
|
||||
final Action findActionWithDetails = controllerManagement.findActionWithDetails(actionId);
|
||||
if (findActionWithDetails == null) {
|
||||
logAndThrowMessageError(message,
|
||||
"Got intermediate notification about action " + actionId + " but action does not exist");
|
||||
}
|
||||
return findActionWithDetails;
|
||||
} catch (@SuppressWarnings("squid:S1166") final EntityNotFoundException e) {
|
||||
final Optional<Action> findActionWithDetails = controllerManagement.findActionWithDetails(actionId);
|
||||
if (!findActionWithDetails.isPresent()) {
|
||||
logAndThrowMessageError(message,
|
||||
"Got intermediate notification about action " + actionId + " but action does not exist");
|
||||
}
|
||||
return null;
|
||||
|
||||
return findActionWithDetails.get();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
package org.eclipse.hawkbit.amqp;
|
||||
|
||||
import static org.fest.assertions.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.mockito.Matchers.any;
|
||||
import static org.mockito.Matchers.anyString;
|
||||
@@ -17,6 +17,7 @@ import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.net.URL;
|
||||
import java.util.Optional;
|
||||
|
||||
import org.eclipse.hawkbit.api.HostnameResolver;
|
||||
import org.eclipse.hawkbit.artifact.repository.model.DbArtifact;
|
||||
@@ -134,8 +135,8 @@ public class AmqpControllerAuthenticationTest {
|
||||
.thenReturn(CONFIG_VALUE_FALSE);
|
||||
|
||||
final ControllerManagement controllerManagement = mock(ControllerManagement.class);
|
||||
when(controllerManagement.findByControllerId(anyString())).thenReturn(targteMock);
|
||||
when(controllerManagement.findByTargetId(any(Long.class))).thenReturn(targteMock);
|
||||
when(controllerManagement.findByControllerId(anyString())).thenReturn(Optional.of(targteMock));
|
||||
when(controllerManagement.findByTargetId(any(Long.class))).thenReturn(Optional.of(targteMock));
|
||||
|
||||
when(targteMock.getSecurityToken()).thenReturn(CONTROLLER_ID);
|
||||
when(targteMock.getControllerId()).thenReturn(CONTROLLER_ID);
|
||||
@@ -156,8 +157,8 @@ public class AmqpControllerAuthenticationTest {
|
||||
new JpaSoftwareModuleType("a key", "a name", null, 1), "a name", null, null, null));
|
||||
testArtifact.setId(1L);
|
||||
|
||||
when(artifactManagementMock.findArtifact(ARTIFACT_ID)).thenReturn(testArtifact);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(SHA1)).thenReturn(testArtifact);
|
||||
when(artifactManagementMock.findArtifact(ARTIFACT_ID)).thenReturn(Optional.of(testArtifact));
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(SHA1)).thenReturn(Optional.of(testArtifact));
|
||||
|
||||
final DbArtifact artifact = new DbArtifact();
|
||||
artifact.setSize(ARTIFACT_SIZE);
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
package org.eclipse.hawkbit.amqp;
|
||||
|
||||
import static org.fest.assertions.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
@@ -126,13 +126,13 @@ public class AmqpMessageDispatcherServiceTest extends AbstractIntegrationTest {
|
||||
|
||||
private Message getCaptureAdressEvent(final TargetAssignDistributionSetEvent targetAssignDistributionSetEvent) {
|
||||
final Target target = targetManagement
|
||||
.findTargetByControllerID(targetAssignDistributionSetEvent.getControllerId());
|
||||
.findTargetByControllerID(targetAssignDistributionSetEvent.getControllerId()).get();
|
||||
final Message sendMessage = createArgumentCapture(target.getTargetInfo().getAddress());
|
||||
return sendMessage;
|
||||
}
|
||||
|
||||
private Action createAction(final DistributionSet testDs) {
|
||||
return deploymentManagement.findAction(assignDistributionSet(testDs, testTarget).getActions().get(0));
|
||||
return deploymentManagement.findAction(assignDistributionSet(testDs, testTarget).getActions().get(0)).get();
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -177,8 +177,8 @@ public class AmqpMessageDispatcherServiceTest extends AbstractIntegrationTest {
|
||||
for (final Artifact artifact : testdataFactory.createArtifacts(module.getId())) {
|
||||
receivedList.add(new DbArtifact());
|
||||
}
|
||||
module = softwareManagement.findSoftwareModuleById(module.getId());
|
||||
dsA = distributionSetManagement.findDistributionSetById(dsA.getId());
|
||||
module = softwareManagement.findSoftwareModuleById(module.getId()).get();
|
||||
dsA = distributionSetManagement.findDistributionSetById(dsA.getId()).get();
|
||||
|
||||
final Action action = createAction(dsA);
|
||||
|
||||
@@ -205,7 +205,7 @@ public class AmqpMessageDispatcherServiceTest extends AbstractIntegrationTest {
|
||||
module.getArtifacts().forEach(dbArtifact -> {
|
||||
final Optional<org.eclipse.hawkbit.dmf.json.model.Artifact> found = softwareModule.getArtifacts()
|
||||
.stream().filter(dmfartifact -> dmfartifact.getFilename().equals(dbArtifact.getFilename()))
|
||||
.findFirst();
|
||||
.findAny();
|
||||
|
||||
assertTrue("The artifact should exist in message", found.isPresent());
|
||||
assertThat(found.get().getSize()).isEqualTo(dbArtifact.getSize());
|
||||
|
||||
@@ -8,8 +8,9 @@
|
||||
*/
|
||||
package org.eclipse.hawkbit.amqp;
|
||||
|
||||
import static org.fest.assertions.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.mockito.Matchers.any;
|
||||
import static org.mockito.Matchers.anyObject;
|
||||
import static org.mockito.Matchers.anyString;
|
||||
import static org.mockito.Mockito.mock;
|
||||
@@ -126,6 +127,8 @@ public class AmqpMessageHandlerServiceTest {
|
||||
public void before() throws Exception {
|
||||
messageConverter = new Jackson2JsonMessageConverter();
|
||||
when(rabbitTemplate.getMessageConverter()).thenReturn(messageConverter);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(SHA1)).thenReturn(Optional.empty());
|
||||
|
||||
amqpMessageHandlerService = new AmqpMessageHandlerService(rabbitTemplate, amqpMessageDispatcherServiceMock,
|
||||
controllerManagementMock, entityFactoryMock);
|
||||
|
||||
@@ -163,7 +166,7 @@ public class AmqpMessageHandlerServiceTest {
|
||||
final ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
|
||||
when(controllerManagementMock.findOrRegisterTargetIfItDoesNotexist(targetIdCaptor.capture(),
|
||||
uriCaptor.capture())).thenReturn(targetMock);
|
||||
when(controllerManagementMock.findOldestActiveActionByTarget(Matchers.any())).thenReturn(Optional.empty());
|
||||
when(controllerManagementMock.findOldestActiveActionByTarget(any())).thenReturn(Optional.empty());
|
||||
|
||||
amqpMessageHandlerService.onMessage(message, MessageType.THING_CREATED.name(), TENANT, "vHost");
|
||||
|
||||
@@ -295,6 +298,8 @@ public class AmqpMessageHandlerServiceTest {
|
||||
public void updateActionStatusWithoutExistActionId() {
|
||||
final MessageProperties messageProperties = createMessageProperties(MessageType.EVENT);
|
||||
messageProperties.setHeader(MessageHeaderKey.TOPIC, EventTopic.UPDATE_ACTION_STATUS.name());
|
||||
when(controllerManagementMock.findActionWithDetails(any())).thenReturn(Optional.empty());
|
||||
|
||||
final ActionUpdateStatus actionUpdateStatus = createActionUpdateStatus(ActionStatus.DOWNLOAD);
|
||||
final Message message = amqpMessageHandlerService.getMessageConverter().toMessage(actionUpdateStatus,
|
||||
messageProperties);
|
||||
@@ -337,7 +342,7 @@ public class AmqpMessageHandlerServiceTest {
|
||||
messageProperties);
|
||||
|
||||
final Artifact localArtifactMock = mock(Artifact.class);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(anyString())).thenReturn(localArtifactMock);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(anyString())).thenReturn(Optional.of(localArtifactMock));
|
||||
when(controllerManagementMock.getActionForDownloadByTargetAndSoftwareModule(anyObject(), anyObject()))
|
||||
.thenThrow(EntityNotFoundException.class);
|
||||
|
||||
@@ -365,7 +370,7 @@ public class AmqpMessageHandlerServiceTest {
|
||||
when(localArtifactMock.getSha1Hash()).thenReturn(SHA1);
|
||||
|
||||
final DbArtifact dbArtifactMock = mock(DbArtifact.class);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(SHA1)).thenReturn(localArtifactMock);
|
||||
when(artifactManagementMock.findFirstArtifactBySHA1(SHA1)).thenReturn(Optional.of(localArtifactMock));
|
||||
when(controllerManagementMock.hasTargetArtifactAssigned(securityToken.getControllerId(), SHA1))
|
||||
.thenReturn(true);
|
||||
when(artifactManagementMock.loadArtifactBinary(anyString())).thenReturn(dbArtifactMock);
|
||||
@@ -395,15 +400,15 @@ public class AmqpMessageHandlerServiceTest {
|
||||
|
||||
// Mock
|
||||
final Action action = createActionWithTarget(22L, Status.FINISHED);
|
||||
when(controllerManagementMock.findActionWithDetails(Matchers.any())).thenReturn(action);
|
||||
when(controllerManagementMock.addUpdateActionStatus(Matchers.any())).thenReturn(action);
|
||||
when(controllerManagementMock.findActionWithDetails(any())).thenReturn(Optional.of(action));
|
||||
when(controllerManagementMock.addUpdateActionStatus(any())).thenReturn(action);
|
||||
final ActionStatusBuilder builder = mock(ActionStatusBuilder.class);
|
||||
final ActionStatusCreate create = mock(ActionStatusCreate.class);
|
||||
when(builder.create(22L)).thenReturn(create);
|
||||
when(create.status(Matchers.any())).thenReturn(create);
|
||||
when(create.status(any())).thenReturn(create);
|
||||
when(entityFactoryMock.actionStatus()).thenReturn(builder);
|
||||
// for the test the same action can be used
|
||||
when(controllerManagementMock.findOldestActiveActionByTarget(Matchers.any())).thenReturn(Optional.of(action));
|
||||
when(controllerManagementMock.findOldestActiveActionByTarget(any())).thenReturn(Optional.of(action));
|
||||
|
||||
final MessageProperties messageProperties = createMessageProperties(MessageType.EVENT);
|
||||
messageProperties.setHeader(MessageHeaderKey.TOPIC, EventTopic.UPDATE_ACTION_STATUS.name());
|
||||
@@ -415,14 +420,14 @@ public class AmqpMessageHandlerServiceTest {
|
||||
amqpMessageHandlerService.onMessage(message, MessageType.EVENT.name(), TENANT, "vHost");
|
||||
|
||||
// verify
|
||||
verify(controllerManagementMock).updateLastTargetQuery(Matchers.any(String.class), Matchers.isNull(URI.class));
|
||||
verify(controllerManagementMock).updateLastTargetQuery(any(String.class), Matchers.isNull(URI.class));
|
||||
|
||||
final ArgumentCaptor<String> tenantCaptor = ArgumentCaptor.forClass(String.class);
|
||||
final ArgumentCaptor<Target> targetCaptor = ArgumentCaptor.forClass(Target.class);
|
||||
final ArgumentCaptor<Long> actionIdCaptor = ArgumentCaptor.forClass(Long.class);
|
||||
|
||||
verify(amqpMessageDispatcherServiceMock, times(1)).sendUpdateMessageToTarget(tenantCaptor.capture(),
|
||||
targetCaptor.capture(), actionIdCaptor.capture(), Matchers.any(Collection.class));
|
||||
targetCaptor.capture(), actionIdCaptor.capture(), any(Collection.class));
|
||||
final String tenant = tenantCaptor.getValue();
|
||||
final String controllerId = targetCaptor.getValue().getControllerId();
|
||||
final Long actionId = actionIdCaptor.getValue();
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
package org.eclipse.hawkbit.amqp;
|
||||
|
||||
import static org.fest.assertions.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.ArrayList;
|
||||
@@ -61,7 +61,7 @@ public class BaseAmqpServiceTest {
|
||||
ActionUpdateStatus.class);
|
||||
|
||||
assertThat(convertedActionUpdateStatus).as("Converted Action Status is wrong")
|
||||
.isEqualsToByComparingFields(actionUpdateStatus);
|
||||
.isEqualToComparingFieldByField(actionUpdateStatus);
|
||||
|
||||
convertedActionUpdateStatus = baseAmqpService.convertMessage(null, ActionUpdateStatus.class);
|
||||
assertThat(convertedActionUpdateStatus).as("Converted Object should be null when message is null").isNull();
|
||||
|
||||
Reference in New Issue
Block a user