diff --git a/AGENTS.md b/AGENTS.md index 948d102..69cd698 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,8 @@ public face of the library on nuget.org. method. Records are patched with a `with` expression. Other types go through constructor binding and an object initializer. - Because the generated assembly is separate, it can only reference **public** types and public - setters or init accessors. Non-public source types throw `NotSupportedException`. + setters or init accessors. Non-public source types throw `NotSupportedException`, unless the + experimental Emit builder below is turned on. - Source property attributes are carried over: `[JsonConverter]` becomes `[OptionalConverter]`, `[JsonPropertyName]` is copied, and each `ValidationAttribute` becomes an `[OptionalValidation(type, index)]` that runs only when the property is present. @@ -43,6 +44,10 @@ public face of the library on nuget.org. `PTrampert.SimplePatch.Emitted`, so it also supports internal source types whose assembly declares `[InternalsVisibleTo("PTrampert.SimplePatch.Emitted")]`, as Castle DynamicProxy does. Private nested types aren't supported. `PatchClassBuilderTest` runs against both builders. +- The public static `PatchClassBuilder.UseExperimentalDynamicClassBuilder` flag (off by default) + makes `PatchClassBuilder.GetPatchClassFor` delegate to `EmitPatchClassBuilder` instead of + `RoslynPatchClassBuilder`. It is process-wide, so the OpenAPI integrations follow it too. Tests + that set it are `[NonParallelizable]` and reset it in `TearDown`. - `JsonOptionsExtensions.AddSimplePatchConverters` registers `OptionalJsonConverterFactory` and `PatchJsonConverterFactory`. - The OpenAPI packages build the patch schema from the **source model's** schema, not from the diff --git a/PTrampert.SimplePatch.OpenApi.Test/ExperimentalDynamicClassBuilderTest.cs b/PTrampert.SimplePatch.OpenApi.Test/ExperimentalDynamicClassBuilderTest.cs new file mode 100644 index 0000000..cc4abc6 --- /dev/null +++ b/PTrampert.SimplePatch.OpenApi.Test/ExperimentalDynamicClassBuilderTest.cs @@ -0,0 +1,44 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.OpenApi; +using Microsoft.Extensions.DependencyInjection; +using PTrampert.SimplePatch.OpenApi.Test.TestObjects; + +namespace PTrampert.SimplePatch.OpenApi.Test; + +// PatchClassBuilder.UseExperimentalDynamicClassBuilder is process-wide, so these tests must not +// overlap with others, and each one puts it back afterwards. +[NonParallelizable] +public class ExperimentalDynamicClassBuilderTest +{ + [TearDown] + public void TearDown() => PatchClassBuilder.UseExperimentalDynamicClassBuilder = false; + + [Test] + public async Task PatchSchema_DescribesAnInternalModelWhenTheFlagIsOn() + { + PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseUrls("http://127.0.0.1:0"); + builder.Services.ConfigureHttpJsonOptions(options => options.SerializerOptions.AddSimplePatchConverters()); + builder.Services.AddOpenApi(options => options.AddSimplePatchSchemas()); + + await using var app = builder.Build(); + app.MapPatch("/people/{id:int}", (int id, IPatchObject patch) => Results.Ok()); + + // Endpoints only reach the document generator once the app has started. + await app.StartAsync(); + var document = await app.Services + .GetRequiredKeyedService("v1") + .GetOpenApiDocumentAsync(); + await app.StopAsync(); + + var patchSchema = document.Components!.Schemas!["IPatchObjectOfInternalPersonTestModel"]; + Assert.Multiple((Action)(() => + { + Assert.That(patchSchema.Properties?.Keys, Is.EquivalentTo(new[] { "name", "email" })); + Assert.That(patchSchema.Properties!["name"].MaxLength, Is.EqualTo(255)); + })); + } +} diff --git a/PTrampert.SimplePatch.OpenApi.Test/PTrampert.SimplePatch.OpenApi.Test.csproj b/PTrampert.SimplePatch.OpenApi.Test/PTrampert.SimplePatch.OpenApi.Test.csproj index 0d3045a..5f428ff 100644 --- a/PTrampert.SimplePatch.OpenApi.Test/PTrampert.SimplePatch.OpenApi.Test.csproj +++ b/PTrampert.SimplePatch.OpenApi.Test/PTrampert.SimplePatch.OpenApi.Test.csproj @@ -24,6 +24,11 @@ + + + + + diff --git a/PTrampert.SimplePatch.OpenApi.Test/TestObjects/InternalPersonTestModel.cs b/PTrampert.SimplePatch.OpenApi.Test/TestObjects/InternalPersonTestModel.cs new file mode 100644 index 0000000..652592e --- /dev/null +++ b/PTrampert.SimplePatch.OpenApi.Test/TestObjects/InternalPersonTestModel.cs @@ -0,0 +1,13 @@ +using System.ComponentModel.DataAnnotations; + +namespace PTrampert.SimplePatch.OpenApi.Test.TestObjects; + +// Internal, which only the experimental Emit builder can patch. The test project grants the +// generated assemblies access with InternalsVisibleTo in its project file. +internal record InternalPersonTestModel +{ + [StringLength(255, MinimumLength = 3)] + public string? Name { get; init; } + + public string? Email { get; init; } +} diff --git a/PTrampert.SimplePatch.Swashbuckle.Test/ExperimentalDynamicClassBuilderTest.cs b/PTrampert.SimplePatch.Swashbuckle.Test/ExperimentalDynamicClassBuilderTest.cs new file mode 100644 index 0000000..c0d07a9 --- /dev/null +++ b/PTrampert.SimplePatch.Swashbuckle.Test/ExperimentalDynamicClassBuilderTest.cs @@ -0,0 +1,34 @@ +using Microsoft.Extensions.DependencyInjection; +using PTrampert.SimplePatch.Swashbuckle.Test.TestObjects; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace PTrampert.SimplePatch.Swashbuckle.Test; + +// PatchClassBuilder.UseExperimentalDynamicClassBuilder is process-wide, so these tests must not +// overlap with others, and each one puts it back afterwards. +[NonParallelizable] +public class ExperimentalDynamicClassBuilderTest +{ + [TearDown] + public void TearDown() => PatchClassBuilder.UseExperimentalDynamicClassBuilder = false; + + [Test] + public void PatchSchema_DescribesAnInternalModelWhenTheFlagIsOn() + { + PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; + var services = new ServiceCollection(); + services.AddSwaggerGen(options => options.AddSimplePatchSchemas()); + using var provider = services.BuildServiceProvider(); + var generator = provider.GetRequiredService(); + + var repository = new SchemaRepository(); + generator.GenerateSchema(typeof(IPatchObject), repository); + + var patchSchema = repository.Schemas["InternalPersonTestModelIPatchObject"]; + Assert.Multiple((Action)(() => + { + Assert.That(patchSchema.Properties?.Keys, Is.EquivalentTo(new[] { "name", "email" })); + Assert.That(patchSchema.Properties!["name"].MaxLength, Is.EqualTo(255)); + })); + } +} diff --git a/PTrampert.SimplePatch.Swashbuckle.Test/PTrampert.SimplePatch.Swashbuckle.Test.csproj b/PTrampert.SimplePatch.Swashbuckle.Test/PTrampert.SimplePatch.Swashbuckle.Test.csproj index cce253d..06ecbd0 100644 --- a/PTrampert.SimplePatch.Swashbuckle.Test/PTrampert.SimplePatch.Swashbuckle.Test.csproj +++ b/PTrampert.SimplePatch.Swashbuckle.Test/PTrampert.SimplePatch.Swashbuckle.Test.csproj @@ -20,6 +20,11 @@ + + + + + diff --git a/PTrampert.SimplePatch.Swashbuckle.Test/TestObjects/InternalPersonTestModel.cs b/PTrampert.SimplePatch.Swashbuckle.Test/TestObjects/InternalPersonTestModel.cs new file mode 100644 index 0000000..9081fa6 --- /dev/null +++ b/PTrampert.SimplePatch.Swashbuckle.Test/TestObjects/InternalPersonTestModel.cs @@ -0,0 +1,13 @@ +using System.ComponentModel.DataAnnotations; + +namespace PTrampert.SimplePatch.Swashbuckle.Test.TestObjects; + +// Internal, which only the experimental Emit builder can patch. The test project grants the +// generated assemblies access with InternalsVisibleTo in its project file. +internal record InternalPersonTestModel +{ + [StringLength(255, MinimumLength = 3)] + public string? Name { get; init; } + + public string? Email { get; init; } +} diff --git a/PTrampert.SimplePatch.Test/UseExperimentalDynamicClassBuilderTest.cs b/PTrampert.SimplePatch.Test/UseExperimentalDynamicClassBuilderTest.cs new file mode 100644 index 0000000..9639fac --- /dev/null +++ b/PTrampert.SimplePatch.Test/UseExperimentalDynamicClassBuilderTest.cs @@ -0,0 +1,77 @@ +using System.ComponentModel.DataAnnotations; +using System.Text.Json; +using PTrampert.SimplePatch.Test.TestObjects; + +namespace PTrampert.SimplePatch.Test; + +// The flag is process-wide, so these tests must not overlap with others that build patch classes +// through PatchClassBuilder, and each one puts the flag back afterwards. +[NonParallelizable] +public class UseExperimentalDynamicClassBuilderTest +{ + [TearDown] + public void TearDown() => PatchClassBuilder.UseExperimentalDynamicClassBuilder = false; + + [Test] + public void IsOffByDefault() + { + Assert.That(PatchClassBuilder.UseExperimentalDynamicClassBuilder, Is.False); + } + + [Test] + public void GetPatchClassFor_UsesTheEmitBuilderWhenOn() + { + PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; + + Assert.That(PatchClassBuilder.Instance.GetPatchClassFor(typeof(PlainClassTestObject)), + Is.SameAs(EmitPatchClassBuilder.Instance.GetPatchClassFor(typeof(PlainClassTestObject)))); + } + + [Test] + public void GetPatchClassFor_UsesTheRoslynBuilderWhenTurnedBackOff() + { + PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; + PatchClassBuilder.Instance.GetPatchClassFor(typeof(PlainClassTestObject)); + PatchClassBuilder.UseExperimentalDynamicClassBuilder = false; + + Assert.That(PatchClassBuilder.Instance.GetPatchClassFor(typeof(PlainClassTestObject)), + Is.SameAs(RoslynPatchClassBuilder.Instance.GetPatchClassFor(typeof(PlainClassTestObject))), + "Each builder keeps its own cache, so the Emit builder's type must not leak into the Roslyn path."); + } + + [Test] + public void GetPatchClassFor_InternalTypeWhenOff_ThrowsNamingTheFlag() + { + var ex = Assert.Throws( + () => PatchClassBuilder.Instance.GetPatchClassFor(typeof(InternalTestObject))); + + Assert.That(ex!.Message, Does.Contain(nameof(PatchClassBuilder.UseExperimentalDynamicClassBuilder))); + } + + [Test] + public void Deserialize_InternalTypeWhenOn_PatchesAndValidates() + { + PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; + // New options, so System.Text.Json hasn't cached a converter for the type from another test. + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + options.AddSimplePatchConverters(); + + var patch = JsonSerializer.Deserialize>( + """{ "display_name": null, "initOnly": "New" }""", options)!; + var invalid = JsonSerializer.Deserialize>( + """{ "rating": 20 }""", options)!; + var result = patch.Patch(new InternalClassTestObject { Name = "Old", InitOnly = "Old", Rating = 3 }); + var validationResults = new List(); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Name, Is.Null, "An explicit null should be applied."); + Assert.That(result.InitOnly, Is.EqualTo("New")); + Assert.That(result.Rating, Is.EqualTo(3), "A property the patch leaves out should keep its value."); + Assert.That(Validator.TryValidateObject(patch, new ValidationContext(patch), validationResults, true), + Is.True); + Assert.That(Validator.TryValidateObject(invalid, new ValidationContext(invalid), validationResults, true), + Is.False, "The source property's [Range] should run on the patch."); + })); + } +} diff --git a/PTrampert.SimplePatch/PatchClassBuilder.cs b/PTrampert.SimplePatch/PatchClassBuilder.cs index 97d6319..ea5b27b 100644 --- a/PTrampert.SimplePatch/PatchClassBuilder.cs +++ b/PTrampert.SimplePatch/PatchClassBuilder.cs @@ -8,7 +8,8 @@ namespace PTrampert.SimplePatch; /// /// This is the library's default builder, and the one uses. /// It delegates to the builder that generates the classes, so that the implementation can change -/// without changing this public type. +/// without changing this public type. selects +/// which builder that is. /// public class PatchClassBuilder : IPatchClassBuilder { @@ -20,6 +21,32 @@ public class PatchClassBuilder : IPatchClassBuilder public static PatchClassBuilder Instance { get; } = new(); #pragma warning restore CS0618 + /// + /// Experimental. When , generates patch + /// classes with Reflection.Emit instead of compiling C# with Roslyn. The Emit builder also + /// supports source types that aren't public. Defaults to . + /// + /// + /// + /// The setting is process-wide, so it applies to and the + /// OpenAPI integration packages alike. Set it once at startup, before any patch class is built. + /// Each builder keeps its own cache, but System.Text.Json and the OpenAPI document generators + /// cache the types they have already resolved, so changing the setting later doesn't replace + /// patch types that are already in use. + /// + /// + /// An internal source type is supported when its assembly declares + /// [assembly: InternalsVisibleTo("PTrampert.SimplePatch.Emitted")], which grants access to + /// the generated assemblies. The same applies to any internal property types and accessors the + /// patch class uses. Private and protected nested types aren't supported. + /// + /// + /// The Emit builder is planned to replace the Roslyn builder in the next major version, which will + /// remove this setting: https://github.com/PaulTrampert/PTrampert.SimplePatch/issues/126 + /// + /// + public static bool UseExperimentalDynamicClassBuilder { get; set; } + /// /// Creates a builder. /// @@ -46,7 +73,12 @@ public PatchClassBuilder() /// The type to get a patch type for. /// The generated patch type. /// - /// is not public, or is nested in or constructed from a type that is not public. + /// is not public, or is nested in or constructed from a type that is not + /// public, and is off. With it on, a type or + /// accessor the generated assembly can't access, such as a private nested type, or an internal + /// type whose assembly doesn't grant [InternalsVisibleTo]. /// - public Type GetPatchClassFor(Type type) => RoslynPatchClassBuilder.Instance.GetPatchClassFor(type); + public Type GetPatchClassFor(Type type) => UseExperimentalDynamicClassBuilder + ? EmitPatchClassBuilder.Instance.GetPatchClassFor(type) + : RoslynPatchClassBuilder.Instance.GetPatchClassFor(type); } diff --git a/PTrampert.SimplePatch/RoslynPatchClassBuilder.cs b/PTrampert.SimplePatch/RoslynPatchClassBuilder.cs index 91fe7b6..f402177 100644 --- a/PTrampert.SimplePatch/RoslynPatchClassBuilder.cs +++ b/PTrampert.SimplePatch/RoslynPatchClassBuilder.cs @@ -58,7 +58,9 @@ private static Type CreatePatchClass(Type type) { throw new NotSupportedException( $"Cannot create a patch class for '{type.FullName}' because it is not public. Patch source " - + "types must be public, as must any types they are nested in and any generic type arguments."); + + "types must be public, as must any types they are nested in and any generic type arguments. " + + $"To patch internal types, set {nameof(PatchClassBuilder)}." + + $"{nameof(PatchClassBuilder.UseExperimentalDynamicClassBuilder)} to true."); } // The Patch method body is a hand-written snippet, so every name in it has to be formatted diff --git a/README.md b/README.md index fb10868..7c805c8 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,41 @@ JsonSerializer.Serialize(new PersonPatch { Name = "New Name", Email = null }, op No `DefaultIgnoreCondition` is needed for this, so it doesn't affect your other types. +## Non-public write models (experimental) + +By default, a write model must be public, as must any type it is nested in. The patch class is +compiled into a separate assembly, which can only refer to public types, so `IPatchObject` +throws `NotSupportedException` for an `internal` `T`. + +An experimental builder generates the patch class with Reflection.Emit instead, and supports +internal write models. Turn it on once at startup, before any patch type is built: + +```csharp +PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; +``` + +Then grant the generated assemblies access to the assembly that declares your internal models: + +```csharp +[assembly: InternalsVisibleTo("PTrampert.SimplePatch.Emitted")] +``` + +or, in the project file: + +```xml + + + +``` + +- The setting is process-wide, so the OpenAPI integrations below use the same builder. +- An internal property type declared in another assembly needs the same grant from that assembly. +- Private and protected nested types, such as a `private class` inside a controller, aren't + supported. +- The Emit builder is planned to become the only builder in the next major version + ([#126](https://github.com/PaulTrampert/PTrampert.SimplePatch/issues/126)), which will remove this + setting. + ## OpenAPI Out of the box, an OpenAPI generator describes a `[FromBody] IPatchObject` parameter from the diff --git a/docs/getting-started.md b/docs/getting-started.md index 3ac0ec6..6fea990 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -85,3 +85,20 @@ dotnet add package PTrampert.SimplePatch ``` See the [README](../README.md#openapi) for the full set of options. + +## Internal Write Models (Experimental) + +Write models must be public by default. To use an `internal` write model, turn on the +experimental Reflection.Emit builder at startup, before any patch type is built, and grant its +generated assemblies access to your internal types: + +```csharp +PatchClassBuilder.UseExperimentalDynamicClassBuilder = true; +``` + +```csharp +[assembly: InternalsVisibleTo("PTrampert.SimplePatch.Emitted")] +``` + +Private and protected nested types aren't supported. See the +[README](../README.md#non-public-write-models-experimental) for details.