Open isinyaaa opened 8 months ago
Thanks for raising this @isinyaaa
The get_field_deserializers
, serialize
and create_from_discriminator_value
are methods generated in all models generated by Kiota so that they may used by the relevant serializers to infer the type/property information needed a runtime.
https://learn.microsoft.com/en-us/openapi/kiota/models#field-deserializers
Any chance you can confirm if there's an issue the existing members cause in your scenario?
Thanks for reverting back @andrueastman !
Any chance you can confirm if there's an issue the existing members cause in your scenario?
I cannot find any and the reproducer shared by @isinyaaa shows that the duplication of the fields is happening even on a minimal example.
Thanks for raising this @isinyaaa
The
get_field_deserializers
,serialize
andcreate_from_discriminator_value
are methods generated in all models generated by Kiota so that they may used by the relevant serializers to infer the type/property information needed a runtime.https://learn.microsoft.com/en-us/openapi/kiota/models#field-deserializers
Any chance you can confirm if there's an issue the existing members cause in your scenario?
At least on the Python case, I can't find any problems, but I'm not sure on the patterns used for other languages that might e.g. use those unnecessary fields in some situation and fail to generate a valid object upon serialization.
Sorry. I suspect that I may have not got the gist of the issue correctly.
Just to confirm, the issue here is not with the method members i.e. get_field_deserializers, serialize and create_from_discriminator_value but that the generated model has self.a, self.aor_b_a, self.aor_b_a0, self.aor_b_b, self.aor_b_b0 and self.b when we should ideally only expect self.a and self.b. Yes?
That's correct.
Just chiming in here
I believe the expected result given this input OpenAPI description should be
from __future__ import annotations
from dataclasses import dataclass, field
from kiota_abstractions.serialization import Parsable, ParseNode, SerializationWriter
from kiota_abstractions.serialization.composed_type_wrapper import ComposedTypeWrapper
from typing import Any, Callable, Dict, List, Optional, TYPE_CHECKING, Union
if TYPE_CHECKING:
from .a import A
from .b import B
@dataclass
class AorB(ComposedTypeWrapper, Parsable):
"""
Composed type wrapper for classes A, B
"""
@staticmethod
def create_from_discriminator_value(parse_node: Optional[ParseNode] = None) -> AorB:
"""
Creates a new instance of the appropriate class based on discriminator value
param parse_node: The parse node to use to read the discriminator value and create the object
Returns: AorB
"""
if not parse_node:
raise TypeError("parse_node cannot be null.")
try:
mapping_value = parse_node.get_child_node("type").get_str_value()
except AttributeError:
mapping_value = None
result = AorB()
if mapping_value and mapping_value.casefold() == "a".casefold():
from .a import A
result.a = A()
- elif mapping_value and mapping_value.casefold() == "a".casefold():
- from .a import A
-
- result.aor_b_a = A()
- elif mapping_value and mapping_value.casefold() == "a".casefold():
- from .a import A
-
- result.aor_b_a0 = A()
- elif mapping_value and mapping_value.casefold() == "b".casefold():
- from .b import B
-
- result.aor_b_b = B()
- elif mapping_value and mapping_value.casefold() == "b".casefold():
- from .b import B
-
- result.aor_b_b0 = B()
elif mapping_value and mapping_value.casefold() == "b".casefold():
from .b import B
result.b = B()
return result
def get_field_deserializers(self,) -> Dict[str, Callable[[ParseNode], None]]:
"""
The deserialization information for the current model
Returns: Dict[str, Callable[[ParseNode], None]]
"""
from .a import A
from .b import B
if self.a:
return self.a.get_field_deserializers()
- if self.aor_b_a:
- return self.aor_b_a.get_field_deserializers()
- if self.aor_b_a0:
- return self.aor_b_a0.get_field_deserializers()
- if self.aor_b_b:
- return self.aor_b_b.get_field_deserializers()
- if self.aor_b_b0:
- return self.aor_b_b0.get_field_deserializers()
if self.b:
return self.b.get_field_deserializers()
return {}
def serialize(self,writer: SerializationWriter) -> None:
"""
Serializes information the current object
param writer: Serialization writer to use to serialize this model
Returns: None
"""
if not writer:
raise TypeError("writer cannot be null.")
if self.a:
writer.write_object_value(None, self.a)
- elif self.aor_b_a:
- writer.write_object_value(None, self.aor_b_a)
- elif self.aor_b_a0:
- writer.write_object_value(None, self.aor_b_a0)
- elif self.aor_b_b:
- writer.write_object_value(None, self.aor_b_b)
- elif self.aor_b_b0:
- writer.write_object_value(None, self.aor_b_b0)
elif self.b:
writer.write_object_value(None, self.b)
One first step to understand better where this issue is coming from would be to compare the result across languages to see whether we get this duplication consistently, or simply for Python. @isinyaaa Would you mind running the generation across languages (except TypeScript/Ruby/Swift) and reporting the behaviour here please?
One first step to understand better where this issue is coming from would be to compare the result across languages to see whether we get this duplication consistently, or simply for Python. @isinyaaa Would you mind running the generation across languages (except TypeScript/Ruby/Swift) and reporting the behaviour here please?
Here you go:
Thank you for the additional detailed information. Next step would be to understand better where the problem is coming from, would you mind putting together a draft PR with a test case similar to this one but with your repro instead.
Then debugging that unit test (we need to make sure it calls into the refiners as well), it'd be nice to set a breakpoint in this method to understand if when we get here we have two or more entries. If more than two, the issue is in the main builder (I'll provide more information once we have the findings) If two, the issue is most likely in this method.
I'm making some progress over this one.
Key takeaways:
oneOf
+ discriminator appears in the requesetBody
@isinyaaa @andreaTP can you please try with the latest preview from yesterday and confirm whether you observe the problem? We've made significant improvements to the handling of allof edge scenarios with #4668 and #4381
Checked and the problem is still present.
I have a simple model that can of two types: A or B, like what's shown in the spec below.
When I try to generate code (tested with both Python and Go), I get some superfluous values and unnecessary checks. e.g. in Python:
Ideally, the A or B type should simply correspond to something like
that is, without any superfluous fields.