Conversation
There was a problem hiding this comment.
Pull request overview
Adds an (optional) Hazelcast-based discovery path to list remote Frank instances/configurations and expose them to the frontend, primarily for local development workflows.
Changes:
- Introduces a
hazelcast.enabledfeature flag (default off; enabled inapplication-local.properties) and publishes config metadata. - Adds backend Hazelcast integration (service/controller + local “configurations directory” responder + Hazelcast-related Spring config).
- Adds frontend polling + UI section to discover and connect to remote instances.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application.properties | Adds default hazelcast.enabled=false. |
| src/main/resources/application-local.properties | Enables Hazelcast for local profile and adds configurations.directory hint. |
| src/main/resources/META-INF/additional-spring-configuration-metadata.json | Documents hazelcast.enabled for Spring metadata tooling. |
| src/main/java/org/frankframework/flow/security/UserWorkspaceContext.java | Adjusts request scope proxying for UserWorkspaceContext. |
| src/main/java/org/frankframework/flow/hazelcast/HazelcastService.java | Discovers cluster members and fetches configurations via management bus. |
| src/main/java/org/frankframework/flow/hazelcast/HazelcastController.java | Exposes GET /api/hazelcast/instances. |
| src/main/java/org/frankframework/flow/hazelcast/HazelcastConfigurationDTO.java | DTO for configuration discovery payloads. |
| src/main/java/org/frankframework/flow/hazelcast/FrankInstanceDTO.java | DTO for remote instance listing returned to the UI. |
| src/main/java/org/frankframework/flow/hazelcast/ConfigurationsDirectory.java | Responds to bus “configuration find” requests using a local directory listing. |
| src/main/java/org/frankframework/flow/common/config/WebConfiguration.java | Tweaks shared ObjectMapper and adds an InputStreamHttpMessageConverter bean. |
| src/main/java/org/frankframework/flow/common/config/SecurityChainConfigurer.java | Adds a Spring Security filter chain (anonymous-only authorization as written). |
| src/main/java/org/frankframework/flow/common/config/HazelcastConfig.java | Wires gateway factory + management bus channel under hazelcast.enabled. |
| src/main/java/org/frankframework/flow/common/config/ConfigurationsConfig.java | Session-scoped configuration list fetched via outbound bus call. |
| src/main/java/org/frankframework/flow/FlowWarInitializer.java | Adds a WAR initializer entrypoint class. |
| src/main/java/org/frankframework/flow/FlowApplication.java | Updates app bootstrap and adds SpringBootServletInitializer + config properties scan. |
| src/main/frontend/app/services/hazelcast-service.ts | Adds frontend API wrapper for instance discovery. |
| src/main/frontend/app/routes/projectlanding/project-landing.tsx | Polls discovery endpoint and renders “Remote” instances section. |
| pom.xml | Adds management-gateway dependency and additional Spring framework dependency/version property. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/frankframework/flow/hazelcast/ConfigurationsDirectory.java
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/common/config/SecurityChainConfigurer.java
Outdated
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/hazelcast/HazelcastController.java
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/common/config/ConfigurationsConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/hazelcast/ConfigurationsDirectory.java
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/common/config/HazelcastConfig.java
Show resolved
Hide resolved
src/main/java/org/frankframework/flow/common/config/WebConfiguration.java
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <dependency> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-beans</artifactId> | ||
| <version>${spring-framework.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
This declares an explicit spring-beans dependency with a hard-coded Spring Framework version even though the project already uses the Spring Boot parent BOM. This is easy to forget to keep in sync with the rest of Spring artifacts and can cause version skew at runtime. Prefer relying on Spring Boot’s dependency management (remove the explicit spring-beans dependency/version), or override Spring Framework via the Boot-managed property only (without adding a redundant direct dependency).
| <dependency> | |
| <groupId>org.springframework</groupId> | |
| <artifactId>spring-beans</artifactId> | |
| <version>${spring-framework.version}</version> | |
| </dependency> |
| import org.springframework.boot.builder.SpringApplicationBuilder; | ||
| import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; | ||
|
|
||
| /** | ||
| * Spring Boot entrypoint when running as a WAR application deployed to an external servlet container. | ||
| * For standalone JAR execution, see {@link FlowApplication}. | ||
| */ | ||
| public class FlowWarInitializer extends SpringBootServletInitializer { | ||
|
|
||
| @Override | ||
| protected SpringApplicationBuilder configure(SpringApplicationBuilder builder) { | ||
| return builder.sources(FlowApplication.class); | ||
| } | ||
| } |
There was a problem hiding this comment.
FlowApplication already extends SpringBootServletInitializer, and this class adds a second WebApplicationInitializer for WAR deployments. In a servlet container, having two SpringBootServletInitializer subclasses can trigger multiple application context initializations. Keep only one initializer (either remove this class, or stop extending SpringBootServletInitializer in FlowApplication).
| import org.springframework.boot.builder.SpringApplicationBuilder; | |
| import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; | |
| /** | |
| * Spring Boot entrypoint when running as a WAR application deployed to an external servlet container. | |
| * For standalone JAR execution, see {@link FlowApplication}. | |
| */ | |
| public class FlowWarInitializer extends SpringBootServletInitializer { | |
| @Override | |
| protected SpringApplicationBuilder configure(SpringApplicationBuilder builder) { | |
| return builder.sources(FlowApplication.class); | |
| } | |
| } | |
| /** | |
| * Spring Boot entrypoint when running as a WAR application deployed to an external servlet container. | |
| * For standalone JAR execution, see {@link FlowApplication}. | |
| * <p> | |
| * This class is retained only for backward compatibility. Actual initialization | |
| * is handled by {@link FlowApplication}, which extends SpringBootServletInitializer. | |
| */ | |
| public class FlowWarInitializer { | |
| // No-op: initialization is performed by FlowApplication. | |
| } |
| return index == 0 | ||
| ? path.getRoot().toString() | ||
| : path.getRoot().resolve(path.subpath(0, index)).toString(); |
There was a problem hiding this comment.
buildPathUpTo assumes path.getRoot() is non-null. For relative paths (or some non-default Path implementations), getRoot() can be null and this will throw a NullPointerException. Consider handling a null root by returning path.subpath(0, index).toString() (and for index==0, return an empty path or the original input) so relative paths are supported safely.
| return index == 0 | |
| ? path.getRoot().toString() | |
| : path.getRoot().resolve(path.subpath(0, index)).toString(); | |
| Path root = path.getRoot(); | |
| if (root == null) { | |
| // Relative path: safely build prefix without assuming a root | |
| return index == 0 | |
| ? "" | |
| : path.subpath(0, index).toString(); | |
| } | |
| // Absolute path: preserve existing behavior | |
| return index == 0 | |
| ? root.toString() | |
| : root.resolve(path.subpath(0, index)).toString(); |
| try { | ||
| List<HazelcastConfigurationDTO> configurations = fetchConfigurations(gateway, member); | ||
| result.add(toFrankInstance(member.getName(), member.getId().toString(), configurations)); | ||
| } catch (Exception e) { | ||
| log.warn("Member [{}] did not respond ({}), skipping", member.getName(), e.getMessage()); | ||
| } |
There was a problem hiding this comment.
collectWorkerInstances adds a FrankInstanceDTO even when the configuration list is empty (e.g., member returns no configs or parsing fails). This can surface “live” instances in the UI that can never be opened (projectPath ends up null). Consider skipping members with empty configurations (or at least only adding the instance when a non-blank directory/projectPath can be derived).
| useEffect(() => { | ||
| if (!isLocalEnvironment) return | ||
| const controller = new AbortController() | ||
|
|
||
| const discover = () => { | ||
| discoverFrankInstances(controller.signal) | ||
| .then(setFrankInstances) | ||
| .catch(() => { | ||
| // Discovery failure is non-critical; Hazelcast may not be running | ||
| }) | ||
| .finally(() => setIsDiscovering(false)) | ||
| } | ||
|
|
||
| setIsDiscovering(true) | ||
| discover() | ||
| const interval = setInterval(discover, 3000) | ||
|
|
||
| return () => { | ||
| controller.abort() | ||
| clearInterval(interval) | ||
| } | ||
| }, [isLocalEnvironment]) |
There was a problem hiding this comment.
The polling loop can start a new discoverFrankInstances request every 3s even if the previous request hasn’t completed yet, which can lead to overlapping in-flight requests and unnecessary load. Consider guarding against concurrent runs (e.g., track an in-flight flag) or aborting the previous request before starting a new one, and ensure state updates are skipped after cleanup to avoid setting state on an unmounted component.
|
I'll look at this later this week as this is quite important to be done correctly |


It works both for OutboundGateway and LocalGateway

