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

Use enum type for discriminator #13846

Merged
merged 18 commits into from
Jul 4, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -3526,22 +3526,30 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch
return null;
}
CodegenDiscriminator discriminator = new CodegenDiscriminator();
String discPropName = sourceDiscriminator.getPropertyName();
discriminator.setPropertyName(toVarName(discPropName));
String discriminatorPropertyName = sourceDiscriminator.getPropertyName();
discriminator.setPropertyName(toVarName(discriminatorPropertyName));
discriminator.setPropertyBaseName(sourceDiscriminator.getPropertyName());
discriminator.setPropertyGetter(toGetter(discriminator.getPropertyName()));

if (sourceDiscriminator.getExtensions() != null) {
discriminator.setVendorExtensions(sourceDiscriminator.getExtensions());
}

// FIXME: for now, we assume that the discriminator property is String
discriminator.setPropertyType(typeMapping.get("string"));
// FIXME: there are other ways to define the type of the discriminator property (inline
// for example). Handling those scenarios is too complicated for me, I'm leaving it for
// the future..
String propertyType =
Optional.ofNullable(schema.getProperties())
.map(p -> (Schema<?>) p.get(discriminatorPropertyName))
.map(Schema::get$ref)
.map(ModelUtils::getSimpleRef)
.orElseGet(() -> typeMapping.get("string"));
discriminator.setPropertyType(propertyType);

