From db52b57053004ed6ee4af5aeb22c155257d0edbe Mon Sep 17 00:00:00 2001 From: Nikol Georgieva Date: Sat, 3 Oct 2026 19:20:50 +0300 Subject: [PATCH] bpm: the Inbox list omits a task completed while it is being built instead of failing with 500 (#7575) Cause: GET /services/inbox/tasks lists the tasks with one query and then reads each one again (identity links, process instance). A task another session completed in between failed that read - the tenant validator's "Task with id [..] not found or does not belong to current tenant", or, when it was its process's last task, a NullPointerException on the ended process instance - and the uncaught exception failed the whole list with a 500 for everyone loading it at that moment. Change: each listed task is mapped through mapListedTask. A per-task read that throws is followed by one tenant-scoped existence query (new isTaskActive on BpmProviderFlowable / BpmService); a task confirmed gone is omitted and logged at DEBUG, while a failure on a task that still exists propagates as before. Applies to both list endpoints (/tasks and /instance/{id}/tasks). Verified: BpmInboxEndpointVanishedTaskTest (new: both race outcomes, the propagate control, the instance listing) and the module suite (56); BpmInboxConcurrentCompletionIT (new: lists continuously while 40 tasks are completed tail-first) - 11 of 36 list calls answered 500 against the unfixed endpoint, all 200 with the fix; BpmTaskLabelKeyIT and IntentWorkflowStatusIT green on H2; formatter:validate with the cache wiped; release-profile javadoc build of the module. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../flowable/config/BpmProviderFlowable.java | 14 ++ .../flowable/endpoint/BpmInboxEndpoint.java | 36 +++- .../bpm/flowable/service/BpmService.java | 10 + .../BpmInboxEndpointVanishedTaskTest.java | 124 +++++++++++ .../api/BpmInboxConcurrentCompletionIT.java | 197 ++++++++++++++++++ 5 files changed, 377 insertions(+), 4 deletions(-) create mode 100644 components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpointVanishedTaskTest.java create mode 100644 tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/BpmInboxConcurrentCompletionIT.java diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmProviderFlowable.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmProviderFlowable.java index dad8d7c2c19..abf42de1c2a 100644 --- a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmProviderFlowable.java +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmProviderFlowable.java @@ -476,6 +476,20 @@ public Optional getProcessLabelKeys(String processDefinitionId .map(catalog -> new ProcessLabelKeys(catalog, catalog + "." + process.getId())); } + /** + * Whether the task is still active in the current tenant - i.e. neither completed nor deleted. + * + * @param taskId the task id + * @return true while the task exists + */ + public boolean isTaskActive(String taskId) { + return processEngine.getTaskService() + .createTaskQuery() + .taskTenantId(getTenantId()) + .taskId(taskId) + .count() > 0; + } + /** * The actions a task's form completes it with, as its user task declares them: * diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpoint.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpoint.java index fc22f6c34a7..0146e937ac7 100644 --- a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpoint.java +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpoint.java @@ -67,13 +67,41 @@ public ResponseEntity> getProcessInstanceTasks(@PathVariable("id") * on every poll (issue #7141). * * @param tasks the tasks, with their process variables loaded - * @return the DTOs + * @return the DTOs, without the tasks completed while the list was being built */ private List mapToDTOs(List tasks) { Map> labelKeys = new HashMap<>(); - return tasks.stream() - .map(task -> mapToDTO(task, labelKeys)) - .collect(Collectors.toList()); + List dtos = new ArrayList<>(tasks.size()); + for (Task task : tasks) { + mapListedTask(task, labelKeys).ifPresent(dtos::add); + } + return dtos; + } + + /** + * Maps one listed task, omitting it when it was completed after the listing query found it (issue + * #7575). The row's per-task reads are statements of their own, so another session completing the + * task in between makes them fail - its identity links are "not found", and when it was its + * process's last task the process instance is gone too - and that one failure used to fail the + * whole list with a 500, for every caller loading it at that moment. + *

+ * Only a task confirmed gone is omitted: a read that failed while the task still exists is a real + * failure and propagates as before. + * + * @param task the listed task + * @param labelKeys the task-label catalogs resolved so far + * @return the DTO, or empty when the task no longer exists + */ + private Optional mapListedTask(Task task, Map> labelKeys) { + try { + return Optional.of(mapToDTO(task, labelKeys)); + } catch (RuntimeException ex) { + if (bpmService.isTaskActive(task.getId())) { + throw ex; + } + logger.debug("Task [{}] was completed while the task list was being built - omitted from the list", task.getId(), ex); + return Optional.empty(); + } } private static PrincipalType extractPrincipalType(String type) { diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/service/BpmService.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/service/BpmService.java index b98036d41ee..4b81a8b53b6 100644 --- a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/service/BpmService.java +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/service/BpmService.java @@ -396,6 +396,16 @@ public void removeVariable(String executionId, String variableName) { bpmProviderFlowable.removeVariable(executionId, variableName); } + /** + * Whether the task is still active in the current tenant - i.e. neither completed nor deleted. + * + * @param taskId the task id + * @return true while the task exists + */ + public boolean isTaskActive(String taskId) { + return bpmProviderFlowable.isTaskActive(taskId); + } + public List getTaskIdentityLinks(String taskId) { return bpmProviderFlowable.getTaskService() .getTaskIdentityLinks(taskId); diff --git a/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpointVanishedTaskTest.java b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpointVanishedTaskTest.java new file mode 100644 index 00000000000..a71dbabcbb1 --- /dev/null +++ b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/endpoint/BpmInboxEndpointVanishedTaskTest.java @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.engine.bpm.flowable.endpoint; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.eclipse.dirigible.components.engine.bpm.flowable.dto.ProcessInstanceData; +import org.eclipse.dirigible.components.engine.bpm.flowable.dto.TaskDTO; +import org.eclipse.dirigible.components.engine.bpm.flowable.service.BpmService; +import org.eclipse.dirigible.components.engine.bpm.flowable.service.PrincipalType; +import org.flowable.task.api.Task; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * A task completed while the Inbox list is being built is omitted, not a 500 for every caller + * (issue #7575). The listing query and each row's per-task reads are separate statements, so a task + * another session completes in between is "not found" by the per-task read - and, when it was its + * process's last task, its process instance is gone too. Only a task confirmed gone is omitted: a + * read failing for any other reason still fails the list. + */ +class BpmInboxEndpointVanishedTaskTest { + + private final BpmService bpmService = mock(BpmService.class); + + private final BpmInboxEndpoint endpoint = new BpmInboxEndpoint(bpmService); + + @BeforeEach + void noLabels() { + when(bpmService.getProcessLabelKeys(anyString())).thenReturn(Optional.empty()); + } + + private Task task(String id) { + Task task = mock(Task.class); + when(task.getId()).thenReturn(id); + when(task.getProcessInstanceId()).thenReturn("instance-" + id); + when(task.getProcessVariables()).thenReturn(Map.of()); + when(bpmService.getProcessInstanceById("instance-" + id)).thenReturn(new ProcessInstanceData()); + when(bpmService.getTaskIdentityLinks(id)).thenReturn(List.of()); + when(bpmService.isTaskActive(id)).thenReturn(true); + return task; + } + + /** The issue's trace: the per-task read's tenant validator no longer finds the completed task. */ + @Test + void aTaskCompletedBeforeItsIdentityLinksAreReadIsOmitted() { + List found = List.of(task("t1"), task("t2"), task("t3")); + when(bpmService.getTaskIdentityLinks("t2")).thenThrow( + new IllegalArgumentException("Task with id [t2] not found or does not belong to current tenant")); + when(bpmService.isTaskActive("t2")).thenReturn(false); + when(bpmService.findTasksWithProcessVariables(PrincipalType.CANDIDATE_GROUPS)).thenReturn(found); + + List tasks = endpoint.getTasks("groups") + .getBody(); + + assertEquals(List.of("t1", "t3"), ids(tasks), "the vanished task is omitted, the rest of the list still answers"); + } + + /** + * Completing a process's last task ends the process: its instance is gone by the time it is read. + */ + @Test + void aTaskWhoseProcessEndedBeforeItWasReadIsOmitted() { + List found = List.of(task("t1"), task("t2")); + when(bpmService.getProcessInstanceById("instance-t1")).thenThrow(new NullPointerException("processInstance")); + when(bpmService.isTaskActive("t1")).thenReturn(false); + when(bpmService.findTasksWithProcessVariables(PrincipalType.ASSIGNEE)).thenReturn(found); + + List tasks = endpoint.getTasks("assignee") + .getBody(); + + assertEquals(List.of("t2"), ids(tasks)); + } + + /** A per-task read that fails while the task is still there is a real failure, not the race. */ + @Test + void aFailureOnATaskThatStillExistsStillFailsTheList() { + List found = List.of(task("t1")); + IllegalStateException failure = new IllegalStateException("database unavailable"); + when(bpmService.getTaskIdentityLinks("t1")).thenThrow(failure); + when(bpmService.findTasksWithProcessVariables(PrincipalType.ASSIGNEE)).thenReturn(found); + + IllegalStateException thrown = assertThrows(IllegalStateException.class, () -> endpoint.getTasks("assignee")); + + assertSame(failure, thrown); + } + + /** The listing of one process instance's tasks takes the same path. */ + @Test + void aProcessInstanceListingOmitsAVanishedTaskToo() { + List found = List.of(task("t1"), task("t2")); + when(bpmService.getTaskIdentityLinks("t1")).thenThrow( + new IllegalArgumentException("Task with id [t1] not found or does not belong to current tenant")); + when(bpmService.isTaskActive("t1")).thenReturn(false); + when(bpmService.findTasksWithProcessVariables("instance", PrincipalType.ASSIGNEE)).thenReturn(found); + + List tasks = endpoint.getProcessInstanceTasks("instance", "assignee") + .getBody(); + + assertEquals(List.of("t2"), ids(tasks)); + } + + private static List ids(List tasks) { + return tasks.stream() + .map(TaskDTO::getId) + .toList(); + } +} diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/BpmInboxConcurrentCompletionIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/BpmInboxConcurrentCompletionIT.java new file mode 100644 index 00000000000..b7a20e2e79b --- /dev/null +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/BpmInboxConcurrentCompletionIT.java @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.integration.tests.api; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Base64; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.eclipse.dirigible.components.initializers.synchronizer.SynchronizationProcessor; +import org.eclipse.dirigible.repository.api.IRepository; +import org.eclipse.dirigible.repository.api.IRepositoryStructure; +import org.eclipse.dirigible.tests.base.IntegrationTest; +import org.eclipse.dirigible.tests.framework.restassured.RestAssuredExecutor; +import org.eclipse.dirigible.tests.framework.tenant.DirigibleTestTenant; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.web.server.LocalServerPort; + +import io.restassured.http.ContentType; + +/** + * Listing the Inbox while its tasks are being completed never fails the list (issue #7575). + * + *

+ * The list is one task query followed by per-task reads, so a task another session completes in + * between is "not found" by its read - and, being its process's only task here, its process + * instance is gone too. That one vanished task used to fail the whole list with a 500 for whoever + * loaded it at that moment. The race is a matter of timing, so this lists continuously while every + * task is being completed and requires every list call to answer: with the fix it cannot fail, + * without it it fails whenever the race is hit. + * + *

+ * The concurrent half uses a plain HTTP client with explicit credentials: + * {@link RestAssuredExecutor} swaps REST Assured's static base URI and authentication around each + * call, which two threads cannot share. + */ +class BpmInboxConcurrentCompletionIT extends IntegrationTest { + + private static final String PROJECT = "bpm-inbox-concurrent-completion-it"; + private static final String PROCESS_KEY = "bpm-inbox-concurrent-completion-it-process"; + private static final String BPMN_REGISTRY_PATH = IRepositoryStructure.PATH_REGISTRY_PUBLIC + "/" + PROJECT + "/process.bpmn"; + private static final String GROUP_TASKS = "/services/inbox/tasks?type=groups"; + private static final int INSTANCES = 40; + private static final long ASSERTION_TIMEOUT_SECONDS = 60; + + @Autowired + private IRepository repository; + + @Autowired + private SynchronizationProcessor synchronizationProcessor; + + @Autowired + private RestAssuredExecutor restAssuredExecutor; + + @LocalServerPort + private int port; + + private final HttpClient http = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(10)) + .build(); + + @AfterEach + void cleanup() { + if (repository.hasResource(BPMN_REGISTRY_PATH)) { + repository.removeResource(BPMN_REGISTRY_PATH); + synchronizationProcessor.forceProcessSynchronizers(); + } + } + + @Test + void the_group_task_list_answers_while_its_tasks_are_completed() throws Exception { + repository.createResource(BPMN_REGISTRY_PATH, BPMN.getBytes(StandardCharsets.UTF_8), false, "application/xml", true); + synchronizationProcessor.forceProcessSynchronizers(); + for (int i = 0; i < INSTANCES; i++) { + startProcess(i); + } + List taskIds = new ArrayList<>(); + restAssuredExecutor.execute(() -> taskIds.addAll(given().when() + .get(GROUP_TASKS) + .then() + .statusCode(200) + .body("findAll { it.processDefinitionId.startsWith('" + PROCESS_KEY + + ":') }", hasSize(INSTANCES)) + .extract() + .>path("findAll { it.processDefinitionId.startsWith('" + + PROCESS_KEY + ":') }.id")), + ASSERTION_TIMEOUT_SECONDS); + + AtomicBoolean completing = new AtomicBoolean(true); + CompletableFuture> listings = CompletableFuture.supplyAsync(() -> { + List statuses = new ArrayList<>(); + while (completing.get()) { + statuses.add(send(HttpRequest.newBuilder(uri(GROUP_TASKS)) + .GET())); + } + return statuses; + }); + // Completed from the END of the list: every list query still holds the tail, and the lister walks + // towards it while it disappears. Completed in list order, the lister would only ever run ahead + // of the completions and never meet a task that vanished behind its own query. + List completionOrder = new ArrayList<>(taskIds); + Collections.reverse(completionOrder); + List completions = new ArrayList<>(); + try { + for (String taskId : completionOrder) { + completions.add(send(HttpRequest.newBuilder(uri("/services/inbox/tasks/" + taskId)) + .header("Content-Type", "application/json") + .POST(HttpRequest.BodyPublishers.ofString("{\"action\":\"COMPLETE\"}")))); + } + } finally { + completing.set(false); + } + List listingStatuses = listings.get(ASSERTION_TIMEOUT_SECONDS, TimeUnit.SECONDS); + + assertTrue(completions.stream() + .allMatch(status -> status == 200), + "every task should have been completed: " + completions); + assertTrue(!listingStatuses.isEmpty(), "the list should have been loaded while the tasks were being completed"); + assertEquals(List.of(), listingStatuses.stream() + .filter(status -> status != 200) + .toList(), + "no list call may fail because a listed task was completed meanwhile (of " + listingStatuses.size() + " calls)"); + } + + private void startProcess(int index) { + String body = "{\"processDefinitionKey\":\"" + PROCESS_KEY + "\",\"businessKey\":\"" + PROCESS_KEY + "-" + index + + "\",\"parameters\":\"{}\"}"; + restAssuredExecutor.execute(() -> given().contentType(ContentType.JSON) + .body(body) + .when() + .post("/services/bpm/bpm-processes/instance") + .then() + .statusCode(200)); + } + + private int send(HttpRequest.Builder request) { + DirigibleTestTenant tenant = DirigibleTestTenant.createDefaultTenant(); + String credentials = Base64.getEncoder() + .encodeToString((tenant.getUsername() + ":" + tenant.getPassword()).getBytes(StandardCharsets.UTF_8)); + try { + return http.send(request.header("Authorization", "Basic " + credentials) + .timeout(Duration.ofSeconds(30)) + .build(), + HttpResponse.BodyHandlers.discarding()) + .statusCode(); + } catch (InterruptedException ex) { + Thread.currentThread() + .interrupt(); + throw new IllegalStateException("Interrupted while calling the inbox", ex); + } catch (java.io.IOException ex) { + throw new IllegalStateException("Failed to call the inbox", ex); + } + } + + private URI uri(String path) { + return URI.create("http://localhost:" + port + path); + } + + /** One candidate-group task, the process's only step: completing it ends the process. */ + private static final String BPMN = """ + + + + + + + + + + + """.formatted(PROCESS_KEY); +}