feat(assignLabels): improve duplicate label handling

Author: Joxit

middleware/assignLabels.js

@@ -1,6 +1,6 @@
 const _ = require('lodash');
 
-const defaultLabelGenerator = require('pelias-labels');
+const defaultLabelGenerator = require('pelias-labels').partsGenerator;
 
 function setup(labelGenerator) {
   function middleware(req, res, next) {
@@ -10,17 +10,55 @@ function setup(labelGenerator) {
   return middleware;
 }
 
+function getLabelFromLayer(parts, layer) {
+  const part = parts.find(p => p.layer === layer);
+  return _.get(part, 'label');
+}
+
+function filterUnambiguousParts(part, second) {
+  if (part.role === 'required') {
+    return false;
+  }
+  const label = getLabelFromLayer(second.parts, part.layer);
+  return label && label !== part.label;
+}
+
+function getBestLayers(results) {
+  const first = results[0];
+  const second = results[1];
+  return first.parts.filter(p => filterUnambiguousParts(p, second)).map(p => p.layer);
+}
+
 function assignLabel(req, res, next, labelGenerator) {
 
   // do nothing if there's nothing to process
   if (!res || !res.data) {
     return next();
   }
 
+  // This object will help for label deduplication
+  const dedupLabel = {};
+
+  // First we assign for all result the default label with all required layers
   res.data.forEach(function (result) {
-    result.label = labelGenerator(result, _.get(req, 'clean.lang.iso6393'));
+    const { labelParts, separator } = labelGenerator(result, _.get(req, 'clean.lang.iso6393'));
+    result.label = labelParts.filter(e => e.role === 'required').map(e => e.label).join(separator);
+    dedupLabel[result.label] = dedupLabel[result.label] || [];
+    dedupLabel[result.label].push({ result, labelParts, separator });
   });
 
+  // We check all values with more than one entry
+  Object.values(dedupLabel)
+    .filter(results => results.length > 1)
+    .forEach(results => {
+      // This array will contain all optional layers that should be displayed
+      const bestLayers = getBestLayers(results);
+      // We reassign the label with the new value
+      results.forEach(({ result, labelParts, separator }) => {
+        result.label = labelParts.filter(e => e.role === 'required' || bestLayers.indexOf(e.layer) >= 0).map(e => e.label).join(separator);
+      });
+    });
+
   next();
 }
 

package.json

@@ -50,7 +50,7 @@
     "morgan": "^1.8.2",
     "pelias-compare": "^0.1.16",
     "pelias-config": "^5.0.1",
-    "pelias-labels": "^1.19.0",
+    "pelias-labels": "pelias/labels#joxit/feat/with-optional",
     "pelias-logger": "^1.2.0",
     "pelias-microservice-wrapper": "^1.10.0",
     "pelias-model": "^9.0.0",

test/unit/middleware/assignLabels.js

@@ -1,5 +1,9 @@
 var proxyquire =  require('proxyquire').noCallThru();
 
+function partsGenerator(cb) {
+  return { partsGenerator: cb };
+}
+
 module.exports.tests = {};
 
 module.exports.tests.serialization = function(test, common) {
@@ -30,10 +34,10 @@ module.exports.tests.serialization = function(test, common) {
   test('labels should be assigned to all results', function(t) {
     var labelGenerator = function(result) {
       if (result.id === 1) {
-        return 'label 1';
+        return { labelParts: [{ label: 'label 1', role: 'required' }], separator: ', '};
       }
       if (result.id === 2) {
-        return 'label 2';
+        return { labelParts: [{ label: 'label 2', role: 'required' }], separator: ', '};
       }
 
     };
@@ -73,11 +77,11 @@ module.exports.tests.serialization = function(test, common) {
 
   test('no explicit labelGenerator supplied should use pelias-labels module', function(t) {
     var assignLabels = proxyquire('../../../middleware/assignLabels', {
-      'pelias-labels': function(result) {
+      'pelias-labels': partsGenerator(function(result) {
         if (result.id === 1) {
-          return 'label 1';
+          return { labelParts: [{ label: 'label 1', role: 'required' }], separator: ', '};
         }
-      }
+      })
     })();
 
     var input = {