Fix non found exception content of target management (un)assignTag (#1894)

Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java

@@ -666,14 +666,10 @@ public interface TargetManagement {
     /**
      * Assign a {@link TargetTag} assignment to given {@link Target}s.
      *
-     * @param controllerIds
+     * @param controllerIds to assign for
-     *            to assign for
+     * @param targetTagId to assign
-     * @param targetTagId
-     *            to assign
      * @return list of assigned targets
-     *
+     * @throws EntityNotFoundException if given targetTagId or at least one of the targets do not exist
-     * @throws EntityNotFoundException
-     *             if given tagId or at least one of the targets do not exist
      */
     @PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_REPOSITORY_AND_UPDATE_TARGET)
     List<Target> assignTag(@NotEmpty Collection<String> controllerIds, long targetTagId);
@@ -683,11 +679,11 @@ public interface TargetManagement {
      *
      * @param controllerIds to un-assign for
      * @param targetTagId to un-assign
-     * @return the unassigned target or <null> if no target is unassigned
+     * @return list of unassigned targets
-     * @throws EntityNotFoundException if Tag with given ID does not exist
+     * @throws EntityNotFoundException if given targetTagId or at least one of the targets do not exist
      */
     @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_TARGET)
-    List<Target> unassignTag(@NotEmpty List<String> controllerIds, long targetTagId);
+    List<Target> unassignTag(@NotEmpty Collection<String> controllerIds, long targetTagId);
 
     /**
      * Un-assign a {@link TargetType} assignment to given {@link Target}.

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java

@@ -20,6 +20,9 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import jakarta.persistence.EntityManager;
@@ -533,50 +536,42 @@ public class JpaTargetManagement implements TargetManagement {
     @Retryable(include = {
             ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
     public List<Target> assignTag(final Collection<String> controllerIds, final long targetTagId) {
-        final JpaTargetTag tag = targetTagRepository.findById(targetTagId)
+        return updateTag(controllerIds, targetTagId, (tag, target) -> {
-                .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, targetTagId));
+            if (target.getTags().contains(tag)) {
-
+                return target;
-        final List<JpaTarget> allTargets = targetRepository
+            } else {
-                .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds));
+                target.addTag(tag);
-        if (allTargets.size() < controllerIds.size()) {
+                return targetRepository.save(target);
-            throw new EntityNotFoundException(Target.class, controllerIds,
+            }
-                    allTargets.stream().map(Target::getControllerId).toList());
+        });
-        }
-
-        targetRepository.getAccessController()
-                .ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, allTargets));
-
-        allTargets.forEach(target -> target.addTag(tag));
-
-        final List<Target> result = allTargets.stream().map(targetRepository::save).map(Target.class::cast).toList();
-
-        // No reason to save the tag
-        entityManager.detach(tag);
-        return result;
     }
-
     @Override
     @Transactional
     @Retryable(include = {
             ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
-    public List<Target> unassignTag(final List<String> controllerIds, final long targetTagId) {
+    public List<Target> unassignTag(final Collection<String> controllerIds, final long targetTagId) {
+        return updateTag(controllerIds, targetTagId, (tag, target) -> {
+            if (target.getTags().contains(tag)) {
+                target.removeTag(tag);
+                return targetRepository.save(target);
+            } else {
+                return target;
+            }
+        });
+    }
+    private List<Target> updateTag(final Collection<String> controllerIds, final long targetTagId, final BiFunction<JpaTargetTag, JpaTarget, Target> updater) {
         final JpaTargetTag tag = targetTagRepository.findById(targetTagId)
                 .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, targetTagId));
-
+        final List<JpaTarget> targets = targetRepository
-        final List<JpaTarget> allTargets = targetRepository
                 .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds));
-        if (allTargets.size() < controllerIds.size()) {
+        if (targets.size() < controllerIds.size()) {
-            throw new EntityNotFoundException(Target.class, controllerIds,
+            throw new EntityNotFoundException(Target.class, notFound(controllerIds, targets));
-                    allTargets.stream().map(Target::getControllerId).toList());
         }
-
         targetRepository.getAccessController()
-                .ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, allTargets));
+                .ifPresent(acm -> acm.assertOperationAllowed(AccessController.Operation.UPDATE, targets));
-
-        allTargets.forEach(target -> target.removeTag(tag));
-
-        final List<Target> result = allTargets.stream().map(targetRepository::save).map(Target.class::cast).toList();
 
+        // apply update and collect modified targets
+        final List<Target> result = targets.stream().map(target -> updater.apply(tag, target)).toList();
         // No reason to save the tag
         entityManager.detach(tag);
         return result;
@@ -919,6 +914,12 @@ public class JpaTargetManagement implements TargetManagement {
                 List.of(TargetSpecifications.hasRequestControllerAttributesTrue()));
     }
 
+    private static Collection<String> notFound(final Collection<String> controllerIds, final List<JpaTarget> foundTargets) {
+        final Map<String, JpaTarget> foundTargetMap = foundTargets.stream()
+                .collect(Collectors.toMap(Target::getControllerId, Function.identity()));
+        return controllerIds.stream().filter(id -> !foundTargetMap.containsKey(id)).toList();
+    }
+
     @Override
     @Transactional
     @Retryable(include = {