diff --git a/backend/building-block/src/test/kotlin/com/ritense/buildingblock/service/BuildingBlockStartableItemVersionIT.kt b/backend/building-block/src/test/kotlin/com/ritense/buildingblock/service/BuildingBlockStartableItemVersionIT.kt new file mode 100644 index 0000000000..524ca9b766 --- /dev/null +++ b/backend/building-block/src/test/kotlin/com/ritense/buildingblock/service/BuildingBlockStartableItemVersionIT.kt @@ -0,0 +1,235 @@ +/* + * Copyright 2015-2026 Ritense BV, the Netherlands. + * + * Licensed under EUPL, Version 1.2 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.ritense.buildingblock.service + +import com.fasterxml.jackson.databind.ObjectMapper +import com.ritense.authorization.AuthorizationContext.Companion.runWithoutAuthorization +import com.ritense.buildingblock.BaseIntegrationTest +import com.ritense.buildingblock.repository.BuildingBlockInstanceRepository +import com.ritense.buildingblock.web.rest.dto.CreateCaseDefinitionBuildingBlockLinkDto +import com.ritense.document.domain.impl.request.ModifyDocumentRequest +import com.ritense.document.domain.impl.request.NewDocumentRequest +import com.ritense.processdocument.domain.impl.request.ModifyDocumentAndStartProcessRequest +import com.ritense.processdocument.domain.impl.request.NewDocumentAndStartProcessRequest +import com.ritense.processdocument.service.ProcessDocumentService +import com.ritense.valtimo.contract.buildingblock.BuildingBlockDefinitionId +import com.ritense.valtimo.contract.case_.CaseDefinitionId +import com.ritense.valtimo.service.OperatonProcessService +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.operaton.bpm.engine.RepositoryService +import org.operaton.bpm.engine.RuntimeService +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.transaction.annotation.Transactional +import java.io.ByteArrayInputStream +import java.util.UUID + +/** + * Regression tests for GZAC issue 819: a building block version that is linked to a case as an action + * must be started at that exact version, even when a newer draft version of the same building block has + * redeployed the same process definition key under a higher engine version. + */ +@Transactional +class BuildingBlockStartableItemVersionIT @Autowired constructor( + private val processDocumentService: ProcessDocumentService, + private val caseDefinitionBuildingBlockLinkService: CaseDefinitionBuildingBlockLinkService, + private val buildingBlockInstanceRepository: BuildingBlockInstanceRepository, + private val repositoryService: RepositoryService, + private val runtimeService: RuntimeService, + private val operatonProcessService: OperatonProcessService, + private val objectMapper: ObjectMapper, +) : BaseIntegrationTest() { + + @Test + fun `should start the linked building block version and not the latest one`() { + val linkedVersionProcessDefinitionId = mainProcessDefinitionIdOf(BUILDING_BLOCK_VERSION) + linkBuildingBlockToCase() + val caseDocumentId = startCase() + + deployNewerBuildingBlockVersionOfMainProcess() + + // Guard: both versions share one process key, so a lookup by key alone is ambiguous. + assertThat(deployedVersionTagsOfMainProcess()) + .contains("BB:$BUILDING_BLOCK_KEY:$BUILDING_BLOCK_VERSION", "BB:$BUILDING_BLOCK_KEY:$DRAFT_VERSION") + + val result = runWithoutAuthorization { + processDocumentService.modifyDocumentAndStartProcess( + ModifyDocumentAndStartProcessRequest( + MAIN_PROCESS_KEY, + ModifyDocumentRequest(caseDocumentId.toString(), objectMapper.createObjectNode()) + ).withProcessDefinitionId(linkedVersionProcessDefinitionId) + ) + } + + assertThat(result.errors()).isEmpty() + val processInstanceId = result.resultingProcessInstanceId().orElseThrow().toString() + val startedDefinitionId = runtimeService.createProcessInstanceQuery() + .processInstanceId(processInstanceId) + .singleResult() + .processDefinitionId + assertThat(startedDefinitionId).isEqualTo(linkedVersionProcessDefinitionId) + + // The listener derives the building block version from the started definition's version tag, so + // starting the wrong version leaves the case without a building block instance altogether. + val instances = buildingBlockInstancesOf(caseDocumentId) + assertThat(instances).hasSize(1) + assertThat(instances.first().definition.id) + .isEqualTo(BuildingBlockDefinitionId.of(BUILDING_BLOCK_KEY, BUILDING_BLOCK_VERSION)) + } + + @Test + fun `should fail instead of guessing a version when only the process definition key is given`() { + linkBuildingBlockToCase() + val caseDocumentId = startCase() + deployNewerBuildingBlockVersionOfMainProcess() + + val result = runWithoutAuthorization { + processDocumentService.modifyDocumentAndStartProcess( + ModifyDocumentAndStartProcessRequest( + MAIN_PROCESS_KEY, + ModifyDocumentRequest(caseDocumentId.toString(), objectMapper.createObjectNode()) + ) + ) + } + + assertThat(result.errors()).isNotEmpty() + assertThat(buildingBlockInstancesOf(caseDocumentId)).isEmpty() + } + + @Test + fun `should resolve a standalone building block start by its building block blueprint`() { + val linkedVersionProcessDefinitionId = mainProcessDefinitionIdOf(BUILDING_BLOCK_VERSION) + deployNewerBuildingBlockVersionOfMainProcess() + + val result = runWithoutAuthorization { + processDocumentService.newDocumentAndStartProcess( + NewDocumentAndStartProcessRequest( + MAIN_PROCESS_KEY, + NewDocumentRequest( + BUILDING_BLOCK_KEY, + null, + null, + BUILDING_BLOCK_KEY, + BUILDING_BLOCK_VERSION, + objectMapper.createObjectNode() + ) + ) + ) + } + + assertThat(result.errors()).isEmpty() + val startedDefinitionId = runtimeService.createProcessInstanceQuery() + .processInstanceId(result.resultingProcessInstanceId().orElseThrow().toString()) + .singleResult() + .processDefinitionId + assertThat(startedDefinitionId).isEqualTo(linkedVersionProcessDefinitionId) + } + + /** + * Resolves the process definition of a building block version by its version tag rather than through + * the `main` link, so the test does not depend on state other integration tests may have committed. + */ + private fun mainProcessDefinitionIdOf(versionTag: String): String { + return repositoryService.createProcessDefinitionQuery() + .processDefinitionKey(MAIN_PROCESS_KEY) + .versionTag("BB:$BUILDING_BLOCK_KEY:$versionTag") + .orderByProcessDefinitionVersion() + .desc() + .list() + .firstOrNull() + ?.id + ?: throw IllegalStateException("No process definition for building block $BUILDING_BLOCK_KEY:$versionTag") + } + + private fun buildingBlockInstancesOf(caseDocumentId: UUID) = + buildingBlockInstanceRepository.findAll().filter { it.caseDocumentId == caseDocumentId } + + private fun linkBuildingBlockToCase() { + runWithoutAuthorization { + caseDefinitionBuildingBlockLinkService.createLink( + CaseDefinitionId.of(CASE_DEFINITION_KEY, CASE_DEFINITION_VERSION), + CreateCaseDefinitionBuildingBlockLinkDto(BUILDING_BLOCK_KEY, BUILDING_BLOCK_VERSION) + ) + } + } + + /** + * Redeploys the building block's main process under a newer building block version tag - the same + * thing creating a draft version does (see BuildingBlockDefinitionEventListener.copyProcessDefinitions), + * but done directly so the test does not depend on the state of the shared `bezwaar` fixture. + */ + private fun deployNewerBuildingBlockVersionOfMainProcess() { + val bpmn = requireNotNull(javaClass.classLoader.getResourceAsStream(MAIN_PROCESS_RESOURCE)) { + "Missing test resource $MAIN_PROCESS_RESOURCE" + }.use { it.readBytes() } + + runWithoutAuthorization { + operatonProcessService.deploy( + BuildingBlockDefinitionId.of(BUILDING_BLOCK_KEY, DRAFT_VERSION), + "$MAIN_PROCESS_KEY.bpmn", + ByteArrayInputStream(bpmn), + true, + true + ) + } + + // Both versions now share one process definition key, which is exactly the ambiguity that made the + // wrong version start. + assertThat(mainProcessDefinitionIdOf(DRAFT_VERSION)) + .isNotEqualTo(mainProcessDefinitionIdOf(BUILDING_BLOCK_VERSION)) + } + + private fun deployedVersionTagsOfMainProcess(): List { + return repositoryService.createProcessDefinitionQuery() + .processDefinitionKey(MAIN_PROCESS_KEY) + .list() + .map { it.versionTag } + } + + private fun startCase(): UUID { + val result = runWithoutAuthorization { + processDocumentService.newDocumentAndStartProcess( + NewDocumentAndStartProcessRequest( + CASE_MAIN_PROCESS_KEY, + NewDocumentRequest( + CASE_DEFINITION_KEY, + CASE_DEFINITION_KEY, + CASE_DEFINITION_VERSION, + objectMapper.createObjectNode() + ) + ) + ) + } + return result.resultingDocument() + .orElseThrow { IllegalStateException("Case document not created: ${result.errors()}") } + .id() + .id + } + + private companion object { + const val BUILDING_BLOCK_KEY = "bezwaar" + const val BUILDING_BLOCK_VERSION = "1.0.0" + // Deliberately distinctive so other integration tests in this module cannot have created it. + const val DRAFT_VERSION = "8.1.9" + const val CASE_DEFINITION_KEY = "bb-case" + const val CASE_DEFINITION_VERSION = "1.0.0" + const val CASE_MAIN_PROCESS_KEY = "bb-case-plain-main" + const val MAIN_PROCESS_KEY = "building-block-process" + const val MAIN_PROCESS_RESOURCE = + "config/building-block/bezwaar/1-0-0/bpmn/building-block-process.bpmn" + } +} diff --git a/backend/building-block/src/test/resources/config/case/bb-case/1-0-0/bpmn/bb-case-plain-main.bpmn b/backend/building-block/src/test/resources/config/case/bb-case/1-0-0/bpmn/bb-case-plain-main.bpmn new file mode 100644 index 0000000000..32d3562821 --- /dev/null +++ b/backend/building-block/src/test/resources/config/case/bb-case/1-0-0/bpmn/bb-case-plain-main.bpmn @@ -0,0 +1,39 @@ + + + + + Flow_1 + + + Flow_1 + Flow_2 + + + Flow_2 + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/backend/case/src/main/java/com/ritense/document/domain/DocumentDefinition.java b/backend/case/src/main/java/com/ritense/document/domain/DocumentDefinition.java index 78b793cb66..408884a117 100644 --- a/backend/case/src/main/java/com/ritense/document/domain/DocumentDefinition.java +++ b/backend/case/src/main/java/com/ritense/document/domain/DocumentDefinition.java @@ -16,9 +16,11 @@ package com.ritense.document.domain; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; import com.ritense.document.domain.validation.DocumentContentValidationResult; +import com.ritense.valtimo.contract.BlueprintId; import com.ritense.valtimo.contract.buildingblock.BuildingBlockDefinitionId; import com.ritense.valtimo.contract.case_.CaseDefinitionId; import java.time.temporal.Temporal; @@ -46,6 +48,18 @@ interface Id { @JsonProperty default BuildingBlockDefinitionId buildingBlockDefinitionId() { return null; } + + /** + * The blueprint this document definition belongs to, regardless of its type. A document + * definition is owned by either a case definition or a building block definition, so use + * this whenever the caller does not care which of the two it is - {@link #caseDefinitionId()} + * on its own returns {@code null} for building block documents. + */ + @JsonIgnore + default BlueprintId asBlueprintId() { + CaseDefinitionId caseDefinitionId = caseDefinitionId(); + return caseDefinitionId != null ? caseDefinitionId : buildingBlockDefinitionId(); + } } } \ No newline at end of file diff --git a/backend/core/src/main/java/com/ritense/valtimo/service/OperatonProcessService.java b/backend/core/src/main/java/com/ritense/valtimo/service/OperatonProcessService.java index 802ca54017..d89ec34439 100644 --- a/backend/core/src/main/java/com/ritense/valtimo/service/OperatonProcessService.java +++ b/backend/core/src/main/java/com/ritense/valtimo/service/OperatonProcessService.java @@ -25,6 +25,7 @@ import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byActive; import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byBlueprintId; import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byKey; +import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byKeyOfUnlinkedProcess; import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byLatestVersion; import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byNotLinkedToBuildingBlock; import static com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.byNotLinkedToCaseDefinition; @@ -268,22 +269,62 @@ public ProcessInstanceWithDefinition startProcess( ) { final OperatonProcessDefinition processDefinition = AuthorizationContext .runWithoutAuthorization(() -> { - var pd = operatonRepositoryService.findProcessDefinition( - // TODO: FIX THIS NOW - byKey(processDefinitionKey).and(byBlueprintId(blueprintId)) - ); - if (pd != null) { - return pd; - } else { - // Needed by the VerzoekPlugin: - return operatonRepositoryService.findProcessDefinition( - byKey(processDefinitionKey).and(OperatonProcessDefinitionSpecificationHelper.maxVersionOf(byNotLinkedToCaseDefinition())) + if (blueprintId != null) { + var pd = operatonRepositoryService.findProcessDefinition( + byKey(processDefinitionKey).and(byBlueprintId(blueprintId)) ); + if (pd != null) { + return pd; + } } + // Needed by the VerzoekPlugin: + return operatonRepositoryService.findProcessDefinition(byKeyOfUnlinkedProcess(processDefinitionKey)); }); if (processDefinition == null) { - throw new IllegalStateException("No process definition found with key: '" + processDefinitionKey + "' and blueprintId: '" + blueprintId + "'"); + throw new IllegalStateException( + "No process definition found with key: '" + processDefinitionKey + "' and blueprintId: '" + blueprintId + + "'" + deployedVersionTagsSuffixFor(processDefinitionKey) + ); } + + return startProcessInstance(processDefinition, businessKey, variables); + } + + /** + * Starts the exact process definition identified by {@code processDefinitionId}. Prefer this over + * {@link #startProcess(String, String, BlueprintId, Map)} whenever the caller already knows which + * version has to be started - for example because it came from a process link - since resolving a + * process definition from its key alone cannot tell versions apart. + */ + public ProcessInstanceWithDefinition startProcessById( + String processDefinitionId, + String businessKey, + Map variables + ) { + final OperatonProcessDefinition processDefinition = AuthorizationContext + .runWithoutAuthorization(() -> operatonRepositoryService.findProcessDefinitionById(processDefinitionId)); + if (processDefinition == null) { + throw new ProcessDefinitionNotFoundException("definition with id: '" + processDefinitionId + "'"); + } + // Resolving by key never returned a detached definition. Starting by id bypasses that filter, so + // guard explicitly: a detached definition has been superseded by a newer deployment and starting + // it would run a process that no longer belongs to any blueprint version. + if (processDefinition.getVersionTag() != null + && processDefinition.getVersionTag().startsWith(DETACHED_PROCESS_DEFINITION_PREFIX)) { + throw new IllegalStateException( + "Process definition '" + processDefinitionId + "' has been superseded by a newer deployment" + + " and can no longer be started. Reload the case and try again." + ); + } + + return startProcessInstance(processDefinition, businessKey, variables); + } + + private ProcessInstanceWithDefinition startProcessInstance( + OperatonProcessDefinition processDefinition, + String businessKey, + Map variables + ) { businessKey = businessKey.equals(UNDEFINED_BUSINESS_KEY) ? null : businessKey; authorizationService.requirePermission( @@ -313,6 +354,21 @@ public ProcessInstanceWithDefinition startProcess( } } + /** + * Lists the version tags deployed under a process key, to explain why a lookup missed. A blueprint's + * process can only be found by its version tag, so seeing the tags that do exist is what tells the + * reader whether the key is unknown or the blueprint version is. + */ + private String deployedVersionTagsSuffixFor(String processDefinitionKey) { + String versionTags = AuthorizationContext.runWithoutAuthorization(() -> + operatonRepositoryService.findProcessDefinitions(byKey(processDefinitionKey)).stream() + .map(definition -> definition.getVersionTag() == null ? "" : definition.getVersionTag()) + .distinct() + .collect(Collectors.joining(", ")) + ); + return versionTags.isEmpty() ? "" : ". Version tags deployed for this key: " + versionTags; + } + /** * @deprecated Please use getDefinitionByKeyAndCaseDefinition(...) */ diff --git a/backend/core/src/main/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelper.kt b/backend/core/src/main/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelper.kt index 67097243f6..2745d3bb6d 100644 --- a/backend/core/src/main/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelper.kt +++ b/backend/core/src/main/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelper.kt @@ -121,6 +121,19 @@ class OperatonProcessDefinitionSpecificationHelper { } } + /** + * Matches the latest version of a process key that does not belong to a case definition or a + * building block. Definitions owned by a blueprint must be resolved by their version tag, never by + * key, because every blueprint version redeploys the same key under a new engine version. + */ + @JvmStatic + fun byKeyOfUnlinkedProcess(processDefinitionKey: String): Specification { + val unlinked = byNotLinkedToCaseDefinition().and(byNotLinkedToBuildingBlock()) + return byKey(processDefinitionKey) + .and(unlinked) + .and(maxVersionOf(unlinked)) + } + @JvmStatic fun byNotLinkedToCaseDefinition() = Specification { root, _, cb -> cb.or( diff --git a/backend/core/src/test/java/com/ritense/valtimo/service/OperatonProcessServiceTest.java b/backend/core/src/test/java/com/ritense/valtimo/service/OperatonProcessServiceTest.java index ec43df4835..f0a3d6f135 100644 --- a/backend/core/src/test/java/com/ritense/valtimo/service/OperatonProcessServiceTest.java +++ b/backend/core/src/test/java/com/ritense/valtimo/service/OperatonProcessServiceTest.java @@ -25,14 +25,23 @@ import static org.hamcrest.collection.IsCollectionWithSize.hasSize; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.inOrder; 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.ritense.authorization.AuthorizationContext; import com.ritense.authorization.AuthorizationService; +import com.ritense.authorization.AuthorizationSupportedHelper; +import com.ritense.valtimo.exception.ProcessDefinitionNotFoundException; import com.ritense.valtimo.operaton.domain.OperatonHistoricProcessInstance; +import com.ritense.valtimo.operaton.domain.OperatonProcessDefinition; import com.ritense.valtimo.operaton.repository.OperatonDecisionDefinitionRepository; import com.ritense.valtimo.operaton.repository.OperatonExecutionRepository; import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionRepository; @@ -47,6 +56,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; import javax.xml.parsers.DocumentBuilderFactory; @@ -60,6 +70,7 @@ import org.operaton.bpm.model.bpmn.instance.ServiceTask; import org.operaton.bpm.model.dmn.Dmn; import org.operaton.bpm.model.dmn.DmnModelInstance; +import org.operaton.bpm.engine.runtime.ProcessInstance; import org.operaton.bpm.model.dmn.instance.Decision; import org.hamcrest.Matcher; import org.hamcrest.core.IsEqual; @@ -67,7 +78,9 @@ import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.core.ResolvableType; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; @@ -83,6 +96,8 @@ class OperatonProcessServiceTest { private static final LocalDateTime FIRST_OF_JANUARY_2017 = getDate(2017,1, 1); private static final LocalDateTime FIRST_OF_JANUARY_2016 = getDate(2016,1, 1); + private static final String PROCESS_DEFINITION_ID = "test-process:2:2f8b1e3a"; + private static final String BUSINESSKEY1 = "businessKey1"; private static final String BUSINESSKEY2 = "businessKey2"; private static final String BUSINESSKEY3 = "businessKey3"; @@ -148,6 +163,12 @@ class OperatonProcessServiceTest { @BeforeEach public void beforeEach() { MockitoAnnotations.openMocks(this); + // RelatedEntityAuthorizationRequest validates the resource type against the Spring context on + // construction, so the start-process tests need AuthorizationSupportedHelper to be initialised. + var applicationContext = mock(ApplicationContext.class); + when(applicationContext.getBeanNamesForType(any(ResolvableType.class))) + .thenReturn(new String[] {"authorizationSpecificationFactory"}); + AuthorizationSupportedHelper.INSTANCE.setApplicationContext(applicationContext); } @Test @@ -413,6 +434,105 @@ private BpmnModelInstance modelWithNoExtensionAttrs() { String.format(MINIMAL_BPMN_TEMPLATE, "").getBytes(StandardCharsets.UTF_8))); } + @Test + void startProcessByIdShouldStartTheExactDefinitionWithoutResolvingByKey() { + var processDefinition = processDefinition(PROCESS_DEFINITION_ID, null, false); + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(processDefinition); + when(formService.submitStartForm(eq(PROCESS_DEFINITION_ID), eq(BUSINESSKEY1), any())) + .thenReturn(mock(ProcessInstance.class, RETURNS_DEEP_STUBS)); + + var result = AuthorizationContext.runWithoutAuthorization(() -> createService() + .startProcessById(PROCESS_DEFINITION_ID, BUSINESSKEY1, Map.of())); + + assertThat(result.getProcessDefinition(), is(processDefinition)); + verify(formService).submitStartForm(eq(PROCESS_DEFINITION_ID), eq(BUSINESSKEY1), any()); + // The whole point of starting by id: the lossy resolution by key must not run. + verify(operatonRepositoryService, never()).findProcessDefinition(any()); + } + + @Test + void startProcessByIdShouldThrowWhenDefinitionDoesNotExist() { + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(null); + + var service = createService(); + assertThrows( + ProcessDefinitionNotFoundException.class, + () -> AuthorizationContext.runWithoutAuthorization( + () -> service.startProcessById(PROCESS_DEFINITION_ID, BUSINESSKEY1, Map.of())) + ); + } + + @Test + void startProcessByIdShouldThrowWhenDefinitionIsDetached() { + var detached = processDefinition( + PROCESS_DEFINITION_ID, + OperatonProcessService.DETACHED_PROCESS_DEFINITION_PREFIX + "BB:bezwaar:1.0.0", + false + ); + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(detached); + + var service = createService(); + assertThrows( + IllegalStateException.class, + () -> AuthorizationContext.runWithoutAuthorization( + () -> service.startProcessById(PROCESS_DEFINITION_ID, BUSINESSKEY1, Map.of())) + ); + verify(formService, never()).submitStartForm(any(), any(), any()); + } + + @Test + void startProcessByIdShouldActivateAndReSuspendASuspendedDefinition() { + var suspended = processDefinition(PROCESS_DEFINITION_ID, null, true); + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(suspended); + when(formService.submitStartForm(eq(PROCESS_DEFINITION_ID), any(), any())) + .thenReturn(mock(ProcessInstance.class, RETURNS_DEEP_STUBS)); + + AuthorizationContext.runWithoutAuthorization(() -> createService() + .startProcessById(PROCESS_DEFINITION_ID, BUSINESSKEY1, Map.of())); + + var inOrder = inOrder(repositoryService, formService); + inOrder.verify(repositoryService).activateProcessDefinitionById(PROCESS_DEFINITION_ID); + inOrder.verify(formService).submitStartForm(eq(PROCESS_DEFINITION_ID), any(), any()); + inOrder.verify(repositoryService).suspendProcessDefinitionById(PROCESS_DEFINITION_ID); + } + + @Test + void startProcessByIdShouldReSuspendADefinitionWhenStartingFails() { + var suspended = processDefinition(PROCESS_DEFINITION_ID, null, true); + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(suspended); + when(formService.submitStartForm(eq(PROCESS_DEFINITION_ID), any(), any())) + .thenThrow(new IllegalStateException("boom")); + + var service = createService(); + assertThrows( + IllegalStateException.class, + () -> AuthorizationContext.runWithoutAuthorization( + () -> service.startProcessById(PROCESS_DEFINITION_ID, BUSINESSKEY1, Map.of())) + ); + verify(repositoryService).suspendProcessDefinitionById(PROCESS_DEFINITION_ID); + } + + @Test + void startProcessByIdShouldTranslateTheUndefinedBusinessKeyToNull() { + var processDefinition = processDefinition(PROCESS_DEFINITION_ID, null, false); + when(operatonRepositoryService.findProcessDefinitionById(PROCESS_DEFINITION_ID)).thenReturn(processDefinition); + when(formService.submitStartForm(any(), any(), any())) + .thenReturn(mock(ProcessInstance.class, RETURNS_DEEP_STUBS)); + + AuthorizationContext.runWithoutAuthorization(() -> createService() + .startProcessById(PROCESS_DEFINITION_ID, "UNDEFINED_BUSINESS_KEY", Map.of())); + + verify(formService).submitStartForm(eq(PROCESS_DEFINITION_ID), isNull(), any()); + } + + private OperatonProcessDefinition processDefinition(String id, String versionTag, boolean suspended) { + var processDefinition = mock(OperatonProcessDefinition.class); + when(processDefinition.getId()).thenReturn(id); + when(processDefinition.getVersionTag()).thenReturn(versionTag); + when(processDefinition.isSuspended()).thenReturn(suspended); + return processDefinition; + } + private BpmnModelInstance modelWithCamundaExpression(String expression) { return Bpmn.readModelFromStream(new ByteArrayInputStream( String.format(MINIMAL_BPMN_TEMPLATE, " camunda:expression=\"" + expression + "\"") diff --git a/backend/core/src/test/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelperIntTest.kt b/backend/core/src/test/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelperIntTest.kt index 7a1cad0303..688e0f5d9d 100644 --- a/backend/core/src/test/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelperIntTest.kt +++ b/backend/core/src/test/kotlin/com/ritense/valtimo/operaton/repository/OperatonProcessDefinitionSpecificationHelperIntTest.kt @@ -1,6 +1,7 @@ package com.ritense.valtimo.operaton.repository import com.ritense.valtimo.BaseIntegrationTest +import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.byKeyOfUnlinkedProcess import org.assertj.core.api.Assertions import org.operaton.bpm.engine.RepositoryService import org.junit.jupiter.api.Test @@ -109,6 +110,42 @@ class OperatonProcessDefinitionSpecificationHelperIntTest @Autowired constructor Assertions.assertThat(resultIds).doesNotContain(version1Id) } + @Test + @Transactional + fun `unlinked spec should prefer an untagged definition over a higher versioned building block one`() { + // Version 1 of this key is deployed by the case fixture and therefore carries a CD: version tag, + // so deploy two more: one that stays untagged and a higher one standing in for a building block. + val untaggedId = deployUserTaskProcess() + val buildingBlockDefinitionId = deployUserTaskProcess() + definitionRepository.setVersionTag(buildingBlockDefinitionId, "BB:bezwaar:1.0.1") + + val resultIds = definitionRepository.findAll(byKeyOfUnlinkedProcess(USER_TASK_PROCESS)).map { it.id } + + Assertions.assertThat(resultIds).containsExactly(untaggedId) + Assertions.assertThat(resultIds).doesNotContain(buildingBlockDefinitionId) + } + + private fun deployUserTaskProcess(): String { + return repositoryService.createDeployment() + .addClasspathResource("config/case/everything/1-0-0/bpmn/$USER_TASK_PROCESS.bpmn") + .deployWithResult() + .deployedProcessDefinitions.first() + .id + } + + @Test + @Transactional + fun `unlinked spec should match nothing when every version of the key belongs to a building block`() { + repositoryService.createProcessDefinitionQuery() + .processDefinitionKey(USER_TASK_PROCESS) + .list() + .forEach { definitionRepository.setVersionTag(it.id, "BB:bezwaar:1.0.0") } + + val resultIds = definitionRepository.findAll(byKeyOfUnlinkedProcess(USER_TASK_PROCESS)).map { it.id } + + Assertions.assertThat(resultIds).isEmpty() + } + companion object { const val USER_TASK_PROCESS = "user-task-process" } diff --git a/backend/form-view-model/src/main/kotlin/com/ritense/formviewmodel/service/ProcessAuthorizationService.kt b/backend/form-view-model/src/main/kotlin/com/ritense/formviewmodel/service/ProcessAuthorizationService.kt index 1ddce4bd99..a3b719e13c 100644 --- a/backend/form-view-model/src/main/kotlin/com/ritense/formviewmodel/service/ProcessAuthorizationService.kt +++ b/backend/form-view-model/src/main/kotlin/com/ritense/formviewmodel/service/ProcessAuthorizationService.kt @@ -12,8 +12,7 @@ import com.ritense.valtimo.operaton.service.OperatonRepositoryService import com.ritense.valtimo.contract.annotation.SkipComponentScan import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.byBlueprintId import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.byKey -import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.maxVersionOf -import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.byNotLinkedToCaseDefinition +import com.ritense.valtimo.operaton.repository.OperatonProcessDefinitionSpecificationHelper.Companion.byKeyOfUnlinkedProcess import org.springframework.stereotype.Service @Service @@ -27,16 +26,13 @@ class ProcessAuthorizationService( processDefinitionKey: String, document: JsonSchemaDocument? = null, ) { + // Authorization has to be checked against the definition that will actually be started, so this + // mirrors how OperatonProcessService resolves one: by version tag when the document belongs to a + // blueprint, and otherwise the latest version of the key among processes without a blueprint. val processDefinition = runWithoutAuthorization { - operatonRepositoryService.findProcessDefinition( - byKey(processDefinitionKey) - .and(byBlueprintId(document?.definitionId()?.caseDefinitionId())) - ) - // Needed by form-view-model - ?: operatonRepositoryService.findProcessDefinition( - byKey(processDefinitionKey) - .and(maxVersionOf(byNotLinkedToCaseDefinition())) - ) + document?.definitionId()?.asBlueprintId()?.let { + operatonRepositoryService.findProcessDefinition(byKey(processDefinitionKey).and(byBlueprintId(it))) + } ?: operatonRepositoryService.findProcessDefinition(byKeyOfUnlinkedProcess(processDefinitionKey)) } require(processDefinition != null) diff --git a/backend/form/src/main/kotlin/com/ritense/form/service/impl/DefaultFormSubmissionService.kt b/backend/form/src/main/kotlin/com/ritense/form/service/impl/DefaultFormSubmissionService.kt index a4847da132..f87de899e9 100644 --- a/backend/form/src/main/kotlin/com/ritense/form/service/impl/DefaultFormSubmissionService.kt +++ b/backend/form/src/main/kotlin/com/ritense/form/service/impl/DefaultFormSubmissionService.kt @@ -140,6 +140,7 @@ class DefaultFormSubmissionService( document, taskInstanceId, documentDefinitionNameToUse, + processDefinition.id, processDefinition.key, processDefinition.getBlueprintId(), categorizedKeyValues.createDocumentWithContent, @@ -399,6 +400,7 @@ class DefaultFormSubmissionService( document: Document?, taskInstanceId: String?, documentDefinitionName: String, + processDefinitionId: String, processDefinitionKey: String, blueprintId: BlueprintId?, documentContent: JsonNode, @@ -413,6 +415,7 @@ class DefaultFormSubmissionService( if (document == null) { newDocumentAndStartProcessRequest( documentDefinitionName, + processDefinitionId, processDefinitionKey, blueprintId, documentContent, @@ -421,6 +424,7 @@ class DefaultFormSubmissionService( } else { modifyDocumentAndStartProcessRequest( document, + processDefinitionId, processDefinitionKey, documentContent, withProcessVars, @@ -445,6 +449,7 @@ class DefaultFormSubmissionService( private fun newDocumentAndStartProcessRequest( documentDefinitionName: String, + processDefinitionId: String, processDefinitionKey: String, blueprintId: BlueprintId?, documentContent: JsonNode, @@ -488,11 +493,12 @@ class DefaultFormSubmissionService( ) ).withProcessVars(withProcessVars) } - } + }.withProcessDefinitionId(processDefinitionId) } private fun modifyDocumentAndStartProcessRequest( document: Document, + processDefinitionId: String, processDefinitionKey: String, documentContent: JsonNode, withProcessVars: Map, @@ -505,6 +511,7 @@ class DefaultFormSubmissionService( documentContent ).withJsonPatch(withJsonPatch) ).withProcessVars(withProcessVars) + .withProcessDefinitionId(processDefinitionId) } private fun modifyDocumentAndCompleteTaskRequest( diff --git a/backend/form/src/test/kotlin/com/ritense/form/service/DefaultFormSubmissionServiceTest.kt b/backend/form/src/test/kotlin/com/ritense/form/service/DefaultFormSubmissionServiceTest.kt index 45fc84349a..b7a446e08a 100644 --- a/backend/form/src/test/kotlin/com/ritense/form/service/DefaultFormSubmissionServiceTest.kt +++ b/backend/form/src/test/kotlin/com/ritense/form/service/DefaultFormSubmissionServiceTest.kt @@ -49,6 +49,7 @@ import com.ritense.processlink.domain.ActivityTypeWithEventName.USER_TASK_CREATE import com.ritense.processlink.service.ProcessLinkService import com.ritense.valtimo.operaton.domain.OperatonProcessDefinition import com.ritense.valtimo.operaton.service.OperatonRepositoryService +import com.ritense.valtimo.contract.buildingblock.BuildingBlockDefinitionId import com.ritense.valtimo.contract.case_.CaseDefinitionId import com.ritense.valtimo.contract.event.ExternalDataSubmittedEvent import com.ritense.valtimo.contract.json.MapperSingleton @@ -61,6 +62,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.isA import org.mockito.kotlin.mock import org.mockito.kotlin.times @@ -132,6 +134,7 @@ class DefaultFormSubmissionServiceTest { formProcessLink = formProcessLink() processDefinition = mock() + whenever(processDefinition.id).thenReturn(PROCESS_DEFINITION_ID) whenever(processDefinition.key).thenReturn("myProcessDefinitionKey") whenever(processDefinition.getBlueprintId()).thenReturn(CaseDefinitionId("test", "1.0.0")) whenever(repositoryService.findProcessDefinitionById(formProcessLink.processDefinitionId)) @@ -334,6 +337,74 @@ class DefaultFormSubmissionServiceTest { assertThat(documentNotFoundException.errors()).isNotEmpty() } + @Test + fun `should pass the process link's process definition id when starting a process for a new document`() { + val formData = formData() + val document = createDocument(JsonDocumentContent.build(formData), caseDefinitionId) + whenever(processDocumentService.dispatch(any())) + .thenReturn(ModifyDocumentAndCompleteTaskResultSucceeded(document)) + + defaultFormSubmissionService.handleSubmission( + processLinkId = formProcessLink(START_EVENT_START).id, + formData = formData, + documentId = null, + taskInstanceId = null, + documentDefinitionName = "aName" + ) + + val captor = argumentCaptor() + verify(processDocumentService).dispatch(captor.capture()) + assertThat(captor.firstValue.processDefinitionId()).isEqualTo(PROCESS_DEFINITION_ID) + } + + @Test + fun `should pass the process link's process definition id when starting a process for an existing document`() { + val documentId = UUID.randomUUID().toString() + val formData = formData() + val document = createDocument(JsonDocumentContent.build(formData), caseDefinitionId) + whenever(documentService.get(documentId)).thenReturn(document) + whenever(processDocumentService.dispatch(any())) + .thenReturn(ModifyDocumentAndCompleteTaskResultSucceeded(document)) + + defaultFormSubmissionService.handleSubmission( + processLinkId = formProcessLink(START_EVENT_START).id, + formData = formData, + documentId = documentId, + taskInstanceId = null, + documentDefinitionName = null + ) + + val captor = argumentCaptor() + verify(processDocumentService).dispatch(captor.capture()) + assertThat(captor.firstValue.processDefinitionId()).isEqualTo(PROCESS_DEFINITION_ID) + } + + @Test + fun `should pass the process definition id for a building block process started from a case document`() { + val documentId = UUID.randomUUID().toString() + val formData = formData() + val document = createDocument(JsonDocumentContent.build(formData), caseDefinitionId) + whenever(documentService.get(documentId)).thenReturn(document) + // A building block's main process is started from the case document, so the blueprint on the + // process definition is a building block id while the document belongs to a case definition. + whenever(processDefinition.getBlueprintId()) + .thenReturn(BuildingBlockDefinitionId.of("bezwaar", "1.0.0")) + whenever(processDocumentService.dispatch(any())) + .thenReturn(ModifyDocumentAndCompleteTaskResultSucceeded(document)) + + defaultFormSubmissionService.handleSubmission( + processLinkId = formProcessLink(START_EVENT_START).id, + formData = formData, + documentId = documentId, + taskInstanceId = null, + documentDefinitionName = null + ) + + val captor = argumentCaptor() + verify(processDocumentService).dispatch(captor.capture()) + assertThat(captor.firstValue.processDefinitionId()).isEqualTo(PROCESS_DEFINITION_ID) + } + private fun formProcessLink(activityType: ActivityTypeWithEventName = USER_TASK_CREATE): FormProcessLink { val formProcessLink = FormProcessLink( id = UUID.randomUUID(), @@ -443,5 +514,6 @@ class DefaultFormSubmissionServiceTest { companion object { private const val USERNAME = "test@test.com" private const val PROCESS_DEFINITION_KEY = "formlink-one-task-process" + private const val PROCESS_DEFINITION_ID = "myProcessDefinitionKey:1:33333333-3333-3333-3333-333333333333" } } diff --git a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/ModifyDocumentAndStartProcessRequest.java b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/ModifyDocumentAndStartProcessRequest.java index 5bd3885acc..5c1d866c6d 100644 --- a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/ModifyDocumentAndStartProcessRequest.java +++ b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/ModifyDocumentAndStartProcessRequest.java @@ -38,6 +38,14 @@ public class ModifyDocumentAndStartProcessRequest implements Request { @JsonProperty private String processInstanceId; + /** + * The exact process definition version to start. Deliberately {@link JsonIgnore}: this request is + * bound from client JSON by ProcessDocumentResource, and letting a caller supply a process + * definition id would bypass resolution by key and blueprint entirely. + */ + @JsonIgnore + private String processDefinitionId; + @JsonIgnore private Map processVars; @@ -74,6 +82,16 @@ public ModifyDocumentAndStartProcessRequest withProcessVars(Map return this; } + public ModifyDocumentAndStartProcessRequest withProcessDefinitionId(String processDefinitionId) { + this.processDefinitionId = processDefinitionId; + return this; + } + + @JsonIgnore + public String processDefinitionId() { + return processDefinitionId; + } + public Map getProcessVars() { return processVars; } diff --git a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/NewDocumentAndStartProcessRequest.java b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/NewDocumentAndStartProcessRequest.java index 0990867bcf..d275dfa807 100644 --- a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/NewDocumentAndStartProcessRequest.java +++ b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/NewDocumentAndStartProcessRequest.java @@ -38,6 +38,14 @@ public class NewDocumentAndStartProcessRequest implements Request { @JsonProperty private String processInstanceId; + /** + * The exact process definition version to start. Deliberately {@link JsonIgnore}: this request is + * bound from client JSON by ProcessDocumentResource, and letting a caller supply a process + * definition id would bypass resolution by key and blueprint entirely. + */ + @JsonIgnore + private String processDefinitionId; + @JsonIgnore private Map processVars; @@ -66,6 +74,16 @@ public NewDocumentAndStartProcessRequest withProcessVars(Map pro return this; } + public NewDocumentAndStartProcessRequest withProcessDefinitionId(String processDefinitionId) { + this.processDefinitionId = processDefinitionId; + return this; + } + + @JsonIgnore + public String processDefinitionId() { + return processDefinitionId; + } + public void setProcessInstanceId(String processInstanceId) { this.processInstanceId = processInstanceId; } diff --git a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/StartProcessForDocumentRequest.java b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/StartProcessForDocumentRequest.java index f88b3c335e..7a7abb63a0 100644 --- a/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/StartProcessForDocumentRequest.java +++ b/backend/process-document/src/main/java/com/ritense/processdocument/domain/impl/request/StartProcessForDocumentRequest.java @@ -29,6 +29,13 @@ public class StartProcessForDocumentRequest implements Request { private final String processDefinitionKey; private final Map processVars; + /** + * The exact process definition version to start. Resolving by key alone cannot tell versions of a + * blueprint-owned process apart, so callers that already know the version should supply it here. + */ + @JsonIgnore + private String processDefinitionId; + @JsonIgnore private Consumer additionalModifications; @@ -54,6 +61,16 @@ public Map getProcessVars() { return this.processVars; } + public StartProcessForDocumentRequest withProcessDefinitionId(String processDefinitionId) { + this.processDefinitionId = processDefinitionId; + return this; + } + + @JsonIgnore + public String getProcessDefinitionId() { + return this.processDefinitionId; + } + @Override public Request withAdditionalModifications(Consumer function) { this.additionalModifications = function; @@ -77,17 +94,19 @@ public boolean equals(Object o) { StartProcessForDocumentRequest that = (StartProcessForDocumentRequest) o; return Objects.equals(getDocumentId(), that.getDocumentId()) && Objects.equals(getProcessDefinitionKey(), that.getProcessDefinitionKey()) + && Objects.equals(getProcessDefinitionId(), that.getProcessDefinitionId()) && Objects.equals(getProcessVars(), that.getProcessVars()); } @Override public int hashCode() { - return Objects.hash(getDocumentId(), getProcessDefinitionKey(), getProcessVars()); + return Objects.hash(getDocumentId(), getProcessDefinitionKey(), getProcessDefinitionId(), getProcessVars()); } public String toString() { return "StartProcessForDocumentRequest(documentId=" + this.getDocumentId() + ", processDefinitionKey=" + this.getProcessDefinitionKey() + + ", processDefinitionId=" + this.getProcessDefinitionId() + ", processVars=" + this.getProcessVars() + ")"; } } \ No newline at end of file diff --git a/backend/process-document/src/main/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentService.java b/backend/process-document/src/main/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentService.java index fa80f5795d..71e366a331 100644 --- a/backend/process-document/src/main/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentService.java +++ b/backend/process-document/src/main/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentService.java @@ -140,6 +140,7 @@ public NewDocumentAndStartProcessResult newDocumentAndStartProcess( ); final var processInstanceWithDefinition = startProcess( + request.processDefinitionId(), document, processDefinitionKey, request.getProcessVars() @@ -297,7 +298,7 @@ public ModifyDocumentAndStartProcessResult modifyDocumentAndStartProcess( //Part 2 process start final var processDefinitionKey = request.processDefinitionKey(); final var processInstanceWithDefinition = startProcess( - document, processDefinitionKey, request.getProcessVars()); + request.processDefinitionId(), document, processDefinitionKey, request.getProcessVars()); final var operatonProcessInstanceId = new OperatonProcessInstanceId( processInstanceWithDefinition.getProcessInstanceDto().getId() ); @@ -342,7 +343,7 @@ public StartProcessForDocumentResult startProcessForDocument(StartProcessForDocu //Part 2 process start final var processDefinitionKey = request.getProcessDefinitionKey(); final var processInstanceWithDefinition = startProcess( - document, processDefinitionKey, request.getProcessVars()); + request.getProcessDefinitionId(), document, processDefinitionKey, request.getProcessVars()); final var operatonProcessInstanceId = new OperatonProcessInstanceId( processInstanceWithDefinition.getProcessInstanceDto().getId() ); @@ -445,17 +446,33 @@ public JsonSchemaDocument getCaseDocument( return documentService.getDocumentBy(caseDocumentId); } + /** + * Starts the process for a document. When the caller knows the exact process definition version - + * for instance because the request originated from a process link - that version is started + * directly. Otherwise the definition is resolved from its key and the document's blueprint, which + * cannot tell versions of a blueprint-owned process apart. + */ private ProcessInstanceWithDefinition startProcess( + @Nullable String processDefinitionId, Document document, String processDefinitionKey, Map processVars ) { - return runWithoutAuthorization(() -> operatonProcessService.startProcess( - processDefinitionKey, - document.id().toString(), - document.definitionId().caseDefinitionId(), - processVars - )); + return runWithoutAuthorization(() -> { + if (processDefinitionId != null) { + return operatonProcessService.startProcessById( + processDefinitionId, + document.id().toString(), + processVars + ); + } + return operatonProcessService.startProcess( + processDefinitionKey, + document.id().toString(), + document.definitionId().asBlueprintId(), + processVars + ); + }); } private FunctionResult findTaskById(String taskId) { diff --git a/backend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessDocumentsService.kt b/backend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessDocumentsService.kt index cab58a9c5a..d82841afa2 100644 --- a/backend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessDocumentsService.kt +++ b/backend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessDocumentsService.kt @@ -97,7 +97,7 @@ class ProcessDocumentsService( operatonProcessService.startProcess( processDefinitionKey, businessKey, - document.get().definitionId().caseDefinitionId(), + document.get().definitionId().asBlueprintId(), variables ) } else { diff --git a/backend/process-document/src/test/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentServiceTest.java b/backend/process-document/src/test/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentServiceTest.java index 4a2ff76036..6f13eb25aa 100644 --- a/backend/process-document/src/test/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentServiceTest.java +++ b/backend/process-document/src/test/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentServiceTest.java @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -38,6 +39,7 @@ import com.ritense.processdocument.service.result.StartProcessForDocumentResult; import com.ritense.valtimo.operaton.domain.OperatonProcessDefinition; import com.ritense.valtimo.operaton.domain.ProcessInstanceWithDefinition; +import com.ritense.valtimo.contract.buildingblock.BuildingBlockDefinitionId; import com.ritense.valtimo.contract.case_.CaseDefinitionId; import com.ritense.valtimo.service.OperatonProcessService; import com.ritense.valtimo.service.OperatonTaskService; @@ -152,4 +154,77 @@ void startProcessForDocument_shouldReturnSuccessWhenProcessWasStarted() { "test-name" ); } + + @Test + void startProcessForDocument_shouldStartTheExactVersionWhenAProcessDefinitionIdIsSupplied() { + CaseDefinitionId caseDefinitionId = new CaseDefinitionId("house", "1.0.0"); + JsonSchemaDocumentDefinitionId documentDefinitionId = + JsonSchemaDocumentDefinitionId.existingId("testdef", caseDefinitionId); + String processDefinitionId = "test-name:2:cf1b5e21"; + + JsonSchemaDocument document = mock(JsonSchemaDocument.class); + UUID documentUuid = UUID.randomUUID(); + JsonSchemaDocumentId id = JsonSchemaDocumentId.existingId(documentUuid); + when(document.id()).thenReturn(id); + when(document.definitionId()).thenReturn(documentDefinitionId); + doReturn(Optional.of(document)).when(documentService).findBy(id); + + ProcessInstance processInstance = mock(ProcessInstance.class); + when(processInstance.getId()).thenReturn(UUID.randomUUID().toString()); + OperatonProcessDefinition processDefinition = mock(OperatonProcessDefinition.class); + when(processDefinition.getName()).thenReturn("test-name"); + + Map processVars = new HashMap<>(); + ProcessInstanceWithDefinition processInstanceWithDefinition = + new ProcessInstanceWithDefinition(processInstance, processDefinition); + when(operatonProcessService.startProcessById(processDefinitionId, documentUuid.toString(), processVars)) + .thenReturn(processInstanceWithDefinition); + + StartProcessForDocumentResult result = processDocumentService.startProcessForDocument( + new StartProcessForDocumentRequest(id, "test-name", processVars) + .withProcessDefinitionId(processDefinitionId) + ); + + assertTrue(result instanceof StartProcessForDocumentResultSucceeded); + verify(operatonProcessService).startProcessById(processDefinitionId, documentUuid.toString(), processVars); + // Resolving by key cannot tell versions apart, so it must not be consulted at all. + verify(operatonProcessService, never()).startProcess(any(), any(), any(), any()); + } + + @Test + void startProcessForDocument_shouldResolveABuildingBlockDocumentByItsBuildingBlockBlueprint() { + BuildingBlockDefinitionId buildingBlockDefinitionId = BuildingBlockDefinitionId.of("bezwaar", "1.0.0"); + JsonSchemaDocumentDefinitionId documentDefinitionId = + JsonSchemaDocumentDefinitionId.forBuildingBlock("testdef", buildingBlockDefinitionId); + + JsonSchemaDocument document = mock(JsonSchemaDocument.class); + UUID documentUuid = UUID.randomUUID(); + JsonSchemaDocumentId id = JsonSchemaDocumentId.existingId(documentUuid); + when(document.id()).thenReturn(id); + when(document.definitionId()).thenReturn(documentDefinitionId); + doReturn(Optional.of(document)).when(documentService).findBy(id); + + ProcessInstance processInstance = mock(ProcessInstance.class); + when(processInstance.getId()).thenReturn(UUID.randomUUID().toString()); + OperatonProcessDefinition processDefinition = mock(OperatonProcessDefinition.class); + when(processDefinition.getName()).thenReturn("test-name"); + + Map processVars = new HashMap<>(); + ProcessInstanceWithDefinition processInstanceWithDefinition = + new ProcessInstanceWithDefinition(processInstance, processDefinition); + when(operatonProcessService.startProcess( + "test-name", documentUuid.toString(), buildingBlockDefinitionId, processVars + )).thenReturn(processInstanceWithDefinition); + + StartProcessForDocumentResult result = processDocumentService.startProcessForDocument( + new StartProcessForDocumentRequest(id, "test-name", processVars) + ); + + assertTrue(result instanceof StartProcessForDocumentResultSucceeded); + // Previously the case definition id was passed, which is null for a building block document and + // therefore degraded to "latest version of this key". + verify(operatonProcessService).startProcess( + "test-name", documentUuid.toString(), buildingBlockDefinitionId, processVars + ); + } } diff --git a/backend/process-document/src/test/java/com/ritense/processdocument/web/rest/ProcessDocumentResourceTest.java b/backend/process-document/src/test/java/com/ritense/processdocument/web/rest/ProcessDocumentResourceTest.java index 5507dfab2a..1e03f89a97 100644 --- a/backend/process-document/src/test/java/com/ritense/processdocument/web/rest/ProcessDocumentResourceTest.java +++ b/backend/process-document/src/test/java/com/ritense/processdocument/web/rest/ProcessDocumentResourceTest.java @@ -18,10 +18,13 @@ import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; import static com.ritense.valtimo.contract.domain.ValtimoMediaType.APPLICATION_JSON_UTF8_VALUE; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -33,6 +36,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.ritense.case_.domain.definition.CaseDefinition; import com.ritense.case_.service.ActiveCaseDefinitionService; import com.ritense.document.domain.impl.JsonDocumentContent; @@ -66,6 +70,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.springframework.data.web.PageableHandlerMethodArgumentResolver; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.test.context.bean.override.mockito.MockitoBean; @@ -326,4 +331,78 @@ void shouldRejectModifyDocumentAndStartProcessWhenProcessDefinitionKeyIsMissing( .andDo(print()) .andExpect(status().isBadRequest()); } + + /** + * A client-supplied process definition id would bypass resolution by key and blueprint, making it + * possible to start a superseded or suspended definition, or one belonging to a different case. + */ + @Test + void shouldNotBindAClientSuppliedProcessDefinitionIdOnModifyDocumentAndStartProcess() throws Exception { + var content = new JsonDocumentContent("{\"street\": \"Funenparks\"}"); + final CreateDocumentResult result = createDocument(definition(), content); + var resultSucceeded = new ModifyDocumentAndStartProcessResultSucceeded( + result.resultingDocument().orElseThrow(), + new OperatonProcessInstanceId(UUID.randomUUID().toString()) + ); + when(processDocumentService.modifyDocumentAndStartProcess(any())).thenReturn(resultSucceeded); + + var validRequest = new ModifyDocumentAndStartProcessRequest( + "some-key", + new ModifyDocumentRequest(UUID.randomUUID().toString(), objectMapper.readTree("{}")) + ); + var json = (ObjectNode) objectMapper.readTree(TestUtil.convertObjectToJsonBytes(validRequest)); + json.put("processDefinitionId", "evil-process:9:deadbeef"); + + mockMvc.perform( + post("/api/v1/process-document/operation/modify-document-and-start-process") + .characterEncoding(StandardCharsets.UTF_8.name()) + .contentType(APPLICATION_JSON_VALUE) + .content(objectMapper.writeValueAsBytes(json))) + .andDo(print()) + .andExpect(status().isOk()); + + var captor = ArgumentCaptor.forClass(ModifyDocumentAndStartProcessRequest.class); + verify(processDocumentService).modifyDocumentAndStartProcess(captor.capture()); + assertNull(captor.getValue().processDefinitionId()); + } + + @Test + void shouldNotBindAClientSuppliedProcessDefinitionIdOnNewDocumentAndStartProcess() throws Exception { + var content = new JsonDocumentContent("{\"street\": \"Funenparks\"}"); + final CreateDocumentResult result = createDocument(definition(), content); + var resultSucceeded = new NewDocumentAndStartProcessResultSucceeded( + result.resultingDocument().orElseThrow(), + new OperatonProcessInstanceId(UUID.randomUUID().toString()) + ); + when(processDocumentService.newDocumentAndStartProcess(any())).thenReturn(resultSucceeded); + + var validRequest = new NewDocumentAndStartProcessRequest( + "some-key", + new NewDocumentRequest("house", "house", "1.0.0", objectMapper.readTree("{}")) + ); + var json = (ObjectNode) objectMapper.readTree(TestUtil.convertObjectToJsonBytes(validRequest)); + json.put("processDefinitionId", "evil-process:9:deadbeef"); + + mockMvc.perform( + post("/api/v1/process-document/operation/new-document-and-start-process") + .characterEncoding(StandardCharsets.UTF_8.name()) + .contentType(APPLICATION_JSON_VALUE) + .content(objectMapper.writeValueAsBytes(json))) + .andDo(print()) + .andExpect(status().isOk()); + + var captor = ArgumentCaptor.forClass(NewDocumentAndStartProcessRequest.class); + verify(processDocumentService).newDocumentAndStartProcess(captor.capture()); + assertNull(captor.getValue().processDefinitionId()); + } + + @Test + void shouldNotSerialiseTheProcessDefinitionIdOntoTheWire() throws Exception { + var request = new ModifyDocumentAndStartProcessRequest( + "some-key", + new ModifyDocumentRequest(UUID.randomUUID().toString(), objectMapper.createObjectNode()) + ).withProcessDefinitionId("some-process:1:abc"); + + assertFalse(objectMapper.writeValueAsString(request).contains("processDefinitionId")); + } } diff --git a/backend/process-document/src/test/kotlin/com/ritense/processdocument/service/UnlinkedProcessStartIntTest.kt b/backend/process-document/src/test/kotlin/com/ritense/processdocument/service/UnlinkedProcessStartIntTest.kt new file mode 100644 index 0000000000..3dda7bb68e --- /dev/null +++ b/backend/process-document/src/test/kotlin/com/ritense/processdocument/service/UnlinkedProcessStartIntTest.kt @@ -0,0 +1,112 @@ +/* + * Copyright 2015-2026 Ritense BV, the Netherlands. + * + * Licensed under EUPL, Version 1.2 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.ritense.processdocument.service + +import com.fasterxml.jackson.databind.ObjectMapper +import com.ritense.authorization.AuthorizationContext.Companion.runWithoutAuthorization +import com.ritense.document.domain.impl.request.NewDocumentRequest +import com.ritense.document.service.DocumentService +import com.ritense.processdocument.BaseIntegrationTest +import com.ritense.processdocument.domain.impl.request.StartProcessForDocumentRequest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.operaton.bpm.engine.RepositoryService +import org.operaton.bpm.engine.RuntimeService +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.transaction.annotation.Transactional + +/** + * Unlinked ("system") process definitions have no version tag, so they can only ever be resolved by key. + * Excluding blueprint-owned definitions from that key-based lookup must not change anything for them. + */ +@Transactional +class UnlinkedProcessStartIntTest : BaseIntegrationTest() { + + @Autowired + lateinit var processDocumentService: ProcessDocumentService + + @Autowired + lateinit var documentService: DocumentService + + @Autowired + lateinit var repositoryService: RepositoryService + + @Autowired + lateinit var runtimeService: RuntimeService + + @Autowired + lateinit var objectMapper: ObjectMapper + + @Test + fun `should start the latest version of an unlinked process when only its key is given`() { + deploySystemProcess() + val latestDefinitionId = deploySystemProcess() + val documentId = createDocument() + + val result = runWithoutAuthorization { + processDocumentService.startProcessForDocument( + StartProcessForDocumentRequest(documentId, SYSTEM_PROCESS_KEY, emptyMap()) + ) + } + + assertThat(result.errors()).isEmpty() + assertThat(startedDefinitionIdOf(result.processInstanceId().orElseThrow().toString())) + .isEqualTo(latestDefinitionId) + } + + @Test + fun `should start the requested version of an unlinked process when its process definition id is given`() { + val firstDefinitionId = deploySystemProcess() + deploySystemProcess() + val documentId = createDocument() + + val result = runWithoutAuthorization { + processDocumentService.startProcessForDocument( + StartProcessForDocumentRequest(documentId, SYSTEM_PROCESS_KEY, emptyMap()) + .withProcessDefinitionId(firstDefinitionId) + ) + } + + assertThat(result.errors()).isEmpty() + assertThat(startedDefinitionIdOf(result.processInstanceId().orElseThrow().toString())) + .isEqualTo(firstDefinitionId) + } + + private fun deploySystemProcess(): String { + return repositoryService.createDeployment() + .addClasspathResource("bpmn/$SYSTEM_PROCESS_KEY.bpmn") + .deployWithResult() + .deployedProcessDefinitions + .first() + .id + } + + private fun createDocument() = runWithoutAuthorization { + documentService.createDocument( + NewDocumentRequest("house", "house", "1.0.0", objectMapper.readTree("""{"street": "aStreet"}""")) + ).resultingDocument().orElseThrow() + }.id() + + private fun startedDefinitionIdOf(processInstanceId: String) = runtimeService.createProcessInstanceQuery() + .processInstanceId(processInstanceId) + .singleResult() + .processDefinitionId + + private companion object { + const val SYSTEM_PROCESS_KEY = "system-process" + } +} diff --git a/backend/process-document/src/test/resources/bpmn/system-process.bpmn b/backend/process-document/src/test/resources/bpmn/system-process.bpmn new file mode 100644 index 0000000000..c9b755c03e --- /dev/null +++ b/backend/process-document/src/test/resources/bpmn/system-process.bpmn @@ -0,0 +1,42 @@ + + + + + + Flow_1 + + + Flow_1 + Flow_2 + + + Flow_2 + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/backend/process-link-url/src/main/kotlin/com/ritense/processlink/url/service/URLProcessLinkService.kt b/backend/process-link-url/src/main/kotlin/com/ritense/processlink/url/service/URLProcessLinkService.kt index f80020fa3e..299d7c2bc7 100644 --- a/backend/process-link-url/src/main/kotlin/com/ritense/processlink/url/service/URLProcessLinkService.kt +++ b/backend/process-link-url/src/main/kotlin/com/ritense/processlink/url/service/URLProcessLinkService.kt @@ -81,6 +81,7 @@ class URLProcessLinkService( document, taskInstanceId, documentDefinitionNameToUse, + processDefinition.id, processDefinition.key, processDefinition.getBlueprintId() ) @@ -125,6 +126,7 @@ class URLProcessLinkService( document: Document?, taskInstanceId: String?, documentDefinitionName: String, + processDefinitionId: String, processDefinitionKey: String, blueprintId: BlueprintId? ): Request { @@ -132,12 +134,14 @@ class URLProcessLinkService( if (document == null) { newDocumentAndStartProcessRequest( documentDefinitionName, + processDefinitionId, processDefinitionKey, blueprintId ) } else { modifyDocumentAndStartProcessRequest( document, + processDefinitionId, processDefinitionKey, ) } @@ -153,6 +157,7 @@ class URLProcessLinkService( private fun newDocumentAndStartProcessRequest( documentDefinitionName: String, + processDefinitionId: String, processDefinitionKey: String, blueprintId: BlueprintId?, ): NewDocumentAndStartProcessRequest { @@ -194,11 +199,12 @@ class URLProcessLinkService( ) ) } - } + }.withProcessDefinitionId(processDefinitionId) } private fun modifyDocumentAndStartProcessRequest( document: Document, + processDefinitionId: String, processDefinitionKey: String, ): ModifyDocumentAndStartProcessRequest { return ModifyDocumentAndStartProcessRequest( @@ -207,7 +213,7 @@ class URLProcessLinkService( document.id().toString(), objectMapper.createObjectNode() ) - ) + ).withProcessDefinitionId(processDefinitionId) } private fun modifyDocumentAndCompleteTaskRequest( diff --git a/documentation/release-notes/13.x.x/13.41.0/README.md b/documentation/release-notes/13.x.x/13.41.0/README.md index a400ce5c85..360490da5e 100644 --- a/documentation/release-notes/13.x.x/13.41.0/README.md +++ b/documentation/release-notes/13.x.x/13.41.0/README.md @@ -18,6 +18,19 @@ ## Bugfixes +* **Actions now respect the linked building block version** + + Starting a building block from the actions of a case now always runs the version of that building block + that is linked to the case. Previously a newer version of the same building block took over: after + creating a new version and changing its process, starting the action still ran the newer version, which + led to an error when that version wrote to fields the linked version does not have. Changes to other + versions of a building block no longer affect the version that is linked. + + A process that belongs to a building block can now only be started for a specific version. Starting one + by process definition key alone - for example from a custom plugin or a `startProcessByProcessDefinitionKey` + expression outside of a building block - now reports a clear error instead of silently running whichever + version happened to be deployed last. + * **A divider widget without a title no longer shows a dash** A divider widget that is configured without a title now stays empty, both in the widget list on the @@ -30,7 +43,7 @@ Fixed an issue where duplicating a divider opened the duplication dialog with an empty, invalid key that could not be edited, leaving the Duplicate button disabled. The dialog now pre-populates the divider key with a unique default value and allows it to be edited before duplicating. - + * **Start form of a building block now opens in the panel** Starting a building block from the 'Start' menu of a case did nothing when the start form of its