Skip to content

Commit 08ec68e

Browse files
prashanthjosprashanth-chaitanya
prashanthjos
and
prashanth-chaitanya
authored
Fixing unevaluated properties with larger test base (#544)
* Fixing unevaluated properties with larger test base * Fixing the memory leak by reseting collector context * Fixing the memory leak by reseting collector context Co-authored-by: Prashanth Josyula <[email protected]>
1 parent 7e9375f commit 08ec68e

16 files changed

+1817
-111
lines changed

‎src/main/java/com/networknt/schema/AdditionalPropertiesValidator.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,18 @@ public AdditionalPropertiesValidator(String schemaPath, JsonNode schemaNode, Jso
6464
parseErrorCode(getValidatorType().getErrorCodeKey());
6565
}
6666

67+
private void addToEvaluatedProperties(String propertyPath) {
68+
Object evaluatedProperties = CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES);
69+
List<String> evaluatedPropertiesList = null;
70+
if (evaluatedProperties == null) {
71+
evaluatedPropertiesList = new ArrayList<>();
72+
CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, evaluatedPropertiesList);
73+
} else {
74+
evaluatedPropertiesList = (List<String>) evaluatedProperties;
75+
}
76+
evaluatedPropertiesList.add(propertyPath);
77+
}
78+
6779
public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String at) {
6880
if (logger.isDebugEnabled()) debug(logger, node, rootNode, at);
6981

@@ -73,6 +85,13 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
7385
return errors;
7486
}
7587

88+
// if allowAdditionalProperties is true, add all the properties as evaluated.
89+
if (allowAdditionalProperties) {
90+
for (Iterator<String> it = node.fieldNames(); it.hasNext(); ) {
91+
addToEvaluatedProperties(at + "." + it.next());
92+
}
93+
}
94+
7695
for (Iterator<String> it = node.fieldNames(); it.hasNext(); ) {
7796
String pname = it.next();
7897
// skip the context items
@@ -106,6 +125,47 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
106125
return Collections.unmodifiableSet(errors);
107126
}
108127

128+
@Override
129+
public Set<ValidationMessage> walk(JsonNode node, JsonNode rootNode, String at, boolean shouldValidateSchema) {
130+
if (shouldValidateSchema) {
131+
return validate(node, rootNode, at);
132+
}
133+
134+
if (node == null || !node.isObject()) {
135+
// ignore no object
136+
return Collections.emptySet();
137+
}
138+
139+
// Else continue walking.
140+
for (Iterator<String> it = node.fieldNames(); it.hasNext(); ) {
141+
String pname = it.next();
142+
// skip the context items
143+
if (pname.startsWith("#")) {
144+
continue;
145+
}
146+
boolean handledByPatternProperties = false;
147+
for (Pattern pattern : patternProperties) {
148+
Matcher m = pattern.matcher(pname);
149+
if (m.find()) {
150+
handledByPatternProperties = true;
151+
break;
152+
}
153+
}
154+
155+
if (!allowedProperties.contains(pname) && !handledByPatternProperties) {
156+
if (allowAdditionalProperties) {
157+
if (additionalPropertiesSchema != null) {
158+
ValidatorState state = (ValidatorState) CollectorContext.getInstance().get(ValidatorState.VALIDATOR_STATE_KEY);
159+
if (state != null && state.isWalkEnabled()) {
160+
additionalPropertiesSchema.walk(node.get(pname), rootNode, at + "." + pname, state.isValidationEnabled());
161+
}
162+
}
163+
}
164+
}
165+
}
166+
return Collections.emptySet();
167+
}
168+
109169
@Override
110170
public void preloadJsonSchema() {
111171
if(additionalPropertiesSchema!=null) {

‎src/main/java/com/networknt/schema/AllOfValidator.java‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@
1616

1717
package com.networknt.schema;
1818

19-
import java.util.ArrayList;
20-
import java.util.Collections;
21-
import java.util.Iterator;
22-
import java.util.LinkedHashSet;
23-
import java.util.List;
24-
import java.util.Set;
19+
import java.util.*;
2520

2621
import com.fasterxml.jackson.databind.JsonNode;
2722
import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -49,17 +44,35 @@ public AllOfValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentS
4944
public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String at) {
5045
debug(logger, node, rootNode, at);
5146

52-
Set<ValidationMessage> errors = new LinkedHashSet<ValidationMessage>();
47+
// get the Validator state object storing validation data
48+
ValidatorState state = (ValidatorState) CollectorContext.getInstance().get(ValidatorState.VALIDATOR_STATE_KEY);
49+
50+
Set<ValidationMessage> childSchemaErrors = new LinkedHashSet<ValidationMessage>();
5351

5452
// As AllOf might contain multiple schemas take a backup of evaluatedProperties.
5553
Object backupEvaluatedProperties = CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES);
5654

57-
// Make the evaluatedProperties list empty.
58-
CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, new ArrayList<>());
55+
List<String> totalEvaluatedProperties = new ArrayList<>();
5956

