Call ClientBuilderEnterpriseHook whenever we create a client - #7590
Call ClientBuilderEnterpriseHook whenever we create a client#7590richvdh wants to merge 3 commits into
Conversation
Make sure that the `ClientBuilderEnterpriseHook` is called on every ClientBuilder we create, even if we don't yet have a user ID.
a67c76c to
82d97f5
Compare
| * [io.element.android.libraries.matrix.impl.auth.RustMatrixAuthenticationService] after successful | ||
| * authentication. | ||
| * | ||
| * TODO: feels like this could do with a better name, since it doesn't actually create a client. |
There was a problem hiding this comment.
The create methods do not create a Client, but they create a RustMatrixClient. The methods could return MatrixClient though.
There was a problem hiding this comment.
It feels strange that we have two different methods called create with very different behaviours. I feel like it would be much clearer if this was called wrapClient or something of that nature.
bmarty
left a comment
bmarty
left a comment
There was a problem hiding this comment.
I think that the changes make sense. It's quite complex code, since we are using the same method from the SDK to either create a client to authenticate, or create a client to restore an existing session.
Maybe we could revisit the code and let the client used to authenticate be destroyed, and always create a fresh once the user has just logged in, as if a session has been restored.
…lder_enterprise_hook
…lder_enterprise_hook
|
#7591 works for us. |
2 participants
Currently, the
ClientBuilderEnterpriseHookis only called when we restore a previous session, rather than when performing a new login.I'm putting this up for discussion rather than in the expectation it will land as-is. I don't really think it's the right solution, because it means that the session ID is not set for clients used for a login (and hence the functionality in the Enterprise hook which depends on that does not work correctly).
However, since we're trying to create the client before we know the user ID, it's hard to see what would be better.
Content
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist