Skip to content
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
29 changes: 21 additions & 8 deletions src/Features/JsonPatch.SystemTextJson/src/JsonPatchDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Adapters;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Converters;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Exceptions;
Expand All @@ -18,7 +21,7 @@ namespace Microsoft.AspNetCore.JsonPatch.SystemTextJson;
// documents for cases where there's no class/DTO to work on. Typical use case: backend not built in
// .NET or architecture doesn't contain a shared DTO layer.
[JsonConverter(typeof(JsonPatchDocumentConverter))]
public class JsonPatchDocument : IJsonPatchDocument
public class JsonPatchDocument : IJsonPatchDocument, IEndpointParameterMetadataProvider
{
public List<Operation> Operations { get; private set; }

Expand All @@ -27,7 +30,7 @@ public class JsonPatchDocument : IJsonPatchDocument

public JsonPatchDocument()
{
Operations = new List<Operation>();
Operations = [];
SerializerOptions = JsonSerializerOptions.Default;
}

Expand Down Expand Up @@ -205,17 +208,27 @@ IList<Operation> IJsonPatchDocument.GetOperations()
{
foreach (var op in Operations)
{
var untypedOp = new Operation();

untypedOp.op = op.op;
untypedOp.value = op.value;
untypedOp.path = op.path;
untypedOp.from = op.from;
var untypedOp = new Operation
{
op = op.op,
value = op.value,
path = op.path,
from = op.from
};

allOps.Add(untypedOp);
}
}

return allOps;
}

/// <inheritdoc/>
static void IEndpointParameterMetadataProvider.PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder)
{
ArgumentNullException.ThrowIfNull(parameter);
ArgumentNullException.ThrowIfNull(builder);

builder.Metadata.Add(new AcceptsMetadata(["application/json-patch+json"], parameter.ParameterType));
}
}
17 changes: 14 additions & 3 deletions src/Features/JsonPatch.SystemTextJson/src/JsonPatchDocumentOfT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Adapters;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Converters;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson.Exceptions;
Expand All @@ -23,7 +25,7 @@ namespace Microsoft.AspNetCore.JsonPatch.SystemTextJson;
// including type data in the JsonPatchDocument serialized as JSON (to allow for correct deserialization) - that's
// not according to RFC 6902, and would thus break cross-platform compatibility.
[JsonConverter(typeof(JsonPatchDocumentConverterFactory))]
public class JsonPatchDocument<TModel> : IJsonPatchDocument where TModel : class
public class JsonPatchDocument<TModel> : IJsonPatchDocument, IEndpointParameterMetadataProvider where TModel : class
{
public List<Operation<TModel>> Operations { get; private set; }

Expand All @@ -32,7 +34,7 @@ public class JsonPatchDocument<TModel> : IJsonPatchDocument where TModel : class

public JsonPatchDocument()
{
Operations = new List<Operation<TModel>>();
Operations = [];
SerializerOptions = JsonSerializerOptions.Default;
}

Expand Down Expand Up @@ -657,11 +659,20 @@ IList<Operation> IJsonPatchDocument.GetOperations()
return allOps;
}

/// <inheritdoc/>
static void IEndpointParameterMetadataProvider.PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder)
{
ArgumentNullException.ThrowIfNull(parameter);
ArgumentNullException.ThrowIfNull(builder);

builder.Metadata.Add(new AcceptsMetadata(["application/json-patch+json"], typeof(TModel)));
}