6057
for (JsonSchema schema : schemas) {
6158
try {
62-
errors.addAll(schema.validate(node, rootNode, at));
59+
// Make the evaluatedProperties list empty.
60+
CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, new ArrayList<>());
61+
62+
Set<ValidationMessage> localErrors = new HashSet<>();
63+
64+
if (!state.isWalkEnabled()) {
65+
localErrors = schema.validate(node, rootNode, at);
66+
} else {
67+
localErrors = schema.walk(node, rootNode, at, true);
68+
}
69+
70+
childSchemaErrors.addAll(localErrors);
71+
72+
// Keep Collecting total evaluated properties.
73+
if (localErrors.isEmpty()) {
74+
totalEvaluatedProperties.addAll((List<String>) CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES));
75+
}
6376

6477
if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) {
6578
final Iterator<JsonNode> arrayElements = schemaNode.elements();
@@ -93,28 +106,29 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
93106
}
94107
}
95108
} finally {
96-
if (errors.isEmpty()) {
109+
if (childSchemaErrors.isEmpty()) {
97110
List<String> backupEvaluatedPropertiesList = (backupEvaluatedProperties == null ? new ArrayList<>() : (List<String>) backupEvaluatedProperties);
98-
backupEvaluatedPropertiesList.addAll((List<String>) CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES));
111+
backupEvaluatedPropertiesList.addAll(totalEvaluatedProperties);
99112
CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedPropertiesList);
100113
} else {
101114
CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedProperties);
102115
}
103116
}
104117
}
105118

106-
return Collections.unmodifiableSet(errors);
119+
return Collections.unmodifiableSet(childSchemaErrors);
107120
}
108121

109122
@Override
110123
public Set<ValidationMessage> walk(JsonNode node, JsonNode rootNode, String at, boolean shouldValidateSchema) {
111-
Set<ValidationMessage> validationMessages = new LinkedHashSet<ValidationMessage>();
112-
124+
if (shouldValidateSchema) {
125+
return validate(node, rootNode, at);
126+
}
113127
for (JsonSchema schema : schemas) {
114128
// Walk through the schema
115-
validationMessages.addAll(schema.walk(node, rootNode, at, shouldValidateSchema));
129+
schema.walk(node, rootNode, at, false);
116130
}
117-
return Collections.unmodifiableSet(validationMessages);
131+
return Collections.emptySet();
118132
}
119133

120134
@Override

‎src/main/java/com/networknt/schema/AnyOfValidator.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public AnyOfValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentS
5353
public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String at) {
5454
debug(logger, node, rootNode, at);
5555

56+
// get the Validator state object storing validation data
57+
ValidatorState state = (ValidatorState) CollectorContext.getInstance().get(ValidatorState.VALIDATOR_STATE_KEY);
58+
5659
if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) {
5760
validationContext.enterDiscriminatorContext(this.discriminatorContext, at);
5861
}
@@ -68,6 +71,7 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
6871

