You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by rs...@apache.org on 2021/12/04 13:41:57 UTC
[avro] branch branch-1.11 updated: AVRO-3256: IDL type reference with annotation throws error (#1407)
This is an automated email from the ASF dual-hosted git repository.
rskraba pushed a commit to branch branch-1.11
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/branch-1.11 by this push:
new 6aa5791 AVRO-3256: IDL type reference with annotation throws error (#1407)
6aa5791 is described below
commit 6aa5791cd9505ff11056f09f95fdcc574c0629e1
Author: Oscar Westra van Holthe - Kind <op...@users.noreply.github.com>
AuthorDate: Sat Dec 4 14:41:18 2021 +0100
AVRO-3256: IDL type reference with annotation throws error (#1407)
* AVRO-3256: IDL type reference with annotation throws error
Previous versions would alter the referenced type when encountering an
annotation on (for example) a field type. This change makes references
read-only.
* AVRO-3256: Document new behavior of annotations
Documented that references to named types cannot be annotated. Also
described where annotations for named types should go.
Lastly, the example has been fixed to match this change, and now also
contains various types of documentation.
---
doc/src/content/xdocs/idl.xml | 32 ++++++++++++------
.../javacc/org/apache/avro/compiler/idl/idl.jj | 34 ++++++++++---------
.../src/test/idl/AnnotationOnTypeReference.avdl} | 37 +++-----------------
lang/java/compiler/src/test/idl/input/simple.avdl | 2 +-
lang/java/compiler/src/test/idl/output/simple.avpr | 3 +-
.../idl/TestReferenceAnnotationNotAllowed.java | 39 ++++++++++++++++++++++
lang/java/grpc/src/test/avro/TestService.avdl | 6 ++--
lang/java/tools/src/test/idl/protocol.avdl | 6 ++--
8 files changed, 90 insertions(+), 69 deletions(-)
diff --git a/doc/src/content/xdocs/idl.xml b/doc/src/content/xdocs/idl.xml
index 52a6075..0d5e5da 100644
--- a/doc/src/content/xdocs/idl.xml
+++ b/doc/src/content/xdocs/idl.xml
@@ -101,7 +101,7 @@ protocol MyProtocol {
<p>
This is equivalent to (and generates) the following JSON protocol definition:
</p>
- <!-- "namespace" : null, TODO: this is generated but shouldnt be - AVRO-263 -->
+ <!-- "namespace" : null, TODO: this is generated but shouldn't be - AVRO-263 -->
<source>
{
"protocol" : "MyProtocol",
@@ -384,7 +384,7 @@ record MyRecord {
string @order("ignore") myIgnoredField;
}
</source>
- <p>A field's type may also be preceded by annotations, e.g.: </p>
+ <p>A field's type (with the exception of type references) may also be preceded by annotations, e.g.: </p>
<source>
record MyRecord {
@java-class("java.util.ArrayList") array<string> myStrings;
@@ -429,50 +429,62 @@ record MyRecord {
<p>Some annotations like those listed above are handled
specially. All other annotations are added as properties to
the protocol, message, schema or field.</p>
+ <p>Note that for named types, annotations should be added to
+ the type definition; they cannot be added to the type references.</p>
</section>
</section>
<section id="example">
<title>Complete Example</title>
- <p>The following is a complete example of a Avro IDL file that shows most of the above features:</p>
+ <p>The following is an example of an Avro IDL file that shows most of the above features:</p>
<source>
+/*
+* Header with license information.
+*/
+
/**
* An example protocol in Avro IDL
*/
@namespace("org.apache.avro.test")
protocol Simple {
-
+ /** Documentation for the enum type Kind */
@aliases(["org.foo.KindOf"])
enum Kind {
FOO,
BAR, // the bar enum value
BAZ
- }
+ } = FOO; // For schema evolution purposes, unmatched values do not throw an error, but are resolved to FOO.
+ /** MD5 hash; good enough to avoid most collisions, and smaller than (for example) SHA256. */
fixed MD5(16);
record TestRecord {
- @order("ignore")
- string name;
+ /** Record name; has no intrinsic order */
+ string @order("ignore") name;
- @order("descending")
- Kind kind;
+ Kind @order("descending") kind;
MD5 hash;
- union { MD5, null} @aliases(["hash"]) nullableHash;
+ /*
+ Note that 'null' is the first union type. Just like .avsc / .avpr files, the default value must be of the first union type.
+ */
+ union { null, MD5 } /** Optional field */ @aliases(["hash"]) nullableHash = null;
array<long> arrayOfLongs;
}
+ /** Errors are records that can be thrown from a method */
error TestError {
string message;
}
string hello(string greeting);
+ /** Return what was given. Demonstrates the use of backticks to name types/fields/messages/parameters after keywords */
TestRecord echo(TestRecord `record`);
int add(int arg1, int arg2);
bytes echoBytes(bytes data);
void `error`() throws TestError;
+ // The oneway keyword forces the method to return null.
void ping() oneway;
}
</source>
diff --git a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
index 15d251e..d4be8a5 100644
--- a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
+++ b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
@@ -63,6 +63,7 @@ package org.apache.avro.compiler.idl;
import java.io.*;
import java.net.*;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
@@ -1068,8 +1069,7 @@ Schema UnionDefinition():
List<Schema> schemata = new ArrayList<Schema>();
}
{
- // TODO should probably disallow other unions here in the parser?
-
+ // Don't disallow unions here: its constructor disallows nested unions and throws a descriptive exception.
"union"
"{"
s = Type()
@@ -1141,7 +1141,7 @@ List<String> EnumBody():
}
{
"{"
- [ EnumConstant(symbols) ( LOOKAHEAD(2) "," EnumConstant(symbols) )* ]
+ [ EnumConstant(symbols) ( "," EnumConstant(symbols) )* ]
"}"
{
return symbols;
@@ -1317,17 +1317,9 @@ void FieldDeclaration(List<Field> fields):
Map<String, JsonNode> props = new LinkedHashMap<String, JsonNode>();
}
{
- // TODO should we be able to specify properties on any Type?
- // or just on field declarations as done here
-
- ( SchemaProperty(props) )*
type = Type()
VariableDeclarator(type, fields) ( "," VariableDeclarator(type, fields) )*
";"
- {
- for (String key : props.keySet())
- Accessor.addProp(type, key, props.get(key));
- }
}
void VariableDeclarator(Schema type, List<Field> fields):
@@ -1440,16 +1432,27 @@ Schema Type():
{
( SchemaProperty(props) )*
+ s = UnannotatedType(props)
+ {
+ for (String key : props.keySet())
+ Accessor.addProp(s, key, props.get(key));
+ return s;
+ }
+}
+
+Schema UnannotatedType(Map<String, JsonNode> props):
+{
+ Schema s;
+}
+{
(
- LOOKAHEAD(2) s = ReferenceType()
+ s = ReferenceType() { if (!props.isEmpty()) { throw error("Type references may not be annotated", token); } }
| s = PrimitiveType()
| s = UnionDefinition()
| s = ArrayType()
| s = MapType()
)
{
- for (String key : props.keySet())
- Accessor.addProp(s, key, props.get(key));
return s;
}
}
@@ -1543,9 +1546,8 @@ Schema ResultType():
Schema schema;
}
{
- LOOKAHEAD(2)
"void" { return Schema.create(Type.NULL); }
- | schema = Type() { return schema; }
+ | schema = UnannotatedType(Collections.emptyMap()) { return schema; }
}
String PropertyName():
diff --git a/lang/java/tools/src/test/idl/protocol.avdl b/lang/java/compiler/src/test/idl/AnnotationOnTypeReference.avdl
similarity index 62%
copy from lang/java/tools/src/test/idl/protocol.avdl
copy to lang/java/compiler/src/test/idl/AnnotationOnTypeReference.avdl
index 89fbb24..03f6f7c 100644
--- a/lang/java/tools/src/test/idl/protocol.avdl
+++ b/lang/java/compiler/src/test/idl/AnnotationOnTypeReference.avdl
@@ -1,4 +1,4 @@
-/**
+/*
* 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
@@ -17,42 +17,15 @@
*/
/**
- * An example protocol in Avro IDL
+ * A stripped down version of a previous `simple.avdl`, keeping the part where a type reference had an annotation (this is wrong).
*/
@namespace("org.apache.avro.test")
protocol Simple {
-
- @aliases(["org.foo.KindOf"])
- enum Kind {
- FOO,
- BAR, // the bar enum value
- BAZ
- }
-
+ /** An MD5 hash. */
fixed MD5(16);
+ /** A TestRecord. */
record TestRecord {
- @order("ignore")
- string name;
-
- @order("descending")
- Kind kind;
-
- MD5 hash;
-
- union { MD5, null} @aliases(["hash"]) nullableHash;
-
- array<long> arrayOfLongs;
+ @foo("bar") MD5 hash = "0000000000000000";
}
-
- error TestError {
- string message;
- }
-
- string hello(string greeting);
- TestRecord echo(TestRecord `record`);
- int add(int arg1, int arg2);
- bytes echoBytes(bytes data);
- void `error`() throws TestError;
- void ping() oneway;
}
diff --git a/lang/java/compiler/src/test/idl/input/simple.avdl b/lang/java/compiler/src/test/idl/input/simple.avdl
index 0d09272..a9f67ed 100644
--- a/lang/java/compiler/src/test/idl/input/simple.avdl
+++ b/lang/java/compiler/src/test/idl/input/simple.avdl
@@ -50,7 +50,7 @@ protocol Simple {
/** The status of the record. */
Status status = "A";
- @foo("bar") MD5 hash = "0000000000000000";
+ MD5 hash = "0000000000000000";
union {null, MD5} @aliases(["hash", "hsh"]) nullableHash = null;
diff --git a/lang/java/compiler/src/test/idl/output/simple.avpr b/lang/java/compiler/src/test/idl/output/simple.avpr
index 5710f08..6128d44 100644
--- a/lang/java/compiler/src/test/idl/output/simple.avpr
+++ b/lang/java/compiler/src/test/idl/output/simple.avpr
@@ -18,8 +18,7 @@
"type" : "fixed",
"name" : "MD5",
"doc" : "An MD5 hash.",
- "size" : 16,
- "foo" : "bar"
+ "size" : 16
}, {
"type" : "record",
"name" : "TestRecord",
diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestReferenceAnnotationNotAllowed.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestReferenceAnnotationNotAllowed.java
new file mode 100644
index 0000000..2a095b0
--- /dev/null
+++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestReferenceAnnotationNotAllowed.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed 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.compiler.idl;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class TestReferenceAnnotationNotAllowed {
+
+ @Test
+ public void testReferenceAnnotationNotAllowed() {
+
+ final ClassLoader cl = Thread.currentThread().getContextClassLoader();
+ Idl idl = new Idl(cl.getResourceAsStream("AnnotationOnTypeReference.avdl"), "UTF-8");
+
+ try {
+ idl.CompilationUnit();
+
+ fail("Compilation should fail: annotations on type references are not allowed.");
+ } catch (ParseException e) {
+ assertEquals("Type references may not be annotated, at line 29, column 17", e.getMessage());
+ }
+ }
+}
diff --git a/lang/java/grpc/src/test/avro/TestService.avdl b/lang/java/grpc/src/test/avro/TestService.avdl
index 2a4537a..9a4629a 100644
--- a/lang/java/grpc/src/test/avro/TestService.avdl
+++ b/lang/java/grpc/src/test/avro/TestService.avdl
@@ -29,11 +29,9 @@ protocol TestService {
fixed MD5(4);
record TestRecord {
- @order("ignore")
- string name;
+ string @order("ignore") name;
- @order("descending")
- Kind kind;
+ Kind @order("descending") kind;
MD5 hash;
diff --git a/lang/java/tools/src/test/idl/protocol.avdl b/lang/java/tools/src/test/idl/protocol.avdl
index 89fbb24..2de0897 100644
--- a/lang/java/tools/src/test/idl/protocol.avdl
+++ b/lang/java/tools/src/test/idl/protocol.avdl
@@ -32,11 +32,9 @@ protocol Simple {
fixed MD5(16);
record TestRecord {
- @order("ignore")
- string name;
+ string @order("ignore") name;
- @order("descending")
- Kind kind;
+ Kind @order("descending") kind;
MD5 hash;