// Internal for testing
internal string GetPath<TProp>(Expression<Func<TModel, TProp>> expr, string position)
{
var segments = GetPathSegments(expr.Body);
var path = String.Join("/", segments);
var path = string.Join('/', segments);
if (position != null)
{
path += "/" + position;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
<Compile Include="$(SharedSourceRoot)CallerArgument\CallerArgumentExpressionAttribute.cs" LinkBase="Shared" />
</ItemGroup>

<ItemGroup>
<Reference Include="Microsoft.AspNetCore.Http.Abstractions" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.AspNetCore.JsonPatch.SystemTextJson.Tests" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Features/JsonPatch/build.cmd
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@ECHO OFF
SET RepoRoot=%~dp0..\..
SET RepoRoot=%~dp0..\..\..
%RepoRoot%\eng\build.cmd -projects %~dp0**\*.*proj %*
36 changes: 29 additions & 7 deletions src/Features/JsonPatch/src/JsonPatchDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Reflection;
using Microsoft.AspNetCore.JsonPatch.Adapters;
using Microsoft.AspNetCore.JsonPatch.Converters;
using Microsoft.AspNetCore.JsonPatch.Exceptions;
Expand All @@ -12,13 +13,22 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;

#if NET
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
#endif

namespace Microsoft.AspNetCore.JsonPatch;

// Implementation details: the purpose of this type of patch document is to allow creation of such
// documents for cases where there's no class/DTO to work on. Typical use case: backend not built in
// .NET or architecture doesn't contain a shared DTO layer.
[JsonConverter(typeof(JsonPatchDocumentConverter))]
#if NET
public class JsonPatchDocument : IJsonPatchDocument, IEndpointParameterMetadataProvider
#else
public class JsonPatchDocument : IJsonPatchDocument
#endif
{
public List<Operation> Operations { get; private set; }

Expand All @@ -27,7 +37,7 @@ public class JsonPatchDocument : IJsonPatchDocument

public JsonPatchDocument()
{
Operations = new List<Operation>();
Operations = [];
ContractResolver = new DefaultContractResolver();
}

Expand Down Expand Up @@ -205,17 +215,29 @@ IList<Operation> IJsonPatchDocument.GetOperations()
{
foreach (var op in Operations)
{
var untypedOp = new Operation();

untypedOp.op = op.op;
untypedOp.value = op.value;
untypedOp.path = op.path;
untypedOp.from = op.from;
var untypedOp = new Operation
{
op = op.op,
value = op.value,
path = op.path,
from = op.from
};

allOps.Add(untypedOp);
}
}

return allOps;
}

#if NET
/// <inheritdoc/>
static void IEndpointParameterMetadataProvider.PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder)
{
ArgumentNullException.ThrowIfNull(parameter);
ArgumentNullException.ThrowIfNull(builder);

builder.Metadata.Add(new AcceptsMetadata(["application/json-patch+json"], parameter.ParameterType));
}
#endif
}
28 changes: 24 additions & 4 deletions src/Features/JsonPatch/src/JsonPatchDocumentOfT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Globalization;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.AspNetCore.JsonPatch.Adapters;
using Microsoft.AspNetCore.JsonPatch.Converters;
using Microsoft.AspNetCore.JsonPatch.Exceptions;
Expand All @@ -15,14 +16,23 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;

#if NET
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
#endif

namespace Microsoft.AspNetCore.JsonPatch;

// Implementation details: the purpose of this type of patch document is to ensure we can do type-checking
// when producing a JsonPatchDocument. However, we cannot send this "typed" over the wire, as that would require
// including type data in the JsonPatchDocument serialized as JSON (to allow for correct deserialization) - that's
// not according to RFC 6902, and would thus break cross-platform compatibility.
[JsonConverter(typeof(TypedJsonPatchDocumentConverter))]
#if NET
public class JsonPatchDocument<TModel> : IJsonPatchDocument, IEndpointParameterMetadataProvider where TModel : class
#else
public class JsonPatchDocument<TModel> : IJsonPatchDocument where TModel : class
#endif
{
public List<Operation<TModel>> Operations { get; private set; }

Expand All @@ -31,7 +41,7 @@ public class JsonPatchDocument<TModel> : IJsonPatchDocument where TModel : class

public JsonPatchDocument()
{
Operations = new List<Operation<TModel>>();
Operations = [];
ContractResolver = new DefaultContractResolver();
}

Expand Down Expand Up @@ -656,11 +666,22 @@ IList<Operation> IJsonPatchDocument.GetOperations()
return allOps;
}

#if NET
/// <inheritdoc/>
static void IEndpointParameterMetadataProvider.PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder)
{
ArgumentNullException.ThrowIfNull(parameter);
ArgumentNullException.ThrowIfNull(builder);

builder.Metadata.Add(new AcceptsMetadata(["application/json-patch+json"], typeof(TModel)));
}
#endif

