Skip to content

feat: Add ReadFrom parsing support to ConfigurationOptions for AZ Affinity #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 199 additions & 19 deletions sources/Valkey.Glide/Abstract/ConfigurationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@
Ssl = "ssl",
Version = "version",
SetClientLibrary = "setlib",
Protocol = "protocol"
Protocol = "protocol",
ReadFrom = "readFrom",
Az = "az"
;

private static readonly Dictionary<string, string> normalizedOptions = new[]
Expand All @@ -71,6 +73,8 @@
Version,
SetClientLibrary,
Protocol,
ReadFrom,
Az
}.ToDictionary(x => x, StringComparer.OrdinalIgnoreCase);

public static string TryNormalize(string value)
Expand All @@ -83,13 +87,13 @@
}
}

// Private fields
private bool? ssl;

private Proxy? proxy;

private RetryStrategy? reconnectRetryPolicy;

private ReadFrom? readFrom;
private string? tempAz; // Temporary storage for AZ during parsing
private ReadFromStrategy? tempReadFromStrategy; // Temporary storage for ReadFrom strategy during parsing
Comment on lines +95 to +96
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If those need for parsing only - create them in the parsing method.


/// <summary>
/// Gets or sets whether connect/configuration timeouts should be explicitly notified via a TimeoutException.
Expand Down Expand Up @@ -243,7 +247,14 @@
public ReadFrom? ReadFrom
{
get => readFrom;
set => readFrom = value;
set
{
if (value.HasValue)
{
ValidateReadFromConfiguration(value.Value);
}
readFrom = value;
}
}

/// <summary>
Expand Down Expand Up @@ -295,21 +306,28 @@
/// <summary>
/// Create a copy of the configuration.
/// </summary>
public ConfigurationOptions Clone() => new()
public ConfigurationOptions Clone()
{
ClientName = ClientName,
ConnectTimeout = ConnectTimeout,
User = User,
Password = Password,
ssl = ssl,
proxy = proxy,
ResponseTimeout = ResponseTimeout,
DefaultDatabase = DefaultDatabase,
reconnectRetryPolicy = reconnectRetryPolicy,
readFrom = readFrom,
EndPoints = EndPoints.Clone(),
Protocol = Protocol,
};
var cloned = new ConfigurationOptions

Check failure on line 311 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Object initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0017)

Check failure on line 311 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Use explicit type instead of 'var' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0008)

Check failure on line 311 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Object initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0017)

Check failure on line 311 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Use explicit type instead of 'var' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0008)
{
ClientName = ClientName,
ConnectTimeout = ConnectTimeout,
User = User,
Password = Password,
ssl = ssl,
proxy = proxy,
ResponseTimeout = ResponseTimeout,
DefaultDatabase = DefaultDatabase,
reconnectRetryPolicy = reconnectRetryPolicy,
EndPoints = EndPoints.Clone(),
Protocol = Protocol,
};

// Use property setter to ensure validation
cloned.ReadFrom = readFrom;

Check failure on line 327 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Object initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0017)

Check failure on line 327 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Object initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0017)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to set as other fields set?


return cloned;
}

/// <summary>
/// Apply settings to configure this instance of <see cref="ConfigurationOptions"/>, e.g. for a specific scenario.
Expand Down Expand Up @@ -356,6 +374,10 @@
Append(sb, OptionKeys.ResponseTimeout, ResponseTimeout);
Append(sb, OptionKeys.DefaultDatabase, DefaultDatabase);
Append(sb, OptionKeys.Protocol, FormatProtocol(Protocol));
if (readFrom.HasValue)
{
FormatReadFrom(sb, readFrom.Value);
}

return sb.ToString();

Expand Down Expand Up @@ -403,6 +425,8 @@
ssl = null;
readFrom = null;
reconnectRetryPolicy = null;
tempAz = null;
tempReadFromStrategy = null;
EndPoints.Clear();
}

Expand Down Expand Up @@ -463,6 +487,12 @@
case OptionKeys.ResponseTimeout:
ResponseTimeout = OptionKeys.ParseInt32(key, value);
break;
case OptionKeys.ReadFrom:
ParseReadFromParameter(key, value);
break;
case OptionKeys.Az:
ParseAzParameter(key, value);
break;
default:
if (!ignoreUnknown) throw new ArgumentException($"Keyword '{key}' is not supported.", key);
break;
Expand All @@ -476,9 +506,159 @@
}
}
}

// Validate ReadFrom configuration after all parameters have been parsed
if (tempReadFromStrategy.HasValue)
{
ValidateAndSetReadFrom();
}

return this;
}

private void ParseReadFromParameter(string key, string value) => tempReadFromStrategy = ParseReadFromStrategy(key, value);// Don't validate immediately - wait until all parsing is complete

private void ParseAzParameter(string key, string value)
{
if (string.IsNullOrWhiteSpace(value))
{
throw new ArgumentException("Availability zone cannot be empty or whitespace", key);
}
tempAz = value;
// Don't validate immediately - wait until all parsing is complete
}

