You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by cu...@apache.org on 2010/10/07 23:31:15 UTC

svn commit: r1005644 - in /avro/trunk: CHANGES.txt lang/java/src/java/org/apache/avro/Schema.java lang/java/src/test/java/org/apache/avro/TestSchema.java

Author: cutting
Date: Thu Oct  7 21:31:15 2010
New Revision: 1005644

URL: http://svn.apache.org/viewvc?rev=1005644&view=rev
Log:
AVRO-671. Check that type and field names conform to specified requirements.

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/src/java/org/apache/avro/Schema.java
    avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1005644&r1=1005643&r2=1005644&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Thu Oct  7 21:31:15 2010
@@ -7,6 +7,9 @@ Avro 1.5.0 (unreleased)
     AVRO-670. Allow DataFileWriteTool to accept schema files as input with new
     --schema-file and --schema command-line flags. (Ron Bodkin via philz)
 
+    AVRO-671. Java: Check that type and field names conform to
+    specified requirements. (cutting)
+
 Avro 1.4.1 (unreleased)
 
   NEW FEATURES

Modified: avro/trunk/lang/java/src/java/org/apache/avro/Schema.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java?rev=1005644&r1=1005643&r2=1005644&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/Schema.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/Schema.java Thu Oct  7 21:31:15 2010
@@ -380,7 +380,7 @@ public abstract class Schema {
     }
     public Field(String name, Schema schema, String doc,
         JsonNode defaultValue, Order order) {
-      this.name = name;
+      this.name = validateName(name);
       this.schema = schema;
       this.doc = doc;
       this.defaultValue = defaultValue;
@@ -433,10 +433,10 @@ public abstract class Schema {
       int lastDot = name.lastIndexOf('.');
       if (lastDot < 0) {                          // unqualified name
         this.space = space;                       // use default space
-        this.name = name;
+        this.name = validateName(name);
       } else {                                    // qualified name
         this.space = name.substring(0, lastDot);  // get space from name
-        this.name = name.substring(lastDot+1, name.length());
+        this.name = validateName(name.substring(lastDot+1, name.length()));
       }
       this.full = (this.space == null) ? this.name : this.space+"."+this.name;
     }
@@ -655,7 +655,7 @@ public abstract class Schema {
       this.ordinals = new HashMap<String,Integer>();
       int i = 0;
       for (String symbol : symbols)
-        if (ordinals.put(symbol, i++) != null)
+        if (ordinals.put(validateName(symbol), i++) != null)
           throw new SchemaParseException("Duplicate enum symbol: "+symbol);
     }
     public List<String> getEnumSymbols() { return symbols; }
@@ -939,6 +939,21 @@ public abstract class Schema {
       return super.put(name, schema);
     }
   }
+  
+  private static String validateName(String name) {
+    int length = name.length();
+    if (length == 0)
+      throw new SchemaParseException("Empty name");
+    char first = name.charAt(0);
+    if (!(Character.isLetter(first) || first == '_'))
+      throw new SchemaParseException("Illegal initial character: "+name);
+    for (int i = 1; i < length; i++) {
+      char c = name.charAt(i);
+      if (!(Character.isLetterOrDigit(c) || c == '_'))
+        throw new SchemaParseException("Illegal character in: "+name);
+    }
+    return name;
+  }
 
   /** @see #parse(String) */
   static Schema parse(JsonNode schema, Names names) {

Modified: avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java?rev=1005644&r1=1005643&r2=1005644&view=diff
==============================================================================
--- avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java (original)
+++ avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java Thu Oct  7 21:31:15 2010
@@ -181,13 +181,29 @@ public class TestSchema {
                     +"[{\"name\":\"f\"}]}");       // no type
     checkParseError("{\"type\":\"record\",\"name\":\"X\",\"fields\":"
                     +"[{\"type\":\"long\"}]}");    // no name
+    // check invalid record names
+    checkParseError("{\"type\":\"record\",\"name\":\"1X\",\"fields\":[]}");
+    checkParseError("{\"type\":\"record\",\"name\":\"X$\",\"fields\":[]}");
+    // check invalid field names
+    checkParseError("{\"type\":\"record\",\"name\":\"X\",\"fields\":["
+                    +"{\"name\":\"1f\",\"type\":\"int\"}]}");
+    checkParseError("{\"type\":\"record\",\"name\":\"X\",\"fields\":["
+                    +"{\"name\":\"f$\",\"type\":\"int\"}]}");
+    checkParseError("{\"type\":\"record\",\"name\":\"X\",\"fields\":["
+                    +"{\"name\":\"f.g\",\"type\":\"int\"}]}");
   }
 
   @Test
   public void testEnum() throws Exception {
     check(BASIC_ENUM_SCHEMA, "\"B\"", new GenericData.EnumSymbol("B"), false);
     checkParseError("{\"type\":\"enum\"}");        // symbols required
-    checkParseError("{\"type\":\"enum\",\"symbols\": [\"X\",\"X\"]}");
+    checkParseError("{\"type\":\"enum\",\"symbols\": [\"X\"]}"); // name reqd
+    // check no duplicate symbols
+    checkParseError("{\"type\":\"enum\",\"name\":\"X\",\"symbols\":[\"X\",\"X\"]}");
+    // check no invalid symbols
+    checkParseError("{\"type\":\"enum\",\"name\":\"X\",\"symbols\":[\"1X\"]}");
+    checkParseError("{\"type\":\"enum\",\"name\":\"X\",\"symbols\":[\"X$\"]}");
+    checkParseError("{\"type\":\"enum\",\"name\":\"X\",\"symbols\":[\"X.Y\"]}");
   }
 
   @Test