getsentry
`as_object` on Schema?
I am not sure this is a good idea though.Perhaps we can impl JsonSchemaExt on Schema instead of SchemaObject, with the newly introduced method as_object:
trait JsonSchemaExt {
fn is_true(&self) -> bool;
fn effective_type(&mut self) -> InternalJsonSchemaType;
fn as_object(&mut self) -> Option<&mut SchemaObject>;
}
impl JsonSchemaExt for Schema {
fn is_true(&self) -> bool {
matches!(self, Schema::Bool(true))
}
fn effective_type(&mut self) -> InternalJsonSchemaType {
match self {
Schema::Object(obj) => {
if let Some(ref ty) = obj.instance_type {
match ty {
SingleOrVec::Single(ty) => JsonSchemaType::from(**ty).into(),
SingleOrVec::Vec(tys) => InternalJsonSchemaType::Multiple(
tys.iter().copied().map(JsonSchemaType::from).collect(),
),
}
} else if let Some(ref constant) = obj.const_value {
serde_value_to_own(constant).into()
} else if !obj.object().properties.is_empty() {
JsonSchemaType::Object.into()
} else if let Some(ref mut any_of) = obj.subschemas().any_of {
InternalJsonSchemaType::Multiple(
any_of
.iter_mut()
.flat_map(|a| Self::effective_type(a).explode())
.collect::<BTreeSet<_>>()
.into_iter()
.collect(),
)
} else if obj.subschemas().not.as_ref().map_or(false, |x| x.is_true()) {
InternalJsonSchemaType::Never
} else {
InternalJsonSchemaType::Any
}
}
Schema::Bool(true) => InternalJsonSchemaType::Any,
Schema::Bool(false) => InternalJsonSchemaType::Never,
}
}
fn as_object(&mut self) -> Option<&mut SchemaObject> {
match self {
Schema::Object(obj) => Some(obj),
_ => None,
}
}
}
A main advantage here is that we can get rid of cloning Schema just for obtaining &mut SchemaObject, as done in some places. e.g.
https://github.com/getsentry/json-schema-diff/blob/19d0fd5fec55904a7a0370a503f5cb71656b9797/src/diff_walker.rs#L146
A drawback would be increased lines of code. We would rewrite diff_properties as below:
fn diff_properties(
&mut self,
json_path: &str,
lhs: &mut Schema,
rhs: &mut Schema,
) -> Result<(), Error> {
let lhs_props: BTreeSet<_> = lhs
.as_object()
.map(|obj| obj.object().properties.keys().cloned().collect())
.unwrap_or_default();
let rhs_props: BTreeSet<_> = rhs
.as_object()
.map(|obj| obj.object().properties.keys().cloned().collect())
.unwrap_or_default();
let lhs_additional_properties = lhs
.as_object()
.and_then(|obj| obj.object().additional_properties.as_ref())
.map_or(true, |x| x.is_true());
for removed in lhs_props.difference(&rhs_props) {
(self.cb)(Change {
path: json_path.to_owned(),
change: ChangeKind::PropertyRemove {
lhs_additional_properties,
removed: removed.clone(),
},
});
}
I believe this discussion is worth having upstream in schemars. If they reject, go for it here
On Sat, 20 May 2023, 20:21 Yudai Kiyofuji, @.***> wrote:
I am not sure this is a good idea though.Perhaps we can impl JsonSchemaExt on Schema instead of SchemaObject, with the newly introduced method as_object:
trait JsonSchemaExt { fn is_true(&self) -> bool; fn effective_type(&mut self) -> InternalJsonSchemaType; fn as_object(&mut self) -> Option<&mut SchemaObject>; }
impl JsonSchemaExt for Schema { fn is_true(&self) -> bool { matches!(self, Schema::Bool(true)) }
fn effective_type(&mut self) -> InternalJsonSchemaType { match self { Schema::Object(obj) => { if let Some(ref ty) = obj.instance_type { match ty { SingleOrVec::Single(ty) => JsonSchemaType::from(**ty).into(), SingleOrVec::Vec(tys) => InternalJsonSchemaType::Multiple( tys.iter().copied().map(JsonSchemaType::from).collect(), ), } } else if let Some(ref constant) = obj.const_value { serde_value_to_own(constant).into() } else if !obj.object().properties.is_empty() { JsonSchemaType::Object.into() } else if let Some(ref mut any_of) = obj.subschemas().any_of { InternalJsonSchemaType::Multiple( any_of .iter_mut() .flat_map(|a| Self::effective_type(a).explode()) .collect::<BTreeSet<_>>() .into_iter() .collect(), ) } else if obj.subschemas().not.as_ref().map_or(false, |x| x.is_true()) { InternalJsonSchemaType::Never } else { InternalJsonSchemaType::Any } } Schema::Bool(true) => InternalJsonSchemaType::Any, Schema::Bool(false) => InternalJsonSchemaType::Never, } } fn as_object(&mut self) -> Option<&mut SchemaObject> { match self { Schema::Object(obj) => Some(obj), _ => None, } }}
A main advantage here is that we can get rid of cloning Schema just for obtaining &mut SchemaObject, as done in some places. e.g.
https://github.com/getsentry/json-schema-diff/blob/19d0fd5fec55904a7a0370a503f5cb71656b9797/src/diff_walker.rs#L146
A drawback would be increased lines of code. We would rewrite diff_properties as below:
fn diff_properties( &mut self, json_path: &str, lhs: &mut Schema, rhs: &mut Schema, ) -> Result<(), Error> { let lhs_props: BTreeSet<> = lhs .as_object() .map(|obj| obj.object().properties.keys().cloned().collect()) .unwrap_or_default(); let rhs_props: BTreeSet<> = rhs .as_object() .map(|obj| obj.object().properties.keys().cloned().collect()) .unwrap_or_default();
let lhs_additional_properties = lhs .as_object() .and_then(|obj| obj.object().additional_properties.as_ref()) .map_or(true, |x| x.is_true()); for removed in lhs_props.difference(&rhs_props) { (self.cb)(Change { path: json_path.to_owned(), change: ChangeKind::PropertyRemove { lhs_additional_properties, removed: removed.clone(), }, }); }— Reply to this email directly, view it on GitHub https://github.com/getsentry/json-schema-diff/issues/36, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGMPRL4UGX6KLUNLH3547TXHEDUJANCNFSM6AAAAAAYI4DPHE . You are receiving this because you are subscribed to this thread.Message ID: @.***>
