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

UnmappedMemberHandling not working with Microsoft.AspNetCore.OpenApi #57981

Closed
1 task done
JTeeuwissen opened this issue Sep 20, 2024 · 3 comments
Closed
1 task done
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.
Milestone

Comments

@JTeeuwissen
Copy link
Contributor

JTeeuwissen commented Sep 20, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

In response to: #56318 (comment)

JsonUnmappedMemberHandling.Disallow does not seem to work.

If I enable it for this object

class ExampleObject
{
	public int ExampleProperty { get; init; }
}

I get

"ExampleObject": {
    "type": "object",
    "properties": {
        "exampleProperty": {
            "type": "integer",
            "format": "int32"
        }
    }
}

or an exception, depending on the way of configuration.

Expected Behavior

I expect to see

"ExampleObject": {
    "type": "object",
    "additionalProperties": false,
    "properties": {
        "exampleProperty": {
            "type": "integer",
            "format": "int32"
        }
    }
}

Steps To Reproduce

using System.Text.Json.Serialization;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddOpenApi();

// This seems to do nothing
builder.Services.ConfigureHttpJsonOptions(options =>
	options.SerializerOptions.UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow);

var app = builder.Build();

app.MapGet("/hello", () => new ExampleObject
{
	ExampleProperty = 123
});

app.MapOpenApi();

app.Run();

// This attributes causes an error
[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)]
class ExampleObject
{
	public int ExampleProperty { get; init; }
}

Exceptions (if any)

System.Text.Json.JsonException: 'Expected StartObject token to represent beginning of schema.'
   at OpenApiJsonSchema.JsonConverter.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options) in -\OpenApiJsonSchema.cs:line 21

presumably because additional properties is expected to be an object, and not False.

.NET Version

9.0.100-rc.1.24452.12

Anything else?

Something like the transformer below provides the expected behavior for my test cases

internal class DisableAdditionalPropertiesTransformer : IOpenApiSchemaTransformer
{
	public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken)
	{
		if (schema is { Type: "object", AdditionalProperties: null } &&
		    context.JsonTypeInfo.Kind is not JsonTypeInfoKind.Dictionary &&
		    !context.JsonTypeInfo.Properties.Any(property => property.IsExtensionData))
		{
			schema.AdditionalPropertiesAllowed = false;
		}

		return Task.CompletedTask;
	}
}
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Sep 20, 2024
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Sep 22, 2024
@captainsafia captainsafia added this to the 9.0.0 milestone Sep 22, 2024
@captainsafia
Copy link
Member

@JTeeuwissen Thanks for filing this issue!

I've included a bug fix for this in a PR I'm working to address other feedback in the area. The fix for this should ship in .NET 9.

Once the PR is merged, you should be able to access preview builds of the package to access this fix. I can share instructions on how to do that once it ships if you're interested in using the preview bits.

@captainsafia captainsafia self-assigned this Sep 22, 2024
wtgodbe pushed a commit that referenced this issue Sep 25, 2024
* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
captainsafia added a commit that referenced this issue Sep 26, 2024
* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
@captainsafia
Copy link
Member

@JTeeuwissen The fix for this has landed in nightly package version 9.0.0-rtm.24476.2.

You'll need to use the following PackageReference:

<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.0-rtm.24476.2" />

And make sure that you have a reference to the nightly dotnet9 feed in your nuget.config file:

<add key="dotnet9" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet9/nuget/v3/index.json" />

Are you able to verify the fix on your end?

@captainsafia captainsafia added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Sep 26, 2024
@JTeeuwissen
Copy link
Contributor Author

@JTeeuwissen The fix for this has landed in nightly package version 9.0.0-rtm.24476.2.

You'll need to use the following PackageReference:

<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.0-rtm.24476.2" />

And make sure that you have a reference to the nightly dotnet9 feed in your nuget.config file:

<add key="dotnet9" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet9/nuget/v3/index.json" />

Are you able to verify the fix on your end?

[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)]

seems to work but

builder.Services.ConfigureHttpJsonOptions(options =>
	options.SerializerOptions.UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow);

still seemingly does nothing.

captainsafia added a commit that referenced this issue Dec 31, 2024
… (#58096)

* Fix JsonUnmappedMemberHandling attribute handling to close #57981

* Fix enum handling for MVC actions to close #57979

* Fix self-referential schema handling to close #58006

* Fix concurrent request handling for OpenAPI documents (#57972)

* fix: Allow concurrent requests

* test: Update test

* test: Use Parallel.ForEachAsync

* feat: Use valueFactory overload

* feat: Pass valueFactory directly

* Harden self-referencing schema ID check

---------

Co-authored-by: Justin Lampe <xC0dex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.
Projects
None yet
Development

No branches or pull requests

3 participants
@captainsafia @JTeeuwissen and others