Improve Target Tag REST & management API (#1880)

* added methods to unassign by multiple controller ids
* deprecated toggle assigments - too complex to undestand
* deprecated unassign (management) of single controller id - in favour of methods with controller ids

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

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

@@ -52,21 +52,6 @@ import org.springframework.security.access.prepost.PreAuthorize;
  */
 public interface TargetManagement {
 
-    /**
-     * Assign a {@link TargetTag} assignment to given {@link Target}s.
-     *
-     * @param controllerIds
-     *            to assign for
-     * @param tagId
-     *            to assign
-     * @return list of assigned targets
-     * 
-     * @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 tagId);
-
     /**
      * Counts number of targets with the given distribution set assigned.
      *
@@ -653,14 +638,13 @@ public interface TargetManagement {
      * assigned, they will be. Only if all of them have the tag already assigned
      * they will be removed instead.
      *
-     * @param controllerIds
+     * @deprecated since 0.6.0 - not very usable with very unclear logic
-     *            to toggle for
+     * @param controllerIds to toggle for
-     * @param tagName
+     * @param tagName to toggle
-     *            to toggle
      * @return TagAssigmentResult with all metadata of the assignment outcome.
-     * @throws EntityNotFoundException
+     * @throws EntityNotFoundException if tag with given name does not exist
-     *             if tag with given name does not exist
      */
+    @Deprecated
     @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_TARGET)
     TargetTagAssignmentResult toggleTagAssignment(@NotEmpty Collection<String> controllerIds, @NotEmpty String tagName);
 
@@ -695,26 +679,49 @@ public interface TargetManagement {
     @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_TARGET)
     TargetTypeAssignmentResult unassignType(@NotEmpty Collection<String> controllerIds);
 
+    /**
+     * Assign a {@link TargetTag} assignment to given {@link Target}s.
+     *
+     * @param controllerIds
+     *            to assign for
+     * @param targetTagId
+     *            to assign
+     * @return list of assigned targets
+     *
+     * @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);
+
+    /**
+     * Un-assign a {@link TargetTag} assignment to given {@link Target}s.
+     *
+     * @param controllerIds to un-assign for
+     * @param targetTagId to un-assign
+     * @return the unassigned target or <null> if no target is unassigned
+     * @throws EntityNotFoundException if Tag with given ID does not exist
+     */
+    @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_TARGET)
+    List<Target> unassignTag(@NotEmpty List<String> controllerIds, long targetTagId);
+
     /**
      * Un-assign a {@link TargetTag} assignment to given {@link Target}.
      *
-     * @param controllerId
+     * @deprecated since 0.6.0 - use {@link #unassignTag(List, long)} instead
-     *            to un-assign for
+     * @param controllerId to un-assign for
-     * @param targetTagId
+     * @param targetTagId to un-assign
-     *            to un-assign
      * @return the unassigned target or <null> if no target is unassigned
-     * 
+     * @throws EntityNotFoundException if TAG with given ID does not exist
-     * @throws EntityNotFoundException
-     *             if TAG with given ID does not exist
      */
+    @Deprecated(forRemoval = true)
     @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_TARGET)
     Target unassignTag(@NotEmpty String controllerId, long targetTagId);
 
     /**
      * Un-assign a {@link TargetType} assignment to given {@link Target}.
      *
-     * @param controllerId
+     * @param controllerId to un-assign for
-     *            to un-assign for
      * @return the unassigned target
      *
      */

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

@@ -483,7 +483,8 @@ public class JpaTargetManagement implements TargetManagement {
     @Retryable(include = {
             ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
     public TargetTagAssignmentResult toggleTagAssignment(final Collection<String> controllerIds, final String tagName) {
-        final TargetTag tag = targetTagRepository.findByNameEquals(tagName)
+        final TargetTag tag = targetTagRepository
+                .findByNameEquals(tagName)
                 .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, tagName));
         final List<JpaTarget> allTargets = targetRepository
                 .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds));
@@ -568,9 +569,9 @@ public class JpaTargetManagement implements TargetManagement {
     @Transactional
     @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 tagId) {
+    public List<Target> assignTag(final Collection<String> controllerIds, final long targetTagId) {
-        final JpaTargetTag tag = targetTagRepository.findById(tagId)
+        final JpaTargetTag tag = targetTagRepository.findById(targetTagId)
-                .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, tagId));
+                .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, targetTagId));
 
         final List<JpaTarget> allTargets = targetRepository
                 .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds));
@@ -591,6 +592,33 @@ public class JpaTargetManagement implements TargetManagement {
         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) {
+        final JpaTargetTag tag = targetTagRepository.findById(targetTagId)
+                .orElseThrow(() -> new EntityNotFoundException(TargetTag.class, targetTagId));
+
+        final List<JpaTarget> allTargets = targetRepository
+                .findAll(TargetSpecifications.byControllerIdWithTagsInJoin(controllerIds));
+        if (allTargets.size() < controllerIds.size()) {
+            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.removeTag(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 = {

hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTargetTagRestApi.java

@@ -290,11 +290,11 @@ public interface MgmtTargetTagRestApi {
      * Handles the POST request to toggle the assignment of targets by the given
      * tag id.
      *
+     * @deprecated since 0.6.0 - not very usable with very unclear logic
      * @param targetTagId
      *            the ID of the target tag to retrieve
      * @param assignedTargetRequestBodies
      *            list of controller ids to be toggled
-     *
      * @return the list of assigned targets and unassigned targets.
      */
     @Operation(summary = "Toggles target tag assignment", description = "Handles the POST request of toggle target " +
@@ -318,6 +318,7 @@ public interface MgmtTargetTagRestApi {
             + MgmtRestConstants.TARGET_TAG_TARGETS_REQUEST_MAPPING + "/toggleTagAssignment", consumes = {
                     MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }, produces = {
                             MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE })
+    @Deprecated(forRemoval = true)
     ResponseEntity<MgmtTargetTagAssigmentResult> toggleTagAssignment(@PathVariable("targetTagId") Long targetTagId,
             List<MgmtAssignedTargetRequestBody> assignedTargetRequestBodies);
 
@@ -325,7 +326,7 @@ public interface MgmtTargetTagRestApi {
      * Handles the PUT request to assign targets to the given tag id.
      *
      * @param targetTagId the ID of the target tag to retrieve
-     * @param assignedTargetControlIdsStream stream of controller ids to be assigned
+     * @param controllerIds stream of controller ids to be assigned
      *
      * @return the list of assigned targets.
      */
@@ -350,17 +351,45 @@ public interface MgmtTargetTagRestApi {
     @PutMapping(
             value = MgmtRestConstants.TARGET_TAG_V1_REQUEST_MAPPING + MgmtRestConstants.TARGET_TAG_TARGETS_REQUEST_MAPPING,
             consumes = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE } )
-    ResponseEntity<Void> assignTargetsByControllerIds(
+    ResponseEntity<Void> assignTargets(
             @PathVariable("targetTagId") Long targetTagId,
             @Schema(description = "List of controller ids to be assigned", example = "[\"controllerId1\", \"controllerId2\"]")
-            @RequestBody List<String> assignedTargetControlIds);
+            @RequestBody List<String> controllerIds);
+    /**
+     * Handles the DELETE request to unassign one target from the given tag id.
+     *
+     * @param targetTagId the ID of the target tag
+     * @param controllerId the ID of the target to unassign
+     * @return http status code
+     */
+    @Operation(summary = "Unassign targets from a given tagId",
+            description = "Handles the DELETE request to unassign the given targets.")
+    @ApiResponses(value = {
+            @ApiResponse(responseCode = "200", description = "Successfully retrieved"),
+            @ApiResponse(responseCode = "400", description = "Bad Request - e.g. invalid parameters",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ExceptionInfo.class))),
+            @ApiResponse(responseCode = "401", description = "The request requires user authentication."),
+            @ApiResponse(responseCode = "403", description = "Insufficient permissions, entity is not allowed to be " +
+                    "changed (i.e. read-only) or data volume restriction applies."),
+            @ApiResponse(responseCode = "404", description = "Target not found.",
+                    content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))),
+            @ApiResponse(responseCode = "405", description = "The http request method is not allowed on the resource."),
+            @ApiResponse(responseCode = "406", description = "In case accept header is specified and not application/json."),
+            @ApiResponse(responseCode = "429", description = "Too many requests. The server will refuse further attempts " +
+                    "and the client has to wait another second.")
+    })
+    @DeleteMapping(value = MgmtRestConstants.TARGET_TAG_V1_REQUEST_MAPPING + MgmtRestConstants.TARGET_TAG_TARGETS_REQUEST_MAPPING)
+    ResponseEntity<Void> unassignTargets(
+            @PathVariable("targetTagId") Long targetTagId,
+            @Schema(description = "List of controller ids to be unassigned", example = "[\"controllerId1\", \"controllerId2\"]")
+            @RequestBody List<String> controllerId);
 
     /**
      * Handles the POST request to assign targets to the given tag id.
      *
+     * @deprecated since 0.6.0 in favour of {@link #assignTargets}
      * @param targetTagId the ID of the target tag to retrieve
      * @param assignedTargetRequestBodies list of controller ids to be assigned
-     *
      * @return the list of assigned targets.
      */
     @Operation(summary = "Assign target(s) to given tagId and return targets",
@@ -385,16 +414,15 @@ public interface MgmtTargetTagRestApi {
             + MgmtRestConstants.TARGET_TAG_TARGETS_REQUEST_MAPPING, consumes = { MediaTypes.HAL_JSON_VALUE,
                     MediaType.APPLICATION_JSON_VALUE }, produces = { MediaTypes.HAL_JSON_VALUE,
                             MediaType.APPLICATION_JSON_VALUE })
-    ResponseEntity<List<MgmtTarget>> assignTargets(@PathVariable("targetTagId") Long targetTagId,
+    @Deprecated(forRemoval = true)
+    ResponseEntity<List<MgmtTarget>> assignTargetsByRequestBody(@PathVariable("targetTagId") Long targetTagId,
             List<MgmtAssignedTargetRequestBody> assignedTargetRequestBodies);
 
     /**
      * Handles the DELETE request to unassign one target from the given tag id.
      *
-     * @param targetTagId
+     * @param targetTagId the ID of the target tag
-     *            the ID of the target tag
+     * @param controllerId the ID of the target to unassign
-     * @param controllerId
-     *            the ID of the target to unassign
      * @return http status code
      */
     @Operation(summary = "Unassign target from a given tagId",

hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetTagResource.java

@@ -179,17 +179,21 @@ public class MgmtTargetTagResource implements MgmtTargetTagRestApi {
     }
 
     @Override
-    public ResponseEntity<Void> assignTargetsByControllerIds(
+    public ResponseEntity<Void> assignTargets(final Long targetTagId, final List<String> controllerIds) {
-            @PathVariable("targetTagId") Long targetTagId,
+        log.debug("Assign {} targets for target tag {}", controllerIds.size(), targetTagId);
-            @Schema(description = "List of controller ids to be assigned", example = "[\"controllerId1\", \"controllerId2\"]")
+        this.targetManagement.assignTag(controllerIds, targetTagId);
-            @RequestBody List<String> assignedTargetControlIds) {
-        log.debug("Assign {} targets for target tag {}", assignedTargetControlIds.size(), targetTagId);
-        this.targetManagement.assignTag(assignedTargetControlIds, targetTagId);
         return ResponseEntity.ok().build();
     }
 
     @Override
-    public ResponseEntity<List<MgmtTarget>> assignTargets(@PathVariable("targetTagId") final Long targetTagId,
+    public ResponseEntity<Void> unassignTargets(final Long targetTagId, final List<String> controllerIds) {
+        log.debug("Unassign {} targets for target tag {}", controllerIds.size(), targetTagId);
+        this.targetManagement.unassignTag(controllerIds, targetTagId);
+        return ResponseEntity.ok().build();
+    }
+
+    @Override
+    public ResponseEntity<List<MgmtTarget>> assignTargetsByRequestBody(@PathVariable("targetTagId") final Long targetTagId,
             @RequestBody final List<MgmtAssignedTargetRequestBody> assignedTargetRequestBodies) {
         log.debug("Assign targets {} for target tag {}", assignedTargetRequestBodies, targetTagId);
         final List<Target> assignedTarget = this.targetManagement

hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetTagResourceTest.java

@@ -304,7 +304,7 @@ public class MgmtTargetTagResourceTest extends AbstractManagementApiIntegrationT
     @Description("Verfies that tag assignments done through tag API command are correctly stored in the repository.")
     @ExpectEvents({ @Expect(type = TargetTagCreatedEvent.class, count = 1),
             @Expect(type = TargetCreatedEvent.class, count = 2), @Expect(type = TargetUpdatedEvent.class, count = 2) })
-    public void assignTargets() throws Exception {
+    public void assignTargetsByRequestBody() throws Exception {
         final TargetTag tag = testdataFactory.createTargetTags(1, "").get(0);
         final int targetsAssigned = 2;
         final List<Target> targets = testdataFactory.createTargets(targetsAssigned);