You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by bl...@apache.org on 2015/12/08 22:34:43 UTC

svn commit: r1718715 - in /avro/trunk: CHANGES.txt lang/js/doc/API.md lang/js/lib/schemas.js lang/js/test/test_schemas.js

Author: blue
Date: Tue Dec  8 21:34:42 2015
New Revision: 1718715

URL: http://svn.apache.org/viewvc?rev=1718715&view=rev
Log:
AVRO-1767: Fall back to unwrapped union values. Contributed by Matthieu Monsch.

Previously, union wrapping during `type.clone` calls would be done in
place of normal copy. Now it is done as a backup if a normal copy fails.
This is more consistent with the documentation, and makes it much
simpler to wrap nested unions.

Other changes:

+ Add an optional argument to `type.getName` to expose built-in type
  names (useful to implement an "optional" logical type for example).
+ Document (and rename) `assertLogicalTypes` parsing option.
+ Tweak logical type validity check, enabling error hooks to work even
  when a logical type is valid for a subset of its underlying type's
  values (e.g.  when defining an "even integer" type). Previously the
  path would have been correct but the type would have been that of the
  underlying one.

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/js/doc/API.md
    avro/trunk/lang/js/lib/schemas.js
    avro/trunk/lang/js/test/test_schemas.js

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1718715&r1=1718714&r2=1718715&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Tue Dec  8 21:34:42 2015
@@ -63,6 +63,8 @@ Avro 1.8.0 (10 August 2014)
 
     AVRO-1747. JS: Add Javascript IO implementation. (Matthieu Monsch via blue)
 
+    AVRO-1767. JS: Fall back to unwrapped union values. (Matthieu Monsch via blue)
+
   OPTIMIZATIONS
 
   IMPROVEMENTS

Modified: avro/trunk/lang/js/doc/API.md
URL: http://svn.apache.org/viewvc/avro/trunk/lang/js/doc/API.md?rev=1718715&r1=1718714&r2=1718715&view=diff
==============================================================================
--- avro/trunk/lang/js/doc/API.md (original)
+++ avro/trunk/lang/js/doc/API.md Tue Dec  8 21:34:42 2015
@@ -33,12 +33,17 @@ limitations under the License.
     `'./Schema.avsc'`).
   + A decoded schema object (e.g. `{type: 'array', items: 'int'}`).
 + `opts` {Object} Parsing options. The following keys are currently supported:
+  + `namespace` {String} Optional parent namespace.
+  + `registry` {Object} Registry of predefined type names. This can for example
+    be used to override the types used for primitives or to split a schema
+    declaration over multiple files.
   + `logicalTypes` {Object} Optional dictionary of
     [`LogicalType`](#class-logicaltypeattrs-opts-types). This can be used to
     support serialization and deserialization of arbitrary native objects.
-  + `namespace` {String} Optional parent namespace.
-  + `registry` {Object} Optional registry of predefined type names. This can
-    for example be used to override the types used for primitives.
+  + `assertLogicalTypes` {Boolean} The Avro specification mandates that we fall
+    through to the underlying type if a logical type is invalid. When set, this
+    option will override this behavior and throw an error when a logical type
+    can't be applied.
   + `typeHook(attrs, opts)` {Function} Function called before each new type is
     instantiated. The relevant decoded schema is available as first argument
     and the parsing options as second. This function can optionally return a
@@ -336,7 +341,10 @@ evolution can be used to significantly s
 
 Returns a random value of `type`.
 
-##### `type.getName()`
+##### `type.getName([noRef])`
+
++ `noRef` {Boolean} Return built-in names (e.g. `'record'`, `'map'`,
+  `'boolean'`) rather than user-specified references.
 
 Returns `type`'s fully qualified name if it exists, `undefined` otherwise.
 

Modified: avro/trunk/lang/js/lib/schemas.js
URL: http://svn.apache.org/viewvc/avro/trunk/lang/js/lib/schemas.js?rev=1718715&r1=1718714&r2=1718715&view=diff
==============================================================================
--- avro/trunk/lang/js/lib/schemas.js (original)
+++ avro/trunk/lang/js/lib/schemas.js Tue Dec  8 21:34:42 2015
@@ -115,10 +115,7 @@ function createType(attrs, opts) {
       try {
         return new DerivedType(attrs, opts);
       } catch (err) {
-        if (opts.assertLogicalType) {
-          // The spec mandates that we fall through to the underlying type if
-          // the logical type is invalid. We provide this option to ease
-          // debugging.
+        if (opts.assertLogicalTypes) {
           throw err;
         }
         LOGICAL_TYPE = null;
@@ -305,7 +302,9 @@ Type.prototype.compareBuffers = function
   return this._match(new Tap(buf1), new Tap(buf2));
 };
 
-Type.prototype.getName = function () { return this._name; };
+Type.prototype.getName = function (noRef) {
+  return noRef ? getTypeName(this) : this._name;
+};
 
 Type.prototype.getSchema = function (noDeref) {
   // Since JS objects are unordered, this implementation (unfortunately)
@@ -870,19 +869,9 @@ UnionType.prototype._copy = function (va
   if (val === null && this._indices['null'] !== undefined) {
     return null;
   }
+
   var i, l, obj;
-  if (wrap === 1) {
-    // Promote into first match (convenience, slow).
-    i = 0;
-    l = this._types.length;
-    while (i < l && obj === undefined) {
-      try {
-        obj = this._types[i]._copy(val, opts);
-      } catch (err) {
-        i++;
-      }
-    }
-  } else if (typeof val == 'object') {
+  if (typeof val == 'object') {
     var keys = Object.keys(val);
     if (keys.length === 1) {
       var name = keys[0];
@@ -905,6 +894,18 @@ UnionType.prototype._copy = function (va
       }
     }
   }
+  if (wrap === 1 && obj === undefined) {
+    // Try promoting into first match (convenience, slow).
+    i = 0;
+    l = this._types.length;
+    while (i < l && obj === undefined) {
+      try {
+        obj = this._types[i]._copy(val, opts);
+      } catch (err) {
+        i++;
+      }
+    }
+  }
   if (obj !== undefined) {
     return new this._constructors[i](obj);
   }
@@ -1801,23 +1802,31 @@ LogicalType.prototype._read = function (
   return this._fromValue(this._underlyingType._read(tap));
 };
 
-LogicalType.prototype._write = function (tap, val) {
-  this._underlyingType._write(tap, this._toValue(val));
+LogicalType.prototype._write = function (tap, any) {
+  this._underlyingType._write(tap, this._toValue(any));
 };
 
-LogicalType.prototype._check = function (val, cb) {
-  return this._underlyingType._check(this._toValue(val), cb);
+LogicalType.prototype._check = function (any, cb) {
+  try {
+    var val = this._toValue(any);
+  } catch (err) {
+    if (cb) {
+      cb(PATH.slice(), any, this);
+    }
+    return false;
+  }
+  return this._underlyingType._check(val, cb);
 };
 
-LogicalType.prototype._copy = function (val, opts) {
+LogicalType.prototype._copy = function (any, opts) {
   var type = this._underlyingType;
   switch (opts && opts.coerce) {
     case 3: // To string.
-      return type._copy(this._toValue(val), opts);
+      return type._copy(this._toValue(any), opts);
     case 2: // From string.
-      return this._fromValue(type._copy(val, opts));
+      return this._fromValue(type._copy(any, opts));
     default: // Normal copy.
-      return this._fromValue(type._copy(this._toValue(val), opts));
+      return this._fromValue(type._copy(this._toValue(any), opts));
   }
 };
 

Modified: avro/trunk/lang/js/test/test_schemas.js
URL: http://svn.apache.org/viewvc/avro/trunk/lang/js/test/test_schemas.js?rev=1718715&r1=1718714&r2=1718715&view=diff
==============================================================================
--- avro/trunk/lang/js/test/test_schemas.js (original)
+++ avro/trunk/lang/js/test/test_schemas.js Tue Dec  8 21:34:42 2015
@@ -58,6 +58,12 @@ suite('types', function () {
       assert.equal(t.compareBuffers(bt, bt), 0);
     });
 
+    test('get name', function () {
+      var t = new types.BooleanType();
+      assert.strictEqual(t.getName(), undefined);
+      assert.equal(t.getName(true), 'boolean');
+    });
+
   });
 
   suite('IntType', function () {
@@ -1460,6 +1466,22 @@ suite('types', function () {
       assert.throws(function () { v2.createResolver(v1); });
     });
 
+    test('getName', function () {
+      var t = createType({
+        type: 'record',
+        name: 'Person',
+        doc: 'Hi!',
+        namespace: 'earth',
+        aliases: ['Human'],
+        fields: [
+          {name: 'friends', type: {type: 'array', items: 'string'}},
+          {name: 'age', aliases: ['years'], type: {type: 'int'}}
+        ]
+      });
+      assert.strictEqual(t.getName(), 'earth.Person');
+      assert.equal(t.getName(true), 'record');
+    });
+
     test('getSchema', function () {
       var t = createType({
         type: 'record',
@@ -1879,7 +1901,7 @@ suite('types', function () {
       assert.throws(function () {
         createType(attrs, {
           logicalTypes: logicalTypes,
-          assertLogicalType: true
+          assertLogicalTypes: true
         });
       });
     });
@@ -1968,6 +1990,151 @@ suite('types', function () {
       assert.equal(t2.fromBuffer(buf, res), +d);
     });
 
+    suite('union logical types', function () {
+      // Unions are slightly tricky to override with logical types since their
+      // schemas aren't represented as objects.
+
+      var schema = [
+        'null',
+        {
+          name: 'Person',
+          type: 'record',
+          fields: [
+            {name: 'name', type: 'string'},
+            {name: 'age', type: ['null', 'int'], 'default': null}
+          ]
+        }
+      ];
+
+      function createUnionTypeHook(Type) {
+        var visited = [];
+        return function(attrs, opts) {
+          if (attrs instanceof Array && !~visited.indexOf(attrs)) {
+            visited.push(attrs);
+            return new Type(attrs, opts);
+          }
+        };
+      }
+
+      test('unwrapped', function () {
+
+        /**
+        * A generic union type which exposes its values directly.
+        *
+        * This implementation is pretty minimal, we could optimize it by caching
+        * the underlying union's type names for example.
+        *
+        */
+        function UnwrappedUnionType(attrs, opts) {
+          types.LogicalType.call(this, attrs, opts);
+        }
+        util.inherits(UnwrappedUnionType, types.LogicalType);
+
+        UnwrappedUnionType.prototype._fromValue = function (val) {
+          return val === null ? null : val[Object.keys(val)[0]];
+        };
+
+        UnwrappedUnionType.prototype._toValue = function (any) {
+          return this.getUnderlyingType().clone(any, {wrapUnions: true});
+        };
+
+        var t1 = createType(
+          schema,
+          {typeHook: createUnionTypeHook(UnwrappedUnionType)}
+        );
+        var obj = {name: 'Ann', age: 23};
+        assert(t1.isValid(obj));
+        var buf = t1.toBuffer(obj);
+        var t2 = createType(schema);
+        assert.deepEqual(
+          t2.fromBuffer(buf),
+          {Person: {name: 'Ann', age: {'int': 23}}}
+        );
+
+      });
+
+      test('optional', function () {
+
+        /**
+        * A basic optional type.
+        *
+        * It assumes an underlying union of the form `["null", ???]`.
+        *
+        * Enhancements include:
+        *
+        * + Performing a check in the constructor on the underlying type (i.e.
+        *   union with the correct form).
+        * + Code-generating the conversion methods (especially a constructor
+        *   for `_toValue`).
+        *
+        */
+        function OptionalType(attrs, opts) {
+          types.LogicalType.call(this, attrs, opts);
+          var type = this.getUnderlyingType().getTypes()[1];
+          this._name = type.getName() || type.getName(true);
+        }
+        util.inherits(OptionalType, types.LogicalType);
+
+        OptionalType.prototype._fromValue = function (val) {
+          return val === null ? null : val[this._name];
+        };
+
+        OptionalType.prototype._toValue = function (any) {
+          if (any === null) {
+            return null;
+          }
+          var obj = {};
+          obj[this._name] = any;
+          return obj;
+        };
+
+        var t1 = createType(
+          schema,
+          {typeHook: createUnionTypeHook(OptionalType)}
+        );
+        var obj = {name: 'Ann', age: 23};
+        assert(t1.isValid(obj));
+        var buf = t1.toBuffer(obj);
+        var t2 = createType(schema);
+        assert.deepEqual(
+          t2.fromBuffer(buf),
+          {Person: {name: 'Ann', age: {'int': 23}}}
+        );
+
+      });
+
+    });
+
+    test('even integer', function () {
+      function EvenIntType(attrs, opts) {
+        types.LogicalType.call(this, attrs, opts, [types.IntType]);
+      }
+      util.inherits(EvenIntType, types.LogicalType);
+      EvenIntType.prototype._fromValue = function (val) {
+        this._assertValid(val);
+        return val;
+      };
+      EvenIntType.prototype._toValue = EvenIntType.prototype._fromValue;
+      EvenIntType.prototype._assertValid = function (any) {
+        if (any !== (any | 0) || any % 2) {
+          throw new Error('invalid');
+        }
+      };
+
+      var opts = {logicalTypes: {'even-integer': EvenIntType}};
+      var t = createType({type: 'int', logicalType: 'even-integer'}, opts);
+      assert(t.isValid(2));
+      assert(!t.isValid(3));
+      assert(!t.isValid('abc'));
+      assert.equal(t.fromBuffer(new Buffer([4])), 2);
+      assert.equal(t.clone(4), 4);
+      assert.equal(t.fromString('6'), 6);
+      assert.throws(function () { t.clone(3); });
+      assert.throws(function () { t.fromString('5'); });
+      assert.throws(function () { t.toBuffer(3); });
+      assert.throws(function () { t.fromBuffer(new Buffer([2])); });
+    });
+
   });
 
   suite('createType', function  () {
@@ -2205,7 +2372,7 @@ suite('types', function () {
 
   });
 
-  suite('type names', function () {
+  suite('type references', function () {
 
     test('existing', function () {
       var type = createType({