handle single-element anyOf/allOf/oneOf #2
Conversation
Created using spr 1.3.6-beta.1
| metadata: new_meta, | ||
| }, | ||
| ) => { | ||
| if old_meta != new_meta { |
There was a problem hiding this comment.
Is it okay to just compare the metadata like this?
There was a problem hiding this comment.
Close enough until we have some tighter comparison?
ahl
left a comment
ahl
left a comment
There was a problem hiding this comment.
This is all good and thank you for doing it. I do think we might consider these cases a little more generally--in particular such that we don't require the singleton subschema to be a $ref.
I could imagine saying
- are both new and old singleton subschemas? then compare those subschemas?
- is new a singleton subschema, compare its subschema to old
- is old a singleton subschema, compare its subschema to new
For example, I made this test:
[
{
"op": "replace",
"path": "/paths/~1hello~1{name}/get/parameters/0/schema",
"value": {
"allOf": [
{
"type": "string"
}
]
}
}
]and it fails like this:
--- change-param-to-allof.json
+++ patched
@@ -76,7 +76,11 @@
"in": "path",
"name": "name",
"schema": {
- "type": "string"
+ "allOf": [
+ {
+ "type": "string"
+ }
+ ]
}
},
{
Result for patch:
[
Change {
message: "schema kind changed from regular type to allOf",
old_path: [
"#/paths/~1hello~1{name}/get/parameters/0/schema",
],
new_path: [
"#/paths/~1hello~1{name}/get/parameters/0/schema",
],
comparison: Input,
class: Incompatible,
details: Datatype,
},
]
| metadata: new_meta, | ||
| }, | ||
| ) => { | ||
| if old_meta != new_meta { |
There was a problem hiding this comment.
Close enough until we have some tighter comparison?
Fixed and added tests for these cases. |
| let old_not = old_schema_kind.append_deref(old_not.as_ref(), "not"); | ||
| let new_not = new_schema_kind.append_deref(new_not.as_ref(), "not"); | ||
| self.compare_schema_ref_helper(dry_run, comparison, old_not, new_not) |
There was a problem hiding this comment.
Also ended up fixing this (I think?) since it was pretty straightforward.
Created using spr 1.3.6-beta.1
| "#/paths/~1hello~1{name}/get/parameters/0/schema", | ||
| ], | ||
| comparison: Input, | ||
| class: Trivial, |
Previously, if an element changed between:
anyOf,allOf, oroneOfThen, we wouldn't be able to tell that they were semantically equivalent. (This kind of change can occur if a doc comment gets added or goes away.)
Fix this by unwrapping single-element wrappers, drilling through them to compare the underlying refs.
Also add some debugging info I found useful.