From a0d149cc1d73b37896c23ba50731ebbc2cda3c7f Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Thu, 23 Jan 2025 15:02:03 +0200 Subject: [PATCH] Sonar Fixes (9) (#2221) Signed-off-by: Avgustin Marinov --- .../ddi/json/model/DdiArtifactHashTest.java | 4 +- .../rest/resource/DataConversionHelper.java | 1 - .../resource/DdiConfirmationBaseTest.java | 2 +- .../rest/resource/DdiInstalledBaseTest.java | 1 + .../hawkbit/amqp/BaseAmqpServiceTest.java | 9 +-- .../jpa/CustomBaseRepositoryFactoryBean.java | 17 ++-- .../RepositoryApplicationConfiguration.java | 2 +- .../jpa/repository/BaseEntityRepository.java | 4 +- .../repository/BaseEntityRepositoryACM.java | 80 +++++++++---------- .../jpa/rollout/RolloutScheduler.java | 36 +++------ .../eclipse/hawkbit/sdk/dmf/amqp/Amqp.java | 1 + .../hawkbit/ui/simple/view/Constants.java | 3 + 12 files changed, 69 insertions(+), 91 deletions(-) diff --git a/hawkbit-ddi/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiArtifactHashTest.java b/hawkbit-ddi/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiArtifactHashTest.java index 77ccbba70f..ef771b9924 100644 --- a/hawkbit-ddi/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiArtifactHashTest.java +++ b/hawkbit-ddi/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiArtifactHashTest.java @@ -38,10 +38,10 @@ void shouldSerializeAndDeserializeObject() throws IOException { final String sha1Hash = "11111"; final String md5Hash = "22222"; final String sha256Hash = "33333"; - final DdiArtifactHash DdiArtifact = new DdiArtifactHash(sha1Hash, md5Hash, sha256Hash); + final DdiArtifactHash ddiArtifact = new DdiArtifactHash(sha1Hash, md5Hash, sha256Hash); // Test - final String serializedDdiArtifact = OBJECT_MAPPER.writeValueAsString(DdiArtifact); + final String serializedDdiArtifact = OBJECT_MAPPER.writeValueAsString(ddiArtifact); final DdiArtifactHash deserializedDdiArtifact = OBJECT_MAPPER.readValue(serializedDdiArtifact, DdiArtifactHash.class); assertThat(serializedDdiArtifact).contains(sha1Hash, md5Hash, sha256Hash); assertThat(deserializedDdiArtifact.getSha1()).isEqualTo(sha1Hash); diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DataConversionHelper.java b/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DataConversionHelper.java index e53c19df29..a77ace12d4 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DataConversionHelper.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DataConversionHelper.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import lombok.AccessLevel; import lombok.NoArgsConstructor; diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfirmationBaseTest.java b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfirmationBaseTest.java index ae4a148295..3fda6b9a2c 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfirmationBaseTest.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfirmationBaseTest.java @@ -341,7 +341,7 @@ void activateAutoConfirmation(final String initiator, final String remark) throw final DdiActivateAutoConfirmation body = new DdiActivateAutoConfirmation(initiator, remark); mvc.perform(post(ACTIVATE_AUTO_CONFIRM, tenantAware.getCurrentTenant(), controllerId) - .content(getMapper().writeValueAsString(body)).contentType(MediaType.APPLICATION_JSON_UTF8)) + .content(getMapper().writeValueAsString(body)).contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java index b22aaadf4e..e0993ad1ca 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java @@ -626,6 +626,7 @@ public void badInstalledAction() throws Exception { .andExpect(status().isNotAcceptable()); } + @SuppressWarnings("java:S1144") // java:S1144 - used in MethodSource, sonar my find it incorrectly as unused private static Stream actionTypeForDeployment() { return Stream.of(Action.ActionType.SOFT, Action.ActionType.FORCED); } diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/BaseAmqpServiceTest.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/BaseAmqpServiceTest.java index d8b601a50f..5c4ccc4f71 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/BaseAmqpServiceTest.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/amqp/BaseAmqpServiceTest.java @@ -62,10 +62,9 @@ void convertMessageTest() { @Description("Tests invalid null message content") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) }) void convertMessageWithNullContent() { - final Message message = createMessage("".getBytes()); - assertThatExceptionOfType(MessageConversionException.class) - .as("Expected MessageConversionException for invalid JSON") - .isThrownBy(() -> baseAmqpService.convertMessage(message, DmfActionUpdateStatus.class)); + assertThatExceptionOfType(IllegalArgumentException.class) + .as("Expected IllegalArgumentException for invalid (null) JSON") + .isThrownBy(() -> createMessage(null)); } @Test @@ -74,7 +73,7 @@ void convertMessageWithNullContent() { void updateActionStatusWithEmptyContent() { final Message message = createMessage("".getBytes()); assertThatExceptionOfType(MessageConversionException.class) - .as("Expected MessageConversionException for invalid JSON") + .as("Expected MessageConversionException for invalid (empty) JSON") .isThrownBy(() -> baseAmqpService.convertMessage(message, DmfActionUpdateStatus.class)); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java index fbbd8b4233..bcea738b29 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java @@ -12,32 +12,27 @@ import jakarta.persistence.EntityManager; import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.jpa.repository.support.JpaRepositoryFactoryBean; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.RepositoryFactorySupport; /** - * A {@link JpaRepositoryFactoryBean} extension that allow injection of custom - * repository factories by using a {@link BaseRepositoryTypeProvider} - * implementation, allows injecting different base repository implementations based on repository type - * - * @param - * @param - * @param + * A {@link JpaRepositoryFactoryBean} extension that allow injection of custom repository factories by using a + * {@link BaseRepositoryTypeProvider} implementation, allows injecting different base repository implementations based on repository type */ +@SuppressWarnings("java:S119") // java:S119 - ID is inherited from JpaRepositoryFactoryBean public class CustomBaseRepositoryFactoryBean, S, ID> extends JpaRepositoryFactoryBean { - @Autowired - BaseRepositoryTypeProvider baseRepoProvider; + private final BaseRepositoryTypeProvider baseRepoProvider; /** * Creates a new {@link JpaRepositoryFactoryBean} for the given repository interface. * * @param repositoryInterface must not be {@literal null}. */ - public CustomBaseRepositoryFactoryBean(final Class repositoryInterface) { + public CustomBaseRepositoryFactoryBean(final Class repositoryInterface, final BaseRepositoryTypeProvider baseRepoProvider) { super(repositoryInterface); + this.baseRepoProvider = baseRepoProvider; } @Override diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java index ea20af4d2e..f86a4cdbeb 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java @@ -927,7 +927,7 @@ AutoCleanupScheduler autoCleanupScheduler(final SystemManagement systemManagemen @ConditionalOnProperty(prefix = "hawkbit.rollout.scheduler", name = "enabled", matchIfMissing = true) RolloutScheduler rolloutScheduler(final SystemManagement systemManagement, final RolloutHandler rolloutHandler, final SystemSecurityContext systemSecurityContext) { - return new RolloutScheduler(systemManagement, rolloutHandler, systemSecurityContext); + return new RolloutScheduler(rolloutHandler, systemManagement, systemSecurityContext); } /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java index cdddd9ef7f..a1f046d49a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java @@ -118,11 +118,11 @@ default BaseEntityRepository withACM(@Nullable final AccessController acce } } - default Specification byIdSpec(final Long id) { + default Specification byIdSpec(final Long id) { return (root, query, cb) -> cb.equal(root.get(AbstractJpaBaseEntity_.id), id); } - default Specification byIdsSpec(final Iterable ids) { + default Specification byIdsSpec(final Iterable ids) { final Collection collection; if (ids instanceof Collection idCollection) { collection = idCollection; diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepositoryACM.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepositoryACM.java index eda12fdbaf..b42a58d7c8 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepositoryACM.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepositoryACM.java @@ -92,7 +92,8 @@ public void delete(@NonNull final T entity) { @Override public void deleteAllById(@NonNull final Iterable ids) { - final Set idList = toSetDistinct(ids); + final Set idList = new HashSet<>(); + ids.forEach(idList::add); if (count(AccessController.Operation.DELETE, byIdsSpec(idList)) != idList.size()) { throw new InsufficientPermissionException("Has at least one id that is not allowed for deletion!"); } @@ -342,54 +343,51 @@ static { try { + // try to call a BaseEntityRepositoryACM method if declared + // TODO - cache mapping so to speed things + final Method delegateMethod = + BaseEntityRepositoryACM.class.getDeclaredMethod(method.getName(), method.getParameterTypes()); + return delegateMethod.invoke(repositoryACM, args); + } catch (final InvocationTargetException e) { + throw e.getCause() == null ? e : e.getCause(); + } catch (final NoSuchMethodException nsme) { + // not found try { - // TODO - cache mapping so to speed things - final Method delegateMethod = - BaseEntityRepositoryACM.class.getDeclaredMethod(method.getName(), method.getParameterTypes()); - return delegateMethod.invoke(repositoryACM, args); - } catch (final NoSuchMethodException e) { - // call to repository itself - } - if (method.getName().startsWith("find") || method.getName().startsWith("get")) { - final Object result = method.invoke(repository, args); - // Iterable, List, Page, Slice - if (Iterable.class.isAssignableFrom(method.getReturnType())) { - for (final Object e : (Iterable) result) { - if (repository.getDomainClass().isAssignableFrom(e.getClass())) { - accessController.assertOperationAllowed(AccessController.Operation.READ, (T) e); + // call to call a repository method + if (method.getName().startsWith("find") || method.getName().startsWith("get")) { + final Object result = method.invoke(repository, args); + // Iterable, List, Page, Slice + if (Iterable.class.isAssignableFrom(method.getReturnType())) { + for (final Object e : (Iterable) result) { + if (repository.getDomainClass().isAssignableFrom(e.getClass())) { + accessController.assertOperationAllowed(AccessController.Operation.READ, (T) e); + } } + } else if (Optional.class.isAssignableFrom(method.getReturnType()) && ((Optional) result) + .filter(value -> repository.getDomainClass().isAssignableFrom(value.getClass())) + .isPresent()) { + return ((Optional) result).filter( + t -> { + // if not accessible - throws exception (as for iterables or single entities) + accessController.assertOperationAllowed(AccessController.Operation.READ, t); + return true; + }); + } else if (repository.getDomainClass().isAssignableFrom(method.getReturnType())) { + accessController.assertOperationAllowed(AccessController.Operation.READ, (T) result); } - } else if (Optional.class.isAssignableFrom(method.getReturnType()) && ((Optional) result) - .filter(value -> repository.getDomainClass().isAssignableFrom(value.getClass())) - .isPresent()) { - return ((Optional) result).filter( - t -> { - // if not accessible - throws exception (as for iterables or single entities) - accessController.assertOperationAllowed(AccessController.Operation.READ, t); - return true; - }); - } else if (repository.getDomainClass().isAssignableFrom(method.getReturnType())) { - accessController.assertOperationAllowed(AccessController.Operation.READ, (T) result); + return result; + } else if ("toString".equals(method.getName()) && method.getParameterCount() == 0) { + return BaseEntityRepositoryACM.class.getSimpleName() + + "(repository: " + repository + ", accessController: " + accessController + ")"; + } else { + return method.invoke(repository, args); } - return result; - } else if ("toString".equals(method.getName()) && method.getParameterCount() == 0) { - return BaseEntityRepositoryACM.class.getSimpleName() + - "(repository: " + repository + ", accessController: " + accessController + ")"; - } else { - return method.invoke(repository, args); + } catch (final InvocationTargetException ite) { + throw ite.getCause() == null ? ite : ite.getCause(); } - } catch (final InvocationTargetException e) { - throw e.getCause() == null ? e : e.getCause(); } }); log.info("Proxy created -> {}", acmProxy); return acmProxy; } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - private static Set toSetDistinct(final Iterable i) { - final Set set = new HashSet<>(); - i.forEach(set::add); - return set; - } } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java index 05d38018e3..7b69dcf549 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java @@ -26,49 +26,31 @@ public class RolloutScheduler { private static final String PROP_SCHEDULER_DELAY_PLACEHOLDER = "${hawkbit.rollout.scheduler.fixedDelay:2000}"; private final SystemManagement systemManagement; - private final RolloutHandler rolloutHandler; - private final SystemSecurityContext systemSecurityContext; - /** - * Constructor. - * - * @param systemManagement to find all tenants - * @param rolloutHandler to run the rollout handler - * @param systemSecurityContext to run as system - */ - public RolloutScheduler(final SystemManagement systemManagement, final RolloutHandler rolloutHandler, - final SystemSecurityContext systemSecurityContext) { + public RolloutScheduler( + final RolloutHandler rolloutHandler, final SystemManagement systemManagement, final SystemSecurityContext systemSecurityContext) { this.systemManagement = systemManagement; this.rolloutHandler = rolloutHandler; this.systemSecurityContext = systemSecurityContext; } /** - * Scheduler method called by the spring-async mechanism. Retrieves all - * tenants from the {@link SystemManagement#findTenants} and runs for each - * tenant the {@link RolloutHandler#handleAll()} in the - * {@link SystemSecurityContext}. + * Scheduler method called by the spring-async mechanism. Retrieves all tenants from the {@link SystemManagement#findTenants} and + * runs for each tenant the {@link RolloutHandler#handleAll()} in the {@link SystemSecurityContext}. */ @Scheduled(initialDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER, fixedDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER) public void runningRolloutScheduler() { log.debug("rollout schedule checker has been triggered."); - // run this code in system code privileged to have the necessary - // permission to query and create entities. + // run this code in system code privileged to have the necessary permission to query and create entities. systemSecurityContext.runAsSystem(() -> { - // workaround eclipselink that is currently not possible to - // execute a query without multi-tenancy if MultiTenant - // annotation is used. - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=355458. So - // iterate through all tenants and execute the rollout check for - // each tenant seperately. - + // workaround eclipselink that is currently not possible to execute a query without multi-tenancy if MultiTenant + // annotation is used. https://bugs.eclipse.org/bugs/show_bug.cgi?id=355458. So, iterate through all tenants and execute the rollout + // check for each tenant separately. systemManagement.forEachTenant(tenant -> rolloutHandler.handleAll()); - return null; }); } - -} +} \ No newline at end of file diff --git a/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/Amqp.java b/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/Amqp.java index 0b527107df..ddf087bad7 100644 --- a/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/Amqp.java +++ b/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/Amqp.java @@ -47,6 +47,7 @@ public VHost getVhost(final DMF dmf, final boolean initVHost) { return vHosts.computeIfAbsent(vHost, vh -> new VHost(getConnectionFactory(dmf, vHost), amqpProperties, initVHost)); } + @SuppressWarnings("java:S4449") // java:S4449 - setUsername/Password is called with non-null private ConnectionFactory getConnectionFactory(final DMF dmf, final String vHost) { final CachingConnectionFactory connectionFactory = new CachingConnectionFactory(); connectionFactory.setHost(rabbitProperties.getHost()); diff --git a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/Constants.java b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/Constants.java index eb341b8cba..83814adfbe 100644 --- a/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/Constants.java +++ b/hawkbit-simple-ui/src/main/java/org/eclipse/hawkbit/ui/simple/view/Constants.java @@ -9,6 +9,9 @@ */ package org.eclipse.hawkbit.ui.simple.view; +// java:S1214 - implementations of Constants interface extends other classes, so if make this class we shall go for static imports +// which is not not better +@SuppressWarnings("java:S1214") public interface Constants { // properties