search

AmateurECE / redfish-codegen

Implementation of DMTF's Redfish data models, generated from their OpenAPI documents.
Apache License 2.0
4 stars 2 forks source link

Partial deserialization results from redfish-generator #1

Open pmundt opened 8 months ago

pmundt commented 8 months ago

I am presently working on a number of redfish related projects in Rust, including a client/CLI tool, DB persistence/graph modeling, etc. and trying to reuse the generated models. The one snag I hit was with single-entry ID fields (e.g., those containing only @odata.id) just not showing up on deserialization, which I eventually tracked down to the logic that skips deserialization of immutable properties in redfish-generator/src/main/java/com/twardyece/dmtf/policies/ODataPropertyPolicy.java and redfish-generator/src/main/resources/templates/model.mustache.

As I'm not entirely sure what the motivation for skipping the deserialization for these properties was in the first place, it's unclear what the preferred resolution would be. The option I'm leaning towards is to add a feature flag for redfish-codegen that can control the deserialization behaviour. The alternative would be to just rip those parts out and have it deserialize everything all the time, but I assume this is undesired as there was obviously some amount of effort that went in to getting this behaviour in the first place. Would a feature flag be a reasonable compromise?

AmateurECE commented 7 months ago

Please forgive me, I have been engaged with other personal issues for a while so I did not see this issue until now! Thanks very much for creating this issue and doing what I can only assume is a significant amount of debug legwork to understand the issue. I initially ignored these parameters in the deserialization, but I'm realizing now that I may have misinterpreted the specification:

The service shall ignore OData annotations in the request body, such as the resource identifier, type, and ETag properties, except for the conditions listed below. If the update request only contains OData annotations, the service should return the HTTP 400 Bad Request status code with the NoOperation message from the Base Message Registry, except for the conditions listed below:

• Writable reference properties. • In deep operations when specifying subordinate resources.

I am aligned with the suggestion to add a feature flag for the code generation to control the deserialization behavior, but I am willing to explore ripping this out entirely as well. The intent there was not to require clients to have to populate OData annotations for all requests. I suppose this could be done by wrapping those properties in Option, but I might want to look further into the implications of that change.

AmateurECE commented 7 months ago

Ah, I see! You're likely running into this issue in client code, is that correct? I will admit that I wrote the generator with the use case of easing implementation of services in mind, so I am not surprised that some of the design choices impede the ability to use these models in client code.

That being said, that's a use case that would add significant value to these crates, and I think deserves special attention. Maybe it makes sense to create a "client" feature flag, and this could be the first design change that's enabled by that flag?