DRAFT: Get client sessions irrespectively of their presence state #2760
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GregDThomas
reviewed
Apr 15, 2025
guusdk
force-pushed
the
getclientsessions
branch
4 times, most recently
from
dee3699 to
3df1d52
Compare
`org.jivesoftware.openfire.SessionManager#getSessions(java.lang.String)` returned sessions only when they are 'available'. This is a (side?)effect of its implementation depending on `org.jivesoftware.openfire.RoutingTable#getRoutes` I don't know if there are cases where it is desirable to get only 'available' sessions. I do know that being able to retrieve only 'available' sessions introduces issues. For one, it prevents most code to evaluate non-available session. For example, code can fail to terminate a non-available session (or a session where the availability state hasn't been updated yet due to a race condition). This commit introduces a new implementation that allows ClientSessions to be obtained irrespectively of their availability status. I'm not at all certain that this is the right approach. I am offering this up as an illustration, for discussion purposes.
The safeguard that is modified here asserts that a to-be-terminated session isn't replaced by another session. The previous implementation checked if a new _route_ exists for the JID. A route is a more specific session: one that is 'routable' (e.g. has sent initial presence). That distinction is not relevant for the feature that's implemented here. It is unlikely that this change will result in a functional difference (as having a detached session that's not routable is unlikely), yet change should be a better representation of what is being asserted.
…han Route
When determining if data associated to remote entities can be cleaned up, it is better to evaluate if this data is associated to existing _sessions_ than _routes_.
A route is a more specific session: one that is 'routable' (e.g. has sent initial presence). It is conceivable that a directed presence has been sent to a non-routable ('hidden') entity, that shouldn't be removed.
When determining if there's a potential conflict during resource binding, it is better to evaluate if this data is associated to existing _sessions_ than _routes_. For this use-case, it is not needed to evaluate the 'routability' of a session. Even sessions that aren't routable should be evaluated
Conceptually, a _route_ is more specific than a _session_. When logging information about a session, using a session rather than a route is a better fit.
When determining if there's a potential other/older session that's being resumed, it is better look for existing _sessions_ than _routes_, when you assume that a route is a more specific session (e.g. a session that is 'route-able' because it has sent initial presence).
… RoutingTable Sessions obtained from a routing table are supposed to be _route-able_. Although many/most use cases probably intend to use such a session, sessions obtained from the SessionManager can include more sessions (such as pre-authenticated sessions). When a session is desired, that aught to be retreived from the SessionManager, not a RoutingTable.
guusdk
force-pushed
the
getclientsessions
branch
from
3df1d52 to
c47a118
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
org.jivesoftware.openfire.SessionManager#getSessions(java.lang.String)returned sessions only when they are 'available'. This is a (side?)effect of its implementation depending onorg.jivesoftware.openfire.RoutingTable#getRoutesI don't know if there are cases where it is desirable to get only 'available' sessions.
I do know that being able to retrieve only 'available' sessions introduces issues. For one, it prevents most code to evaluate non-available session. For example, code can fail to terminate a non-available session (or a session where the availability state hasn't been updated yet due to a race condition).
This commit introduces a new implementation that allows ClientSessions to be obtained irrespectively of their availability status.
I'm not at all certain that this is the right approach. I am offering this up as an illustration, for discussion purposes.