jsonschema-rs icon indicating copy to clipboard operation
jsonschema-rs copied to clipboard

feat: Custom keyword validation

Open samgqroberts opened this issue 1 year ago • 7 comments

This was originally put up by @bnjt here https://github.com/Stranger6667/jsonschema-rs/pull/394. That repository was deleted which deleted the PR, so this PR re-creates that PR.

Additionally, this PR adds an improvement to the original PR whereby existing keywords can also have their behavior be overridden by custom logic.

samgqroberts avatar Apr 10 '24 15:04 samgqroberts

Let me know if there is anything I can do to help with moving this forward :)

Stranger6667 avatar Apr 18 '24 09:04 Stranger6667

@Stranger6667 sounds good! day job has taken over this week but I'll dig back into this PR this weekend

samgqroberts avatar Apr 19 '24 00:04 samgqroberts

I am thinking about moving the internal state of CompiledCustomKeywordValidator to the inner validator, so if they are not needed, the user may choose to keep the overall structure smaller, i.e. pushing this decision to the user rather than always storing it (as in compile_custom_keyword_validator)

So, instead of closure without arguments, it could be an equivalent of compile-like functions which will return Box<CustomValidator> which in turn will be stored in the wrapper.

However, at this point, the CustomKeywordValidator trait looks very akin to Validator, so I am thinking if it should be used instead, this way there will be one less indirection + we'll reuse the existing code. Let me know what you think!

Stranger6667 avatar Apr 21 '24 12:04 Stranger6667

I have an idea how what the API could look like:

#[derive(Debug)]
struct AsciiKeyword {
    size: usize
}

impl jsonschema::CustomKeyword for AsciiKeyword {
    fn is_valid(&self, instance: &Value) -> bool {
        todo!()
    }
}

fn ascii_keyword_factory(schema: &Value) -> Arc<dyn jsonschema::CustomKeyword> {
    Arc::new(AsciiKeyword { size: 42 })
}

let validator = JSONSchema::options()
    .with_keyword(
        "ascii",
        |schema| -> Arc<dyn jsonschema::CustomKeyword> {
            Arc::new(AsciiKeyword { size: 42 })
        }
    )
    .with_keyword("also-ascii", ascii_keyword_factory)
    .compile(&schema)
    .unwrap();

I've implemented it for the new jsonschema version - at least it compiles and the code above works. Here is the implementation:

  • Traits - CustomKeyword that provides the relevant methods (is_valid, etc), CustomKeywordFactory is what is actually stored inside options, and is implemented for functions that accept 1 argument of the Value type. Those should be extended to match all the needed arguments from the current implementation
  • Function to add new keywords

With some adjustments (mainly for other arguments like path, etc) it should work

P.S. I haven't solved how to use such trait objects just yet, but I think it should be possible

Stranger6667 avatar Apr 22 '24 20:04 Stranger6667

I'll push some changes to make things work with the API I described above, though there is still enough room to adjust compilation / is_valid / validate signatures so that your use case is still possible

Stranger6667 avatar Apr 23 '24 17:04 Stranger6667

@Stranger6667 Thanks for running with this. I'll be ready to take another look after your updates.

samgqroberts avatar Apr 24 '24 12:04 samgqroberts

I'll add some more changes soon! Here are a few observations:

  • Some logic should be moved to the "build" phase, so as soon as we know that the schema is invalid it should be reported. Hence the keyword factory should return Result
  • Should custom validators return a single error? It may simplify the logic a bit

Otherwise, I think that the path forward is more or less clear

Stranger6667 avatar Apr 24 '24 19:04 Stranger6667

Update:

There are not that many things left:

  • [x] Tune privacy of some types (InstancePath)
  • [x] Update docs (missing docs + doctests)
  • [x] Tests for schema errors inside custom keywords
  • [x] Custom validation errors (there should be a way to provide a custom error message)
  • [ ] Expose Validate instead of Keyword (they are more or less the same)
  • [x] Try to make keyword factories work with closures without adding a lifetime to CompilationOptions
  • [x] Extract changes related to paths.rs to a separate PR

Stranger6667 avatar Apr 30 '24 18:04 Stranger6667

Codecov Report

Attention: Patch coverage is 94.66192% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 90.02%. Comparing base (092b573) to head (b284f75).

Files Patch % Lines
jsonschema/src/compilation/mod.rs 94.59% 12 Missing :warning:
jsonschema/src/keywords/custom.rs 87.50% 3 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
+ Coverage   89.70%   90.02%   +0.32%     
==========================================
  Files          57       58       +1     
  Lines        9643     9923     +280     
==========================================
+ Hits         8650     8933     +283     
+ Misses        993      990       -3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 01 '24 13:05 codecov[bot]

I think it is ready to merge!

From what I see it should be possible to implement all the cases mentioned in different issues:

  • Original cases in this PR by @samgqroberts (they could be seen in tests)
  • Cases originally added by @bnjt in #394 (their reworked versions are in this PR)
  • File path validation described by @tamasfe in #379
  • Sub-schema-based validation from tests in #354 by @antouhou (custom min-max logic could be re-implemented in a custom validator quite easily)

Please, let me know if there is anything missing to cover your use cases. I'd be glad to work on them here, or in a follow-up PR.

Thank you all for opening those issues and providing a lot of details and context, I appreciate it a lot! Sorry for the delay, but I hope to get this feature released this week.

cc @eirnym @platoscave as you participated in #394, feel free to leave your comments here :)

Stranger6667 avatar May 01 '24 14:05 Stranger6667

👏 Thank you @Stranger6667 – the final draft looks great for my use case

samgqroberts avatar May 18 '24 16:05 samgqroberts

Thank you @samgqroberts ! Let me know if you need anything else from my side :) I’d be happy to chat

Stranger6667 avatar May 18 '24 17:05 Stranger6667