OICP support #128
OICP support #128
Conversation
WalkthroughAdds a new steve-oicp gateway module, configures OpenAPI code generation for OICP 2.3 server/client, introduces an OCPP→OICP adapter, and exposes REST controllers for authorization, EVSE data/status, and charging notifications. Root POM references the new module and adjusts plugin versioning. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CPO as OICP Client (CPO/EMP)
participant API as OICP REST Controller
participant AD as OcppToOicpAdapter
participant REPO as Core Repos/Services
rect rgb(240,245,255)
note over CPO,API: EVSE Data Pull (v2.3)
CPO->>API: eRoamingPullEvseDataV23(providerID, request)
API->>AD: getEVSEData(providerID, request)
AD->>REPO: Fetch charge point overviews
REPO-->>AD: Overviews + connectors
AD-->>API: ERoamingEVSEData
API-->>CPO: 200 OK (ERoamingEVSEData)
end
rect rgb(240,255,240)
note over CPO,API: EVSE Status Pull (v2.1)
CPO->>API: eRoamingPullEvseStatusV21(providerID, request)
API->>AD: getEVSEStatus(providerID, request)
AD->>REPO: Fetch connector statuses
REPO-->>AD: Status DTOs
AD-->>API: ERoamingPullEvseStatusV21200Response
API-->>CPO: 200 OK (Status)
end
rect rgb(255,250,240)
note over CPO,API: Authorization Start
CPO->>API: eRoamingAuthorizeStartV21(request)
API->>AD: authorizeStart(request)
AD->>REPO: Validate RFID/tag
REPO-->>AD: Tag state
AD-->>API: ERoamingAuthorizationStart (AUTHORIZED/NOT_AUTHORIZED)
API-->>CPO: 200 OK (Result)
end
rect rgb(255,240,245)
note over CPO,API: Charging Notifications / CDR
CPO->>API: eRoamingChargingNotificationsV11(request)
API->>AD: processChargingNotification(request)
AD-->>API: ERoamingAcknowledgment (true/false)
API-->>CPO: 200 OK (Ack)
CPO->>API: eRoamingChargeDetailRecordV22(cdr)
API->>AD: processChargeDetailRecord(cdr)
AD-->>API: ERoamingAcknowledgment
API-->>CPO: 200 OK (Ack)
end
opt Error path
API-->>CPO: 200 OK with StatusCode(code=3000/4000) and result=false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (6)
pom.xml (1)
315-364: Pin the Spotless plugin version.We previously hard-pinned
spotless-maven-plugin. Dropping the<version>now makes builds depend on whichever release Maven resolves at runtime, which risks sudden breakages when upstream publishes a new major. Please restore the explicit version (or manage it in<pluginManagement>).steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (3)
145-203: Avoid masking exceptions with generic catch.The catch block at line 195 catches all exceptions and converts them to NOT_AUTHORIZED responses. This could hide programming errors (NPE, IllegalStateException, etc.) that should be logged differently or allowed to propagate.
Consider catching only expected exceptions (e.g., repository access failures) and letting unexpected ones bubble up to be handled by a global exception handler.
217-236: Complete the TODO for notification handling.Line 229's TODO indicates different notification types should be handled differently, but currently all types are just acknowledged. For production, you'll need to implement actual processing logic for start, end, progress, and error notifications.
This appears to be a stub implementation. Do you want me to open an issue to track implementing the actual notification handling logic?
255-280: Add null-safe handling for missing charge point details.Line 259 uses
orElseThrow()without a descriptive exception. IfgetDetails()returns empty, this throws a genericNoSuchElementExceptionthat doesn't clearly indicate the charge point details are missing.Apply this diff to provide a clearer exception:
- var details = - chargePointRepository.getDetails(chargePoint.getChargeBoxPk()).orElseThrow(); + var details = chargePointRepository.getDetails(chargePoint.getChargeBoxPk()) + .orElseThrow(() -> new IllegalStateException( + "Charge point details not found for ID: " + chargePoint.getChargeBoxId()));steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
26-38: Add private constructor to prevent instantiation.This is a utility class with only static methods, but lacks a private constructor. While not critical, adding one prevents accidental instantiation and clearly signals the class's intent.
Apply this diff:
public class OicpResponse { + private OicpResponse() { + throw new UnsupportedOperationException("Utility class"); + } + public static <T> ResponseEntity<T> toResponse(T data) { return ResponseEntity.of(Optional.of(data)); }steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java (1)
56-66: Document unsupported operations.These methods throw
UnsupportedOperationExceptionfor operations not yet implemented. Consider adding a comment or logging to clarify whether these are planned features or intentionally excluded from the initial OICP support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pom.xml(1 hunks)steve-gateway/steve-oicp/pom.xml(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/ChargingNotificationsController.java(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.java(1 hunks)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/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-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/ChargingNotificationsController.javasteve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.javasteve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.javasteve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.javasteve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.javasteve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java
🧠 Learnings (1)
📚 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 **/src/{main,test}/java/**/*.java : Run Spotless and keep Java source formatted according to spotless-maven-plugin before committing (./mvnw spotless:apply).
Applied to files:
pom.xml
🧬 Code graph analysis (5)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/ChargingNotificationsController.java (5)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (1)
Slf4j(74-342)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java (1)
Slf4j(48-125)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.java (1)
Slf4j(39-60)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java (1)
Slf4j(40-62)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
OicpResponse(26-38)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java (2)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (1)
Slf4j(74-342)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
OicpResponse(26-38)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.java (3)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (1)
Slf4j(74-342)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java (1)
Slf4j(40-62)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
OicpResponse(26-38)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java (2)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (1)
Slf4j(74-342)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
OicpResponse(26-38)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (6)
steve-core/src/main/java/de/rwth/idsg/steve/repository/dto/ChargePoint.java (1)
ChargePoint(33-86)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java (1)
Slf4j(48-125)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/ChargingNotificationsController.java (1)
Slf4j(39-62)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.java (1)
Slf4j(39-60)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java (1)
Slf4j(40-62)steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/OicpResponse.java (1)
OicpResponse(26-38)
⏰ 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). (10)
- GitHub Check: build (25, mariadb:10.5.21)
- GitHub Check: build (25, mariadb:10.6.14)
- GitHub Check: build (25, mariadb:10.4.30)
- GitHub Check: build (21, mariadb:10.6.14)
- GitHub Check: build (21, mysql:8.0)
- GitHub Check: build (25, mariadb:10.3)
- GitHub Check: build (21, mariadb:10.5.21)
- GitHub Check: build (25, mysql:8.0)
- GitHub Check: build (21, mariadb:10.3)
- GitHub Check: build (21, mariadb:10.4.30)
🔇 Additional comments (10)
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java (3)
205-214: Authorization stop needs validation logic.This method unconditionally returns AUTHORIZED without any validation. In a real scenario, you should verify the session exists and the stop request is valid.
Is this intentionally stubbed for the initial implementation, or should it include validation similar to
authorizeStart?
238-252: CDR processing is incomplete.This method only logs the charge detail record but doesn't persist or process it. For OICP compliance, CDRs typically need to be stored for billing and reconciliation purposes.
Is this intentionally deferred, or should CDR persistence be implemented now?
329-341: LGTM: Status mapping is correct.The OCPP to OICP status mapping logic correctly handles null input and uses case-insensitive comparison for string matching. The mapping choices are appropriate for the OICP standard.
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseDataController.java (1)
39-60: LGTM: Clean controller implementation.The controller follows Spring best practices with proper conditional loading, dependency injection, error handling, and logging. The error handling correctly wraps exceptions and returns appropriate OICP error responses.
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/ChargingNotificationsController.java (1)
39-62: LGTM: Consistent controller pattern.The implementation follows the same clean pattern as other controllers in this PR, with proper error handling and logging. The delegation to the adapter is correct.
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/EvseStatusController.java (1)
40-62: LGTM: Well-structured controller.The controller correctly implements the EVSE status endpoint with proper error handling and logging, consistent with other controllers in this module.
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/controller/AuthorizationController.java (4)
68-84: LGTM: Proper authorization start handling.The endpoint correctly delegates to the adapter with appropriate error handling. The fallback error response (code 6000) is reasonable for authorization failures.
86-101: LGTM: Clean authorization stop implementation.The implementation follows the same pattern as authorization start with consistent error handling and logging.
103-118: LGTM: CDR endpoint properly implemented.The charge detail record endpoint correctly delegates processing to the adapter with appropriate error handling (code 4000 for processing failures).
120-125: Unsupported CDR retrieval operation.Similar to remote start/stop, this method throws
UnsupportedOperationException. Consider whether retrieving charge detail records should be part of the initial OICP implementation.Is CDR retrieval functionality planned for a future iteration?
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-annotations-jakarta</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-annotations</artifactId> | ||
| <version>2.2.38</version> | ||
| </dependency> |
There was a problem hiding this comment.
Remove duplicated Swagger annotations dependency.
Both swagger-annotations-jakarta and swagger-annotations ship the exact same classes (io.swagger.v3.oas.annotations.*). With the global <banDuplicateClasses findAllDuplicates="true"/>, the build will now fail. Keep only the Jakarta artifact (needed for Spring Boot 3) and drop the vanilla swagger-annotations.
🤖 Prompt for AI Agents
In steve-gateway/steve-oicp/pom.xml around lines 36 to 43, there is a duplicated
Swagger annotations dependency causing duplicate-class failures; remove the
<dependency> block for io.swagger.core.v3:swagger-annotations (the non-Jakarta
artifact) and keep only the swagger-annotations-jakarta dependency required for
Spring Boot 3, updating the pom so only the jakarta artifact remains and no
duplicate classes are declared.
| public ERoamingEVSEData getEVSEData(String providerID, ERoamingPullEVSEData request) { | ||
| log.debug("Converting charge points to OICP EVSE data for provider: {}", providerID); | ||
|
|
||
| var form = new ChargePointQueryForm(); | ||
| // TODO update form based on request filters | ||
|
|
||
| var evseDataList = chargePointRepository.getOverview(form).stream() | ||
| .map(this::convertChargePointToEVSEData) | ||
| .filter(Objects::nonNull) | ||
| .toList(); | ||
|
|
||
| log.debug("Converted {} charge point overview to OICP EVSE data records", evseDataList.size()); | ||
| return ERoamingEVSEData.builder() | ||
| .statusCode(successCode()) | ||
| .content(evseDataList) | ||
| .size(evseDataList.size()) | ||
| .numberOfElements(evseDataList.size()) | ||
| .first(true) | ||
| .last(true) | ||
| .number(0) | ||
| .totalElements(evseDataList.size()) | ||
| .totalPages(1) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Complete the TODO and implement pagination properly.
The EVSE data retrieval has two issues:
- Request filters are not applied (line 87 TODO)
- Pagination metadata is hardcoded (lines 100-104), returning all data as a single page regardless of actual dataset size
For large deployments, this could cause performance issues and memory pressure.
Would you like me to generate a verification script to check if there are existing pagination utilities in the codebase that could be leveraged here?
🤖 Prompt for AI Agents
In
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java
around lines 83 to 106, the ChargePointQueryForm is not populated from the
ERoamingPullEVSEData request and pagination metadata is hardcoded; populate the
form with filtering values from the request (providerID, geographic/time
filters, status/type, and any other supported fields) and implement proper
pagination by reading page and size from the request (or sensible defaults),
then query the repository using a paginated API if available (preferred) or
apply stream skip/limit to the full result if not; compute totalElements from
the repository/count query, derive totalPages = ceil(totalElements/size), set
number = page, size = pageSize, numberOfElements = actual returned list size,
first = page == 0, last = page >= totalPages - 1, and fill those calculated
values into the ERoamingEVSEData builder instead of the current hardcoded
values.
| public ERoamingPullEvseStatusV21200Response getEVSEStatus( | ||
| String providerID, ERoamingPullEvseStatusV21Request request) { | ||
| log.debug("Getting EVSE status for provider: {}", providerID); | ||
|
|
||
| var form = new ConnectorStatusForm(); | ||
| // TODO update form based on request filters | ||
| switch (request) { | ||
| case ERoamingPullEVSEStatus status -> { | ||
| // Handle ERoamingPullEVSEStatus specific logic | ||
| // For now, no specific filters are applied | ||
| } | ||
| case ERoamingPullEVSEStatusByID byId -> { | ||
| // Handle ERoamingPullEVSEStatusByID specific logic | ||
| // For now, no specific filters are applied | ||
| } | ||
| case ERoamingPullEVSEStatusByOperatorID byOperatorId -> { | ||
| // Handle ERoamingPullEVSEStatusByOperatorID specific logic | ||
| // For now, no specific filters are applied | ||
| } | ||
| default -> throw new IllegalArgumentException("Unknown request type: " + request.getClass()); | ||
| } | ||
|
|
||
| var statusRecords = chargePointRepository.getChargePointConnectorStatus(form).stream() | ||
| .map(OcppToOicpAdapter::convertConnectorStatusToEVSEStatus) | ||
| .filter(Objects::nonNull) | ||
| .toList(); | ||
|
|
||
| log.debug("Converted {} connector statuses to OICP EVSE status records", statusRecords.size()); | ||
| return ERoamingEVSEStatusByID.builder() | ||
| .statusCode(successCode()) | ||
| .evSEStatusRecords(ERoamingEVSEStatusByIDEVSEStatusRecords.builder() | ||
| .evseStatusRecord(statusRecords) | ||
| .build()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Implement request type filtering.
The switch statement (lines 114-128) has empty cases for all three request types, and the TODO at line 113 indicates filters are not yet applied. This means all three request variants return identical unfiltered results, which defeats their purpose.
🤖 Prompt for AI Agents
In
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java
around lines 108-142, the switch over request types is empty so all variants
return the same unfiltered results; populate the ConnectorStatusForm inside each
case: for ERoamingPullEVSEStatus leave form unfiltered (global), for
ERoamingPullEVSEStatusByID extract the EVSE/connector IDs from the byId request
and set them on the form (single or list as appropriate), and for
ERoamingPullEVSEStatusByOperatorID set the operatorId on the form; validate
null/empty inputs before setting fields and ensure the repository call uses the
updated form so each request variant returns the correct filtered records.
| private static @Nullable AddressIso19773 convertAddress(ChargePoint.@Nullable Details addressRecord) { | ||
| if (addressRecord == null) { | ||
| return null; | ||
| } | ||
|
|
||
| return AddressIso19773.builder() | ||
| .country(addressRecord.getCountry().getName()) | ||
| .city(addressRecord.getCity()) | ||
| .street(addressRecord.getStreet()) | ||
| .postalCode(addressRecord.getZipCode()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Potential NPE if country is null.
Line 288 calls addressRecord.getCountry().getName() which could throw NPE if country is null. Based on the ChargePoint.Details definition, country is of type CountryCode and may be nullable.
Add a null check for country or handle the case where address data is incomplete:
private static @Nullable AddressIso19773 convertAddress(ChargePoint.@Nullable Details addressRecord) {
if (addressRecord == null) {
return null;
}
+ if (addressRecord.getCountry() == null) {
+ return null;
+ }
return AddressIso19773.builder()
.country(addressRecord.getCountry().getName())
.city(addressRecord.getCity())
.street(addressRecord.getStreet())
.postalCode(addressRecord.getZipCode())
.build();
}📝 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.
| private static @Nullable AddressIso19773 convertAddress(ChargePoint.@Nullable Details addressRecord) { | |
| if (addressRecord == null) { | |
| return null; | |
| } | |
| return AddressIso19773.builder() | |
| .country(addressRecord.getCountry().getName()) | |
| .city(addressRecord.getCity()) | |
| .street(addressRecord.getStreet()) | |
| .postalCode(addressRecord.getZipCode()) | |
| .build(); | |
| } | |
| private static @Nullable AddressIso19773 convertAddress(ChargePoint.@Nullable Details addressRecord) { | |
| if (addressRecord == null) { | |
| return null; | |
| } | |
| if (addressRecord.getCountry() == null) { | |
| return null; | |
| } | |
| return AddressIso19773.builder() | |
| .country(addressRecord.getCountry().getName()) | |
| .city(addressRecord.getCity()) | |
| .street(addressRecord.getStreet()) | |
| .postalCode(addressRecord.getZipCode()) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In
steve-gateway/steve-oicp/src/main/java/de/rwth/idsg/steve/gateway/oicp/adapter/OcppToOicpAdapter.java
around lines 282 to 293, the call to addressRecord.getCountry().getName() can
NPE when country is null; guard against a null country before calling getName()
and pass null (or an empty string if preferred) into the
AddressIso19773.builder(). For example, compute a countryName variable that is
addressRecord.getCountry() != null ? addressRecord.getCountry().getName() : null
and use that in the builder so the method safely handles incomplete address
records.
|
@fmandache Based on your work, I’ve implemented these changes. From what I can tell, several parts of the business logic and some mandatory controllers are still missing. Is there a specific reason for these omissions? @goekay Fyi. |
Summary by CodeRabbit
New Features
Chores