Conversation
WalkthroughRefactors servlet/bootstrap and Jetty wiring: removes SteveAppContext, adds WebConfig (WebApplicationInitializer), simplifies JettyServer constructor, moves SteveConfiguration.profile from enum to String, updates API docs to bean-driven SpringDoc properties, adjusts WebSocket handshake, and updates root POM dependencyManagement and websocket module dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant WebCfg as WebConfig
participant Jetty as JettyServer
participant Servlet as ServletContainer
participant Spring as Spring Contexts
App->>WebCfg: WebConfig.initialize(config, logFileRetriever)
App->>Jetty: new JettyServer(config)
Jetty->>Jetty: createWebApp(config, resources, compression)
Jetty->>Servlet: start(server with WebAppContext)
Servlet->>WebCfg: onStartup(servletContext)
WebCfg->>Spring: create root context (register beans)
WebCfg->>Spring: create web context (scan, register MVC/CXF)
WebCfg->>Servlet: register DispatcherServlet, CXFServlet, Security filter
sequenceDiagram
autonumber
participant Client
participant Sec as SecurityConfiguration
participant Ctrl as RootRedirectController
participant Disp as DispatcherServlet
Client->>Disp: GET /
Disp->>Sec: authorize("/")
Sec-->>Disp: permit
Disp->>Ctrl: handle root
Ctrl-->>Disp: "redirect:/manager"
Disp-->>Client: 302 Redirect
sequenceDiagram
autonumber
participant Client
participant HH as HandshakeHandler
participant JRS as JettyRequestUpgradeStrategy
participant WS as WebSocketEndpoint
Client->>HH: HTTP upgrade request
HH->>JRS: configure(maxTextSize, idleTimeout)
JRS-->>HH: upgrade ready
HH-->>Client: 101 Switching Protocols
Client-->>WS: websocket frames
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
juherr
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java (1)
46-51: Remove unused static import org.mockito.ArgumentMatchers.eqSpotless flags the unused import in steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java (lines ~46-51); remove it to fix the pipeline.
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify;
🧹 Nitpick comments (6)
steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java (1)
330-330: Verification currently relies on object equality; prefer capturing/field-matching to avoid false negatives.Spring deserializes JSON into a new OcppTagForm instance, so strict equality may be brittle unless equals is field-based. Capture and compare fields instead.
Apply this diff:
- verify(ocppTagsService).updateOcppTag(form); + ArgumentCaptor<OcppTagForm> captor = ArgumentCaptor.forClass(OcppTagForm.class); + verify(ocppTagsService).updateOcppTag(captor.capture()); + assertThat(captor.getValue()) + .usingRecursiveComparison() + .isEqualTo(form);steve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.java (1)
33-35: Harden profile handling and avoid overwriting externally set profile.Trim input and only set
spring.profiles.activeif not already defined. Prevents surprises and tolerates whitespace.Apply:
- var profile = p.getString("profile"); - System.setProperty("spring.profiles.active", profile); + var profile = p.getString("profile").trim(); + if (System.getProperty("spring.profiles.active") == null) { + System.setProperty("spring.profiles.active", profile); + }Also applies to: 53-53
steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java (1)
35-35: Clean up Javadoc noise and keep line length compliant.The Javadoc includes pasted Checkstyle output and a broken URL. Replace with a short reference; keep under 120 chars.
Apply:
- /** - * https://docs.spring.io/spring-framework/reference/web/websocket/server.html#websocket-server-runtime-configurationCheck failure[checkstyle] src/main/java/de/rwth/idsg/steve/config/WebSocketConfiguration.java#L73 <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck>Check failure: [checkstyle] src/main/java/de/rwth/idsg/steve/config/WebSocketConfiguration.java#L73 <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck>Line is longer than 120 characters (found 121).build and run tests / checkstyleView detailsCode has alerts. Press enter to view. - * - * Otherwise, defaults come from {@link WebSocketConstants} - */ + /** + * See Spring docs: + * https://docs.spring.io/spring-framework/reference/web/websocket/server.html#websocket-server-runtime-configuration + * Otherwise, defaults come from {@link WebSocketConstants}. + */Also: keeping
setAllowedOrigins("*")is unchanged; confirm this is intentional for device clients.Also applies to: 66-66, 75-89
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (1)
33-36: Option: also handle HEAD on root.Some load balancers/monitors send HEAD to "/". You can map HEAD to the same redirect.
-import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMethod; @@ - @GetMapping + @RequestMapping(method = {RequestMethod.GET, RequestMethod.HEAD}) public String redirectToManager() { return "redirect:/manager"; }steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java (1)
34-34: Optional: simplify by dropping the org.springdoc component scan entirely.Given you already @import SpringDocConfiguration, SpringDocWebMvcConfiguration, SwaggerConfig, etc., scanning "org.springdoc" may be redundant and the exclusion filter becomes unnecessary.
-import org.springdoc.webmvc.ui.SwaggerUiHome; -import org.springframework.context.annotation.FilterType; @@ -@ComponentScan( - basePackages = {"org.springdoc"}, - // exclude because SwaggerUiHome's root redirect clashes with our own RootRedirectController - excludeFilters = @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, classes = SwaggerUiHome.class)) +// Rely on explicit @Import; avoid scanning org.springdoc package to prevent unintended controllersAlso applies to: 39-39
steve/src/main/java/de/rwth/idsg/steve/Application.java (1)
21-21: Consider thread-safety for static field initialization.The static fields
WebConfig.configandWebConfig.logFileRetrieverare being set directly without synchronization. While these are typically set once during application startup, there's a theoretical race condition if multipleApplicationinstances are created concurrently.Consider using a more robust initialization pattern:
public Application(SteveConfiguration config, LogFileRetriever logFileRetriever) { this.server = new JettyServer(config); - WebConfig.config = config; - WebConfig.logFileRetriever = logFileRetriever; + WebConfig.initialize(config, logFileRetriever); }Then in WebConfig:
private static volatile boolean initialized = false; public static synchronized void initialize(SteveConfiguration config, LogFileRetriever logFileRetriever) { if (!initialized) { WebConfig.config = config; WebConfig.logFileRetriever = logFileRetriever; initialized = true; } }Also applies to: 42-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
pom.xml(1 hunks)steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java(2 hunks)steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java(1 hunks)steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java(1 hunks)steve-ocpp-transport-websocket/pom.xml(1 hunks)steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java(2 hunks)steve-ocpp/pom.xml(0 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/config/JspConfiguration.java(0 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java(1 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java(1 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.java(2 hunks)steve/src/main/java/de/rwth/idsg/steve/Application.java(2 hunks)steve/src/main/java/de/rwth/idsg/steve/JettyServer.java(5 hunks)steve/src/main/java/de/rwth/idsg/steve/SteveAppContext.java(0 hunks)steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/ApplicationTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/OperationalTestSoapOCPP16.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/StressTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.java(1 hunks)
💤 Files with no reviewable changes (3)
- steve-ocpp/pom.xml
- steve-ui-jsp/src/main/java/de/rwth/idsg/steve/config/JspConfiguration.java
- steve/src/main/java/de/rwth/idsg/steve/SteveAppContext.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Java source code must be formatted using the Spotless Maven plugin (run spotless:apply/spotless:check before committing)
Files:
steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.javasteve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.javasteve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.javasteve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.javasteve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.javasteve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.javasteve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.javasteve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.javasteve/src/main/java/de/rwth/idsg/steve/JettyServer.javasteve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.javasteve/src/test/java/de/rwth/idsg/steve/ApplicationTest.javasteve/src/test/java/de/rwth/idsg/steve/OperationalTestSoapOCPP16.javasteve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.javasteve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.javasteve/src/main/java/de/rwth/idsg/steve/config/WebConfig.javasteve/src/main/java/de/rwth/idsg/steve/Application.javasteve/src/test/java/de/rwth/idsg/steve/StressTest.java
🧠 Learnings (1)
📚 Learning: 2025-09-02T20:01:57.746Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T20:01:57.746Z
Learning: Applies to steve/src/main/resources/config/main.properties : Default application configuration resides in steve/src/main/resources/config/main.properties; edit this file or override via command-line properties
Applied to files:
steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.javasteve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.javasteve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.javasteve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.javasteve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.javasteve/src/main/java/de/rwth/idsg/steve/Application.java
🧬 Code graph analysis (2)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (1)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java (1)
Controller(35-51)
steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java (2)
steve/src/main/java/de/rwth/idsg/steve/Application.java (1)
Slf4j(36-96)steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java (1)
Slf4j(43-148)
🪛 GitHub Actions: analyze and review code
steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java
[error] 47-47: Spotless formatting check failed. Remove or add import to satisfy formatting. Run 'mvn spotless:apply' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)
🔇 Additional comments (20)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java (1)
37-37: Add/signinendpoint: OK.Matches
formLogin(loginPage(prefix + "/signin")).permitAll(); redirect insignOutaligns. No further changes.Also applies to: 39-42
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.java (1)
24-24: Approve: EndpointInfo as @component — confirm component scanningMaking EndpointInfo a Spring bean looks good; constructor injection is clean. If multiple app contexts are possible, consider explicit scope (default singleton is fine). I couldn't verify that component scanning includes de.rwth.idsg.steve.web.dto because the ripgrep check returned "No files were searched" — confirm WebConfig or your @SpringBootApplication scans that package.
steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java (1)
58-58: Validate profile migration and add a fail‑fast guardprofile was changed to String (steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java:58). No ApplicationProfile enum references were found in this repository — still verify external consumers compiled against the old enum.
Add a non‑blank (and optionally allow‑list) check in postConstruct — minimal diff:
public void postConstruct() { + if (profile == null || profile.isBlank()) { + throw new IllegalArgumentException("profile must not be blank"); + } if (!(jetty.httpEnabled || jetty.httpsEnabled)) { throw new IllegalArgumentException( "HTTP and HTTPS are both disabled. Well, how do you want to access the server, then?"); } }If external modules depend on the enum, either provide a deprecated enum adapter (map enum -> string) or document the breaking change and validate against an allow‑list.
steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java (1)
71-71: Permit root path: OK with RootRedirectController.Change aligns with redirect to "/manager" — confirm no other mappings bind "/" (controllers, servlet/web.xml, servlet filters, or static handlers). Automated repo search here returned "No files were searched"; run these locally and paste results.
#!/bin/bash set -euo pipefail echo "== Find RootRedirectController ==" rg -n --hidden --no-ignore -S 'RootRedirectController' || true echo "== Find mapping annotations ==" rg -n --hidden --no-ignore -S '@(GetMapping|RequestMapping|Controller)' -C3 || true echo "== Find explicit \"/\" usages (mapping values/paths) ==" rg -n --hidden --no-ignore -S '"/"' -C3 || true rg -n --hidden --no-ignore -S '\b(value|path)\s*=\s*"/"' -C3 || true echo "== Check web.xml / servlet mappings ==" rg -n --hidden --no-ignore -S 'web\.xml|<url-pattern>|<servlet-mapping>' -C3 || true # fallbacks if rg is unavailable git grep -n 'RootRedirectController' || true git grep -n '"/"' || true git grep -n '<url-pattern>' || truepom.xml (1)
142-151: Approve: switch to hibernate-validator-bom — verify no module POM hardcodes jakarta.validation-api version.Sandbox search failed; run this locally to find stray hardcoded entries and remove them so the BOM manages jakarta.validation-api:
find . -type f -name 'pom.xml' -not -path '*/target/*' -print0 | xargs -0 -n1 perl -0777 -ne 'my $f=$ARGV[0]; while (/<dependency>(.*?)<\/dependency>/sg) { my $dep=$1; if ($dep =~ /<artifactId>\s*jakarta.validation-api\s*<\/artifactId>/i) { if ($dep =~ /<version>\s*([^<]+)\s*<\/version>/i) { print "$f: hardcoded version -> $1\n"; } else { print "$f: jakarta.validation-api dependency without <version>\n"; } } }' {}steve/src/test/java/de/rwth/idsg/steve/ApplicationTest.java (1)
55-55: Profile assertion updated correctly.Consistent with the String-based profile.
steve/src/test/java/de/rwth/idsg/steve/StressTest.java (1)
57-57: String-based profile assertion looks good.No further changes needed.
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (1)
29-31: Approved — "/" is permitted anonymously in SecurityConfiguration.
SecurityConfiguration registers requestMatchers("/", "/static/", config.getPaths().getSoapMapping() + "/", ...).permitAll(), so the RootRedirectController's redirect to /manager will not be intercepted.steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java (1)
51-55: Approve — excluding SwaggerUiHome is correctVerified: steve-api/pom.xml declares org.springdoc:springdoc-openapi-starter-webmvc-ui (v2.8.13) and ApiDocsConfiguration imports org.springdoc.webmvc.ui.SwaggerUiHome; excluding it prevents springdoc’s "/" handler from shadowing our RootRedirectController.
steve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.java (1)
67-67: Assertion update to string profile — LGTM.Matches the enum→String change. Automated search returned no ApplicationProfile matches; manually verify there are no remaining enum usages in the repo.
steve/src/test/java/de/rwth/idsg/steve/OperationalTestSoapOCPP16.java (1)
81-81: LGTM!The assertion correctly validates the profile configuration as a string value, which is consistent with the broader refactoring that switches from enum-based to String-based profile handling.
steve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.java (1)
55-55: LGTM!The test assertion has been properly updated to match the String-based profile value, aligning with the new configuration approach.
steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java (3)
69-71: Root mapping configuration looks good.The DispatcherServlet is correctly configured with the root mapping from the configuration.
79-86: Security filter configuration is properly set up.The DelegatingFilterProxy is correctly configured with appropriate dispatcher types and URL patterns for Spring Security integration.
54-60: No null-checks needed — statics are initialized before Jetty calls onStartup.Application(SteveConfiguration, LogFileRetriever) assigns WebConfig.config and WebConfig.logFileRetriever (Application.java:43–44) during construction; server.start() is invoked later (Application.start()), and Jetty/Spring call WebApplicationInitializer.onStartup during server startup — the suppliers will see non-null values.
steve/src/main/java/de/rwth/idsg/steve/JettyServer.java (5)
59-65: Good practice with ResourceFactory lifecycle management.The use of
ResourceFactory.closeable()with proper cleanup in thestop()method ensures resources are properly released.
67-69: Constructor simplification aligns with WebConfig pattern.The removal of
LogFileRetrieverfrom the constructor is consistent with the new WebConfig-based initialization approach.
110-110: Clean refactoring of connector creation methods.The extraction of
createHttpConnectorandcreateHttpsConnectoras static methods improves code organization and reusability while maintaining the same functionality.Also applies to: 114-114, 120-128, 130-152
154-179: WebAppContext configuration is comprehensive and well-structured.The method properly sets up the web application context with all necessary configurations including:
- Context path and base resource
- Exception propagation for startup failures
- Directory listing security
- Container scanning for Spring's WebApplicationInitializer
- Optional gzip compression
The detailed comments explaining the container scan pattern are particularly helpful.
203-205: Resource cleanup properly implemented.The
stop()method correctly closes thewebAppResourceFactoryafter stopping the server.
| <dependency> | ||
| <groupId>org.eclipse.jetty.websocket</groupId> | ||
| <artifactId>jetty-websocket-jetty-api</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
Jetty WS dependency: verify correct artifact and scope.
jetty-websocket-jetty-api may be insufficient at runtime; JettyRequestUpgradeStrategy typically needs Jetty WebSocket SERVER classes. Consider adding jetty-websocket-jetty-server (or mark API as provided if the server brings it).
Proposed addition if needed:
<dependency>
<groupId>org.eclipse.jetty.websocket</groupId>
<artifactId>jetty-websocket-jetty-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty.websocket</groupId>
+ <artifactId>jetty-websocket-jetty-server</artifactId>
+ </dependency>Web check to confirm exact artifacts for Spring 6.2 + Jetty 12:
🌐 Web query:
For Spring Framework 6.2.x and Jetty 12, which Jetty WebSocket artifacts are required at runtime when using org.springframework.web.socket.server.jetty.JettyRequestUpgradeStrategy?
💡 Result:
Short answer — include Jetty's WebSocket API + server implementation for Jetty 12 (and the Jakarta WebSocket implementation if you rely on the standard jakarta.websocket API). In practice add these runtime artifacts:
- org.eclipse.jetty.websocket:jetty-websocket-jetty-api. [1]
- org.eclipse.jetty.websocket:jetty-websocket-jetty-server (the server implementation). [1]
- If you use the standard Jakarta WebSocket API endpoints (jakarta.websocket), also include jakarta.websocket:jakarta.websocket-api plus Jetty's Jakarta implementation: org.eclipse.jetty.ee10.websocket:jetty-ee10-websocket-jakarta-server. [2]
Sources: Spring 6.2 API (JettyRequestUpgradeStrategy for Jetty 12 EE10). [3] Jetty 12 WebSocket docs. [1][2]
References
[1] Jetty WebSocket docs (Jetty 12).
[2] Jetty EE10 Jakarta WebSocket implementation docs.
[3] Spring Framework 6.2 Javadoc — JettyRequestUpgradeStrategy.
Add Jetty server WebSocket implementation (runtime)
File: steve-ocpp-transport-websocket/pom.xml
jetty-websocket-jetty-api is only the API; add the server implementation at runtime or mark the API as provided if your runtime supplies it. If using Jakarta WebSocket endpoints, also include jakarta.websocket:jakarta.websocket-api and org.eclipse.jetty.ee10.websocket:jetty-ee10-websocket-jakarta-server.
<dependency>
<groupId>org.eclipse.jetty.websocket</groupId>
<artifactId>jetty-websocket-jetty-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty.websocket</groupId>
+ <artifactId>jetty-websocket-jetty-server</artifactId>
+ </dependency>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <dependency> | |
| <groupId>org.eclipse.jetty.websocket</groupId> | |
| <artifactId>jetty-websocket-jetty-api</artifactId> | |
| </dependency> | |
| <dependency> | |
| <groupId>org.eclipse.jetty.websocket</groupId> | |
| <artifactId>jetty-websocket-jetty-api</artifactId> | |
| </dependency> | |
| <dependency> | |
| <groupId>org.eclipse.jetty.websocket</groupId> | |
| <artifactId>jetty-websocket-jetty-server</artifactId> | |
| </dependency> |
🤖 Prompt for AI Agents
In steve-ocpp-transport-websocket/pom.xml around lines 36 to 39, the POM
currently declares only the jetty-websocket-jetty-api (the API) which lacks a
runtime server implementation; either add the Jetty server implementation
dependency (e.g.,
org.eclipse.jetty.ee10.websocket:jetty-ee10-websocket-jakarta-server) with
appropriate scope (runtime) or change the API dependency to
<scope>provided</scope> if your runtime supplies it, and if you use Jakarta
WebSocket endpoints also add jakarta.websocket:jakarta.websocket-api (and keep
the Jetty EE10 Jakarta server dependency) so the runtime has both API and server
implementation available.
5d4a128 to
cbb3665
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java (1)
58-58: String profile loses enum guarantees — add a minimal validation.
Now that profile is a String, guard against null/blank to prevent misconfig at startup.Apply this minimal check early in postConstruct():
public void postConstruct() { + if (profile == null || profile.isBlank()) { + throw new IllegalArgumentException("Profile must be set and non-blank"); + } if (!(jetty.httpEnabled || jetty.httpsEnabled)) { throw new IllegalArgumentException( "HTTP and HTTPS are both disabled. Well, how do you want to access the server, then?"); } }steve/src/main/java/de/rwth/idsg/steve/Application.java (1)
41-44: Static bootstrap: provide a reset hook to avoid test pollution.
Since WebConfig holds static state, consider exposing WebConfig.reset() and invoking it on shutdown to keep tests/JVM reuse clean.In Application.stop():
public void stop() throws Exception { server.stop(); + // Optional: clear static bootstrap state to avoid cross-test leakage + // WebConfig.reset(); }And in WebConfig (new method):
public static void reset() { // null out static fields or close resources as needed }steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java (1)
64-77: Swagger UI path: consider base path instead of explicit index.html.
Using the base path is a bit more future-proof and typically redirects to index.html. Also consider setting configUrl explicitly to match the moved api-docs path.Apply if consistent with your springdoc version:
- p.setPath("/manager/swagger-ui/index.html"); + p.setPath("/manager/swagger-ui"); + p.setConfigUrl("/manager/v3/api-docs/swagger-config");Smoke-check both URLs after change:
- GET /manager/swagger-ui
- GET /manager/v3/api-docs/swagger-config
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
pom.xml(1 hunks)steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java(2 hunks)steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java(1 hunks)steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java(1 hunks)steve-ocpp-transport-websocket/pom.xml(1 hunks)steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java(2 hunks)steve-ocpp/pom.xml(0 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/config/JspConfiguration.java(0 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java(1 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java(1 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.java(2 hunks)steve/src/main/java/de/rwth/idsg/steve/Application.java(2 hunks)steve/src/main/java/de/rwth/idsg/steve/JettyServer.java(5 hunks)steve/src/main/java/de/rwth/idsg/steve/SteveAppContext.java(0 hunks)steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/ApplicationTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/OperationalTestSoapOCPP16.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/StressTest.java(1 hunks)steve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.java(1 hunks)
💤 Files with no reviewable changes (3)
- steve/src/main/java/de/rwth/idsg/steve/SteveAppContext.java
- steve-ocpp/pom.xml
- steve-ui-jsp/src/main/java/de/rwth/idsg/steve/config/JspConfiguration.java
🚧 Files skipped from review as they are similar to previous changes (15)
- steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java
- steve/src/test/java/de/rwth/idsg/steve/issues/Issue73Fix.java
- steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java
- steve-ocpp-transport-websocket/pom.xml
- steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java
- steve/src/main/java/de/rwth/idsg/steve/utils/SteveConfigurationReader.java
- steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java
- steve/src/test/java/de/rwth/idsg/steve/ApplicationTest.java
- steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java
- steve/src/main/java/de/rwth/idsg/steve/JettyServer.java
- steve/src/test/java/de/rwth/idsg/steve/OperationalTestSoapOCPP16.java
- pom.xml
- steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/dto/EndpointInfo.java
- steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java
- steve/src/test/java/de/rwth/idsg/steve/StressTest.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Java source code must be formatted using the Spotless Maven plugin (run spotless:apply/spotless:check before committing)
Files:
steve/src/main/java/de/rwth/idsg/steve/Application.javasteve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.javasteve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.javasteve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java
🧠 Learnings (1)
📚 Learning: 2025-09-02T20:01:57.746Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T20:01:57.746Z
Learning: Applies to steve/src/main/resources/config/main.properties : Default application configuration resides in steve/src/main/resources/config/main.properties; edit this file or override via command-line properties
Applied to files:
steve/src/main/java/de/rwth/idsg/steve/Application.javasteve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)
🔇 Additional comments (6)
steve/src/test/java/de/rwth/idsg/steve/ApplicationJsonTest.java (1)
67-67: Profile assertion aligned with string-based config — LGTM.
The switch to a plain string ("test") matches the new SteveConfiguration.getProfile() contract.steve/src/main/java/de/rwth/idsg/steve/Application.java (2)
21-21: Import of WebConfig for bootstrap — LGTM.
Import is consistent with the new startup flow.
41-44: No init-order hazard — current order is fine.
Verified: WebConfig.initialize is only called from Application; JettyServer’s constructor contains no WebConfig references, so reordering is unnecessary.steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java (3)
30-30: LGTM: targeted SpringDoc import aligns with bean-driven config.
No concerns.
79-83: LGTM: OAuth properties bean provided for wiring; defaults are safe.
No action.
51-62: Approve — no lingering System.setProperty("springdoc.*") calls foundSearched the repo with ripgrep (--no-ignore); no System.setProperty("springdoc.") occurrences and no springdoc. property keys in config files. Only expected org.springdoc imports/annotations remain (e.g. steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java and web/api controllers).
| SwaggerUiOAuthProperties.class, | ||
| JacksonAutoConfiguration.class | ||
| }) | ||
| @Import({SpringDocConfiguration.class, SpringDocWebMvcConfiguration.class, SwaggerConfig.class}) |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm autoconfig overlap.
If any Spring Boot springdoc starter autoconfiguration is active elsewhere, this explicit @import can double-register beans. If Boot autoconfig is used, prefer removing the manual @import; otherwise keep as-is.
Run to check for springdoc starters/autoconfig:
🏁 Script executed:
#!/bin/bash
rg -n -C2 -g 'pom.xml' 'springdoc-openapi-starter|org.springdoc'Length of output: 316
Remove manual Springdoc @import — starter autoconfig present.
pom.xml declares org.springdoc:springdoc-openapi-starter-webmvc-ui (v2.8.13); the starter auto-configures SpringDoc beans, so the explicit @import duplicates bean registration — remove the @import.
Location: steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java
@Import({SpringDocConfiguration.class, SpringDocWebMvcConfiguration.class, SwaggerConfig.class})
🤖 Prompt for AI Agents
In steve-api/src/main/java/de/rwth/idsg/steve/config/ApiDocsConfiguration.java
around line 48, remove the manual @Import({SpringDocConfiguration.class,
SpringDocWebMvcConfiguration.class, SwaggerConfig.class}) annotation because the
org.springdoc springdoc-openapi-starter-webmvc-ui starter auto-configures these
beans; delete that @Import line (or the entire annotation) so the starter’s
auto-configuration is used and duplicate bean registration is avoided.
cbb3665 to
d28211b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (3)
21-23: Prefer @GetMapping and drop explicit HEAD importHEAD is handled implicitly for GET in Spring MVC; using @GetMapping is more idiomatic and lets you remove RequestMethod. Also keeps imports minimal.
-import org.springframework.stereotype.Controller; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping;
33-35: Simplify mapping to @GetMapping (HEAD handled automatically)Keeps intent clear and avoids redundant method array. Optionally add a trailing slash in the redirect if
/manageritself redirects to/manager/in your setup to save a hop.- @RequestMapping(method = {RequestMethod.GET, RequestMethod.HEAD}) - public String redirectToManager() { - return "redirect:/manager"; - } + @GetMapping + public String redirectToManager() { + return "redirect:/manager"; + }
27-27: Standardize @SInCE date formatIf the project prefers ISO-8601, switch to 2025-09-17 for consistency; otherwise ignore.
- * @since 17.09.2025 + * @since 2025-09-17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java(1 hunks)steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/Application.java(4 hunks)steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- steve/src/main/java/de/rwth/idsg/steve/Application.java
- steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java
- steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Java source code must be formatted using the Spotless Maven plugin (run spotless:apply/spotless:check before committing)
Files:
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java
🧬 Code graph analysis (1)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (1)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/SignInOutController.java (1)
Controller(35-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
🔇 Additional comments (2)
steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java (2)
29-31: LGTM overallController name, package, and class-level mapping look consistent with the new web controller layout.
29-31: No conflicting "/" mapping; access is permitted.
RootRedirectController maps "/" → redirect:/manager (steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/RootRedirectController.java) and SecurityConfiguration explicitly .requestMatchers("/", ...).permitAll() (steve/src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java). No other @*Mapping("/") occurrences found.
juherr
force-pushed
the
backport/steve-1820
branch
from
d28211b to
3dce8ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
steve/src/main/java/de/rwth/idsg/steve/Application.java (1)
39-47: Constructor wiring simplified; add null‑guards for robustnessFail fast on bad inputs and keep
servercreation using the validatedconfig.- public Application(SteveConfiguration config, LogFileRetriever logFileRetriever) { - this.config = config; - this.logFileRetriever = logFileRetriever; - this.server = new JettyServer(config); - } + public Application(SteveConfiguration config, LogFileRetriever logFileRetriever) { + this.config = java.util.Objects.requireNonNull(config, "config"); + this.logFileRetriever = java.util.Objects.requireNonNull(logFileRetriever, "logFileRetriever"); + this.server = new JettyServer(this.config); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java(1 hunks)steve/src/main/java/de/rwth/idsg/steve/Application.java(4 hunks)steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- steve-api/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java
- steve/src/main/java/de/rwth/idsg/steve/config/WebConfig.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Java source code must be formatted using the Spotless Maven plugin (run spotless:apply/spotless:check before committing)
Files:
steve/src/main/java/de/rwth/idsg/steve/Application.java
🧠 Learnings (1)
📚 Learning: 2025-09-02T20:01:57.746Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T20:01:57.746Z
Learning: Applies to steve/src/main/resources/config/main.properties : Default application configuration resides in steve/src/main/resources/config/main.properties; edit this file or override via command-line properties
Applied to files:
steve/src/main/java/de/rwth/idsg/steve/Application.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
- GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
- GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
- GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
🔇 Additional comments (1)
steve/src/main/java/de/rwth/idsg/steve/Application.java (1)
21-21: WebConfig import — OKImport aligns with the new bootstrap wiring.
| public void start() throws Exception { | ||
| WebConfig.initialize(config, logFileRetriever); | ||
| server.start(); | ||
| } |
There was a problem hiding this comment.
Ensure cleanup if start() fails
If server.start() (or initialize) throws, WebConfig.clear() should still run to avoid leaking static state when embedders don’t call stop().
- public void start() throws Exception {
- WebConfig.initialize(config, logFileRetriever);
- server.start();
- }
+ public void start() throws Exception {
+ try {
+ WebConfig.initialize(config, logFileRetriever);
+ server.start();
+ } catch (Exception e) {
+ try {
+ WebConfig.clear();
+ } catch (Exception suppressed) {
+ e.addSuppressed(suppressed);
+ }
+ throw e;
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void start() throws Exception { | |
| WebConfig.initialize(config, logFileRetriever); | |
| server.start(); | |
| } | |
| public void start() throws Exception { | |
| try { | |
| WebConfig.initialize(config, logFileRetriever); | |
| server.start(); | |
| } catch (Exception e) { | |
| try { | |
| WebConfig.clear(); | |
| } catch (Exception suppressed) { | |
| e.addSuppressed(suppressed); | |
| } | |
| throw e; | |
| } | |
| } |
🤖 Prompt for AI Agents
In steve/src/main/java/de/rwth/idsg/steve/Application.java around lines 87 to
90, calling WebConfig.initialize(...) and server.start() can throw and currently
WebConfig.clear() won't run if start() fails; wrap the initialize/start sequence
in a try block and ensure WebConfig.clear() is invoked in a finally (or call
clear in the catch before rethrowing) so static state is cleaned on failure,
then rethrow the exception to preserve original behavior.
| public void stop() throws Exception { | ||
| server.stop(); | ||
| WebConfig.clear(); | ||
| } |
There was a problem hiding this comment.
Always clear WebConfig even if server.stop() throws
Run clear() in a finally block so static state is released even when Jetty shutdown fails.
- public void stop() throws Exception {
- server.stop();
- WebConfig.clear();
- }
+ public void stop() throws Exception {
+ try {
+ server.stop();
+ } finally {
+ WebConfig.clear();
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void stop() throws Exception { | |
| server.stop(); | |
| WebConfig.clear(); | |
| } | |
| public void stop() throws Exception { | |
| try { | |
| server.stop(); | |
| } finally { | |
| WebConfig.clear(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In steve/src/main/java/de/rwth/idsg/steve/Application.java around lines 96 to
99, ensure WebConfig.clear() is always executed even if server.stop() throws by
moving the clear call into a finally block: call server.stop() inside a try
(handling or letting the exception propagate) and invoke WebConfig.clear()
inside the corresponding finally so static state is released regardless of
shutdown failures.
1 participant
steve-community#1820
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Tests