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.

Sign up for GitHub

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

Jump to bottom

Add async methods to the ResolverClient #18814

Merged
merged 18 commits into from
Feb 17, 2021
Merged

Add async methods to the ResolverClient #18814

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
067cdcd
Changes to fix issues.
azabbasi Feb 13, 2021
bfed035
Update RepositoryHandler.cs
azabbasi Feb 16, 2021
04ce075
Update CultureInfo to use CurrentCulture
azabbasi Feb 16, 2021
651835b
Formatting updates
azabbasi Feb 16, 2021
5aec2b6
Add ClientDiagnostics - Add sync methods.
azabbasi Feb 16, 2021
7ce0464
Merge branch 'feature/modelsRepo' into azabbasi/RemoteFetcher
azabbasi Feb 16, 2021
57f86a4
Remove extra line
azabbasi Feb 16, 2021
e54f752
Add clientDiagnostics to LocalModelFetcher
azabbasi Feb 16, 2021
901cf6d
Throw if cancellation is requested.
azabbasi Feb 16, 2021
c753287
Update LocalModelFetcher.cs
azabbasi Feb 16, 2021
b4efb9a
Update ResolverEventSource.cs
azabbasi Feb 16, 2021
5a29be0
Merge branch 'feature/modelsRepo' into azabbasi/RemoteFetcher
azabbasi Feb 17, 2021
30f1817
Add Async APIs to the ResolverClient
azabbasi Feb 17, 2021
63204bb
Update ModelQuery.cs
azabbasi Feb 17, 2021
9d03859
Make DefaultModelRepository public.
azabbasi Feb 17, 2021
b1eaeae
Use async bool pattern.
azabbasi Feb 17, 2021
9d8c3d2
Make ModelSRepository plural
azabbasi Feb 17, 2021
bb10fd2
Address comments
azabbasi Feb 17, 2021
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
2 changes: 1 addition & 1 deletion sdk/modelsrepository/Azure.Iot.ModelsRepository/src/Fetchers/LocalModelFetcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public FetchResult Fetch(string dtmi, Uri repositoryUri, CancellationToken cance
fnfError = string.Format(CultureInfo.CurrentCulture, ServiceStrings.ErrorFetchingModelContent, tryContentPath);
}

throw new RequestFailedException(fnfError, new FileNotFoundException(fnfError));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on RequestFailedException documentation, it only applies to requests that have an HTTP status.
This exception is caught at RepositoryHandler level and converted to a ResolverException which is currently the Exception that is exposed to users.
We will have a discussion about the Exception that is thrown from the ResolverException point of view during the initial API review

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add doc comments to the client that indicate what exception types can be thrown and when.

