Skip to content

Commit

Permalink
Sonar Fixes (7) (#2215)
Browse files Browse the repository at this point in the history
Signed-off-by: Avgustin Marinov <[email protected]>
  • Loading branch information
avgustinmm authored Jan 21, 2025
1 parent bbb5f40 commit f09db20
Show file tree
Hide file tree
Showing 15 changed files with 77 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,12 @@ public void evictCaches(final String tenant) {
cacheManager.evictCaches(tenant);
}

private Map<Long, List<TotalTargetCountActionStatus>> retrieveFromCache(final List<Long> ids,
private @NotNull Map<Long, List<TotalTargetCountActionStatus>> retrieveFromCache(final List<Long> ids,
@NotNull final Cache cache) {
return ids.stream().map(id -> cache.get(id, CachedTotalTargetCountActionStatus.class)).filter(Objects::nonNull)
.collect(Collectors.toMap(CachedTotalTargetCountActionStatus::getId,
CachedTotalTargetCountActionStatus::getStatus));
return ids.stream()
.map(id -> cache.get(id, CachedTotalTargetCountActionStatus.class))
.filter(Objects::nonNull)
.collect(Collectors.toMap(CachedTotalTargetCountActionStatus::getId, CachedTotalTargetCountActionStatus::getStatus));
}

private List<TotalTargetCountActionStatus> retrieveFromCache(final Long id, @NotNull final Cache cache) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata;
import org.eclipse.hawkbit.repository.model.MetaData;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.util.StringUtils;
import org.springframework.validation.annotation.Validated;

/**
Expand Down Expand Up @@ -68,12 +67,12 @@ public DistributionSetBuilder distributionSet() {

@Override
public MetaData generateDsMetadata(final String key, final String value) {
return new JpaDistributionSetMetadata(key, StringUtils.trimWhitespace(value));
return new JpaDistributionSetMetadata(key, value == null ? null : value.strip());
}

@Override
public MetaData generateTargetMetadata(final String key, final String value) {
return new JpaTargetMetadata(key, StringUtils.trimWhitespace(value));
return new JpaTargetMetadata(key, value == null ? null : value.strip());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public static String[] getFilterNameAndVersionEntries(final String filterString)
: (filterString + "%");
final String filterVersion = semicolonIndex != -1 ? (filterString.substring(semicolonIndex + 1) + "%") : "%";

return new String[] { !StringUtils.isEmpty(filterName) ? filterName : "%", filterVersion };
return new String[] { ObjectUtils.isEmpty(filterName) ? "%" : filterName, filterVersion };
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,7 @@ private void handleStopRollout(final JpaRollout rollout) {
rollout.setStatus(RolloutStatus.FINISHED);
rolloutRepository.save(rollout);

final List<Long> groupIds = rollout.getRolloutGroups().stream().map(RolloutGroup::getId)
.collect(Collectors.toList());

final List<Long> groupIds = rollout.getRolloutGroups().stream().map(RolloutGroup::getId).toList();
afterCommit.afterCommit(() -> eventPublisherHolder.getEventPublisher().publishEvent(new RolloutStoppedEvent(
tenantAware.getCurrentTenant(), eventPublisherHolder.getApplicationId(), rollout.getId(), groupIds)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private static String createRolloutLockKey(final String tenant) {
private void handleRolloutInNewTransaction(final long rolloutId, final String handlerId) {
DeploymentHelper.runInNewTransaction(txManager, handlerId + "-" + rolloutId, status -> {
rolloutManagement.get(rolloutId).ifPresentOrElse(
rollout -> {
rollout ->
// auditor is retrieved and set on transaction commit
// if not overridden, the system user will be the auditor
rollout.getAccessControlContext().ifPresentOrElse(
Expand All @@ -113,9 +113,7 @@ private void handleRolloutInNewTransaction(final long rolloutId, final String ha
rollout.getCreatedBy(), () -> {
rolloutExecutor.execute(rollout);
return null;
})
);
},
})),
() -> log.error("Could not retrieve rollout with id {}. Will not continue with execution.",
rolloutId));
return 0L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import org.eclipse.hawkbit.repository.model.DistributionSetType;
import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

/**
* Create/build implementation.
Expand All @@ -47,7 +45,7 @@ public class JpaDistributionSetCreate extends AbstractDistributionSetUpdateCreat

@Override
public DistributionSetCreate type(final String type) {
this.type = ObjectUtils.isEmpty(type) ? type : type.strip();
this.type = type == null ? null : type.strip();
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public RolloutCreate distributionSetId(final long distributionSetId) {
* @return this builder
*/
public RolloutCreate targetFilterQuery(final String targetFilterQuery) {
this.targetFilterQuery = StringUtils.trimWhitespace(targetFilterQuery);
this.targetFilterQuery = targetFilterQuery == null ? null : targetFilterQuery.strip();
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ public Target findOrRegisterTargetIfItDoesNotExist(final String controllerId, fi
}
private Target findOrRegisterTargetIfItDoesNotExist0(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))
return targetRepository.findOne(spec)
.map(target -> updateTarget(target, address, name, type))
.orElseGet(() -> createTarget(controllerId, address, name, type));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,18 @@ public JpaDeploymentManagement(
@Override
@Transactional(isolation = Isolation.READ_COMMITTED)
public List<DistributionSetAssignmentResult> assignDistributionSets(final List<DeploymentRequest> deploymentRequests) {
return assignDistributionSets(tenantAware.getCurrentUsername(), deploymentRequests, null);
return assignDistributionSets0(tenantAware.getCurrentUsername(), deploymentRequests, null);
}

@Override
@Transactional(isolation = Isolation.READ_COMMITTED)
public List<DistributionSetAssignmentResult> assignDistributionSets(
final String initiatedBy, final List<DeploymentRequest> deploymentRequests, final String actionMessage) {
return assignDistributionSets0(initiatedBy, deploymentRequests, actionMessage);
}

private List<DistributionSetAssignmentResult> assignDistributionSets0(
final String initiatedBy, final List<DeploymentRequest> deploymentRequests, final String actionMessage) {
WeightValidationHelper.usingContext(systemSecurityContext, tenantConfigurationManagement).validate(deploymentRequests);
return assignDistributionSets(initiatedBy, deploymentRequests, actionMessage, onlineDsAssignmentStrategy);
}
Expand Down Expand Up @@ -232,6 +237,10 @@ public List<DistributionSetAssignmentResult> offlineAssignedDistributionSets(fin
@Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX,
backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Action cancelAction(final long actionId) {
return cancelAction0(actionId);
}

private Action cancelAction0(final long actionId) {
log.debug("cancelAction({})", actionId);

final JpaAction action = actionRepository.findById(actionId)
Expand Down Expand Up @@ -393,6 +402,10 @@ public List<Action> findActiveActionsWithHighestWeight(final String controllerId
@Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX,
backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Action forceQuitAction(final long actionId) {
return forceQuitAction0(actionId);
}

private Action forceQuitAction0(final long actionId) {
final JpaAction action = actionRepository.findById(actionId)
.orElseThrow(() -> new EntityNotFoundException(Action.class, actionId));

Expand Down Expand Up @@ -495,12 +508,14 @@ public void startScheduledActions(final List<Action> rolloutGroupActions) {

@Override
public Optional<DistributionSet> getAssignedDistributionSet(final String controllerId) {
return targetRepository.findWithDetailsByControllerId(controllerId, JpaTarget_.GRAPH_TARGET_ASSIGNED_DISTRIBUTION_SET).map(JpaTarget::getAssignedDistributionSet);
return targetRepository.findWithDetailsByControllerId(controllerId, JpaTarget_.GRAPH_TARGET_ASSIGNED_DISTRIBUTION_SET)
.map(JpaTarget::getAssignedDistributionSet);
}

@Override
public Optional<DistributionSet> getInstalledDistributionSet(final String controllerId) {
return targetRepository.findWithDetailsByControllerId(controllerId, JpaTarget_.GRAPH_TARGET_INSTALLED_DISTRIBUTION_SET).map(JpaTarget::getInstalledDistributionSet);
return targetRepository.findWithDetailsByControllerId(controllerId, JpaTarget_.GRAPH_TARGET_INSTALLED_DISTRIBUTION_SET)
.map(JpaTarget::getInstalledDistributionSet);
}

@Override
Expand Down Expand Up @@ -542,7 +557,7 @@ public void cancelActionsForDistributionSet(final CancelationType cancelationTyp
.forEach(action -> {
try {
assertTargetUpdateAllowed(action);
cancelAction(action.getId());
cancelAction0(action.getId());
log.debug("Action {} canceled", action.getId());
} catch (final InsufficientPermissionException e) {
log.trace("Could not cancel action {} due to insufficient permissions.", action.getId(), e);
Expand All @@ -555,7 +570,7 @@ public void cancelActionsForDistributionSet(final CancelationType cancelationTyp
.forEach(action -> {
try {
assertTargetUpdateAllowed(action);
forceQuitAction(action.getId());
forceQuitAction0(action.getId());
log.debug("Action {} force canceled (force)", action.getId());
} catch (final InsufficientPermissionException e) {
log.trace("Could not cancel action {} due to insufficient permissions.", action.getId(), e);
Expand Down Expand Up @@ -588,7 +603,7 @@ private static Map<Long, List<TargetWithActionType>> convertRequest(final Collec
* statements
*/
private static List<List<Long>> getTargetEntitiesAsChunks(final List<JpaTarget> targetEntities) {
return ListUtils.partition(targetEntities.stream().map(Target::getId).collect(Collectors.toList()), Constants.MAX_ENTRIES_IN_STATEMENT);
return ListUtils.partition(targetEntities.stream().map(Target::getId).toList(), Constants.MAX_ENTRIES_IN_STATEMENT);
}

private static DistributionSetAssignmentResult buildAssignmentResult(
Expand Down Expand Up @@ -855,16 +870,16 @@ private void enforceMaxActionsPerTarget(final Collection<DeploymentRequest> depl
quota, Action.class, Target.class, actionRepository::countByTargetControllerId));
}

private void closeOrCancelActiveActions(final AbstractDsAssignmentStrategy assignmentStrategy,
final List<List<Long>> targetIdsChunks) {
private void closeOrCancelActiveActions(final AbstractDsAssignmentStrategy assignmentStrategy, final List<List<Long>> targetIdsChunks) {
if (isActionsAutocloseEnabled()) {
assignmentStrategy.closeActiveActions(targetIdsChunks);
} else {
assignmentStrategy.cancelActiveActions(targetIdsChunks);
}
}

private void setAssignedDistributionSetAndTargetUpdateStatus(final AbstractDsAssignmentStrategy assignmentStrategy,
private void setAssignedDistributionSetAndTargetUpdateStatus(
final AbstractDsAssignmentStrategy assignmentStrategy,
final JpaDistributionSet set, final List<List<Long>> targetIdsChunks) {
final String currentUser = auditorProvider.getCurrentAuditor().orElse(null);
assignmentStrategy.setAssignedDistributionSetAndTargetStatus(set, targetIdsChunks, currentUser);
Expand Down Expand Up @@ -970,8 +985,7 @@ private List<Action> startScheduledActionsAndHandleOpenCancellationFirst(final L
}

private void closeOrCancelOpenDeviceActions(final List<JpaAction> actions) {
final List<Long> targetIds = actions.stream().map(JpaAction::getTarget).map(Target::getId)
.collect(Collectors.toList());
final List<Long> targetIds = actions.stream().map(JpaAction::getTarget).map(Target::getId).toList();
if (isActionsAutocloseEnabled()) {
onlineDsAssignmentStrategy.closeObsoleteUpdateActions(targetIds);
} else {
Expand Down Expand Up @@ -999,7 +1013,7 @@ private void setAssignmentOnTargets(final List<JpaAction> actions) {
mergedTarget.setAssignedDistributionSet((JpaDistributionSet) savedAction.getDistributionSet());
mergedTarget.setUpdateStatus(TargetUpdateStatus.PENDING);
return mergedTarget;
}).collect(Collectors.toList());
}).toList();

targetRepository.saveAll(assignedDsTargets);
}
Expand Down Expand Up @@ -1041,12 +1055,12 @@ private void assertTargetReadAllowed(final String controllerId) {
}

private JpaAction assertTargetUpdateAllowed(final JpaAction action) {
targetRepository.findOne(TargetSpecifications.hasId(action.getTarget().getId())).ifPresentOrElse(target -> {
targetRepository.getAccessController()
.ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, target));
}, () -> {
throw new EntityNotFoundException(Action.class, action);
});
targetRepository.findOne(TargetSpecifications.hasId(action.getTarget().getId())).ifPresentOrElse(
target -> targetRepository.getAccessController()
.ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, target)),
() -> {
throw new EntityNotFoundException(Action.class, action);
});
return action;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,17 @@ public DistributionSet update(final DistributionSetUpdate u) {
@Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX,
backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public void delete(final long id) {
delete(List.of(id));
delete0(List.of(id));
}

@Override
@Transactional
@Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX,
backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public void delete(final Collection<Long> distributionSetIDs) {
delete0(distributionSetIDs);
}
private void delete0(final Collection<Long> distributionSetIDs) {
getDistributionSets(distributionSetIDs); // throws EntityNotFoundException if any of these do not exists
final List<JpaDistributionSet> setsFound = distributionSetRepository.findAll(
AccessController.Operation.DELETE, distributionSetRepository.byIdsSpec(distributionSetIDs));
Expand Down Expand Up @@ -418,8 +421,7 @@ public void deleteMetaData(final long id, final String key) {

// touch it to update the lock revision because we are modifying the
// DS indirectly, it will, also check UPDATE access
JpaManagementHelper.touch(entityManager, distributionSetRepository,
(JpaDistributionSet) metadata.getDistributionSet());
JpaManagementHelper.touch(entityManager, distributionSetRepository, metadata.getDistributionSet());
distributionSetMetadataRepository.deleteById(metadata.getId());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ public void delete(final Collection<Long> ids) {
final List<JpaDistributionSetTag> setsFound = distributionSetTagRepository.findAllById(ids);

if (setsFound.size() < ids.size()) {
throw new EntityNotFoundException(DistributionSetTag.class, ids,
setsFound.stream().map(DistributionSetTag::getId).collect(Collectors.toList()));
throw new EntityNotFoundException(
DistributionSetTag.class, ids,
setsFound.stream().map(DistributionSetTag::getId).toList());
}

distributionSetTagRepository.deleteAll(setsFound);
Expand Down
Loading

0 comments on commit f09db20

Please sign in to comment.