From fbaa352f7f7fc7d721c7d951292d79e553d29d4c Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Thu, 23 Jan 2025 16:48:24 +0200 Subject: [PATCH] Sonar Fixes (10) (#2222) Signed-off-by: Avgustin Marinov --- .../ddi/rest/resource/DdiRootController.java | 56 +++++---- .../amqp/AmqpMessageDispatcherService.java | 1 + .../amqp/DelegatingAmqpErrorHandlerTest.java | 21 ++-- .../deprecated/DeprecatedMgmtResource.java | 55 +++++---- .../deprecated/DeprecatedMgmtRestApi.java | 7 +- .../repository/model/ArtifactUpload.java | 4 +- .../repository/model/DeploymentRequest.java | 1 + .../jpa/utils/DeploymentHelper.java | 4 +- .../DistributionSetAccessControllerTest.java | 115 +++++++++--------- .../management/DeploymentManagementTest.java | 52 ++++---- .../hawkbit/sdk/demo/device/DeviceApp.java | 1 + 11 files changed, 172 insertions(+), 145 deletions(-) diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java b/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java index 06c8b3114..1699501bb 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java @@ -102,28 +102,42 @@ public class DdiRootController implements DdiRootControllerRestApi { * File suffix for MDH hash download (see Linux md5sum). */ private static final String ARTIFACT_MD5_DWNL_SUFFIX = ".MD5SUM"; - @Autowired - private ConfirmationManagement confirmationManagement; - @Autowired - private ApplicationEventPublisher eventPublisher; - @Autowired(required = false) + + private final ControllerManagement controllerManagement; + private final ConfirmationManagement confirmationManagement; + private final ArtifactManagement artifactManagement; + private final ArtifactUrlHandler artifactUrlHandler; + private final SystemManagement systemManagement; + private final ApplicationEventPublisher eventPublisher; + private final BusProperties bus; + private final HawkbitSecurityProperties securityProperties; + private final TenantAware tenantAware; + private final EntityFactory entityFactory; private ServiceMatcher serviceMatcher; - @Autowired - private BusProperties bus; - @Autowired - private ControllerManagement controllerManagement; - @Autowired - private ArtifactManagement artifactManagement; - @Autowired - private HawkbitSecurityProperties securityProperties; - @Autowired - private TenantAware tenantAware; - @Autowired - private SystemManagement systemManagement; - @Autowired - private ArtifactUrlHandler artifactUrlHandler; - @Autowired - private EntityFactory entityFactory; + + @SuppressWarnings("java:S107") + public DdiRootController( + final ControllerManagement controllerManagement, final ConfirmationManagement confirmationManagement, + final ArtifactManagement artifactManagement, final ArtifactUrlHandler artifactUrlHandler, + final SystemManagement systemManagement, + final ApplicationEventPublisher eventPublisher, final BusProperties bus, + final HawkbitSecurityProperties securityProperties, final TenantAware tenantAware, final EntityFactory entityFactory) { + this.controllerManagement = controllerManagement; + this.confirmationManagement = confirmationManagement; + this.artifactManagement = artifactManagement; + this.artifactUrlHandler = artifactUrlHandler; + this.systemManagement = systemManagement; + this.eventPublisher = eventPublisher; + this.bus = bus; + this.securityProperties = securityProperties; + this.tenantAware = tenantAware; + this.entityFactory = entityFactory; + } + + @Autowired(required = false) + public void setServiceMatcher(final ServiceMatcher serviceMatcher) { + this.serviceMatcher = serviceMatcher; + } @Override public ResponseEntity> getSoftwareModulesArtifacts( diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java index e6a02ce2f..699e1b970 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageDispatcherService.java @@ -114,6 +114,7 @@ public class AmqpMessageDispatcherService extends BaseAmqpService { * @param distributionSetManagement to retrieve modules * @param tenantConfigurationManagement to access tenant configuration */ + @SuppressWarnings("java:S107") protected AmqpMessageDispatcherService( final RabbitTemplate rabbitTemplate, final AmqpMessageSenderService amqpSenderService, final ArtifactUrlHandler artifactUrlHandler, diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/DelegatingAmqpErrorHandlerTest.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/DelegatingAmqpErrorHandlerTest.java index c632a8c16..665971856 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/DelegatingAmqpErrorHandlerTest.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/DelegatingAmqpErrorHandlerTest.java @@ -24,28 +24,27 @@ import org.springframework.util.ErrorHandler; @Story("Delegating Conditional Error Handler") class DelegatingAmqpErrorHandlerTest { + private final DelegatingConditionalErrorHandler delegatingConditionalErrorHandler = + new DelegatingConditionalErrorHandler( + List.of(new IllegalArgumentExceptionHandler(), new IndexOutOfBoundsExceptionHandler()), + new DefaultErrorHandler()); + @Test @Description("Verifies that with a list of conditional error handlers, the error is delegated to specific handler.") void verifyDelegationHandling() { - List handlers = new ArrayList<>(); - handlers.add(new IllegalArgumentExceptionHandler()); - handlers.add(new IndexOutOfBoundsExceptionHandler()); + final Throwable error = new Throwable(new IllegalArgumentException()); assertThatExceptionOfType(IllegalArgumentException.class) .as("Expected handled exception to be of type IllegalArgumentException") - .isThrownBy(() -> new DelegatingConditionalErrorHandler(handlers, new DefaultErrorHandler()) - .handleError(new Throwable(new IllegalArgumentException()))); + .isThrownBy(() -> delegatingConditionalErrorHandler.handleError(error)); } @Test @Description("Verifies that default handler is used if no handlers are defined for the specific exception.") void verifyDefaultDelegationHandling() { - List handlers = new ArrayList<>(); - handlers.add(new IllegalArgumentExceptionHandler()); - handlers.add(new IndexOutOfBoundsExceptionHandler()); + final Throwable error = new Throwable(new NullPointerException()); assertThatExceptionOfType(RuntimeException.class) .as("Expected handled exception to be of type RuntimeException") - .isThrownBy(() -> new DelegatingConditionalErrorHandler(handlers, new DefaultErrorHandler()) - .handleError(new Throwable(new NullPointerException()))); + .isThrownBy(() -> delegatingConditionalErrorHandler.handleError(error)); } // Test class @@ -82,4 +81,4 @@ class DelegatingAmqpErrorHandlerTest { throw new RuntimeException(t); } } -} +} \ No newline at end of file diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtResource.java b/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtResource.java index a8e07036c..3b70f75b0 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtResource.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtResource.java @@ -64,28 +64,34 @@ public class DeprecatedMgmtResource implements DeprecatedMgmtRestApi { // logger that logs usage of deprecated API private static final Logger DEPRECATED_USAGE_LOGGER = LoggerFactory.getLogger("DEPRECATED_USAGE"); - @Autowired - private DistributionSetRepository distributionSetRepository; - @Autowired - private DistributionSetTagManagement distributionSetTagManagement; - @Autowired - private DistributionSetManagement distributionSetManagement; - @Autowired - private TargetRepository targetRepository; - @Autowired - private TargetTagRepository targetTagRepository; - @Autowired - private TargetManagement targetManagement; - @Autowired - private TargetTagManagement targetTagManagement; - @Autowired - private PlatformTransactionManager txManager; - @Autowired - private EntityManager entityManager; - + private final DistributionSetRepository distributionSetRepository; + private final DistributionSetTagManagement distributionSetTagManagement; + private final DistributionSetManagement distributionSetManagement; + private final TargetRepository targetRepository; + private final TargetTagRepository targetTagRepository; + private final TargetManagement targetManagement; + private final TargetTagManagement targetTagManagement; + private final PlatformTransactionManager txManager; + private final EntityManager entityManager; private final TenantConfigHelper tenantConfigHelper; - DeprecatedMgmtResource(final SystemSecurityContext securityContext, final TenantConfigurationManagement configurationManagement) { + @SuppressWarnings("squid:S107") + DeprecatedMgmtResource( + final DistributionSetRepository distributionSetRepository, final DistributionSetTagManagement distributionSetTagManagement, + final DistributionSetManagement distributionSetManagement, + final TargetRepository targetRepository, final TargetTagRepository targetTagRepository, final TargetManagement targetManagement, + final TargetTagManagement targetTagManagement, + final PlatformTransactionManager txManager, final EntityManager entityManager, + final SystemSecurityContext securityContext, final TenantConfigurationManagement configurationManagement) { + this.distributionSetRepository = distributionSetRepository; + this.distributionSetTagManagement = distributionSetTagManagement; + this.distributionSetManagement = distributionSetManagement; + this.targetRepository = targetRepository; + this.targetTagRepository = targetTagRepository; + this.targetManagement = targetManagement; + this.targetTagManagement = targetTagManagement; + this.txManager = txManager; + this.entityManager = entityManager; tenantConfigHelper = TenantConfigHelper.usingContext(securityContext, configurationManagement); } @@ -180,12 +186,12 @@ public class DeprecatedMgmtResource implements DeprecatedMgmtRestApi { result = new DistributionSetTagAssignmentResult(ids.size() - toBeChangedDSs.size(), Collections.emptyList(), Collections.unmodifiableList( - toBeChangedDSs.stream().map(distributionSetRepository::save).collect(Collectors.toList())), + toBeChangedDSs.stream().map(distributionSetRepository::save).toList()), distributionSetTag); } else { result = new DistributionSetTagAssignmentResult(ids.size() - toBeChangedDSs.size(), Collections.unmodifiableList( - toBeChangedDSs.stream().map(distributionSetRepository::save).collect(Collectors.toList())), + toBeChangedDSs.stream().map(distributionSetRepository::save).toList()), Collections.emptyList(), distributionSetTag); } return result; @@ -247,7 +253,7 @@ public class DeprecatedMgmtResource implements DeprecatedMgmtRestApi { private static List findDistributionSetIds( final List assignedDistributionSetRequestBodies) { return assignedDistributionSetRequestBodies.stream() - .map(MgmtAssignedDistributionSetRequestBody::getDistributionSetId).collect(Collectors.toList()); + .map(MgmtAssignedDistributionSetRequestBody::getDistributionSetId).toList(); } private DistributionSetTag findDistributionTagById(final Long distributionsetTagId) { @@ -257,7 +263,6 @@ public class DeprecatedMgmtResource implements DeprecatedMgmtRestApi { private List findTargetControllerIds( final List assignedTargetRequestBodies) { - return assignedTargetRequestBodies.stream().map(MgmtAssignedTargetRequestBody::getControllerId) - .collect(Collectors.toList()); + return assignedTargetRequestBodies.stream().map(MgmtAssignedTargetRequestBody::getControllerId).toList(); } } \ No newline at end of file diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtRestApi.java b/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtRestApi.java index cf13b90f7..053877cde 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtRestApi.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource-deprecated/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/deprecated/DeprecatedMgmtRestApi.java @@ -37,16 +37,17 @@ import org.springframework.web.bind.annotation.RequestBody; /** * REST Resource handling for DistributionSetTag CRUD operations. + * + * @deprecated since 0.6.0 */ // no request mapping specified here to avoid CVE-2021-22044 in Feign client -@Deprecated(forRemoval = true) +@Deprecated(forRemoval = true, since = "0.6.0") @Tag(name = "Deprecated operations", description = "Deprecated REST operations.", extensions = @Extension(name = OpenApiConfiguration.X_HAWKBIT, properties = @ExtensionProperty(name = "order", value = "2147483647"))) public interface DeprecatedMgmtRestApi { /** - * Handles the POST request to toggle the assignment of distribution sets by - * the given tag id.
+ * Handles the POST request to toggle the assignment of distribution sets by the given tag id.
* From {@link org.eclipse.hawkbit.mgmt.rest.api.MgmtDistributionSetTagRestApi} * * @param distributionsetTagId the ID of the distribution set tag to retrieve diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java index cd0bbaa39..2718c4263 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java @@ -67,11 +67,11 @@ public class ArtifactUpload { * @param filename of the artifact * @param providedSha1Sum optional sha1 checksum to check the new file against * @param providedMd5Sum optional md5 checksum to check the new file against - * @param overrideExisting to true if the artifact binary can be overridden - * if it already exists + * @param overrideExisting to true if the artifact binary can be overridden if it already exists * @param contentType the contentType of the file * @param filesize the size of the file in bytes. */ + @SuppressWarnings("java:S107") public ArtifactUpload(final InputStream inputStream, final long moduleId, final String filename, final String providedMd5Sum, final String providedSha1Sum, final String providedSha256Sum, final boolean overrideExisting, final String contentType, final long filesize) { diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/DeploymentRequest.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/DeploymentRequest.java index c7bcf6183..fcaaae798 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/DeploymentRequest.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/DeploymentRequest.java @@ -56,6 +56,7 @@ public class DeploymentRequest { * with CONFIRMATION_FLOW active via tenant configuration) * @throws InvalidMaintenanceScheduleException if the parameters do not define a valid maintenance schedule. */ + @SuppressWarnings("java:S107") public DeploymentRequest(final String controllerId, final Long distributionSetId, final ActionType actionType, final long forceTime, final Integer weight, final String maintenanceSchedule, final String maintenanceWindowDuration, final String maintenanceWindowTimeZone, diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/utils/DeploymentHelper.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/utils/DeploymentHelper.java index 89bfdfeb8..fe3f0da2a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/utils/DeploymentHelper.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/utils/DeploymentHelper.java @@ -16,8 +16,8 @@ import jakarta.validation.constraints.NotNull; import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.eclipse.hawkbit.repository.jpa.model.AbstractJpaBaseEntity_; import org.eclipse.hawkbit.repository.jpa.model.JpaAction; -import org.eclipse.hawkbit.repository.jpa.model.JpaAction_; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; import org.eclipse.hawkbit.repository.jpa.repository.ActionRepository; @@ -59,7 +59,7 @@ public final class DeploymentHelper { final JpaTarget target = (JpaTarget) action.getTarget(); final List nextActiveActions = actionRepository - .findAll(ActionSpecifications.byTargetIdAndIsActive(target.getId()), Sort.by(Sort.Order.asc(JpaAction_.ID))) + .findAll(ActionSpecifications.byTargetIdAndIsActive(target.getId()), Sort.by(Sort.Order.asc(AbstractJpaBaseEntity_.ID))) .stream() .filter(a -> !a.getId().equals(action.getId())) .map(Action.class::cast) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java index c9921f9e6..6d93834ca 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java @@ -35,7 +35,7 @@ import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSetFilter; -import org.eclipse.hawkbit.repository.model.DistributionSetTag; +import org.eclipse.hawkbit.repository.model.MetaData; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.junit.jupiter.api.Test; @@ -68,53 +68,53 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { TargetSpecifications.hasId(permittedAction.getTarget().getId()), target -> target.getId().equals(permittedAction.getTarget().getId())); + final Long permittedActionId = permitted.getId(); + // verify distributionSetManagement#findAll assertThat(distributionSetManagement.findAll(Pageable.unpaged()).get().map(Identifiable::getId).toList()) - .containsOnly(permitted.getId()); + .containsOnly(permittedActionId); // verify distributionSetManagement#findByRsql assertThat(distributionSetManagement.findByRsql("name==*", Pageable.unpaged()).get().map(Identifiable::getId) - .toList()).containsOnly(permitted.getId()); + .toList()).containsOnly(permittedActionId); // verify distributionSetManagement#findByCompleted assertThat(distributionSetManagement.findByCompleted(Pageable.unpaged(), true).get().map(Identifiable::getId) - .toList()).containsOnly(permitted.getId()); + .toList()).containsOnly(permittedActionId); // verify distributionSetManagement#findByDistributionSetFilter assertThat(distributionSetManagement - .findByDistributionSetFilter(DistributionSetFilter.builder().isDeleted(false).build(), Pageable.unpaged() - ) - .get().map(Identifiable::getId).toList()).containsOnly(permitted.getId()); + .findByDistributionSetFilter(DistributionSetFilter.builder().isDeleted(false).build(), Pageable.unpaged()) + .get().map(Identifiable::getId).toList()).containsOnly(permittedActionId); // verify distributionSetManagement#get - assertThat(distributionSetManagement.get(permitted.getId())).isPresent(); - assertThat(distributionSetManagement.get(hidden.getId())).isEmpty(); + assertThat(distributionSetManagement.get(permittedActionId)).isPresent(); + final Long hiddenId = hidden.getId(); + assertThat(distributionSetManagement.get(hiddenId)).isEmpty(); // verify distributionSetManagement#getWithDetails - assertThat(distributionSetManagement.getWithDetails(permitted.getId())).isPresent(); - assertThat(distributionSetManagement.getWithDetails(hidden.getId())).isEmpty(); + assertThat(distributionSetManagement.getWithDetails(permittedActionId)).isPresent(); + assertThat(distributionSetManagement.getWithDetails(hiddenId)).isEmpty(); // verify distributionSetManagement#get - assertThat(distributionSetManagement.getValid(permitted.getId()).getId()).isEqualTo(permitted.getId()); - assertThatThrownBy(() -> { - assertThat(distributionSetManagement.getValid(hidden.getId())); - }).as("Distribution set should not be found.").isInstanceOf(EntityNotFoundException.class); + assertThat(distributionSetManagement.getValid(permittedActionId).getId()).isEqualTo(permittedActionId); + assertThatThrownBy(() -> distributionSetManagement.getValid(hiddenId)) + .as("Distribution set should not be found.").isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#get - assertThatThrownBy(() -> { - distributionSetManagement.get(Arrays.asList(permitted.getId(), hidden.getId())); - }).as("Fail if request hidden.").isInstanceOf(EntityNotFoundException.class); + final List allActionIds = Arrays.asList(permittedActionId, hiddenId); + assertThatThrownBy(() -> distributionSetManagement.get(allActionIds)) + .as("Fail if request hidden.").isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#getByNameAndVersion - assertThat(distributionSetManagement.findByNameAndVersion(permitted.getName(), permitted.getVersion())) - .isPresent(); + assertThat(distributionSetManagement.findByNameAndVersion(permitted.getName(), permitted.getVersion())).isPresent(); assertThat(distributionSetManagement.findByNameAndVersion(hidden.getName(), hidden.getVersion())).isEmpty(); // verify distributionSetManagement#getByAction assertThat(distributionSetManagement.findByAction(permittedAction.getId())).isPresent(); - assertThatThrownBy(() -> { - distributionSetManagement.findByAction(hiddenAction.getId()); - }).as("Action is hidden.").isInstanceOf(InsufficientPermissionException.class); + final Long hiddenActionId = hiddenAction.getId(); + assertThatThrownBy(() -> distributionSetManagement.findByAction(hiddenActionId)) + .as("Action is hidden.").isInstanceOf(InsufficientPermissionException.class); } @Test @@ -140,41 +140,46 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { defineAccess(AccessController.Operation.UPDATE, permitted); // verify distributionSetManagement#assignSoftwareModules - assertThat(distributionSetManagement.assignSoftwareModules(permitted.getId(), Collections.singletonList(swModule.getId()))) + final var singleModuleIdList = Collections.singletonList(swModule.getId()); + assertThat(distributionSetManagement.assignSoftwareModules(permitted.getId(), singleModuleIdList)) .satisfies(ds -> assertThat(ds.getModules().stream().map(Identifiable::getId).toList()).contains(swModule.getId())); - assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(readOnly.getId(), Collections.singletonList(swModule.getId()))) + final Long readOnlyId = readOnly.getId(); + assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(readOnlyId, singleModuleIdList)) .as("Distribution set not allowed to me modified.") .isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(hidden.getId(), Collections.singletonList(swModule.getId()))) + final Long hiddenId = hidden.getId(); + assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(hiddenId, singleModuleIdList)) .as("Distribution set should not be visible.") .isInstanceOf(EntityNotFoundException.class); final JpaDistributionSetMetadata metadata = new JpaDistributionSetMetadata("test", "test"); // verify distributionSetManagement#createMetaData - distributionSetManagement.putMetaData(permitted.getId(), Collections.singletonList(metadata)); - assertThatThrownBy(() -> distributionSetManagement.putMetaData(readOnly.getId(), Collections.singletonList(metadata))) + final List metadataList = Collections.singletonList(metadata); + distributionSetManagement.putMetaData(permitted.getId(), metadataList); + assertThatThrownBy(() -> distributionSetManagement.putMetaData(readOnlyId, metadataList)) .as("Distribution set not allowed to me modified.") .isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> distributionSetManagement.putMetaData(hidden.getId(), Collections.singletonList(metadata))) + assertThatThrownBy(() -> distributionSetManagement.putMetaData(hiddenId, metadataList)) .as("Distribution set should not be visible.") .isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#updateMetaData distributionSetManagement.updateMetaData(permitted.getId(), metadata); - assertThatThrownBy(() -> distributionSetManagement.updateMetaData(readOnly.getId(), metadata)) + assertThatThrownBy(() -> distributionSetManagement.updateMetaData(readOnlyId, metadata)) .as("Distribution set not allowed to me modified.") .isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> distributionSetManagement.updateMetaData(hidden.getId(), metadata)) + assertThatThrownBy(() -> distributionSetManagement.updateMetaData(hiddenId, metadata)) .as("Distribution set should not be visible.") .isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#deleteMetaData - distributionSetManagement.deleteMetaData(permitted.getId(), metadata.getKey()); - assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(readOnly.getId(), metadata.getKey())) + final String metadataKey = metadata.getKey(); + distributionSetManagement.deleteMetaData(permitted.getId(), metadataKey); + assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(readOnlyId, metadataKey)) .as("Distribution set not allowed to me modified.") .isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(hidden.getId(), metadata.getKey())) + assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(hiddenId, metadataKey)) .as("Distribution set should not be visible.") .isInstanceOf(EntityNotFoundException.class); } @@ -189,12 +194,12 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { final DistributionSet permitted = testdataFactory.createDistributionSet(); final DistributionSet readOnly = testdataFactory.createDistributionSet(); final DistributionSet hidden = testdataFactory.createDistributionSet(); - final DistributionSetTag dsTag = distributionSetTagManagement.create(entityFactory.tag().create().name("dsTag")); - final DistributionSetTag dsTag2 = distributionSetTagManagement.create(entityFactory.tag().create().name("dsTag2")); + final Long dsTagId = distributionSetTagManagement.create(entityFactory.tag().create().name("dsTag")).getId(); + final Long dsTag2Id = distributionSetTagManagement.create(entityFactory.tag().create().name("dsTag2")).getId(); // perform tag assignment before setting access rules distributionSetManagement.assignTag(Arrays.asList(permitted.getId(), readOnly.getId(), hidden.getId()), - dsTag.getId()); + dsTagId); // entities created - reset rules testAccessControlManger.deleteAllRules(); @@ -204,53 +209,55 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { // allow updating the permitted distributionSet defineAccess(AccessController.Operation.UPDATE, permitted); - assertThat(distributionSetManagement.findByTag(dsTag.getId(), Pageable.unpaged()).get().map(Identifiable::getId) + assertThat(distributionSetManagement.findByTag(dsTagId, Pageable.unpaged()).get().map(Identifiable::getId) .toList()).containsOnly(permitted.getId(), readOnly.getId()); - assertThat(distributionSetManagement.findByRsqlAndTag("name==*", dsTag.getId(), Pageable.unpaged()).get() + assertThat(distributionSetManagement.findByRsqlAndTag("name==*", dsTagId, Pageable.unpaged()).get() .map(Identifiable::getId).toList()).containsOnly(permitted.getId(), readOnly.getId()); // verify distributionSetManagement#unassignTag on permitted target assertThat(distributionSetManagement - .unassignTag(Collections.singletonList(permitted.getId()), dsTag.getId())) + .unassignTag(Collections.singletonList(permitted.getId()), dsTagId)) .size() .isEqualTo(1); // verify distributionSetManagement#assignTag on permitted target - assertThat(distributionSetManagement.assignTag(Collections.singletonList(permitted.getId()), dsTag.getId())) + assertThat(distributionSetManagement.assignTag(Collections.singletonList(permitted.getId()), dsTagId)) .hasSize(1); // verify distributionSetManagement#unAssignTag on permitted target - assertThat(distributionSetManagement.unassignTag(List.of(permitted.getId()), dsTag.getId()) + assertThat(distributionSetManagement.unassignTag(List.of(permitted.getId()), dsTagId) .get(0).getId()) .isEqualTo(permitted.getId()); // assignment is denied for readOnlyTarget (read, but no update permissions) + final List readOblyList = Collections.singletonList(readOnly.getId()); assertThatThrownBy(() -> - distributionSetManagement.unassignTag(Collections.singletonList(readOnly.getId()), dsTag.getId())) + distributionSetManagement.unassignTag(readOblyList, dsTagId)) .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for readOnlyTarget (read, but no update permissions) // dsTag2- since - it is tagged with dsTag and won't do anything if assigning dsTag assertThatThrownBy(() -> { - distributionSetManagement.assignTag(Collections.singletonList(readOnly.getId()), dsTag2.getId()); + distributionSetManagement.assignTag(readOblyList, dsTag2Id); }).as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for hiddenTarget since it's hidden - assertThatThrownBy(() -> distributionSetManagement.unassignTag(Collections.singletonList(hidden.getId()), dsTag.getId())) + final List hiddenList = Collections.singletonList(hidden.getId()); + assertThatThrownBy(() -> distributionSetManagement.unassignTag(hiddenList, dsTagId)) .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(EntityNotFoundException.class); // assignment is denied for hiddenTarget since it's hidden assertThatThrownBy(() -> { - distributionSetManagement.assignTag(Collections.singletonList(hidden.getId()), dsTag.getId()); + distributionSetManagement.assignTag(hiddenList, dsTagId); }).as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(EntityNotFoundException.class); // assignment is denied for hiddenTarget since it's hidden - assertThatThrownBy(() -> { - distributionSetManagement.unassignTag(List.of(hidden.getId()), dsTag.getId()); - }).as("Missing update permissions for target to toggle tag assignment.") + final List hiddenIdList = List.of(hidden.getId()); + assertThatThrownBy(() -> distributionSetManagement.unassignTag(hiddenIdList, dsTagId)) + .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(EntityNotFoundException.class); } @@ -287,12 +294,10 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { .updateAutoAssignDS(new AutoAssignDistributionSetUpdate(targetFilterQuery.getId()) .ds(readOnly.getId()).actionType(Action.ActionType.FORCED).confirmationRequired(false)) .getAutoAssignDistributionSet().getId(); - assertThatThrownBy(() -> { - targetFilterQueryManagement - .updateAutoAssignDS(new AutoAssignDistributionSetUpdate(targetFilterQuery.getId()) - .ds(hidden.getId()).actionType(Action.ActionType.FORCED).confirmationRequired(false)) - .getAutoAssignDistributionSet().getId(); - }).isInstanceOf(EntityNotFoundException.class); + final AutoAssignDistributionSetUpdate autoAssignDistributionSetUpdate = new AutoAssignDistributionSetUpdate(targetFilterQuery.getId()) + .ds(hidden.getId()).actionType(Action.ActionType.FORCED).confirmationRequired(false); + assertThatThrownBy(() -> targetFilterQueryManagement.updateAutoAssignDS(autoAssignDistributionSetUpdate)) + .isInstanceOf(EntityNotFoundException.class); } private void defineAccess(final AccessController.Operation operation, final DistributionSet... distributionSets) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DeploymentManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DeploymentManagementTest.java index 7cebd9e07..7e1e5001e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DeploymentManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DeploymentManagementTest.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.stream.IntStream; import java.util.stream.Stream; import jakarta.validation.ConstraintViolationException; @@ -196,18 +197,19 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { @Test @Description("Test verifies that the 'max actions per target' quota is enforced.") void assertMaxActionsPerTargetQuotaIsEnforced() { + enableMultiAssignments(); + final int maxActions = quotaManagement.getMaxActionsPerTarget(); final Target testTarget = testdataFactory.createTarget(); - final DistributionSet ds1 = testdataFactory.createDistributionSet("ds1"); + final Long ds1Id = testdataFactory.createDistributionSet("ds1").getId(); - enableMultiAssignments(); + final String controllerId = testTarget.getControllerId(); for (int i = 0; i < maxActions; i++) { - deploymentManagement.offlineAssignedDistributionSets(Collections - .singletonList(new SimpleEntry<>(testTarget.getControllerId(), ds1.getId()))); + deploymentManagement.offlineAssignedDistributionSets(List.of(new SimpleEntry<>(controllerId, ds1Id))); } assertThatExceptionOfType(AssignmentQuotaExceededException.class) - .isThrownBy(() -> assignDistributionSet(ds1.getId(), testTarget.getControllerId(), 77)); + .isThrownBy(() -> assignDistributionSet(ds1Id, controllerId, 77)); } @Test @@ -277,10 +279,10 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { // not exists assignDS.add(100L); - final DistributionSetTag tag = distributionSetTagManagement.create(entityFactory.tag().create().name("Tag1")); + final Long tagId = distributionSetTagManagement.create(entityFactory.tag().create().name("Tag1")).getId(); assertThatExceptionOfType(EntityNotFoundException.class) - .isThrownBy(() -> distributionSetManagement.assignTag(assignDS, tag.getId())) + .isThrownBy(() -> distributionSetManagement.assignTag(assignDS, tagId)) .withMessageContaining("DistributionSet") .withMessageContaining(String.valueOf(100L)); } @@ -455,7 +457,7 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { final Target target = action.getTarget(); final DistributionSet ds = testdataFactory.createDistributionSet("newDS", true); - final Action assigningAction = assignSet(target, ds); + final Long assigningActionId = assignSet(target, ds).getId(); // verify assignment assertThat(actionRepository.findAll()).as("wrong size of action").hasSize(2); @@ -464,7 +466,7 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { // force quit assignment assertThatExceptionOfType(ForceQuitActionNotAllowedException.class) .as("expected ForceQuitActionNotAllowedException") - .isThrownBy(() -> deploymentManagement.forceQuitAction(assigningAction.getId())); + .isThrownBy(() -> deploymentManagement.forceQuitAction(assigningActionId)); } @Test @@ -706,8 +708,9 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { final DeploymentRequest targetToDS1 = DeploymentManagement .deploymentRequest(target.getControllerId(), distributionSets.get(1).getId()).setWeight(565).build(); + final List deploymentRequests = List.of(targetToDS0, targetToDS1); Assertions.assertThatExceptionOfType(MultiAssignmentIsNotEnabledException.class) - .isThrownBy(() -> deploymentManagement.assignDistributionSets(Arrays.asList(targetToDS0, targetToDS1))); + .isThrownBy(() -> deploymentManagement.assignDistributionSets(deploymentRequests)); enableMultiAssignments(); assertThat(getResultingActionCount( @@ -981,16 +984,18 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { final DeploymentRequest weightTooHigh = DeploymentManagement.deploymentRequest(targetId, dsId) .setWeight(Action.WEIGHT_MAX + 1).build(); enableMultiAssignments(); + final List deploymentRequestsTooLow = Collections.singletonList(weightTooLow); Assertions.assertThatExceptionOfType(ConstraintViolationException.class) - .isThrownBy(() -> deploymentManagement.assignDistributionSets(Collections.singletonList(weightTooLow))); + .isThrownBy(() -> deploymentManagement.assignDistributionSets(deploymentRequestsTooLow)); + final List deploymentRequestsTooHigh = Collections.singletonList(weightTooHigh); Assertions.assertThatExceptionOfType(ConstraintViolationException.class).isThrownBy( - () -> deploymentManagement.assignDistributionSets(Collections.singletonList(weightTooHigh))); - final Long valideActionId1 = getFirstAssignedAction( + () -> deploymentManagement.assignDistributionSets(deploymentRequestsTooHigh)); + final Long validActionId1 = getFirstAssignedAction( deploymentManagement.assignDistributionSets(Collections.singletonList(valideRequest1)).get(0)).getId(); - final Long valideActionId2 = getFirstAssignedAction( + final Long validActionId2 = getFirstAssignedAction( deploymentManagement.assignDistributionSets(Collections.singletonList(valideRequest2)).get(0)).getId(); - assertThat(actionRepository.findById(valideActionId1).get().getWeight()).get().isEqualTo(Action.WEIGHT_MAX); - assertThat(actionRepository.findById(valideActionId2).get().getWeight()).get().isEqualTo(Action.WEIGHT_MIN); + assertThat(actionRepository.findById(validActionId1).get().getWeight()).get().isEqualTo(Action.WEIGHT_MAX); + assertThat(actionRepository.findById(validActionId2).get().getWeight()).get().isEqualTo(Action.WEIGHT_MIN); } /** @@ -1283,10 +1288,9 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { .asList(DistributionSetSpecification.isDeleted(true), DistributionSetSpecification.isCompleted(true))), PAGE).getContent()).as("wrong size of founded ds").hasSize(noOfDistributionSets); - for (final DistributionSet ignored : deploymentResult.getDistributionSets()) { - testdataFactory.sendUpdateActionStatusToTargets(deploymentResult.getDeployedTargets(), Status.FINISHED, - Collections.singletonList("blabla alles gut")); - } + IntStream.range(0, deploymentResult.getDistributionSets().size()).forEach(i -> testdataFactory.sendUpdateActionStatusToTargets( + deploymentResult.getDeployedTargets(), Status.FINISHED, Collections.singletonList("blabla alles gut"))); + // try to delete again distributionSetManagement.delete(deploymentResult.getDistributionSetIDs()); // verify that the result is the same, even though distributionSet dsA @@ -1304,22 +1308,18 @@ class DeploymentManagementTest extends AbstractJpaIntegrationTest { @Test @Description("Deletes multiple targets and verifies that all related metadata is also deleted.") void deletesTargetsAndVerifyCascadeDeletes() { - final String undeployedTargetPrefix = "undep-T"; final int noOfUndeployedTargets = 2; final String deployedTargetPrefix = "dep-T"; final int noOfDeployedTargets = 4; - final int noOfDistributionSets = 3; final DeploymentResult deploymentResult = prepareComplexRepo(undeployedTargetPrefix, noOfUndeployedTargets, deployedTargetPrefix, noOfDeployedTargets, noOfDistributionSets, "myTestDS"); - for (final DistributionSet ignored : deploymentResult.getDistributionSets()) { - testdataFactory.sendUpdateActionStatusToTargets(deploymentResult.getDeployedTargets(), Status.FINISHED, - Collections.singletonList("blabla alles gut")); - } + IntStream.range(0, deploymentResult.getDistributionSets().size()).forEach(i -> testdataFactory.sendUpdateActionStatusToTargets( + deploymentResult.getDeployedTargets(), Status.FINISHED, Collections.singletonList("blabla alles gut"))); assertThat(targetManagement.count()).as("size of targets is wrong").isNotZero(); assertThat(actionStatusRepository.count()).as("size of action status is wrong").isNotZero(); diff --git a/hawkbit-sdk/hawkbit-sdk-demo/src/main/java/org/eclipse/hawkbit/sdk/demo/device/DeviceApp.java b/hawkbit-sdk/hawkbit-sdk-demo/src/main/java/org/eclipse/hawkbit/sdk/demo/device/DeviceApp.java index a04f0aa6e..62bddd7b7 100644 --- a/hawkbit-sdk/hawkbit-sdk-demo/src/main/java/org/eclipse/hawkbit/sdk/demo/device/DeviceApp.java +++ b/hawkbit-sdk/hawkbit-sdk-demo/src/main/java/org/eclipse/hawkbit/sdk/demo/device/DeviceApp.java @@ -69,6 +69,7 @@ public class DeviceApp { private final DdiController device; private final MgmtApi mgmtApi; + @SuppressWarnings("java:S3358") Shell(final DdiTenant ddiTenant, final MgmtApi mgmtApi, final Optional updateHandler) { this.ddiTenant = ddiTenant; this.mgmtApi = mgmtApi;