steve-community
Refactor SessionContextStore to make it a singleton
Was mentioned by @juherr in https://github.com/steve-community/steve/pull/1822
Am I mistaken to think that SessionContextStore should actually be a singleton in the application? Right now, it seems we’re creating three instances — one per WebSocketEndpoint (1.2, 1.5, 1.6).
https://github.com/steve-community/steve/pull/1822#discussion_r2366218067
it cannot be a singleton just by easily refactoring it and putting a @Component on it. reason is:
within ChargePointHelperServiceImpl we are accessing different AbstractWebSocketEndpoint instances (ocpp 1.2, 1.5 and 1.6) in order to read statistics from them (getNumberOfChargeBoxes, getACopy, etc.). and we need the version differentiation in some cases (see setNumOcpp12JChargeBoxes, setNumOcpp15JChargeBoxes etc.).
in short: by having three instances baked into ocpp version-specific AbstractWebSocketEndpoints, we get some partitioning by ocpp versions. if we would have just one instance, we would lose that. we could recreate these partitions within the singleton SessionContextStore, but then we would have a very similar outcome to current one. what would we gain?
I think there's currently little risk of data duplication if a station switches versions quickly.
One possible concern is that the lookup table might produce unexpected results if there’s a dedicated table per version. Would it make sense to define the expected behavior in case a station switches between OCPP versions?
Even if such a situation indicates the station isn’t strictly spec-compliant, it could be worth adding a safeguard. Perhaps introducing a test case would help document and validate the intended behavior.
https://github.com/steve-community/steve/pull/1822#discussion_r2366269802
That said, it feels more like a philosophical question: where should we draw the line of partitioning?
At the moment, it seems only the UI actually needs this distinction, which doesn’t feel like a strong enough reason to enforce it in the core business logic. Implementing the partition at the controller level would likely be straightforward and less intrusive.
Alternatively, a deeper integration handled directly within the AbstractWebSocketEndpoint logic could offer a cleaner long-term design and naturally support horizontal scalability, though it would come with broader impacts.
At the moment, it seems only the UI actually needs this distinction
yes, it is only relevant for UI, but still it is relevant.
let's assume we did a singleton SessionContextStore, but still wanted the version separation for UI. we can always get over this hurdle by getting all the chargeBoxIds from the SessionContextStore and then looking up their versions from DB in order to prepare the DTO for the UI. but it feels less practical and cumbersome (doing an unnecessary round trip with DB), when we can have the data in memory already present.
IMO having a singleton SessionContextStore feels like just for the sake of doing it.
There could be another approach where you skip the database lookup by storing both the chargeboxId and version in the store, instead of just the chargeboxId.
I think there's currently little risk of data duplication if a station switches versions quickly.
this needs a more elaborate answer. so here it is...
all websocket sessions are tied to an ocpp version. the ocpp version MUST be provided as the Sec-WebSocket-Protocol header during the websocket handshake. therefore, a station cannot switch versions and continue using the old/open session. station MUST create a new session. lets explore what this means (for steve):
a new session with another version will be created. steve will store this new version in DB. from here on, all outgoing requests (steve -> station) will use this version and therefore the session associated with it. so, we actually always use the most recent info.
the old/open session can be closed by station or not. it would be best if it closed. but that session and pipeline could only be used by incoming requests (station -> steve) for steve to send the response to. most probably, the connection would stay open and would not be used until some external force closes it (timeout, restart, etc.). if the station can maintain multiple sessions with different versions, i would argue that it is in the interest of the station to use the session of the newest version, as well.
having said that, i find the probability of this happening (i.e. station switching versions on-the-fly without restart or closing all existing connections, and recreating with new version) very low. it sounds like a very insane thing to do. do you mention this because you saw this happening out in the wild, or are we discussing a very hypothetical and theoretical possibility?
Thankfully, I haven’t encountered such behavior so far. Still, with all the possible combinations of hardware quirks, OCPP or HTTP proxies, and network hiccups, anything’s possible.
By the way, it was just an architectural consideration that came to mind when I stumbled upon this piece of code.
I understand that the hypothetical issue is possible, and whether to share the same context store instance or not is ultimately a matter of design preference.