Features/modules #126
Features/modules #126
Conversation
WalkthroughReorganizes the Maven multi-module layout (adds steve-bom), relocates/renames several modules under nested directories, centralizes dependency versions in a new BOM, adds pluginManagement entries for license and spotless, updates child POM parent relativePath values and steve-ocpp → steve-ocpp-commons, and extends SteveProperties with OCPP WebSocket settings. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![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
🧹 Nitpick comments (1)
pom.xml (1)
52-144: Consider importing steve-bom for centralized version management.The parent POM declares individual dependencies in its dependencyManagement section, while the new
steve-bommodule also provides centralized dependency management.Consider whether child modules should import the steve-bom or continue using the parent's dependencyManagement. Having both could lead to version conflicts or confusion about which source of truth to use.
If you want child modules to use the BOM, you could import it in the parent's dependencyManagement:
<dependencyManagement> <dependencies> + <dependency> + <groupId>de.rwth.idsg</groupId> + <artifactId>steve-bom</artifactId> + <version>${project.version}</version> + <type>pom</type> + <scope>import</scope> + </dependency> <dependency> <groupId>de.rwth.idsg</groupId>Alternatively, if the parent POM's dependencyManagement is the intended source of truth, the steve-bom might be redundant (unless it's for external consumers).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pom.xml(2 hunks)steve-bom/pom.xml(1 hunks)steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java(2 hunks)steve-db/steve-jooq/pom.xml(1 hunks)steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java(0 hunks)steve-ocpp/steve-ocpp-1-x/pom.xml(1 hunks)steve-ocpp/steve-ocpp-2-x/pom.xml(1 hunks)
💤 Files with no reviewable changes (1)
- steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java
🧰 Additional context used
📓 Path-based instructions (2)
steve-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep core business logic in steve-core and keep it framework-agnostic (no direct dependencies on Spring, jOOQ, Jetty, JSP, or other adapter frameworks in core).
Files:
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java
**/src/{main,test}/java/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Run Spotless and keep Java source formatted according to spotless-maven-plugin before committing (./mvnw spotless:apply).
Files:
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java
🧠 Learnings (3)
📚 Learning: 2025-09-29T19:19:47.728Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T19:19:47.728Z
Learning: Applies to steve-jooq/** : Implement all database persistence in the steve-jooq module using jOOQ; do not place database access code in other modules.
Applied to files:
pom.xmlsteve-db/steve-jooq/pom.xml
📚 Learning: 2025-09-29T19:19:47.728Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T19:19:47.728Z
Learning: Applies to steve-core/** : Keep core business logic in steve-core and keep it framework-agnostic (no direct dependencies on Spring, jOOQ, Jetty, JSP, or other adapter frameworks in core).
Applied to files:
pom.xml
📚 Learning: 2025-08-29T07:17:42.970Z
Learnt from: juherr
PR: juherr/evolve#62
File: steve/README.md:119-121
Timestamp: 2025-08-29T07:17:42.970Z
Learning: SteVe project uses jOOQ which requires live database connection during Maven build process to run Flyway migrations and generate Java code from database schema in the generate-sources phase.
Applied to files:
steve-db/steve-jooq/pom.xml
⏰ 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). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (11)
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java (1)
26-26: LGTM! Import aligns with new property.The
Durationimport is necessary for the newidleTimeoutproperty in theWsclass.steve-db/steve-jooq/pom.xml (2)
8-8: LGTM! Correctly updates parent path.The
relativePathis correctly set to../../pom.xmlto account for the module's relocation tosteve-db/steve-jooq.
22-64: Lombok provided by parent POM; removing it from steve-jooq’s POM is safe.steve-ocpp/steve-ocpp-1-x/pom.xml (2)
8-8: LGTM! Correctly updates parent path.The
relativePathis correctly set to../../pom.xmlto account for the module's relocation tosteve-ocpp/steve-ocpp-1-x.
16-16: Verify complete migration fromsteve-ocpptosteve-ocpp-commons
- No
<artifactId>steve-ocpp</artifactId>references or oldimport steve.ocpplines remain;steve-ocpp-commonsmodule is present.- Manually confirm that all previous
steve-ocppfunctionality exists insteve-ocpp-commonsand that no breaking API changes were introduced.steve-ocpp/steve-ocpp-2-x/pom.xml (2)
8-8: LGTM! Correctly updates parent path.The
relativePathis correctly set to../../pom.xmlto account for the module's relocation tosteve-ocpp/steve-ocpp-2-x.
16-16: LGTM! Consistent dependency migration.The dependency change from
steve-ocpptosteve-ocpp-commonsis consistent with the same refactoring applied tosteve-ocpp-1-x.The verification script provided in the review for
steve-ocpp/steve-ocpp-1-x/pom.xmlwill also validate this change.steve-bom/pom.xml (1)
1-71: LGTM! Well-structured BOM module.The new BOM (Bill of Materials) module follows Maven best practices:
- Standalone structure (no parent) avoids circular dependencies
- Centralizes version management for all steve modules using
${project.version}- Includes all key modules: core, db, ocpp variants, transports, api, and ui
- Appropriately skips code formatting via
spotless-maven-pluginconfigurationThis will simplify dependency management across the project.
pom.xml (3)
24-36: LGTM! Well-organized module structure.The module reorganization into logical groups improves maintainability:
steve-db/for database modules (steve-jooq)steve-ocpp/for OCPP-related modules (commons, 1-x, 2-x, transports)steve-ui/for UI modules (steve-ui-jsp)The addition of
steve-bomfor centralized dependency management follows Maven best practices.
66-66: LGTM! Consistent dependency management update.The change from
steve-ocpptosteve-ocpp-commonsin dependencyManagement aligns with the module refactoring observed in steve-ocpp-1-x and steve-ocpp-2-x POMs.The verification script in the steve-ocpp-1-x review will confirm this migration is complete.
29-35: No action required—reactor build order is correct. All nested modules exist and steve-ocpp-1-x and steve-ocpp-2-x declare their dependency on steve-ocpp-commons, which Maven’s reactor will build before its dependents.
| private Ws ws = new Ws(); | ||
|
|
||
| @Data | ||
| public static class Ws { | ||
| private @Nullable Integer maxTextMessageSize; | ||
| private @Nullable Duration idleTimeout; | ||
| private String[] allowedOriginPatterns; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify WebSocket configuration migration completeness.
The new Ws nested class consolidates WebSocket-related properties under steve.ocpp.ws. According to the AI summary, OcppWebSocketConfiguration was removed.
Ensure all WebSocket configuration from the removed class has been migrated to these properties and verify that the consuming code has been updated accordingly.
🏁 Script executed:
#!/bin/bash
# Verify that OcppWebSocketConfiguration has been removed and check for any remaining references
# Check if OcppWebSocketConfiguration still exists
echo "=== Checking if OcppWebSocketConfiguration exists ==="
fd -t f -e java OcppWebSocketConfiguration
# Search for references to the old configuration class
echo "=== Searching for references to OcppWebSocketConfiguration ==="
rg -n "OcppWebSocketConfiguration" --type java
# Search for usages of the new Ws configuration
echo "=== Searching for usages of new Ws properties ==="
rg -n "\.ws\.(maxTextMessageSize|idleTimeout|allowedOriginPatterns)" --type java -A2 -B2Length of output: 1047
Migrate OcppWebSocketConfiguration to use SteveProperties.Ws
Update steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java to pull maxTextMessageSize, idleTimeout and allowedOriginPatterns from SteveProperties.Ws instead of using its own DEFAULT_MAX_MSG_SIZE; adjust ConcurrentWebSocketHandler and AbstractWebSocketEndpoint imports and buffer size logic accordingly.
🤖 Prompt for AI Agents
In
steve-ocpp-transport-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java
(replace usage of DEFAULT_MAX_MSG_SIZE), change the configuration to read
maxTextMessageSize, idleTimeout and allowedOriginPatterns from
SteveProperties.Ws: inject SteveProperties, fetch ws = steveProperties.getWs(),
and use ws.getAllowedOriginPatterns() for CORS setup, ws.getIdleTimeout() for
session/handshake timeouts (convert Duration to milliseconds as needed), and
ws.getMaxTextMessageSize() as the buffer/handler max message size (falling back
to the previous constant only if ws.getMaxTextMessageSize() is null). Also
update imports to use the project's ConcurrentWebSocketHandler and
AbstractWebSocketEndpoint classes, and ensure the buffer sizing logic uses the
configured maxTextMessageSize (or default) consistently when constructing
handlers/endpoints.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pom.xml (1)
170-240: Rely on pluginManagement for Spotless version.We now manage the Spotless version via
<pluginManagement>, but the execution block still hard-codes2.46.1. Drop the duplicate<version>element from the<build><plugins>entry so we inherit from pluginManagement and keep the version in one place.Also applies to: 324-373
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pom.xml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-29T19:19:47.728Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T19:19:47.728Z
Learning: Applies to steve-jooq/** : Implement all database persistence in the steve-jooq module using jOOQ; do not place database access code in other modules.
Applied to files:
pom.xml
📚 Learning: 2025-09-29T19:19:47.728Z
Learnt from: CR
PR: juherr/evolve#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T19:19:47.728Z
Learning: Applies to steve-core/** : Keep core business logic in steve-core and keep it framework-agnostic (no direct dependencies on Spring, jOOQ, Jetty, JSP, or other adapter frameworks in core).
Applied to files:
pom.xml
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java (2)
93-95: Consider consistent visibility for constants.The constant
DEFAULT_MAX_MSG_SIZEispublicwhileDEFAULT_IDLE_TIMEOUTandDEFAULT_ALLOWED_ORIGINSareprivate. Unless external access is specifically required forDEFAULT_MAX_MSG_SIZE, consider making all constants have the same visibility for consistency.Apply this diff to make all constants private:
- public static final int DEFAULT_MAX_MSG_SIZE = 8_388_608; // 8 MB for max message size + private static final int DEFAULT_MAX_MSG_SIZE = 8_388_608; // 8 MB for max message size
105-107: Consider making send time limit configurable.The
getSendTimeLimit()method returns a hardcodedDuration.ofSeconds(10)instead of using a configurable field. This is inconsistent withgetBufferSizeLimit(), which uses the configuredmaxTextMessageSize.If this 10-second timeout is intentional and should not be configurable, consider:
- Extracting it as a constant (e.g.,
DEFAULT_SEND_TIME_LIMIT) for clarity- Adding a comment explaining why it's fixed at 10 seconds
If it should be configurable, add a corresponding field in the
Wsclass.Option 1: Extract as a constant (if it should remain fixed):
private static final Duration DEFAULT_IDLE_TIMEOUT = Duration.ofHours(2); private static final String[] DEFAULT_ALLOWED_ORIGINS = new String[] {"*"}; + private static final Duration DEFAULT_SEND_TIME_LIMIT = Duration.ofSeconds(10); private int maxTextMessageSize = DEFAULT_MAX_MSG_SIZE; private Duration idleTimeout = DEFAULT_IDLE_TIMEOUT; private String[] allowedOriginPatterns = DEFAULT_ALLOWED_ORIGINS; public int getBufferSizeLimit() { return 5 * maxTextMessageSize; } public Duration getSendTimeLimit() { - return Duration.ofSeconds(10); + return DEFAULT_SEND_TIME_LIMIT; }Option 2: Make it configurable (if it should be customizable):
private int maxTextMessageSize = DEFAULT_MAX_MSG_SIZE; private Duration idleTimeout = DEFAULT_IDLE_TIMEOUT; private String[] allowedOriginPatterns = DEFAULT_ALLOWED_ORIGINS; + private Duration sendTimeLimit = Duration.ofSeconds(10); public int getBufferSizeLimit() { return 5 * maxTextMessageSize; } public Duration getSendTimeLimit() { - return Duration.ofSeconds(10); + return sendTimeLimit; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
steve-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep core business logic in steve-core and keep it framework-agnostic (no direct dependencies on Spring, jOOQ, Jetty, JSP, or other adapter frameworks in core).
Files:
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java
**/src/{main,test}/java/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Run Spotless and keep Java source formatted according to spotless-maven-plugin before committing (./mvnw spotless:apply).
Files:
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java
🔇 Additional comments (3)
steve-core/src/main/java/de/rwth/idsg/steve/config/SteveProperties.java (3)
26-27: LGTM!The
Durationimport is necessary for the new WebSocket timeout configurations in theWsclass.
89-89: LGTM!The
wsfield declaration follows the same initialization pattern as other configuration groups in this class.
101-103: LGTM!The
getBufferSizeLimit()method correctly derives the buffer size from the configuredmaxTextMessageSizewith a reasonable multiplier of 5.
Summary by CodeRabbit
New Features
Chores