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 15:36:48 UTC

[GitHub] [johnzon] manuelrazola opened a new pull request #72: Json Schema Validation: added integer type validation.

manuelrazola opened a new pull request #72:
URL: https://github.com/apache/johnzon/pull/72


   Hi!
   As per the official json-schema specification, integer type validation is missing from the current implementation.
   http://json-schema.org/understanding-json-schema/reference/numeric.html
   
   This PR tries to tackle that in the least intrusive way possible.
   Thanks in advance!


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
manuelrazola commented on a change in pull request #72:
URL: https://github.com/apache/johnzon/pull/72#discussion_r534315786



##########
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:
       Possibly, will test how it would look like and come back tomorrow ;)




----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
manuelrazola commented on a change in pull request #72:
URL: https://github.com/apache/johnzon/pull/72#discussion_r535006380



##########
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:
       Implemented as requested




----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
manuelrazola commented on a change in pull request #72:
URL: https://github.com/apache/johnzon/pull/72#discussion_r535008028



##########
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:
       Updated tests




----------------------------------------------------------------
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



[GitHub] [johnzon] rmannibucau merged pull request #72: [JOHNZON-327] Json Schema Validation: added integer type validation.

Posted by GitBox <gi...@apache.org>.
rmannibucau merged pull request #72:
URL: https://github.com/apache/johnzon/pull/72


   


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
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