Skip to content

2040 preparation introduce evse#2041

Merged
goekay merged 4 commits into
masterfrom
2040-preparation-introduce-evse
May 17, 2026
Merged

2040 preparation introduce evse#2041
goekay merged 4 commits into
masterfrom
2040-preparation-introduce-evse

Conversation

@goekay

@goekay goekay commented May 13, 2026

Copy link
Copy Markdown
Member
  • connector is promoted and renamed to evse.
  • evse_connector is introduced as the physical socket layer.
  • old connector_pk history references become evse_pk, which matches the “OCPP 1 connector becomes EVSE” transition.
  • OCPP 1 runtime paths correctly namespace EVSE lookup with topology_source = ocpp1.
  • OCPP 1 runtime creation now creates one physical connector under each non-zero EVSE, with evse_connector.connector_id = 1.
  • EVSE 0 is correctly left without a physical connector.
  • import/export includes EVSE_CONNECTOR.
  • tests seed EVSE_CONNECTOR and cover the OCPP 1 connector-to-EVSE mapping.
  • the business logic and flows map database evse_id/evse_pk for ocpp1 stations back to connectorId/connectorPk. as a result, the service and web (api and mvc) layers remain untouched.

@goekay goekay linked an issue May 13, 2026 that may be closed by this pull request
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Introduce EVSE model and physical connector layer for OCPP 1 compatibility

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Refactor connector table/model to evse with renamed columns
• Introduce evse_connector table for physical socket layer
• Update all repository queries to use EVSE with topology_source filtering
• Add OCPP 1 runtime logic to create physical connectors under non-zero EVSEs
• Update tests and import/export to support new EVSE/EVSE_CONNECTOR structure
Diagram
flowchart LR
  A["CONNECTOR table"] -->|rename| B["EVSE table"]
  B -->|add topology_source| C["EVSE with OCPP1 filtering"]
  C -->|create physical layer| D["EVSE_CONNECTOR table"]
  E["Repository queries"] -->|refactor| F["Use EVSE + topology_source"]
  F -->|auto-create| D
Loading

Grey Divider

File Changes

1. src/main/java/de/rwth/idsg/steve/repository/impl/ChargePointRepositoryImpl.java Refactoring +26/-23

Update connector queries to use EVSE table

src/main/java/de/rwth/idsg/steve/repository/impl/ChargePointRepositoryImpl.java


2. src/main/java/de/rwth/idsg/steve/repository/impl/ChargingProfileRepositoryImpl.java Refactoring +30/-26

Refactor charging profile queries for EVSE model

src/main/java/de/rwth/idsg/steve/repository/impl/ChargingProfileRepositoryImpl.java


3. src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java ✨ Enhancement +81/-33

Implement EVSE/EVSE_CONNECTOR creation logic

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java


View more (9)
4. src/main/java/de/rwth/idsg/steve/repository/impl/ReservationRepositoryImpl.java Refactoring +30/-24

Update reservation queries to use EVSE

src/main/java/de/rwth/idsg/steve/repository/impl/ReservationRepositoryImpl.java


5. src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java Refactoring +29/-23

Refactor transaction queries for EVSE model

src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java


6. src/main/java/de/rwth/idsg/steve/service/DataImportExportService.java ✨ Enhancement +4/-2

Add EVSE_CONNECTOR to import/export tables

src/main/java/de/rwth/idsg/steve/service/DataImportExportService.java


7. src/test/java/de/rwth/idsg/steve/certification/ocpp16/Ocpp16JsonCsmsCertificationIT.java 🧪 Tests +8/-6

Update certification test for EVSE model

src/test/java/de/rwth/idsg/steve/certification/ocpp16/Ocpp16JsonCsmsCertificationIT.java


8. src/test/java/de/rwth/idsg/steve/repository/impl/AbstractRepositoryITBase.java 🧪 Tests +20/-4

Seed EVSE and EVSE_CONNECTOR in test setup

src/test/java/de/rwth/idsg/steve/repository/impl/AbstractRepositoryITBase.java


9. src/test/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImplIT.java 🧪 Tests +41/-15

Add test for EVSE_CONNECTOR auto-creation

src/test/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImplIT.java


10. src/test/java/de/rwth/idsg/steve/repository/impl/ReservationRepositoryImplIT.java 🧪 Tests +20/-16

Update reservation tests for EVSE model

src/test/java/de/rwth/idsg/steve/repository/impl/ReservationRepositoryImplIT.java


11. src/test/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImplIT.java 🧪 Tests +18/-15

Update transaction tests for EVSE model

src/test/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImplIT.java


12. src/main/resources/db/migration/V1_1_4__update.sql Database migration +135/-0

Database migration for EVSE and EVSE_CONNECTOR

src/main/resources/db/migration/V1_1_4__update.sql


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented May 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. insertIgnoreConnector lacks input validation ✓ Resolved 📘 Rule violation ☼ Reliability
Description
insertIgnoreConnector() uses externally supplied chargeBoxIdentity directly in SQL predicates
without validating null/blank, which can lead to ambiguous queries or runtime failures instead of a
clear validation error.
Code

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[R434-455]

+    public static void insertIgnoreConnector(DSLContext ctx, String chargeBoxIdentity, int connectorId, boolean inTransactionAlready) {
+        if (inTransactionAlready) {
+            insertIgnoreConnectorInternal(ctx, chargeBoxIdentity, connectorId);
+        } else {
+            ctx.transaction(configuration ->
+                insertIgnoreConnectorInternal(DSL.using(configuration), chargeBoxIdentity, connectorId)
+            );
+        }
+    }
+
+    private static void insertIgnoreConnectorInternal(DSLContext ctx, String chargeBoxIdentity, int connectorId) {
+        var topology = ctx.select(EVSE.EVSE_PK, EVSE_CONNECTOR.EVSE_CONNECTOR_PK)
+            .from(CHARGE_BOX)
+            .leftJoin(EVSE)
+                .on(EVSE.CHARGE_BOX_ID.eq(CHARGE_BOX.CHARGE_BOX_ID))
+                .and(EVSE.TOPOLOGY_SOURCE.eq(EvseTopologySource.ocpp1))
+                .and(EVSE.EVSE_ID.eq(connectorId))
+            .leftJoin(EVSE_CONNECTOR)
+                .on(EVSE_CONNECTOR.EVSE_PK.eq(EVSE.EVSE_PK))
+                .and(EVSE_CONNECTOR.CONNECTOR_ID.eq(1))
+            .where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxIdentity))
+            .forUpdate()
Evidence
Rule 5 requires validating externally supplied inputs for null/blank before building SQL predicates.
The added insertIgnoreConnector()/insertIgnoreConnectorInternal() code uses chargeBoxIdentity
directly in CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxIdentity) without any null/blank guard or
validation exception.

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[434-455]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`insertIgnoreConnector()` accepts `chargeBoxIdentity` (request/station-derived) and uses it to build SQL predicates without validating null/blank. This violates the input validation requirement and can cause unclear behavior or runtime errors.
## Issue Context
The compliance requirement expects fail-fast validation (e.g., IllegalArgument/BadRequest-style) before dereferencing/using inputs in SQL predicate construction.
## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[434-455]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. insertIgnoreConnectorInternal uses generic exception ✓ Resolved 📘 Rule violation ⛨ Security
Description
When the charge box is not found, the new code throws a generic SteveException with an internal
identifier in the message, which may map to an incorrect client error and can leak internal details.
Code

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[R458-460]

+        if (topology == null) {
+            throw new SteveException("Charge box with id '%s' not found", chargeBoxIdentity); // should not happen
+        }
Evidence
Rule 6 requires semantically correct exception types for not-found conditions and avoiding leaking
internal details in client-visible errors. The new code throws `SteveException("Charge box with id
'%s' not found", chargeBoxIdentity)` on a not-found condition.

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[458-460]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A not-found condition for `chargeBoxIdentity` throws a generic `SteveException` and includes the identifier in the exception message. This can produce incorrect error semantics for clients and may leak internal details.
## Issue Context
Per compliance, not-found conditions should map to a not-found/404-style exception, and client-visible messages should be generic (detailed context should be logged internally).
## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[458-460]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Unconditional FOR UPDATE lock ✓ Resolved 🐞 Bug ➹ Performance
Description
insertIgnoreConnectorInternal always runs a SELECT ... FOR UPDATE on the charge_box row even
when the EVSE/evse_connector already exist, causing avoidable per-chargeBoxId serialization in hot
paths (StatusNotification/MeterValues ingestion). Under concurrent message load for the same station
this can increase latency and produce lock wait timeouts without adding correctness once rows are
already present.
Code

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[R444-456]

+    private static void insertIgnoreConnectorInternal(DSLContext ctx, String chargeBoxIdentity, int connectorId) {
+        var topology = ctx.select(EVSE.EVSE_PK, EVSE_CONNECTOR.EVSE_CONNECTOR_PK)
+            .from(CHARGE_BOX)
+            .leftJoin(EVSE)
+                .on(EVSE.CHARGE_BOX_ID.eq(CHARGE_BOX.CHARGE_BOX_ID))
+                .and(EVSE.TOPOLOGY_SOURCE.eq(EvseTopologySource.ocpp1))
+                .and(EVSE.EVSE_ID.eq(connectorId))
+            .leftJoin(EVSE_CONNECTOR)
+                .on(EVSE_CONNECTOR.EVSE_PK.eq(EVSE.EVSE_PK))
+                .and(EVSE_CONNECTOR.CONNECTOR_ID.eq(1))
+            .where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxIdentity))
+            .forUpdate()
+            .fetchOne();
Evidence
The repository uses insertIgnoreConnectorInternal from the OCPP1 ingestion paths
insertConnectorStatus and insertMeterValues. The internal query always applies .forUpdate() on
CHARGE_BOX regardless of whether any insert is needed, which serializes concurrent calls per
chargeBoxId.

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[145-193]
src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[444-456]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`OcppServerRepositoryImpl.insertIgnoreConnectorInternal` unconditionally acquires a row lock via `SELECT ... FOR UPDATE` on the `charge_box` row. Because this method is invoked from message-ingestion hot paths, it can create avoidable contention for stations sending concurrent messages (even when the EVSE/physical connector already exist).
## Issue Context
The lock is helpful only for the *creation* race; once an EVSE and default `evse_connector(connector_id=1)` exist, continuing to lock `charge_box` on every call adds contention without further correctness benefit.
## Fix approach (keep correctness, reduce contention)
- First perform a non-locking lookup to check if the EVSE exists and whether a default physical connector exists.
- Only if either is missing, enter a transaction/locking path to create the missing rows.
- Alternatively, use idempotent upserts (`INSERT ... ON DUPLICATE KEY IGNORE`) for EVSE and EVSE_CONNECTOR and then `SELECT evse_pk`, which avoids the explicit `FOR UPDATE` lock entirely.
## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[434-486]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. EVSE creation only debug-logged 🐞 Bug ◔ Observability
Description
Automatic creation of EVSE and the default physical connector is logged only at DEBUG level, so
topology changes triggered by real station traffic are typically invisible in production INFO-level
logs. This reduces auditability/troubleshooting when unexpected EVSEs/connectors appear.
Code

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[R465-485]

+        if (evsePk == null) {
+            evsePk = ctx.insertInto(EVSE)
+                .set(EVSE.CHARGE_BOX_ID, chargeBoxIdentity)
+                .set(EVSE.TOPOLOGY_SOURCE, EvseTopologySource.ocpp1)
+                .set(EVSE.EVSE_ID, connectorId)
+                .returning(EVSE.EVSE_PK)
+                .fetchOne(EVSE.EVSE_PK);
+
+            log.debug("The connector {}/{} is NEW, and inserted into DB.", chargeBoxIdentity, connectorId);
+        }
+
+        if (connectorId != 0 && evseConnectorPk == null) {
+            final int defaultEvseConnectorId = 1;
+
+            ctx.insertInto(EVSE_CONNECTOR)
+                .set(EVSE_CONNECTOR.EVSE_PK, evsePk)
+                .set(EVSE_CONNECTOR.CONNECTOR_ID, defaultEvseConnectorId)
+                .execute();
-        if (count == 1) {
-            log.info("The connector {}/{} is NEW, and inserted into DB.", chargeBoxIdentity, connectorId);
+            log.debug("The evse_connector {}/{}/{} is NEW, and inserted into DB.", chargeBoxIdentity, connectorId, defaultEvseConnectorId);
    }
Evidence
The only EVSE/EVSE_CONNECTOR auto-creation logic in this PR logs creation at DEBUG, and it is called
from OCPP message ingestion flows.

src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[145-193]
src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[465-485]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`insertIgnoreConnectorInternal` logs EVSE and EVSE_CONNECTOR auto-creation with `log.debug(...)`. In common production configurations (INFO level), these topology mutations become effectively unlogged.
## Issue Context
This method is invoked by OCPP runtime ingestion and is the path that expands topology based on incoming station messages.
## Fix
Promote these creation logs to INFO (or at least WARN for unexpected connectorIds) so operators can audit when new EVSEs / physical connectors are created.
## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/repository/impl/OcppServerRepositoryImpl.java[465-485]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

goekay added 3 commits May 13, 2026 13:00
* previously, we were "insert or ignore"ing connector ids as they come. and then, we were using another SQL query to select
the connector pk relating to this connector id, because the initial "insert or ignore" was not able to return the PK. with the
new impl of insertIgnoreConnectorInternal, we can return evsePk. as a result, the additional SQL query is NOT necessary at all.
subsequent queries that need evsePk can just use the literal value of returned evsePk.

* localize all evsePk selection queries that are scattered across multiple repository impl classes. now, we have evsePkSelect methods
in Ocpp1ConnectorEvseBridge, which can be referenced by repository impls.
@goekay goekay force-pushed the 2040-preparation-introduce-evse branch from b2810f8 to ccc54db Compare May 13, 2026 11:01
* add them to the ChargePointForm
* render them for information purposes
* allow select and necesssary fields to be edited
@goekay

goekay commented May 14, 2026

Copy link
Copy Markdown
Member Author

how "device model" section looks and behaves like on charge point details page:

Screenshot 2026-05-14 at 11 18 04

@goekay goekay merged commit e9a18ad into master May 17, 2026
41 checks passed
@goekay goekay deleted the 2040-preparation-introduce-evse branch May 17, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preparation: Introduce "EVSE"

1 participant