kopium icon indicating copy to clipboard operation
kopium copied to clipboard

Allow integer enums to have correct integer serialization

Open clux opened this issue 2 years ago • 3 comments

  • [x] Add discriminant to integer enums
  • [x] Improve integration test for httproute so that it uses serde logic for the statusCode field
  • [ ] Add serde_repr
  • [ ] Drop #[serde(rename)] attrs on serde_repr enums

Last thing I want to fix before next release.

clux avatar Nov 12 '22 14:11 clux

Well, the test actually works even without repr (surprisingly), but turns out the enum isn't actually used in the schema because kopium just infers i64 🙃

#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct HTTPRouteRulesFiltersRequestRedirect {
    ...
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "statusCode")]
    pub status_code: Option<i64>,
}

#[derive(Serialize, Deserialize, Clone, Debug)]
pub enum HTTPRouteRulesFiltersRequestRedirectStatusCode {
    #[serde(rename = "301")]
    r#_301 = 301,
    #[serde(rename = "302")]
    r#_302 = 302,
}

clux avatar Nov 12 '22 15:11 clux

I think we actually just look at the type in the schema:

statusCode:
    default: 302
    description: "StatusCode is the HTTP status
      code to be used in response. \n Support: Core
      \n Note that values may be added to this enum,
      implementations must ensure that unknown values
      will not cause a crash. \n Unknown values
      here must result in the implementation setting
      the Accepted Condition for the Route to `status:
      False`, with a Reason of `UnsupportedValue`."
    enum:
    - 301
    - 302
    type: integer

and see, "oh, yeah, integer" and then separately recurse down to find enum structs (which then generates the struct - which isn't used). I think this is a reflection of #101 that enums are not parsed at the right level. I'll try to have another look at this tomorrow.

clux avatar Nov 12 '22 15:11 clux

Was hoping to have time to fix this properly, but don't want to block the release for it anymore. Doing this properly needs a bigger refactor of the analyzer for enums, and utimately there's a lot of great improvements in main already.

clux avatar Nov 18 '22 03:11 clux