6972
try {
7073
for (JsonSchema schema : schemas) {
74+
Set<ValidationMessage> errors = new HashSet<>();
7175
if (schema.getValidators().containsKey(typeValidatorName)) {
7276
TypeValidator typeValidator = ((TypeValidator) schema.getValidators().get(typeValidatorName));
7377
//If schema has type validator and node type doesn't match with schemaType then ignore it
@@ -77,7 +81,11 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
7781
continue;
7882
}
7983
}
80-
Set<ValidationMessage> errors = schema.validate(node, rootNode, at);
84+
if (!state.isWalkEnabled()) {
85+
errors = schema.validate(node, rootNode, at);
86+
} else {
87+
errors = schema.walk(node, rootNode, at, true);
88+
}
8189
if (errors.isEmpty() && (!this.validationContext.getConfig().isOpenAPI3StyleDiscriminators())) {
8290
// Clear all errors.
8391
allErrors.clear();
@@ -125,21 +133,13 @@ private void addEvaluatedProperties(Object backupEvaluatedProperties) {
125133

126134
@Override
127135
public Set<ValidationMessage> walk(JsonNode node, JsonNode rootNode, String at, boolean shouldValidateSchema) {
128-
ArrayList<Set<ValidationMessage>> results = new ArrayList<>(schemas.size());
129-
for (JsonSchema schema : schemas) {
130-
results.add(schema.walk(node, rootNode, at, shouldValidateSchema));
136+
if (shouldValidateSchema) {
137+
return validate(node, rootNode, at);
131138
}
132-
if(! shouldValidateSchema) {
133-
return new LinkedHashSet<>();
134-
}
135-
boolean atLeastOneValid = results.stream()
136-
.anyMatch(Set::isEmpty);
137-
if(atLeastOneValid) {
138-
return new LinkedHashSet<>();
139+
for (JsonSchema schema : schemas) {
140+
schema.walk(node, rootNode, at, false);
139141
}
140-
return results.stream()
141-
.flatMap(Collection::stream)
142-
.collect(Collectors.toCollection(LinkedHashSet::new));
142+
return new LinkedHashSet<>();
143143
}
144144

145145
@Override

‎src/main/java/com/networknt/schema/IfValidator.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
7171

7272
Set<ValidationMessage> errors = new LinkedHashSet<ValidationMessage>();
7373

74-
boolean ifConditionPassed;
74+
boolean ifConditionPassed = false;
7575
try {
7676
try {
7777
ifConditionPassed = ifSchema.validate(node, rootNode, at).isEmpty();
@@ -108,7 +108,8 @@ public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String
108108
if (errors.isEmpty()) {
109109
List<String> backupEvaluatedPropertiesList = (backupEvaluatedProperties == null ? new ArrayList<>() : (List<String>) backupEvaluatedProperties);
110110

111-
if (ifEvaluatedProperties != null) {
111+
// If the "if" keyword condition is passed then only add if properties as evaluated.
112+
if (ifEvaluatedProperties != null && ifConditionPassed) {
112113
backupEvaluatedPropertiesList.addAll((List<String>) ifEvaluatedProperties);
113114
}
114115

‎src/main/java/com/networknt/schema/JsonSchema.java‎

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,14 @@ private JsonNode getMessageNode(JsonNode schemaNode, JsonSchema parentSchema) {
258258

259259
@Override
260260
public Set<ValidationMessage> validate(JsonNode node) {
261-
Set<ValidationMessage> errors = validate(node, node, AT_ROOT);
262-
// Process UnEvaluatedProperties after all the validators are called.
263-
errors.addAll(processUnEvaluatedProperties(node, node, AT_ROOT, true, true));
264-
return errors;
261+
try {
262+
Set<ValidationMessage> errors = validate(node, node, AT_ROOT);
263+
return errors;
264+
} finally {
265+
if (validationContext.getConfig().isResetCollectorContext()) {
266+
CollectorContext.getInstance().reset();
267+
}
268+
}
265269
}
266270

267271
public Set<ValidationMessage> validate(JsonNode jsonNode, JsonNode rootNode, String at) {
@@ -274,6 +278,10 @@ public Set<ValidationMessage> validate(JsonNode jsonNode, JsonNode rootNode, Str
274278
for (JsonValidator v : getValidators().values()) {
275279
errors.addAll(v.validate(jsonNode, rootNode, at));
276280
}
281+
282+
// Process UnEvaluatedProperties after all the validators are called if there are no errors.
283+
errors.addAll(processUnEvaluatedProperties(jsonNode, rootNode, at, true, true));
284+
277285
if (null != config && config.isOpenAPI3StyleDiscriminators()) {
278286
ObjectNode discriminator = (ObjectNode) schemaNode.get("discriminator");
279287
if (null != discriminator) {
@@ -324,10 +332,10 @@ private ValidationResult validateAndCollect(JsonNode jsonNode, JsonNode rootNode
324332
SchemaValidatorsConfig config = validationContext.getConfig();
325333
// Get the collector context from the thread local.
326334
CollectorContext collectorContext = getCollectorContext();
335+
// Set the walkEnabled and isValidationEnabled flag in internal validator state.
336+
setValidatorState(false, true);
327337
// Validate.
328338
Set<ValidationMessage> errors = validate(jsonNode, rootNode, at);
329-
// Validate UnEvaluatedProperties after all the validators are processed.
330-
errors.addAll(processUnEvaluatedProperties(jsonNode, rootNode, at, true, true));
331339
// When walk is called in series of nested call we don't want to load the collectors every time. Leave to the API to decide when to call collectors.
332340
if (config.doLoadCollectors()) {
333341
// Load all the data from collectors into the context.
@@ -337,7 +345,9 @@ private ValidationResult validateAndCollect(JsonNode jsonNode, JsonNode rootNode
337345
ValidationResult validationResult = new ValidationResult(errors, collectorContext);
338346
return validationResult;
339347
} finally {
340-
ThreadInfo.remove(CollectorContext.COLLECTOR_CONTEXT_THREAD_LOCAL_KEY);
348+
if (validationContext.getConfig().isResetCollectorContext()) {
349+
CollectorContext.getInstance().reset();
350+
}
341351
}
342352
}
343353

@@ -353,24 +363,30 @@ private ValidationResult validateAndCollect(JsonNode jsonNode, JsonNode rootNode
353363
* @return result of ValidationResult
354364
*/
355365
public ValidationResult walk(JsonNode node, boolean shouldValidateSchema) {
356-
// Get the config.
357-
SchemaValidatorsConfig config = validationContext.getConfig();
358-
// Get the collector context from the thread local.
359-
CollectorContext collectorContext = getCollectorContext();
360-
// Set the walkEnabled flag in internal validator state.
361-
setValidatorState(true, shouldValidateSchema);
362-
// Walk through the schema.
363-
Set<ValidationMessage> errors = walk(node, node, AT_ROOT, shouldValidateSchema);
364-
// When walk is called in series of nested call we don't want to load the collectors every time. Leave to the API to decide when to call collectors.
365-
if (config.doLoadCollectors()) {
366-
// Load all the data from collectors into the context.
367-
collectorContext.loadCollectors();
366+
try {
367+
// Get the config.
368+
SchemaValidatorsConfig config = validationContext.getConfig();
369+
// Get the collector context from the thread local.
370+
CollectorContext collectorContext = getCollectorContext();
371+
// Set the walkEnabled flag in internal validator state.
372+
setValidatorState(true, shouldValidateSchema);
373+
// Walk through the schema.
374+
Set<ValidationMessage> errors = walk(node, node, AT_ROOT, shouldValidateSchema);
375+
// When walk is called in series of nested call we don't want to load the collectors every time. Leave to the API to decide when to call collectors.
376+
if (config.doLoadCollectors()) {
377+
// Load all the data from collectors into the context.
378+
collectorContext.loadCollectors();
379+
}
380+
// Process UnEvaluatedProperties after all the validators are called.
381+
errors.addAll(processUnEvaluatedProperties(node, node, AT_ROOT, shouldValidateSchema, false));
382+
// Collect errors and collector context into validation result.
383+
ValidationResult validationResult = new ValidationResult(errors, collectorContext);
384+
return validationResult;
385+
} finally {
386+
if (validationContext.getConfig().isResetCollectorContext()) {
387+
CollectorContext.getInstance().reset();
388+
}
368389
}
369-
// Process UnEvaluatedProperties after all the validators are called.
370-
errors.addAll(processUnEvaluatedProperties(node, node, AT_ROOT, shouldValidateSchema, false));
371-
// Collect errors and collector context into validation result.
372-
ValidationResult validationResult = new ValidationResult(errors, collectorContext);
373-
return validationResult;
374390
}
375391

376392
@Override
@@ -408,6 +424,10 @@ public Set<ValidationMessage> walk(JsonNode node, JsonNode rootNode, String at,
408424
validationMessages);
409425
}
410426
}
427+
if (shouldValidateSchema) {
428+
// Process UnEvaluatedProperties after all the validators are called if there are no errors.
429+
validationMessages.addAll(processUnEvaluatedProperties(node, rootNode, at, true, true));
430+
}
411431
return validationMessages;
412432
}
413433

0 commit comments

Comments
 (0)