From 857fbdf4056d17a33fc31b793d178009f712bf8f Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Tue, 31 Jan 2023 16:28:04 -0800 Subject: [PATCH 1/3] adds changes to allow open content within schema and adds APIs to retrive open content * adds open_content() in IslType as well IslSchema to retrieve open content * adds load_isl_schema() that load a schema content as an intrernal ISL model which can be programmaticlaly manipulated * adds related tests for open content --- .../tests/ion-schema-tests-1-0.rs | 1 - ion-schema/src/constraint.rs | 10 ++ ion-schema/src/isl/isl_constraint.rs | 10 +- ion-schema/src/isl/isl_type.rs | 27 +++++- ion-schema/src/isl/mod.rs | 34 +++++++ ion-schema/src/system.rs | 91 ++++++++++++++++++- ion-schema/src/types.rs | 2 +- 7 files changed, 164 insertions(+), 11 deletions(-) diff --git a/ion-schema-tests-runner/tests/ion-schema-tests-1-0.rs b/ion-schema-tests-runner/tests/ion-schema-tests-1-0.rs index 8695c20..1cc6970 100644 --- a/ion-schema-tests-runner/tests/ion-schema-tests-1-0.rs +++ b/ion-schema-tests-runner/tests/ion-schema-tests-1-0.rs @@ -16,7 +16,6 @@ ion_schema_tests!( "constraints::scale::invalid::scale_must_be_an_integer_or_range__10_", "constraints::scale::invalid::scale_must_be_an_integer_or_range__11_", "constraints::scale::invalid::scale_must_be_an_integer_or_range__04_", - "constraints::unknown_constraint::", "constraints::valid_values::all_types::value_should_be_invalid_for_type_valid_values_all_types__04_", "schema::import::diamond_import::should_be_a_valid_schema", "schema::import::diamond_import::value_should_be_valid_for_type_diamond_import", diff --git a/ion-schema/src/constraint.rs b/ion-schema/src/constraint.rs index 2e8df7e..ba8812a 100644 --- a/ion-schema/src/constraint.rs +++ b/ion-schema/src/constraint.rs @@ -53,6 +53,7 @@ pub enum Constraint { TimestampOffset(TimestampOffsetConstraint), TimestampPrecision(TimestampPrecisionConstraint), Type(TypeConstraint), + Unknown(String, Element), Utf8ByteLength(Utf8ByteLengthConstraint), ValidValues(ValidValuesConstraint), } @@ -334,6 +335,10 @@ impl Constraint { valid_values: valid_values.values().to_owned(), })) } + IslConstraint::Unknown(constraint_name, element) => Ok(Constraint::Unknown( + constraint_name.to_owned(), + element.to_owned(), + )), } } @@ -379,6 +384,11 @@ impl Constraint { utf8_byte_length.validate(value, type_store) } Constraint::ValidValues(valid_values) => valid_values.validate(value, type_store), + Constraint::Unknown(_, _) => { + // No op + // `Unknown` represents open content which can be ignored for validation + Ok(()) + } } } } diff --git a/ion-schema/src/isl/isl_constraint.rs b/ion-schema/src/isl/isl_constraint.rs index 86a0cc6..73ef4e8 100644 --- a/ion-schema/src/isl/isl_constraint.rs +++ b/ion-schema/src/isl/isl_constraint.rs @@ -35,6 +35,7 @@ pub enum IslConstraint { TimestampOffset(IslTimestampOffsetConstraint), TimestampPrecision(Range), Type(IslTypeRef), + Unknown(String, Element), // Unknown constraint is used to store open contents Utf8ByteLength(Range), ValidValues(IslValidValuesConstraint), } @@ -446,12 +447,9 @@ impl IslConstraint { RangeType::NonNegativeInteger, )?)), "valid_values" => Ok(IslConstraint::ValidValues(value.try_into()?)), - _ => Err(invalid_schema_error_raw( - "Type: ".to_owned() - + type_name - + " can not be built as constraint: " - + constraint_name - + " does not exist", + _ => Ok(IslConstraint::Unknown( + constraint_name.to_string(), + value.to_owned(), )), } } diff --git a/ion-schema/src/isl/isl_type.rs b/ion-schema/src/isl/isl_type.rs index 931f660..4566eef 100644 --- a/ion-schema/src/isl/isl_type.rs +++ b/ion-schema/src/isl/isl_type.rs @@ -44,7 +44,15 @@ impl IslType { } /// Verifies if the [IslType] allows open content or not - pub fn open_content(&self) -> bool { + pub fn is_open_content_allowed(&self) -> bool { + match &self { + IslType::Named(named_type) => named_type.is_open_content_allowed(), + IslType::Anonymous(anonymous_type) => anonymous_type.is_open_content_allowed(), + } + } + + /// Provides open content that is there in the type definition + pub fn open_content(&self) -> Vec<(String, Element)> { match &self { IslType::Named(named_type) => named_type.open_content(), IslType::Anonymous(anonymous_type) => anonymous_type.open_content(), @@ -85,7 +93,22 @@ impl IslTypeImpl { &self.constraints } - pub fn open_content(&self) -> bool { + pub fn open_content(&self) -> Vec<(String, Element)> { + let mut open_content = vec![]; + for constraint in &self.constraints { + match constraint { + IslConstraint::Unknown(constraint_name, element) => { + open_content.push((constraint_name.to_owned(), element.to_owned())) + } + _ => { + // no op + } + } + } + open_content + } + + pub fn is_open_content_allowed(&self) -> bool { let mut open_content = true; if self.constraints.contains(&IslConstraint::ContentClosed) { open_content = false; diff --git a/ion-schema/src/isl/mod.rs b/ion-schema/src/isl/mod.rs index 5637f01..f05fe15 100644 --- a/ion-schema/src/isl/mod.rs +++ b/ion-schema/src/isl/mod.rs @@ -74,6 +74,7 @@ use crate::isl::isl_import::{IslImport, IslImportType}; use crate::isl::isl_type::IslType; +use ion_rs::value::owned::Element; pub mod isl_constraint; pub mod isl_import; @@ -93,6 +94,8 @@ pub struct IslSchema { types: Vec, // Represents all the inline IslImportTypes in this schema file. inline_imported_types: Vec, + // Represents open content as `Element`s + open_content: Vec, } impl IslSchema { @@ -100,11 +103,13 @@ impl IslSchema { imports: Vec, types: Vec, inline_imports: Vec, + open_content: Vec, ) -> Self { Self { imports, types, inline_imported_types: inline_imports, + open_content, } } @@ -119,6 +124,12 @@ impl IslSchema { pub fn inline_imported_types(&self) -> &[IslImportType] { &self.inline_imported_types } + + /// Provides top level open content for given schema + /// For open content defined within type definitions use IslType#open_content() + pub fn open_content(&self) -> &Vec { + &self.open_content + } } #[cfg(test)] @@ -172,6 +183,29 @@ mod isl_tests { ) } + #[test] + fn test_open_content_for_type_def() -> IonSchemaResult<()> { + let type_def = load_named_type( + r#" + // type definition with open content + type:: { + name: my_int, + type: int, + unknown_constraint: "this is an open content field value" + } + "#, + ); + + assert_eq!( + type_def.open_content(), + vec![( + "unknown_constraint".to_owned(), + element_reader().read_one(r#""this is an open content field value""#.as_bytes())? + )] + ); + Ok(()) + } + #[rstest( isl_type1,isl_type2, case::type_constraint_with_anonymous_type( diff --git a/ion-schema/src/system.rs b/ion-schema/src/system.rs index 25dcc17..679e7c7 100644 --- a/ion-schema/src/system.rs +++ b/ion-schema/src/system.rs @@ -758,6 +758,7 @@ impl Resolver { let mut isl_imports: Vec = vec![]; let mut isl_types: Vec = vec![]; let mut isl_inline_imports: Vec = vec![]; + let mut open_content = vec![]; let mut found_header = false; let mut found_footer = false; @@ -810,6 +811,8 @@ impl Resolver { else if annotations.contains(&&text_token("schema_footer")) { found_footer = true; } else { + // open content + open_content.push(value); continue; } } @@ -818,7 +821,12 @@ impl Resolver { return invalid_schema_error("For any schema while a header and footer are both optional, a footer is required if a header is present (and vice-versa)."); } - Ok(IslSchema::new(isl_imports, isl_types, isl_inline_imports)) + Ok(IslSchema::new( + isl_imports, + isl_types, + isl_inline_imports, + open_content, + )) } /// Converts given ISL representation into a [`Schema`] @@ -930,6 +938,34 @@ impl Resolver { } unresolvable_schema_error("Unable to load schema: ".to_owned() + id) } + + /// Loads an [`IslSchema`] using authorities and type_store + // If we are loading the root schema then this will be set to `None` ( i.e. in the beginning when + // this method is called from the load_schema method of schema_system it is set to `None`) + // Otherwise if we are loading an import of the schema then this will be set to `Some(isl_import)` + // to be loaded (i.e. Inside schema_from_elements while loading imports this will be set to + // `Some(isl_import)`) + fn load_isl_schema>( + &mut self, + id: A, + load_isl_import: Option<&IslImport>, + ) -> IonSchemaResult { + let id: &str = id.as_ref(); + + for authority in &self.authorities { + return match authority.elements(id) { + Err(error) => match error { + IonSchemaError::IoError { source } => match source.kind() { + ErrorKind::NotFound => continue, + _ => Err(IonSchemaError::IoError { source }), + }, + _ => Err(error), + }, + Ok(schema_content) => self.isl_schema_from_elements(schema_content.into_iter(), id), + }; + } + unresolvable_schema_error("Unable to load ISL model: ".to_owned() + id) + } } /// Provides functions for instantiating instances of [`Schema`]. @@ -953,6 +989,13 @@ impl SchemaSystem { .load_schema(id, &mut TypeStore::default(), None) } + /// Requests each of the provided [`DocumentAuthority`]s, in order, to get ISL model for the + /// requested schema id until one successfully resolves it. + /// If an authority throws an exception, resolution silently proceeds to the next authority. + pub fn load_isl_schema>(&mut self, id: A) -> IonSchemaResult { + self.resolver.load_isl_schema(id, None) + } + /// Returns authorities associated with this [`SchemaSystem`] fn authorities(&mut self) -> &[Box] { &self.resolver.authorities @@ -1749,4 +1792,50 @@ mod schema_system_tests { let schema = schema_system.load_schema("sample.isl"); assert!(schema.is_err()); } + + #[test] + fn open_content_test() -> IonSchemaResult<()> { + // map with (id, ion content) + let map_authority = [( + "sample.isl", + r#" + schema_header::{} + + type::{ + name: my_type, + type: string, + } + + open_content_1::{ + unknown_constraint: "this is an open content struct" + } + + open_content_2::{ + unknown_constraint: "this is an open content struct" + } + + schema_footer::{} + "#, + )]; + let mut schema_system = + SchemaSystem::new(vec![Box::new(MapDocumentAuthority::new(map_authority))]); + let schema = schema_system.load_isl_schema("sample.isl")?; + let expected_open_content = element_reader().read_all( + r#" + open_content_1::{ + unknown_constraint: "this is an open content struct" + } + + open_content_2::{ + unknown_constraint: "this is an open content struct" + } + "# + .as_bytes(), + )?; + + // verify the open content that is retrieved from the ISL model is same as the expected open content + assert_eq!(&schema.open_content().len(), &2); + assert_eq!(schema.open_content(), &expected_open_content); + Ok(()) + } } diff --git a/ion-schema/src/types.rs b/ion-schema/src/types.rs index d319bc1..6317794 100644 --- a/ion-schema/src/types.rs +++ b/ion-schema/src/types.rs @@ -400,7 +400,7 @@ impl TypeDefinitionImpl { isl_constraint, type_store, pending_types, - isl_type.open_content(), + isl_type.is_open_content_allowed(), )?; constraints.push(constraint); } From a87721338d740b1b4c27dcc064aac59402ce9004 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Wed, 1 Feb 2023 12:36:55 -0800 Subject: [PATCH 2/3] adds suggested changes --- ion-schema/src/isl/isl_type.rs | 9 ++------- ion-schema/src/isl/mod.rs | 25 +++++++++++++++++++------ ion-schema/src/system.rs | 11 ++++------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/ion-schema/src/isl/isl_type.rs b/ion-schema/src/isl/isl_type.rs index 4566eef..1c50edd 100644 --- a/ion-schema/src/isl/isl_type.rs +++ b/ion-schema/src/isl/isl_type.rs @@ -96,13 +96,8 @@ impl IslTypeImpl { pub fn open_content(&self) -> Vec<(String, Element)> { let mut open_content = vec![]; for constraint in &self.constraints { - match constraint { - IslConstraint::Unknown(constraint_name, element) => { - open_content.push((constraint_name.to_owned(), element.to_owned())) - } - _ => { - // no op - } + if let IslConstraint::Unknown(constraint_name, element) = constraint { + open_content.push((constraint_name.to_owned(), element.to_owned())) } } open_content diff --git a/ion-schema/src/isl/mod.rs b/ion-schema/src/isl/mod.rs index f05fe15..43f5bac 100644 --- a/ion-schema/src/isl/mod.rs +++ b/ion-schema/src/isl/mod.rs @@ -86,15 +86,28 @@ pub mod util; /// Provides an internal representation of an schema file #[derive(Debug, Clone)] pub struct IslSchema { - // Represents all the IslImports inside the schema file. - // For more information: https://amazon-ion.github.io/ion-schema/docs/isl-1-0/spec#imports + /// Represents all the IslImports inside the schema file. + /// For more information: https://amazon-ion.github.io/ion-schema/docs/isl-1-0/spec#imports imports: Vec, - // Represents all the IslType defined in this schema file. - // For more information: https://amazon-ion.github.io/ion-schema/docs/isl-1-0/spec#type-definitions + /// Represents all the IslType defined in this schema file. + /// For more information: https://amazon-ion.github.io/ion-schema/docs/isl-1-0/spec#type-definitions types: Vec, - // Represents all the inline IslImportTypes in this schema file. + /// Represents all the inline IslImportTypes in this schema file. inline_imported_types: Vec, - // Represents open content as `Element`s + /// Represents open content as `Element`s + /// Note: This doesn't preserve the information about where the open content is placed in the schema file. + /// e.g. This method doesn't differentiate between following schemas with open content: + /// ```ion + /// $foo + /// $bar + /// type::{ name: foo, codepoint_length: 1 } + /// ``` + /// + /// ```ion + /// type::{ name: foo, codepoint_length: 1 } + /// $foo + /// $bar + /// ``` open_content: Vec, } diff --git a/ion-schema/src/system.rs b/ion-schema/src/system.rs index 679e7c7..60095fe 100644 --- a/ion-schema/src/system.rs +++ b/ion-schema/src/system.rs @@ -954,14 +954,11 @@ impl Resolver { for authority in &self.authorities { return match authority.elements(id) { - Err(error) => match error { - IonSchemaError::IoError { source } => match source.kind() { - ErrorKind::NotFound => continue, - _ => Err(IonSchemaError::IoError { source }), - }, - _ => Err(error), - }, Ok(schema_content) => self.isl_schema_from_elements(schema_content.into_iter(), id), + Err(IonSchemaError::IoError { source: e }) if e.kind() == ErrorKind::NotFound => { + continue + } + Err(error) => Err(error), }; } unresolvable_schema_error("Unable to load ISL model: ".to_owned() + id) From 5a5ab527bfec591eec85f34f7446bf694d99f4df Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Thu, 2 Feb 2023 12:32:51 -0800 Subject: [PATCH 3/3] make is_open_content_allowed private crate function --- ion-schema/src/isl/isl_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ion-schema/src/isl/isl_type.rs b/ion-schema/src/isl/isl_type.rs index 1c50edd..eb20208 100644 --- a/ion-schema/src/isl/isl_type.rs +++ b/ion-schema/src/isl/isl_type.rs @@ -103,7 +103,7 @@ impl IslTypeImpl { open_content } - pub fn is_open_content_allowed(&self) -> bool { + pub(crate) fn is_open_content_allowed(&self) -> bool { let mut open_content = true; if self.constraints.contains(&IslConstraint::ContentClosed) { open_content = false;