Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix response code when trying to update a published workflow #224

Merged
merged 2 commits into from
Feb 14, 2023
Merged

Fix response code when trying to update a published workflow #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4dd65f0
Fix response code when trying to update a published workflow.
symphony-soufiane Feb 13, 2023
ec4734e
Merge branch 'master' into fix-response-code-forbidden-operation-2
symphony-soufiane Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions ...low-bot-app/src/main/java/com/symphony/bdk/workflow/exception/GlobalExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@
package com.symphony.bdk.workflow.exception;

import com.symphony.bdk.workflow.api.v1.dto.ErrorResponse;

import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

import javax.persistence.OptimisticLockException;
Expand Down Expand Up @@ -57,6 +53,13 @@ public ResponseEntity<ErrorResponse> handle(IllegalArgumentException exception)
return handle(exception.getMessage(), HttpStatus.BAD_REQUEST);
}

@ExceptionHandler(UnsupportedOperationException.class)
public ResponseEntity<ErrorResponse> handle(UnsupportedOperationException exception) {
log.error("Unsupported operation exception: [{}]", exception.getMessage());
log.debug("", exception);
return handle(exception.getMessage(), HttpStatus.UNPROCESSABLE_ENTITY);
}

@ExceptionHandler(OptimisticLockException.class)
public ResponseEntity<ErrorResponse> handle(OptimisticLockException exception) {
log.error("Optimistic locking exception: [{}]", exception.getMessage());
Expand Down
12 changes: 8 additions & 4 deletions ...bot-app/src/main/java/com/symphony/bdk/workflow/management/WorkflowManagementService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
@Slf4j
public class WorkflowManagementService {
private static final String WORKFLOW_NOT_EXIST_EXCEPTION_MSG = "Workflow %s does not exist.";
private static final String WORKFLOW_UPDATE_FORBIDDEN_EXCEPTION_MSG = "Update on a published Workflow is forbidden.";
private final WorkflowEngine<CamundaTranslatedWorkflowContext> workflowEngine;
private final VersionedWorkflowRepository versionRepository;
private final ObjectConverter objectConverter;
Expand Down Expand Up @@ -104,14 +105,17 @@ public Optional<VersionedWorkflowView> get(String id, Long version) {
}

private VersionedWorkflow readAndValidate(Workflow workflow) {
Optional<VersionedWorkflow> activeVersion = versionRepository.findByWorkflowIdAndPublishedFalse(workflow.getId());
List<VersionedWorkflow> activeVersion = versionRepository.findByWorkflowId(workflow.getId());
if (activeVersion.isEmpty()) {
throw new NotFoundException(String.format(WORKFLOW_NOT_EXIST_EXCEPTION_MSG, workflow.getId()));
}
if (activeVersion.get().getPublished()) {
throw new IllegalArgumentException("Update on a published Workflow is forbidden.");

if (activeVersion.stream().anyMatch(VersionedWorkflow::getPublished)) {
throw new UnsupportedOperationException(WORKFLOW_UPDATE_FORBIDDEN_EXCEPTION_MSG);
}
return activeVersion.get();

// The list can only contain 1 draft version item
return activeVersion.get(0);
}

public void delete(String id) {
Expand Down
15 changes: 12 additions & 3 deletions ...bot-app/src/test/java/com/symphony/bdk/workflow/exception/GlobalExceptionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package com.symphony.bdk.workflow.exception;

import static org.assertj.core.api.Assertions.assertThat;

import com.symphony.bdk.workflow.api.v1.dto.ErrorResponse;

import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;

import static org.assertj.core.api.Assertions.assertThat;

class GlobalExceptionHandlerTest {

private final GlobalExceptionHandler globalExceptionHandler = new GlobalExceptionHandler();
Expand All @@ -32,6 +31,16 @@ void testIllegalArgumentException() {
assertThat(response.getBody()).isEqualTo(expectedErrorResponse);
}

@Test
void testUnsupportedOperationException() {
ErrorResponse expectedErrorResponse = new ErrorResponse("Unsupported operation exception's message");
ResponseEntity<ErrorResponse> response = globalExceptionHandler.handle(
new UnsupportedOperationException("Unsupported operation exception's message"));

assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY);
assertThat(response.getBody()).isEqualTo(expectedErrorResponse);
}

@Test
void testNotFoundException() {
ErrorResponse expectedErrorResponse = new ErrorResponse("NotFound exception's message");
Expand Down
51 changes: 33 additions & 18 deletions ...app/src/test/java/com/symphony/bdk/workflow/management/WorkflowManagementServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
package com.symphony.bdk.workflow.management;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.github.fge.jsonschema.core.exceptions.ProcessingException;
import com.symphony.bdk.workflow.api.v1.dto.SwadlView;
import com.symphony.bdk.workflow.converter.ObjectConverter;
import com.symphony.bdk.workflow.engine.WorkflowEngine;
Expand All @@ -23,8 +11,6 @@
import com.symphony.bdk.workflow.swadl.v1.Workflow;
import com.symphony.bdk.workflow.versioning.model.VersionedWorkflow;
import com.symphony.bdk.workflow.versioning.repository.VersionedWorkflowRepository;

import com.github.fge.jsonschema.core.exceptions.ProcessingException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
Expand All @@ -33,8 +19,22 @@
import org.mockito.junit.jupiter.MockitoExtension;

import java.io.IOException;
import java.util.Collections;
import java.util.Optional;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
public class WorkflowManagementServiceTest {

Expand Down Expand Up @@ -144,7 +144,7 @@ void testUpdate_latestNoPublishedVersion_publishSucceed() {
when(conveter.convert(anyString(), eq(Workflow.class))).thenReturn(workflow);
VersionedWorkflow noPublishedVersion = new VersionedWorkflow();
noPublishedVersion.setPublished(false);
when(versionRepository.findByWorkflowIdAndPublishedFalse(anyString())).thenReturn(Optional.of(noPublishedVersion));
when(versionRepository.findByWorkflowId(anyString())).thenReturn(Collections.singletonList(noPublishedVersion));
CamundaTranslatedWorkflowContext context = mock(CamundaTranslatedWorkflowContext.class);
when(camundaEngine.translate(any(Workflow.class))).thenReturn(context);
when(camundaEngine.deploy(any(CamundaTranslatedWorkflowContext.class))).thenReturn("id");
Expand All @@ -167,7 +167,7 @@ void testUpdate_latestNoPublishedVersion_updateOnlySucceed() {
VersionedWorkflow noPublishedVersion = new VersionedWorkflow();
noPublishedVersion.setPublished(false);
noPublishedVersion.setActive(false);
when(versionRepository.findByWorkflowIdAndPublishedFalse(anyString())).thenReturn(Optional.of(noPublishedVersion));
when(versionRepository.findByWorkflowId(anyString())).thenReturn(Collections.singletonList(noPublishedVersion));
CamundaTranslatedWorkflowContext context = mock(CamundaTranslatedWorkflowContext.class);
when(camundaEngine.translate(any(Workflow.class))).thenReturn(context);
when(versionRepository.save(any(VersionedWorkflow.class))).thenReturn(noPublishedVersion);
Expand All @@ -179,10 +179,25 @@ void testUpdate_latestNoPublishedVersion_updateOnlySucceed() {
verify(camundaEngine, never()).deploy(any(CamundaTranslatedWorkflowContext.class));
}

@Test
void testUpdate_updatePublishedWorkflow() {
Properties properties = new Properties();
properties.setPublish(true);
workflow.setProperties(properties);
when(conveter.convert(anyString(), eq(Workflow.class))).thenReturn(workflow);
VersionedWorkflow publishedVersion = new VersionedWorkflow();
publishedVersion.setPublished(true);
when(versionRepository.findByWorkflowId(anyString())).thenReturn(Collections.singletonList(publishedVersion));

assertThatThrownBy(() -> workflowManagementService.update(swadlView))
.isInstanceOf(UnsupportedOperationException.class)
.hasMessage("Update on a published Workflow is forbidden.");
}

@Test
void testUpdate_noActiveVersion_notFoundException() {
when(conveter.convert(anyString(), eq(Workflow.class))).thenReturn(workflow);
when(versionRepository.findByWorkflowIdAndPublishedFalse(anyString())).thenReturn(Optional.empty());
when(versionRepository.findByWorkflowId(anyString())).thenReturn(Collections.emptyList());

assertThatThrownBy(() -> workflowManagementService.update(swadlView)).isInstanceOf(NotFoundException.class);
}
Expand Down