From 0e8f4253c05ec15204236a2b192d5a414864d2b6 Mon Sep 17 00:00:00 2001
From: Julian Simioni <julian@simioni.org>
Date: Tue, 29 Sep 2015 19:08:23 -0400
Subject: [PATCH] Return multiple results in place when osm node and way share
 an id

It turns out the _type parameter to the Elasticsearch
[multiget](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-multi-get.html)
API does not allow an array of possible values. We were depending on its
ability to search multiple types to allow searching for OSM nodes and
ways.

But, since this doesn't work we essentially have to do it ourselves.
There is also the problem that OSM nodes and ways share an ID space. So
a gid such as `osm:venue:5` could in theory correspond to 2 records.
It seems like the only nice thing to do in that case is return both
results.

This PR "unrolls" such queries. For example, in the case of
`osm:venue:5`, the sanitisers will return the following array of objects
to be turned into multiget queries:

```
[{
  id: 5,
  types: ["osmway", "osmnode"]
}]
```

Before, this would turn into a multiget query with only one entry, like
this:
```
{
  "docs": [
    {
      "_index": "pelias",
      "_type": [
       " osmnode",
        "osmway"
      ],
      "_id": 5
    }
  ]
}
```

now it would look like this:
{
  "docs": [
    {
      "_index": "pelias",
      "_type": "osmnode",
      "_id": 5
    },
   {
      "_index": "pelias",
      "_type": "osmnode",
      "_id": 5
    }
  ]
}

TLDR you might get back more records from /place than the number of ids
you specified, but at least you will definitely get back what you are
looking for.
---
 controller/place.js           | 33 +++++++++++++++++++++++++++------
 test/unit/controller/place.js |  4 ++--
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/controller/place.js b/controller/place.js
index 2a4462cc..c4e78978 100644
--- a/controller/place.js
+++ b/controller/place.js
@@ -13,14 +13,35 @@ function setup( backend ){
       return next();
     }
 
-    var query = req.clean.ids.map( function(id) {
+    /* req.clean.ids contains an array of objects with id and types properties.
+     * types is an array of one or more types, since it can't always be known which single
+     * type a gid might belong to (osmnode and osmway both have source osm and layer venue).
+     *
+     * However, the mget Elasticsearch query only accepts a single type at a
+     * time.
+     *
+     * So, first create a new array that, has an entry
+     * with each type and id combination. This requires creating a new array with more entries
+     * than req.clean.ids in the case where entries have multiple types.
+     */
+
+    var recordsToReturn = req.clean.ids.reduce(function (acc, ids_element) {
+      ids_element.types.forEach(function(type) {
+        acc.push({
+          id: ids_element.id,
+          type: type
+        });
+      });
+      return acc;
+    }, []);
+
+    /*
+     * Next, map the list of records to an Elasticsearch mget query
+     */
+    var query = recordsToReturn.map( function(id) {
       return {
         _index: 'pelias',
-        /*
-         * some gids aren't resolvable to a single type (ex: osmnode and osmway
-         * both have source osm and layer venue), so expect an array of
-         * possible values. */
-        _type: id.types,
+        _type: id.type,
         _id: id.id
       };
     });
diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js
index 1d38f5d5..e8a8c333 100644
--- a/test/unit/controller/place.js
+++ b/test/unit/controller/place.js
@@ -41,7 +41,7 @@ module.exports.tests.functional_success = function(test, common) {
 
   test('functional success', function(t) {
     var backend = mockBackend( 'client/mget/ok/1', function( cmd ){
-      t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: [ 'a' ] } ] } }, 'correct backend command');
+      t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command');
     });
     var controller = setup( backend );
     var res = {
@@ -70,7 +70,7 @@ module.exports.tests.functional_success = function(test, common) {
 module.exports.tests.functional_failure = function(test, common) {
   test('functional failure', function(t) {
     var backend = mockBackend( 'client/mget/fail/1', function( cmd ){
-      t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: [ 'b' ] } ] } }, 'correct backend command');
+      t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'b' } ] } }, 'correct backend command');
     });
     var controller = setup( backend );
     var req = { clean: { ids: [ {'id' : 123, types: [ 'b' ] } ] }, errors: [], warnings: [] };