private void ValidateAndSetReadFrom()
{
if (tempReadFromStrategy.HasValue)
{
var strategy = tempReadFromStrategy.Value;

Check failure on line 535 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Use explicit type instead of 'var' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0008)

Check failure on line 535 in sources/Valkey.Glide/Abstract/ConfigurationOptions.cs

View workflow job for this annotation

GitHub Actions / Analyze

Use explicit type instead of 'var' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0008)

// Validate strategy and AZ combinations
switch (strategy)
{
case ReadFromStrategy.AzAffinity:
if (tempAz == null)
{
throw new ArgumentException("Availability zone should be set when using AzAffinity strategy");
}
if (string.IsNullOrWhiteSpace(tempAz))
{
throw new ArgumentException("Availability zone cannot be empty or whitespace when using AzAffinity strategy");
}
readFrom = new ReadFrom(strategy, tempAz);
break;

case ReadFromStrategy.AzAffinityReplicasAndPrimary:
if (tempAz == null)
{
throw new ArgumentException("Availability zone should be set when using AzAffinityReplicasAndPrimary strategy");
}
if (string.IsNullOrWhiteSpace(tempAz))
{
throw new ArgumentException("Availability zone cannot be empty or whitespace when using AzAffinityReplicasAndPrimary strategy");
}
readFrom = new ReadFrom(strategy, tempAz);
break;

case ReadFromStrategy.Primary:
if (!string.IsNullOrWhiteSpace(tempAz))
{
throw new ArgumentException("Availability zone should not be set when using Primary strategy");
}
readFrom = new ReadFrom(strategy);
break;

case ReadFromStrategy.PreferReplica:
if (!string.IsNullOrWhiteSpace(tempAz))
{
throw new ArgumentException("Availability zone should not be set when using PreferReplica strategy");
}
readFrom = new ReadFrom(strategy);
break;

default:
throw new ArgumentException($"ReadFrom strategy '{strategy}' is not supported. Valid strategies are: Primary, PreferReplica, AzAffinity, AzAffinityReplicasAndPrimary");
}
}
}

private static ReadFromStrategy ParseReadFromStrategy(string key, string value)
{
if (string.IsNullOrWhiteSpace(value))
{
throw new ArgumentException($"Keyword '{key}' requires a ReadFrom strategy value; the value cannot be empty", key);
}

try
{
return Enum.Parse<ReadFromStrategy>(value, ignoreCase: true);
}
catch (ArgumentException)
{
throw new ArgumentException($"ReadFrom strategy '{value}' is not supported. Valid strategies are: Primary, PreferReplica, AzAffinity, AzAffinityReplicasAndPrimary", key);
}
}

private static void ValidateReadFromConfiguration(ReadFrom readFromConfig)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method repeats ValidateAndSetReadFrom. Can you remove duplicates or re-use the code?

{
switch (readFromConfig.Strategy)
{
case ReadFromStrategy.AzAffinity:
if (readFromConfig.Az == null)
{
throw new ArgumentException("Availability zone should be set when using AzAffinity strategy");
}
if (string.IsNullOrWhiteSpace(readFromConfig.Az))
{
throw new ArgumentException("Availability zone cannot be empty or whitespace");
}
break;

case ReadFromStrategy.AzAffinityReplicasAndPrimary:
if (readFromConfig.Az == null)
{
throw new ArgumentException("Availability zone should be set when using AzAffinityReplicasAndPrimary strategy");
}
if (string.IsNullOrWhiteSpace(readFromConfig.Az))
{
throw new ArgumentException("Availability zone cannot be empty or whitespace");
}
break;

case ReadFromStrategy.Primary:
if (!string.IsNullOrWhiteSpace(readFromConfig.Az))
{
throw new ArgumentException("Availability zone should not be set when using Primary strategy");
}
break;

case ReadFromStrategy.PreferReplica:
if (!string.IsNullOrWhiteSpace(readFromConfig.Az))
{
throw new ArgumentException("Availability zone should not be set when using PreferReplica strategy");
}
break;

default:
throw new ArgumentOutOfRangeException(nameof(readFromConfig), $"ReadFrom strategy '{readFromConfig.Strategy}' is not supported");
}
}

/// <summary>
/// Formats a ReadFrom struct to its string representation and appends it to the StringBuilder.
/// </summary>
/// <param name="sb">The StringBuilder to append to.</param>
/// <param name="readFromConfig">The ReadFrom configuration to format.</param>
private static void FormatReadFrom(StringBuilder sb, ReadFrom readFromConfig)
{
Append(sb, OptionKeys.ReadFrom, readFromConfig.Strategy.ToString());
if (!string.IsNullOrWhiteSpace(readFromConfig.Az))
{
Append(sb, OptionKeys.Az, readFromConfig.Az);
}
}

/// <summary>
/// Specify the connection protocol type.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion sources/Valkey.Glide/Abstract/ConnectionMultiplexer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private ConnectionMultiplexer(ConfigurationOptions configuration, Database db)
_db = db;
}

private static T CreateClientConfigBuilder<T>(ConfigurationOptions configuration)
internal static T CreateClientConfigBuilder<T>(ConfigurationOptions configuration)
where T : ClientConfigurationBuilder<T>, new()
{
T config = new();
Expand Down
Loading
Loading