Summary

MongoCredentialWithCache is in a detrimental state to put it mildly, but it seems like we should be able to improve things.

Description

There appear to be the three following possible approaches on how Authenticator may be designed (if one sees more, please add):

1. There is a single thread-safe Authenticator instance per MongoClient that owns state shared by all connections as well as connection-confined state. The state confined to each InternalConnection may be represented via an entry in ConcurrentHashMap where the key is (ConnectionDescription.getServiceId(), ConnectionDescription.getConnectionId()).
2. There is a single thread-safe Authenticator instance per MongoClient that owns state shared by all connections and accesses connection-confined state in its authenticate / reauthenticate methods via InternalConnection, which is passed as an argument.
3. [this one is used in the driver] There is a single thread-safe object that owns state shared by all connections, and there are multiple Authenticator instances, one per InternalConnection, that all have access to the shared state and own connection-confined state.

Problems with the current design

Allows for only one implementation of the thread-safe object that owns state shared by all connections

Approach #3 is used in the driver, with MongoCredentialWithCache being the object that owns state shared by all connections. Different Authenticator implementations need different state shared by all connections, yet the only currently possible implementation of that state is MongoCredentialWithCache. The implementation of OidcAuthenticator had to make the MongoCredentialWithCache code even weirder than it was: it had to add state and methods that are specific to OidcAuthenticator, but as a result are visible to all other implementations of Authenticator. Here is the corresponding PR discussion.

We can make Authenticator generic over the type of the state shared by all connections, split MongoCredentialWithCache into multiple classes that better fit the needs of different Authenticator implementations, and change InternalStreamConnectionFactory such that it gives its Authenticator implementation the shared state of the type needed by that implementation. This approach came up in both cases when I was independently discussing the issue with jeff.yemin@mongodb.com and maxim.katcharov@mongodb.com.

Breaks isolation by sharing the lock owned by MongoCredentialWithCache with all Authenticator instances

A situation when one object owns a lock, but has to share it with other objects, which use it explicitly, appears to violate the encapsulation principle and suggests that the object boundaries may have not been designed optimally. It seems to me that of the three approaches described above, only approach #3 results in this specific issue. I raised this concern when reviewing the OidcAuthenticator implementation. Here is the corresponding PR discussion.

I admit the possibility of other approaches either encountering unforeseen complications (this is relevant to approach #1), or also having to break OOP principles (this is relevant to approach #2) if tried to be implemented. I am not currently suggesting to change to approach from #3 to either #2 or #1. I have described this problem here and my thoughts on it just in case.