Skip to content
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

Remove ResolverException in favor of RequestFailedException #18874

Merged
merged 2 commits into from
Feb 19, 2021
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@

namespace Azure.Iot.ModelsRepository
{
/// <summary>
/// DtmiConventions implements the core aspects of the IoT model repo conventions
/// which includes DTMI validation and calculating a URI path from a DTMI.
/// </summary>
internal static class DtmiConventions
{
private static readonly Regex s_validDtmiRegex = new Regex(@"^dtmi:[A-Za-z](?:[A-Za-z0-9_]*[A-Za-z0-9])?(?::[A-Za-z](?:[A-Za-z0-9_]*[A-Za-z0-9])?)*;[1-9][0-9]{0,8}$");
Copy link
Contributor

Choose a reason for hiding this comment

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

Independent of this PR, can we get a comment on this Regex that describes how it works. The universal truth about regex is that the author knows what it does, but it can appear undecipherable to others. :'-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@

namespace Azure.Iot.ModelsRepository.Fetchers
{
/// <summary>
/// The FetchResult class has the purpose of containing key elements of
/// an IModelFetcher Fetch() operation including model definition, path and whether
/// it was from an expanded (pre-calculated) fetch.
/// </summary>
internal class FetchResult
{
public string Definition { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@

namespace Azure.Iot.ModelsRepository.Fetchers
{
/// <summary>
/// The IModelFetcher is an abstraction that supports fetching
/// model content via a particular protocol or mechanism of interaction.
/// </summary>
internal interface IModelFetcher
{
Task<FetchResult> FetchAsync(string dtmi, Uri repositoryUri, CancellationToken cancellationToken = default);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

namespace Azure.Iot.ModelsRepository.Fetchers
{
/// <summary>
/// The LocalModelFetcher is an implementation of IModelFetcher
/// for supporting local filesystem based model content fetching.
/// </summary>
internal class LocalModelFetcher : IModelFetcher
{
private readonly bool _tryExpanded;
Expand Down Expand Up @@ -65,7 +69,9 @@ public FetchResult Fetch(string dtmi, Uri repositoryUri, CancellationToken cance
fnfError = string.Format(CultureInfo.CurrentCulture, ServiceStrings.ErrorFetchingModelContent, tryContentPath);
}

throw new FileNotFoundException(fnfError);
throw new RequestFailedException(
$"{string.Format(CultureInfo.CurrentCulture, ServiceStrings.GenericResolverError, dtmi)} {fnfError}",
drwill-ms marked this conversation as resolved.
Show resolved Hide resolved
new FileNotFoundException(fnfError));
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

namespace Azure.Iot.ModelsRepository.Fetchers
{
/// <summary>
/// The RemoteModelFetcher is an implementation of IModelFetcher
/// for supporting http[s] based model content fetching.
drwill-ms marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
internal class RemoteModelFetcher : IModelFetcher
{
private readonly HttpPipeline _pipeline;
Expand Down Expand Up @@ -54,7 +58,9 @@ public FetchResult Fetch(string dtmi, Uri repositoryUri, CancellationToken cance
}
catch (Exception)
{
remoteFetchError = string.Format(CultureInfo.CurrentCulture, StandardStrings.ErrorFetchingModelContent, tryContentPath);
remoteFetchError =
$"{string.Format(CultureInfo.CurrentCulture, ServiceStrings.GenericResolverError, dtmi)} " +
string.Format(CultureInfo.CurrentCulture, StandardStrings.ErrorFetchingModelContent, tryContentPath);
}
}

Expand Down Expand Up @@ -95,7 +101,9 @@ public async Task<FetchResult> FetchAsync(string dtmi, Uri repositoryUri, Cancel
}
catch (Exception)
{
remoteFetchError = string.Format(CultureInfo.CurrentCulture, StandardStrings.ErrorFetchingModelContent, tryContentPath);
remoteFetchError =
$"{string.Format(CultureInfo.CurrentCulture, ServiceStrings.GenericResolverError, dtmi)} " +
string.Format(CultureInfo.CurrentCulture, StandardStrings.ErrorFetchingModelContent, tryContentPath);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Azure.Iot.ModelsRepository
{
/// <summary>
/// ModelMetadata is designated to store KPIs from model parsing.
/// </summary>
internal class ModelMetadata
{
public ModelMetadata(string id, IList<string> extends, IList<string> componentSchemas)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
namespace Azure.Iot.ModelsRepository
{
/// <summary>
/// The <c>ModelQuery</c> class is responsible for parsing DTDL v2 models to produce key metadata.
/// In the current form <c>ModelQuery</c> is focused on determining model dependencies recursively
/// The ModelQuery class is responsible for parsing DTDL v2 models to produce key metadata.
/// In the current form ModelQuery is focused on determining model dependencies recursively
/// via extends and component schemas.
/// </summary>
internal class ModelQuery
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ internal static class ModelRepositoryConstants
public const string ExpandedJsonFileExtension = ".expanded.json";

/// <summary>
/// The <c>ModelRepositoryConstants.ModelProperties</c> class contains DTDL v2 property names and property values
/// used by the <c>ModelQuery</c> class to parse DTDL model key indicators.
/// The ModelRepositoryConstants.ModelProperties class contains DTDL v2 property names and property values
/// used by the ModelQuery class to parse DTDL model key indicators.
/// </summary>
internal static class ModelProperties
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,69 +12,69 @@
namespace Azure.Iot.ModelsRepository
{
/// <summary>
/// The <c>ModelsRepoClient</c> class supports DTDL model resolution providing functionality to
/// resolve models by retrieving model definitions and their dependencies.
/// The ModelsRepoClient class supports operations against DTDL model repositories following the
/// conventions of the Azure IoT Plug and Play Models repository.
/// </summary>
public class ModelsRepoClient
{
private readonly RepositoryHandler _repositoryHandler;
private readonly ClientDiagnostics _clientDiagnostics;

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with default client options while pointing to
/// the Azure IoT Plug and Play Model repository https://devicemodels.azure.com for resolution.
/// Initializes the ModelsRepoClient with default client options while pointing to
/// the Azure IoT Plug and Play Models repository https://devicemodels.azure.com for resolution.
/// </summary>
public ModelsRepoClient() : this(new Uri(DefaultModelsRepository), new ModelsRepoClientOptions()) { }

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with default client options while pointing to
/// Initializes the ModelsRepoClient with default client options while pointing to
/// a custom <paramref name="repositoryUri"/> for resolution.
/// </summary>
/// <param name="repositoryUri">
/// The model repository <c>Uri</c> value. This can be a remote endpoint or local directory.
/// The model repository Uri value. This can be a remote endpoint or local directory.
/// </param>
public ModelsRepoClient(Uri repositoryUri) : this(repositoryUri, new ModelsRepoClientOptions()) { }

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with custom client <paramref name="options"/> while pointing to
/// Initializes the ModelsRepoClient with custom client <paramref name="options"/> while pointing to
/// the Azure IoT Plug and Play Model repository https://devicemodels.azure.com for resolution.
/// </summary>
/// <param name="options">
/// <c>ModelsRepoClientOptions</c> to configure resolution and client behavior.
/// ModelsRepoClientOptions to configure resolution and client behavior.
/// </param>
public ModelsRepoClient(ModelsRepoClientOptions options) : this(new Uri(DefaultModelsRepository), options) { }

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with default client options while pointing to
/// Initializes the ModelsRepoClient with default client options while pointing to
/// a custom <paramref name="repositoryUriStr"/> for resolution.
/// </summary>
/// <param name="repositoryUriStr">
/// The model repository <c>Uri</c> in string format. This can be a remote endpoint or local directory.
/// The model repository Uri in string format. This can be a remote endpoint or local directory.
/// </param>
public ModelsRepoClient(string repositoryUriStr) : this(repositoryUriStr, new ModelsRepoClientOptions()) { }

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with custom client <paramref name="options"/> while pointing to
/// Initializes the ModelsRepoClient with custom client <paramref name="options"/> while pointing to
/// a custom <paramref name="repositoryUriStr"/> for resolution.
/// </summary>
/// <param name="repositoryUriStr">
/// The model repository <c>Uri</c> in string format. This can be a remote endpoint or local directory.
/// The model repository Uri in string format. This can be a remote endpoint or local directory.
/// </param>
/// <param name="options">
/// <c>ModelsRepoClientOptions</c> to configure resolution and client behavior.
/// ModelsRepoClientOptions to configure resolution and client behavior.
/// </param>
public ModelsRepoClient(string repositoryUriStr, ModelsRepoClientOptions options)
: this(new Uri(repositoryUriStr), options) { }

/// <summary>
/// Initializes the <c>ModelsRepoClient</c> with custom client <paramref name="options"/> while pointing to
/// Initializes the ModelsRepoClient with custom client <paramref name="options"/> while pointing to
/// a custom <paramref name="repositoryUri"/> for resolution.
/// </summary>
/// <param name="repositoryUri">
/// The model repository <c>Uri</c>. This can be a remote endpoint or local directory.
/// The model repository Uri. This can be a remote endpoint or local directory.
/// </param>
/// <param name="options">
/// <c>ModelsRepoClientOptions</c> to configure resolution and client behavior.
/// ModelsRepoClientOptions to configure resolution and client behavior.
/// </param>
public ModelsRepoClient(Uri repositoryUri, ModelsRepoClientOptions options)
{
Expand All @@ -90,10 +90,10 @@ public ModelsRepoClient(Uri repositoryUri, ModelsRepoClientOptions options)
/// Resolves a model definition identified by <paramref name="dtmi"/> and optionally its dependencies.
/// </summary>
/// <returns>
/// An <c>IDictionary</c> containing the model definition(s) where the key is the dtmi
/// An IDictionary containing the model definition(s) where the key is the dtmi
/// and the value is the raw model definition string.
/// </returns>
/// <exception cref="ResolverException">Thrown when a resolution failure occurs.</exception>
/// <exception cref="RequestFailedException">Thrown when a resolution failure occurs.</exception>
/// <param name="dtmi">A well-formed DTDL model Id. For example 'dtmi:com:example:Thermostat;1'.</param>
/// <param name="cancellationToken">The cancellationToken.</param>
[SuppressMessage(
Expand All @@ -119,10 +119,10 @@ public virtual async Task<IDictionary<string, string>> ResolveAsync(string dtmi,
/// Resolves a model definition identified by <paramref name="dtmi"/> and optionally its dependencies.
/// </summary>
/// <returns>
/// An <c>IDictionary</c> containing the model definition(s) where the key is the dtmi
/// An IDictionary containing the model definition(s) where the key is the dtmi
/// and the value is the raw model definition string.
/// </returns>
/// <exception cref="ResolverException">Thrown when a resolution failure occurs.</exception>
/// <exception cref="RequestFailedException">Thrown when a resolution failure occurs.</exception>
/// <param name="dtmi">A well-formed DTDL model Id. For example 'dtmi:com:example:Thermostat;1'.</param>
/// <param name="cancellationToken">The cancellationToken.</param>
[SuppressMessage(
Expand All @@ -149,10 +149,10 @@ public virtual IDictionary<string, string> Resolve(string dtmi, CancellationToke
/// Resolves a collection of model definitions identified by <paramref name="dtmis"/> and optionally their dependencies.
/// </summary>
/// <returns>
/// An <c>IDictionary</c> containing the model definition(s) where the key is the dtmi
/// An IDictionary containing the model definition(s) where the key is the dtmi
/// and the value is the raw model definition string.
/// </returns>
/// <exception cref="ResolverException">Thrown when a resolution failure occurs.</exception>
/// <exception cref="RequestFailedException">Thrown when a resolution failure occurs.</exception>
/// <param name="dtmis">A collection of well-formed DTDL model Ids.</param>
/// <param name="cancellationToken">The cancellationToken.</param>
[SuppressMessage("Usage", "AZC0015:Unexpected client method return type.", Justification = "<Pending>")]
Expand All @@ -176,10 +176,10 @@ public virtual async Task<IDictionary<string, string>> ResolveAsync(IEnumerable<
/// Resolves a collection of model definitions identified by <paramref name="dtmis"/> and optionally their dependencies.
/// </summary>
/// <returns>
/// An <c>IDictionary</c> containing the model definition(s) where the key is the dtmi
/// An IDictionary containing the model definition(s) where the key is the dtmi
/// and the value is the raw model definition string.
/// </returns>
/// <exception cref="ResolverException">Thrown when a resolution failure occurs.</exception>
/// <exception cref="RequestFailedException">Thrown when a resolution failure occurs.</exception>
/// <param name="dtmis">A collection of well-formed DTDL model Ids.</param>
/// <param name="cancellationToken">The cancellationToken.</param>
[SuppressMessage("Usage", "AZC0015:Unexpected client method return type.", Justification = "<Pending>")]
Expand All @@ -205,17 +205,17 @@ public virtual IDictionary<string, string> Resolve(IEnumerable<string> dtmis, Ca
public static bool IsValidDtmi(string dtmi) => DtmiConventions.IsDtmi(dtmi);

/// <summary>
/// Gets the <c>Uri</c> associated with the ModelsRepoClient instance.
/// Gets the Uri associated with the ModelsRepoClient instance.
/// </summary>
public Uri RepositoryUri { get; }

/// <summary>
/// Gets the <c>ModelsRepoClientOptions</c> associated with the ModelsRepoClient instance.
/// Gets the ModelsRepoClientOptions associated with the ModelsRepoClient instance.
/// </summary>
public ModelsRepoClientOptions ClientOptions { get; }

/// <summary>
/// Azure Device Models Repository used by default.
/// The global Azure IoT Models Repository endpoint used by default.
/// </summary>
public static string DefaultModelsRepository => ModelRepositoryConstants.DefaultModelsRepository;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,11 @@ private async Task<IDictionary<string, string>> ProcessAsync(IEnumerable<string>
if (!parsedDtmi.Equals(targetDtmi, StringComparison.Ordinal))
{
ModelsRepoEventSource.Instance.IncorrectDtmiCasing(targetDtmi, parsedDtmi);
string formatErrorMsg = string.Format(CultureInfo.CurrentCulture, ServiceStrings.IncorrectDtmiCasing, targetDtmi, parsedDtmi);
throw new ResolverException(targetDtmi, formatErrorMsg, new FormatException(formatErrorMsg));
string formatErrorMsg =
$"{string.Format(CultureInfo.CurrentCulture, ServiceStrings.GenericResolverError, targetDtmi)} " +
string.Format(CultureInfo.CurrentCulture, ServiceStrings.IncorrectDtmiCasing, targetDtmi, parsedDtmi);

throw new RequestFailedException(formatErrorMsg, new FormatException(formatErrorMsg));
}

processedModels.Add(targetDtmi, result.Definition);
Expand All @@ -124,28 +127,14 @@ private async Task<IDictionary<string, string>> ProcessAsync(IEnumerable<string>
return processedModels;
}

private async Task<FetchResult> FetchAsync(string dtmi, CancellationToken cancellationToken)
private Task<FetchResult> FetchAsync(string dtmi, CancellationToken cancellationToken)
{
try
{
return await _modelFetcher.FetchAsync(dtmi, _repositoryUri, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
throw new ResolverException(dtmi, ex.Message, ex);
}
return _modelFetcher.FetchAsync(dtmi, _repositoryUri, cancellationToken);
}

private FetchResult Fetch(string dtmi, CancellationToken cancellationToken)
{
try
{
return _modelFetcher.Fetch(dtmi, _repositoryUri, cancellationToken);
}
catch (Exception ex)
{
throw new ResolverException(dtmi, ex.Message, ex);
}
return _modelFetcher.Fetch(dtmi, _repositoryUri, cancellationToken);
}

private static Queue<string> PrepareWork(IEnumerable<string> dtmis)
Expand All @@ -156,8 +145,12 @@ private static Queue<string> PrepareWork(IEnumerable<string> dtmis)
if (!DtmiConventions.IsDtmi(dtmi))
{
ModelsRepoEventSource.Instance.InvalidDtmiInput(dtmi);
string invalidArgMsg = string.Format(CultureInfo.CurrentCulture, ServiceStrings.InvalidDtmiFormat, dtmi);
throw new ResolverException(dtmi, invalidArgMsg, new ArgumentException(invalidArgMsg));

string invalidArgMsg =
$"{string.Format(CultureInfo.CurrentCulture, ServiceStrings.GenericResolverError, dtmi)} " +
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at this point, you might as well not use string interpolation. We're already concatenating the second string for line break reasons. The only thing we're adding in is the space. May as well just concat all 3.

string.Format(CultureInfo.CurrentCulture, ServiceStrings.InvalidDtmiFormat, dtmi);

throw new RequestFailedException(invalidArgMsg, new ArgumentException(invalidArgMsg));
}

toProcessModels.Enqueue(dtmi);
Expand Down
Loading