From b00e1a3bf4d1afc4244ccec15168b23229e2033e Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Wed, 14 Oct 2009 21:54:20 -0700 Subject: [PATCH] Support for docs in schemas. --- src/doc/content/xdocs/spec.xml | 10 +- src/java/org/apache/avro/Protocol.java | 1 + src/java/org/apache/avro/Schema.java | 101 ++++++++++++------ .../org/apache/avro/generic/GenericData.java | 4 +- .../org/apache/avro/reflect/ReflectData.java | 13 +-- .../avro/specific/SpecificCompiler.java | 24 ++++- .../org/apache/avro/TestProtocolGeneric.java | 4 +- src/test/java/org/apache/avro/TestSchema.java | 36 +++++++ 8 files changed, 148 insertions(+), 45 deletions(-) diff --git a/src/doc/content/xdocs/spec.xml b/src/doc/content/xdocs/spec.xml index ab40037..5ed3088 100644 --- a/src/doc/content/xdocs/spec.xml +++ b/src/doc/content/xdocs/spec.xml @@ -86,15 +86,19 @@
Records -

Records use the type name "record" and support two attributes:

+

Records use the type name "record" and support three attributes:

diff --git a/src/java/org/apache/avro/Protocol.java b/src/java/org/apache/avro/Protocol.java index 3446c03..f04a0b4 100644 --- a/src/java/org/apache/avro/Protocol.java +++ b/src/java/org/apache/avro/Protocol.java @@ -318,6 +318,7 @@ private Message parseMessage(String messageName, JsonNode json) { throw new SchemaParseException("No param type: "+field); fields.put(fieldNameNode.getTextValue(), new Field(Schema.parse(fieldTypeNode,types), + null, /* TODO(philip) */ field.get("default"))); } Schema request = Schema.createRecord(fields); diff --git a/src/java/org/apache/avro/Schema.java b/src/java/org/apache/avro/Schema.java index 0351450..6e3e26b 100644 --- a/src/java/org/apache/avro/Schema.java +++ b/src/java/org/apache/avro/Schema.java @@ -94,21 +94,21 @@ public static Schema create(Type type) { /** Create an anonymous record schema. */ public static Schema createRecord(LinkedHashMap fields) { - Schema result = createRecord(null, null, false); + Schema result = createRecord(null, null, null, false); result.setFields(fields); return result; } /** Create a named record schema. */ - public static Schema createRecord(String name, String namespace, + public static Schema createRecord(String name, String doc, String namespace, boolean isError) { - return new RecordSchema(name, namespace, isError); + return new RecordSchema(name, doc, namespace, isError); } /** Create an enum schema. */ - public static Schema createEnum(String name, String namespace, + public static Schema createEnum(String name, String doc, String namespace, List values) { - return new EnumSchema(name, namespace, values); + return new EnumSchema(name, doc, namespace, values); } /** Create an array schema. */ @@ -127,8 +127,8 @@ public static Schema createUnion(List types) { } /** Create a union schema. */ - public static Schema createFixed(String name, String space, int size) { - return new FixedSchema(name, space, size); + public static Schema createFixed(String name, String doc, String space, int size) { + return new FixedSchema(name, doc, space, size); } /** Return the type of this schema. */ @@ -160,8 +160,16 @@ public int getEnumOrdinal(String symbol) { } /** If this is a record, enum or fixed, returns its name, otherwise the name - * of the primitive type. */ + * of the primitive type. Never returns null. */ public String getName() { return type.name; } + + /** + * If this is a record, enum, or fixed, returns its docstring, + * if available. Otherwise, returns null. + */ + public String getDoc() { + return null; + } /** If this is a record, enum or fixed, returns its namespace, if any. */ public String getNamespace() { @@ -233,14 +241,16 @@ public enum Order { private int position = -1; private final Schema schema; + private final String doc; private final JsonNode defaultValue; private final Order order; - public Field(Schema schema, JsonNode defaultValue) { - this(schema, defaultValue, Order.ASCENDING); + public Field(Schema schema, String doc, JsonNode defaultValue) { + this(schema, doc, defaultValue, Order.ASCENDING); } - public Field(Schema schema, JsonNode defaultValue, Order order) { + public Field(Schema schema, String doc, JsonNode defaultValue, Order order) { this.schema = schema; + this.doc = doc; this.defaultValue = defaultValue; this.order = order; } @@ -248,6 +258,10 @@ public Field(Schema schema, JsonNode defaultValue, Order order) { public int pos() { return position; } /** This field's {@link Schema}. */ public Schema schema() { return schema; } + /** + * This field's documentation within the record, if set. May return null. + */ + public String doc() { return doc; } public JsonNode defaultValue() { return defaultValue; } public Order order() { return order; } public boolean equals(Object other) { @@ -303,11 +317,14 @@ public void writeName(Names names, JsonGenerator gen) throws IOException { private static abstract class NamedSchema extends Schema { private final Name name; - public NamedSchema(Type type, String name, String space) { + private final String doc; + public NamedSchema(Type type, String name, String doc, String space) { super(type); this.name = new Name(name, space); + this.doc = doc; } public String getName() { return name.name; } + public String getDoc() { return doc; } public String getNamespace() { return name.space; } public boolean writeNameRef(Names names, JsonGenerator gen) throws IOException { @@ -355,8 +372,8 @@ private static class RecordSchema extends NamedSchema { private Map fields; private Iterable> fieldSchemas; private final boolean isError; - public RecordSchema(String name, String space, boolean isError) { - super(Type.RECORD, name, space); + public RecordSchema(String name, String doc, String space, boolean isError) { + super(Type.RECORD, name, doc, space); this.isError = isError; } public boolean isError() { return isError; } @@ -437,8 +454,8 @@ void fieldsToJson(Names names, JsonGenerator gen) throws IOException { private static class EnumSchema extends NamedSchema { private final List symbols; private final Map ordinals; - public EnumSchema(String name, String space, List symbols) { - super(Type.ENUM, name, space); + public EnumSchema(String name, String doc, String space, List symbols) { + super(Type.ENUM, name, doc, space); this.symbols = symbols; this.ordinals = new HashMap(); int i = 0; @@ -550,8 +567,8 @@ void toJson(Names names, JsonGenerator gen) throws IOException { private static class FixedSchema extends NamedSchema { private final int size; - public FixedSchema(String name, String space, int size) { - super(Type.FIXED, name, space); + public FixedSchema(String name, String doc, String space, int size) { + super(Type.FIXED, name, doc, space); this.size = size; } public int getFixedSize() { return size; } @@ -693,13 +710,16 @@ static Schema parse(JsonNode schema, Names names) { if (typeNode == null) throw new SchemaParseException("No type: "+schema); String type = typeNode.getTextValue(); - String name = null, space = null; + String name = null, space = null, doc = null; if (type.equals("record") || type.equals("error") || type.equals("enum") || type.equals("fixed")) { - JsonNode nameNode = schema.get("name"); - name = nameNode != null ? nameNode.getTextValue() : null; - JsonNode spaceNode = schema.get("namespace"); - space = spaceNode!=null?spaceNode.getTextValue():names.space(); + String key = "name"; + name = getStringValueOrNull(schema, "name"); + doc = getStringValueOrNull(schema, "doc"); + space = getStringValueOrNull(schema, "namespace"); + if (space == null) { + space = names.space(); + } if (names.space() == null && space != null) names.space(space); // set default namespace if (name == null) @@ -708,15 +728,17 @@ static Schema parse(JsonNode schema, Names names) { if (type.equals("record") || type.equals("error")) { // record LinkedHashMap fields = new LinkedHashMap(); RecordSchema result = - new RecordSchema(name, space, type.equals("error")); + new RecordSchema(name, doc, space, type.equals("error")); if (name != null) names.add(result); JsonNode fieldsNode = schema.get("fields"); if (fieldsNode == null || !fieldsNode.isArray()) throw new SchemaParseException("Record has no fields: "+schema); for (JsonNode field : fieldsNode) { - JsonNode fieldNameNode = field.get("name"); - if (fieldNameNode == null) + String fieldName = getStringValueOrNull(field, "name"); + String fieldDoc = getStringValueOrNull(field, "doc"); + if (fieldName == null) { throw new SchemaParseException("No field name: "+field); + } JsonNode fieldTypeNode = field.get("type"); if (fieldTypeNode == null) throw new SchemaParseException("No field type: "+field); @@ -725,8 +747,8 @@ static Schema parse(JsonNode schema, Names names) { JsonNode orderNode = field.get("order"); if (orderNode != null) order = Field.Order.valueOf(orderNode.getTextValue().toUpperCase()); - fields.put(fieldNameNode.getTextValue(), - new Field(fieldSchema, field.get("default"), order)); + fields.put(fieldName, + new Field(fieldSchema, fieldDoc, field.get("default"), order)); } result.setFields(fields); return result; @@ -737,7 +759,7 @@ static Schema parse(JsonNode schema, Names names) { List symbols = new ArrayList(); for (JsonNode n : symbolsNode) symbols.add(n.getTextValue()); - Schema result = new EnumSchema(name, space, symbols); + Schema result = new EnumSchema(name, doc, space, symbols); if (name != null) names.add(result); return result; } else if (type.equals("array")) { // array @@ -745,9 +767,16 @@ static Schema parse(JsonNode schema, Names names) { } else if (type.equals("map")) { // map return new MapSchema(parse(schema.get("values"), names)); } else if (type.equals("fixed")) { // fixed - Schema result = new FixedSchema(name, space, - schema.get("size").getIntValue()); - if (name != null) names.add(result); + JsonNode sizeNode = schema.get("size"); + if (sizeNode == null || !sizeNode.isInt()) { + throw new SchemaParseException( + "Fixed node has no or non-integer size: " + schema); + } + Schema result = new FixedSchema(name, doc, space, + sizeNode.getIntValue()); + if (name != null) { + names.add(result); + } return result; } else throw new SchemaParseException("Type not yet supported: "+type); @@ -761,6 +790,14 @@ static Schema parse(JsonNode schema, Names names) { } } + /** + * Extracts text value associated to key from the container JsonNode. + */ + private static String getStringValueOrNull(JsonNode container, String key) { + JsonNode jsonNode = container.get(key); + return jsonNode != null ? jsonNode.getTextValue() : null; + } + static JsonNode parseJson(String s) { try { return MAPPER.readTree(FACTORY.createJsonParser(new StringReader(s))); diff --git a/src/java/org/apache/avro/generic/GenericData.java b/src/java/org/apache/avro/generic/GenericData.java index 777872e..9a7df0c 100644 --- a/src/java/org/apache/avro/generic/GenericData.java +++ b/src/java/org/apache/avro/generic/GenericData.java @@ -267,7 +267,7 @@ public Schema induce(Object datum) { GenericRecord record = (GenericRecord)datum; LinkedHashMap fields = new LinkedHashMap(); for (Map.Entry entry : record.entrySet()) - fields.put(entry.getKey(), new Field(induce(entry.getValue()), null)); + fields.put(entry.getKey(), new Field(induce(entry.getValue()), null, null)); return Schema.createRecord(fields); } else if (datum instanceof GenericArray) { Schema elementType = null; @@ -299,7 +299,7 @@ public Schema induce(Object datum) { } return Schema.createMap(value); } else if (datum instanceof GenericFixed) { - return Schema.createFixed(null, null, + return Schema.createFixed(null, null, null, ((GenericFixed)datum).bytes().length); } else if (datum instanceof Utf8) return Schema.create(Type.STRING); diff --git a/src/java/org/apache/avro/reflect/ReflectData.java b/src/java/org/apache/avro/reflect/ReflectData.java index a47c441..071175d 100644 --- a/src/java/org/apache/avro/reflect/ReflectData.java +++ b/src/java/org/apache/avro/reflect/ReflectData.java @@ -217,7 +217,8 @@ public Schema getSchema(java.lang.reflect.Type type) { /** * Create a schema for a type and it's fields. Note that by design only fields * of the direct class, not it's super classes, are used for creating the - * schema. Also, fields are not permitted to be null. + * schema. Also, fields are not permitted to be null. Javadoc is unavailable + * via reflection, so the "doc" field of the schema is simply null. */ @SuppressWarnings(value="unchecked") protected Schema createSchema(java.lang.reflect.Type type, @@ -269,28 +270,28 @@ else if (type instanceof ParameterizedType) { Enum[] constants = (Enum[])c.getEnumConstants(); for (int i = 0; i < constants.length; i++) symbols.add(constants[i].name()); - schema = Schema.createEnum(name, space, symbols); + schema = Schema.createEnum(name, null /* doc */, space, symbols); names.put(fullName, schema); return schema; } // fixed if (GenericFixed.class.isAssignableFrom(c)) { int size = ((FixedSize)c.getAnnotation(FixedSize.class)).value(); - schema = Schema.createFixed(name, space, size); + schema = Schema.createFixed(name, null /* doc */, space, size); names.put(fullName, schema); return schema; } // record LinkedHashMap fields = new LinkedHashMap(); - schema = Schema.createRecord(name, space, + schema = Schema.createRecord(name, null /* doc */, space, Throwable.class.isAssignableFrom(c)); if (!names.containsKey(fullName)) names.put(fullName, schema); for (Field field : c.getDeclaredFields()) if ((field.getModifiers()&(Modifier.TRANSIENT|Modifier.STATIC))==0) { Schema fieldSchema = createFieldSchema(field, names); - fields.put(field.getName(), new Schema.Field(fieldSchema, null)); + fields.put(field.getName(), new Schema.Field(fieldSchema, null, null)); } schema.setFields(fields); } @@ -338,7 +339,7 @@ private Message getMessage(Method method, Protocol protocol, java.lang.reflect.Type[] paramTypes = method.getGenericParameterTypes(); for (int i = 0; i < paramTypes.length; i++) fields.put(paramNames[i], - new Schema.Field(createSchema(paramTypes[i], names), null)); + new Schema.Field(createSchema(paramTypes[i], names), null, null)); Schema request = Schema.createRecord(fields); Schema response = createSchema(method.getGenericReturnType(), names); diff --git a/src/java/org/apache/avro/specific/SpecificCompiler.java b/src/java/org/apache/avro/specific/SpecificCompiler.java index 0aa9c5b..f715c87 100644 --- a/src/java/org/apache/avro/specific/SpecificCompiler.java +++ b/src/java/org/apache/avro/specific/SpecificCompiler.java @@ -185,6 +185,7 @@ private void compile(Schema schema) throws IOException { try { switch (schema.getType()) { case RECORD: + doc(0, schema); line(0, "public class "+type(schema)+ (schema.isError() ? " extends SpecificExceptionBase" @@ -194,16 +195,19 @@ private void compile(Schema schema) throws IOException { line(1, "public static final Schema _SCHEMA = Schema.parse(\"" +esc(schema)+"\");"); // field declations - for (Map.Entry field : schema.getFieldSchemas()) + for (Map.Entry field : schema.getFieldSchemas()) { + doc(1, field.getValue()); line(1,"public "+unbox(field.getValue())+" "+field.getKey()+";"); + } // schema method line(1, "public Schema getSchema() { return _SCHEMA; }"); // get method line(1, "public Object get(int _field) {"); line(2, "switch (_field) {"); int i = 0; - for (Map.Entry field : schema.getFieldSchemas()) + for (Map.Entry field : schema.getFieldSchemas()) { line(2, "case "+(i++)+": return "+field.getKey()+";"); + } line(2, "default: throw new AvroRuntimeException(\"Bad index\");"); line(2, "}"); line(1, "}"); @@ -221,6 +225,7 @@ private void compile(Schema schema) throws IOException { line(0, "}"); break; case ENUM: + doc(0, schema); line(0, "public enum "+type(schema)+" { "); StringBuilder b = new StringBuilder(); int count = 0; @@ -233,6 +238,7 @@ private void compile(Schema schema) throws IOException { line(0, "}"); break; case FIXED: + doc(0, schema); line(0, "@FixedSize("+schema.getFixedSize()+")"); line(0, "public class "+type(schema)+" extends SpecificFixed {}"); break; @@ -246,6 +252,20 @@ private void compile(Schema schema) throws IOException { } } + private void doc(int indent, Schema schema) throws IOException { + String doc = schema.getDoc(); + if (doc != null) { + line(indent, "/** " + escapeForJavaDoc(doc) + " */"); + } + } + + /** + * TODO(philip) XXX figure out proper escaping + */ + private String escapeForJavaDoc(String doc) { + return doc; + } + private static final Schema NULL_SCHEMA = Schema.create(Schema.Type.NULL); private String type(Schema schema) { diff --git a/src/test/java/org/apache/avro/TestProtocolGeneric.java b/src/test/java/org/apache/avro/TestProtocolGeneric.java index 70e6c85..23aec95 100644 --- a/src/test/java/org/apache/avro/TestProtocolGeneric.java +++ b/src/test/java/org/apache/avro/TestProtocolGeneric.java @@ -165,9 +165,9 @@ public void testHandshake() throws IOException { Protocol protocol = new Protocol("Simple", "org.apache.avro.test"); LinkedHashMap fields = new LinkedHashMap(); fields.put("extra", - new Schema.Field(Schema.create(Schema.Type.BOOLEAN), null)); + new Schema.Field(Schema.create(Schema.Type.BOOLEAN), null, null)); fields.put("greeting", - new Schema.Field(Schema.create(Schema.Type.STRING), null)); + new Schema.Field(Schema.create(Schema.Type.STRING), null, null)); Protocol.Message message = protocol.createMessage("hello", Schema.createRecord(fields), diff --git a/src/test/java/org/apache/avro/TestSchema.java b/src/test/java/org/apache/avro/TestSchema.java index ed5b06b..e1bc506 100644 --- a/src/test/java/org/apache/avro/TestSchema.java +++ b/src/test/java/org/apache/avro/TestSchema.java @@ -176,6 +176,41 @@ public void testUnion() throws Exception { "{\"Bar\":\"a\"}"); checkJson(union, "X", "{\"Baz\":\"X\"}"); } + + /** + * Makes sure that "doc" tags are transcribed in the schemas. + * Note that there are docs both for fields and for the records + * themselves. + */ + @Test + public void testDocs() { + String jsonSchema = "{\n" + + " \"type\": \"record\",\n" + + " \"name\": \"outer_record\",\n" + + " \"doc\": \"This is not a world record.\",\n" + + " \"fields\": [\n" + + " { \"type\": { \"type\": \"fixed\", \"doc\": \"Very Inner Fixed\", " + + " \"name\": \"very_inner_fixed\", \"size\": 1 },\n" + + " \"doc\": \"Inner Fixed\", \"name\": \"inner_fixed\" },\n" + + " { \"type\": \"string\", \"doc\": \"Inner String\", \n" + + " \"name\": \"inner_string\" },\n" + + " { \"type\": { \"type\": \"enum\", \"doc\": \"Very Inner Enum\", \n" + + " \"name\": \"very_inner_enum\", \n" + + " \"symbols\": [ \"A\", \"B\", \"C\" ] },\n" + + " \"doc\": \"Inner Enum\", \"name\": \"inner_enum\" },\n" + + " { \"type\": [\"string\", \"int\"], \"doc\": \"Inner Union\", \n" + + " \"name\": \"inner_union\" }\n" + + " ]\n" + + "}\n"; + Schema schema = Schema.parse(jsonSchema); + assertEquals("This is not a world record.", schema.getDoc()); + assertEquals("Inner Fixed", schema.getFields().get("inner_fixed").doc()); + assertEquals("Very Inner Fixed", schema.getFields().get("inner_fixed").schema().getDoc()); + assertEquals("Inner String", schema.getFields().get("inner_string").doc()); + assertEquals("Inner Enum", schema.getFields().get("inner_enum").doc()); + assertEquals("Very Inner Enum", schema.getFields().get("inner_enum").schema().getDoc()); + assertEquals("Inner Union", schema.getFields().get("inner_union").doc()); + } private static void check(String schemaJson, String defaultJson, Object defaultValue) throws Exception { @@ -289,4 +324,5 @@ private static void checkDefault(String schemaJson, String defaultJson, assertEquals("Wrong toString", expected, Schema.parse(expected.toString())); } + }