Skip to content

Commit 6f44455

Browse files
justin-tayjustin-tay
authored
Explicitly handle if the discriminator property value is null (#988)
1 parent 0f983b0 commit 6f44455

File tree

5 files changed

+197
-15
lines changed

5 files changed

+197
-15
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ && canShortCircuit() && canShortCircuit(executionContext)) {
106106
// return empty errors.
107107
return errors;
108108
} else if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) {
109-
if (executionContext.getCurrentDiscriminatorContext().isDiscriminatorMatchFound()) {
109+
DiscriminatorContext currentDiscriminatorContext = executionContext.getCurrentDiscriminatorContext();
110+
if (currentDiscriminatorContext.isDiscriminatorMatchFound()
111+
|| currentDiscriminatorContext.isDiscriminatorIgnore()) {
110112
if (!errors.isEmpty()) {
111113
// The following is to match the previous logic adding to all errors
112114
// which is generally discarded as it returns errors but the allErrors
@@ -137,7 +139,8 @@ && canShortCircuit() && canShortCircuit(executionContext)) {
137139
}
138140

139141
if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()
140-
&& executionContext.getCurrentDiscriminatorContext().isActive()) {
142+
&& executionContext.getCurrentDiscriminatorContext().isActive()
143+
&& !executionContext.getCurrentDiscriminatorContext().isDiscriminatorIgnore()) {
141144
return Collections.singleton(message().instanceNode(node).instanceLocation(instanceLocation)
142145
.locale(executionContext.getExecutionConfig().getLocale())
143146
.arguments(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ protected static void checkDiscriminatorMatch(final DiscriminatorContext current
114114
final String discriminatorPropertyValue,
115115
final JsonSchema jsonSchema) {
116116
if (discriminatorPropertyValue == null) {
117-
currentDiscriminatorContext.markMatch();
117+
currentDiscriminatorContext.markIgnore();
118118
return;
119119
}
120120

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ public class DiscriminatorContext {
1010

1111
private boolean discriminatorMatchFound = false;
1212

13+
private boolean discriminatorIgnore = false;
14+
1315
public void registerDiscriminator(final SchemaLocation schemaLocation, final ObjectNode discriminator) {
1416
this.discriminators.put("#" + schemaLocation.getFragment().toString(), discriminator);
1517
}
@@ -26,10 +28,25 @@ public void markMatch() {
2628
this.discriminatorMatchFound = true;
2729
}
2830

31+
/**
32+
* Indicate that discriminator processing should be ignored.
33+
* <p>
34+
* This is used when the discriminator property value is missing from the data.
35+
* <p>
36+
* See issue #436 for background.
37+
*/
38+
public void markIgnore() {
39+
this.discriminatorIgnore = true;
40+
}
41+
2942
public boolean isDiscriminatorMatchFound() {
3043
return this.discriminatorMatchFound;
3144
}
3245

46+
public boolean isDiscriminatorIgnore() {
47+
return this.discriminatorIgnore;
48+
}
49+
3350
/**
3451
* Returns true if we have a discriminator active. In this case no valid match in anyOf should lead to validation failure
3552
*

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

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,23 +110,37 @@ public Set<ValidationMessage> validate(ExecutionContext executionContext, JsonNo
110110
// matching discriminator to be discarded. Note that the discriminator cannot
111111
// affect the actual validation result.
112112
if (discriminator != null && !discriminator.getPropertyName().isEmpty()) {
113-
String discriminatorPropertyValue = node.get(discriminator.getPropertyName()).asText();
114-
discriminatorPropertyValue = discriminator.getMapping().getOrDefault(discriminatorPropertyValue,
115-
discriminatorPropertyValue);
116-
JsonNode refNode = schema.getSchemaNode().get("$ref");
117-
if (refNode != null) {
118-
String ref = refNode.asText();
119-
if (ref.equals(discriminatorPropertyValue) || ref.endsWith("/" + discriminatorPropertyValue)) {
120-
executionContext.getCurrentDiscriminatorContext().markMatch();
113+
JsonNode discriminatorPropertyNode = node.get(discriminator.getPropertyName());
114+
if (discriminatorPropertyNode != null) {
115+
String discriminatorPropertyValue = discriminatorPropertyNode.asText();
116+
discriminatorPropertyValue = discriminator.getMapping().getOrDefault(discriminatorPropertyValue,
117+
discriminatorPropertyValue);
118+
JsonNode refNode = schema.getSchemaNode().get("$ref");
119+
if (refNode != null) {
120+
String ref = refNode.asText();
121+
if (ref.equals(discriminatorPropertyValue) || ref.endsWith("/" + discriminatorPropertyValue)) {
122+
executionContext.getCurrentDiscriminatorContext().markMatch();
123+
}
121124
}
125+
} else {
126+
// See issue 436 where the condition was relaxed to not cause an assertion
127+
// due to missing discriminator property value
128+
// Also see BaseJsonValidator#checkDiscriminatorMatch
129+
executionContext.getCurrentDiscriminatorContext().markIgnore();
122130
}
123131
}
124-
boolean discriminatorMatchFound = executionContext.getCurrentDiscriminatorContext().isDiscriminatorMatchFound();
125-
if (discriminatorMatchFound && childErrors == null) {
132+
DiscriminatorContext currentDiscriminatorContext = executionContext.getCurrentDiscriminatorContext();
133+
if (currentDiscriminatorContext.isDiscriminatorMatchFound() && childErrors == null) {
126134
// Note that the match is set if found and not reset so checking if childErrors
127135
// found is null triggers on the correct schema
128136
childErrors = new SetView<>();
129137
childErrors.union(schemaErrors);
138+
} else if (currentDiscriminatorContext.isDiscriminatorIgnore()) {
139+
// This is the normal handling when discriminators aren't enabled
140+
if (childErrors == null) {
141+
childErrors = new SetView<>();
142+
}
143+
childErrors.union(schemaErrors);
130144
}
131145
} else if (!schemaErrors.isEmpty() && reportChildErrors(executionContext)) {
132146
// This is the normal handling when discriminators aren't enabled
@@ -140,7 +154,8 @@ public Set<ValidationMessage> validate(ExecutionContext executionContext, JsonNo
140154

141155
if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()
142156
&& (discriminator != null || executionContext.getCurrentDiscriminatorContext().isActive())
143-
&& !executionContext.getCurrentDiscriminatorContext().isDiscriminatorMatchFound()) {
157+
&& !executionContext.getCurrentDiscriminatorContext().isDiscriminatorMatchFound()
158+
&& !executionContext.getCurrentDiscriminatorContext().isDiscriminatorIgnore()) {
144159
errors = Collections.singleton(message().instanceNode(node).instanceLocation(instanceLocation)
145160
.locale(executionContext.getExecutionConfig().getLocale())
146161
.arguments(

‎src/test/java/com/networknt/schema/DiscriminatorValidatorTest.java

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ void discriminatorInOneOfShouldOnlyReportErrorsInMatchingDiscriminator() {
538538
assertEquals("required", list.get(1).getType());
539539
assertEquals("numberOfBeds", list.get(1).getProperty());
540540
}
541-
541+
542542
@Test
543543
void discriminatorMappingInOneOfShouldOnlyReportErrorsInMatchingDiscriminator() {
544544
String schemaData = "{\r\n"
@@ -640,4 +640,151 @@ void discriminatorMappingInOneOfShouldOnlyReportErrorsInMatchingDiscriminator()
640640
assertEquals("numberOfBeds", list.get(1).getProperty());
641641
}
642642

643+
/**
644+
* See issue 436 and 985.
645+
*/
646+
@Test
647+
void oneOfMissingDiscriminatorValue() {
648+
String schemaData = " {\r\n"
649+
+ " \"type\": \"object\",\r\n"
650+
+ " \"discriminator\": { \"propertyName\": \"name\" },\r\n"
651+
+ " \"oneOf\": [\r\n"
652+
+ " {\r\n"
653+
+ " \"$ref\": \"#/defs/Foo\"\r\n"
654+
+ " },\r\n"
655+
+ " {\r\n"
656+
+ " \"$ref\": \"#/defs/Bar\"\r\n"
657+
+ " }\r\n"
658+
+ " ],\r\n"
659+
+ " \"defs\": {\r\n"
660+
+ " \"Foo\": {\r\n"
661+
+ " \"type\": \"object\",\r\n"
662+
+ " \"properties\": {\r\n"
663+
+ " \"name\": {\r\n"
664+
+ " \"const\": \"Foo\"\r\n"
665+
+ " }\r\n"
666+
+ " },\r\n"
667+
+ " \"required\": [ \"name\" ],\r\n"
668+
+ " \"additionalProperties\": false\r\n"
669+
+ " },\r\n"
670+
+ " \"Bar\": {\r\n"
671+
+ " \"type\": \"object\",\r\n"
672+
+ " \"properties\": {\r\n"
673+
+ " \"name\": {\r\n"
674+
+ " \"const\": \"Bar\"\r\n"
675+
+ " }\r\n"
676+
+ " },\r\n"
677+
+ " \"required\": [ \"name\" ],\r\n"
678+
+ " \"additionalProperties\": false\r\n"
679+
+ " }\r\n"
680+
+ " }\r\n"
681+
+ " }";
682+
683+
String inputData = "{}";
684+
685+
JsonSchemaFactory factory = JsonSchemaFactory.getInstance(VersionFlag.V202012);
686+
SchemaValidatorsConfig config = new SchemaValidatorsConfig();
687+
config.setOpenAPI3StyleDiscriminators(true);
688+
JsonSchema schema = factory.getSchema(schemaData, config);
689+
Set<ValidationMessage> messages = schema.validate(inputData, InputFormat.JSON);
690+
assertEquals(3, messages.size());
691+
List<ValidationMessage> list = messages.stream().collect(Collectors.toList());
692+
assertEquals("oneOf", list.get(0).getType());
693+
assertEquals("required", list.get(1).getType());
694+
assertEquals("required", list.get(2).getType());
695+
}
696+
697+
/**
698+
* See issue 436.
699+
*/
700+
@Test
701+
void anyOfMissingDiscriminatorValue() {
702+
String schemaData = "{\r\n"
703+
+ " \"type\": \"array\",\r\n"
704+
+ " \"items\": {\r\n"
705+
+ " \"anyOf\": [\r\n"
706+
+ " {\r\n"
707+
+ " \"$ref\": \"#/components/schemas/Kitchen\"\r\n"
708+
+ " },\r\n"
709+
+ " {\r\n"
710+
+ " \"$ref\": \"#/components/schemas/BedRoom\"\r\n"
711+
+ " }\r\n"
712+
+ " ]\r\n"
713+
+ " },\r\n"
714+
+ " \"components\": {\r\n"
715+
+ " \"schemas\": {\r\n"
716+
+ " \"Room\": {\r\n"
717+
+ " \"type\": \"object\",\r\n"
718+
+ " \"properties\": {\r\n"
719+
+ " \"@type\": {\r\n"
720+
+ " \"type\": \"string\"\r\n"
721+
+ " }\r\n"
722+
+ " },\r\n"
723+
+ " \"required\": [\r\n"
724+
+ " \"@type\"\r\n"
725+
+ " ],\r\n"
726+
+ " \"discriminator\": {\r\n"
727+
+ " \"propertyName\": \"@type\"\r\n"
728+
+ " }\r\n"
729+
+ " },\r\n"
730+
+ " \"BedRoom\": {\r\n"
731+
+ " \"type\": \"object\",\r\n"
732+
+ " \"allOf\": [\r\n"
733+
+ " {\r\n"
734+
+ " \"$ref\": \"#/components/schemas/Room\"\r\n"
735+
+ " },\r\n"
736+
+ " {\r\n"
737+
+ " \"type\": \"object\",\r\n"
738+
+ " \"properties\": {\r\n"
739+
+ " \"numberOfBeds\": {\r\n"
740+
+ " \"type\": \"integer\"\r\n"
741+
+ " }\r\n"
742+
+ " },\r\n"
743+
+ " \"required\": [\r\n"
744+
+ " \"numberOfBeds\"\r\n"
745+
+ " ]\r\n"
746+
+ " }\r\n"
747+
+ " ]\r\n"
748+
+ " },\r\n"
749+
+ " \"Kitchen\": {\r\n"
750+
+ " \"type\": \"object\",\r\n"
751+
+ " \"allOf\": [\r\n"
752+
+ " {\r\n"
753+
+ " \"$ref\": \"#/components/schemas/Room\"\r\n"
754+
+ " },\r\n"
755+
+ " {\r\n"
756+
+ " \"type\": \"object\",\r\n"
757+
+ " \"properties\": {\r\n"
758+
+ " \"hasMicrowaveOven\": {\r\n"
759+
+ " \"type\": \"boolean\"\r\n"
760+
+ " }\r\n"
761+
+ " },\r\n"
762+
+ " \"required\": [\r\n"
763+
+ " \"hasMicrowaveOven\"\r\n"
764+
+ " ]\r\n"
765+
+ " }\r\n"
766+
+ " ]\r\n"
767+
+ " }\r\n"
768+
+ " }\r\n"
769+
+ " }\r\n"
770+
+ "}";
771+
772+
String inputData = "[\r\n"
773+
+ " {\r\n"
774+
+ " \"hasMicrowaveOven\": true\r\n"
775+
+ " },\r\n"
776+
+ " {\r\n"
777+
+ " \"@type\": \"BedRoom\",\r\n"
778+
+ " \"numberOfBeds\": 4\r\n"
779+
+ " }\r\n"
780+
+ "]";
781+
782+
JsonSchemaFactory factory = JsonSchemaFactory.getInstance(VersionFlag.V202012);
783+
SchemaValidatorsConfig config = new SchemaValidatorsConfig();
784+
config.setOpenAPI3StyleDiscriminators(true);
785+
JsonSchema schema = factory.getSchema(schemaData, config);
786+
Set<ValidationMessage> messages = schema.validate(inputData, InputFormat.JSON);
787+
List<ValidationMessage> list = messages.stream().collect(Collectors.toList());
788+
assertEquals("required", list.get(0).getType());
789+
}
643790
}

0 commit comments

Comments
 (0)