From d579a526b3cc2e0c36d17fae8df549a03187f177 Mon Sep 17 00:00:00 2001 From: Adam Leventhal Date: Sat, 18 Feb 2023 06:23:29 -0800 Subject: [PATCH] handle redundant types with $ref; handle untagged enum full name conflicts (#200) --- typify-impl/src/convert.rs | 33 ++++++++++++++--- typify-impl/src/enums.rs | 6 +++ typify-impl/src/lib.rs | 11 ++---- typify-impl/src/util.rs | 2 +- typify/tests/schemas/id-or-name.json | 48 +++++++++++++++--------- typify/tests/schemas/id-or-name.rs | 49 +++++++++++++++++++++++++ typify/tests/schemas/various-enums.json | 17 +++++++++ typify/tests/schemas/various-enums.rs | 22 +++++++++++ 8 files changed, 157 insertions(+), 31 deletions(-) diff --git a/typify-impl/src/convert.rs b/typify-impl/src/convert.rs index 1ccb8089..e41e388e 100644 --- a/typify-impl/src/convert.rs +++ b/typify-impl/src/convert.rs @@ -6,7 +6,7 @@ use crate::type_entry::{ EnumTagType, TypeEntry, TypeEntryDetails, TypeEntryEnum, TypeEntryNewtype, TypeEntryStruct, Variant, VariantDetails, }; -use crate::util::{all_mutually_exclusive, none_or_single, recase, Case, StringValidator}; +use crate::util::{all_mutually_exclusive, none_or_single, recase, ref_key, Case, StringValidator}; use log::info; use schemars::schema::{ ArrayValidation, InstanceType, Metadata, ObjectValidation, Schema, SchemaObject, SingleOrVec, @@ -330,6 +330,32 @@ impl TypeSpace { extensions: _, } => self.convert_reference(metadata, reference), + // Accept references that... for some reason... include the type. + // TODO this could be generalized to validate any redundant + // validation here or could be used to compute a new, more + // constrained type. + SchemaObject { + metadata, + instance_type, + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: None, + array: None, + object: None, + reference: Some(reference), + extensions: _, + } => { + let ref_schema = self.definitions.get(ref_key(reference)).unwrap(); + assert!(matches!(ref_schema, Schema::Object(SchemaObject { + instance_type: it, .. + }) if instance_type == it)); + + self.convert_reference(metadata, reference) + } + // Enum of a single, known, non-String type (strings above). SchemaObject { instance_type: Some(SingleOrVec::Single(_)), @@ -910,10 +936,7 @@ impl TypeSpace { metadata: &'a Option>, ref_name: &str, ) -> Result<(TypeEntry, &'a Option>)> { - let key = match ref_name.rfind('/') { - Some(idx) => &ref_name[idx + 1..], - None => ref_name, - }; + let key = ref_key(ref_name); let type_id = self .ref_to_id .get(key) diff --git a/typify-impl/src/enums.rs b/typify-impl/src/enums.rs index e4390779..44cf07c5 100644 --- a/typify-impl/src/enums.rs +++ b/typify-impl/src/enums.rs @@ -743,6 +743,11 @@ impl TypeSpace { } (Some(name), Some(prefix)) => { common_prefix = Some(get_common_prefix(name, prefix)); + // If the common prefix is the whole name, we can't use + // these names. + if common_prefix.as_ref() == Some(name) { + names_from_variants = false; + } } } @@ -764,6 +769,7 @@ impl TypeSpace { } else { format!("Variant{}", idx) }; + assert!(!name.is_empty()); Variant { name, rename: None, diff --git a/typify-impl/src/lib.rs b/typify-impl/src/lib.rs index c4d6573e..11b6be2a 100644 --- a/typify-impl/src/lib.rs +++ b/typify-impl/src/lib.rs @@ -384,12 +384,7 @@ impl TypeSpace { // previous step because each type may create additional types. This // effectively is doing the work of `add_type_with_name` but for a // batch of types. - for (index, (ref_name, schema)) in definitions.into_iter().enumerate() { - let type_name = match ref_name.rfind('/') { - Some(idx) => &ref_name[idx..], - None => &ref_name, - }; - + for (index, (type_name, schema)) in definitions.into_iter().enumerate() { info!( "converting type: {} with schema {}", type_name, @@ -399,9 +394,9 @@ impl TypeSpace { // Check for manually replaced types. Proceed with type conversion // if there is none; use the specified type if there is. let type_id = TypeId(base_id + index as u64); - let check_name = sanitize(type_name, Case::Pascal); + let check_name = sanitize(&type_name, Case::Pascal); match self.settings.replace.get(&check_name) { - None => self.convert_ref_type(type_name, schema, type_id)?, + None => self.convert_ref_type(&type_name, schema, type_id)?, Some(replace_type) => { let type_entry = TypeEntry::new_native( diff --git a/typify-impl/src/util.rs b/typify-impl/src/util.rs index c6efe10c..08c6f0b6 100644 --- a/typify-impl/src/util.rs +++ b/typify-impl/src/util.rs @@ -483,7 +483,7 @@ pub(crate) fn constant_string_value(schema: &Schema) -> Option<&str> { } } -pub(crate) fn ref_key(ref_name: &String) -> &str { +pub(crate) fn ref_key(ref_name: &str) -> &str { match ref_name.rfind('/') { Some(idx) => &ref_name[idx + 1..], None => ref_name, diff --git a/typify/tests/schemas/id-or-name.json b/typify/tests/schemas/id-or-name.json index 92438f7a..e2a6d10a 100644 --- a/typify/tests/schemas/id-or-name.json +++ b/typify/tests/schemas/id-or-name.json @@ -1,32 +1,46 @@ { "$schema": "http://json-schema.org/draft-07/schema#", - "title": "IdOrName", - "oneOf": [ - { - "title": "Id", - "allOf": [ + "definitions": { + "IdOrName": { + "oneOf": [ { - "type": "string", - "format": "uuid" - } - ] - }, - { - "title": "Name", - "allOf": [ + "title": "Id", + "allOf": [ + { + "type": "string", + "format": "uuid" + } + ] + }, { - "$ref": "#/definitions/Name" + "title": "Name", + "allOf": [ + { + "$ref": "#/definitions/Name" + } + ] } ] - } - ], - "definitions": { + }, "Name": { "title": "A name unique within the parent collection", "description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.", "type": "string", "pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$", "maxLength": 63 + }, + "IdOrNameRedundant": { + "$comment": "tests references that include a redundant type field", + "oneOf": [ + { + "type": "string", + "format": "uuid" + }, + { + "type": "string", + "$ref": "#/definitions/Name" + } + ] } } } \ No newline at end of file diff --git a/typify/tests/schemas/id-or-name.rs b/typify/tests/schemas/id-or-name.rs index 1842f049..5d3af7a3 100644 --- a/typify/tests/schemas/id-or-name.rs +++ b/typify/tests/schemas/id-or-name.rs @@ -49,6 +49,55 @@ impl ToString for IdOrName { } } } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(untagged)] +pub enum IdOrNameRedundant { + Variant0(uuid::Uuid), + Variant1(Name), +} +impl From<&IdOrNameRedundant> for IdOrNameRedundant { + fn from(value: &IdOrNameRedundant) -> Self { + value.clone() + } +} +impl std::str::FromStr for IdOrNameRedundant { + type Err = &'static str; + fn from_str(value: &str) -> Result { + if let Ok(v) = value.parse() { + Ok(Self::Variant0(v)) + } else if let Ok(v) = value.parse() { + Ok(Self::Variant1(v)) + } else { + Err("string conversion failed for all variants") + } + } +} +impl std::convert::TryFrom<&str> for IdOrNameRedundant { + type Error = &'static str; + fn try_from(value: &str) -> Result { + value.parse() + } +} +impl std::convert::TryFrom<&String> for IdOrNameRedundant { + type Error = &'static str; + fn try_from(value: &String) -> Result { + value.parse() + } +} +impl std::convert::TryFrom for IdOrNameRedundant { + type Error = &'static str; + fn try_from(value: String) -> Result { + value.parse() + } +} +impl ToString for IdOrNameRedundant { + fn to_string(&self) -> String { + match self { + Self::Variant0(x) => x.to_string(), + Self::Variant1(x) => x.to_string(), + } + } +} #[doc = "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID."] #[derive(Clone, Debug, Serialize)] pub struct Name(String); diff --git a/typify/tests/schemas/various-enums.json b/typify/tests/schemas/various-enums.json index 5f81dd5c..ecbaebc0 100644 --- a/typify/tests/schemas/various-enums.json +++ b/typify/tests/schemas/various-enums.json @@ -94,6 +94,23 @@ "state", "alternate" ] + }, + "JankNames": { + "anyOf": [ + { + "title": "Animation Specification", + "type": "string" + }, + { + "title": "Animation Specification", + "type": "object", + "maxProperties": 1, + "minProperties": 1, + "additionalProperties": { + "type": "string" + } + } + ] } } } \ No newline at end of file diff --git a/typify/tests/schemas/various-enums.rs b/typify/tests/schemas/various-enums.rs index fcc87d32..39e17cb9 100644 --- a/typify/tests/schemas/various-enums.rs +++ b/typify/tests/schemas/various-enums.rs @@ -55,6 +55,17 @@ impl Default for AlternativeEnum { } } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct AnimationSpecification { + #[serde(flatten)] + pub extra: std::collections::HashMap, +} +impl From<&AnimationSpecification> for AnimationSpecification { + fn from(value: &AnimationSpecification) -> Self { + value.clone() + } +} +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct DiskAttachment { pub alternate: AlternativeEnum, pub state: DiskAttachmentState, @@ -236,6 +247,17 @@ impl ToString for Ipv6Net { } } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(untagged)] +pub enum JankNames { + Variant0(String), + Variant1(AnimationSpecification), +} +impl From<&JankNames> for JankNames { + fn from(value: &JankNames) -> Self { + value.clone() + } +} +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct NullStringEnumWithUnknownFormat(pub Option); impl std::ops::Deref for NullStringEnumWithUnknownFormat { type Target = Option;