allthingssecurity opened a new pull request, #29435:
URL: https://github.com/apache/camel/pull/29435

   # Description
   
   [CAMEL-26681](https://issues.apache.org/jira/browse/CAMEL-26681) (item 1)
   
   Since CAMEL-25841 the consumers and producers of one account share one 
connection, and the producers connect and log it in again through 
`XmppEndpoint.reconnect(connection)`, under the account's lock (CAMEL-25961). 
The consumer's connection monitor (`XmppConsumer.checkConnection`) still called 
`connection.connect()` and `endpoint.login(connection)` itself, without that 
lock. This was safe, as Smack's `connect` and `login` are synchronized and the 
monitor treated `AlreadyConnectedException` and `AlreadyLoggedInException` as 
done, but the users of an account reconnected in two different ways.
   
   Change: `checkConnection` calls `endpoint.reconnect(connection)` when the 
connection is not connected or not logged in, so the consumers and producers of 
an account connect and log it in again in one place, under the account's lock 
(Smack's `ReconnectionManager` still reconnects on its own, as before; Camel 
treats what it did as done). Kept as before:
   - the room is entered again after a login (`rejoinRoomIfNeeded()`, and the 
`authenticated` listener);
   - a user with `login=false` is connected but not logged in 
(`XmppComponent.connectAndLogin` has the same condition);
   - `AlreadyConnectedException` and `AlreadyLoggedInException` (from Smack's 
`ReconnectionManager` or another user of the account) only mean that it was 
done already;
   - a failed connect is logged at WARN and tried again at the next check; that 
WARN now also covers a login that fails with a `SmackException` (before, it 
went to the WARN of the monitor's runnable). With `login=false` and a user, the 
monitor now takes the account's lock at each check to find there is nothing to 
do.
   
   `XmppComponent` gets a package-private `connectionLock(endpoint)` for the 
test, next to the existing `connectionUsers(endpoint)` (its 
`SharedConnection.lock` field is now typed `ReentrantLock`).
   
   The login now uses `connection.login()`, as the producers do: the user, 
password and resource the connection logged in with when it was created (by 
`XmppEndpoint.createConnection`, through `endpoint.login`).
   
   Tests (with the existing `FakeXmppServer`):
   - 
`XmppConsumerReconnectTest.connectionMonitorConnectsAgainUnderTheLockOfTheAccount`:
 a chat and a room consumer of one account; with Smack's `ReconnectionManager` 
turned off for the connection, the test holds the account's lock 
(`connectionLock`, as a producer does while it reconnects) and drops the 
connection. The monitors wait for the lock and the connection stays down; after 
the unlock the consumer is in the room again and receives the next room message.
   - `XmppConsumerConnectionRaceTest`: its two cases (connected meanwhile by 
the `ReconnectionManager`: still logged in; logged in meanwhile: not a failure) 
now record the login on the connection instead of on the endpoint, as the 
monitor logs in through the shared reconnect; plus a case for `login=false` 
with a user (connected, not logged in).
   
   On main the new test fails (the monitor never waits for the lock: 
`ConditionTimeoutException` after 10 s); the race test cases pass on main and 
with the change. With the change, the new test and the race test: 80/80 over 20 
runs. camel-xmpp tests (offline, `-DskipITs`, with the core modules in the 
reactor): 32 run, 0 failures, 5 skipped (the Google Talk tests and 
`XmppRobustConnectionTest`, which need a real server); the formatter and 
import-sort plugins changed nothing.
   
   Not in this PR: items 2 (a consumer and a group chat producer of one room 
share Smack's `MultiUserChat`, so both enter the room again after a reconnect) 
and 3 (a docs sentence on the first group chat message after a reconnect) of 
CAMEL-26681. Item 2 needs state shared by all users of a room on a connection, 
which is a design choice for a separate change; item 3 is a docs change for the 
same reconnect notes and fits better with item 2.
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (Camel 4 
uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked 
there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for 
the change (usually before you start working on it): 
[CAMEL-26681](https://issues.apache.org/jira/browse/CAMEL-26681).
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful 
subject line and body.
   - [ ] I have run `mvn clean install -DskipTests` locally from root folder 
and I have committed all auto-generated changes.
     (I built the core modules and camel-xmpp with the formatter and 
import-sort plugins and ran the camel-xmpp tests; no generated file changes. I 
did not run the full root build.)
   
   # AI-assisted contributions
   
   - [x] If this PR includes AI-generated code, commits have proper 
co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR 
description identifies the AI tool used.
     This PR was prepared with Claude Code (Claude Opus 5.5). The commit 
carries a `Co-Authored-By` trailer.
   
   _Claude Code on behalf of allthingssecurity_
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to