Skip to content

Commit

Permalink
code review (incl renaming and doc updates)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pvdbosch
pvdbosch committed Jan 9, 2025
1 parent 7a46703 commit e269712
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 48 deletions.
39 changes: 22 additions & 17 deletions core/src/main/java/io/github/belgif/rest/guide/validator/core/ApiFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,31 +112,35 @@ private static Set<SchemaDefinition> getSubSchemas(SchemaDefinition schemaDefini
return subSchemas;
}

private static Set<SchemaDefinition> getRecursiveSubSchemas(SchemaDefinition schemaDefinition, Parser.ParserResult result, boolean includeTopLevelSchemas) {
/**
* @return all subschemas of given schema, including recursively referenced ones
*/
private static Set<SchemaDefinition> getAllSubSchemas(SchemaDefinition schemaDefinition, Parser.ParserResult result, boolean includeTopLevelSchemas) {
Set<SchemaDefinition> subSchemas = getSubSchemas(schemaDefinition, result, includeTopLevelSchemas);
Set<SchemaDefinition> output = new HashSet<>();
for (SchemaDefinition schema : subSchemas) {
output.addAll(getRecursiveSubSchemas(schema, result, includeTopLevelSchemas));
output.addAll(getAllSubSchemas(schema, result, includeTopLevelSchemas));
}
output.addAll(subSchemas);
return output;
}

/**
* Returns all the properties (schemas) in a complex schema
* Returns all the properties (with their schemas) at top-level of a possibly composite schema.
* Properties only reachable via discriminator are not added, unless valueNode parameter is used.
*
* @param result Result from validationparser
* @param schema Schema of which the properties have to be returned
* @param exampleNode JsonNode of an example that can be used to also collect all properties based on a discriminator value
* @param valueNode optional JSON value compliant with the schema
*/
public static Map<String, Schema> getRecursiveProperties(Schema schema, Parser.ParserResult result, JsonNode exampleNode) {
public static Map<String, Schema> getAllProperties(Schema schema, Parser.ParserResult result, JsonNode valueNode) {
Map<String, Schema> properties = new HashMap<>();
SchemaDefinition schemaDef = recursiveResolve(schema, result);
Set<SchemaDefinition> definitions = new HashSet<>();
definitions.add(schemaDef);
definitions.addAll(getRecursiveSubSchemas(schemaDef, result, true));
if (exampleNode != null) {
definitions.addAll(findDiscriminatorSchemas(definitions, result, exampleNode));
definitions.addAll(getAllSubSchemas(schemaDef, result, true));
if (valueNode != null) {
definitions.addAll(findDiscriminatorSchemas(definitions, result, valueNode));
}
definitions.forEach(schemaDefinition -> {
if (schemaDefinition.getModel().getProperties() != null) {
Expand All @@ -146,41 +150,42 @@ public static Map<String, Schema> getRecursiveProperties(Schema schema, Parser.P
return properties;
}

private static Set<SchemaDefinition> findDiscriminatorSchemas(Set<SchemaDefinition> schemaDefinitions, Parser.ParserResult result, JsonNode exampleNode) {
private static Set<SchemaDefinition> findDiscriminatorSchemas(Set<SchemaDefinition> schemaDefinitions, Parser.ParserResult result, JsonNode valueNode) {
Set<SchemaDefinition> schemaDefinitionsWithDiscriminators = schemaDefinitions.stream()
.filter(def -> def.getModel().getDiscriminator() != null && exampleNode.has(def.getModel().getDiscriminator().getPropertyName()))
.filter(def -> def.getModel().getDiscriminator() != null && valueNode.has(def.getModel().getDiscriminator().getPropertyName()))
.collect(Collectors.toSet());

Set<SchemaDefinition> schemaDefinitionMappings = new HashSet<>();
for (SchemaDefinition schemaDefinition : schemaDefinitionsWithDiscriminators) {
String discriminator = schemaDefinition.getModel().getDiscriminator().getPropertyName();
String discriminatorValue = exampleNode.get(discriminator).asText();
String discriminatorValue = valueNode.get(discriminator).asText();
String mapping;
if (schemaDefinition.getModel().getDiscriminator().getMapping() != null) {
mapping = schemaDefinition.getModel().getDiscriminator().getMapping().get(discriminatorValue);
} else {
// TODO: if mapping present, but property value not found in discriminator mapping, also fallback to this
mapping = "#/components/schemas/" + discriminatorValue;
}

try {
OpenApiDefinition<Schema> resolvedDef = result.resolve(mapping, schemaDefinition.getOpenApiFile());
SchemaDefinition schemaDef = (SchemaDefinition) resolvedDef;
schemaDefinitionMappings.add(schemaDef);
schemaDefinitionMappings.addAll(ApiFunctions.getRecursiveSubSchemas(schemaDef, result, true));
schemaDefinitionMappings.addAll(ApiFunctions.getAllSubSchemas(schemaDef, result, true)); // TODO: move this to caller?
} catch (RuntimeException e) {
log.error("Cannot find schema for discriminator {} : {}", discriminator, discriminatorValue);
}
}
return schemaDefinitionMappings;
}

public static Set<String> getRequiredValues(SchemaDefinition schemaDefinition, Parser.ParserResult result) {
Set<String> requiredValues = new HashSet<>();
public static Set<String> getRequiredProperties(SchemaDefinition schemaDefinition, Parser.ParserResult result) {
Set<String> requiredProperties = new HashSet<>();
if (schemaDefinition.getModel() != null && schemaDefinition.getModel().getRequired() != null) {
requiredValues.addAll(schemaDefinition.getModel().getRequired());
requiredProperties.addAll(schemaDefinition.getModel().getRequired());
}
getRecursiveSubSchemas(schemaDefinition, result, false).forEach(schema -> requiredValues.addAll(getRequiredValues(schema, result)));
return requiredValues;
getAllSubSchemas(schemaDefinition, result, false).forEach(schema -> requiredProperties.addAll(getRequiredProperties(schema, result)));
return requiredProperties;
}

public static boolean isPropertyRequiredAndReadOnly(SchemaDefinition schemaDefinition, String propertyName, Parser.ParserResult result) {
Expand Down
40 changes: 17 additions & 23 deletions core/src/main/java/io/github/belgif/rest/guide/validator/core/util/SchemaValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,31 @@ public class SchemaValidator {
private SchemaValidator() {
}

public static List<String> getNonDefinedProperties(ExampleDefinition exampleDefinition) {
public static List<String> getUndefinedProperties(ExampleDefinition exampleDefinition) {
SchemaDefinition schemaDefinition = getSchemaDefinition(exampleDefinition);
ExampleDefinition example = (ExampleDefinition) exampleDefinition.getResult().resolve(exampleDefinition.getModel());
JsonNode exampleNode = getExampleNode(example);
if (schemaDefinition.getModel().getAdditionalPropertiesBoolean() != null) { //Other example validator will point out issues when additionalProperties is specifically set to false or true
if (schemaDefinition.getModel().getAdditionalPropertiesBoolean() != null) {
// Properties are only considered as undefined when additionalProperties are not explicitly allowed in the schema
// if additionalProperties is false, example schema validation will mark any undefined properties as invalid, so can be ignored here

// TODO: composite with additional properties? What if in nested object only? - move to fromObjectNode?
return new ArrayList<>();
}
return getNonDefinedProperties(exampleNode, schemaDefinition);
return getUndefinedProperties(exampleNode, schemaDefinition);
}

private static List<String> getNonDefinedProperties(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
private static List<String> getUndefinedProperties(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
List<String> missingProperties = new ArrayList<>();
missingProperties.addAll(getNonDefinedPropertiesFromArrayNode(exampleNode, schemaDefinition));
missingProperties.addAll(getNonDefinedPropertiesFromObjectNode(exampleNode, schemaDefinition));
missingProperties.addAll(getUndefinedPropertiesFromArrayNode(exampleNode, schemaDefinition));
missingProperties.addAll(getUndefinedPropertiesFromObjectNode(exampleNode, schemaDefinition));

return missingProperties;
}

private static List<String> getNonDefinedPropertiesFromObjectNode(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
private static List<String> getUndefinedPropertiesFromObjectNode(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
List<String> missingProperties = new ArrayList<>();
Map<String, Schema> definedProperties = ApiFunctions.getRecursiveProperties(schemaDefinition.getModel(), schemaDefinition.getResult(), exampleNode);
Map<String, Schema> definedProperties = ApiFunctions.getAllProperties(schemaDefinition.getModel(), schemaDefinition.getResult(), exampleNode);

Iterator<String> exampleFieldNames = exampleNode.fieldNames();
while (exampleFieldNames.hasNext()) {
Expand All @@ -65,33 +69,23 @@ private static List<String> getNonDefinedPropertiesFromObjectNode(JsonNode examp
missingProperties.add(exampleFieldName + " not found in: #" + schemaDefinition.getPrintableJsonPointer());
} else {
SchemaDefinition def = (SchemaDefinition) schemaDefinition.getResult().resolve(definedProperties.get(exampleFieldName));
if (subSchemaShouldBeInspectedFurther(def)) {
missingProperties.addAll(getNonDefinedProperties(exampleNode.get(exampleFieldName), def));
}
missingProperties.addAll(getUndefinedProperties(exampleNode.get(exampleFieldName), def));
}
}
return missingProperties;
}

private static List<String> getNonDefinedPropertiesFromArrayNode(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
private static List<String> getUndefinedPropertiesFromArrayNode(JsonNode exampleNode, SchemaDefinition schemaDefinition) {
List<String> missingProperties = new ArrayList<>();
if (schemaDefinition.getModel().getType() != null && schemaDefinition.getModel().getType().equals(Schema.SchemaType.ARRAY)) {
if (schemaDefinition.getModel().getType() != null && schemaDefinition.getModel().getType().equals(Schema.SchemaType.ARRAY)) { // TODO: What if schema is an allOf of an array and a description?
SchemaDefinition arrayItemSchemaDefinition = (SchemaDefinition) schemaDefinition.getResult().resolve(schemaDefinition.getModel().getItems());
if (subSchemaShouldBeInspectedFurther(arrayItemSchemaDefinition)) {
for (JsonNode arrayItem : exampleNode) {
missingProperties.addAll(getNonDefinedProperties(arrayItem, arrayItemSchemaDefinition));
}
for (JsonNode arrayItem : exampleNode) { //TODO: should we test exampleNode.isArray here (returns properties if it's on object)?
missingProperties.addAll(getUndefinedProperties(arrayItem, arrayItemSchemaDefinition));
}
}
return missingProperties;
}

private static boolean subSchemaShouldBeInspectedFurther(SchemaDefinition schemaDefinition) {
Schema schema = schemaDefinition.getModel();
return (schema.getType() != null && (schema.getType().equals(Schema.SchemaType.OBJECT) || schema.getType().equals(Schema.SchemaType.ARRAY))) ||
schema.getAllOf() != null || schema.getAnyOf() != null || schema.getOneOf() != null;
}

public static String getExampleViolations(ExampleDefinition exampleDefinition) {
try {
SchemaDefinition schemaDefinition = getSchemaDefinition(exampleDefinition);
Expand Down
10 changes: 5 additions & 5 deletions ...github/belgif/rest/guide/validator/rules/oas/Rule-ExamplesShouldValidateAgainstSchema.drl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ function void violationExamplesShouldValidateAgainstSchema(ViolationReport oas,
"Example does not validate against schema", violation, example);
}

function void violationExamplesShouldNotDefineNonExistingProperties(ViolationReport oas, ExampleDefinition example, String violation) {
oas.addViolation("[exam-prop]",
"Property used in example not present in schema: ", violation, example, ViolationLevel.RECOMMENDED);
function void violationExamplesShouldNotHaveUndefinedProperties(ViolationReport oas, ExampleDefinition example, String violation) {
oas.addViolation("[oas-exampl]",
"Example shouldn't contain undefined property", violation, example, ViolationLevel.RECOMMENDED);
}

rule "Example should be valid against schema"
Expand All @@ -33,7 +33,7 @@ end
rule "Example should not contain undefined properties"
when
$example: ExampleDefinition(definitionType.equals(DefinitionType.INLINE))
$violation: String() from SchemaValidator.getNonDefinedProperties($example)
$violation: String() from SchemaValidator.getUndefinedProperties($example)
then
violationExamplesShouldNotDefineNonExistingProperties(oas, $example, $violation);
violationExamplesShouldNotHaveUndefinedProperties(oas, $example, $violation);
end
2 changes: 1 addition & 1 deletion ...ain/resources/io/github/belgif/rest/guide/validator/rules/oas/Rule-ReadOnlyProperties.drl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function void violationReadOnlyProperties(ViolationReport oas, SchemaDefinition
rule "ReadOnly Properties"
when
$schema: SchemaDefinition(model.getRef() == null)
Map($property: /entrySet) from ApiFunctions.getRecursiveProperties($schema.getModel(), parserResult, null)
Map($property: /entrySet) from ApiFunctions.getAllProperties($schema.getModel(), parserResult, null)
$propertyKey: String() from $property.getKey()
eval ( ApiFunctions.isPropertyRequiredAndReadOnly($schema, $propertyKey, parserResult) )
then
Expand Down
4 changes: 2 additions & 2 deletions ...elgif/rest/guide/validator/rules/oas/Rule-ValuesOfRequiredShouldBeDefinedAsProperties.drl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ function void violationRequiredProperties(ViolationReport oas, SchemaDefinition
rule "Values of required should be defined as properties"
when
$schema: SchemaDefinition(isHighLevelSchema())
$required: String() from ApiFunctions.getRequiredValues($schema, parserResult)
not Map( containsKey($required) ) from ApiFunctions.getRecursiveProperties($schema.getModel(), parserResult, null)
$required: String() from ApiFunctions.getRequiredProperties($schema, parserResult)
not Map( containsKey($required) ) from ApiFunctions.getAllProperties($schema.getModel(), parserResult, null)
then
violationRequiredProperties(oas, $schema, $required);
end

0 comments on commit e269712

Please sign in to comment.