Skip to content
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

[vividus-plugin-json] Add step to validate JSON against schema #3597

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

IBrun
Copy link
Contributor

@IBrun IBrun commented Jan 30, 2023

No description provided.

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #3597 (11c72b6) into master (321c018) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #3597   +/-   ##
=========================================
  Coverage     96.98%   96.99%           
- Complexity     6068     6076    +8     
=========================================
  Files           855      856    +1     
  Lines         17350    17384   +34     
  Branches       1123     1127    +4     
=========================================
+ Hits          16827    16861   +34     
  Misses          417      417           
  Partials        106      106           
Impacted Files Coverage Δ
.../vividus/json/steps/JsonSchemaValidationSteps.java 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@IBrun IBrun marked this pull request as ready for review January 30, 2023 11:03
@IBrun IBrun requested a review from a team as a code owner January 30, 2023 11:03
@IBrun IBrun changed the title [vividus-plugin-json] Add step for validate JSON against schema [vividus-plugin-json] Add step to validate JSON against schema Jan 30, 2023
Then JSON `$json` is valid against schema `$schema`
----

* `$json` - The JSON.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `$json` - The JSON.
* `$json` - The JSON to validate.

@Then("JSON `$json` is valid against schema `$schema`")
public void validateJsonAgainstSchema(String json, String schema)
{
JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012);
Copy link
Collaborator

@valfirst valfirst Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, it makes sense to use version of the schema specified in it, otherwise we should default to 2020-12

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valfirst could you please elaborate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ikalinin1 the schema itself specfies its version "$schema": "https://json-schema.org/draft/2020-12/schema",

if (!errors.isEmpty())
{
softAssert.recordFailedAssertion(
String.format("JSON is not valid against schema: %s", StringUtils.join(errors, ", ")));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does error look like?

@@ -485,6 +490,25 @@ public void assertNumberOfJsonElements(String json, String jsonPath, ComparisonR
assertJsonElementsNumber(jsonPath, actualNumber, comparisonRule, elementsNumber);
}

/**
* Validates json against json schema
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe javadoc should mention the version of the schema

@@ -500,3 +502,59 @@ Then number of JSON elements from `
]
` by JSON path `$..accountId` is equal to 2
----

=== Validate JSON document
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
=== Validate JSON document
=== Validate JSON against schema

if (!errors.isEmpty())
{
softAssert.recordFailedAssertion(
String.format("JSON is not valid against schema: %s", StringUtils.join(errors, ", ")));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java.lang.String#join(java.lang.CharSequence, java.lang.Iterable<? extends java.lang.CharSequence>) ?

@@ -13,6 +13,7 @@ dependencies {
implementation(group: 'io.qameta.allure', name: 'allure-jsonunit', version: versions.allure) {
exclude (group: 'io.qameta.allure')
}
implementation(group: "com.networknt", name: "json-schema-validator", version: "1.0.76");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

singlequotes as per all other cases where string interpolation doesn't happen

"description": "Tags for the product",
"type": "array",
"items": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update this example to validate that the array contains a specific list of items not any strigns

@IBrun IBrun marked this pull request as draft January 31, 2023 12:15
@IBrun IBrun force-pushed the json_validation branch 2 times, most recently from 9f83bcb to 7cd5728 Compare January 31, 2023 12:31
@IBrun IBrun marked this pull request as ready for review January 31, 2023 12:53
@IBrun IBrun requested review from ikalinin1, uarlouski and valfirst and removed request for ikalinin1 and uarlouski January 31, 2023 12:53
@IBrun IBrun force-pushed the json_validation branch 2 times, most recently from ff7bdf6 to d63f8ec Compare January 31, 2023 13:57
Comment on lines 519 to 520
:schema-specification: https://json-schema.org/specification-links.html[schema specification]
:meta-schema-2020-12: https://json-schema.org/specification-links.html#2020-12[2020-12]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can inline attributes used once

----
"$schema": "https://json-schema.org/draft/2020-12/schema"
----
If it is not present in the schema than JSON is validated using {meta-schema-2020-12} specification.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If it is not present in the schema than JSON is validated using {meta-schema-2020-12} specification.
If the version is not present in the schema then JSON is validated according to {meta-schema-2020-12} version.

====
:schema-specification: https://json-schema.org/specification-links.html[schema specification]
:meta-schema-2020-12: https://json-schema.org/specification-links.html#2020-12[2020-12]
Step validates JSON according {schema-specification} given in the schema. E.g.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Step validates JSON according {schema-specification} given in the schema. E.g.
The step validates JSON according to {schema-specification} version provided in the schema itself, e.g.:

@@ -485,6 +493,45 @@ public void assertNumberOfJsonElements(String json, String jsonPath, ComparisonR
assertJsonElementsNumber(jsonPath, actualNumber, comparisonRule, elementsNumber);
}

/**
* Validates json against json schema
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make javadoc as much possible close to the documentation

{
JsonNode schemaNode = jsonUtils.readTree(schema);
SpecVersion.VersionFlag version;
if (schemaNode.has(SCHEMA_TAG))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having this complexity added it makes sense to move the step to a new class

JsonSchema jsonSchema = factory.getSchema(schemaNode);
JsonNode jsonNode = jsonUtils.readTree(json);
Set<ValidationMessage> errors = jsonSchema.validate(jsonNode);
if (!errors.isEmpty())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to record passed assertion in case of success

if (!errors.isEmpty())
{
softAssert.recordFailedAssertion("JSON is not valid against schema");
errors.forEach(error -> softAssert.recordFailedAssertion(error.toString()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be a single assertion error with all schema violations listed

@IBrun IBrun removed the request for review from ikalinin1 February 1, 2023 10:00
@IBrun IBrun requested review from valfirst and removed request for uarlouski February 1, 2023 10:00
import org.vividus.softassert.ISoftAssert;
import org.vividus.util.json.JsonUtils;

public class JsonValidationSteps
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public class JsonValidationSteps
public class JsonSchemaValidationSteps

Comment on lines 100 to 107
List<String> messages = new ArrayList<>();
int index = 1;
for (ValidationMessage validationMessage : validationMessages)
{
messages.add(String.format("%d) %s", index, validationMessage.toString()));
index++;
}
return String.join(String.format(",%n"), messages);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not to use StringBuilder?

Comment on lines 15 to 19
<bean id="jsonSteps" class="org.vividus.json.steps.JsonSteps">
<constructor-arg>
<bean class="org.vividus.json.softassert.JsonSoftAssert" parent="softAssert" />
</constructor-arg>
</bean>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To move definition of JsonSoftAssert bean from definition of jsonSteps bean

{
if (index != 1)
{
messagesBuilder.append(",").append(System.lineSeparator());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
messagesBuilder.append(",").append(System.lineSeparator());
messagesBuilder.append(',').append(System.lineSeparator());

}
else
{
softAssert.recordFailedAssertion(String.format("JSON is not valid against schema:%n%s",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here String.format is used, but below StringBuilder is used, it makes sense to use single StringBuilder to build full string

@IBrun IBrun requested review from valfirst, uarlouski and ikalinin1 and removed request for valfirst and uarlouski February 1, 2023 13:09
@valfirst valfirst merged commit 42d4fcb into vividus-framework:master Feb 1, 2023
@IBrun IBrun deleted the json_validation branch June 29, 2023 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants