Iterate on client creation by bmarty · Pull Request #7591 · element-hq/element-x-android

bmarty · 2026-08-28T16:40:13Z

Content

Make sure that a RustMatrixClient is always created the same way, either when restoring a session or when signing in.

It ensure that the hook are correctly applied.

(See #7590 (review)).

Motivation and context

Ensure that hook works the same way.

Screenshots / GIFs

Tests

Login flows and session restoration should still work as expected.

Tested devices

Physical
Emulator
OS version(s):

Checklist

bmarty · 2026-08-28T16:43:35Z

-                clientEnterpriseHook(matrixClient)
-
-                newMatrixClientObservers.forEach { it.invoke(matrixClient) }
-                sessionStore.addSession(sessionData)


Worth noting that matrixClient.waitForKnownVerificationState() was not called in this case.

github-actions · 2026-08-28T16:46:57Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/yHw1nY

codecov · 2026-08-28T17:23:28Z

Codecov Report

❌ Patch coverage is 77.14286% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.09%. Comparing base (039c0df) to head (3cde152).
⚠️ Report is 12 commits behind head on develop.

Files with missing lines	Patch %	Lines
...atrix/impl/auth/RustMatrixAuthenticationService.kt	68.18%	4 Missing and 3 partials ⚠️
...erprise/impl/DefaultClientBuilderEnterpriseHook.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #7591      +/-   ##
===========================================
+ Coverage    80.98%   81.09%   +0.10%     
===========================================
  Files         2781     2782       +1     
  Lines        81840    81844       +4     
  Branches     11252    11251       -1     
===========================================
+ Hits         66280    66370      +90     
+ Misses       11217    11120      -97     
- Partials      4343     4354      +11

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jmartinesp · 2026-08-31T07:17:21Z

-    suspend fun create(
-        client: Client,
-        sessionData: SessionData,
-        isMessageSearchAvailable: Boolean,
-    ): RustMatrixClient {


Note by doing this we're not reusing an already valid client and are forcing the creation of a different client, which can mean ~1s for opening the SDK DBs again I think. It's not that important since it happens on the login flow, but that's why we had 2 methods and tried to avoid destroying the current client in the log in flows.

I see. So maybe we could have a second hook to update the client once it's logged in? Not sure if this will work for @richvdh .

To let this PR focused on this change, I'll extract the first commit to its own PR.

I have measured 580ms spent on the new method finalizeClientCreation, but only 80 ms for the method restoreSession in it, so the impact is actually very small (?)

In theory we could work around it, but for my usecase it would be much better to run the hook before the client is created. I don't need to know the userid though, so provided the hook actually gets called whenever a client is created, that would be fine.

I do not understand, how the hook using MatrixClient can be called before MatrixClient is created? Are you talking about ClientEnterpriseHook or something else?

I'm talking about ClientBuilderEnterpriseHook. I need a way to call methods on the underlying rust-side ClientBuilder each time we build a Client.

I could probably work around it by adding methods to the rust-side MatrixClient and calling them after the client is created, but I'd much prefer to avoid that.

richvdh · 2026-09-07T15:06:45Z

+
+    /**
+     * Customize the [MatrixClientBuilder] for enterprise features.
+     * This method is invoked when a new [MatrixClientBuilder] is created to build a client for a specific session.


would be good to be explicit if this is called as well as the other method, or instead of it.

(also imho the two implementations should have different names, since they serve somewhat different purposes, but ymmv)

The documentation of the other method is explicit: "This method is invoked everytime a new [MatrixClientBuilder] is created.", but yes, I can be explicit here to.

Regarding the different name, I think the different signature is enough. Do you have a suggestion for a better name?

personally I'd call them something like beforeClientCreation and beforeClientCreationWithSession. (h/t @andybalaam who helped me here)

(nb you don't actually have to "tweak" the client builder in the methods -- you could just use them as an informative hook and return the builder untweaked. Hence, it feels like beforeX is a better name.)

But I really don't mind, it's very much up to you!

I have done the renaming in a817b6f

…entication.

…sion Since the login flows stopped handing their client over to RustMatrixClientFactory and let finalizeClientCreation() build a fresh one from the SessionData, two problems appeared. The client built by loginWithQrCode() was never owned by anyone: it is not assigned to currentClient, so clear() had nothing to close and the Rust Client stayed open for the whole app lifetime, holding the session stores of the freshly logged in account. It is now tracked in currentClient, and also disposed of when the QR login fails, which is safe because that flow always builds its own client. The client of the session was also built while the login client was still open. Both use the same session paths, so their state and crypto SQLite stores, and the search index when message search is enabled, were opened twice at the same time with no cross process lock to protect them. clear() now runs before the final client is created. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

sessionPaths keeps pointing at the directories of the account which has just been logged in, and it was never reset. Adding another account calls setHomeserver(), whose rotateSessionPath() starts by deleting the directories of the previous sessionPaths, so the file and cache directories of the first account were wiped. finalizeClientCreation() now forgets the session paths once the login succeeded. It is done there rather than in clear() on purpose: after a failed attempt the directories are orphaned and must still be deleted by the rotateSessionPath() of the next attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

sonarqubecloud · 2026-09-08T18:39:39Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ganfra

Ok for me like this. Have you tried using QRCode login? Is there some risks to lose data when closing the client quickly after login?

jmartinesp · 2026-09-09T09:50:28Z

Ok for me like this. Have you tried using QRCode login? Is there some risks to lose data when closing the client quickly after login?

AFAICT we haven't started any sync or encryption task at that point, so I think it should be ok.

ganfra · 2026-09-09T09:57:09Z

Ok for me like this. Have you tried using QRCode login? Is there some risks to lose data when closing the client quickly after login?

AFAICT we haven't started any sync or encryption task at that point, so I think it should be ok.

Is there not any verification thing done automatically when doing QR code login?

bmarty · 2026-09-09T10:37:29Z

Ok for me like this. Have you tried using QRCode login? Is there some risks to lose data when closing the client quickly after login?

I have checked again the QrCode login flow and it works fine, the session is created and verified.

bmarty added the PR-Misc For other changes label Aug 28, 2026

bmarty commented Aug 28, 2026

View reviewed changes

jmartinesp reviewed Aug 31, 2026

View reviewed changes

bmarty mentioned this pull request Aug 31, 2026

Remove ExternalSession, fallback login with Element Web is gone. #7608

Merged

richvdh mentioned this pull request Sep 2, 2026

Call ClientBuilderEnterpriseHook whenever we create a client #7590

Closed

12 tasks

bmarty force-pushed the bma/iterateOnClientCreation branch from 8462298 to e647698 Compare September 7, 2026 07:25

richvdh reviewed Sep 7, 2026

View reviewed changes

bmarty and others added 5 commits September 8, 2026 16:46

Ensure we create a fresh Client when the sessionId is know after auth…

5ce531f

…entication.

Iterate on hooks implementation

1f18a18

Rename methods and update documentation.

a817b6f

bmarty force-pushed the bma/iterateOnClientCreation branch from 4553b2d to a817b6f Compare September 8, 2026 15:10

Fix quality and tests.

3cde152

bmarty marked this pull request as ready for review September 9, 2026 07:58

bmarty requested a review from a team as a code owner September 9, 2026 07:58

bmarty requested review from ganfra and removed request for a team September 9, 2026 07:58

ganfra reviewed Sep 9, 2026

View reviewed changes

ganfra approved these changes Sep 9, 2026

View reviewed changes

Uh oh!

Conversation

bmarty commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 8, 2026

Quality Gate passed

Uh oh!

ganfra left a comment

Choose a reason for hiding this comment

Uh oh!

jmartinesp commented Sep 9, 2026

Uh oh!

ganfra commented Sep 9, 2026

Uh oh!

bmarty commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bmarty commented Aug 28, 2026 •

edited

Loading

github-actions Bot commented Aug 28, 2026 •

edited

Loading

codecov Bot commented Aug 28, 2026 •

edited

Loading

bmarty Aug 31, 2026 •

edited

Loading