// check to see if the discriminator property is an enum string
if (schema.getProperties() != null &&
schema.getProperties().get(discPropName) instanceof StringSchema) {
StringSchema s = (StringSchema) schema.getProperties().get(discPropName);
schema.getProperties().get(discriminatorPropertyName) instanceof StringSchema) {
StringSchema s = (StringSchema) schema.getProperties().get(discriminatorPropertyName);
if (s.getEnum() != null && !s.getEnum().isEmpty()) { // it's an enum string
discriminator.setIsEnum(true);
}
Expand Down Expand Up @@ -3586,7 +3594,7 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch
}
// if there are composed oneOf/anyOf schemas, add them to this discriminator
if (ModelUtils.isComposedSchema(schema) && !this.getLegacyDiscriminatorBehavior()) {
List<MappedModel> otherDescendants = getOneOfAnyOfDescendants(schemaName, discPropName, (ComposedSchema) schema, openAPI);
List<MappedModel> otherDescendants = getOneOfAnyOfDescendants(schemaName, discriminatorPropertyName, (ComposedSchema) schema, openAPI);
for (MappedModel otherDescendant : otherDescendants) {
if (!uniqueDescendants.contains(otherDescendant)) {
uniqueDescendants.add(otherDescendant);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.stream.Collectors;

import static junit.framework.Assert.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;
import static org.testng.Assert.*;

public class DefaultCodegenTest {
Expand Down Expand Up @@ -1509,6 +1510,12 @@ public void testComposedSchemaOneOfDiscriminatorMap() {
hs.add(new CodegenDiscriminator.MappedModel(mn, mn));
Assert.assertEquals(cm.discriminator.getMappedModels(), hs);

// ref oneOf models with enum property discriminator
modelName = "FruitOneOfEnumMappingDisc";
sc = openAPI.getComponents().getSchemas().get(modelName);
cm = codegen.fromModel(modelName, sc);
assertThat(cm.discriminator.getPropertyType()).isEqualTo("FruitTypeEnum");

// ref oneOf models with discriminator in the grandparent schemas of those oneof models
modelName = "FruitGrandparentDisc";
sc = openAPI.getComponents().getSchemas().get(modelName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,55 @@ public void testDiscriminatorWithoutMappingIssue14731() throws IOException {
assertFileContains(Paths.get(outputPath + "/src/main/java/org/openapitools/model/ChildWithoutMappingBDTO.java"), "@JsonTypeName");
}

@Test
void testOneOfWithEnumDiscriminator() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');
OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/3_0/oneOfDiscriminator.yaml", null, new ParseOptions()).getOpenAPI();

SpringCodegen codegen = new SpringCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(CXFServerFeatures.LOAD_TEST_DATA_FROM_FILE, "true");
codegen.setUseOneOfInterfaces(true);

ClientOptInput input = new ClientOptInput();
input.openAPI(openAPI);
input.config(codegen);

DefaultGenerator generator = new DefaultGenerator();
codegen.setHateoas(true);
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
//generator.setGeneratorPropertyDefault(CodegenConstants.USE_ONEOF_DISCRIMINATOR_LOOKUP, "true");
generator.setGeneratorPropertyDefault(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "false");

codegen.setUseOneOfInterfaces(true);
codegen.setLegacyDiscriminatorBehavior(false);

generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "true");
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");

generator.opts(input).generate();

assertFileContains(
Paths.get(outputPath + "/src/main/java/org/openapitools/model/FruitOneOfEnumMappingDisc.java"),
"public FruitTypeEnum getFruitType();"
);
assertFileContains(
Paths.get(outputPath + "/src/main/java/org/openapitools/model/AppleOneOfEnumMappingDisc.java"),
"private FruitTypeEnum fruitType;",
"public FruitTypeEnum getFruitType() {"
);
assertFileContains(
Paths.get(outputPath + "/src/main/java/org/openapitools/model/BananaOneOfEnumMappingDisc.java"),
"private FruitTypeEnum fruitType;",
"public FruitTypeEnum getFruitType() {"
);
}

@Test
public void testTypeMappings() {
final SpringCodegen codegen = new SpringCodegen();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,38 @@ components:
type: integer
oneOf:
- $ref: '#/components/schemas/FruitType'
FruitTypeEnum:
type: string
enum: [APPLE, BANANA]
FruitOneOfEnumMappingDisc:
type: object
properties:
fruitType:
$ref: "#/components/schemas/FruitTypeEnum"
required:
- fruitType
oneOf:
- $ref: '#/components/schemas/AppleOneOfEnumMappingDisc'
- $ref: '#/components/schemas/BananaOneOfEnumMappingDisc'
discriminator:
propertyName: fruitType
mapping:
Copy link
Contributor

Choose a reason for hiding this comment

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

The mapping to the enum value might cause problems with the auto generated JsonSubType.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually generated a sample Project:

@JsonIgnoreProperties(
  value = "fruitType", // ignore manually set fruitType, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the fruitType to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "fruitType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = AppleOneOfEnumMappingDisc.class, name = "APPLE"),
  @JsonSubTypes.Type(value = AppleOneOfEnumMappingDisc.class, name = "AppleOneOfEnumMappingDisc"),
  @JsonSubTypes.Type(value = BananaOneOfEnumMappingDisc.class, name = "BANANA"),
  @JsonSubTypes.Type(value = BananaOneOfEnumMappingDisc.class, name = "BananaOneOfEnumMappingDisc")
})

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2022-11-03T09:30:55.032940+01:00[Europe/Berlin]")
public interface FruitOneOfEnumMappingDisc {
    public FruitTypeEnum getFruitType();
}

@welshm Isn't there another issue / pr dealing with duplicated JsonSubTypes?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bernie-schelberg-mywave Could you please add a enum based discriminator to Ione of the generated sample projects (for example bin/configs/spring-boot-oneof.yaml)?

Choose a reason for hiding this comment

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

I actually generated a sample Project:

@JsonIgnoreProperties(
  value = "fruitType", // ignore manually set fruitType, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the fruitType to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "fruitType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = AppleOneOfEnumMappingDisc.class, name = "APPLE"),
  @JsonSubTypes.Type(value = AppleOneOfEnumMappingDisc.class, name = "AppleOneOfEnumMappingDisc"),
  @JsonSubTypes.Type(value = BananaOneOfEnumMappingDisc.class, name = "BANANA"),
  @JsonSubTypes.Type(value = BananaOneOfEnumMappingDisc.class, name = "BananaOneOfEnumMappingDisc")
})

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2022-11-03T09:30:55.032940+01:00[Europe/Berlin]")
public interface FruitOneOfEnumMappingDisc {
    public FruitTypeEnum getFruitType();
}

@welshm Isn't there another issue / pr dealing with duplicated JsonSubTypes?

I'm running openapi-generator version 6.2.0 and the above is the current behaviour when you add a mapping to the discriminator. Both the derived classes and the mapping values are added as JsonSubTypes.

The documentations seems to indicate that this is the expected behaviour.

The mapping in the discriminator includes any descendent schemas that allOf inherit from self, any oneOf schemas, any anyOf schemas, any x-discriminator-values, and the discriminator mapping schemas in the OAS document AND Codegen validates that oneOf and anyOf schemas contain the required discriminator and throws an error if the discriminator is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cachescrubber Yes, there's another project for duplicated JsonSubTypes: #13815

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (added enum based discriminator to samples)

APPLE: '#/components/schemas/AppleOneOfEnumMappingDisc'
BANANA: '#/components/schemas/BananaOneOfEnumMappingDisc'
AppleOneOfEnumMappingDisc:
type: object
required:
- seeds
properties:
seeds:
type: integer
BananaOneOfEnumMappingDisc:
type: object
required:
- length
properties:
length:
type: integer
FruitGrandparentDisc:
oneOf:
- $ref: '#/components/schemas/AppleGrandparentDisc'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,38 @@ components:
type: string
allOf:
- $ref: '#/components/schemas/Pizza'
FruitType:
type: string
enum: [APPLE, BANANA]
Fruit:
type: object
properties:
fruitType:
$ref: "#/components/schemas/FruitType"
required:
- fruitType
oneOf:
- $ref: '#/components/schemas/Apple'
- $ref: '#/components/schemas/Banana'
discriminator:
propertyName: fruitType
mapping:
APPLE: '#/components/schemas/Apple'
BANANA: '#/components/schemas/Banana'
Apple:
type: object
required:
- seeds
properties:
seeds:
type: integer
Banana:
type: object
required:
- length
properties:
length:
type: integer

requestBodies:
Foo:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
README.md
analysis_options.yaml
doc/Addressable.md
doc/Apple.md
doc/Banana.md
doc/Bar.md
doc/BarApi.md
doc/BarCreate.md
Expand All @@ -14,6 +16,8 @@ doc/Foo.md
doc/FooApi.md
doc/FooRef.md
doc/FooRefOrValue.md
doc/Fruit.md
doc/FruitType.md
doc/Pasta.md
doc/Pizza.md
doc/PizzaSpeziale.md
Expand All @@ -29,6 +33,8 @@ lib/src/auth/bearer_auth.dart
lib/src/auth/oauth.dart
lib/src/date_serializer.dart
lib/src/model/addressable.dart
lib/src/model/apple.dart
lib/src/model/banana.dart
lib/src/model/bar.dart
lib/src/model/bar_create.dart
lib/src/model/bar_ref.dart
Expand All @@ -40,6 +46,8 @@ lib/src/model/extensible.dart
lib/src/model/foo.dart
lib/src/model/foo_ref.dart
lib/src/model/foo_ref_or_value.dart
lib/src/model/fruit.dart
lib/src/model/fruit_type.dart
lib/src/model/pasta.dart
lib/src/model/pizza.dart
lib/src/model/pizza_speziale.dart
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Class | Method | HTTP request | Description
## Documentation For Models

- [Addressable](doc/Addressable.md)
- [Apple](doc/Apple.md)
- [Banana](doc/Banana.md)
- [Bar](doc/Bar.md)
- [BarCreate](doc/BarCreate.md)
- [BarRef](doc/BarRef.md)
Expand All @@ -83,6 +85,8 @@ Class | Method | HTTP request | Description
- [Foo](doc/Foo.md)
- [FooRef](doc/FooRef.md)
- [FooRefOrValue](doc/FooRefOrValue.md)
- [Fruit](doc/Fruit.md)
- [FruitType](doc/FruitType.md)
- [Pasta](doc/Pasta.md)
- [Pizza](doc/Pizza.md)
- [PizzaSpeziale](doc/PizzaSpeziale.md)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# openapi.model.Apple

## Load the model package
```dart
import 'package:openapi/api.dart';
```

## Properties
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**seeds** | **int** | |

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)


Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# openapi.model.Banana

## Load the model package
```dart
import 'package:openapi/api.dart';
```

## Properties
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**length** | **int** | |

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)


Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# openapi.model.Fruit

## Load the model package
```dart
import 'package:openapi/api.dart';
```

## Properties
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**fruitType** | [**FruitType**](FruitType.md) | |
**seeds** | **int** | |
**length** | **int** | |

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)


Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# openapi.model.FruitType

## Load the model package
```dart
import 'package:openapi/api.dart';
```

## Properties
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)


Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export 'package:openapi/src/api/bar_api.dart';
export 'package:openapi/src/api/foo_api.dart';

export 'package:openapi/src/model/addressable.dart';
export 'package:openapi/src/model/apple.dart';
export 'package:openapi/src/model/banana.dart';
export 'package:openapi/src/model/bar.dart';
export 'package:openapi/src/model/bar_create.dart';
export 'package:openapi/src/model/bar_ref.dart';
Expand All @@ -23,6 +25,8 @@ export 'package:openapi/src/model/extensible.dart';
export 'package:openapi/src/model/foo.dart';
export 'package:openapi/src/model/foo_ref.dart';
export 'package:openapi/src/model/foo_ref_or_value.dart';
export 'package:openapi/src/model/fruit.dart';
export 'package:openapi/src/model/fruit_type.dart';
export 'package:openapi/src/model/pasta.dart';
export 'package:openapi/src/model/pizza.dart';
export 'package:openapi/src/model/pizza_speziale.dart';
Loading
Loading