Action history cleanup/purge initial (#2728)
* Action history cleanup/purge initial Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * apply changes after review Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * fix hibernate build by annotating delete methods with transactional annotation Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * changes after review and new test cases for new requirements * accept 0 for keep last Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * Fix ManagementSecurityTest Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * apply object utils check Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * fix for oldestAction deletion Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * remove unused comment Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * rename action ids variable Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * Fix access control handling Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com> --------- Signed-off-by: strailov <Stanislav.Trailov@bosch.io> Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com> Co-authored-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
committed by
GitHub
parent
9984c89183
commit
f1c3d0175e
@@ -12,6 +12,7 @@ package org.eclipse.hawkbit.mgmt.rest.resource;
|
||||
import static org.eclipse.hawkbit.mgmt.rest.resource.util.PagingUtility.sanitizeActionSortParam;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.eclipse.hawkbit.audit.AuditLog;
|
||||
import org.eclipse.hawkbit.mgmt.json.model.PagedList;
|
||||
import org.eclipse.hawkbit.mgmt.json.model.action.MgmtAction;
|
||||
import org.eclipse.hawkbit.mgmt.rest.api.MgmtActionRestApi;
|
||||
@@ -26,8 +27,11 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.domain.Slice;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.ObjectUtils;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
@Slf4j
|
||||
@RestController
|
||||
@ConditionalOnProperty(name = "hawkbit.rest.MgmtActionResource.enabled", matchIfMissing = true)
|
||||
@@ -71,6 +75,34 @@ public class MgmtActionResource implements MgmtActionRestApi {
|
||||
return ResponseEntity.ok(MgmtActionMapper.toResponse(action, MgmtRepresentationMode.FULL));
|
||||
}
|
||||
|
||||
@Override
|
||||
@AuditLog(entity = "Actions",type = AuditLog.Type.DELETE, description = "Delete Action", logResponse = true)
|
||||
public ResponseEntity<Void> deleteAction(Long actionId) {
|
||||
deploymentManagement.deleteAction(actionId);
|
||||
return ResponseEntity.ok().build();
|
||||
}
|
||||
|
||||
@Override
|
||||
@AuditLog(entity = "Actions", type = AuditLog.Type.DELETE, description = "Delete Actions", logResponse = true)
|
||||
public ResponseEntity<Void> deleteActions(String rsqlParam, List<Long> actionIds) {
|
||||
|
||||
final boolean isActionIdsNotEmpty = !ObjectUtils.isEmpty(actionIds);
|
||||
if (!ObjectUtils.isEmpty(rsqlParam)) {
|
||||
|
||||
if (isActionIdsNotEmpty) {
|
||||
throw new IllegalArgumentException("Only one of the parameters should be provided!");
|
||||
}
|
||||
|
||||
deploymentManagement.deleteActionsByRsql(rsqlParam);
|
||||
} else if (isActionIdsNotEmpty) {
|
||||
deploymentManagement.deleteActionsByIds(actionIds);
|
||||
} else {
|
||||
throw new IllegalArgumentException("Either action id list or rsql filter should be provided.");
|
||||
}
|
||||
|
||||
return ResponseEntity.ok().build();
|
||||
}
|
||||
|
||||
private MgmtRepresentationMode getRepresentationModeFromString(final String representationModeParam) {
|
||||
return MgmtRepresentationMode.fromValue(representationModeParam)
|
||||
.orElseGet(() -> {
|
||||
|
||||
@@ -73,6 +73,7 @@ import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.domain.Slice;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.ObjectUtils;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
|
||||
/**
|
||||
@@ -226,6 +227,26 @@ public class MgmtTargetResource implements MgmtTargetRestApi {
|
||||
return ResponseEntity.ok(new PagedList<>(MgmtTargetMapper.toResponse(targetId, activeActions.getContent()), totalActionCount));
|
||||
}
|
||||
|
||||
@Override
|
||||
@AuditLog(entity = "Target", type = AuditLog.Type.DELETE, description = "Delete Actions For Target")
|
||||
public ResponseEntity<Void> deleteActionsForTarget(final String targetId, final int keepLast, final List<Long> actionIds) {
|
||||
|
||||
if (keepLast < 0 && ObjectUtils.isEmpty(actionIds)) {
|
||||
throw new IllegalArgumentException("Either keepLast OR action ID list should be provided!");
|
||||
}
|
||||
|
||||
if (!ObjectUtils.isEmpty(actionIds)) {
|
||||
if (keepLast >= 0) {
|
||||
throw new IllegalArgumentException("Only one of the parameters should be provided!");
|
||||
}
|
||||
deploymentManagement.deleteTargetActionsByIds(targetId, actionIds);
|
||||
} else {
|
||||
deploymentManagement.deleteOldestTargetActions(targetId, keepLast);
|
||||
}
|
||||
|
||||
return ResponseEntity.ok().build();
|
||||
}
|
||||
|
||||
@Override
|
||||
public ResponseEntity<MgmtAction> getAction(final String targetId, final Long actionId) {
|
||||
return getValidatedAction(targetId, actionId)
|
||||
|
||||
@@ -35,6 +35,7 @@ import org.eclipse.hawkbit.repository.model.Rollout;
|
||||
import org.eclipse.hawkbit.repository.model.Target;
|
||||
import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.test.web.servlet.ResultActions;
|
||||
|
||||
/**
|
||||
@@ -479,9 +480,6 @@ class MgmtActionResourceTest extends AbstractManagementApiIntegrationTest {
|
||||
mvc.perform(put(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isMethodNotAllowed());
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isMethodNotAllowed());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -510,6 +508,105 @@ class MgmtActionResourceTest extends AbstractManagementApiIntegrationTest {
|
||||
.andExpect(status().isNotFound());
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldSuccessfullyDeleteSingleAction() throws Exception {
|
||||
List<DistributionSetAssignmentResult> assignmentResults = createTargetsAndPerformAssignment(2);
|
||||
Action action1 = assignmentResults.get(0).getAssignedEntity().get(0);
|
||||
Action action2 = assignmentResults.get(1).getAssignedEntity().get(0);
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING)
|
||||
.param(MgmtRestConstants.REQUEST_PARAMETER_SORTING, "ID:ASC"))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk())
|
||||
|
||||
// verify action 1
|
||||
.andExpect(jsonPath("content.[0].id", equalTo(action1.getId().intValue())))
|
||||
.andExpect(jsonPath("content.[1].id", equalTo(action2.getId().intValue())));
|
||||
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + action1.getId()))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk());
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + action1.getId()))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isNotFound());
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldSuccessfullyDeleteMultipleActions() throws Exception {
|
||||
final List<DistributionSetAssignmentResult> assignmentResults = createTargetsAndPerformAssignment(4);
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING)
|
||||
.param(MgmtRestConstants.REQUEST_PARAMETER_SORTING, "ID:ASC"))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk())
|
||||
|
||||
// verify action 1
|
||||
.andExpect(jsonPath("content.[0].id", equalTo(assignmentResults.get(0).getAssignedEntity().get(0).getId().intValue())))
|
||||
.andExpect(jsonPath("content.[1].id", equalTo(assignmentResults.get(1).getAssignedEntity().get(0).getId().intValue())))
|
||||
.andExpect(jsonPath("content.[2].id", equalTo(assignmentResults.get(2).getAssignedEntity().get(0).getId().intValue())))
|
||||
.andExpect(jsonPath("content.[3].id", equalTo(assignmentResults.get(3).getAssignedEntity().get(0).getId().intValue())));
|
||||
|
||||
final List<Long> actionIdsToDelete = new ArrayList<>();
|
||||
long deletedActionId1 = assignmentResults.get(2).getAssignedEntity().get(0).getId();
|
||||
actionIdsToDelete.add(deletedActionId1);
|
||||
long deletedActionId2 = assignmentResults.get(3).getAssignedEntity().get(0).getId();
|
||||
actionIdsToDelete.add(deletedActionId2);
|
||||
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING)
|
||||
.content(toJson(actionIdsToDelete)).contentType(MediaType.APPLICATION_JSON))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + deletedActionId1))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isNotFound());
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + deletedActionId2))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isNotFound());
|
||||
Action deletedAction3 = assignmentResults.get(1).getAssignedEntity().get(0);
|
||||
String rsql = "target.name==" + deletedAction3.getTarget().getName();
|
||||
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING)
|
||||
.param(MgmtRestConstants.REQUEST_PARAMETER_SEARCH, rsql).contentType(MediaType.APPLICATION_JSON))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + deletedAction3.getId()))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isNotFound());
|
||||
|
||||
mvc.perform(get(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING + "/" + assignmentResults.get(0).getAssignedEntity().get(0).getId()))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk());
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldReceiveBadRequestWhenNeeded() throws Exception {
|
||||
// bad request on both empty parameters
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING).contentType(MediaType.APPLICATION_JSON))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
// bad request when both parameters are present
|
||||
mvc.perform(delete(MgmtRestConstants.ACTION_V1_REQUEST_MAPPING).contentType(MediaType.APPLICATION_JSON)
|
||||
.param(MgmtRestConstants.REQUEST_PARAMETER_SEARCH, "target.name==test")
|
||||
.content(toJson(List.of(1,2,3)))
|
||||
.contentType(MediaType.APPLICATION_JSON))
|
||||
.andExpect(status().isBadRequest());
|
||||
}
|
||||
|
||||
private List<DistributionSetAssignmentResult> createTargetsAndPerformAssignment(int n) {
|
||||
final List<Target> targets = new ArrayList<>();
|
||||
for (int i = 0; i < n; i++) {
|
||||
targets.add(testdataFactory.createTarget("target-" + i));
|
||||
}
|
||||
|
||||
final List<DistributionSetAssignmentResult> results = new ArrayList<>();
|
||||
for (Target target : targets) {
|
||||
DistributionSet distributionSet = testdataFactory.createDistributionSet();
|
||||
results.add(assignDistributionSet(distributionSet.getId(), target.getControllerId()));
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
private static String generateActionLink(final String targetId, final Long actionId) {
|
||||
return "http://localhost" + MgmtRestConstants.TARGET_V1_REQUEST_MAPPING +
|
||||
"/" + targetId + "/" + MgmtRestConstants.TARGET_V1_ACTIONS + "/" + actionId;
|
||||
|
||||
@@ -81,6 +81,7 @@ import org.eclipse.hawkbit.util.IpUtil;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.json.JSONArray;
|
||||
import org.json.JSONObject;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.Arguments;
|
||||
@@ -2858,6 +2859,76 @@ class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest {
|
||||
.andExpect(jsonPath("detailStatus", equalTo("error")));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDeletionOfLastNTargetActions() throws Exception {
|
||||
final Target testTarget = testdataFactory.createTarget("testTarget");
|
||||
|
||||
for (int i = 0; i < 10; i++) {
|
||||
final DistributionSet distributionSet = testdataFactory.createDistributionSet();
|
||||
assignDistributionSet(distributionSet.getId(), testTarget.getControllerId());
|
||||
}
|
||||
|
||||
long actionsPerTarget = actionRepository.countByTargetId(testTarget.getId());
|
||||
Assertions.assertEquals(10, actionsPerTarget);
|
||||
List<Action> oldActions = deploymentManagement.findActionsByTarget(testTarget.getControllerId(), PAGE).getContent();
|
||||
|
||||
mvc.perform(delete(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/{targetId}/actions", testTarget.getControllerId())
|
||||
.param("keepLast", "5"))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk());
|
||||
|
||||
//the last 5 actions should be left
|
||||
List<Action> actions = deploymentManagement.findActionsByTarget(testTarget.getControllerId(), PAGE).getContent();
|
||||
Assertions.assertEquals(5, actions.size());
|
||||
for (int i = 0; i < 5; i++) {
|
||||
// last 5 actions should remain
|
||||
Assertions.assertEquals(oldActions.get(i + 5), actions.get(i));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void testThatDeletionOfLastNTargetActionsReturnsBadRequestWhenNeeded() throws Exception {
|
||||
final Target testTarget = testdataFactory.createTarget();
|
||||
// either numberOfActions or actionIds list should be present
|
||||
mvc.perform(delete(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/{targetId}/actions", testTarget.getControllerId()))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
// both parameters present should also lead to bad request
|
||||
mvc.perform(delete(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/{targetId}/actions", testTarget.getControllerId())
|
||||
.param("keepLast", "5")
|
||||
.content(toJson(List.of(1,2,3)))
|
||||
.contentType(MediaType.APPLICATION_JSON))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isBadRequest());
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDeletionOfTargetActionsById() throws Exception {
|
||||
final Target testTarget = testdataFactory.createTarget("testTarget");
|
||||
for (int i = 0; i < 10; i++) {
|
||||
final DistributionSet distributionSet = testdataFactory.createDistributionSet();
|
||||
long dsId = distributionSet.getId();
|
||||
assignDistributionSet(dsId, testTarget.getControllerId());
|
||||
}
|
||||
|
||||
final List<Long> evenActionIds = deploymentManagement.findActionsByTarget(testTarget.getControllerId(), PAGE).getContent()
|
||||
.stream()
|
||||
.filter(action -> action.getId() % 2 == 0)
|
||||
.map(Identifiable::getId).toList();
|
||||
|
||||
mvc.perform(delete(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/{targetId}/actions", testTarget.getControllerId())
|
||||
.content(toJson(evenActionIds))
|
||||
.contentType(MediaType.APPLICATION_JSON))
|
||||
.andDo(MockMvcResultPrinter.print())
|
||||
.andExpect(status().isOk());
|
||||
|
||||
long remaining = actionRepository.countByTargetId(testTarget.getId());
|
||||
Assertions.assertEquals(10 - evenActionIds.size(), remaining);
|
||||
List<Action> remainingActions = deploymentManagement.findActionsByTarget(testTarget.getControllerId(), PAGE).getContent();
|
||||
remainingActions.forEach(action -> Assertions.assertTrue(action.getId() % 2 != 0));
|
||||
}
|
||||
|
||||
private static Stream<Arguments> confirmationOptions() {
|
||||
return Stream.of(Arguments.of(true, true), Arguments.of(true, false), Arguments.of(false, true),
|
||||
Arguments.of(false, false), Arguments.of(true, null), Arguments.of(false, null));
|
||||
|
||||
Reference in New Issue
Block a user