Skip to content

Add Azure.Core JsonPatchDocument to DigitalTwins project to replace UpdateOperationsUtility #16129

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

Merged
timtay-microsoft merged 3 commits into from
Oct 21, 2020
Merged

Add Azure.Core JsonPatchDocument to DigitalTwins project to replace UpdateOperationsUtility #16129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44ce4ae
Add Azure.Core JsonPatchDocument to replace our UpdateOperationsUtility
timtay-microsoft Oct 20, 2020
83e4cd4
fixup
timtay-microsoft Oct 20, 2020
5720b61
more comments
timtay-microsoft Oct 20, 2020
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
20 changes: 6 additions & 14 deletions sdk/digitaltwins/Azure.DigitalTwins.Core/api/Azure.DigitalTwins.Core.netstandard2.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ public DigitalTwinsClient(System.Uri endpoint, Azure.Core.TokenCredential creden
public virtual System.Threading.Tasks.Task<Azure.Response> PublishTelemetryAsync(string digitalTwinId, string messageId, string payload, Azure.DigitalTwins.Core.PublishTelemetryOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Pageable<string> Query(string query, Azure.DigitalTwins.Core.QueryOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.AsyncPageable<string> QueryAsync(string query, Azure.DigitalTwins.Core.QueryOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateComponent(string digitalTwinId, string componentName, string jsonPatch, Azure.DigitalTwins.Core.UpdateComponentOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateComponentAsync(string digitalTwinId, string componentName, string jsonPatch, Azure.DigitalTwins.Core.UpdateComponentOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateDigitalTwin(string digitalTwinId, string jsonPatch, Azure.DigitalTwins.Core.UpdateDigitalTwinOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateDigitalTwinAsync(string digitalTwinId, string jsonPatch, Azure.DigitalTwins.Core.UpdateDigitalTwinOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateRelationship(string digitalTwinId, string relationshipId, string jsonPatch, Azure.DigitalTwins.Core.UpdateRelationshipOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateRelationshipAsync(string digitalTwinId, string relationshipId, string jsonPatch, Azure.DigitalTwins.Core.UpdateRelationshipOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateComponent(string digitalTwinId, string componentName, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateComponentOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
Copy link
Copy Markdown
Contributor

@drwill-ms drwill-ms Oct 20, 2020

Choose a reason for hiding this comment

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

Curious we chose to require this specific data type, rather than a string that has json. My reasoning being that a customer wanted to use their own library, they can't do that now. Thoughts?

Copy link
Copy Markdown
Member Author

@timtay-microsoft timtay-microsoft Oct 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That's a fair point. I can revert these APIs back to taking strings rather than the JsonPatchDocument itself for now. I'll add a note in our board review to talk about this further

Scratch that, the JsonPatchDocument class allows for users to set their own ObjectSerializer, so this isn't an issue. I'd still like to use this class directly in our APIs, especially if we are getting away from representing untyped json as strings

Copy link
Copy Markdown
Contributor

@drwill-ms drwill-ms Oct 20, 2020

Choose a reason for hiding this comment

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

I'd recommend leaving the way you have it for now, reach out to Krzysztof directly for direction advice, and then mention it in the board review.

Copy link
Copy Markdown
Member Author

@timtay-microsoft timtay-microsoft Oct 20, 2020

Choose a reason for hiding this comment

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

@drwill-ms just in case you signed off thinking I would revert the API shape back to taking a string. Thoughts on taking the JsonPatchDocument knowing that it takes an ObjectSerializer that allows for users to provide their favorite json library?

Copy link
Copy Markdown
Contributor

@drwill-ms drwill-ms Oct 20, 2020

Choose a reason for hiding this comment

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

Nope, I didn't sign off with that understanding. I'd have been more explicit if that were the case.

public virtual System.Threading.Tasks.Task<Azure.Response> UpdateComponentAsync(string digitalTwinId, string componentName, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateComponentOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateDigitalTwin(string digitalTwinId, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateDigitalTwinOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateDigitalTwinAsync(string digitalTwinId, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateDigitalTwinOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response UpdateRelationship(string digitalTwinId, string relationshipId, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateRelationshipOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateRelationshipAsync(string digitalTwinId, string relationshipId, Azure.JsonPatchDocument jsonPatchDocument, Azure.DigitalTwins.Core.UpdateRelationshipOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
}
public partial class DigitalTwinsClientOptions : Azure.Core.ClientOptions
{
Expand Down Expand Up @@ -300,14 +300,6 @@ public ModelProperties() { }
[System.Text.Json.Serialization.JsonPropertyNameAttribute("$metadata")]
public Azure.DigitalTwins.Core.Serialization.ComponentMetadata Metadata { get { throw null; } }
}
public partial class UpdateOperationsUtility
{
public UpdateOperationsUtility() { }
public void AppendAddOp(string path, object value) { }
public void AppendRemoveOp(string path) { }
public void AppendReplaceOp(string path, object value) { }
public string Serialize() { throw null; }
}
public partial class WritableProperty
{
public WritableProperty() { }
Expand Down
7 changes: 3 additions & 4 deletions ...digitaltwins/Azure.DigitalTwins.Core/samples/DigitalTwinsClientSample/ComponentSamples.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ await client.CreateModelsAsync(

// Update Component1 by replacing the property ComponentProp1 value,
// using an optional utility to build the payload.
var componentUpdateUtility = new UpdateOperationsUtility();
componentUpdateUtility.AppendReplaceOp("/ComponentProp1", "Some new value");
string updatePayload = componentUpdateUtility.Serialize();
await client.UpdateComponentAsync(basicDtId, "Component1", updatePayload);
var componentJsonPatchDocument = new JsonPatchDocument();
componentJsonPatchDocument.AppendReplace("/ComponentProp1", "Some new value");
await client.UpdateComponentAsync(basicDtId, "Component1", componentJsonPatchDocument);
Console.WriteLine($"Updated component for digital twin '{basicDtId}'.");

#endregion Snippet:DigitalTwinsSampleUpdateComponent
Expand Down
7 changes: 3 additions & 4 deletions sdk/digitaltwins/Azure.DigitalTwins.Core/samples/Readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,9 @@ To update a component or in other words to replace, remove and/or add a componen
```C# Snippet:DigitalTwinsSampleUpdateComponent
// Update Component1 by replacing the property ComponentProp1 value,
// using an optional utility to build the payload.
var componentUpdateUtility = new UpdateOperationsUtility();
componentUpdateUtility.AppendReplaceOp("/ComponentProp1", "Some new value");
string updatePayload = componentUpdateUtility.Serialize();
await client.UpdateComponentAsync(basicDtId, "Component1", updatePayload);
var componentJsonPatchDocument = new JsonPatchDocument();
componentJsonPatchDocument.AppendReplace("/ComponentProp1", "Some new value");
await client.UpdateComponentAsync(basicDtId, "Component1", componentJsonPatchDocument);
Console.WriteLine($"Updated component for digital twin '{basicDtId}'.");
```

Expand Down
16 changes: 14 additions & 2 deletions sdk/digitaltwins/Azure.DigitalTwins.Core/src/Azure.DigitalTwins.Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<!-- Assembly props -->
<PropertyGroup>
<AssemblyTitle>Digital Twins SDK</AssemblyTitle>
Expand Down Expand Up @@ -51,5 +51,17 @@
</Compile>
</ItemGroup>

<Import Project="$(RepoRoot)\sdk\core\Azure.Core\src\Azure.Core.props" />
<!--
This project currently takes a project reference on Azure.Core, rather than deferring to Azure.Core.props to choose between project reference and nuget reference.
This is because the project needs access to the JsonPatchDocument type that is defined in Azure.Core, but has not been released in a nuget yet. After the next
release of Azure.Core, the below can be changed back so that we just import Azure.Core.props
-->

<!--Uncomment the below to take reference from Azure.Core based on how Azure.Core.props defines how libraries should take reference-->
Copy link
Copy Markdown
Contributor

@drwill-ms drwill-ms Oct 20, 2020

Choose a reason for hiding this comment

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

Can you elaborate on the problem here?

Copy link
Copy Markdown
Member Author

@timtay-microsoft timtay-microsoft Oct 20, 2020

Choose a reason for hiding this comment

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

I can add more comments to the code itself, too, but the issue here is that the Azure.Core library hasn't had a release since they added the JsonPatchDocument class. So the only way we'll get access to it is to take a project reference instead.

This all becomes moot if we don't want the JsonPatchDocument in our API shape, though

Copy link
Copy Markdown
Contributor

@drwill-ms drwill-ms Oct 20, 2020

Choose a reason for hiding this comment

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

Gotcha. Might put it in those words as the existing comment didn't make sense to me.

<!--<Import Project="$(RepoRoot)\sdk\core\Azure.Core\src\Azure.Core.props" />-->

<!--Remove the below after uncommenting the above in order to stop taking a project reference on Azure.Core-->
<ItemGroup>
<ProjectReference Include="..\..\..\core\Azure.Core\src\Azure.Core.csproj" />
</ItemGroup>
</Project>
Loading