feat(redfish): expose standard reset action helpers#114
Conversation
Add typed high-level wrappers for DMTF Redfish reset actions on `ComputerSystem`, `Manager`, `Chassis`, and `PowerSupply`. Also omit absent optional action parameters from generated request bodies. This lets reset(None) send an empty action body rather than ResetType: null, matching Redfish schemas where ResetType may be omitted. Spec references: - ComputerSystem.Reset: schema/redfish-csdl/csdl/ComputerSystem_v1.xml - Manager.Reset: schema/redfish-csdl/csdl/Manager_v1.xml - Manager.ResetToDefaults: schema/redfish-csdl/csdl/Manager_v1.xml - Chassis.Reset: schema/redfish-csdl/csdl/Chassis_v1.xml - PowerSupply.Reset: schema/redfish-csdl/csdl/PowerSupply_v1.xml Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
| ) | ||
| } | ||
| }; | ||
| let serde = Self::gen_action_parameter_serde_annotation(rename, p.required); |
There was a problem hiding this comment.
I have concerns WRT this function. gen_de_struct_field_type can generate , T, Option<T>, Option<Option<T>, Vec<T>, Option<Vec<T>, Option<Option<Vec<T>. gen_action_parameter_serde_annotation only adds "skip_serializing_if = Option::is_none".
Maybe old generated code was not quite correct in some situations however I don't think that new code can correctly handle "null" fields.
I think we need something like:
fn gen_ser_struct_field_serde_annot(
rename: impl ToTokens,
nullable: IsNullable,
required: IsRequired,
) -> TokenStream {
if required.into_inner() && nullable.into_inner() {
quote! { #[serde(rename=#rename, serialize_with="ser_required_nullable")] }
} else if required.into_inner() {
quote! { #[serde(rename=#rename)] }
} else if nullable.into_inner() {
quote! { #[serde(rename=#rename, default, serialize_with="ser_optional_nullable")] }
} else {
quote! { #[serde(rename=#rename, default, skip_serializing_if = "Option::is_none"))] }
}
}
There was a problem hiding this comment.
That's a good point. Functionality wise, the gen_de_struct_field_type decides the type such as the ones you mentioned (Option<T>, Option<Option<T>> etc.). So the nullability is "encoded" by it. The gen_action_parameter_serde_annotation controls if the top-level None should be omitted. One concern is to potentially having a Vec<T> field annotated with skip_serializing_if = "Option::is_none".
The key non-obviousness comes from implicitly coordinating two branching logic based on required. In gen_de_struct_field_type, required generates a non-Option, or an Option that can be serialized to JSON null, which matches the branches in this function.
I added this clarification in comments and added tests to catch regression.
There was a problem hiding this comment.
Maybe we can think of some refactoring to make this coordination obvious?
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
…eration Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
2 participants
Add typed high-level wrappers for DMTF Redfish reset actions on
ComputerSystem,Manager,Chassis, andPowerSupply.Also omit absent optional action parameters from generated request bodies. So
reset(None)sends an empty action body rather thanResetType: null, which matches Redfish schemas where ResetType may be omitted.Spec references: