-
Notifications
You must be signed in to change notification settings - Fork 96
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
OpenAPI 3.X specs with nullable "$refs" create unnecessary interfaces that break both Java/Kotlin Models at Runtime #1667
Comments
That's because Ok, could you compare generated code by standard generator with property |
I did see that in the internal code and noticed the same that it is not externalized in gradle. I'll give it a shot. Given the case that I do have oneof with real options (not nullable) it's not clear that openapi generator upstream even supports it fully? Not sure how the subtype is determined if the schema doesn't have a field to trigger off of. |
It’s good to check this with the standard generator to understand whether this bug is there or not. If not, then you can quickly fix it in our generator. I added a draft PR for integration: micronaut-projects/micronaut-gradle-plugin#1021 |
Hm, I only see useOneOfInterfaces on the java config options. I tried with kotlin just in case, maybe I did it wrong but no changes 🤷🏼 Java does the following, not actually an interface type. @JsonPropertyOrder({
ShoppingNotesDTO.JSON_PROPERTY_MESSAGE,
ShoppingNotesDTO.JSON_PROPERTY_ICON_URL
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2024-08-02T09:55:16.156464-04:00[America/Detroit]", comments = "Generator version: 7.7.0")
public class ShoppingNotesDTO implements FavorDTOShoppingNotes { and @JsonDeserialize(using = FavorDTOShoppingNotes.FavorDTOShoppingNotesDeserializer.class)
@JsonSerialize(using = FavorDTOShoppingNotes.FavorDTOShoppingNotesSerializer.class)
public class FavorDTOShoppingNotes extends AbstractOpenApiSchema { Kotlin has data class FavorDTOShoppingNotes(var actualInstance: Any? = null) {
class CustomTypeAdapterFactory : TypeAdapterFactory {
override fun <T> create(gson: Gson, type: TypeToken<T>): TypeAdapter<T>? {
if (!FavorDTOShoppingNotes::class.java.isAssignableFrom(type.rawType)) {
return null // this class only serializes 'FavorDTOShoppingNotes' and its subtypes
}
val elementAdapter = gson.getAdapter(JsonElement::class.java)
val adapterShoppingNotesDTO = gson.getDelegateAdapter(this, TypeToken.get(ShoppingNotesDTO::class.java))
@Suppress("UNCHECKED_CAST")
return object : TypeAdapter<FavorDTOShoppingNotes?>() {
@Throws(IOException::class)
override fun write(out: JsonWriter,value: FavorDTOShoppingNotes?) {
if (value?.actualInstance == null) {
elementAdapter.write(out, null)
return
}
// check if the actual instance is of the type `ShoppingNotesDTO`
if (value.actualInstance is ShoppingNotesDTO) {
val element = adapterShoppingNotesDTO.toJsonTree(value.actualInstance as ShoppingNotesDTO?)
elementAdapter.write(out, element)
return
}
throw IOException("Failed to serialize as the type doesn't match oneOf schemas: ShoppingNotesDTO")
} and the subtype data class ShoppingNotesDTO (
@field:JsonProperty("message")
val message: kotlin.String? = null,
@field:JsonProperty("icon_url")
val iconUrl: kotlin.String? = null
) {
} |
Based on the code, it looks like that subtype property is set via an openapi extension Also updated original description with more info. |
try to generate it with java generator. We need to see what will be generated with your swagger file |
Ok, I see what were generated. Could you give me a sample swagger file? Or all what I need just add this code ?: shopping_notes:
nullable: true
oneOf:
-
$ref: '#/components/schemas/ShoppingNotesDTO' Because now I'm confused and don't see full image |
Right, but here it is again. openapi: 3.0.0
info:
title: 'Order API'
version: v6
servers:
-
url: 'https://{environment}.com/api/{apiVersion}'
variables:
environment:
enum:
- api-dev.test.com
default: api.dev.test.com
apiVersion:
enum:
- v1
- v2
- v3
- v4
- v5
- v6
- v7
default: v6
paths:
'/orders/{id}':
get:
tags:
- Order
operationId: getOrderById
parameters:
-
$ref: '#/components/parameters/OrderId'
responses:
'200':
description: OK
content:
application/json:
schema:
$ref: '#/components/schemas/OrderDTO'
components:
parameters:
OrderId:
name: id
in: path
description: 'The ID'
required: true
schema:
type: string
schemas:
OrderDTO:
required:
- id
properties:
id:
type: string
shopping_notes:
nullable: true
oneOf:
- $ref: '#/components/schemas/ShoppingNotesDTO'
ShoppingNotesDTO:
properties:
message:
type: string
icon_url:
type: string
type: object Without Interfaces
public class OrderDTO {
@javax.annotation.Nullable
@JsonIgnore
public OrderDTOShoppingNotes getShoppingNotes() {
return shoppingNotes.orElse(null);
}
@JsonDeserialize(using = OrderDTOShoppingNotes.OrderDTOShoppingNotesDeserializer.class)
@JsonSerialize(using = OrderDTOShoppingNotes.OrderDTOShoppingNotesSerializer.class)
public class OrderDTOShoppingNotes extends AbstractOpenApiSchema {
private static final Logger log = Logger.getLogger(OrderDTOShoppingNotes.class.getName());
@Override
public OrderDTOShoppingNotes deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException, JsonProcessingException {
JsonNode tree = jp.readValueAsTree();
Object deserialized = null;
boolean typeCoercion = ctxt.isEnabled(MapperFeature.ALLOW_COERCION_OF_SCALARS);
int match = 0;
JsonToken token = tree.traverse(jp.getCodec()).nextToken();
// deserialize ShoppingNotesDTO
try {
boolean attemptParsing = true;
// ensure that we respect type coercion as set on the client ObjectMapper
if (ShoppingNotesDTO.class.equals(Integer.class) || ShoppingNotesDTO.class.equals(Long.class) || ShoppingNotesDTO.class.equals(Float.class) || ShoppingNotesDTO.class.equals(Double.class) || ShoppingNotesDTO.class.equals(Boolean.class) || ShoppingNotesDTO.class.equals(String.class)) {
attemptParsing = typeCoercion;
if (!attemptParsing) {
attemptParsing |= ((ShoppingNotesDTO.class.equals(Integer.class) || ShoppingNotesDTO.class.equals(Long.class)) && token == JsonToken.VALUE_NUMBER_INT);
attemptParsing |= ((ShoppingNotesDTO.class.equals(Float.class) || ShoppingNotesDTO.class.equals(Double.class)) && token == JsonToken.VALUE_NUMBER_FLOAT);
attemptParsing |= (ShoppingNotesDTO.class.equals(Boolean.class) && (token == JsonToken.VALUE_FALSE || token == JsonToken.VALUE_TRUE));
attemptParsing |= (ShoppingNotesDTO.class.equals(String.class) && token == JsonToken.VALUE_STRING);
attemptParsing |= (token == JsonToken.VALUE_NULL);
}
}
if (attemptParsing) {
deserialized = tree.traverse(jp.getCodec()).readValueAs(ShoppingNotesDTO.class);
// TODO: there is no validation against JSON schema constraints
// (min, max, enum, pattern...), this does not perform a strict JSON
// validation, which means the 'match' count may be higher than it should be.
match++;
log.log(Level.FINER, "Input data matches schema 'ShoppingNotesDTO'");
}
} catch (Exception e) {
// deserialization failed, continue
log.log(Level.FINER, "Input data does not match schema 'ShoppingNotesDTO'", e);
}
if (match == 1) {
OrderDTOShoppingNotes ret = new OrderDTOShoppingNotes();
ret.setActualInstance(deserialized);
return ret;
}
throw new IOException(String.format("Failed deserialization for OrderDTOShoppingNotes: %d classes match result, expected 1", match));
}
@JsonPropertyOrder({
ShoppingNotesDTO.JSON_PROPERTY_MESSAGE,
ShoppingNotesDTO.JSON_PROPERTY_ICON_URL
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2024-08-02T12:54:55.199028-04:00[America/Detroit]", comments = "Generator version: 7.7.0")
public class ShoppingNotesDTO {
public static final String JSON_PROPERTY_MESSAGE = "message";
private String message;
public static final String JSON_PROPERTY_ICON_URL = "icon_url";
private String iconUrl; With Interfaces
@JsonDeserialize(using = OrderDTOShoppingNotes.OrderDTOShoppingNotesDeserializer.class)
@JsonSerialize(using = OrderDTOShoppingNotes.OrderDTOShoppingNotesSerializer.class)
public class OrderDTOShoppingNotes extends AbstractOpenApiSchema {
private static final Logger log = Logger.getLogger(OrderDTOShoppingNotes.class.getName());
@Override
public OrderDTOShoppingNotes deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException, JsonProcessingException {
JsonNode tree = jp.readValueAsTree();
Object deserialized = null;
boolean typeCoercion = ctxt.isEnabled(MapperFeature.ALLOW_COERCION_OF_SCALARS);
int match = 0;
JsonToken token = tree.traverse(jp.getCodec()).nextToken();
// deserialize ShoppingNotesDTO
try {
boolean attemptParsing = true;
// ensure that we respect type coercion as set on the client ObjectMapper
if (ShoppingNotesDTO.class.equals(Integer.class) || ShoppingNotesDTO.class.equals(Long.class) || ShoppingNotesDTO.class.equals(Float.class) || ShoppingNotesDTO.class.equals(Double.class) || ShoppingNotesDTO.class.equals(Boolean.class) || ShoppingNotesDTO.class.equals(String.class)) {
attemptParsing = typeCoercion;
if (!attemptParsing) {
attemptParsing |= ((ShoppingNotesDTO.class.equals(Integer.class) || ShoppingNotesDTO.class.equals(Long.class)) && token == JsonToken.VALUE_NUMBER_INT);
attemptParsing |= ((ShoppingNotesDTO.class.equals(Float.class) || ShoppingNotesDTO.class.equals(Double.class)) && token == JsonToken.VALUE_NUMBER_FLOAT);
attemptParsing |= (ShoppingNotesDTO.class.equals(Boolean.class) && (token == JsonToken.VALUE_FALSE || token == JsonToken.VALUE_TRUE));
attemptParsing |= (ShoppingNotesDTO.class.equals(String.class) && token == JsonToken.VALUE_STRING);
attemptParsing |= (token == JsonToken.VALUE_NULL);
}
}
if (attemptParsing) {
deserialized = tree.traverse(jp.getCodec()).readValueAs(ShoppingNotesDTO.class);
// TODO: there is no validation against JSON schema constraints
// (min, max, enum, pattern...), this does not perform a strict JSON
// validation, which means the 'match' count may be higher than it should be.
match++;
log.log(Level.FINER, "Input data matches schema 'ShoppingNotesDTO'");
}
} catch (Exception e) {
// deserialization failed, continue
log.log(Level.FINER, "Input data does not match schema 'ShoppingNotesDTO'", e);
}
if (match == 1) {
OrderDTOShoppingNotes ret = new OrderDTOShoppingNotes();
ret.setActualInstance(deserialized);
return ret;
}
throw new IOException(String.format("Failed deserialization for OrderDTOShoppingNotes: %d classes match result, expected 1", match));
}
public class ShoppingNotesDTO implements OrderDTOShoppingNotes { |
Ok, I'll check it. By the way, one question, why are you using oneOf in this case? Why don't you use allOf ? |
Good point this might be a waste to chase, since it could just be an issue with Swagger PHP. I can probably work around this as my example is probably not the proper format for a nullable ref to being with... In OpenAPI Spec 3.1 it does shopping_notes:
oneOf:
-
$ref: '#/components/schemas/ShoppingNotesDTO'
-
type: 'null' And, although micronaut openapi fails to generate the models, Errror
it got far enough I could see no more wrapper interface! @field:Nullable
@field:Valid
@field:JsonProperty(JSON_PROPERTY_SHOPPING_NOTES)
@field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
var shoppingNotes: ShoppingNotesDTO? = null, |
It's obvious why this doesn't work, because the problem is in the interpretation of the oneOf block. Usually it is used in conjunction with a discriminator, or you have several models and you want to declare some kind of common interface for them in order to group them somehow. But in your case, you only have one model in the list, so using oneOf looks strange. This does not negate the fact that there is an bug in the generator, it’s just that the schema does not look like the correct one. |
Agreed |
In the future: if you see problems or have suggestions, then create an issue here - then I will be able to respond as quickly as possible |
Starting with components:
schemas:
FavorDTO:
properties:
shopping_notes:
nullable: true
oneOf:
-
$ref: '#/components/schemas/ShoppingNotesDTO' I got the following without the @Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
interface FavorDTOShoppingNotes {
} @Serdeable
@JsonPropertyOrder(
ShoppingNotesDTO.JSON_PROPERTY_MESSAGE,
ShoppingNotesDTO.JSON_PROPERTY_ICON_URL
)
@Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
data class ShoppingNotesDTO(
@field:Nullable
@field:JsonProperty(JSON_PROPERTY_MESSAGE)
@field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
var message: String? = null,
@field:Nullable
@field:JsonProperty(JSON_PROPERTY_ICON_URL)
@field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
var iconUrl: String? = null,
): FavorDTOShoppingNotes {
companion object {
const val JSON_PROPERTY_MESSAGE = "message"
const val JSON_PROPERTY_ICON_URL = "icon_url"
}
} |
Whoops I missed something here. Although this compiles the toplevel schema, in my cases In FavorDTO class @field:Nullable
@field:Valid
@field:JsonProperty(JSON_PROPERTY_SHOPPING_NOTES)
@field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
var shoppingNotes: FavorDTOShoppingNotes? = null, So therefore it still fails at runtime with
@altro3 is this a new issue or is there another option (that interface one) I can modify if exposed now? |
@scprek yeah... it is somethening new. Could you create new issue with reproducer? |
Created #1679 I'm not entirely sure how the upstream works as they seem to use a nested class
|
Expected Behavior
Should not create an interface due to the oneOf. With main openapi generator tool it works
openapi-generator-cli generate -i openapi.yaml -g kotlin -o output/directory --additional-properties=serializationLibrary=jackson
Sub Type
Wrapper
Actual Behaviour
In micronaut's it generates this
Then runtime error of
Steps To Reproduce
Environment Information
Example Application
No response
Version
4.5.1
The text was updated successfully, but these errors were encountered: