refactor addParent() and removeParent()

Author: missinglink

Document.js

@@ -120,11 +120,40 @@ Document.prototype.hasName = model.hasChild( 'name' );
 Document.prototype.delName = model.delChild( 'name' );
 
 // parent
-Document.prototype.addParent = model.pushChild( 'parent' )
-                                  .validate( valid.type('string') )
-                                  .validate( valid.truthy() );
+Document.prototype.addParent = function( field, name, id, abbr ){
+  var add = model.pushChild( 'parent' )
+    .validate( valid.type('string') )
+    .validate( valid.truthy() )
+    .bind(this);
+
+  // mandatory fields, eg: 'country', 'country_id'
+  add( field, name );
+  add( field + '_id', id );
+
+  // optional field, eg: 'country_abbr'
+  if( arguments.length > 3 ){
+    add( field + '_abbr', abbr );
+  }
+
+  // chainable
+  return this;
+};
+
+Document.prototype.removeParent = function( field, name, id, abbr ){
+  var rm = model.spliceChild( 'parent' ).bind(this);
+
+  // mandatory fields, eg: 'country', 'country_id'
+  rm( field, name );
+  rm( field + '_id', id );
 
-Document.prototype.removeParent = model.spliceChild( 'parent' );
+  // optional field, eg: 'country_abbr'
+  if( arguments.length > 3 ){
+    rm( field + '_abbr', abbr );
+  }
+
+  // chainable
+  return this;
+};
 
 // address
 Document.prototype.setAddress = function ( prop, val ){
@@ -261,7 +290,7 @@ Document.adminFields = ['admin0','admin1','admin1_abbr','admin2','local_admin','
 
 Document.addressFields = ['name', 'number', 'street', 'zip'];
 
-Document.parentFields = ['alpha3','country','country_abbr','country_id','region','region_abbr',
+Document.parentFields = ['country','country_abbr','country_id','region','region_abbr',
   'region_id','county','county_abbr','county_id','locality','locality_abbr','locality_id',
   'localadmin','localadmin_abbr','localadmin_id','neighbourhood','neighbourhood_abbr','neighbourhood_id'];
 

README.md

@@ -22,10 +22,8 @@ var poi = new Document( 'geoname', 'venue', 1003 )
   .setName( 'alt', 'Haggerston City Farm' )
   .setAdmin( 'admin0', 'Great Britain' )
   .setAdmin( 'neighborhood', 'Shoreditch' )
-  .addParent( 'country', 'Great Britain' )
-  .addParent( 'neighbourhood', 'Shoreditch' )
-  .addParent( 'alpha3', 'GBR' )
-  .removeParent( 'alpha3', 'GBR' )
+  .addParent( 'country', 'Great Britain', '1001', 'GreatB' )
+  .addParent( 'neighbourhood', 'Shoreditch', '2002' )
   .setAddress( 'number', '10' )
   .setAddress( 'street', 'pelias place' )
   .addCategory( 'foo' )

errors.js

@@ -1,4 +1,6 @@
 
+var util = require('util');
+
 /**
   Custom Errors used in order to distinguish them from generic Errors:
 
@@ -21,13 +23,13 @@
 
 // Custom Error - PeliasModelError
 function PeliasModelError(message) {
-    this.name = 'PeliasModelError';
-    this.message = message;
-    this.stack = (new Error()).stack;
+  Error.captureStackTrace(this, this.constructor);
+  this.name = 'PeliasModelError';
+  this.message = message;
 }
 
 // Extends from js Error object
-PeliasModelError.prototype = new Error();
+util.inherits(PeliasModelError, Error);
 
 // export
 module.exports.PeliasModelError = PeliasModelError;

test/document/parent.js

@@ -6,8 +6,17 @@ module.exports.tests = {};
 module.exports.tests.addParent = function(test) {
   test('addParent', function(t) {
     var doc = new Document('mysource','mylayer','myid');
-    t.equal(doc.addParent('country','liberland'), doc, 'chainable');
+    t.equal(doc.addParent('country','liberland', 'liber_id', 'liber_abbr'), doc, 'chainable');
     t.equal(doc.parent.country[0], 'liberland', 'adder works');
+    t.equal(doc.parent.country_id[0], 'liber_id', 'adder works');
+    t.equal(doc.parent.country_abbr[0], 'liber_abbr', 'adder works');
+    t.end();
+  });
+  test('addParent - omit abbr', function(t) {
+    var doc = new Document('mysource','mylayer','myid');
+    t.equal(doc.addParent('country','liberland', 'liber_id'), doc, 'chainable');
+    t.equal(doc.parent.country[0], 'liberland', 'adder works');
+    t.equal(doc.parent.country_id[0], 'liber_id', 'adder works');
     t.end();
   });
   test('addParent - validate', function(t) {
@@ -21,8 +30,8 @@ module.exports.tests.addParent = function(test) {
     t.throws( doc.addParent.bind(doc, 'country', null), null, 'invalid property' );
     t.throws( doc.addParent.bind(doc, 'country', '\n'), null, 'invalid property' );
     t.equal(doc.parent.street, undefined, 'property unchanged');
-    t.doesNotThrow( doc.addParent.bind(doc, 'country', 'foo'), null, 'invalid property' );
-    t.doesNotThrow( doc.addParent.bind(doc, 'country', '1'), null, 'invalid property' );
+    t.doesNotThrow( doc.addParent.bind(doc, 'country', 'foo', 'bar','baz'), null, 'valid property' );
+    t.doesNotThrow( doc.addParent.bind(doc, 'country', '1', '1', '1'), null, 'valid property' );
     t.end();
   });
 };
@@ -31,8 +40,23 @@ module.exports.tests.removeParent = function(test) {
   test('removeParent', function(t) {
     var doc = new Document('mysource','mylayer','myid');
     doc.parent.country[0] = 'liberland';
-    t.equal(doc.removeParent('country','liberland'), doc, 'chainable');
+    doc.parent.country_id[0] = 'liber_id';
+    doc.parent.country_abbr[0] = 'liber_abbr';
+    t.equal(doc.removeParent('country','liberland', 'liber_id', 'liber_abbr'), doc, 'chainable');
+    t.equal(doc.parent.country.length, 0, 'remover works');
+    t.equal(doc.parent.country_id.length, 0, 'remover works');
+    t.equal(doc.parent.country_abbr.length, 0, 'remover works');
+    t.end();
+  });
+  test('removeParent - omit abbr', function(t) {
+    var doc = new Document('mysource','mylayer','myid');
+    doc.parent.country[0] = 'liberland';
+    doc.parent.country_id[0] = 'liber_id';
+    doc.parent.country_abbr[0] = 'liber_abbr';
+    t.equal(doc.removeParent('country','liberland', 'liber_id'), doc, 'chainable');
     t.equal(doc.parent.country.length, 0, 'remover works');
+    t.equal(doc.parent.country_id.length, 0, 'remover works');
+    t.equal(doc.parent.country_abbr.length, 1, 'abbr ignored');
     t.end();
   });
   test('removeParent - validate', function(t) {

test/serialize/test.js

@@ -30,7 +30,6 @@ module.exports.tests.minimal = function(test) {
       'name': {},
       'phrase': {},
       'parent': {
-        'alpha3': [],
         'country': [],
         'country_abbr': [],
         'country_id': [],
@@ -71,10 +70,8 @@ module.exports.tests.complete = function(test) {
       .setName( 'alt', 'Haggerston City Farm' )
       .setAdmin( 'admin0', 'Great Britain' )
       .setAdmin( 'neighborhood', 'Shoreditch' )
-      .addParent( 'country', 'Great Britain' )
-      .addParent( 'neighbourhood', 'Shoreditch' )
-      .addParent( 'alpha3', 'GBR' )
-      .removeParent( 'alpha3', 'GBR' )
+      .addParent( 'country', 'Great Britain', '1001', 'GreatB' )
+      .addParent( 'neighbourhood', 'Shoreditch', '2002' )
       .setAddress( 'number', '10' )
       .setAddress( 'street', 'pelias place' )
       .addCategory( 'foo' )
@@ -98,6 +95,7 @@ module.exports.tests.complete = function(test) {
       // data partitioning
       'source': 'geoname',
       'layer': 'venue',
+      'alpha3': 'GBR',
 
       // place name (ngram analysis)
       'name':{
@@ -118,16 +116,14 @@ module.exports.tests.complete = function(test) {
       },
 
       // Quattroshapes fields
-      'alpha3': 'GBR',
       'admin0': 'Great Britain',
       'neighborhood': 'Shoreditch',
 
       // WOF fields
       'parent': {
-        'alpha3': [],
         'country': ['Great Britain'],
-        'country_abbr': [],
-        'country_id': [],
+        'country_abbr': ['GreatB'],
+        'country_id': ['1001'],
         'county': [],
         'county_abbr': [],
         'county_id': [],
@@ -139,7 +135,7 @@ module.exports.tests.complete = function(test) {
         'locality_id': [],
         'neighbourhood': ['Shoreditch'],
         'neighbourhood_abbr': [],
-        'neighbourhood_id': [],
+        'neighbourhood_id': ['2002'],
         'region': [],
         'region_abbr': [],
         'region_id': []