Repository: networknt/json-schema-validator
Pull request #397 (2021-04-11 11:28:53)

Proposed fix for #396: Implement propertyNames in terms full schema validation

codecov-io: # [Codecov](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=h1) Report
> Merging [#397](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=desc) (28fd498) into [master](https://codecov.io/gh/networknt/json-schema-validator/commit/dfd63aa54887238f9295efbd316d7742e8f34eed?el=desc) (dfd63aa) will **decrease** coverage by `0.08%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/networknt/json-schema-validator/pull/397/graphs/tree.svg?width=650&height=150&src=pr&token=Dq6aZ2eKq9)](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master     #397      +/-   ##
============================================
- Coverage     70.68%   70.60%   -0.09%     
+ Complexity      753      744       -9     
============================================
  Files            81       81              
  Lines          2900     2885      -15     
  Branches        596      589       -7     
============================================
- Hits           2050     2037      -13     
+ Misses          628      627       -1     
+ Partials        222      221       -1     
```


| [Impacted Files](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...a/com/networknt/schema/PropertyNamesValidator.java](https://codecov.io/gh/networknt/json-schema-validator/pull/397/diff?src=pr&el=tree#diff-c3JjL21haW4vamF2YS9jb20vbmV0d29ya250L3NjaGVtYS9Qcm9wZXJ0eU5hbWVzVmFsaWRhdG9yLmphdmE=) | `100.00% <100.00%> (+2.94%)` | `6.00 <0.00> (-11.00)` | :arrow_up: |
| [...n/java/com/networknt/schema/ValidationMessage.java](https://codecov.io/gh/networknt/json-schema-validator/pull/397/diff?src=pr&el=tree#diff-c3JjL21haW4vamF2YS9jb20vbmV0d29ya250L3NjaGVtYS9WYWxpZGF0aW9uTWVzc2FnZS5qYXZh) | `62.50% <0.00%> (+1.38%)` | `13.00% <0.00%> (+1.00%)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=footer). Last update [dfd63aa...28fd498](https://codecov.io/gh/networknt/json-schema-validator/pull/397?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/main/java/com/networknt/schema/PropertyNamesValidator.java b/src/main/java/com/networknt/schema/PropertyNamesValidator.java
index 23a92d5..a001ed6 100644
--- a/src/main/java/com/networknt/schema/PropertyNamesValidator.java
+++ b/src/main/java/com/networknt/schema/PropertyNamesValidator.java
@@ -15,60 +15,40 @@
  */
 package com.networknt.schema;
 
-import com.fasterxml.jackson.databind.JsonNode;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.*;
-import java.util.regex.Pattern;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.TextNode;
 
 public class PropertyNamesValidator extends BaseJsonValidator implements JsonValidator {
     private static final Logger logger = LoggerFactory.getLogger(PropertyNamesValidator.class);
-    private Map<String, JsonSchema> schemas;
-    private boolean schemaValue = false;
+    private final JsonSchema innerSchema;
     public PropertyNamesValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentSchema, ValidationContext validationContext) {
         super(schemaPath, schemaNode, parentSchema, ValidatorTypeCode.PROPERTYNAMES, validationContext);
-        if(schemaNode.isBoolean()) {
-            schemaValue = schemaNode.booleanValue();
-        } else {
-            schemas = new HashMap<String, JsonSchema>();
-            for (Iterator<String> it = schemaNode.fieldNames(); it.hasNext(); ) {
-                String pname = it.next();
-                schemas.put(pname, new JsonSchema(validationContext, schemaPath + "/" + pname, parentSchema.getCurrentUri(), schemaNode.get(pname), parentSchema));
-            }
-        }
+        innerSchema = new JsonSchema(validationContext, schemaPath, parentSchema.getCurrentUri(), schemaNode, parentSchema);
     }
 
     public Set<ValidationMessage> validate(JsonNode node, JsonNode rootNode, String at) {
         debug(logger, node, rootNode, at);
 
         Set<ValidationMessage> errors = new LinkedHashSet<ValidationMessage>();
-        if(schemas != null) {
-            for (Map.Entry<String, JsonSchema> entry : schemas.entrySet()) {
-                JsonNode propertyNode = node.get(entry.getKey());
-                // check propertyNames
-                if (!node.isObject()) {
-                    continue;
-                }
-                for (Iterator<String> it = node.fieldNames(); it.hasNext(); ) {
-                    String pname = it.next();
-                    int maxLength = entry.getValue().getSchemaNode().intValue();
-                    if("maxLength".equals(entry.getKey()) && pname.length() > maxLength) {
-                        errors.add(buildValidationMessage(at + "." + pname, "maxLength " + maxLength));
-                    }
-                    int minLength = entry.getValue().getSchemaNode().intValue();
-                    if("minLength".equals(entry.getKey()) && pname.length() < minLength) {
-                        errors.add(buildValidationMessage(at + "." + pname, "minLength " + minLength));
-                    }
-                    String pattern = entry.getValue().getSchemaNode().textValue();
-                    if("pattern".equals(entry.getKey()) && !Pattern.matches(pattern,pname)) {
-                        errors.add(buildValidationMessage(at + "." + pname, "pattern " + pattern));
-                    }
-                }
-            }
-        } else {
-            if(!schemaValue && node.isObject() && node.size() != 0) {
-                errors.add(buildValidationMessage(at + "." + node, "false"));
+        for (Iterator<String> it = node.fieldNames(); it.hasNext(); ) {
+            final String pname = it.next();
+            final TextNode pnameText = TextNode.valueOf(pname);
+            final Set<ValidationMessage> schemaErrors = innerSchema.validate(pnameText, node, at + "." + pname);
+            for (final ValidationMessage schemaError : schemaErrors) {
+                final String path = schemaError.getPath();
+                String msg = schemaError.getMessage();
+                if (msg.startsWith(path))
+                    msg = msg.substring(path.length()).replaceFirst("^:\\s*", "");
+
+                errors.add(buildValidationMessage(schemaError.getPath(), msg));
             }
         }
         return Collections.unmodifiableSet(errors);


Test patch:
diff --git a/src/test/java/com/networknt/schema/Issue342Test.java b/src/test/java/com/networknt/schema/Issue342Test.java
new file mode 100644
index 0000000..42abf94
--- /dev/null
+++ b/src/test/java/com/networknt/schema/Issue342Test.java
@@ -0,0 +1,38 @@
+package com.networknt.schema;
+
+import java.io.InputStream;
+import java.util.Set;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class Issue342Test {
+	protected JsonSchema getJsonSchemaFromStreamContentV7(InputStream schemaContent) {
+		JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V7);
+		return factory.getSchema(schemaContent);
+	}
+
+	protected JsonNode getJsonNodeFromStreamContent(InputStream content) throws Exception {
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree(content);
+		return node;
+	}
+
+	@Test
+	public void propertyNameEnumShouldFailV7() throws Exception {
+		String schemaPath = "/schema/issue342-v7.json";
+		String dataPath = "/data/issue342.json";
+		InputStream schemaInputStream = getClass().getResourceAsStream(schemaPath);
+		JsonSchema schema = getJsonSchemaFromStreamContentV7(schemaInputStream);
+		InputStream dataInputStream = getClass().getResourceAsStream(dataPath);
+		JsonNode node = getJsonNodeFromStreamContent(dataInputStream);
+		Set<ValidationMessage> errors = schema.validate(node);
+		Assert.assertEquals(1, errors.size());
+		final ValidationMessage error = errors.iterator().next();
+		Assert.assertEquals("$.z", error.getPath());
+		Assert.assertEquals("Property name $.z is not valid for validation: does not have a value in the enumeration [a, b, c]", error.getMessage());
+	}
+}
diff --git a/src/test/java/com/networknt/schema/Issue375Test.java b/src/test/java/com/networknt/schema/Issue375Test.java
index ebdc5f7..24f36c0 100644
--- a/src/test/java/com/networknt/schema/Issue375Test.java
+++ b/src/test/java/com/networknt/schema/Issue375Test.java
@@ -53,9 +53,9 @@ public class Issue375Test {
         }
 
         List<String> expectedMessages = Arrays.asList(
-            "Property name $.fields.longName123 is not valid for validation: maxLength 5",
-            "Property name $.fields.longName123 is not valid for validation: pattern ^[a-zA-Z]+$",
-            "Property name $.fields.a is not valid for validation: minLength 3");
+            "Property name $.fields.longName123 is not valid for validation: may only be 5 characters long",
+            "Property name $.fields.longName123 is not valid for validation: does not match the regex pattern ^[a-zA-Z]+$",
+            "Property name $.fields.a is not valid for validation: must be at least 3 characters long");
         MatcherAssert.assertThat(errorMessages, Matchers.containsInAnyOrder(expectedMessages.toArray()));
     }
 }
