Skip to content
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

deserialization of discriminator models is broken #1394

Closed
cataggar opened this issue Sep 19, 2023 · 0 comments · Fixed by #1414
Closed

deserialization of discriminator models is broken #1394

cataggar opened this issue Sep 19, 2023 · 0 comments · Fixed by #1414

Comments

@cataggar
Copy link
Member

cataggar commented Sep 19, 2023

In the OpenAPI spec, the definitions with discriminator act as a base type and a union. We are modeling the base type fine by using #[serde(flatten)] in the children. The problem is we are not modeling the union. In Rust, we should have both types.

For naming, here are a couple of options.

  1. Add a new type with a Union suffix.
  2. Append Base to the existing base type.

I manually tried out both in the internal PR and I prefer adding a Union type, but open to discussion.

Using specification\vmware\resource-manager\Microsoft.AVS\stable\2022-05-01\vmware.json as example. The relevant models would be:

#[doc = "A vSphere Distributed Resource Scheduler (DRS) placement policy"]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Default)]
pub struct PlacementPolicy {
    #[serde(flatten)]
    pub resource: Resource,
    #[doc = "Abstract placement policy properties"]
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub properties: Option<PlacementPolicyPropertiesUnion>,
}

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type")]
pub enum PlacementPolicyPropertiesUnion {
    VmVm(VmVmPlacementPolicyProperties),
    VmHost(VmHostPlacementPolicyProperties),
}

#[doc = "Abstract placement policy properties"]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct PlacementPolicyProperties {
    #[doc = "Whether the placement policy is enabled or disabled"]
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub state: Option<placement_policy_properties::State>,
    #[doc = "Display name of the placement policy"]
    #[serde(rename = "displayName", default, skip_serializing_if = "Option::is_none")]
    pub display_name: Option<String>,
    #[doc = "The provisioning state"]
    #[serde(rename = "provisioningState", default, skip_serializing_if = "Option::is_none")]
    pub provisioning_state: Option<placement_policy_properties::ProvisioningState>,
}

#[doc = "VM-VM placement policy properties"]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct VmVmPlacementPolicyProperties {
    #[serde(flatten)]
    pub placement_policy_properties: PlacementPolicyProperties,
    #[doc = "Virtual machine members list"]
    #[serde(rename = "vmMembers")]
    pub vm_members: Vec<String>,
    #[doc = "Placement policy affinity type"]
    #[serde(rename = "affinityType")]
    pub affinity_type: AffinityType,
}

#[doc = "VM-Host placement policy properties"]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct VmHostPlacementPolicyProperties {
    #[serde(flatten)]
    pub placement_policy_properties: PlacementPolicyProperties,
    #[doc = "Virtual machine members list"]
    #[serde(rename = "vmMembers")]
    pub vm_members: Vec<String>,
    #[doc = "Host members list"]
    #[serde(rename = "hostMembers")]
    pub host_members: Vec<String>,
    #[doc = "Placement policy affinity type"]
    #[serde(rename = "affinityType")]
    pub affinity_type: AffinityType,
    #[doc = "VM-Host placement policy affinity strength (should/must)"]
    #[serde(rename = "affinityStrength", default, skip_serializing_if = "Option::is_none")]
    pub affinity_strength: Option<AffinityStrength>,
    #[doc = "Placement policy hosts opt-in Azure Hybrid Benefit type"]
    #[serde(rename = "azureHybridBenefitType", default, skip_serializing_if = "Option::is_none")]
    pub azure_hybrid_benefit_type: Option<AzureHybridBenefitType>,
}

I'm not sure what to do about unknown union variants, but may be that can be a future step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant