Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapType _copy function is not really compatible with Map object #329

Closed
davyzhang opened this issue Nov 25, 2020 · 3 comments
Closed

MapType _copy function is not really compatible with Map object #329

davyzhang opened this issue Nov 25, 2020 · 3 comments

Comments

@davyzhang
Copy link

MapType.prototype._copy = function (val, opts) {
  if (val && typeof val == 'object' && !Array.isArray(val)) {
    var values = this.valuesType;
    var keys = Object.keys(val);
    var i, l, key;
    var copy = {};
    for (i = 0, l = keys.length; i < l; i++) {
      key = keys[i];
      copy[key] = values._copy(val[key], opts);
    }
    return copy;
  }
  throwInvalidError(val, this);
};

If the val is a Map object, this function will copy nothing from the keys.

Can we change it to be compatible with the Map object by checking if it is Map first?

val instanceof Map

should do the trick

@davyzhang
Copy link
Author

That won't be this simple if I try to deserialize the data to the Object or Map. I figure there's no way to make it consistent except adding another parameter

@mtth
Copy link
Owner

mtth commented Dec 6, 2020

Hi @davyzhang. You can use a logical type to represent Avro maps as JS maps. For example something like:

/** A minimal logical type which represents Avro maps as JS maps. */
class MapType extends avro.types.LogicalType {
  _fromValue(obj) {
    return new Map(Object.entries(obj));
  }
  _toValue(map) {
    return map instanceof Map ? Object.fromEntries(map) : undefined;
  }
}

/** A type hook which applies the above logical type to all maps. */
function createTypeHook() {
  const seen = new Set();
  return (schema, opts) => {
    if (!schema || schema.type !== 'map' || seen.has(schema)) {
      return; // Fall back to default logic.
    }
    seen.add(schema);
    return new MapType(schema, opts);
  };
}

Sample usage:

const schema = {
  name: 'Test',
  type: 'record',
  fields: [
    {name: 'foo', type: {type: 'map', values: 'int'}},
  ],
};
const type = avro.Type.forSchema(schema, {typeHook: createTypeHook()});

const data = {foo: new Map([['a', 1]])};
console.log(type.isValid(data)); // true
console.log(type.clone(data).foo.get('a')); // 1

@davyzhang
Copy link
Author

davyzhang commented Dec 7, 2020

That's great! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants