You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@avro.apache.org by GitBox <gi...@apache.org> on 2022/07/21 15:04:16 UTC
[GitHub] [avro] github-code-scanning[bot] commented on a diff in pull request #1776: Avro 2918 polymorphism
github-code-scanning[bot] commented on code in PR #1776:
URL: https://github.com/apache/avro/pull/1776#discussion_r926790787
##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -964,6 +1029,48 @@
this.hashCode = NO_HASHCODE;
}
+ protected int startFieldPos() {
+ return 0;
+ }
+
+ @Override
+ public boolean hasChild() {
+ return this.childs != null && !(this.childs.isEmpty());
+ }
+
+ @Override
+ public Stream<Schema> visitHierarchy() {
+ final Stream<Schema> childsStream;
+ if (this.hasChild()) {
+ Comparator<ExtendedRecordSchema> c = Comparator.comparing(ExtendedRecordSchema::getFullName);
+ childsStream = this.childs.stream().sorted(c).flatMap((ExtendedRecordSchema e) -> e.visitHierarchy());
+ } else {
+ childsStream = Stream.empty();
+ }
+ return Stream.concat(Stream.of(this), childsStream);
+ }
+
+ public void indexHierachy() {
+ final AtomicInteger current = new AtomicInteger(0);
+ this.visitHierarchy().forEach((Schema e) -> {
+ ((RecordSchema) e).index = current.getAndIncrement();
+ });
+ }
+
+ public void terminateInit() {
Review Comment:
## Missing Override annotation
This method overrides [Schema.terminateInit](1); it is advisable to add an Override annotation.
[Show more details](https://github.com/apache/avro/security/code-scanning/2783)
##########
lang/java/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:
##########
@@ -21,44 +21,30 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import org.apache.avro.Schema;
import org.apache.avro.io.TestValidatingIO.Encoding;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
-@RunWith(Parameterized.class)
public class TestResolvingIO {
- protected final Encoding eEnc;
- protected final int iSkipL;
- protected final String sJsWrtSchm;
- protected final String sWrtCls;
- protected final String sJsRdrSchm;
- protected final String sRdrCls;
-
- public TestResolvingIO(Encoding encoding, int skipLevel, String jsonWriterSchema, String writerCalls,
- String jsonReaderSchema, String readerCalls) {
- this.eEnc = encoding;
- this.iSkipL = skipLevel;
- this.sJsWrtSchm = jsonWriterSchema;
- this.sWrtCls = writerCalls;
- this.sJsRdrSchm = jsonReaderSchema;
- this.sRdrCls = readerCalls;
- }
-
- @Test
- public void testIdentical() throws IOException {
- performTest(eEnc, iSkipL, sJsWrtSchm, sWrtCls, sJsWrtSchm, sWrtCls);
+ @ParameterizedTest
+ @MethodSource("data2")
+ public void testIdentical(Encoding encoding, int skipLevel, String jsonWriterSchema, String writerCalls,
+ String jsonReaderSchema, String readerCalls) throws IOException {
Review Comment:
## Useless parameter
The parameter readerCalls is unused.
[Show more details](https://github.com/apache/avro/security/code-scanning/2786)
##########
lang/java/avro/src/main/java/org/apache/avro/Resolver.java:
##########
@@ -805,4 +807,52 @@
throw new IllegalArgumentException("Unknown schema type: " + write.getType());
}
}
+
+ /**
+ * In this case, the reader is a union and the writer is not. For this case, we
+ * need to pick the first branch of the reader that matches the writer and
+ * pretend to the reader that the index of this branch was found in the writer's
+ * data stream.
+ *
+ * To support this case, the {@link ReaderUnion} object has two (public) fields:
+ * <tt>firstMatch</tt> gives the index of the first matching branch in the
+ * reader's schema, and <tt>actualResolution</tt> is the {@link Action} that
+ * resolves the writer's schema with the schema found in the <tt>firstMatch</tt>
+ * branch of the reader's schema.
+ */
+ public static class ReaderExtends extends Action {
+ public final Schema firstMatch;
+ public final Action actualAction;
+
+ public ReaderExtends(Schema w, Schema r, GenericData d, Schema firstMatch, Action actual) {
+ super(w, r, d, Action.Type.READER_EXTENDS);
+ this.firstMatch = firstMatch;
+ this.actualAction = actual;
+ }
+
+ /**
+ * Returns a {@link ReaderUnion} action for resolving <tt>w</tt> and <tt>r</tt>,
+ * or an {@link ErrorAction} if there is no branch in the reader that matches
+ * the writer.
+ *
+ * @throws RuntimeException if <tt>r</tt> is not a union schema or <tt>w</tt>
+ * <em>is</em> a union schema
+ */
+ public static Action resolve(Schema w, Schema r, GenericData d, Map<SeenPair, Action> seen) {
+ if (w.getType() == Schema.Type.RECORD) {
+ throw new IllegalArgumentException("Writer schema is not Record.");
+ }
+ Schema schema = firstMatchingChild(w, r, d, seen);
+ if (schema != null) {
+ return new ReaderExtends(w, r, d, schema, Resolver.resolve(w, schema, d, seen));
+ }
+ return new ErrorAction(w, r, d, ErrorType.NO_MATCHING_BRANCH);
+ }
+
+ private static Schema firstMatchingChild(Schema w, Schema r, GenericData d, Map<SeenPair, Action> seen) {
Review Comment:
## Useless parameter
The parameter d is unused.
[Show more details](https://github.com/apache/avro/security/code-scanning/2787)
##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -1059,6 +1170,69 @@
}
}
+ private static class ExtendedRecordSchema extends RecordSchema {
+
+ private final RecordSchema parent;
+
+ public ExtendedRecordSchema(RecordSchema parent, Name name, String doc, boolean isError) {
+ super(name, doc, isError);
+ this.parent = parent;
+ this.parent.addChild(this);
+ }
+
+ public ExtendedRecordSchema(RecordSchema parent, Name name, String doc, boolean isError, List<Field> fields) {
+ super(name, doc, isError, fields);
+ this.parent = parent;
+ this.parent.addChild(this);
+ }
+
+ @Override
+ public List<Field> getFields() {
+ Stream<Field> parentFields = parent.hasFields() ? parent.getFields().stream() : Stream.empty();
+ Stream<Field> currentFields = this.fields != null ? this.fields.stream() : Stream.empty();
+ return Stream.concat(parentFields, currentFields).collect(Collectors.toList());
+ }
+
+ @Override
+ public boolean hasFields() {
+ return super.hasFields() || parent.hasFields();
+ }
+
+ @Override
+ public Field getField(String fieldname) {
+ Field f = null;
+ if (this.fields != null) {
+ f = super.getField(fieldname);
+ }
+ if (f == null) {
+ f = parent.getField(fieldname);
+ }
+ return f;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return o instanceof ExtendedRecordSchema && parent.equals(((ExtendedRecordSchema) o).parent) && super.equals(o);
+ }
+
+ @Override
+ protected String getJsonType() {
+ return "record:" + parent.name.getQualified(this.name.space);
+ }
+
+ @Override
+ public void indexHierachy() {
+ if (this.index < 0) {
+ this.parent.indexHierachy();
+ }
+ }
+
+ protected int startFieldPos() {
Review Comment:
## Missing Override annotation
This method overrides [RecordSchema.startFieldPos](1); it is advisable to add an Override annotation.
[Show more details](https://github.com/apache/avro/security/code-scanning/2782)
##########
lang/java/avro/src/main/java/org/apache/avro/Resolver.java:
##########
@@ -805,4 +807,52 @@
throw new IllegalArgumentException("Unknown schema type: " + write.getType());
}
}
+
+ /**
+ * In this case, the reader is a union and the writer is not. For this case, we
+ * need to pick the first branch of the reader that matches the writer and
+ * pretend to the reader that the index of this branch was found in the writer's
+ * data stream.
+ *
+ * To support this case, the {@link ReaderUnion} object has two (public) fields:
+ * <tt>firstMatch</tt> gives the index of the first matching branch in the
+ * reader's schema, and <tt>actualResolution</tt> is the {@link Action} that
+ * resolves the writer's schema with the schema found in the <tt>firstMatch</tt>
+ * branch of the reader's schema.
+ */
+ public static class ReaderExtends extends Action {
+ public final Schema firstMatch;
+ public final Action actualAction;
+
+ public ReaderExtends(Schema w, Schema r, GenericData d, Schema firstMatch, Action actual) {
+ super(w, r, d, Action.Type.READER_EXTENDS);
+ this.firstMatch = firstMatch;
+ this.actualAction = actual;
+ }
+
+ /**
+ * Returns a {@link ReaderUnion} action for resolving <tt>w</tt> and <tt>r</tt>,
+ * or an {@link ErrorAction} if there is no branch in the reader that matches
+ * the writer.
+ *
+ * @throws RuntimeException if <tt>r</tt> is not a union schema or <tt>w</tt>
+ * <em>is</em> a union schema
+ */
+ public static Action resolve(Schema w, Schema r, GenericData d, Map<SeenPair, Action> seen) {
+ if (w.getType() == Schema.Type.RECORD) {
+ throw new IllegalArgumentException("Writer schema is not Record.");
+ }
+ Schema schema = firstMatchingChild(w, r, d, seen);
+ if (schema != null) {
+ return new ReaderExtends(w, r, d, schema, Resolver.resolve(w, schema, d, seen));
+ }
+ return new ErrorAction(w, r, d, ErrorType.NO_MATCHING_BRANCH);
+ }
+
+ private static Schema firstMatchingChild(Schema w, Schema r, GenericData d, Map<SeenPair, Action> seen) {
Review Comment:
## Useless parameter
The parameter seen is unused.
[Show more details](https://github.com/apache/avro/security/code-scanning/2788)
##########
lang/java/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:
##########
@@ -21,44 +21,30 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import org.apache.avro.Schema;
import org.apache.avro.io.TestValidatingIO.Encoding;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
-@RunWith(Parameterized.class)
public class TestResolvingIO {
- protected final Encoding eEnc;
- protected final int iSkipL;
- protected final String sJsWrtSchm;
- protected final String sWrtCls;
- protected final String sJsRdrSchm;
- protected final String sRdrCls;
-
- public TestResolvingIO(Encoding encoding, int skipLevel, String jsonWriterSchema, String writerCalls,
- String jsonReaderSchema, String readerCalls) {
- this.eEnc = encoding;
- this.iSkipL = skipLevel;
- this.sJsWrtSchm = jsonWriterSchema;
- this.sWrtCls = writerCalls;
- this.sJsRdrSchm = jsonReaderSchema;
- this.sRdrCls = readerCalls;
- }
-
- @Test
- public void testIdentical() throws IOException {
- performTest(eEnc, iSkipL, sJsWrtSchm, sWrtCls, sJsWrtSchm, sWrtCls);
+ @ParameterizedTest
+ @MethodSource("data2")
+ public void testIdentical(Encoding encoding, int skipLevel, String jsonWriterSchema, String writerCalls,
+ String jsonReaderSchema, String readerCalls) throws IOException {
Review Comment:
## Useless parameter
The parameter jsonReaderSchema is unused.
[Show more details](https://github.com/apache/avro/security/code-scanning/2785)
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org