Maintain insertion order for subschemas of CombinedSchema (#522)

Maintain insertion order for subschemas of CombinedSchema.

This is an alternative to #519 that maintains the subschemas in insertion order.

The subschemas were originally maintained in insertion order, but it was changed due to #405, which ensured that subschemas that were of type CombinedSchema were visited first during validation, and by #498, which tries to maintain a stable order for "equivalent" instances.

This PR simply restores the insertion order, but maintains an internally sorted collection for the two purposes listed above, namely visiting subschemas during validation and equivalence checks during equals/hashCode.

Author: rayokota

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

@@ -156,6 +156,8 @@ public static Builder oneOf(Collection<Schema> schemas) {
 
     private final Collection<Schema> subschemas;
 
+    private final Collection<Schema> sortedSubschemas;
+
     private final ValidationCriterion criterion;
 
     /**
@@ -168,7 +170,8 @@ public CombinedSchema(Builder builder) {
         super(builder);
         this.synthetic = builder.synthetic;
         this.criterion = requireNonNull(builder.criterion, "criterion cannot be null");
-        this.subschemas = sortByCombinedFirst(requireNonNull(builder.subschemas, "subschemas cannot be null"));
+        this.subschemas = builder.subschemas;
+        this.sortedSubschemas = sortByCombinedFirst(requireNonNull(builder.subschemas, "subschemas cannot be null"));
     }
 
     private static int compareBySchemaType(Schema lschema, Schema rschema) {
@@ -191,10 +194,22 @@ public ValidationCriterion getCriterion() {
         return criterion;
     }
 
+    /**
+     * Returns the subschemas in the order they were added.
+     * @return the subschemas in insertion order
+     */
     public Collection<Schema> getSubschemas() {
         return subschemas;
     }
 
+    /*
+     * Internal method that returns the subschemas in the order they should be visited
+     * by the ValidatingVisitor and the equals and hashCode methods.
+     */
+    Collection<Schema> subschemasWithCombinedFirst() {
+        return sortedSubschemas;
+    }
+
     public boolean hasMultipleCombinedSchemasOfSameCriterion() {
         return subschemas.stream()
             .filter(schema -> schema instanceof CombinedSchema)
@@ -235,7 +250,7 @@ public boolean equals(Object o) {
         if (o instanceof CombinedSchema) {
             CombinedSchema that = (CombinedSchema) o;
             return that.canEqual(this) &&
-                    Objects.equals(subschemas, that.subschemas) &&
+                    Objects.equals(sortedSubschemas, that.sortedSubschemas) &&
                     Objects.equals(criterion, that.criterion) &&
                     synthetic == that.synthetic &&
                     super.equals(that);
@@ -246,7 +261,7 @@ public boolean equals(Object o) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), subschemas, criterion, synthetic);
+        return Objects.hash(super.hashCode(), sortedSubschemas, criterion, synthetic);
     }
 
     @Override

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

@@ -157,7 +157,7 @@ void visitStringSchema(StringSchema stringSchema) {
 
     @Override
     void visitCombinedSchema(CombinedSchema combinedSchema) {
-        Collection<Schema> subschemas = combinedSchema.getSubschemas();
+        Collection<Schema> subschemas = combinedSchema.subschemasWithCombinedFirst();
         List<ValidationException> failures = new ArrayList<>(subschemas.size());
         CombinedSchema.ValidationCriterion criterion = combinedSchema.getCriterion();
         for (Schema subschema : subschemas) {

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

@@ -68,7 +68,7 @@ public void subschemasSort() {
                 .isSynthetic(true)
                 .build();
 
-        Object[] subschemas = subject.getSubschemas().toArray();
+        Object[] subschemas = subject.subschemasWithCombinedFirst().toArray();
 
         assertEquals(8, subschemas.length);
         assertEquals(subcombined1, subschemas[0]);
@@ -79,6 +79,18 @@ public void subschemasSort() {
         assertEquals(booleanSchema2, subschemas[5]);
         assertEquals(emptySchema2, subschemas[6]);
         assertEquals(nullSchema2, subschemas[7]);
+
+        subschemas = subject.getSubschemas().toArray();
+
+        assertEquals(8, subschemas.length);
+        assertEquals(nullSchema1, subschemas[0]);
+        assertEquals(emptySchema1, subschemas[1]);
+        assertEquals(booleanSchema1, subschemas[2]);
+        assertEquals(subcombined1, subschemas[3]);
+        assertEquals(booleanSchema2, subschemas[4]);
+        assertEquals(emptySchema2, subschemas[5]);
+        assertEquals(nullSchema2, subschemas[6]);
+        assertEquals(subcombined2, subschemas[7]);
     }
 
     @Test
@@ -172,7 +184,7 @@ public void reportCauses() {
     public void equalsVerifier() {
         EqualsVerifier.forClass(CombinedSchema.class)
                 .withRedefinedSuperclass()
-                .withIgnoredFields("schemaLocation", "location")
+                .withIgnoredFields("schemaLocation", "location", "subschemas")
                 .suppress(Warning.STRICT_INHERITANCE)
                 .verify();
     }