Repository navigation
Avoid usage of reflection API in Enum deserialization #840
Description
Activity
Thanks for bringing this up.
The reason why we had to resolve to reflections are because it's a static method, so it can't be part of the ValuedEnum interface like getValue, because the return type varies (based on the enum type, but maybe we could get around that with generics).
And this is a static method because we don't have an instance yet.If you have a better solution, let me know, I agree that reflection is costly and risky.
And this is a static method because we don't have an instance yet.
Right, but this is a design choice, we can make it a class method and have a static way of retrieving an instance ( similar to what
createFromDiscriminatorValuedoes.
Alternatively, we can add an interface at the parent level, but this means that no enum can be serialized without the scope of an object which seems to be a limitation.I think the first option would probably work better here. In fact this is what we're doing in Go I think the history here was that I implemented it in dotnet, patched it in Java, later on implemented it in Go, and never went back to update java 🤦♂️
Is this something you want to take a look at?
For visibility for everyone, here are the changes we're talking about:
- introduce an "ValuedEnumParser" interface (generic, defines a forValue method, functional)
- make the getEnumValue, and getCollectionOfEnumValues method accept that as a second parameter
- update the generation so it passes a lambda to the method as a second argument.
Metadata
Metadata
Labels
Type
Projects
- Status
kiota-java/components/serialization/json/src/main/java/com/microsoft/kiota/serialization/JsonParseNode.java
Line 309 in 033afd1
This is extremely unsafe and it works only because there is a "hidden" contract with the generator to emit the specific
forValuemethod on the enum class.My first reaction would be to make it more similar to a normal "class", and extend the API of
ValuedEnumto cover the deserialization explicitly.Please note that this is going to be a breaking API change for abstractions and I hope we can fix it before GA.
Do you have any prior art on the subject?
Any consideration?