Skip to content

Commit

Permalink
Fix some sonar findings
Browse files Browse the repository at this point in the history 
Signed-off-by: Avgustin Marinov <[email protected]>
  • Loading branch information
@avgustinmm
avgustinmm committed Dec 17, 2024
1 parent db3ac7f commit d5e1465
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 139 deletions.
6 changes: 3 additions & 3 deletions ...awkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutManagement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import jakarta.validation.ConstraintViolationException;
import jakarta.validation.Valid;
Expand Down Expand Up @@ -39,7 +40,6 @@
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
Expand Down Expand Up @@ -184,8 +184,8 @@ Rollout create(@Valid @NotNull RolloutCreate rollout, @NotNull @Valid List<Rollo
* {@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.
Expand Down
67 changes: 8 additions & 59 deletions .../src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaControllerManagement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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;
Expand Down Expand Up @@ -222,13 +223,11 @@ public Action addCancelActionStatus(final ActionStatusCreate c) {
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;
Expand Down Expand Up @@ -320,14 +319,14 @@ public Page<ActionStatus> findActionStatusByAction(final Pageable pageReq, final

@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))
Expand Down Expand Up @@ -603,10 +602,6 @@ void setTargetRepository(final TargetRepository targetRepositorySpy) {
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);
}
Expand Down Expand Up @@ -702,7 +697,7 @@ private void flushUpdateQueue() {
log.debug("Run flushUpdateQueue.");

final int size = queue.size();
if (size <= 0) {
if (size == 0) {
return;
}

Expand Down Expand Up @@ -733,7 +728,7 @@ private Void updateLastTargetQueries(final String tenant, final List<TargetPoll>
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 -> {
Expand Down Expand Up @@ -1034,6 +1029,7 @@ String timeToNextEvent(final int eventCount, final ZonedDateTime timerResetTime)
}
}

@Data
private static class TargetPoll {

private final String tenant;
Expand All @@ -1043,52 +1039,5 @@ private static class TargetPoll {
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;
}

}
}
59 changes: 11 additions & 48 deletions ...rc/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutGroupManagement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
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;

Expand All @@ -31,7 +29,6 @@
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_;
Expand All @@ -55,9 +52,7 @@
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;
Expand Down Expand Up @@ -106,17 +101,13 @@ public Page<RolloutGroup> findByRolloutWithDetailedStatus(final long rolloutId,
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(),
Expand Down Expand Up @@ -149,20 +140,16 @@ public Page<RolloutGroup> findByRolloutAndRsqlWithDetailedStatus(final long roll
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);
}
Expand Down Expand Up @@ -220,8 +207,7 @@ public Page<Target> findTargetsOfRolloutGroupByRsql(final Pageable pageable, fin
@Override
public Optional<RolloutGroup> getWithDetailedStatus(final long rolloutGroupId) {
final Optional<RolloutGroup> rolloutGroup = get(rolloutGroupId);

if (!rolloutGroup.isPresent()) {
if (rolloutGroup.isEmpty()) {
return rolloutGroup;
}

Expand All @@ -235,8 +221,8 @@ public Optional<RolloutGroup> getWithDetailedStatus(final long rolloutGroupId) {
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;

Expand Down Expand Up @@ -268,9 +254,7 @@ private Map<Long, List<TotalTargetCountActionStatus>> getStatusCountItemForRollo
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);
Expand All @@ -291,30 +275,9 @@ private Predicate getRolloutGroupTargetWithRolloutGroupJoinCondition(final long
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);
}
}

}
}
25 changes: 8 additions & 17 deletions ...jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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;

Expand Down Expand Up @@ -89,10 +90,8 @@
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;

/**
Expand Down Expand Up @@ -229,8 +228,8 @@ public Rollout create(final RolloutCreate rollout, final List<RolloutGroupCreate

@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();
Expand All @@ -240,9 +239,8 @@ public ListenableFuture<RolloutGroupsValidation> validateTargetsInGroups(final L
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
Expand Down Expand Up @@ -295,8 +293,7 @@ public Optional<Rollout> getByName(final String rolloutName) {
@Override
public Optional<Rollout> getWithDetailedStatus(final long rolloutId) {
final Optional<Rollout> rollout = get(rolloutId);

if (!rollout.isPresent()) {
if (rollout.isEmpty()) {
return rollout;
}

Expand Down Expand Up @@ -713,9 +710,7 @@ private Map<Long, List<TotalTargetCountActionStatus>> getStatusCountItemForRollo

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()
Expand All @@ -729,9 +724,6 @@ private Map<Long, List<TotalTargetCountActionStatus>> getStatusCountItemForRollo
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}.
*
Expand Down Expand Up @@ -783,8 +775,7 @@ private RolloutGroupsValidation validateTargetsInGroups(
// 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;
}
Expand Down
Loading

0 comments on commit d5e1465

Please sign in to comment.