refactor: avoid duplicate/unneeded code (search for objects by UID)

Related to #70

src/storage/elasticsearch/common.ts

@@ -13,7 +13,7 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
-import {SCThings, SCThingType} from '@openstapps/core';
+import {SCThingType} from '@openstapps/core';
 import {SCThing} from '@openstapps/core';
 import {
   ESAggMatchAllFilter,
@@ -30,7 +30,7 @@ import {NameList} from 'elasticsearch';
  */
 interface Bucket {
   /**
-   * Number of documents in the agregation bucket
+   * Number of documents in the aggregation bucket
    */
   doc_count: number;
 
@@ -73,21 +73,6 @@ export function isBucketAggregation(agg: BucketAggregation | number): agg is Buc
   return typeof agg !== 'number';
 }
 
-/**
- * A response that contains info about whether the item exists plus the object itself
- */
-export interface ItemExistsResponse {
-  /**
-   * Whether the item exists
-   */
-  exists: boolean;
-
-  /**
-   * The object if it exists
-   */
-  object?: ElasticsearchObject<SCThings>;
-}
-
 /**
  * An aggregation that contains more aggregations nested inside
  */
@@ -183,7 +168,7 @@ export interface ElasticsearchQueryQueryStringConfig {
 }
 
 /**
- * A hit in an elastiscsearch search result
+ * A hit in an elasticsearch search result
  * @see https://www.elastic.co/guide/en/elasticsearch/reference/5.6/mapping-fields.html
  */
 export interface ElasticsearchObject<T extends SCThing> {

src/storage/elasticsearch/elasticsearch.ts

@@ -37,10 +37,9 @@ import {buildAggregations, parseAggregations} from './aggregations';
 import {
   AggregationResponse,
   AggregationSchema,
-  ElasticsearchConfig,
+  ElasticsearchConfig, ElasticsearchObject,
   ElasticsearchQueryDisMaxConfig,
   ElasticsearchQueryQueryStringConfig,
-  ItemExistsResponse,
 } from './common';
 import * as Monitoring from './monitoring';
 import {buildQuery, buildSort} from './query';
@@ -213,39 +212,6 @@ export class Elasticsearch implements Database {
     this.mailQueue = mailQueue;
   }
 
-  /**
-   * Tests if an object already exists
-   *
-   * Returns Elasticsearch Object if it exists
-   */
-  private async doesItemExist(object: SCThings): Promise<ItemExistsResponse> {
-    const searchResponse: ApiResponse<SearchResponse<SCThings>> = await this.client.search({
-      body: {
-        query: {
-          term: {
-            'uid.raw': {
-              value: object.uid,
-            },
-          },
-        },
-      },
-      from: 0,
-      index: Elasticsearch.getListOfAllIndices(),
-      size: 1,
-    });
-
-    if (searchResponse.body.hits.total > 0) {
-      return {
-        exists: true,
-        object: searchResponse.body.hits.hits[0],
-      };
-    }
-
-    return {
-      exists: false,
-    };
-  }
-
   /**
    * Gets a map which contains each alias and all indices that are associated with each alias
    */
@@ -315,6 +281,31 @@ export class Elasticsearch implements Database {
     Logger.ok(`Read alias map from elasticsearch: ${JSON.stringify(this.aliasMap, null, 2)}`);
   }
 
+  /**
+   * Provides an elasticsearch object using containing thing's UID
+   * @param uid an UID to use for the search
+   * @returns an elasticsearch object containing the thing
+   */
+  private async getObject(uid: SCUuid): Promise<ElasticsearchObject<SCThings> | undefined> {
+    const searchResponse: ApiResponse<SearchResponse<SCThings>> = await this.client.search({
+      body: {
+        query: {
+          term: {
+            'uid.raw': {
+              value: uid,
+            },
+          },
+        },
+      },
+      from: 0,
+      index: Elasticsearch.getListOfAllIndices(),
+      size: 1,
+    });
+
+    // return data from response
+    return searchResponse.body.hits.hits[0];
+  }
+
   /**
    * Should be called, when a new bulk was created. Creates a new index and applies a the mapping to the index
    * @param bulk the bulk process that was created
@@ -452,25 +443,13 @@ export class Elasticsearch implements Database {
    * @param uid uid of an SCThing
    */
   public async get(uid: SCUuid): Promise<SCThings> {
-    const searchResponse: ApiResponse<SearchResponse<SCThings>> = await this.client.search({
+    const object = await this.getObject(uid);
-      body: {
-        query: {
-          term: {
-            uid,
-          },
-        },
-      },
-      index: Elasticsearch.getListOfAllIndices(),
-    });
 
-    // get data from response
+    if (typeof object === 'undefined') {
-    const hits = searchResponse.body.hits.hits;
+      throw new Error('Item not found.');
-
-    if (hits.length !== 1) {
-      throw new Error('No unique item found.');
     }
 
-    return hits[0]._source;
+    return object._source;
   }
 
   /**
@@ -504,12 +483,12 @@ export class Elasticsearch implements Database {
         .format(),
     };
 
-    const itemMeta = await this.doesItemExist(obj);
+    const item = await this.getObject(object.uid);
 
     // we have to check that the item will get replaced if the index is rolled over
-    if (itemMeta.exists && typeof itemMeta.object !== 'undefined') {
+    if (typeof item !== 'undefined') {
       const indexOfNew = Elasticsearch.getIndex(obj.type, bulk.source, bulk);
-      const oldIndex = itemMeta.object._index;
+      const oldIndex = item._index;
 
       // new item doesn't replace the old one
       if (oldIndex.substring(0, oldIndex.length - Elasticsearch.INDEX_UID_LENGTH + 1)
@@ -541,15 +520,15 @@ export class Elasticsearch implements Database {
    */
   public async put(object: SCThings): Promise<void> {
 
-    const itemMeta = await this.doesItemExist(object);
+    const item = await this.getObject(object.uid);
 
-    if (itemMeta.exists && typeof itemMeta.object !== 'undefined') {
+    if (typeof item !== 'undefined') {
       await this.client.update({
         body: {
           doc: object,
         },
         id: object.uid,
-        index: itemMeta.object._index,
+        index: item._index,
         type: object.type.toLowerCase(),
       });