diff --git a/src/test/java/com/networknt/schema/Issue396Test.java b/src/test/java/com/networknt/schema/Issue396Test.java
new file mode 100644
index 0000000..e7f127e
--- /dev/null
+++ b/src/test/java/com/networknt/schema/Issue396Test.java
@@ -0,0 +1,45 @@
+package com.networknt.schema;
+
+import java.io.InputStream;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class Issue396Test {
+	protected JsonSchema getJsonSchemaFromStreamContentV7(InputStream schemaContent) {
+		JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V7);
+		return factory.getSchema(schemaContent);
+	}
+
+	protected JsonNode getJsonNodeFromStreamContent(InputStream content) throws Exception {
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree(content);
+		return node;
+	}
+
+	@Test
+	public void testComplexPropertyNamesV7() throws Exception {
+		String schemaPath = "/schema/issue396-v7.json";
+		String dataPath = "/data/issue396.json";
+		InputStream schemaInputStream = getClass().getResourceAsStream(schemaPath);
+		JsonSchema schema = getJsonSchemaFromStreamContentV7(schemaInputStream);
+		InputStream dataInputStream = getClass().getResourceAsStream(dataPath);
+		JsonNode node = getJsonNodeFromStreamContent(dataInputStream);
+
+		final Set<String> invalidPaths = new HashSet<>();
+		node.fields().forEachRemaining(entry -> {
+			if (!entry.getValue().asBoolean())
+				invalidPaths.add("$." + entry.getKey());
+		});
+
+		Set<ValidationMessage> errors = schema.validate(node);
+		final Set<String> failedPaths = errors.stream().map(ValidationMessage::getPath).collect(Collectors.toSet());
+		Assert.assertEquals(failedPaths, invalidPaths);
+	}
+}
diff --git a/src/test/resources/data/issue342.json b/src/test/resources/data/issue342.json
new file mode 100644
index 0000000..4003576
--- /dev/null
+++ b/src/test/resources/data/issue342.json
@@ -0,0 +1,4 @@
+{
+  "a": "ok",
+  "z": "nope"
+}
diff --git a/src/test/resources/data/issue396.json b/src/test/resources/data/issue396.json
new file mode 100644
index 0000000..55c12d8
--- /dev/null
+++ b/src/test/resources/data/issue396.json
@@ -0,0 +1,10 @@
+{
+  "veryveryverylongallowedname": true,
+  "a": true,
+  "x": true,
+  "z": false,
+  "abc": false,
+  "w": false,
+  "ww": false,
+  "WW": true
+}
diff --git a/src/test/resources/schema/issue342-v7.json b/src/test/resources/schema/issue342-v7.json
new file mode 100644
index 0000000..31bfbe1
--- /dev/null
+++ b/src/test/resources/schema/issue342-v7.json
@@ -0,0 +1,11 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "type": "object",
+  "propertyNames": {
+    "enum": [
+      "a",
+      "b",
+      "c"
+    ]
+  }
+}
diff --git a/src/test/resources/schema/issue396-v7.json b/src/test/resources/schema/issue396-v7.json
new file mode 100644
index 0000000..7105184
--- /dev/null
+++ b/src/test/resources/schema/issue396-v7.json
@@ -0,0 +1,40 @@
+{
+  "type": "object",
+  "propertyNames": {
+    "anyOf": [
+      {
+        "minLength": 10
+      },
+      {
+        "$ref": "#/definitions/props"
+      },
+      {
+        "oneOf": [
+          {
+            "enum": [
+              "z",
+              "a",
+              "b"
+            ]
+          },
+          {
+            "$ref": "#/definitions/xyz"
+          }
+        ]
+      }
+    ]
+  },
+  "definitions": {
+    "props": {
+      "minLength": 2,
+      "pattern": "[A-Z]"
+    },
+    "xyz": {
+      "enum": [
+        "x",
+        "y",
+        "z"
+      ]
+    }
+  }
+}