Validating and adding the values for configured addendum namespaces

Document.js

@@ -76,6 +76,8 @@ Document.prototype.toJSON = function(){
   return this;
 };
 
+Document.prototype.configuredAddendumNamespaces = _.get(config, 'addendum_namespaces', {});
+
 /*
  * Returns an object in exactly the format that Elasticsearch wants for inserts
  */
@@ -89,7 +91,7 @@ Document.prototype.toESDocument = function() {
     console.error(e);
   }
 
-  var doc = {
+  const doc = {
     name: this.name,
     phrase: this.name,
     parent: this.parent,
@@ -106,9 +108,13 @@ Document.prototype.toESDocument = function() {
     shape: this.shape
   };
 
-  // add encoded addendum namespaces
-  for( var namespace in this.addendum || {} ){
-    doc.addendum[namespace] = codec.encode(this.addendum[namespace]);
+  // encode non-configured addendum namespaces
+  for( const namespace in this.addendum || {} ){
+    if (this.configuredAddendumNamespaces.hasOwnProperty(namespace)) {
+      doc.addendum[namespace] = this.addendum[namespace];
+    } else {
+      doc.addendum[namespace] = codec.encode(this.addendum[namespace]);
+    }
   }
 
   // remove empty properties
@@ -144,7 +150,7 @@ Document.prototype.toESDocument = function() {
     _index: _.get(config, 'schema.indexName', 'pelias'),
     _id: this.getGid(),
     // In ES7, the only allowed document type will be `_doc`.
-    // However underscores are not allowed until ES6, so use `doc` for now
+    // However, underscores are not allowed until ES6, so use `doc` for now
     // see https://github.com/elastic/elasticsearch/pull/27816
     _type: _.get(config, 'schema.typeName', 'doc'),
     data: doc
@@ -218,7 +224,6 @@ Document.prototype.getGid = function(){
 };
 
 // meta
-
 Document.prototype.setMeta = function( prop, val ){
   this._meta[prop] = val;
   return this;
@@ -273,7 +278,7 @@ Document.prototype.setNameAlias = function( prop, value ){
     this.name[ prop ] = [ stringValue ];
   }
 
-  // is the array empty? ie. no prior call to setName()
+  // is the array empty? i.e. no prior call to setName()
   // in this case we will also set element 0 (the element used for display)
   if( !this.name[ prop ].length ){
     this.setName( prop, value );
@@ -531,10 +536,10 @@ Document.prototype.removeCategory = function( value ){
 Document.prototype.setAddendum = function( namespace, value ){
   validate.type('string', namespace);
   validate.truthy(namespace);
-  validate.type('object', value);
-  if( Object.keys(value).length > 0 ){
-    this.addendum[ namespace ] = value;
-  }
+  const configuredNamespace = this.configuredAddendumNamespaces[namespace];
+  const validationType =  configuredNamespace ? configuredNamespace.type : 'object';
+  validate.type(validationType, value);
+  this.addendum[ namespace ] = value;
   return this;
 };
 
@@ -609,7 +614,6 @@ Document.prototype.getBoundingBox = function(){
   return this.bounding_box;
 };
 
-
 Document.prototype.isSupportedParent = (placetype) => {
   return parentFields.indexOf(placetype) !== -1;
 };

test/Document.js

@@ -1,9 +1,11 @@
-var Document = require('../Document');
+const _ = require('lodash');
+const testDocument = require('./TestDocument');
+const Document = testDocument();
 
 module.exports.tests = {};
 
-module.exports.tests.interface = function(test) {
-  test('valid interface', function(t) {
+module.exports.tests.interface = (test) => {
+  test('valid interface', (t) => {
     t.equal(typeof Document, 'function', 'Document is a function');
 
     t.equal(typeof Document.prototype.getId, 'function', 'getId() is a function');
@@ -19,10 +21,10 @@ module.exports.tests.interface = function(test) {
   });
 };
 
-module.exports.tests.constructor = function(test) {
-  test('constructor args', function(t) {
+module.exports.tests.constructor = (test) => {
+  test('constructor args', (t) => {
 
-    var doc = new Document('mysource','mylayer','myid');
+    const doc = new Document('mysource', 'mylayer', 'myid');
 
     // initial values
     t.deepEqual(doc.name, {}, 'initial value');
@@ -44,9 +46,9 @@ module.exports.tests.constructor = function(test) {
   });
 };
 
-module.exports.tests.clearParent = function(test) {
-  test('clearParent should remove all effects of addParent calls', function(t) {
-    var doc = new Document('mysource','mylayer','myid');
+module.exports.tests.clearParent = (test) => {
+  test('clearParent should remove all effects of addParent calls', (t) => {
+    const doc = new Document('mysource', 'mylayer', 'myid');
     doc.addParent('locality', 'name 1', 'id 1', 'abbr 1');
     doc.addParent('locality', 'name 2', 'id 2', 'abbr 2');
 
@@ -67,7 +69,7 @@ module.exports.tests.clearParent = function(test) {
 
 module.exports.tests.clearAllParents = (test) => {
   test('clearParents should remove all effects of all addParent calls', (t) => {
-    const doc = new Document('mysource','mylayer','myid');
+    const doc = new Document('mysource', 'mylayer', 'myid');
     doc.getParentFields().forEach((type) => {
       doc.addParent(type, 'name 1', 'id 1', 'abbr 1');
       doc.addParent(type, 'name 2', 'id 2', 'abbr 2');
@@ -168,13 +170,132 @@ module.exports.tests.parent_types = (test) => {
 
 };
 
+module.exports.tests.addendum_namespaces = (test) => {
+  test('addendum namespace configured as array should be validated and kept as array in ESDocument', (t) => {
+
+    const Document = testDocument({
+      addendum_namespaces: {
+        tariff_zone_ids: {
+          type: 'array'
+        }
+      }
+    });
+    const doc = new Document('mysource', 'mylayer', 'myid');
+    doc.setAddendum('tariff_zone_ids', ['12', '34', '56']);
+
+    const esDocument = doc.toESDocument();
+    t.true(_.isArray(esDocument.data.addendum.tariff_zone_ids), 'should be array');
+    t.end();
+  });
+
+  test('addendum namespace configured as object should be validated and kept as object in ESDocument', (t) => {
+
+    const Document = testDocument({
+      addendum_namespaces: {
+        tariff_zone_ids: {
+          type: 'object'
+        }
+      }
+    });
+    const doc = new Document('mysource', 'mylayer', 'myid');
+    doc.setAddendum('tariff_zone_ids', { id: '123' });
+
+    const esDocument = doc.toESDocument();
+    t.true(_.isObject(esDocument.data.addendum.tariff_zone_ids), 'should be object');
+    t.deepEqual(esDocument.data.addendum.tariff_zone_ids, { id: '123' }, 'should be a valid object');
+    t.end();
+  });
+
+  test('addendum namespace configured as array should be validated and kept as array in ESDocument', (t) => {
+
+    const Document = testDocument({
+      addendum_namespaces: {
+        tariff_zone_ids: {
+          type: 'array'
+        }
+      }
+    });
+    const doc = new Document('mysource', 'mylayer', 'myid');
+    doc.setAddendum('tariff_zone_ids', ['123', '456', '789']);
+
+    const esDocument = doc.toESDocument();
+    t.true(_.isArray(esDocument.data.addendum.tariff_zone_ids), 'should be array');
+    t.deepEqual(esDocument.data.addendum.tariff_zone_ids, ['123', '456', '789'], 'should be a valid array');
+    t.end();
+  });
+
+  test('addendum namespace not configured should be validated as object and kept as encoded string in ESDocument',
+    (t) => {
+      const Document = testDocument();
+      const doc = new Document('mysource', 'mylayer', 'myid');
+      doc.setAddendum('tariff_zone_ids', { id: '123' });
+
+      const esDocument = doc.toESDocument();
+      t.false(_.isObject(esDocument.data.addendum.tariff_zone_ids), 'should not be object');
+      t.equal(esDocument.data.addendum.tariff_zone_ids, '{"id":"123"}', 'should be encoded object string');
+      t.end();
+    });
+
+  test('mix of configured and non-configured addendum namespaces should be properly handled',
+    (t) => {
+      const Document = testDocument({
+        addendum_namespaces: {
+          description: {
+            type: 'object'
+          }
+        }
+      });
+      const doc = new Document('mysource', 'mylayer', 'myid');
+      doc.setAddendum('tariff_zone_ids', { id: '123' });
+      doc.setAddendum('description', { nor: 'this is something' });
+
+      const esDocument = doc.toESDocument();
+      t.false(_.isObject(esDocument.data.addendum.tariff_zone_ids), 'should not be object');
+      t.equal(esDocument.data.addendum.tariff_zone_ids, '{"id":"123"}', 'should be encoded object string');
+      t.true(_.isObject(esDocument.data.addendum.description), 'should be object');
+      t.deepEqual(esDocument.data.addendum.description, { nor: 'this is something' }, 'should be object');
+      t.end();
+    });
+
+  test('setAddendum should fail if the configured type does not match with provided data', (t) => {
+
+    const Document = testDocument({
+      addendum_namespaces: {
+        tariff_zone_ids: {
+          type: 'array'
+        }
+      }
+    });
+    const doc = new Document('mysource', 'mylayer', 'myid');
+
+    t.throws(
+      () => doc.setAddendum('tariff_zone_ids', '12,34,56'),
+      /invalid document type, expecting: array got: 12,34,56/,
+      'Should fail for invalid data type.');
+
+    t.end();
+  });
+
+  test('non-configured type should always be a valid object', (t) => {
+
+    const doc = new Document('mysource', 'mylayer', 'myid');
+
+    t.throws(
+      () => doc.setAddendum('tariff_zone_ids', [123, 456, 789]),
+      /invalid document type, expecting: object, got array: 123,456,789/,
+      'Should always be a object');
+
+    t.end();
+  });
+};
+
 module.exports.all = function (tape, common) {
 
   function test(name, testFunction) {
     return tape('Document: ' + name, testFunction);
   }
 
-  for( var testCase in module.exports.tests ){
+  for (const testCase in module.exports.tests) {
     module.exports.tests[testCase](test, common);
   }
 };

test/DocumentMapperStream.js

@@ -1,7 +1,8 @@
-var createDocumentMapperStream = require('../DocumentMapperStream');
-var Document = require('../Document');
+const createDocumentMapperStream = require('../DocumentMapperStream');
+const testDocument = require('./TestDocument');
 
 const stream_mock = require('stream-mock');
+const Document = testDocument();
 
 function test_stream(input, testedStream, callback) {
   const reader = new stream_mock.ObjectReadableMock(input);
@@ -15,8 +16,8 @@ module.exports.tests = {};
 
 module.exports.tests.DocumentMapperStream = function(test) {
   test('createDocumentMapperStream', function(t) {
-    var stream = createDocumentMapperStream();
-    var document = new Document('source', 'layer', 'id');
+    const stream = createDocumentMapperStream();
+    const document = new Document('source', 'layer', 'id');
 
     test_stream([document], stream, function(err, results) {
       t.equal(results.length, 1, 'stream returns exactly one result');
@@ -32,7 +33,7 @@ module.exports.all = function (tape, common) {
     return tape('DocumentMapperStream: ' + name, testFunction);
   }
 
-  for( var testCase in module.exports.tests ){
+  for( const testCase in module.exports.tests ){
     module.exports.tests[testCase](test, common);
   }
 };

test/TestDocument.js

@@ -0,0 +1,23 @@
+const proxyquire = require('proxyquire').noCallThru();
+
+const defaultConfig = {
+  schema: {
+    indexName: 'example_index',
+    typeName: 'example_type',
+  },
+  addendum_namespaces: {}
+};
+
+const makePeliasConfig = (config) => {
+  const peliasConfig = Object.assign({}, defaultConfig, config);
+
+  return {
+    generate: () => Object.assign({}, peliasConfig, { get: (name) => peliasConfig[name] })
+  };
+};
+
+const testDocument = (config) => proxyquire('../Document', {
+  'pelias-config': makePeliasConfig(config ? config : {})
+});
+
+module.exports = testDocument;

test/benchmark.js

@@ -1,35 +1,38 @@
+const testDocument = require('./TestDocument');
+const fixtures = require('./serialize/fixtures');
+const iterations = 10000;
 
-var Document = require('../Document');
-var fixtures = require('./serialize/fixtures');
-var iterations = 10000;
+const Document = testDocument();
 
 // return the amount of milliseconds since 01 jan 1970
-function now(){ return (new Date()).getTime(); }
+function now() {
+  return (new Date()).getTime();
+}
 
-var start = now();
-for( var x=0; x<iterations; x++ ){
+const start = now();
+for (let x = 0; x < iterations; x++) {
 
   // create a new doc and serialize it
-  new Document( 'geoname', 'venue', 1003 )
-    .setMeta( 'author', 'peter' )
-    .setName( 'default', 'Hackney City Farm' )
-    .setName( 'alt', 'Haggerston City Farm' )
-    .addParent( 'country', 'Great Britain', '1001', 'GreatB' )
-    .addParent( 'neighbourhood', 'Shoreditch', '2002' )
-    .setAddress( 'number', '10' )
-    .setAddress( 'street', 'pelias place' )
-    .addCategory( 'foo' )
-    .addCategory( 'bar' )
-    .removeCategory( 'foo' )
+  new Document('geoname', 'venue', 1003)
+    .setMeta('author', 'peter')
+    .setName('default', 'Hackney City Farm')
+    .setName('alt', 'Haggerston City Farm')
+    .addParent('country', 'Great Britain', '1001', 'GreatB')
+    .addParent('neighbourhood', 'Shoreditch', '2002')
+    .setAddress('number', '10')
+    .setAddress('street', 'pelias place')
+    .addCategory('foo')
+    .addCategory('bar')
+    .removeCategory('foo')
     .setPopulation(10)
     .setPopularity(3)
     .setCentroid({ lon: 0.5, lat: 50.1 })
     .setShape(fixtures.new_zealand)
     .setBoundingBox(fixtures.new_zealand_bbox)
     .toESDocument();
 }
-var end = now();
+const end = now();
 
-var took = end - start;
-console.error( 'imported', iterations, 'docs in', took, 'ms');
-console.error( 'average speed:', (took / iterations).toFixed(4), 'ms' );
+const took = end - start;
+console.error('imported', iterations, 'docs in', took, 'ms');
+console.error('average speed:', (took / iterations).toFixed(4), 'ms');