Sonar Fixes (9) (eclipse-hawkbit#2221)

Signed-off-by: Avgustin Marinov <[email protected]>
bosch-io · Jan 23, 2025 · a0d149c · a0d149c
1 parent 21b901a
commit a0d149c
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 91 deletions.
diff --git a/...hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiArtifactHashTest.java b/...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/...di-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DataConversionHelper.java b/...di-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/...resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfirmationBaseTest.java b/...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/...di-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java b/...di-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<Action.ActionType> 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
@@ -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/...jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java b/...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 <T>
- * @param <S>
- * @param <ID>
+ * 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<T extends Repository<S, ID>, S, ID> extends JpaRepositoryFactoryBean<T, S, ID> {
 
-    @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<? extends T> repositoryInterface) {
+    public CustomBaseRepositoryFactoryBean(final Class<? extends T> repositoryInterface, final BaseRepositoryTypeProvider baseRepoProvider) {
         super(repositoryInterface);
+        this.baseRepoProvider = baseRepoProvider;
     }
 
     @Override

diff --git a/.../src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java b/.../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/...jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java b/...jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepository.java
@@ -118,11 +118,11 @@ default BaseEntityRepository<T> withACM(@Nullable final AccessController<T> acce
         }
     }
 
-    default <T extends AbstractJpaTenantAwareBaseEntity> Specification<T> byIdSpec(final Long id) {
+    default <S extends AbstractJpaTenantAwareBaseEntity> Specification<S> byIdSpec(final Long id) {
         return (root, query, cb) -> cb.equal(root.get(AbstractJpaBaseEntity_.id), id);
     }
 
-    default <T extends AbstractJpaTenantAwareBaseEntity> Specification<T> byIdsSpec(final Iterable<Long> ids) {
+    default <S extends AbstractJpaTenantAwareBaseEntity> Specification<S> byIdsSpec(final Iterable<Long> ids) {
         final Collection<Long> collection;
         if (ids instanceof Collection<Long> idCollection) {
             collection = idCollection;

diff --git a/.../src/main/java/org/eclipse/hawkbit/repository/jpa/repository/BaseEntityRepositoryACM.java b/.../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<? extends Long> ids) {
-        final Set<Long> idList = toSetDistinct(ids);
+        final Set<Long> 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 <T extends AbstractJpaTenantAwareBaseEntity, R extends BaseEntityReposito
                 repository.getClass().getInterfaces(),
                 (proxy, method, args) -> {
                     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<T>) 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<T>) 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<Long> toSetDistinct(final Iterable<? extends Long> i) {
-        final Set<Long> set = new HashSet<>();
-        i.forEach(set::add);
-        return set;
-    }
 }
diff --git a/...sitory-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java b/...sitory-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;
         });
     }
-
-}
+}
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
@@ -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
@@ -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