-
-
Notifications
You must be signed in to change notification settings - Fork 64
Fix: Base type not generated for types using oneOf with discriminator #906
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
Changes from all commits
85af425
edaf6b6
7cd060f
81ef306
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,283 @@ | ||||||
| using FluentAssertions; | ||||||
| using Refitter.Core; | ||||||
| using Refitter.Tests.Build; | ||||||
| using Refitter.Tests.TestUtilities; | ||||||
| using TUnit.Core; | ||||||
|
|
||||||
| namespace Refitter.Tests.Examples; | ||||||
|
|
||||||
| public class OneOfDiscriminatorTests | ||||||
| { | ||||||
| private const string OpenApiSpec = @" | ||||||
| openapi: 3.0.1 | ||||||
| info: | ||||||
| title: Test | ||||||
| version: v1 | ||||||
| paths: | ||||||
| /api/identityproviders: | ||||||
| get: | ||||||
| operationId: GetIdentityProvider | ||||||
| responses: | ||||||
| '200': | ||||||
| description: Success | ||||||
| content: | ||||||
| application/json: | ||||||
| schema: | ||||||
| $ref: '#/components/schemas/IdentityProviderResponse' | ||||||
| components: | ||||||
| schemas: | ||||||
| IdentityProviderResponse: | ||||||
| type: object | ||||||
| properties: | ||||||
| identityProvider: | ||||||
| $ref: '#/components/schemas/IdentityProvider' | ||||||
| identityProviders: | ||||||
| type: array | ||||||
| items: | ||||||
| $ref: '#/components/schemas/IdentityProvider' | ||||||
| IdentityProvider: | ||||||
| oneOf: | ||||||
| - $ref: '#/components/schemas/AppleIdentityProvider' | ||||||
| - $ref: '#/components/schemas/FacebookIdentityProvider' | ||||||
| discriminator: | ||||||
| propertyName: type | ||||||
| mapping: | ||||||
| Apple: '#/components/schemas/AppleIdentityProvider' | ||||||
| Facebook: '#/components/schemas/FacebookIdentityProvider' | ||||||
| AppleIdentityProvider: | ||||||
| type: object | ||||||
| properties: | ||||||
| type: | ||||||
| type: string | ||||||
| bundleId: | ||||||
| type: string | ||||||
| FacebookIdentityProvider: | ||||||
| type: object | ||||||
| properties: | ||||||
| type: | ||||||
| type: string | ||||||
| appId: | ||||||
| type: string | ||||||
| "; | ||||||
|
|
||||||
| private const string AnyOfOpenApiSpec = @" | ||||||
| openapi: 3.0.1 | ||||||
| info: | ||||||
| title: Test | ||||||
| version: v1 | ||||||
| paths: | ||||||
| /api/notifications: | ||||||
| get: | ||||||
| operationId: GetNotification | ||||||
| responses: | ||||||
| '200': | ||||||
| description: Success | ||||||
| content: | ||||||
| application/json: | ||||||
| schema: | ||||||
| $ref: '#/components/schemas/NotificationResponse' | ||||||
| components: | ||||||
| schemas: | ||||||
| NotificationResponse: | ||||||
| type: object | ||||||
| properties: | ||||||
| notification: | ||||||
| $ref: '#/components/schemas/Notification' | ||||||
| notifications: | ||||||
| type: array | ||||||
| items: | ||||||
| $ref: '#/components/schemas/Notification' | ||||||
| Notification: | ||||||
| anyOf: | ||||||
| - $ref: '#/components/schemas/EmailNotification' | ||||||
| - $ref: '#/components/schemas/SmsNotification' | ||||||
| discriminator: | ||||||
| propertyName: channel | ||||||
| mapping: | ||||||
| Email: '#/components/schemas/EmailNotification' | ||||||
| Sms: '#/components/schemas/SmsNotification' | ||||||
| EmailNotification: | ||||||
| type: object | ||||||
| properties: | ||||||
| channel: | ||||||
| type: string | ||||||
| emailAddress: | ||||||
| type: string | ||||||
| SmsNotification: | ||||||
| type: object | ||||||
| properties: | ||||||
| channel: | ||||||
| type: string | ||||||
| phoneNumber: | ||||||
| type: string | ||||||
| "; | ||||||
|
|
||||||
| [Test] | ||||||
| public async Task Can_Generate_Code() | ||||||
| { | ||||||
| string generatedCode = await GenerateCode(); | ||||||
| generatedCode.Should().NotBeNullOrWhiteSpace(); | ||||||
| } | ||||||
|
|
||||||
| [Test] | ||||||
| public async Task Generates_Base_Type_For_OneOf_With_Discriminator() | ||||||
| { | ||||||
| string generatedCode = await GenerateCode(); | ||||||
| generatedCode.Should().Contain("class IdentityProvider"); | ||||||
|
||||||
| generatedCode.Should().Contain("class IdentityProvider"); | |
| generatedCode.Should().Contain("class IdentityProvider "); |
Copilot
AI
Mar 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion generatedCode.Should().Contain("class Notification") at line 207 is a weak test because it will also match class NotificationResponse (which is always generated). A more precise assertion should distinguish the base type Notification from the response wrapper, e.g. checking for "class Notification\n" or a partial class declaration pattern.
| generatedCode.Should().Contain("class Notification"); | |
| generatedCode.Should().Contain("class Notification\n"); |
Copilot
AI
Mar 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no test coverage for the combination of TrimUnusedSchema = true with oneOf/anyOf + discriminator. The SchemaCleaner runs before ConvertOneOfWithDiscriminatorToAllOf() (in RefitGenerator.CreateAsync vs CSharpClientGeneratorFactory.Create()), which means subtypes are preserved by the cleaner (since it enumerates OneOf/AnyOf entries unconditionally), and the transformation runs afterwards. However, this interaction is not tested. Adding a test with TrimUnusedSchema = true would improve confidence that these two features compose correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
ConvertOneOfWithDiscriminatorToAllOfmethod handlesanyOf+ discriminator (line 84 concatenatesschema.AnyOf), but there is no test case for theanyOfwith discriminator pattern. Given the test suite has comprehensive tests foroneOf+ discriminator, the analogousanyOfpath in the implementation is untested. If NSwag handlesanyOfdifferently fromoneOfduring code generation, this path could silently produce incorrect output.