Sonar Fixes (9) (#2221)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2025-01-23 15:02:03 +02:00
committed by GitHub
parent 21b901a559
commit a0d149cc1d
12 changed files with 72 additions and 94 deletions

View File

@@ -38,10 +38,10 @@ class DdiArtifactHashTest {
final String sha1Hash = "11111"; final String sha1Hash = "11111";
final String md5Hash = "22222"; final String md5Hash = "22222";
final String sha256Hash = "33333"; final String sha256Hash = "33333";
final DdiArtifactHash DdiArtifact = new DdiArtifactHash(sha1Hash, md5Hash, sha256Hash); final DdiArtifactHash ddiArtifact = new DdiArtifactHash(sha1Hash, md5Hash, sha256Hash);
// Test // Test
final String serializedDdiArtifact = OBJECT_MAPPER.writeValueAsString(DdiArtifact); final String serializedDdiArtifact = OBJECT_MAPPER.writeValueAsString(ddiArtifact);
final DdiArtifactHash deserializedDdiArtifact = OBJECT_MAPPER.readValue(serializedDdiArtifact, DdiArtifactHash.class); final DdiArtifactHash deserializedDdiArtifact = OBJECT_MAPPER.readValue(serializedDdiArtifact, DdiArtifactHash.class);
assertThat(serializedDdiArtifact).contains(sha1Hash, md5Hash, sha256Hash); assertThat(serializedDdiArtifact).contains(sha1Hash, md5Hash, sha256Hash);
assertThat(deserializedDdiArtifact.getSha1()).isEqualTo(sha1Hash); assertThat(deserializedDdiArtifact.getSha1()).isEqualTo(sha1Hash);

View File

@@ -11,7 +11,6 @@ package org.eclipse.hawkbit.ddi.rest.resource;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.stream.Collectors;
import lombok.AccessLevel; import lombok.AccessLevel;
import lombok.NoArgsConstructor; import lombok.NoArgsConstructor;

View File

@@ -341,7 +341,7 @@ class DdiConfirmationBaseTest extends AbstractDDiApiIntegrationTest {
final DdiActivateAutoConfirmation body = new DdiActivateAutoConfirmation(initiator, remark); final DdiActivateAutoConfirmation body = new DdiActivateAutoConfirmation(initiator, remark);
mvc.perform(post(ACTIVATE_AUTO_CONFIRM, tenantAware.getCurrentTenant(), controllerId) 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()) .andDo(MockMvcResultPrinter.print())
.andExpect(status().isOk()); .andExpect(status().isOk());

View File

@@ -626,6 +626,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
.andExpect(status().isNotAcceptable()); .andExpect(status().isNotAcceptable());
} }
@SuppressWarnings("java:S1144") // java:S1144 - used in MethodSource, sonar my find it incorrectly as unused
private static Stream<Action.ActionType> actionTypeForDeployment() { private static Stream<Action.ActionType> actionTypeForDeployment() {
return Stream.of(Action.ActionType.SOFT, Action.ActionType.FORCED); return Stream.of(Action.ActionType.SOFT, Action.ActionType.FORCED);
} }

View File

@@ -62,10 +62,9 @@ class BaseAmqpServiceTest {
@Description("Tests invalid null message content") @Description("Tests invalid null message content")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) }) @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) })
void convertMessageWithNullContent() { void convertMessageWithNullContent() {
final Message message = createMessage("".getBytes()); assertThatExceptionOfType(IllegalArgumentException.class)
assertThatExceptionOfType(MessageConversionException.class) .as("Expected IllegalArgumentException for invalid (null) JSON")
.as("Expected MessageConversionException for invalid JSON") .isThrownBy(() -> createMessage(null));
.isThrownBy(() -> baseAmqpService.convertMessage(message, DmfActionUpdateStatus.class));
} }
@Test @Test
@@ -74,7 +73,7 @@ class BaseAmqpServiceTest {
void updateActionStatusWithEmptyContent() { void updateActionStatusWithEmptyContent() {
final Message message = createMessage("".getBytes()); final Message message = createMessage("".getBytes());
assertThatExceptionOfType(MessageConversionException.class) assertThatExceptionOfType(MessageConversionException.class)
.as("Expected MessageConversionException for invalid JSON") .as("Expected MessageConversionException for invalid (empty) JSON")
.isThrownBy(() -> baseAmqpService.convertMessage(message, DmfActionUpdateStatus.class)); .isThrownBy(() -> baseAmqpService.convertMessage(message, DmfActionUpdateStatus.class));
} }

View File

@@ -12,32 +12,27 @@ package org.eclipse.hawkbit.repository.jpa;
import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManager;
import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider; 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.jpa.repository.support.JpaRepositoryFactoryBean;
import org.springframework.data.repository.Repository; import org.springframework.data.repository.Repository;
import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.core.support.RepositoryFactorySupport;
/** /**
* A {@link JpaRepositoryFactoryBean} extension that allow injection of custom * A {@link JpaRepositoryFactoryBean} extension that allow injection of custom repository factories by using a
* repository factories by using a {@link BaseRepositoryTypeProvider} * {@link BaseRepositoryTypeProvider} implementation, allows injecting different base repository implementations based on repository type
* implementation, allows injecting different base repository implementations based on repository type
*
* @param <T>
* @param <S>
* @param <ID>
*/ */
@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> { public class CustomBaseRepositoryFactoryBean<T extends Repository<S, ID>, S, ID> extends JpaRepositoryFactoryBean<T, S, ID> {
@Autowired private final BaseRepositoryTypeProvider baseRepoProvider;
BaseRepositoryTypeProvider baseRepoProvider;
/** /**
* Creates a new {@link JpaRepositoryFactoryBean} for the given repository interface. * Creates a new {@link JpaRepositoryFactoryBean} for the given repository interface.
* *
* @param repositoryInterface must not be {@literal null}. * @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); super(repositoryInterface);
this.baseRepoProvider = baseRepoProvider;
} }
@Override @Override

View File

@@ -927,7 +927,7 @@ public class RepositoryApplicationConfiguration {
@ConditionalOnProperty(prefix = "hawkbit.rollout.scheduler", name = "enabled", matchIfMissing = true) @ConditionalOnProperty(prefix = "hawkbit.rollout.scheduler", name = "enabled", matchIfMissing = true)
RolloutScheduler rolloutScheduler(final SystemManagement systemManagement, RolloutScheduler rolloutScheduler(final SystemManagement systemManagement,
final RolloutHandler rolloutHandler, final SystemSecurityContext systemSecurityContext) { final RolloutHandler rolloutHandler, final SystemSecurityContext systemSecurityContext) {
return new RolloutScheduler(systemManagement, rolloutHandler, systemSecurityContext); return new RolloutScheduler(rolloutHandler, systemManagement, systemSecurityContext);
} }
/** /**

View File

@@ -118,11 +118,11 @@ public interface BaseEntityRepository<T extends AbstractJpaTenantAwareBaseEntity
} }
} }
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); 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; final Collection<Long> collection;
if (ids instanceof Collection<Long> idCollection) { if (ids instanceof Collection<Long> idCollection) {
collection = idCollection; collection = idCollection;

View File

@@ -92,7 +92,8 @@ public class BaseEntityRepositoryACM<T extends AbstractJpaTenantAwareBaseEntity>
@Override @Override
public void deleteAllById(@NonNull final Iterable<? extends Long> ids) { 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()) { if (count(AccessController.Operation.DELETE, byIdsSpec(idList)) != idList.size()) {
throw new InsufficientPermissionException("Has at least one id that is not allowed for deletion!"); throw new InsufficientPermissionException("Has at least one id that is not allowed for deletion!");
} }
@@ -342,54 +343,51 @@ public class BaseEntityRepositoryACM<T extends AbstractJpaTenantAwareBaseEntity>
repository.getClass().getInterfaces(), repository.getClass().getInterfaces(),
(proxy, method, args) -> { (proxy, method, args) -> {
try { try {
try { // try to call a BaseEntityRepositoryACM method if declared
// TODO - cache mapping so to speed things // TODO - cache mapping so to speed things
final Method delegateMethod = final Method delegateMethod =
BaseEntityRepositoryACM.class.getDeclaredMethod(method.getName(), method.getParameterTypes()); BaseEntityRepositoryACM.class.getDeclaredMethod(method.getName(), method.getParameterTypes());
return delegateMethod.invoke(repositoryACM, args); 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);
}
}
} 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);
}
} catch (final InvocationTargetException e) { } catch (final InvocationTargetException e) {
throw e.getCause() == null ? e : e.getCause(); throw e.getCause() == null ? e : e.getCause();
} catch (final NoSuchMethodException nsme) {
// not found
try {
// 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);
}
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();
}
} }
}); });
log.info("Proxy created -> {}", acmProxy); log.info("Proxy created -> {}", acmProxy);
return 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;
}
} }

View File

@@ -26,49 +26,31 @@ public class RolloutScheduler {
private static final String PROP_SCHEDULER_DELAY_PLACEHOLDER = "${hawkbit.rollout.scheduler.fixedDelay:2000}"; private static final String PROP_SCHEDULER_DELAY_PLACEHOLDER = "${hawkbit.rollout.scheduler.fixedDelay:2000}";
private final SystemManagement systemManagement; private final SystemManagement systemManagement;
private final RolloutHandler rolloutHandler; private final RolloutHandler rolloutHandler;
private final SystemSecurityContext systemSecurityContext; private final SystemSecurityContext systemSecurityContext;
/** public RolloutScheduler(
* Constructor. final RolloutHandler rolloutHandler, final SystemManagement systemManagement, final SystemSecurityContext systemSecurityContext) {
*
* @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) {
this.systemManagement = systemManagement; this.systemManagement = systemManagement;
this.rolloutHandler = rolloutHandler; this.rolloutHandler = rolloutHandler;
this.systemSecurityContext = systemSecurityContext; this.systemSecurityContext = systemSecurityContext;
} }
/** /**
* Scheduler method called by the spring-async mechanism. Retrieves all * Scheduler method called by the spring-async mechanism. Retrieves all tenants from the {@link SystemManagement#findTenants} and
* tenants from the {@link SystemManagement#findTenants} and runs for each * runs for each tenant the {@link RolloutHandler#handleAll()} in the {@link SystemSecurityContext}.
* tenant the {@link RolloutHandler#handleAll()} in the
* {@link SystemSecurityContext}.
*/ */
@Scheduled(initialDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER, fixedDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER) @Scheduled(initialDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER, fixedDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER)
public void runningRolloutScheduler() { public void runningRolloutScheduler() {
log.debug("rollout schedule checker has been triggered."); log.debug("rollout schedule checker has been triggered.");
// run this code in system code privileged to have the necessary // run this code in system code privileged to have the necessary permission to query and create entities.
// permission to query and create entities.
systemSecurityContext.runAsSystem(() -> { systemSecurityContext.runAsSystem(() -> {
// workaround eclipselink that is currently not possible to // workaround eclipselink that is currently not possible to execute a query without multi-tenancy if MultiTenant
// 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
// annotation is used. // check for each tenant separately.
// https://bugs.eclipse.org/bugs/show_bug.cgi?id=355458. So
// iterate through all tenants and execute the rollout check for
// each tenant seperately.
systemManagement.forEachTenant(tenant -> rolloutHandler.handleAll()); systemManagement.forEachTenant(tenant -> rolloutHandler.handleAll());
return null; return null;
}); });
} }
}
}

View File

@@ -47,6 +47,7 @@ public class Amqp {
return vHosts.computeIfAbsent(vHost, vh -> new VHost(getConnectionFactory(dmf, vHost), amqpProperties, 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) { private ConnectionFactory getConnectionFactory(final DMF dmf, final String vHost) {
final CachingConnectionFactory connectionFactory = new CachingConnectionFactory(); final CachingConnectionFactory connectionFactory = new CachingConnectionFactory();
connectionFactory.setHost(rabbitProperties.getHost()); connectionFactory.setHost(rabbitProperties.getHost());

View File

@@ -9,6 +9,9 @@
*/ */
package org.eclipse.hawkbit.ui.simple.view; 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 { public interface Constants {
// properties // properties