Skip to content

Commit 830acfd

Browse files
justin-tayjustin-tay
committed
Refactor and fix discriminator
1 parent 276928e commit 830acfd

File tree

6 files changed

+371
-46
lines changed

6 files changed

+371
-46
lines changed

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@
1717
package com.networknt.schema;
1818

1919
import com.networknt.schema.annotation.Annotations;
20+
import com.networknt.schema.keyword.DiscriminatorState;
2021
import com.networknt.schema.path.NodePath;
2122
import com.networknt.schema.result.SchemaResults;
2223
import com.networknt.schema.walk.WalkConfig;
2324

2425
import java.util.ArrayList;
26+
import java.util.HashMap;
2527
import java.util.List;
28+
import java.util.Map;
2629
import java.util.Stack;
2730
import java.util.function.Consumer;
2831

@@ -38,8 +41,14 @@ public class ExecutionContext {
3841
private Annotations annotations = null;
3942
private SchemaResults results = null;
4043
private List<Error> errors = new ArrayList<>();
44+
45+
private Map<NodePath, DiscriminatorState> discriminatorMapping = new HashMap<>();
4146

42-
/**
47+
public Map<NodePath, DiscriminatorState> getDiscriminatorMapping() {
48+
return discriminatorMapping;
49+
}
50+
51+
/**
4352
* This is used during the execution to determine if the validator should fail fast.
4453
* <p>
4554
* This valid is determined by the previous validator.

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

Lines changed: 72 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@
3434
* {@link KeywordValidator} for anyOf.
3535
*/
3636
public class AnyOfValidator extends BaseKeywordValidator {
37-
private static final String DISCRIMINATOR_REMARK = "and the discriminator-selected candidate schema didn't pass validation";
38-
3937
private final List<Schema> schemas;
4038

4139
private Boolean canShortCircuit = null;
@@ -71,7 +69,8 @@ protected void validate(ExecutionContext executionContext, JsonNode node, JsonNo
7169
}
7270
int numberOfValidSubSchemas = 0;
7371
List<Error> existingErrors = executionContext.getErrors();
74-
List<Error> allErrors = null;
72+
List<Error> allErrors = null; // Keeps track of all the errors for reporting if in the end none of the schemas match
73+
List<Error> discriminatorErrors = null;
7574
List<Error> errors = new ArrayList<>();
7675
executionContext.setErrors(errors);
7776
try {
@@ -109,64 +108,97 @@ protected void validate(ExecutionContext executionContext, JsonNode node, JsonNo
109108

110109
if (errors.isEmpty() && (!this.schemaContext.isDiscriminatorKeywordEnabled())
111110
&& canShortCircuit() && canShortCircuit(executionContext)) {
112-
// Clear all errors. Note that this is checked in finally.
113-
allErrors = null;
111+
// Successful so return only the existing errors, ie. no new errors
114112
executionContext.setErrors(existingErrors);
115113
return;
116114
} else if (this.schemaContext.isDiscriminatorKeywordEnabled()) {
117-
DiscriminatorContext currentDiscriminatorContext = executionContext.getCurrentDiscriminatorContext();
118-
if (currentDiscriminatorContext.isDiscriminatorMatchFound()
119-
|| currentDiscriminatorContext.isDiscriminatorIgnore()) {
120-
if (!errors.isEmpty()) {
121-
// The following is to match the previous logic adding to all errors
122-
// which is generally discarded as it returns errors but the allErrors
123-
// is getting processed in finally
124-
if (allErrors == null) {
125-
allErrors = new ArrayList<>();
126-
}
127-
allErrors.add(error().instanceNode(node).instanceLocation(instanceLocation)
128-
.locale(executionContext.getExecutionConfig().getLocale())
129-
.arguments(DISCRIMINATOR_REMARK)
130-
.build());
131-
} else {
132-
// Clear all errors. Note that this is checked in finally.
133-
allErrors = null;
134-
}
135-
existingErrors.addAll(errors);
136-
executionContext.setErrors(existingErrors);
137-
return;
115+
JsonNode refNode = schema.getSchemaNode().get("$ref");
116+
DiscriminatorState state = executionContext.getDiscriminatorMapping().get(instanceLocation);
117+
boolean discriminatorMatchFound = false;
118+
if (refNode != null) {
119+
// Check if there is a match
120+
String discriminatingValue = state.getDiscriminatingValue();
121+
if (discriminatingValue != null) {
122+
String ref = refNode.asText();
123+
if (state.isExplicitMapping() && ref.equals(discriminatingValue)) {
124+
// Explicit matching
125+
discriminatorMatchFound = true;
126+
state.setMatch(ref);
127+
} else if (!state.isExplicitMapping() && ref.endsWith(discriminatingValue)) {
128+
// Implicit matching
129+
discriminatorMatchFound = true;
130+
state.setMatch(ref);
131+
}
132+
}
133+
}
134+
if (discriminatorMatchFound) {
135+
/*
136+
* Note that discriminator cannot change the outcome of the evaluation but can be used to filter off
137+
* any additional messages
138+
*/
139+
if (!errors.isEmpty()) {
140+
/*
141+
* This means that the discriminated value has errors and doesn't match so these errors
142+
* are the only ones that will be reported *IF* there are no other schemas that successfully
143+
* validate to meet the requirement of anyOf.
144+
*
145+
* If there are any successful schemas as per anyOf, all these errors will be discarded.
146+
*/
147+
discriminatorErrors = new ArrayList<>(errors);
148+
allErrors = null; // This is no longer needed
149+
}
138150
}
139151
}
140-
if (allErrors == null) {
141-
allErrors = new ArrayList<>();
152+
153+
/*
154+
* This adds all the errors for this schema to the list that contains all the errors for later reporting.
155+
*
156+
* There's no need to add these if there was a discriminator match with errors as only the discriminator
157+
* errors will be reported if all the schemas fail.
158+
*/
159+
if (!errors.isEmpty() && discriminatorErrors == null) {
160+
if (allErrors == null) {
161+
allErrors = new ArrayList<>();
162+
}
163+
allErrors.addAll(errors);
142164
}
143-
allErrors.addAll(errors);
144165
}
145166
} finally {
146167
// Restore flag
147168
executionContext.setFailFast(failFast);
148169
}
149170

150-
if (this.schemaContext.isDiscriminatorKeywordEnabled()
151-
&& executionContext.getCurrentDiscriminatorContext().isActive()
152-
&& !executionContext.getCurrentDiscriminatorContext().isDiscriminatorIgnore()) {
153-
existingErrors.add(error().instanceNode(node).instanceLocation(instanceLocation)
154-
.locale(executionContext.getExecutionConfig().getLocale())
155-
.arguments(
156-
"based on the provided discriminator. No alternative could be chosen based on the discriminator property")
157-
.build());
158-
executionContext.setErrors(existingErrors);
159-
return;
171+
if (this.schemaContext.isDiscriminatorKeywordEnabled()) {
172+
// https://spec.openapis.org/oas/v3.1.0#discriminator-object
173+
// If the discriminator value does not match an implicit or explicit mapping, no schema can be determined and validation SHOULD fail. Mapping keys MUST be string values, but tooling MAY convert response values to strings for comparison.
174+
175+
/*
176+
* The only case where the discriminator can change the outcome of the result is if the discriminator value does not match an implicit or explicit mapping
177+
*/
178+
DiscriminatorState state = executionContext.getDiscriminatorMapping().get(instanceLocation);
179+
if (state != null && state.getMatch() == null && state.getPropertyValue() != null) {
180+
// state.getPropertyValue() == null means there is no value at propertyName
181+
existingErrors.add(error().keyword("discriminator").instanceNode(node).instanceLocation(instanceLocation)
182+
.locale(executionContext.getExecutionConfig().getLocale())
183+
.arguments(
184+
"based on the provided discriminator. No alternative could be chosen based on the discriminator property")
185+
.build());
186+
}
160187
}
161188
} finally {
162189
if (this.schemaContext.isDiscriminatorKeywordEnabled()) {
163190
executionContext.leaveDiscriminatorContextImmediately(instanceLocation);
164191
}
165192
}
166193
if (numberOfValidSubSchemas >= 1) {
194+
// Successful so return only the existing errors, ie. no new errors
167195
executionContext.setErrors(existingErrors);
168196
} else {
169-
if (allErrors != null) {
197+
if (discriminatorErrors != null) {
198+
// If errors are present matching the discriminator, only these errors should be reported
199+
existingErrors.addAll(discriminatorErrors);
200+
} else if (allErrors != null) {
201+
// As the anyOf has failed, report all the errors
170202
existingErrors.addAll(allErrors);
171203
}
172204
executionContext.setErrors(existingErrors);

‎src/main/java/com/networknt/schema/keyword/DiscriminatorState.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package com.networknt.schema.keyword;
2+
3+
/**
4+
* Discriminator state for an instance location.
5+
*/
6+
public class DiscriminatorState {
7+
private String propertyName;
8+
private String propertyValue;
9+
private String discriminatingValue = null;
10+
private boolean explicitMapping = false;
11+
private String match;
12+
13+
public String getPropertyName() {
14+
return propertyName;
15+
}
16+
17+
public void setPropertyName(String propertyName) {
18+
this.propertyName = propertyName;
19+
}
20+
21+
public String getPropertyValue() {
22+
return propertyValue;
23+
}
24+
25+
public void setPropertyValue(String propertyValue) {
26+
this.propertyValue = propertyValue;
27+
}
28+
29+
public String getDiscriminatingValue() {
30+
return discriminatingValue;
31+
}
32+
33+
public void setDiscriminatingValue(String discriminatingValue) {
34+
this.discriminatingValue = discriminatingValue;
35+
}
36+
37+
public boolean isExplicitMapping() {
38+
return explicitMapping;
39+
}
40+
41+
public void setExplicitMapping(boolean explicitMapping) {
42+
this.explicitMapping = explicitMapping;
43+
}
44+
45+
public void setMatch(String match) {
46+
this.match = match;
47+
}
48+
49+
public String getMatch() {
50+
return this.match;
51+
}
52+
}

‎src/main/java/com/networknt/schema/keyword/DiscriminatorValidator.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,31 @@ public DiscriminatorValidator(SchemaLocation schemaLocation, NodePath evaluation
6767
@Override
6868
public void validate(ExecutionContext executionContext, JsonNode node, JsonNode rootNode,
6969
NodePath instanceLocation) {
70-
// Do nothing
70+
// Check for discriminator mapping
71+
if ("".equals(this.propertyName)) {
72+
// Invalid discriminator as propertyName cannot be empty
73+
return;
74+
}
75+
JsonNode propertyValue = node.get(this.propertyName);
76+
DiscriminatorState state = new DiscriminatorState();
77+
DiscriminatorState existing = executionContext.getDiscriminatorMapping().put(instanceLocation, state);
78+
state.setPropertyName(this.propertyName);
79+
if (propertyValue != null && propertyValue.isTextual()) {
80+
String value = propertyValue.asText();
81+
state.setPropertyValue(value);
82+
// Check for explicit mapping
83+
String mapped = mapping.get(value);
84+
if (mapped == null) {
85+
// If explicit mapping not found use implicit value
86+
state.setDiscriminatingValue(value);
87+
state.setExplicitMapping(false);
88+
} else {
89+
state.setDiscriminatingValue(mapped);
90+
state.setExplicitMapping(true);
91+
}
92+
} else {
93+
// discriminator property name is missing so the property value in the state is null
94+
}
7195
}
7296

7397
/**
@@ -162,7 +186,7 @@ public static void registerAndMergeDiscriminator(final DiscriminatorContext curr
162186
final JsonNode mappingValueToAdd = fieldToAdd.getValue();
163187

164188
final JsonNode currentMappingValue = mappingOnContextDiscriminator.get(mappingKeyToAdd);
165-
if (null != currentMappingValue && currentMappingValue != mappingValueToAdd) {
189+
if (null != currentMappingValue && !currentMappingValue.equals(mappingValueToAdd)) {
166190
throw new SchemaException(instanceLocation + "discriminator mapping redefinition from " + mappingKeyToAdd
167191
+ "/" + currentMappingValue + " to " + mappingValueToAdd);
168192
} else if (null == currentMappingValue) {

0 commit comments

Comments
 (0)