throw new FileNotFoundException(fnfError);
}
catch (Exception ex)
{
Expand Down
147 changes: 73 additions & 74 deletions sdk/modelsrepository/Azure.Iot.ModelsRepository/src/ModelQuery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Collections.Generic;
using System.IO;
using System.Text.Json;
using System.Threading.Tasks;
Copy link
Member

Choose a reason for hiding this comment

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

Placeholder/reminder that there is a cleaner implementation of model parsing logic recently merged in the DMR tools repo, which I will update here after this PR is merged.


namespace Azure.Iot.ModelsRepository
{
Expand All @@ -29,16 +28,14 @@ public ModelMetadata GetMetadata()

public string GetId()
{
using (JsonDocument document = JsonDocument.Parse(_content, _parseOptions))
{
JsonElement _root = document.RootElement;
using JsonDocument document = JsonDocument.Parse(_content, _parseOptions);
JsonElement _root = document.RootElement;

if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("@id", out JsonElement id))
if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("@id", out JsonElement id))
{
if (id.ValueKind == JsonValueKind.String)
{
if (id.ValueKind == JsonValueKind.String)
{
return id.GetString();
}
return id.GetString();
}
}

Expand All @@ -49,34 +46,32 @@ public IList<string> GetExtends()
{
List<string> dependencies = new List<string>();

using (JsonDocument document = JsonDocument.Parse(_content, _parseOptions))
{
JsonElement _root = document.RootElement;
using JsonDocument document = JsonDocument.Parse(_content, _parseOptions);
JsonElement _root = document.RootElement;

if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("extends", out JsonElement extends))
if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("extends", out JsonElement extends))
{
if (extends.ValueKind == JsonValueKind.Array)
{
if (extends.ValueKind == JsonValueKind.Array)
foreach (JsonElement extendElement in extends.EnumerateArray())
{
foreach (JsonElement extendElement in extends.EnumerateArray())
if (extendElement.ValueKind == JsonValueKind.String)
{
if (extendElement.ValueKind == JsonValueKind.String)
{
dependencies.Add(extendElement.GetString());
}
else if (extendElement.ValueKind == JsonValueKind.Object)
{
// extends can have multiple levels and can contain components.
// TODO: Support object ctor - inefficient serialize.
ModelMetadata nested_interface = new ModelQuery(JsonSerializer.Serialize(extendElement)).GetMetadata();
dependencies.AddRange(nested_interface.Dependencies);
}
dependencies.Add(extendElement.GetString());
}
else if (extendElement.ValueKind == JsonValueKind.Object)
{
// extends can have multiple levels and can contain components.
// TODO: Support object ctor - inefficient serialize.
ModelMetadata nested_interface = new ModelQuery(JsonSerializer.Serialize(extendElement)).GetMetadata();
dependencies.AddRange(nested_interface.Dependencies);
}
}
else if (extends.ValueKind == JsonValueKind.String)
{
dependencies.Add(extends.GetString());
}
}
else if (extends.ValueKind == JsonValueKind.String)
{
dependencies.Add(extends.GetString());
}
}

return dependencies;
Expand All @@ -87,44 +82,42 @@ public IList<string> GetComponentSchemas()
{
List<string> componentSchemas = new List<string>();

using (JsonDocument document = JsonDocument.Parse(_content, _parseOptions))
{
JsonElement _root = document.RootElement;
using JsonDocument document = JsonDocument.Parse(_content, _parseOptions);
JsonElement _root = document.RootElement;

if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("contents", out JsonElement contents))
if (_root.ValueKind == JsonValueKind.Object && _root.TryGetProperty("contents", out JsonElement contents))
{
if (contents.ValueKind == JsonValueKind.Array)
{
if (contents.ValueKind == JsonValueKind.Array)
foreach (JsonElement element in contents.EnumerateArray())
{
foreach (JsonElement element in contents.EnumerateArray())
if (element.TryGetProperty("@type", out JsonElement type))
{
if (element.TryGetProperty("@type", out JsonElement type))
if (type.ValueKind == JsonValueKind.String && type.GetString() == "Component")
{
if (type.ValueKind == JsonValueKind.String && type.GetString() == "Component")
if (element.TryGetProperty("schema", out JsonElement schema))
{
if (element.TryGetProperty("schema", out JsonElement schema))
if (schema.ValueKind == JsonValueKind.String)
{
if (schema.ValueKind == JsonValueKind.String)
{
componentSchemas.Add(schema.GetString());
}
else if (schema.ValueKind == JsonValueKind.Array)
componentSchemas.Add(schema.GetString());
}
else if (schema.ValueKind == JsonValueKind.Array)
{
foreach (JsonElement schemaElement in schema.EnumerateArray())
{
foreach (JsonElement schemaElement in schema.EnumerateArray())
if (schemaElement.ValueKind == JsonValueKind.String)
{
if (schemaElement.ValueKind == JsonValueKind.String)
{
componentSchemas.Add(schemaElement.GetString());
}
componentSchemas.Add(schemaElement.GetString());
}
}
else if (schema.ValueKind == JsonValueKind.Object)
}
else if (schema.ValueKind == JsonValueKind.Object)
{
if (schema.TryGetProperty("extends", out JsonElement schemaObjExtends))
{
if (schema.TryGetProperty("extends", out JsonElement schemaObjExtends))
if (schemaObjExtends.ValueKind == JsonValueKind.String)
{
if (schemaObjExtends.ValueKind == JsonValueKind.String)
{
componentSchemas.Add(schemaObjExtends.GetString());
}
componentSchemas.Add(schemaObjExtends.GetString());
}
}
}
Expand All @@ -138,39 +131,45 @@ public IList<string> GetComponentSchemas()
return componentSchemas;
}

public async Task<Dictionary<string, string>> ListToDictAsync()
public Dictionary<string, string> ListToDict()
{
Dictionary<string, string> result = new Dictionary<string, string>();

using (JsonDocument document = JsonDocument.Parse(_content, _parseOptions))
{
JsonElement _root = document.RootElement;
using JsonDocument document = JsonDocument.Parse(_content, _parseOptions);
JsonElement _root = document.RootElement;

if (_root.ValueKind == JsonValueKind.Array)
if (_root.ValueKind == JsonValueKind.Array)
{
foreach (JsonElement element in _root.EnumerateArray())
{
foreach (JsonElement element in _root.EnumerateArray())
if (element.ValueKind == JsonValueKind.Object)
{
if (element.ValueKind == JsonValueKind.Object)
{
using (MemoryStream stream = new MemoryStream())
{
await JsonSerializer.SerializeAsync(stream, element).ConfigureAwait(false);
stream.Position = 0;
using MemoryStream stream = WriteJsonElementToStream(element);

using (StreamReader streamReader = new StreamReader(stream))
{
string serialized = await streamReader.ReadToEndAsync().ConfigureAwait(false);
using (StreamReader streamReader = new StreamReader(stream))
azabbasi marked this conversation as resolved.
Show resolved Hide resolved
{
string serialized = streamReader.ReadToEnd();

string id = new ModelQuery(serialized).GetId();
result.Add(id, serialized);
}
}
string id = new ModelQuery(serialized).GetId();
result.Add(id, serialized);
}
}
}
}

return result;
}

private static MemoryStream WriteJsonElementToStream(JsonElement item)
{
var memoryStream = new MemoryStream();
using var writer = new Utf8JsonWriter(memoryStream);

item.WriteTo(writer);
writer.Flush();
memoryStream.Seek(0, SeekOrigin.Begin);

return memoryStream;
}
}
}
2 changes: 2 additions & 0 deletions sdk/modelsrepository/Azure.Iot.ModelsRepository/src/ModelRepositoryConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ internal static class ModelRepositoryConstants
// Set EventSource name to package name replacing '.' with '-'
public const string ModelRepositoryEventSourceName = "Azure-Iot-ModelsRepository";

public const string DefaultModelRepository = "https://devicemodels.azure.com";
azabbasi marked this conversation as resolved.
Show resolved Hide resolved

// File Extensions
public const string JsonFileExtension = ".json";
public const string ExpandedJsonFileExtension = ".expanded.json";
Expand Down
113 changes: 103 additions & 10 deletions sdk/modelsrepository/Azure.Iot.ModelsRepository/src/RepositoryHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,37 +37,100 @@ public async Task<IDictionary<string, string>> ProcessAsync(string dtmi, Cancell
return await ProcessAsync(new List<string>() { dtmi }, cancellationToken).ConfigureAwait(false);
}

public async Task<IDictionary<string, string>> ProcessAsync(IEnumerable<string> dtmis, CancellationToken cancellationToken)
public IDictionary<string, string> Process(string dtmi, CancellationToken cancellationToken)
{
return Process(new List<string>() { dtmi }, cancellationToken);
}

public IDictionary<string, string> Process(IEnumerable<string> dtmis, CancellationToken cancellationToken)
Copy link
Contributor Author

@azabbasi azabbasi Feb 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As of now, this method is the exact replica of the async method with the exception of calling Fetch instead of FetchAsync
I am speaking to a few people to see what is the right way to do this, duplicating this much code just to change a sync vs async call is really expensive.

for now, I will leave the PR in this state so other things can be reviewed while I investigate different options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

using bool async pattern
https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs#L45
https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs#L55-L61

Copy link
Member

Choose a reason for hiding this comment

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

Nice. And good find of prior art.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a reasonable way to do it. :)

{
Dictionary<string, string> processedModels = new Dictionary<string, string>();
azabbasi marked this conversation as resolved.
Show resolved Hide resolved
Queue<string> toProcessModels = new Queue<string>();
Queue<string> toProcessModels = PrepareWork(dtmis);

foreach (string dtmi in dtmis)
while (toProcessModels.Count != 0)
{
if (!DtmiConventions.IsDtmi(dtmi))
cancellationToken.ThrowIfCancellationRequested();

string targetDtmi = toProcessModels.Dequeue();
if (processedModels.ContainsKey(targetDtmi))
{
ResolverEventSource.Instance.InvalidDtmiInput(dtmi);
string invalidArgMsg = string.Format(CultureInfo.CurrentCulture, ServiceStrings.InvalidDtmiFormat, dtmi);
throw new ResolverException(dtmi, invalidArgMsg, new ArgumentException(invalidArgMsg));
ResolverEventSource.Instance.SkippingPreprocessedDtmi(targetDtmi);
continue;
}

toProcessModels.Enqueue(dtmi);
ResolverEventSource.Instance.ProcessingDtmi(targetDtmi);

FetchResult result = Fetch(targetDtmi, cancellationToken);

if (result.FromExpanded)
{
Dictionary<string, string> expanded = new ModelQuery(result.Definition).ListToDict();

foreach (KeyValuePair<string, string> kvp in expanded)
{
if (!processedModels.ContainsKey(kvp.Key))
{
processedModels.Add(kvp.Key, kvp.Value);
}
}

continue;
}

ModelMetadata metadata = new ModelQuery(result.Definition).GetMetadata();

if (ClientOptions.DependencyResolution >= DependencyResolutionOption.Enabled)
{
IList<string> dependencies = metadata.Dependencies;

if (dependencies.Count > 0)
{
ResolverEventSource.Instance.DiscoveredDependencies(string.Join("\", \"", dependencies));
}

foreach (string dep in dependencies)
{
toProcessModels.Enqueue(dep);
}
}

string parsedDtmi = metadata.Id;
if (!parsedDtmi.Equals(targetDtmi, StringComparison.Ordinal))
{
ResolverEventSource.Instance.IncorrectDtmiCasing(targetDtmi, parsedDtmi);
string formatErrorMsg = string.Format(CultureInfo.CurrentCulture, ServiceStrings.IncorrectDtmiCasing, targetDtmi, parsedDtmi);
throw new ResolverException(targetDtmi, formatErrorMsg, new FormatException(formatErrorMsg));
}

processedModels.Add(targetDtmi, result.Definition);
}

while (toProcessModels.Count != 0 && !cancellationToken.IsCancellationRequested)
return processedModels;
}

public async Task<IDictionary<string, string>> ProcessAsync(IEnumerable<string> dtmis, CancellationToken cancellationToken)
{
Dictionary<string, string> processedModels = new Dictionary<string, string>();
Queue<string> toProcessModels = PrepareWork(dtmis);

while (toProcessModels.Count != 0)
{
cancellationToken.ThrowIfCancellationRequested();

string targetDtmi = toProcessModels.Dequeue();
if (processedModels.ContainsKey(targetDtmi))
{
ResolverEventSource.Instance.SkippingPreprocessedDtmi(targetDtmi);
continue;
}

ResolverEventSource.Instance.ProcessingDtmi(targetDtmi);

FetchResult result = await FetchAsync(targetDtmi, cancellationToken).ConfigureAwait(false);

if (result.FromExpanded)
{
Dictionary<string, string> expanded = await new ModelQuery(result.Definition).ListToDictAsync().ConfigureAwait(false);
Dictionary<string, string> expanded = new ModelQuery(result.Definition).ListToDict();

foreach (KeyValuePair<string, string> kvp in expanded)
{
Expand Down Expand Up @@ -122,5 +185,35 @@ private async Task<FetchResult> FetchAsync(string dtmi, CancellationToken cancel
throw new ResolverException(dtmi, ex.Message, ex);
}
}

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

private static Queue<string> PrepareWork(IEnumerable<string> dtmis)
{
Queue<string> toProcessModels = new Queue<string>();
azabbasi marked this conversation as resolved.
Show resolved Hide resolved
foreach (string dtmi in dtmis)
{
if (!DtmiConventions.IsDtmi(dtmi))
{
ResolverEventSource.Instance.InvalidDtmiInput(dtmi);
string invalidArgMsg = string.Format(CultureInfo.CurrentCulture, ServiceStrings.InvalidDtmiFormat, dtmi);
throw new ResolverException(dtmi, invalidArgMsg, new ArgumentException(invalidArgMsg));
}

toProcessModels.Enqueue(dtmi);
}

return toProcessModels;
}
}
}
Loading