Skip to content

second level inheritance for UseOneOfForPolymorphism #3155

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

Merged
merged 2 commits into from
Nov 22, 2024
Merged

second level inheritance for UseOneOfForPolymorphism #3155

merged 2 commits into from
Nov 22, 2024

Conversation

k0ka
Copy link
Contributor

@k0ka k0ka commented Nov 19, 2024

The issue or feature being addressed

The UseAllOfForInheritance option of schema generator only accounts for direct parents. It makes impossible to generate correct discriminator information for a big inheritance tree.

Example

data class structure:

[SwaggerDiscriminator("discriminator")]
[SwaggerSubType(typeof(SubSubType), DiscriminatorValue = nameof(SubSubType))]
abstract public class BaseType
{
}

abstract public class SubType : BaseType
{
}

public class SubSubType : SubType
{
    public int Property { get; set; }
}

Swagger generator options:

builder.Services.AddSwaggerGen(
    options =>
    {
        options.UseAllOfForInheritance();
        options.UseOneOfForPolymorphism();
        options.EnableAnnotations(true, true);
    });

Generated components of the json:

"components": {
    "schemas": {
      "SubSubType": {
        "type": "object",
        "properties": {
          "property": {
            "type": "integer",
            "format": "int32"
          }
        },
        "additionalProperties": false
      }
    }
  }

It doesn't include discriminator and BaseType definition.

If we remove SubType and make SubSubType child of BaseType we would get the expected generated components:

"components": {
    "schemas": {
      "BaseType": {
        "required": [
          "discriminator"
        ],
        "type": "object",
        "properties": {
          "discriminator": {
            "type": "string"
          }
        },
        "additionalProperties": false,
        "discriminator": {
          "propertyName": "discriminator",
          "mapping": {
            "SubSubType": "#/components/schemas/SubSubType"
          }
        }
      },
      "SubSubType": {
        "allOf": [
          {
            "$ref": "#/components/schemas/BaseType"
          },
          {
            "type": "object",
            "properties": {
              "property": {
                "type": "integer",
                "format": "int32"
              }
            },
            "additionalProperties": false
          }
        ]
      }
    }
  }

Details on the issue fix or feature implementation

This PR checks all parents until it finds the one which has this class defined as sub type. I added same assertions to test with one level assertions.

@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.51%. Comparing base (8ea0461) to head (daeadcb).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3155      +/-   ##
==========================================
+ Coverage   83.25%   84.51%   +1.25%     
==========================================
  Files          76       76              
  Lines        3142     3144       +2     
  Branches      526      527       +1     
==========================================
+ Hits         2616     2657      +41     
+ Misses        526      487      -39     
Flag Coverage Δ
Linux 83.26% <100.00%> (+0.01%) ⬆️
Windows 84.47% <100.00%> (+1.21%) ⬆️
macOS 83.26% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@k0ka k0ka changed the title second level inheritance for UseAllOf second level inheritance for UseOneOfForPolymorphism Nov 19, 2024
Copy link
Collaborator

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

Could you also add an example like in the issue description to one of our integration test projects so we have a verification test for this scenario too please?

Fix Annotations discriminator value selector for the 2nd level inheritance
@k0ka
Copy link
Contributor Author

k0ka commented Nov 21, 2024

Hello,

I added this case to NswagClientExample because it was the only integration test which enabled inheritance and polymorphic annotations.
Also, I found a problem in Annotation handler and fixed it. It was using only direct parent annotations to get type value.

@martincostello martincostello enabled auto-merge (squash) November 22, 2024 15:02
@martincostello martincostello added this to the v7.0.1 milestone Nov 22, 2024
@martincostello martincostello merged commit b8e1f0f into domaindrivendev:master Nov 22, 2024
9 checks passed
Copy link

@Eloyjr27 Eloyjr27 left a comment

Choose a reason for hiding this comment

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

ok, all is perfect

This was referenced Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants