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 13 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 @@ -3451,18 +3451,26 @@ 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()));

// 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 @@ -3506,7 +3514,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 @@ -54,6 +54,7 @@
import java.util.*;
import java.util.stream.Collectors;

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

public class DefaultCodegenTest {
Expand Down Expand Up @@ -1446,6 +1447,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 @@ -1213,7 +1213,7 @@ public void testDiscriminatorWithMappingIssue14731() throws IOException {
codegen.setHateoas(true);
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
generator.setGeneratorPropertyDefault(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "false");


codegen.setUseOneOfInterfaces(true);
codegen.setLegacyDiscriminatorBehavior(false);
Expand Down Expand Up @@ -1271,6 +1271,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 Expand Up @@ -1970,7 +2019,7 @@ public void useBeanValidationGenerateAnnotationsForRequestBody_issue13932() thro
.assertParameterAnnotations()
.containsWithNameAndAttributes("Min", ImmutableMap.of("value", "2"));
}

@Test
public void shouldHandleSeparatelyInterfaceAndModelAdditionalAnnotations() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
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 @@ -5,14 +5,6 @@
//

import Foundation

// We reverted the change of PetstoreClientAPI to PetstoreClient introduced in https://github.com/OpenAPITools/openapi-generator/pull/9624
// Because it was causing the following issue https://github.com/OpenAPITools/openapi-generator/issues/9953
// If you are affected by this issue, please consider removing the following two lines,
// By setting the option removeMigrationProjectNameClass to true in the generator
@available(*, deprecated, renamed: "PetstoreClientAPI")
public typealias PetstoreClient = PetstoreClientAPI

bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
open class PetstoreClientAPI {
public static var basePath = "http://localhost"
public static var customHeaders: [String: String] = [:]
Expand All @@ -31,8 +23,6 @@ open class RequestBuilder<T> {
public let requiresAuthentication: Bool

/// Optional block to obtain a reference to the request's progress instance when available.
/// With the URLSession http client the request's progress only works on iOS 11.0, macOS 10.13, macCatalyst 13.0, tvOS 11.0, watchOS 4.0.
/// If you need to get the request's progress in older OS versions, please use Alamofire http client.
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
public var onProgressReady: ((Progress) -> Void)?

required public init(method: String, URLString: String, parameters: [String: Any]?, headers: [String: String] = [:], requiresAuthentication: Bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import Foundation

open class Configuration {

// This value is used to configure the date formatter that is used to serialize dates into JSON format.
// You must set it prior to encoding any dates, and it will only be read once.
@available(*, unavailable, message: "To set a different date format, use CodableHelper.dateFormatter instead.")
public static var dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ"
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
/// Configures the range of HTTP status codes that will result in a successful response
///
/// If a HTTP status code is outside of this range the response will be interpreted as failed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ open class URLSessionRequestBuilder<T>: RequestBuilder<T> {
- intercept and handle errors like authorization
- retry the request.
*/
@available(*, deprecated, message: "Please override execute() method to intercept and handle errors like authorization or retry the request. Check the Wiki for more info. https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-do-i-implement-bearer-token-authentication-with-urlsession-on-the-swift-api-client")
@available(*, unavailable, message: "Please override execute() method to intercept and handle errors like authorization or retry the request. Check the Wiki for more info. https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-do-i-implement-bearer-token-authentication-with-urlsession-on-the-swift-api-client")
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
public var taskCompletionShouldRetry: ((Data?, URLResponse?, Error?, @escaping (Bool) -> Void) -> Void)?

required public init(method: String, URLString: String, parameters: [String: Any]?, headers: [String: String] = [:], requiresAuthentication: Bool) {
Expand Down Expand Up @@ -92,10 +92,6 @@ open class URLSessionRequestBuilder<T>: RequestBuilder<T> {

originalRequest.httpMethod = method.rawValue

headers.forEach { key, value in
originalRequest.setValue(value, forHTTPHeaderField: key)
}

bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
buildHeaders().forEach { key, value in
originalRequest.setValue(value, forHTTPHeaderField: key)
}
Expand Down Expand Up @@ -145,32 +141,13 @@ open class URLSessionRequestBuilder<T>: RequestBuilder<T> {
}

let dataTask = urlSession.dataTask(with: request) { data, response, error in

if let taskCompletionShouldRetry = self.taskCompletionShouldRetry {

taskCompletionShouldRetry(data, response, error) { shouldRetry in

if shouldRetry {
cleanupRequest()
self.execute(apiResponseQueue, completion)
} else {
apiResponseQueue.async {
self.processRequestResponse(urlRequest: request, data: data, response: response, error: error, completion: completion)
cleanupRequest()
}
}
}
} else {
apiResponseQueue.async {
self.processRequestResponse(urlRequest: request, data: data, response: response, error: error, completion: completion)
cleanupRequest()
}
apiResponseQueue.async {
self.processRequestResponse(urlRequest: request, data: data, response: response, error: error, completion: completion)
cleanupRequest()
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
}
}

if #available(iOS 11.0, macOS 10.13, macCatalyst 13.0, tvOS 11.0, watchOS 4.0, *) {
onProgressReady?(dataTask.progress)
}
onProgressReady?(dataTask.progress)
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved

taskIdentifier = dataTask.taskIdentifier
challengeHandlerStore[dataTask.taskIdentifier] = taskDidReceiveChallenge
Expand Down Expand Up @@ -218,10 +195,10 @@ open class URLSessionRequestBuilder<T>: RequestBuilder<T> {

open func buildHeaders() -> [String: String] {
var httpHeaders: [String: String] = [:]
for (key, value) in headers {
for (key, value) in PetstoreClientAPI.customHeaders {
httpHeaders[key] = value
}
for (key, value) in PetstoreClientAPI.customHeaders {
for (key, value) in headers {
bernie-schelberg-mywave marked this conversation as resolved.
Show resolved Hide resolved
httpHeaders[key] = value
}
return httpHeaders
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)


Loading