Java7 backport #1

fpeter8 · 2017-01-18T13:56:32Z

This pull request is the java 7 backport of the original library. There should be no functionality change at all.

During review please be on the lookout for

code formatting differences
heavy and unnecessary use of the Guava library (if writing the loops by hand would be more compact / readable)
wrong imports

coveralls · 2017-01-18T13:58:22Z

Changes Unknown when pulling 8a4c343 on java7-backport into ** on master**.

coveralls · 2017-01-18T13:58:22Z

Changes Unknown when pulling 8a4c343 on java7-backport into ** on master**.

nbali · 2017-01-18T19:11:40Z

core/src/main/java/org/everit/json/schema/loader/SchemaLoader.java

-        IntStream.range(0, arr.length())
-                .mapToObj(arr::get)
-                .forEach(possibleValues::add);
+        Set<Object> possibleValues = Sets.newHashSet(arr);


@fpeter8 I think this is wrong

... okay maybe not, it implements the Iterable in newer versions, but I'm not sure which version does the project use

it's an iterable, but I'll make it more explicit

nbali · 2017-01-18T19:17:56Z

core/src/main/java/org/everit/json/schema/loader/internal/ResolutionScopeChangeListener.java


    @Override
-    default void accept(final URI t) {
+    public final void accept(final URI t) {


probably shouldn't be final

nbali · 2017-01-18T19:20:30Z

core/src/test/java/org/everit/json/schema/CombinedSchemaTest.java

-        CombinedSchema.oneOf(asList(BooleanSchema.INSTANCE));
+        CombinedSchema.allOf(Lists.<Schema>newArrayList(BooleanSchema.INSTANCE));
+        CombinedSchema.anyOf(Lists.<Schema>newArrayList(BooleanSchema.INSTANCE));
+        CombinedSchema.oneOf(Lists.<Schema>newArrayList(BooleanSchema.INSTANCE));


any particular reason for this change?

Java7 needs explicit type declarations, so the staticly imported method wasn't good. I just prefer Lists to Arrays, so I used that.

nbali · 2017-01-18T19:21:37Z

core/src/test/java/org/everit/json/schema/ObjectSchemaTest.java

-                .patternProperty("^string.*", new StringSchema())
-                .addPropertySchema("boolProp", BooleanSchema.INSTANCE)
-                .addRequiredProperty("boolProp");
+        Supplier<ObjectSchema.Builder> newBuilder = new Supplier<ObjectSchema.Builder>() {


(Callable existed in 7)

nbali · 2017-01-18T19:30:52Z

core/src/main/java/org/everit/json/schema/ArraySchema.java

+                        @Override
+                        public ValidationException apply(ValidationException exc) {
+                            ValidationException exception = exc.prepend(copyOfI);
+                            failureCollector.accept(exception);


ifPresent -> should be called only if "exception" isn't null

the whole transform block of Optional runs only if the exception isn't null. I know it's counter-intuitive, if you want I can move it.

nbali · 2017-01-18T20:08:20Z

core/src/main/java/org/everit/json/schema/ValidationException.java

     * @return all messages
     */
    public List<String> getAllMessages() {
        if (causingExceptions.isEmpty()) {
            return Collections.singletonList(getMessage());
        } else {
-            return getAllMessages(causingExceptions).stream().collect(Collectors.toList());
+            return Lists.newArrayList(Iterables.concat(getAllMessages(causingExceptions)));


nbali · 2017-01-18T20:13:40Z

tests/pom.xml

@@ -64,13 +64,14 @@
        <dependency>
            <groupId>org.eclipse.jetty</groupId>
            <artifactId>jetty-server</artifactId>
-            <version>9.3.4.RC0</version>
+            <version>8.2.0.v20160908</version>


version 9 uses Java8

nbali · 2017-01-18T20:17:21Z

core/src/main/java/org/everit/json/schema/Consumer.java

+        return new Consumer<T>() {
+            @Override
+            public void accept(T t) {
+                this.accept(t);


infinite recursion IMO

nbali

......

coveralls · 2017-01-19T14:13:59Z

Changes Unknown when pulling e3029d0 on java7-backport into ** on master**.

csabakassai · 2017-01-19T13:10:11Z

core/src/main/java/org/everit/json/schema/ArraySchema.java

+        return Optional.absent();
+    }
+
+    private static final class RValAddConsumer extends Consumer<ValidationException> {


You can use just a list instead of this custom consumer

csabakassai · 2017-01-19T13:27:30Z

core/src/main/java/org/everit/json/schema/EnumSchema.java

-                .orElseThrow(
-                        () -> new ValidationException(this, String.format("%s is not a valid enum value",
-                                subject), "enum"));
+        FluentIterable.from(possibleValues)


boolean match = false; for(Object val: possibleValues) { if(ObjectComparator.deepEquals(val, subject)) { match = true; break; } } if(!match) { throw new ValidationException(EnumSchema.this, String.format("%s is not a valid enum value", subject), "enum"); }

It's oldschool, but easier to understand

csabakassai · 2017-01-19T13:55:34Z

core/src/main/java/org/everit/json/schema/ValidationException.java

-                .flatMap(cause -> getAllMessages(cause.getCausingExceptions()).stream())
-                .collect(Collectors.toList()));
+        List<String> messages = Lists.newArrayList();
+        FluentIterable.from(causes)


for(ValidationException cause: causes) {
if(cause.getCausingExceptions().isEmpty()) {
messages.add(cause.getMessage());
} else {
messages.addAll(getAllMessages(cause.getCausingExceptions()));
}
}

coveralls · 2017-01-19T14:23:42Z

Changes Unknown when pulling 0115567 on java7-backport into ** on master**.

coveralls · 2017-01-19T14:23:42Z

Changes Unknown when pulling 0115567 on java7-backport into ** on master**.

coveralls · 2017-01-19T14:59:14Z

Changes Unknown when pulling b431bcd on java7-backport into ** on master**.

Fixed

coveralls · 2017-01-19T15:04:33Z

Changes Unknown when pulling 5fdd6f3 on java7-backport into ** on master**.

coveralls · 2017-01-19T15:14:57Z

Changes Unknown when pulling e737557 on java7-backport into ** on master**.

fpeter8 added 2 commits January 18, 2017 13:34

Backported entire library to Java7.

8287042

Fixed travis.yaml

8a4c343

fpeter8 requested review from nbali and csabakassai January 18, 2017 13:56

fpeter8 mentioned this pull request Jan 18, 2017

Java7 backport everit-org/json-schema#72

Closed

fpeter8 requested a review from csbeer January 18, 2017 14:18

nbali reviewed Jan 18, 2017

View reviewed changes

nbali previously requested changes Jan 18, 2017

View reviewed changes

nbali assigned fpeter8 Jan 18, 2017

Bugfix part 1.

e3029d0

fpeter8 requested a review from nbali January 19, 2017 14:12

fpeter8 removed their assignment Jan 19, 2017

More readable code.

0115567

csabakassai previously requested changes Jan 19, 2017

View reviewed changes

Removed unnecessary consumer.

b431bcd

fpeter8 requested a review from csabakassai January 19, 2017 15:01

Reduced Guava magic.

e737557

fpeter8 force-pushed the java7-backport branch from 5fdd6f3 to e737557 Compare January 19, 2017 15:10

csabakassai approved these changes Jan 20, 2017

View reviewed changes

fpeter8 merged commit 8c22c93 into master Jan 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java7 backport #1

Java7 backport #1

fpeter8 commented Jan 18, 2017

coveralls commented Jan 18, 2017

coveralls commented Jan 18, 2017

nbali Jan 18, 2017

nbali Jan 18, 2017

fpeter8 Jan 19, 2017

nbali Jan 18, 2017

nbali Jan 18, 2017

fpeter8 Jan 19, 2017 •

edited

Loading

nbali Jan 18, 2017

nbali Jan 18, 2017

fpeter8 Jan 19, 2017

nbali Jan 18, 2017

nbali Jan 18, 2017

fpeter8 Jan 19, 2017

nbali Jan 18, 2017

nbali left a comment

coveralls commented Jan 19, 2017 •

edited

Loading

csabakassai Jan 19, 2017

csabakassai Jan 19, 2017

csabakassai Jan 19, 2017

coveralls commented Jan 19, 2017

coveralls commented Jan 19, 2017

coveralls commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

Java7 backport #1

Java7 backport #1

Conversation

fpeter8 commented Jan 18, 2017

coveralls commented Jan 18, 2017

coveralls commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpeter8 Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbali left a comment

Choose a reason for hiding this comment

coveralls commented Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 19, 2017

coveralls commented Jan 19, 2017

coveralls commented Jan 19, 2017 • edited Loading

coveralls commented Jan 19, 2017 • edited Loading

coveralls commented Jan 19, 2017 • edited Loading

fpeter8 Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading