Fix some sonar findings (#2149)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-12-17 09:18:32 +02:00
committed by GitHub
parent db3ac7f2dd
commit c684b03249
6 changed files with 37 additions and 139 deletions

View File

@@ -11,6 +11,7 @@ package org.eclipse.hawkbit.repository;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import jakarta.validation.ConstraintViolationException;
import jakarta.validation.Valid;
@@ -39,7 +40,6 @@ import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Slice;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.util.concurrent.ListenableFuture;
/**
* RolloutManagement to control rollouts e.g. like creating, starting, resuming
@@ -184,8 +184,8 @@ public interface RolloutManagement {
* {@link RolloutGroupCreate} for field constraints.
*/
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_READ_AND_TARGET_READ)
ListenableFuture<RolloutGroupsValidation> validateTargetsInGroups(@Valid List<RolloutGroupCreate> groups,
String targetFilter, Long createdAt, @NotNull Long dsTypeId);
CompletableFuture<RolloutGroupsValidation> validateTargetsInGroups(
@Valid List<RolloutGroupCreate> groups, String targetFilter, Long createdAt, @NotNull Long dsTypeId);
/**
* Retrieves all rollouts.

View File

@@ -41,6 +41,7 @@ import jakarta.persistence.criteria.CriteriaQuery;
import jakarta.persistence.criteria.Root;
import jakarta.validation.constraints.NotEmpty;
import lombok.Data;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.collections4.ListUtils;
import org.eclipse.hawkbit.repository.ConfirmationManagement;
@@ -222,13 +223,11 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
final JpaActionStatus actionStatus = create.build();
switch (actionStatus.getStatus()) {
case CANCELED:
case FINISHED: {
case CANCELED, FINISHED: {
handleFinishedCancelation(actionStatus, action);
break;
}
case ERROR:
case CANCEL_REJECTED: {
case ERROR, CANCEL_REJECTED: {
// Cancellation rejected. Back to running.
action.setStatus(Status.RUNNING);
break;
@@ -320,14 +319,14 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
@Override
@Transactional(isolation = Isolation.READ_COMMITTED)
@Retryable(retryFor = ConcurrencyFailureException.class, exclude = EntityAlreadyExistsException.class, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
@Retryable(retryFor = ConcurrencyFailureException.class, noRetryFor = EntityAlreadyExistsException.class, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Target findOrRegisterTargetIfItDoesNotExist(final String controllerId, final URI address) {
return findOrRegisterTargetIfItDoesNotExist(controllerId, address, null, null);
}
@Override
@Transactional(isolation = Isolation.READ_COMMITTED)
@Retryable(retryFor = ConcurrencyFailureException.class, exclude = EntityAlreadyExistsException.class, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
@Retryable(retryFor = ConcurrencyFailureException.class, noRetryFor = EntityAlreadyExistsException.class, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Target findOrRegisterTargetIfItDoesNotExist(final String controllerId, final URI address, final String name, final String type) {
final Specification<JpaTarget> spec = (targetRoot, query, cb) -> cb.equal(targetRoot.get(JpaTarget_.controllerId), controllerId);
return targetRepository.findOne(spec).map(target -> updateTarget(target, address, name, type))
@@ -603,10 +602,6 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
this.targetRepository = targetRepositorySpy;
}
private static String formatQueryInStatementParams(final Collection<String> paramNames) {
return "#" + String.join(",#", paramNames);
}
private static boolean isAddressChanged(final URI addressToUpdate, final URI address) {
return addressToUpdate == null || !addressToUpdate.equals(address);
}
@@ -702,7 +697,7 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
log.debug("Run flushUpdateQueue.");
final int size = queue.size();
if (size <= 0) {
if (size == 0) {
return;
}
@@ -733,7 +728,7 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
log.debug("Persist {} targetqueries.", polls.size());
final List<List<String>> pollChunks = ListUtils.partition(
polls.stream().map(TargetPoll::getControllerId).collect(Collectors.toList()),
polls.stream().map(TargetPoll::getControllerId).toList(),
Constants.MAX_ENTRIES_IN_STATEMENT);
pollChunks.forEach(chunk -> {
@@ -1034,6 +1029,7 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
}
}
@Data
private static class TargetPoll {
private final String tenant;
@@ -1043,52 +1039,5 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
this.tenant = target.getTenant();
this.controllerId = target.getControllerId();
}
public String getTenant() {
return tenant;
}
public String getControllerId() {
return controllerId;
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + (controllerId == null ? 0 : controllerId.hashCode());
result = prime * result + (tenant == null ? 0 : tenant.hashCode());
return result;
}
@Override
public boolean equals(final Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
final TargetPoll other = (TargetPoll) obj;
if (controllerId == null) {
if (other.controllerId != null) {
return false;
}
} else if (!controllerId.equals(other.controllerId)) {
return false;
}
if (tenant == null) {
if (other.tenant != null) {
return false;
}
} else if (!tenant.equals(other.tenant)) {
return false;
}
return true;
}
}
}

View File

@@ -19,9 +19,7 @@ import java.util.stream.Collectors;
import jakarta.persistence.EntityManager;
import jakarta.persistence.criteria.CriteriaBuilder;
import jakarta.persistence.criteria.CriteriaQuery;
import jakarta.persistence.criteria.Join;
import jakarta.persistence.criteria.ListJoin;
import jakarta.persistence.criteria.Order;
import jakarta.persistence.criteria.Predicate;
import jakarta.persistence.criteria.Root;
@@ -31,7 +29,6 @@ import org.eclipse.hawkbit.repository.RolloutStatusCache;
import org.eclipse.hawkbit.repository.TargetFields;
import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
import org.eclipse.hawkbit.repository.jpa.JpaManagementHelper;
import org.eclipse.hawkbit.repository.jpa.model.JpaAction;
import org.eclipse.hawkbit.repository.jpa.model.JpaRolloutGroup;
import org.eclipse.hawkbit.repository.jpa.model.JpaRolloutGroup_;
import org.eclipse.hawkbit.repository.jpa.model.JpaRollout_;
@@ -55,9 +52,7 @@ import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.data.jpa.repository.query.QueryUtils;
import org.springframework.orm.jpa.vendor.Database;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
@@ -106,17 +101,13 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
throwEntityNotFoundExceptionIfRolloutDoesNotExist(rolloutId);
final Page<JpaRolloutGroup> rolloutGroups = rolloutGroupRepository.findByRolloutId(rolloutId, pageable);
final List<Long> rolloutGroupIds = rolloutGroups.getContent().stream().map(RolloutGroup::getId)
.collect(Collectors.toList());
final List<Long> rolloutGroupIds = rolloutGroups.getContent().stream().map(RolloutGroup::getId).toList();
if (rolloutGroupIds.isEmpty()) {
// groups might have been already deleted, so return empty list.
return new PageImpl<>(Collections.emptyList());
}
final Map<Long, List<TotalTargetCountActionStatus>> allStatesForRollout = getStatusCountItemForRolloutGroup(
rolloutGroupIds);
final Map<Long, List<TotalTargetCountActionStatus>> allStatesForRollout = getStatusCountItemForRolloutGroup(rolloutGroupIds);
for (final JpaRolloutGroup rolloutGroup : rolloutGroups) {
final TotalTargetCountStatus totalTargetCountStatus = new TotalTargetCountStatus(
allStatesForRollout.get(rolloutGroup.getId()), (long) rolloutGroup.getTotalTargets(),
@@ -149,20 +140,16 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
throwEntityNotFoundExceptionIfRolloutDoesNotExist(rolloutId);
final Page<RolloutGroup> rolloutGroups = findByRolloutAndRsql(rolloutId, rsqlParam, pageable);
final List<Long> rolloutGroupIds = rolloutGroups.getContent().stream().map(RolloutGroup::getId)
.collect(Collectors.toList());
final List<Long> rolloutGroupIds = rolloutGroups.getContent().stream().map(RolloutGroup::getId).toList();
if (rolloutGroupIds.isEmpty()) {
// groups might already deleted, so return empty list.
// groups might already have been deleted, so return empty list.
return new PageImpl<>(Collections.emptyList());
}
final Map<Long, List<TotalTargetCountActionStatus>> allStatesForRollout = getStatusCountItemForRolloutGroup(
rolloutGroupIds);
final Map<Long, List<TotalTargetCountActionStatus>> allStatesForRollout = getStatusCountItemForRolloutGroup(rolloutGroupIds);
for (final RolloutGroup rolloutGroup : rolloutGroups) {
final TotalTargetCountStatus totalTargetCountStatus = new TotalTargetCountStatus(
allStatesForRollout.get(rolloutGroup.getId()), Long.valueOf(rolloutGroup.getTotalTargets()),
allStatesForRollout.get(rolloutGroup.getId()), (long) rolloutGroup.getTotalTargets(),
rolloutGroup.getRollout().getActionType());
((JpaRolloutGroup) rolloutGroup).setTotalTargetCountStatus(totalTargetCountStatus);
}
@@ -220,8 +207,7 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
@Override
public Optional<RolloutGroup> getWithDetailedStatus(final long rolloutGroupId) {
final Optional<RolloutGroup> rolloutGroup = get(rolloutGroupId);
if (!rolloutGroup.isPresent()) {
if (rolloutGroup.isEmpty()) {
return rolloutGroup;
}
@@ -235,8 +221,8 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
rolloutStatusCache.putRolloutGroupStatus(rolloutGroupId, rolloutStatusCountItems);
}
final TotalTargetCountStatus totalTargetCountStatus = new TotalTargetCountStatus(rolloutStatusCountItems,
Long.valueOf(jpaRolloutGroup.getTotalTargets()), jpaRolloutGroup.getRollout().getActionType());
final TotalTargetCountStatus totalTargetCountStatus = new TotalTargetCountStatus(
rolloutStatusCountItems, (long) jpaRolloutGroup.getTotalTargets(), jpaRolloutGroup.getRollout().getActionType());
jpaRolloutGroup.setTotalTargetCountStatus(totalTargetCountStatus);
return rolloutGroup;
@@ -268,9 +254,7 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
final Map<Long, List<TotalTargetCountActionStatus>> fromCache = rolloutStatusCache
.getRolloutGroupStatus(groupIds);
final List<Long> rolloutGroupIds = groupIds.stream().filter(id -> !fromCache.containsKey(id))
.collect(Collectors.toList());
final List<Long> rolloutGroupIds = groupIds.stream().filter(id -> !fromCache.containsKey(id)).toList();
if (!rolloutGroupIds.isEmpty()) {
final List<TotalTargetCountActionStatus> resultList = actionRepository
.getStatusCountByRolloutGroupIds(rolloutGroupIds);
@@ -291,30 +275,9 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
rolloutGroupId);
}
private List<Order> getOrderBy(final Pageable pageRequest, final CriteriaBuilder cb,
final Join<RolloutTargetGroup, JpaTarget> targetJoin,
final ListJoin<RolloutTargetGroup, JpaAction> actionJoin) {
return pageRequest.getSort().get().flatMap(order -> {
final List<Order> orders;
final String property = order.getProperty();
// we consider status, last_action_status_code as property from
// JpaAction ...
if ("status".equals(property) || "lastActionStatusCode".equals(property)) {
orders = QueryUtils.toOrders(Sort.by(order.getDirection(), property), actionJoin, cb);
}
// ... and every other property from JpaTarget
else {
orders = QueryUtils.toOrders(Sort.by(order.getDirection(), property), targetJoin, cb);
}
return orders.stream();
}).collect(Collectors.toList());
}
private void throwExceptionIfRolloutGroupDoesNotExist(final Long rolloutGroupId) {
if (!rolloutGroupRepository.existsById(rolloutGroupId)) {
throw new EntityNotFoundException(RolloutGroup.class, rolloutGroupId);
}
}
}
}

View File

@@ -18,6 +18,7 @@ import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -89,10 +90,8 @@ import org.springframework.orm.jpa.vendor.Database;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Retryable;
import org.springframework.scheduling.annotation.Async;
import org.springframework.scheduling.annotation.AsyncResult;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
import org.springframework.util.concurrent.ListenableFuture;
import org.springframework.validation.annotation.Validated;
/**
@@ -229,8 +228,8 @@ public class JpaRolloutManagement implements RolloutManagement {
@Override
@Async
public ListenableFuture<RolloutGroupsValidation> validateTargetsInGroups(final List<RolloutGroupCreate> groups,
final String targetFilter, final Long createdAt, final Long dsTypeId) {
public CompletableFuture<RolloutGroupsValidation> validateTargetsInGroups(
final List<RolloutGroupCreate> groups, final String targetFilter, final Long createdAt, final Long dsTypeId) {
final TargetCount targets = calculateTargets(targetFilter, createdAt, dsTypeId);
long totalTargets = targets.total();
@@ -240,9 +239,8 @@ public class JpaRolloutManagement implements RolloutManagement {
throw new ConstraintDeclarationException("Rollout target filter does not match any targets");
}
return new AsyncResult<>(
validateTargetsInGroups(groups.stream().map(RolloutGroupCreate::build).collect(Collectors.toList()),
baseFilter, totalTargets, dsTypeId));
return CompletableFuture.completedFuture(
validateTargetsInGroups(groups.stream().map(RolloutGroupCreate::build).toList(), baseFilter, totalTargets, dsTypeId));
}
@Override
@@ -295,8 +293,7 @@ public class JpaRolloutManagement implements RolloutManagement {
@Override
public Optional<Rollout> getWithDetailedStatus(final long rolloutId) {
final Optional<Rollout> rollout = get(rolloutId);
if (!rollout.isPresent()) {
if (rollout.isEmpty()) {
return rollout;
}
@@ -713,9 +710,7 @@ public class JpaRolloutManagement implements RolloutManagement {
final Map<Long, List<TotalTargetCountActionStatus>> fromCache = rolloutStatusCache.getRolloutStatus(rollouts);
final List<Long> rolloutIds = rollouts.stream().filter(id -> !fromCache.containsKey(id))
.collect(Collectors.toList());
final List<Long> rolloutIds = rollouts.stream().filter(id -> !fromCache.containsKey(id)).toList();
if (!rolloutIds.isEmpty()) {
final List<TotalTargetCountActionStatus> resultList = actionRepository.getStatusCountByRolloutIds(rolloutIds);
final Map<Long, List<TotalTargetCountActionStatus>> fromDb = resultList.stream()
@@ -729,9 +724,6 @@ public class JpaRolloutManagement implements RolloutManagement {
return fromCache;
}
// private v isDeletedWithDistributionSet(final Boolean isDeleted, final Sort sort) {
/**
* Enforces the quota defining the maximum number of {@link Target}s per {@link RolloutGroup}.
*
@@ -783,8 +775,7 @@ public class JpaRolloutManagement implements RolloutManagement {
// new style percent - total percent
final double percentFromRest = RolloutHelper.toPercentFromTheRest(group, groups);
final long reducedTargetsInGroup = Math
.round(percentFromRest / 100 * (double) realTargetsInGroup);
final long reducedTargetsInGroup = Math.round(percentFromRest / 100 * realTargetsInGroup);
groupTargetCounts.add(reducedTargetsInGroup);
unusedTargetsCount += realTargetsInGroup - reducedTargetsInGroup;
}

View File

@@ -79,6 +79,7 @@ public interface BaseEntityRepository<T extends AbstractJpaTenantAwareBaseEntity
List<T> findAllById(final Iterable<Long> ids);
// TODO To be considered if this method is needed at all
/**
* Deletes all entities of a given tenant from this repository. For safety reasons (this is a "delete everything" query after all) we add
* the tenant manually to query even if this will be done by {@link EntityManager} anyhow. The DB should take care of optimizing this away.
@@ -145,14 +146,14 @@ public interface BaseEntityRepository<T extends AbstractJpaTenantAwareBaseEntity
}
final String managementClassSimpleName = domainClassSimpleName.substring(3);
final Class<?> superClass = domainClass.getSuperclass();
if (superClass != null) {
if (superClass.getSimpleName().equals(managementClassSimpleName) && BaseEntity.class.isAssignableFrom(superClass)) {
return (Class<? extends BaseEntity>)superClass;
}
if (superClass != null &&
superClass.getSimpleName().equals(managementClassSimpleName) && BaseEntity.class.isAssignableFrom(superClass)) {
return (Class<? extends BaseEntity>) superClass;
}
for (final Class<?> interfaceClass : domainClass.getInterfaces()) {
if (interfaceClass.getSimpleName().equals(managementClassSimpleName) && BaseEntity.class.isAssignableFrom(interfaceClass)) {
return (Class<? extends BaseEntity>)interfaceClass;
return (Class<? extends BaseEntity>) interfaceClass;
}
}
return domainClass;

View File

@@ -673,15 +673,9 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
.as("and contains the following elements").containsAll(expected);
}
private void assertThatTargetNameEquals(final List<Target> targets1, final int index1, final List<Target> targets2,
final int index2) {
assertThat(targets1.get(index1).getName()).isEqualTo(targets2.get(index2).getName());
}
private DistributionSet createDistSetWithType(final DistributionSetType type) {
final DistributionSetCreate dsCreate = entityFactory.distributionSet().create().name("test-ds").version("1.0")
.type(type);
return distributionSetManagement.create(dsCreate);
}
}
}