-
Notifications
You must be signed in to change notification settings - Fork 36
feat(validate): add --path option to target sub-schema via JSON Pointer #696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7a0ac9f
2162b2e
caaa025
de9cd5e
f978b7d
922ba31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, we probably want something like this for the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Vaibhav701161 I think this comment is still applicable. We need to support |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| #include <iostream> // std::cerr | ||
| #include <string> // std::string | ||
| #include <string_view> // std::string_view | ||
| #include <utility> // std::as_const | ||
|
|
||
| #include "command.h" | ||
| #include "configuration.h" | ||
|
|
@@ -247,6 +248,12 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options) | |
| schema_path, std::make_error_code(std::errc::is_a_directory)}; | ||
| } | ||
|
|
||
| if (options.contains("path") && !options.at("path").empty() && | ||
| options.contains("template") && !options.at("template").empty()) { | ||
| throw OptionConflictError{ | ||
| "The --path option cannot be used with --template"}; | ||
| } | ||
|
|
||
| const auto schema_config_base{schema_from_stdin | ||
| ? std::filesystem::current_path() | ||
| : std::filesystem::path(schema_path)}; | ||
|
|
@@ -258,11 +265,40 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options) | |
| read_configuration(options, configuration_path, schema_config_base)}; | ||
| const auto dialect{default_dialect(options, configuration)}; | ||
|
|
||
| const auto schema{schema_from_stdin | ||
| ? read_from_stdin().document | ||
| : sourcemeta::core::read_yaml_or_json(schema_path)}; | ||
| auto schema = schema_from_stdin | ||
| ? read_from_stdin().document | ||
| : sourcemeta::core::read_yaml_or_json(schema_path); | ||
|
|
||
| std::string path_pointer_string; | ||
|
|
||
| if (options.contains("path") && !options.at("path").empty()) { | ||
| sourcemeta::core::Pointer pointer; | ||
| try { | ||
| pointer = | ||
| sourcemeta::core::to_pointer(std::string{options.at("path").front()}); | ||
| } catch (const sourcemeta::core::PointerParseError &) { | ||
| throw PositionalArgumentError{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is the right error here. |
||
| "The JSON Pointer is not valid", | ||
| "jsonschema validate path/to/schema.json path/to/instance.json " | ||
| "--path '/components/schemas/User'"}; | ||
| } | ||
|
|
||
| path_pointer_string = sourcemeta::core::to_string(pointer); | ||
|
|
||
| if (!sourcemeta::core::is_schema(schema)) { | ||
| const auto *const result = sourcemeta::core::try_get(schema, pointer); | ||
| if (!result) { | ||
| throw PathResolutionError{schema_resolution_base, path_pointer_string}; | ||
| } | ||
Vaibhav701161 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| sourcemeta::core::JSON subschema{*result}; | ||
| schema = std::move(subschema); | ||
Vaibhav701161 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (!sourcemeta::core::is_schema(schema)) { | ||
| throw NotSchemaError{schema_from_stdin ? stdin_path() | ||
| : schema_resolution_base, | ||
| path_pointer_string}; | ||
| } | ||
| } else if (!sourcemeta::core::is_schema(schema)) { | ||
| throw NotSchemaError{schema_from_stdin ? stdin_path() | ||
| : schema_resolution_base}; | ||
| } | ||
|
|
@@ -291,16 +327,22 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options) | |
|
|
||
| const sourcemeta::core::JSON bundled{[&]() { | ||
| try { | ||
| return sourcemeta::core::bundle(schema, sourcemeta::core::schema_walker, | ||
| custom_resolver, dialect, | ||
| schema_default_id); | ||
| return sourcemeta::core::bundle( | ||
| std::as_const(schema), sourcemeta::core::schema_walker, | ||
| custom_resolver, dialect, schema_default_id); | ||
Vaibhav701161 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } catch (const sourcemeta::core::SchemaKeywordError &error) { | ||
| throw sourcemeta::core::FileError<sourcemeta::core::SchemaKeywordError>( | ||
| schema_resolution_base, error); | ||
| } catch (const sourcemeta::core::SchemaFrameError &error) { | ||
| throw sourcemeta::core::FileError<sourcemeta::core::SchemaFrameError>( | ||
| schema_resolution_base, error); | ||
| } catch (const sourcemeta::core::SchemaReferenceError &error) { | ||
| if (!path_pointer_string.empty()) { | ||
| throw PathSchemaReferenceError{ | ||
| schema_resolution_base, std::string{error.identifier()}, | ||
| error.location(), path_pointer_string, error.what()}; | ||
| } | ||
|
|
||
| throw sourcemeta::core::FileError<sourcemeta::core::SchemaReferenceError>( | ||
| schema_resolution_base, std::string{error.identifier()}, | ||
| error.location(), error.what()); | ||
|
|
@@ -399,6 +441,12 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options) | |
| throw sourcemeta::core::FileError<sourcemeta::core::SchemaFrameError>( | ||
| schema_resolution_base, error); | ||
| } catch (const sourcemeta::core::SchemaReferenceError &error) { | ||
| if (!path_pointer_string.empty()) { | ||
| throw PathSchemaReferenceError{ | ||
| schema_resolution_base, std::string{error.identifier()}, | ||
| error.location(), path_pointer_string, error.what()}; | ||
| } | ||
|
|
||
| throw sourcemeta::core::FileError<sourcemeta::core::SchemaReferenceError>( | ||
| schema_resolution_base, std::string{error.identifier()}, | ||
| error.location(), error.what()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| #!/bin/sh | ||
|
|
||
| set -o errexit | ||
| set -o nounset | ||
|
|
||
| TMP="$(mktemp -d)" | ||
| clean() { rm -rf "$TMP"; } | ||
| trap clean EXIT | ||
|
|
||
| cat << 'EOF' > "$TMP/schema.json" | ||
| { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "type": "object" | ||
| } | ||
| EOF | ||
|
|
||
| cat << 'EOF' > "$TMP/instance.json" | ||
| {} | ||
| EOF | ||
|
|
||
| "$1" validate "$TMP/schema.json" "$TMP/instance.json" \ | ||
| --path 'invalid~pointer' > "$TMP/output.txt" 2>&1 \ | ||
| && EXIT_CODE="$?" || EXIT_CODE="$?" | ||
| # Invalid CLI arguments | ||
| test "$EXIT_CODE" = "5" | ||
|
|
||
| cat << EOF > "$TMP/expected.txt" | ||
| error: The JSON Pointer is not valid | ||
|
|
||
| For example: jsonschema validate path/to/schema.json path/to/instance.json --path '/components/schemas/User' | ||
| EOF | ||
|
|
||
| diff "$TMP/output.txt" "$TMP/expected.txt" | ||
|
|
||
| # JSON error | ||
| "$1" validate "$TMP/schema.json" "$TMP/instance.json" \ | ||
| --path 'invalid~pointer' --json > "$TMP/stdout.txt" 2>&1 \ | ||
| && EXIT_CODE="$?" || EXIT_CODE="$?" | ||
| test "$EXIT_CODE" = "5" | ||
|
|
||
| cat << EOF > "$TMP/expected.txt" | ||
| { | ||
| "error": "The JSON Pointer is not valid" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this error is not great. You want to print the actual problematic pointer with something like |
||
| } | ||
| EOF | ||
|
|
||
| diff "$TMP/stdout.txt" "$TMP/expected.txt" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might want to explain more here the relationship with
--entrypoint. Essentially the idea is to use--pathto focus on a specific PART of the document that is a schema. And on top of what you can use--entrypointto focus on a specific subschema of that schema that is PART of a document.This case is also worth adding tests for