Christophe-LeSaec commented on code in PR #1589:
URL: https://github.com/apache/avro/pull/1589#discussion_r880498764


##########
lang/java/idl/src/main/java/org/apache/avro/idl/SchemaVisitorAction.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.idl;
+
+public enum SchemaVisitorAction {

Review Comment:
   if inspired from java.nio.file.FileVisitResult for walkFileTree(Path start, 
FileVisitor<? super Path> visitor), it could be renamed in SchemaVisitorResult ?



##########
lang/java/idl/src/main/java/org/apache/avro/idl/Schemas.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.idl;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+
+import java.util.ArrayDeque;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.IdentityHashMap;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+/**
+ * Avro Schema utilities, to traverse...
+ */
+public final class Schemas {
+
+  private Schemas() {
+  }
+
+  /**
+   * Depth first visit.
+   */
+  public static <T> T visit(final Schema start, final SchemaVisitor<T> 
visitor) {

Review Comment:
   Question of taste, i prefer short static method and separation in classes.
   To do so with this code, i can propose this (look very similar, but cut in 
smaller pieces, sorry for long comment) 
   ```java
   public final class Schemas {
   
     private Schemas() {}
   
     private static abstract class InternalSchemaActions {
       abstract boolean visitNonTerminal(final Schema schema, final 
Iterable<Schema> itSupp);
       abstract boolean visitTerminal(final Schema schema);
     }
   
     static abstract class VisitAction { // to have a typed queue
       abstract boolean onVisitor(InternalSchemaActions actions);
     }
   
     static class SchemaVisitAction extends VisitAction {
   
       private final Schema schema;
   
       private final IdentityHashMap<Schema, Schema> visited;
   
       public SchemaVisitAction(Schema schema, IdentityHashMap<Schema, Schema> 
visited) {
         this.schema = schema;
         this.visited = visited;
       }
   
       @Override
       boolean onVisitor(InternalSchemaActions actions) {
         final boolean terminate;
         if (visited.containsKey(schema)) {
           terminate = actions.visitTerminal(schema);
         } else {
           Schema.Type type = schema.getType();
           switch (type) {
           case ARRAY:
             terminate = actions.visitNonTerminal(schema, 
Collections.singleton(schema.getElementType()));
             visited.put(schema, schema);
             break;
           case RECORD:
             terminate = actions.visitNonTerminal(schema, () -> 
schema.getFields().stream().map(Field::schema)
                 
.collect(Collectors.toCollection(ArrayDeque::new)).descendingIterator());
             visited.put(schema, schema);
             break;
           case UNION:
             terminate = actions.visitNonTerminal(schema, schema.getTypes());
             visited.put(schema, schema);
             break;
           case MAP:
             terminate = actions.visitNonTerminal(schema, 
Collections.singleton(schema.getValueType()));
             visited.put(schema, schema);
             break;
           default:
             terminate = actions.visitTerminal(schema);
             break;
           }
         }
         return terminate;
       }
     }
   
     static class MethodVisitAction extends VisitAction {
   
       private final Supplier<SchemaVisitorAction> actionGetter;
   
       private final Deque<VisitAction> dq;
   
       public MethodVisitAction(Supplier<SchemaVisitorAction> actionGetter, 
Deque<VisitAction> dq) {
         this.actionGetter = actionGetter;
         this.dq = dq;
       }
   
       @Override
       boolean onVisitor(InternalSchemaActions actions) {
         boolean ret = false;
         final SchemaVisitorAction action = actionGetter.get();
         switch (action) {
         case CONTINUE:
           break;
         case SKIP_SUBTREE:
           throw new UnsupportedOperationException();
         case SKIP_SIBLINGS:
           while (dq.peek() instanceof SchemaVisitAction) {
             dq.remove();
           }
           break;
         case TERMINATE:
           ret = true;
           break;
         default:
           throw new UnsupportedOperationException("Invalid action " + action);
         }
         return ret;
       }
     }
   
     static class Visit<T> {
       private final SchemaVisitor<T> visitor;
   
       private final IdentityHashMap<Schema, Schema> visited = new 
IdentityHashMap<>();
   
       private final Deque<VisitAction> dq = new ArrayDeque<>();
   
       public Visit(SchemaVisitor<T> visitor) {
         this.visitor = visitor;
       }
   
       private final InternalSchemaActions actions = new 
InternalSchemaActions() {
         @Override
         public boolean visitNonTerminal(Schema schema, Iterable<Schema> 
itSupp) {
           return Visit.this.visitNonTerminal(schema, itSupp);
         }
   
         @Override
         public boolean visitTerminal(Schema schema) {
           return Visit.this.visitTerminal(schema);
         }
       };
   
       public T visit(final Schema start) {
         // Set of Visited Schemas
   
         // Stack that contains the Schemas to process and afterVisitNonTerminal
         // functions.
         // Deque<Either<Schema, Supplier<SchemaVisitorAction>>>
         // Using Either<...> has a cost we want to avoid...
         VisitAction current = new SchemaVisitAction(start, this.visited);
         boolean terminate = false;
         while (current != null && !terminate) {
           terminate = current.onVisitor(this.actions);
           current = dq.poll();
         }
         return visitor.get();
       }
   
       private boolean visitTerminal(final Schema schema) {
         SchemaVisitorAction action = visitor.visitTerminal(schema);
         switch (action) {
         case CONTINUE:
           break;
         case SKIP_SIBLINGS:
           while (dq.peek() instanceof SchemaVisitAction) {
             dq.remove();
           }
           break;
         case TERMINATE:
           return true;
         case SKIP_SUBTREE:
         default:
           throw new UnsupportedOperationException("Invalid action " + action + 
" for " + schema);
         }
         return false;
       }
   
       private boolean visitNonTerminal(final Schema schema, final 
Iterable<Schema> itSupp) {
         SchemaVisitorAction action = visitor.visitNonTerminal(schema);
         switch (action) {
         case CONTINUE:
           dq.push(new MethodVisitAction(() -> 
visitor.afterVisitNonTerminal(schema), dq));
           itSupp.forEach((Schema supp) -> dq.push(new SchemaVisitAction(supp, 
this.visited)));
           break;
         case SKIP_SUBTREE:
           dq.push(new MethodVisitAction(() -> 
visitor.afterVisitNonTerminal(schema), dq));
           break;
         case SKIP_SIBLINGS:
           while (dq.peek() instanceof SchemaVisitAction) {
             dq.remove();
           }
           break;
         case TERMINATE:
           return true;
         default:
           throw new UnsupportedOperationException("Invalid action " + action + 
" for " + schema);
         }
         return false;
       }
     }
   
     /**
      * Depth first visit.
      */
     public static <T> T visit(final Schema start, final SchemaVisitor<T> 
visitor) {
       // Set of Visited Schemas
       Visit<T> visit = new Visit<>(visitor);
       return visit.visit(start);
     }
   }
   ```



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to