fix: Evaluation paths to exclude $ref / $dynamicRef / $recursiveRef
#904
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #904 +/- ##
==========================================
- Coverage 94.38% 93.85% -0.53%
==========================================
Files 78 78
Lines 17529 18865 +1336
==========================================
+ Hits 16544 17706 +1162
- Misses 985 1159 +174 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #904 will degrade performance by 36.95%Comparing Summary
Benchmarks breakdown
|
Stranger6667
force-pushed
the
dd/fix-evaluation-path
branch
17 times, most recently
from
e06d3c1 to
4b04080
Compare
…Ref` Signed-off-by: Dmitry Dygalo <[email protected]>
Stranger6667
force-pushed
the
dd/fix-evaluation-path
branch
from
4b04080 to
5df9108
Compare
| ### Fixed | ||
|
|
||
| - `schemaLocation` in evaluation output now excludes `$ref`/`$dynamicRef`/`$recursiveRef` per JSON Schema spec. | ||
|
|
| schema_path: &Location, | ||
| ) -> Result<(), ValidationError<'i>> { | ||
| if !instance.is_string() { | ||
| return Err(ctx.custom_error(schema_path, instance_path, instance, "expected a string")); |
There was a problem hiding this comment.
Maybe don' put this complexity to the user and allow to return just a message and then properly fixup ValidationError instance. This way they always be correct paths, etc
| if value.as_bool() == Some(true) { | ||
| Ok(Box::new(MyValidator)) | ||
| } else { | ||
| Err(ValidationError::schema(schema_path, value, "expected true")) |
There was a problem hiding this comment.
Same here, we know schema_path & value upfront, so maybe we can handle errors on the caller side instead & simplify the api?
| errors.extend(self.node.iter_errors(item, &location.push(idx), ctx)); | ||
| errors.extend( | ||
| self.node | ||
| .iter_errors(item, &location.push(idx), ref_path, ctx), |
There was a problem hiding this comment.
should ref_path be extended? it is evaluation path
| /// is already a valid JSON pointer path. | ||
| #[must_use] | ||
| pub(crate) fn join_raw_suffix(&self, suffix: &str) -> Self { | ||
| if suffix.is_empty() { |
| let mut buffer = String::with_capacity(parent.len() + suffix.len()); | ||
| buffer.push_str(parent); | ||
| buffer.push_str(suffix); | ||
| Self(Arc::from(buffer)) |
There was a problem hiding this comment.
just format! will be fine
| Deferred { | ||
| schema_location: Location, | ||
| /// Captured state with precomputed `eval_prefix` (computed at capture time). | ||
| captured: CapturedRefState, | ||
| /// Cached computed result (thread-safe). | ||
| cached: OnceLock<Location>, | ||
| }, |
|
|
||
| #[test] | ||
| fn evaluation_path_ref_unique_items() { | ||
| // Test uniqueItems validator through $ref |
There was a problem hiding this comment.
maybe use test_case???
Signed-off-by: Dmitry Dygalo <[email protected]>
Signed-off-by: Dmitry Dygalo <[email protected]>
Signed-off-by: Dmitry Dygalo <[email protected]>
No description provided.