You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by GitBox <gi...@apache.org> on 2020/12/02 16:04:41 UTC

[GitHub] [johnzon] rmannibucau commented on a change in pull request #72: Json Schema Validation: added integer type validation.

rmannibucau commented on a change in pull request #72:
URL: https://github.com/apache/johnzon/pull/72#discussion_r534282882



##########
File path: johnzon-jsonschema/src/main/java/org/apache/johnzon/jsonschema/spi/builtin/IntegerValidation.java
##########
@@ -0,0 +1,50 @@
+package org.apache.johnzon.jsonschema.spi.builtin;
+
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import javax.json.JsonNumber;
+import javax.json.JsonString;
+import javax.json.JsonValue;
+
+import org.apache.johnzon.jsonschema.ValidationResult;
+import org.apache.johnzon.jsonschema.ValidationResult.ValidationError;
+import org.apache.johnzon.jsonschema.spi.ValidationContext;
+import org.apache.johnzon.jsonschema.spi.ValidationExtension;
+
+public class IntegerValidation implements ValidationExtension {
+
+	@Override
+	public Optional<Function<JsonValue, Stream<ValidationError>>> create(ValidationContext model) {
+		final JsonValue type = model.getSchema().get("type");
+		if (JsonString.class.isInstance(type) && "integer".equals(JsonString.class.cast(type).getString())) {
+			return Optional.of(new Impl(model.toPointer(), model.getValueProvider()));
+		}
+		return Optional.empty();
+	}
+
+	private static class Impl extends BaseValidation {
+
+		private Impl(final String pointer, final Function<JsonValue, JsonValue> valueProvider) {
+			super(pointer, valueProvider, JsonValue.ValueType.NUMBER);
+		}
+
+		@Override
+		protected Stream<ValidationError> onNumber(JsonNumber number) {
+			final double value = number.doubleValue();
+			if (value % 1 == 0) {
+				return Stream.empty();
+			} else {

Review comment:
       you can drop the else (as keyword and directly do "return Stream.of(new ValidationResult.ValidationError(pointer, "Expected integer but got " + value));") since previous block uses "return"

##########
File path: johnzon-jsonschema/src/main/java/org/apache/johnzon/jsonschema/spi/builtin/IntegerValidation.java
##########
@@ -0,0 +1,50 @@
+package org.apache.johnzon.jsonschema.spi.builtin;
+
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import javax.json.JsonNumber;
+import javax.json.JsonString;
+import javax.json.JsonValue;
+
+import org.apache.johnzon.jsonschema.ValidationResult;
+import org.apache.johnzon.jsonschema.ValidationResult.ValidationError;
+import org.apache.johnzon.jsonschema.spi.ValidationContext;
+import org.apache.johnzon.jsonschema.spi.ValidationExtension;
+
+public class IntegerValidation implements ValidationExtension {
+
+	@Override
+	public Optional<Function<JsonValue, Stream<ValidationError>>> create(ValidationContext model) {
+		final JsonValue type = model.getSchema().get("type");
+		if (JsonString.class.isInstance(type) && "integer".equals(JsonString.class.cast(type).getString())) {

Review comment:
       type.getValueType() == STRING ;)

##########
File path: johnzon-jsonschema/src/main/java/org/apache/johnzon/jsonschema/spi/builtin/IntegerValidation.java
##########
@@ -0,0 +1,50 @@
+package org.apache.johnzon.jsonschema.spi.builtin;
+
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import javax.json.JsonNumber;
+import javax.json.JsonString;
+import javax.json.JsonValue;
+
+import org.apache.johnzon.jsonschema.ValidationResult;
+import org.apache.johnzon.jsonschema.ValidationResult.ValidationError;
+import org.apache.johnzon.jsonschema.spi.ValidationContext;
+import org.apache.johnzon.jsonschema.spi.ValidationExtension;
+
+public class IntegerValidation implements ValidationExtension {
+
+	@Override
+	public Optional<Function<JsonValue, Stream<ValidationError>>> create(ValidationContext model) {
+		final JsonValue type = model.getSchema().get("type");
+		if (JsonString.class.isInstance(type) && "integer".equals(JsonString.class.cast(type).getString())) {
+			return Optional.of(new Impl(model.toPointer(), model.getValueProvider()));
+		}
+		return Optional.empty();
+	}
+
+	private static class Impl extends BaseValidation {
+
+		private Impl(final String pointer, final Function<JsonValue, JsonValue> valueProvider) {
+			super(pointer, valueProvider, JsonValue.ValueType.NUMBER);
+		}
+
+		@Override
+		protected Stream<ValidationError> onNumber(JsonNumber number) {
+			final double value = number.doubleValue();

Review comment:
       hmm, wonder if it shouldn't be merged with multipleof impl: org.apache.johnzon.jsonschema.spi.builtin.MultipleOfValidation
   we can image make org.apache.johnzon.jsonschema.spi.builtin.MultipleOfValidation.Impl visible in the package and create method return a new MultipleOfValidation.Impl(...., 1) instead of pseudo-duplicating it, would likely be more explicit we reuse this logic as impl.
   what do you think?

##########
File path: johnzon-jsonschema/src/test/java/org/apache/johnzon/jsonschema/JsonSchemaValidatorTest.java
##########
@@ -355,6 +355,25 @@ public void exclusiveMaximum() {
 
         validator.close();
     }
+    
+    @Test
+    public void integerType() {
+    	final JsonSchemaValidator validator = factory.newInstance(jsonFactory.createObjectBuilder()
+                .add("type", "object")
+                .add("properties", jsonFactory.createObjectBuilder()
+                        .add("age", jsonFactory.createObjectBuilder()
+                                .add("type", "integer")
+                                .build())
+                        .build())
+                .build());
+
+        assertTrue(validator.apply(jsonFactory.createObjectBuilder().build()).isSuccess());
+        assertTrue(validator.apply(jsonFactory.createObjectBuilder().add("age", 30).build()).isSuccess());
+        // check no decimal numbers allowed
+        assertFalse(validator.apply(jsonFactory.createObjectBuilder() .add("age", 30.3) .build()).isSuccess());

Review comment:
       we need to test with BigInteger and BigDecimal too I guess




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org