SpannerConnectionStringBuilder.ConversionOptions are set inconsistently

[jskeet]

When SpannerConnection.ConnectionString is set, it calls the SpannerConnectionStringBuilder(string, ChannelCredential, SessionPoolManager) constructor... whereas the Clone() method explicitly copies ConversionOptions from the source.

It's unclear whether setting SpannerConnectionStringBuilder.ConnectionString uses the indexer to set each value (in which case we don't need to copy it in Clone) or whether it's lost in the SpannerConnectionStringBuilder(string, ChannelCredential, SessionPoolManager) constructor.

We should be consistent and document the reasoning.

[jskeet]

Looking at this again, we have a slight issue when setting ConnectionString, if the previous connection string specified conversion options but the new one doesn't. Here's a failing test:

[Fact]
public void SettingConnectionStringResetsConversionOptions()
{
    var builder = new SpannerConnectionStringBuilder();
    builder.ConnectionString = "SpannerToClrTypeDefaultMappings=DateToSpannerDate";
    builder.ConnectionString = "";
    Assert.Equal(SpannerConversionOptions.Default.DateToConfiguredClrType, builder.ConversionOptions.DateToConfiguredClrType);
}

Basically I suspect we want to create conversion options lazily, rather than responding to keys being set explicitly.

Will fix this.

[jskeet]

Ah - it's actually fairly straightforward; we just need to override Clear(), I think. That is always called when the ConnectionString property is set.

[Rishabh-V]

Hi @jskeet - This seems to have been introduced by me. I can take a look at it and make the changes after discussing it with you tomorrow. Please let me know.

[jskeet]

I've fixed it in a PR that I'll be sending later :)