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

Add async methods to the ResolverClient #18814

Merged
merged 18 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 15 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 @@ -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
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;
}
}
}
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
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

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.

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