From c76a2e2db55669b74caca5b10e5674f81139f870 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Thu, 17 Oct 2024 13:07:10 +0300 Subject: [PATCH] Fix non found exception content of DS management (un)assignTag (#1895) Signed-off-by: Marinov Avgustin --- .../JpaDistributionSetManagement.java | 80 +++++++++++++------ .../jpa/management/JpaTargetManagement.java | 23 ++++-- 2 files changed, 71 insertions(+), 32 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java index 3dad61ba8..7b36423fd 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java @@ -14,9 +14,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -73,6 +75,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSetType; import org.eclipse.hawkbit.repository.model.MetaData; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Statistic; +import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; import org.eclipse.hawkbit.utils.TenantConfigHelper; import org.springframework.dao.ConcurrencyFailureException; @@ -190,45 +193,49 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public List assignTag(final Collection ids, final long dsTagId) { - return updateTags( - ids, - () -> distributionSetTagManagement.get(dsTagId).orElseThrow(() -> new EntityNotFoundException(DistributionSetTag.class, dsTagId)), - (allDs, distributionSetTag) -> { - allDs.forEach(ds -> ds.addTag(distributionSetTag)); - return Collections.unmodifiableList(distributionSetRepository.saveAll(allDs)); - }); + return updateTag(ids, dsTagId, (tag, distributionSet) -> { + if (distributionSet.getTags().contains(tag)) { + return distributionSet; + } else { + distributionSet.addTag(tag); + return distributionSetRepository.save(distributionSet); + } + }); } - @Override @Transactional @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public List unassignTag(final Collection ids, final long dsTagId) { - return updateTags( - ids, - () -> distributionSetTagManagement.get(dsTagId).orElseThrow(() -> new EntityNotFoundException(DistributionSetTag.class, dsTagId)), - (allDs, distributionSetTag) -> { - allDs.forEach(ds -> ds.removeTag(distributionSetTag)); - return Collections.unmodifiableList(distributionSetRepository.saveAll(allDs)); + return updateTag(ids, dsTagId, (tag, distributionSet) -> { + if (distributionSet.getTags().contains(tag)) { + distributionSet.removeTag(tag); + return distributionSetRepository.save(distributionSet); + } else { + return distributionSet; + } }); } - - private T updateTags( - final Collection dsIds, final Supplier tagSupplier, - final BiFunction, DistributionSetTag, T> updater) { + private List updateTag( + final Collection dsIds, final long dsTagId, + final BiFunction updater) { + final DistributionSetTag tag = distributionSetTagManagement.get(dsTagId) + .orElseThrow(() -> new EntityNotFoundException(DistributionSetTag.class, dsTagId)); final List allDs = dsIds.size() == 1 ? - distributionSetRepository.findById(dsIds.iterator().next()).map(List::of).orElseGet(Collections::emptyList) : + distributionSetRepository.findById(dsIds.iterator().next()) + .map(List::of) + .orElseGet(Collections::emptyList) : distributionSetRepository.findAll(DistributionSetSpecification.byIds(dsIds)); if (allDs.size() < dsIds.size()) { - throw new EntityNotFoundException(DistributionSet.class, dsIds, allDs.stream().map(DistributionSet::getId).toList()); + throw new EntityNotFoundException(DistributionSet.class, notFound(dsIds, allDs)); } - final DistributionSetTag distributionSetTag = tagSupplier.get(); try { - return updater.apply(allDs, distributionSetTag); + // apply update and collect modified targets + return allDs.stream().map(distributionSet -> updater.apply(tag, distributionSet)).toList(); } finally { // No reason to save the tag - entityManager.detach(distributionSetTag); + entityManager.detach(tag); } } @@ -791,6 +798,13 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { return true; } + private static Collection notFound(final Collection distributionSetIds, final List foundDistributionSets) { + final Map foundDistributionSetMap = foundDistributionSets.stream() + .collect(Collectors.toMap(JpaDistributionSet::getId, Function.identity())); + return distributionSetIds.stream().filter(id -> !foundDistributionSetMap.containsKey(id)).toList(); + } + + private void lockSoftwareModules(final JpaDistributionSet distributionSet) { distributionSet.getModules().forEach(module -> { if (!module.isLocked()) { @@ -824,7 +838,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public DistributionSetTagAssignmentResult toggleTagAssignment(final Collection ids, final String tagName) { - return updateTags( + return updateTag( ids, () -> distributionSetTagManagement .getByName(tagName) @@ -855,6 +869,24 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { return result; }); } + private T updateTag( + final Collection dsIds, final Supplier tagSupplier, + final BiFunction, DistributionSetTag, T> updater) { + final List allDs = dsIds.size() == 1 ? + distributionSetRepository.findById(dsIds.iterator().next()).map(List::of).orElseGet(Collections::emptyList) : + distributionSetRepository.findAll(DistributionSetSpecification.byIds(dsIds)); + if (allDs.size() < dsIds.size()) { + throw new EntityNotFoundException(DistributionSet.class, dsIds, allDs.stream().map(DistributionSet::getId).toList()); + } + + final DistributionSetTag distributionSetTag = tagSupplier.get(); + try { + return updater.apply(allDs, distributionSetTag); + } finally { + // No reason to save the tag + entityManager.detach(distributionSetTag); + } + } @Override @Transactional diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java index 7a910e8f8..7bdaf3e6b 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java @@ -559,22 +559,29 @@ public class JpaTargetManagement implements TargetManagement { } }); } - private List updateTag(final Collection controllerIds, final long targetTagId, final BiFunction updater) { + private List updateTag( + final Collection controllerIds, final long targetTagId, + final BiFunction updater) { final JpaTargetTag tag = targetTagRepository.findById(targetTagId) .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, targetTagId)); - final List targets = targetRepository - .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds)); + final List targets = controllerIds.size() == 1 ? + targetRepository.findOne(TargetSpecifications.hasControllerId(controllerIds.iterator().next())) + .map(List::of) + .orElseGet(Collections::emptyList) : + targetRepository + .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds)); if (targets.size() < controllerIds.size()) { throw new EntityNotFoundException(Target.class, notFound(controllerIds, targets)); } targetRepository.getAccessController() .ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, targets)); - // apply update and collect modified targets - final List result = targets.stream().map(target -> updater.apply(tag, target)).toList(); - // No reason to save the tag - entityManager.detach(tag); - return result; + try { + return targets.stream().map(target -> updater.apply(tag, target)).toList(); + } finally { + // No reason to save the tag + entityManager.detach(tag); + } } @Override