ClientOptions can't be changed after CosmosClient is created, despite settable members

aaronpowell commented 2 years ago

I'm working on creating a devcontainer that loads the Docker-based emulator side-by-side with the devcontainer to do development against.

Since the emulator is not accessible via localhost (it's cosmos in my case), the certificate provided isn't valid, and as a result, I need to disable SSL validation. The repo for it can be found here: https://github.com/aaronpowell/FSharp.CosmosDb/tree/emulator-integration

I don't control the creation of the CosmosClient, and thus the CosmosClientOptions, meaning I want to use the provided CosmosClient and update the client.ClientOptions.HttpClientFactory to use my own custom one that disables SSL validation, as demonstrated here: https://github.com/Azure/azure-cosmos-dotnet-v3/blob/3895cd4f63e8e4e5da026a5c22b98c908d21855b/Microsoft.Azure.Cosmos.Samples/Usage/HttpClientFactory/Program.cs#L70

Here's my implementation but when it runs I still get an error that the SSL certificate is invalid and debugging shows that I never hit the HttpClientFactory that I provided.

It appears that, despite the HttpClientFactory being a settable member, setting it after the creation of the CosmosClient has no effect.

I explore this further I created the following test:

        [TestMethod]
        public async Task ChangeOptionsReflectedOnUsage()
        {
            CosmosClientOptions options = new();

            CosmosClient client = new(ConnectionString, options);

            bool factoryCalled = false;
            client.ClientOptions.HttpClientFactory = () =>
            {
                factoryCalled = true;
                return new HttpClient();
            };

            try
            {
                await client.CreateDatabaseAsync("test");
            }
            catch (Exception)
            {
                // test failing isn't important
            }

            Assert.IsTrue(factoryCalled, "Should have called the HttpClientFactory method and set the flag.");
        }

This test fails because factoryCalled is never set to true:

ChangeOptionsReflectedOnUsage
   Source: CosmosClientTests.cs line 461
   Duration: 1.5 sec

  Message: 
    Assert.IsTrue failed. Should have called the HttpClientFactory method and set the flag.

  Stack Trace: 
    CosmosClientTests.ChangeOptionsReflectedOnUsage() line 483
    ThreadOperations.ExecuteWithAbortSafety(Action action)

Expected

Changing settable properties on the CosmosClientOptions should be reflected at execution time.

aaronpowell commented 2 years ago

Here's a branch that has the test in it: https://github.com/aaronpowell/azure-cosmos-dotnet-v3/tree/aaronpowell/issue-3343

ealsur commented 2 years ago

This is by design. Once built the ClientOptions are readonly because they are cloned. All the configuration that needs to be set should be passed to the constructor and not modified afterwards. Modifications after a client is created can lead to unexpected behavior.

Duplicate of https://github.com/Azure/azure-cosmos-dotnet-v3/issues/1008

And making the APIs read-only is something that cannot be done in the current version because it would be a breaking change. Hence it is tracked for the next major version: https://github.com/Azure/azure-cosmos-dotnet-v3/issues/1025

Azure / azure-cosmos-dotnet-v3

ClientOptions can't be changed after CosmosClient is created, despite settable members #3343

Expected