diff --git a/components/core/core-base/src/main/java/org/eclipse/dirigible/components/base/registry/RegistryMutationTracker.java b/components/core/core-base/src/main/java/org/eclipse/dirigible/components/base/registry/RegistryMutationTracker.java new file mode 100644 index 00000000000..76e64ed74b8 --- /dev/null +++ b/components/core/core-base/src/main/java/org/eclipse/dirigible/components/base/registry/RegistryMutationTracker.java @@ -0,0 +1,67 @@ +/* + * 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.base.registry; + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; + +import org.springframework.stereotype.Component; + +/** + * Tracks client requests that mutate the registry, so the synchronization reconciler can tell a + * half-applied write from a finished one. + * + *

+ * A publish replaces a collection by DELETING it and copying it back milliseconds later, and both + * steps can happen inside a single request. A reconciler that looks into that hole sees sources + * that are about to reappear as deleted. It therefore asks here whether a mutation was in flight, + * rather than inferring it from "the registry changed" - the file-system watcher reports a genuine + * deletion exactly the same way, and treating the two alike would delay every deletion by a pass. + */ +@Component +public class RegistryMutationTracker { + + /** Registry-mutating requests currently being served. */ + private final AtomicInteger inFlight = new AtomicInteger(); + + /** Registry-mutating requests served so far - lets an observer detect one that came and went. */ + private final AtomicLong completed = new AtomicLong(); + + /** Marks the start of a registry-mutating request. */ + public void enter() { + inFlight.incrementAndGet(); + } + + /** Marks the end of a registry-mutating request. */ + public void exit() { + inFlight.decrementAndGet(); + completed.incrementAndGet(); + } + + /** + * Whether a registry-mutating request is being served right now. + * + * @return true if at least one such request is in flight + */ + public boolean isMutating() { + return inFlight.get() > 0; + } + + /** + * The number of registry-mutating requests served so far. Compare a value taken earlier with the + * current one to find out whether a mutation completed in between. + * + * @return the count of completed registry-mutating requests + */ + public long completedMutations() { + return completed.get(); + } + +} diff --git a/components/core/core-initializers/src/main/java/org/eclipse/dirigible/components/initializers/synchronizer/SynchronizationProcessor.java b/components/core/core-initializers/src/main/java/org/eclipse/dirigible/components/initializers/synchronizer/SynchronizationProcessor.java index 26130a8e820..d4ad8ebec28 100644 --- a/components/core/core-initializers/src/main/java/org/eclipse/dirigible/components/initializers/synchronizer/SynchronizationProcessor.java +++ b/components/core/core-initializers/src/main/java/org/eclipse/dirigible/components/initializers/synchronizer/SynchronizationProcessor.java @@ -20,6 +20,7 @@ import org.eclipse.dirigible.components.base.artefact.topology.TopologyWrapper; import org.eclipse.dirigible.components.base.healthcheck.status.HealthCheckStatus; import org.eclipse.dirigible.components.base.healthcheck.status.HealthCheckStatus.Jobs.JobStatus; +import org.eclipse.dirigible.components.base.registry.RegistryMutationTracker; import org.eclipse.dirigible.components.base.synchronizer.SynchronizationWatcher; import org.eclipse.dirigible.components.base.synchronizer.Synchronizer; import org.eclipse.dirigible.components.base.synchronizer.SynchronizerCallback; @@ -78,6 +79,9 @@ public class SynchronizationProcessor implements SynchronizationWalkerCallback, /** The synchronization watcher. */ private final SynchronizationWatcher synchronizationWatcher; + /** Tells whether a client is publishing to (or unpublishing from) the registry. */ + private final RegistryMutationTracker registryMutationTracker; + /** The initialized. */ private final AtomicBoolean initialized; @@ -94,16 +98,18 @@ public class SynchronizationProcessor implements SynchronizationWalkerCallback, * @param synchronizers the synchronizers * @param definitionService the definition service * @param synchronizationWatcher the synchronization watcher + * @param registryMutationTracker the registry mutation tracker */ @Autowired public SynchronizationProcessor(IRepository repository, List> synchronizers, DefinitionService definitionService, - SynchronizationWatcher synchronizationWatcher) { + SynchronizationWatcher synchronizationWatcher, RegistryMutationTracker registryMutationTracker) { this.repository = repository; this.synchronizers = Collections.synchronizedList(synchronizers); logger.info("Registered [{}] synchronizers: [{}]", synchronizers.size(), synchronizers); this.definitionService = definitionService; this.synchronizationWatcher = synchronizationWatcher; + this.registryMutationTracker = registryMutationTracker; this.synchronizers.forEach(s -> s.setCallback(this)); this.initialized = new AtomicBoolean(false); @@ -200,6 +206,9 @@ public void processSynchronizers() { .passStarted(); processing.set(true); synchronizationWatcher.reset(); + // Sampled before the walk, compared at cleanup: a publish that came and went while this pass + // ran is as dangerous as one still in flight. + long mutationsBeforePass = registryMutationTracker.completedMutations(); try { @@ -389,7 +398,21 @@ public void processSynchronizers() { logger.trace("Cleaning up removed artefacts..."); - // cleanup + // An artefact whose source is gone is removed - UNLESS a publish was in flight while this + // pass ran. A publish replaces a collection by DELETING it and copying it back + // milliseconds later, and a pass that looks into that hole deletes artefacts whose + // sources are about to reappear. That is not cosmetic: a reconciler that rebuilds from + // the WHOLE artefact set - the client-Java batch compile - then compiles a half-empty + // codebase, the batch fails as a whole, and the instance is left with no controllers + // registered until something else changes. + // + // The question asked is "was a client writing to the registry", NOT "did the registry + // change": the file-system watcher reports a genuine deletion exactly like a publish, so + // keying on it would defer every deletion by a pass. A publish always ends by marking the + // registry modified, which schedules the next pass - and forceProcessSynchronizers() + // waits for it - so the deferred cleanup happens promptly. + boolean registryChangedDuringPass = + registryMutationTracker.isMutating() || registryMutationTracker.completedMutations() != mutationsBeforePass; for (Synchronizer synchronizer : synchronizers) { List registered = synchronizer.getService() .getAll(); @@ -397,7 +420,13 @@ public void processSynchronizers() { if (synchronizer.isAccepted(artefact.getType())) { if (!repository.getResource(IRepositoryStructure.PATH_REGISTRY_PUBLIC + artefact.getLocation()) .exists()) { - synchronizer.cleanup(artefact); + if (registryChangedDuringPass) { + logger.info( + "Source of artefact [{}] is missing, but the registry changed during this pass - deferring its cleanup to the next one", + artefact.getLocation()); + } else { + synchronizer.cleanup(artefact); + } } } } diff --git a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/synchronizer/JavaSynchronizer.java b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/synchronizer/JavaSynchronizer.java index eb0327ffb53..fca2ab8fa0d 100644 --- a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/synchronizer/JavaSynchronizer.java +++ b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/synchronizer/JavaSynchronizer.java @@ -222,9 +222,17 @@ private void rebuildAll() { List sources = new ArrayList<>(all.size()); for (JavaFile file : all) { if (!RegistrySourceLoader.exists(file.getLocation())) { - // The artefact's source disappeared between cycles; let the orchestrator's cleanup - // loop delete the artefact row. We must not include a stale source here. - continue; + // The source of a still-registered artefact is momentarily gone - a publish replaces + // a collection by deleting it and copying it back. Compiling now would submit a + // knowingly incomplete batch: every class referencing a missing one fails, javac can + // emit no bytecode for the whole batch, and the client codebase falls back to its + // last-good state - or, on a first publish, to nothing at all. Rebuild on the next + // cycle instead, by when the file is back or the orchestrator's cleanup has dropped + // the artefact. + LOGGER.info("Java source [{}] is registered but currently missing - deferring the rebuild to the next cycle", + file.getLocation()); + dirty.set(true); + return; } byte[] bytes; try { diff --git a/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilter.java b/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilter.java new file mode 100644 index 00000000000..d65f2ba90ab --- /dev/null +++ b/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilter.java @@ -0,0 +1,64 @@ +/* + * 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.ide.workspace.filter; + +import java.io.IOException; + +import org.eclipse.dirigible.components.base.registry.RegistryMutationTracker; +import org.springframework.http.HttpMethod; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +/** + * Reports a publish / unpublish request to the {@link RegistryMutationTracker} for as long as it is + * being served, so the synchronization reconciler does not mistake the gap between the delete and + * the copy of a replaced collection for a deletion. + * + *

+ * Only mutating methods count: the IDE polls these endpoints with GET all the time, and counting + * those would keep the registry permanently "in mutation". + */ +@Component +class RegistryMutationFilter extends OncePerRequestFilter { + + private final RegistryMutationTracker tracker; + + RegistryMutationFilter(RegistryMutationTracker tracker) { + this.tracker = tracker; + } + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws ServletException, IOException { + tracker.enter(); + try { + chain.doFilter(request, response); + } finally { + tracker.exit(); + } + } + + /** + * Should not filter. + * + * @param request the request + * @return true for requests that cannot mutate the registry + */ + @Override + protected boolean shouldNotFilter(HttpServletRequest request) { + return HttpMethod.GET.matches(request.getMethod()) || HttpMethod.HEAD.matches(request.getMethod()); + } + +} diff --git a/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilterConfig.java b/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilterConfig.java new file mode 100644 index 00000000000..7f621d2fc93 --- /dev/null +++ b/components/ide/ide-workspace/src/main/java/org/eclipse/dirigible/components/ide/workspace/filter/RegistryMutationFilterConfig.java @@ -0,0 +1,37 @@ +/* + * 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.ide.workspace.filter; + +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Registers the {@link RegistryMutationFilter} on the endpoints that publish to - or unpublish from + * - the registry. Registering it explicitly also keeps Spring Boot from mapping it to every + * request: a write through a generated application's REST controller is not a registry mutation. + */ +@Configuration +class RegistryMutationFilterConfig { + + /** The publisher and workspace endpoints - the client paths that write to the registry. */ + private static final String[] URL_PATTERNS = { // + "/services/ide/publisher/*", // + "/services/ide/workspace/*", // + "/services/ide/workspaces/*"}; + + @Bean + FilterRegistrationBean registryMutationFilterRegistrationBean(RegistryMutationFilter filter) { + FilterRegistrationBean registration = new FilterRegistrationBean<>(filter); + registration.addUrlPatterns(URL_PATTERNS); + return registration; + } + +} diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/SynchronizerCleanupRaceIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/SynchronizerCleanupRaceIT.java new file mode 100644 index 00000000000..bd3158d4f96 --- /dev/null +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/SynchronizerCleanupRaceIT.java @@ -0,0 +1,146 @@ +/* + * 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 org.assertj.core.api.Assertions.assertThat; + +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.awaitility.Awaitility; +import org.eclipse.dirigible.components.base.registry.RegistryMutationTracker; +import org.eclipse.dirigible.components.base.synchronizer.SynchronizationWatcher; +import org.eclipse.dirigible.components.initializers.synchronizer.SynchronizationProcessor; +import org.eclipse.dirigible.engine.java.service.JavaFileService; +import org.eclipse.dirigible.repository.api.IRepository; +import org.eclipse.dirigible.repository.api.IRepositoryStructure; +import org.eclipse.dirigible.tests.base.IntegrationTest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * A publish replaces a registry collection by deleting it and copying it back milliseconds later. A + * synchronization pass that looks into that hole used to delete the artefacts of the files that + * were about to reappear - and the client-Java batch compile that follows then rebuilt from a + * half-empty codebase, failed as a whole, and left the instance with nothing registered. + * + *

+ * The pass must therefore defer its cleanup while the registry is being written, and still + * reconcile a genuine deletion once the registry is quiet. + */ +class SynchronizerCleanupRaceIT extends IntegrationTest { + + private static final String LOCATION = "/cleanup-race-it/Sample.java"; + + private static final String REGISTRY_PATH = IRepositoryStructure.PATH_REGISTRY_PUBLIC + LOCATION; + + private static final String SOURCE = """ + package cleanuprace; + + public class Sample { + } + """; + + /** + * A second source published while the registry is changing. Its artefact appearing is the proof + * that a full pass actually ran under those conditions - {@code processSynchronizers()} silently + * skips when another run holds the slot, so without this the assertions below could pass on a pass + * that never happened. + */ + private static final String PROBE_LOCATION = "/cleanup-race-it/Probe.java"; + + private static final String PROBE_REGISTRY_PATH = IRepositoryStructure.PATH_REGISTRY_PUBLIC + PROBE_LOCATION; + + private static final String PROBE_SOURCE = """ + package cleanuprace; + + public class Probe { + } + """; + + /** How long to keep driving passes while the publish is in flight, waiting for one to complete. */ + private static final long PASS_TIMEOUT_SECONDS = 120; + + @Autowired + private IRepository repository; + + @Autowired + private SynchronizationProcessor synchronizationProcessor; + + @Autowired + private SynchronizationWatcher synchronizationWatcher; + + @Autowired + private JavaFileService javaFileService; + + @Autowired + private RegistryMutationTracker registryMutationTracker; + + @Test + void an_artefact_survives_a_pass_that_races_a_publish() { + repository.createResource(REGISTRY_PATH, SOURCE.getBytes(StandardCharsets.UTF_8), false, "text/plain", true); + synchronizationProcessor.forceProcessSynchronizers(); + + assertThat(javaFileService.findByLocation(LOCATION)).as("the published source is registered") + .isNotEmpty(); + + // The publish hole: the source is gone for a moment, while a publish request is being served. + repository.removeResource(REGISTRY_PATH); + repository.createResource(PROBE_REGISTRY_PATH, PROBE_SOURCE.getBytes(StandardCharsets.UTF_8), false, "text/plain", true); + runPassesWhilePublishIsInFlight(); + + assertThat(javaFileService.findByLocation(LOCATION)).as("an artefact whose source vanished mid-publish must not be cleaned up") + .isNotEmpty(); + + // Once the publish is over, the deletion is reconciled as before - in a single forced call. + synchronizationProcessor.forceProcessSynchronizers(); + + assertThat(javaFileService.findByLocation(LOCATION)).as("a genuinely deleted source is still cleaned up") + .isEmpty(); + } + + /** + * Drives synchronization passes while a publish is in flight - the state a real publish request is + * in between deleting a collection and copying it back. Returns once the probe artefact proves a + * pass has actually completed under those conditions. + */ + private void runPassesWhilePublishIsInFlight() { + registryMutationTracker.enter(); + try { + Awaitility.await() + .atMost(PASS_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .pollInterval(500, TimeUnit.MILLISECONDS) + .until(() -> { + synchronizationWatcher.force(); + synchronizationProcessor.processSynchronizers(); + return !javaFileService.findByLocation(PROBE_LOCATION) + .isEmpty(); + }); + } finally { + registryMutationTracker.exit(); + } + } + + @AfterEach + void removeSourcesFromRegistry() { + boolean removed = false; + for (String path : List.of(REGISTRY_PATH, PROBE_REGISTRY_PATH)) { + if (repository.hasResource(path)) { + repository.removeResource(path); + removed = true; + } + } + if (removed) { + synchronizationProcessor.forceProcessSynchronizers(); + } + } +}