Skip to content

Commit

Permalink
address todo's
Browse files Browse the repository at this point in the history
  • Loading branch information
kellyyangsong committed Jun 20, 2024
1 parent 2a00a27 commit 2bb103b
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class ConfigurationManager<T> : BaseConfigurationManager, IConfigurationM
private DateTimeOffset _lastRefresh = DateTimeOffset.MinValue;
private bool _isFirstRefreshRequest = true;
private bool _skipDistributedConfigurationManager = false;
private bool _distributedCacheUpdateInProgress = false;

private readonly SemaphoreSlim _refreshLock;
private readonly IDocumentRetriever _docRetriever;
Expand Down Expand Up @@ -166,26 +167,33 @@ public async Task<T> GetConfigurationAsync(CancellationToken cancel)
T configuration = null;
if (DistributedConfigurationManager != null && !_skipDistributedConfigurationManager)
{
// TODO handle try/catch
configuration = await DistributedConfigurationManager.GetConfigurationAsync(MetadataAddress, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false);
if (_configValidator != null)
try
{
// TODO don't throw but log another exception
ConfigurationValidationResult result = _configValidator.Validate(configuration);
if (!result.Succeeded)
configuration = await DistributedConfigurationManager.GetConfigurationAsync(MetadataAddress, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false);
if (_configValidator != null)
{
configuration = null;
LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20810, result.ErrorMessage)));
ConfigurationValidationResult result = _configValidator.Validate(configuration);
if (!result.Succeeded)
{
configuration = null;
LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20812, result.ErrorMessage)));
}
}
}
catch (Exception ex)
{
LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(LogMessages.IDX20811), ex));
}
}

if (configuration == null)
{
LogHelper.LogInformation(LogMessages.IDX20811);

// Don't use the individual CT here, this is a shared operation that shouldn't be affected by an individual's cancellation.
// The transport should have it's own timeouts, etc..
configuration = await _configRetriever.GetConfigurationAsync(MetadataAddress, _docRetriever, CancellationToken.None).ConfigureAwait(false);
if (configuration is IConfigurationRetrievalTime configRetrievalTime)
if (configuration is IConfigurationTimeRetriever configRetrievalTime)
configRetrievalTime.RetrievalTime = DateTimeOffset.UtcNow;

if (_configValidator != null)
Expand All @@ -198,11 +206,16 @@ public async Task<T> GetConfigurationAsync(CancellationToken cancel)
if (_skipDistributedConfigurationManager)
_skipDistributedConfigurationManager = false;

if (DistributedConfigurationManager != null)
// TODO fire and forget (or not if refresh happens on a background thread)
await DistributedConfigurationManager.SetConfigurationAsync(MetadataAddress, configuration, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false);
if (DistributedConfigurationManager != null && !_distributedCacheUpdateInProgress)
{
_distributedCacheUpdateInProgress = true;
_ = DistributedConfigurationManager.SetConfigurationAsync(MetadataAddress, configuration, s_distributedConfigurationOptions, CancellationToken.None).ContinueWith(t =>
{
_distributedCacheUpdateInProgress = false;
}, TaskScheduler.Default);
}

if (configuration is IConfigurationRetrievalTime configurationRetrievalTime)
if (configuration is IConfigurationTimeRetriever configurationRetrievalTime)
_lastRefresh = configurationRetrievalTime.RetrievalTime;
else
_lastRefresh = DateTimeOffset.UtcNow;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
[assembly: SuppressMessage("Performance", "CA1819:Properties should not return arrays", Justification = "Previously released as returning an array", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.Body")]
[assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Previously released read/write", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.Headers")]
[assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Previously released read/write", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.PropertyBag")]
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Any exception type can be thrown by custom distributed configuration manager.", Scope = "member", Target = "~M:Microsoft.IdentityModel.Protocols.ConfigurationManager`1.GetConfigurationAsync(System.Threading.CancellationToken)~System.Threading.Tasks.Task{`0}")]
#if NET6_0_OR_GREATER
[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Adding StringComparison.Ordinal adds a performance penalty.", Scope = "member", Target = "~M:Microsoft.IdentityModel.Protocols.AuthenticationProtocolMessage.BuildRedirectUrl~System.String")]
#endif
2 changes: 2 additions & 0 deletions src/Microsoft.IdentityModel.Protocols/LogMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ internal static class LogMessages
internal const string IDX20808 = "IDX20808: Network error occurred. Status code: '{0}'. \nResponse content: '{1}'. \nAttempting to retrieve document again from: '{2}'.";
internal const string IDX20809 = "IDX20809: Unable to retrieve document from: '{0}'. Status code: '{1}'. \nResponse content: '{2}'.";
internal const string IDX20810 = "IDX20810: Configuration validation failed, see inner exception for more details. Exception: '{0}'.";
internal const string IDX20811 = "IDX20811: Unable to obtain configuration from distributed cache.";
internal const string IDX20812 = "IDX20812: Configuration retrieved from distributed cache validation failed, see inner exception for more details. Exception: '{0}'.";

#pragma warning restore 1591
}
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Tokens/BaseConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Microsoft.IdentityModel.Tokens
/// <summary>
/// Represents a generic metadata configuration which is applicable for both XML and JSON based configurations.
/// </summary>
public abstract class BaseConfiguration /*: IConfigurationRetrievalTime*/ // L2 TODO: internal until L2 cache is implemented S2S.
public abstract class BaseConfiguration /*: IConfigurationTimeRetriever*/ // L2 TODO: internal until L2 cache is implemented S2S.
{
/// <summary>
/// Gets the issuer specified via the metadata endpoint.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.IdentityModel.Tokens
///
/// </summary>
// L2 TODO: internal until L2 cache is implemented S2S.
internal interface IConfigurationRetrievalTime
internal interface IConfigurationTimeRetriever
{
// L2 TODO: internal until L2 cache is implemented S2S.
internal DateTimeOffset RetrievalTime { get; set; }
Expand Down

0 comments on commit 2bb103b

Please sign in to comment.