// Internal for testing
internal string GetPath<TProp>(Expression<Func<TModel, TProp>> expr, string position)
{
var segments = GetPathSegments(expr.Body);
var path = String.Join("/", segments);
var path = string.Join("/", segments);
if (position != null)
{
path += "/" + position;
Expand Down Expand Up @@ -712,8 +733,7 @@ private List<string> GetPathSegments(Expression expr)

private string GetPropertyNameFromMemberExpression(MemberExpression memberExpression)
{
var jsonObjectContract = ContractResolver.ResolveContract(memberExpression.Expression.Type) as JsonObjectContract;
if (jsonObjectContract != null)
if (ContractResolver.ResolveContract(memberExpression.Expression.Type) is JsonObjectContract jsonObjectContract)
{
return jsonObjectContract.Properties
.First(jsonProperty => jsonProperty.UnderlyingName == memberExpression.Member.Name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
<ItemGroup>
<Reference Include="Microsoft.CSharp" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Reference Include="Newtonsoft.Json" />
<Reference Include="Microsoft.AspNetCore.Http.Abstractions" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/OpenApi/sample/Endpoints/MapSchemasEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Immutable;
using System.ComponentModel;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.JsonPatch.SystemTextJson;

public static class SchemasEndpointsExtensions
{
Expand Down Expand Up @@ -36,6 +37,7 @@ public static IEndpointRouteBuilder MapSchemasEndpoints(this IEndpointRouteBuild
schemas.MapPost("/location", (LocationContainer location) => { });
schemas.MapPost("/parent", (ParentObject parent) => Results.Ok(parent));
schemas.MapPost("/child", (ChildObject child) => Results.Ok(child));
schemas.MapPatch("/json-patch", (JsonPatchDocument<ParentObject> patchDoc) => Results.NoContent());

return endpointRouteBuilder;
}
Expand Down
5 changes: 3 additions & 2 deletions src/OpenApi/sample/Sample.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
<Reference Include="Microsoft.AspNetCore" />
<Reference Include="Microsoft.AspNetCore.Authentication.JwtBearer" />
<Reference Include="Microsoft.AspNetCore.Hosting" />
<Reference Include="Microsoft.AspNetCore.OpenApi" />
<Reference Include="Microsoft.AspNetCore.Http" />
<Reference Include="Microsoft.AspNetCore.Http.Results" />
<Reference Include="Microsoft.AspNetCore.JsonPatch.SystemTextJson" />
<Reference Include="Microsoft.AspNetCore.Mvc" />
<Reference Include="Microsoft.AspNetCore.OpenApi" />
<Reference Include="Microsoft.AspNetCore.StaticFiles" />
<Reference Include="Microsoft.Extensions.FileProviders.Embedded" />
<Reference Include="Microsoft.AspNetCore.Mvc" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,28 @@
}
}
}
},
"/schemas-by-ref/json-patch": {
"patch": {
"tags": [
"Sample"
],
"requestBody": {
"content": {
"application/json-patch+json": {
"schema": {
"$ref": "#/components/schemas/JsonPatchDocumentOfParentObject"
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we need a custom JSON convert to model the schema for JSON path document. I assume the schema looks something like:

{
  type: "object",
  properties: {
     "op": "string",
      "path": "string",
      "value": "string"
  }
}

@mikekistler Sanity checking this by you. Does it seem sufficient to use the same schema for all JsonPatchDocument types or should we figure out some way to capture the properties of the model that you're patchinmg in the schema?

Copy link
Member Author

Choose a reason for hiding this comment

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

If needed I could look into doing that in a follow-up, as that's existing behaviour anyway without the media type change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the JsonPatchDocumentOfParentObject is just this (line 624):

 "JsonPatchDocumentOfParentObject": { },

I think we can tackle the issue of the patch body schema in a separate PR. We should probably open an issue for this. I don't have an issue with using the same schema for all patch operations, but it should be more like the schema you show above.

Copy link
Member

Choose a reason for hiding this comment

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

OK, sounds good. We can probably add special handling for the JsonPatchDocument type the same way we do for IFormFile and PipeReader. Hopefully, there's no referemce issues adding a reference to the patching library in the OpenAPI lib.

Copy link
Member Author

Choose a reason for hiding this comment

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

}
}
},
"required": true
},
"responses": {
"200": {
"description": "OK"
}
}
}
}
},
"components": {
Expand Down Expand Up @@ -599,6 +621,7 @@
}
}
},
"JsonPatchDocumentOfParentObject": { },
"LocationContainer": {
"required": [
"location"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,28 @@
}
}
}
},
"/schemas-by-ref/json-patch": {
"patch": {
"tags": [
"Sample"
],
"requestBody": {
"content": {
"application/json-patch+json": {
"schema": {
"$ref": "#/components/schemas/JsonPatchDocumentOfParentObject"
}
}
},
"required": true
},
"responses": {
"200": {
"description": "OK"
}
}
}
}
},
"components": {
Expand Down Expand Up @@ -599,6 +621,7 @@
}
}
},
"JsonPatchDocumentOfParentObject": { },
"LocationContainer": {
"required": [
"location"
Expand Down
Loading