Merge pull request #21 from MarioArnt/tech/debt

refactor: remove duplication
neoxia · Jul 2, 2020 · acc8ce9 · acc8ce9
2 parents 20e00a8 + ad33fb3
commit acc8ce9
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 79 deletions.
diff --git a/src/base-model.ts b/src/base-model.ts
@@ -1,5 +1,5 @@
 /* eslint-disable import/no-unresolved,no-unused-vars */
-import { ObjectSchema, validate } from '@hapi/joi';
+import { ObjectSchema } from '@hapi/joi';
 import { AWSError } from 'aws-sdk';
 import { DocumentClient } from 'aws-sdk/clients/dynamodb';
 import { PromiseResult } from 'aws-sdk/lib/request';
@@ -118,7 +118,7 @@ export default abstract class Model<T> {
       throw Error('No item to save');
     }
     if (this.schema) {
-      const { error } = validate(toSave, this.schema);
+      const { error } = this.schema.validate(toSave);
       if (error) {
         throw new ValidationError('Validation error', error);
       }
@@ -256,6 +256,9 @@ export default abstract class Model<T> {
     options?: Partial<DocumentClient.DeleteItemInput>,
   ): Promise<PromiseResult<DocumentClient.DeleteItemOutput, AWSError>> {
     // Handle method overloading
+    if (!pk_item) {
+      throw Error('Missing HashKey');
+    }
     const pk = (pk_item as any)[this.pk] != null ? (pk_item as any)[this.pk] : pk_item;
     const sk: Key = sk_options != null && isKey(sk_options) ? sk_options : null;
     const deleteOptions: Partial<DocumentClient.GetItemInput> =
@@ -388,14 +391,16 @@ export default abstract class Model<T> {
     if (keys.length === 0) {
       return [];
     }
-    if (isComposite(keys)) {
-      // Split these IDs in batch of 100 as it is AWS DynamoDB batchGetItems operation limit
-      const batches: Array<Array<{ pk: any; sk?: any }>> = keys.reduce((all, one, idx) => {
+    const splitBatch = (_keys: any[]) =>
+      _keys.reduce((all, one, idx) => {
         const chunk = Math.floor(idx / 100);
         const currentBatches = all;
         currentBatches[chunk] = [].concat(all[chunk] || [], one);
         return currentBatches;
       }, []);
+    if (isComposite(keys)) {
+      // Split these IDs in batch of 100 as it is AWS DynamoDB batchGetItems operation limit
+      const batches = splitBatch(keys);
       // Perform the batchGet operation for each batch
       const responsesBatches: T[][] = await Promise.all(
         batches.map((batch: Array<{ pk: any; sk?: any }>) => this.getSingleBatch(batch, options)),
@@ -404,12 +409,7 @@ export default abstract class Model<T> {
       return responsesBatches.reduce((b1, b2) => b1.concat(b2), []);
     }
     // Split these IDs in batch of 100 as it is AWS DynamoDB batchGetItems operation limit
-    const batches: Key[][] = keys.reduce((all, one, idx) => {
-      const chunk = Math.floor(idx / 100);
-      const currentBatches = all;
-      currentBatches[chunk] = [].concat(all[chunk] || [], one);
-      return currentBatches;
-    }, []);
+    const batches = splitBatch(keys);
     // Perform the batchGet operation for each batch
     const responsesBatches: T[][] = await Promise.all(
       batches.map((batch: Key[]) => this.getSingleBatch(batch, options)),

diff --git a/src/condition-attribute.ts b/src/condition-attribute.ts
@@ -39,33 +39,34 @@ export default abstract class ConditionAttribute<T> {
 
   public abstract beginsWith(value: T): Condition<T>;
 
-  protected prepareEq(value: T): IArgs<T> {
+  protected prepareOp(value: T): { id: string; attr: Map<string, string>; values: Map<string, T> } {
     const id = this.getAttributeUniqueIdentifier();
     const { attr, values } = this.fillMaps(id, value);
+    return { id, attr, values };
+  }
+
+  protected prepareEq(value: T): IArgs<T> {
+    const { id, attr, values } = this.prepareOp(value);
     return [`#${id} = :${id}`, attr, values];
   }
 
   protected prepareGt(value: T): IArgs<T> {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return [`#${id} > :${id}`, attr, values];
   }
 
   protected prepareGe(value: T): IArgs<T> {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return [`#${id} >= :${id}`, attr, values];
   }
 
   protected prepareLt(value: T): IArgs<T> {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return [`#${id} < :${id}`, attr, values];
   }
 
   protected prepareLe(value: T): IArgs<T> {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return [`#${id} <= :${id}`, attr, values];
   }
 

diff --git a/src/conditions.ts b/src/conditions.ts
@@ -21,13 +21,17 @@ export default abstract class Condition<T> {
     this.values = values;
   }
 
-  public and(condition: Condition<T>): Condition<T> {
+  protected prepareExpression(condition: Condition<T>): void {
     condition.attributes.forEach((value, key) => {
       this.attributes.set(key, value);
     });
     condition.values.forEach((value, key) => {
       this.values.set(key, value);
     });
+  }
+
+  public and(condition: Condition<T>): Condition<T> {
+    this.prepareExpression(condition);
     this.expression = `${this.expression} AND (${condition.expression})`;
     return this;
   }

diff --git a/src/filter-attribute.ts b/src/filter-attribute.ts
@@ -4,6 +4,12 @@ import { FilterValue, FilterCondition } from './filter-conditions';
 import { Key } from './base-model';
 /* eslint-enable import/no-unresolved,no-unused-vars */
 
+interface IAttributesValues {
+  id: string;
+  attr: Map<string, string>;
+  values: Map<string, string>;
+}
+
 export default class FilterAttribute extends ConditionAttribute<FilterValue> {
   public eq(value: FilterValue): FilterCondition {
     return new FilterCondition(...super.prepareEq(value));
@@ -50,47 +56,47 @@ export default class FilterAttribute extends ConditionAttribute<FilterValue> {
     return new FilterCondition(`#${id} IN (${Array.from(val.keys()).join(',')})`, attr, val);
   }
 
-  public null(): FilterCondition {
+  private prepareAttributes(): { id: string; attr: Map<string, string> } {
     const id = this.getAttributeUniqueIdentifier();
     const attr: Map<string, string> = new Map();
-    const values: Map<string, string> = new Map();
     attr.set(`#${id}`, this.field);
+    return { id, attr };
+  }
+
+  private prepareAttributesAndValues(): IAttributesValues {
+    const { id, attr } = this.prepareAttributes();
+    const values: Map<string, string> = new Map();
     values.set(':null', null);
+    return { id, attr, values };
+  }
+
+  public null(): FilterCondition {
+    const { id, attr, values } = this.prepareAttributesAndValues();
     return new FilterCondition(`#${id} = :null`, attr, values);
   }
 
   public notNull(): FilterCondition {
-    const id = this.getAttributeUniqueIdentifier();
-    const attr: Map<string, string> = new Map();
-    const values: Map<string, string> = new Map();
-    attr.set(`#${id}`, this.field);
-    values.set(':null', null);
+    const { id, attr, values } = this.prepareAttributesAndValues();
     return new FilterCondition(`#${id} <> :null`, attr, values);
   }
 
   public exists(): FilterCondition {
-    const id = this.getAttributeUniqueIdentifier();
-    const attr: Map<string, string> = new Map();
-    attr.set(`#${id}`, this.field);
+    const { id, attr } = this.prepareAttributes();
     return new FilterCondition(`attribute_exists(#${id})`, attr, new Map());
   }
 
   public notExists(): FilterCondition {
-    const id = this.getAttributeUniqueIdentifier();
-    const attr: Map<string, string> = new Map();
-    attr.set(`#${id}`, this.field);
+    const { id, attr } = this.prepareAttributes();
     return new FilterCondition(`attribute_not_exists(#${id})`, attr, new Map());
   }
 
   public contains(value: string): FilterCondition {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return new FilterCondition(`contains(#${id}, :${id})`, attr, values);
   }
 
   public notContains(value: string): FilterCondition {
-    const id = this.getAttributeUniqueIdentifier();
-    const { attr, values } = this.fillMaps(id, value);
+    const { id, attr, values } = this.prepareOp(value);
     return new FilterCondition(`NOT contains(#${id}, :${id})`, attr, values);
   }
 }
diff --git a/src/filter-conditions.ts b/src/filter-conditions.ts
@@ -29,12 +29,7 @@ export class FilterCondition extends Condition<FilterValue> {
   }
 
   public or(condition: FilterCondition): FilterCondition {
-    condition.attributes.forEach((value, key) => {
-      this.attributes.set(key, value);
-    });
-    condition.values.forEach((value, key) => {
-      this.values.set(key, value);
-    });
+    this.prepareExpression(condition);
     this.expression = `${this.expression} OR (${condition.expression})`;
     return this;
   }

diff --git a/src/operation.ts b/src/operation.ts
@@ -66,8 +66,8 @@ export default abstract class Operation<T> {
       builtConditions = buildFilterConditions(filterConditions);
     }
     this.params.FilterExpression = builtConditions.expression;
-    this.addExpressionAttributesName(builtConditions.attributes);
-    this.addExpressionAttributesValue(builtConditions.values);
+    this.addExpressionAttributes(builtConditions.attributes, 'names');
+    this.addExpressionAttributes(builtConditions.values, 'values');
   }
 
   /**
@@ -99,9 +99,12 @@ export default abstract class Operation<T> {
     }
   }
 
-  public abstract projection(
+  public projection(
     fields: Array<string | { list: string; index: number } | { map: string; key: string }>,
-  ): Operation<T>;
+  ): Operation<T> {
+    this.doProject(fields);
+    return this;
+  }
 
   protected doProject(
     fields: Array<string | { list: string; index: number } | { map: string; key: string }>,
@@ -125,7 +128,7 @@ export default abstract class Operation<T> {
       return '';
     });
     this.params.ProjectionExpression = expression.join(', ');
-    this.addExpressionAttributesName(attributes);
+    this.addExpressionAttributes(attributes, 'names');
   }
 
   protected buildResponse(
@@ -141,21 +144,18 @@ export default abstract class Operation<T> {
     };
   }
 
-  protected addExpressionAttributesName(attributes: DocumentClient.ExpressionAttributeNameMap) {
+  protected addExpressionAttributes(
+    attributes:
+      | DocumentClient.ExpressionAttributeNameMap
+      | DocumentClient.ExpressionAttributeValueMap,
+    type: 'names' | 'values',
+  ) {
     if (Object.keys(attributes).length > 0) {
-      if (!this.params.ExpressionAttributeNames) {
-        this.params.ExpressionAttributeNames = {};
-      }
-      Object.assign(this.params.ExpressionAttributeNames, attributes);
-    }
-  }
-
-  protected addExpressionAttributesValue(values: DocumentClient.ExpressionAttributeValueMap) {
-    if (Object.keys(values).length > 0) {
-      if (!this.params.ExpressionAttributeValues) {
-        this.params.ExpressionAttributeValues = {};
+      const key = type === 'names' ? 'ExpressionAttributeNames' : 'ExpressionAttributeValues';
+      if (!this.params[key]) {
+        this.params[key] = {};
       }
-      Object.assign(this.params.ExpressionAttributeValues, values);
+      Object.assign(this.params[key], attributes);
     }
   }
 

diff --git a/src/query.ts b/src/query.ts
@@ -34,8 +34,8 @@ export default class Query<T> extends Operation<T> {
       builtConditions = buildKeyConditions(keyConditions);
     }
     this.params.KeyConditionExpression = builtConditions.expression;
-    this.addExpressionAttributesName(builtConditions.attributes);
-    this.addExpressionAttributesValue(builtConditions.values);
+    this.addExpressionAttributes(builtConditions.attributes, 'names');
+    this.addExpressionAttributes(builtConditions.values, 'values');
     return this;
   }
 
@@ -63,13 +63,6 @@ export default class Query<T> extends Operation<T> {
     return this;
   }
 
-  public projection(
-    fields: Array<string | { list: string; index: number } | { map: string; key: string }>,
-  ): Query<T> {
-    this.doProject(fields);
-    return this;
-  }
-
   /**
    * Whether the sort order should be ascending or descending.
    * Query is always sort on the table/index range key.

diff --git a/src/scan.ts b/src/scan.ts
@@ -24,13 +24,6 @@ export default class Scan<T> extends Operation<T> {
     return this;
   }
 
-  public projection(
-    fields: Array<string | { list: string; index: number } | { map: string; key: string }>,
-  ): Scan<T> {
-    this.doProject(fields);
-    return this;
-  }
-
   /**
    * Scan items in the limit of 1MB
    * @returns Fetched items, and pagination metadata

diff --git a/test/hooks/teardown.ts b/test/hooks/teardown.ts
@@ -5,5 +5,4 @@ import { deleteTables } from './create-tables';
 export default async (): Promise<void> => {
   console.log('Clearing database...');
   await deleteTables();
-  process.exit();
 };