chore(parameters): refactor provider constructor to init client as needed (#1757)

dreamorosi · web-flow · commit 665fff4c47c1 · 2023-10-20T23:23:41.000+02:00
* chore(parameters): refactor provider constructor to init client as needed

* chore(parameters): moved client instrumentation up in baseprovider

* chore(parameters): fix code smells

* chore(parameters): fix code smells
diff --git a/packages/parameters/src/appconfig/AppConfigProvider.ts b/packages/parameters/src/appconfig/AppConfigProvider.ts
@@ -10,10 +10,6 @@ import type {
   AppConfigGetOptions,
   AppConfigGetOutput,
 } from '../types/AppConfigProvider';
-import {
-  addUserAgentMiddleware,
-  isSdkClient,
-} from '@aws-lambda-powertools/commons';
 
 /**
  * ## Intro
@@ -185,7 +181,7 @@ import {
  * For more usage examples, see [our documentation](https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/parameters/).
  */
 class AppConfigProvider extends BaseProvider {
-  public client: AppConfigDataClient;
+  public client!: AppConfigDataClient;
   protected configurationTokenStore = new Map<string, string>();
   protected valueStore = new Map<string, Uint8Array>();
   private application?: string;
@@ -197,27 +193,22 @@ class AppConfigProvider extends BaseProvider {
    * @param {AppConfigProviderOptions} options - The configuration object.
    */
   public constructor(options: AppConfigProviderOptions) {
-    super();
-    this.client = new AppConfigDataClient(options.clientConfig || {});
-    if (options?.awsSdkV3Client) {
-      if (isSdkClient(options.awsSdkV3Client)) {
-        this.client = options.awsSdkV3Client;
-      } else {
-        console.warn(
-          'awsSdkV3Client is not an AWS SDK v3 client, using default client'
-        );
-      }
-    }
-    addUserAgentMiddleware(this.client, 'parameters');
+    super({
+      proto: AppConfigDataClient as new (
+        config?: unknown
+      ) => AppConfigDataClient,
+      clientConfig: options.clientConfig,
+      awsSdkV3Client: options.awsSdkV3Client,
+    });
 
-    this.application =
-      options?.application || this.envVarsService.getServiceName();
+    const { application, environment } = options;
+    this.application = application ?? this.envVarsService.getServiceName();
     if (!this.application || this.application.trim().length === 0) {
       throw new Error(
         'Application name is not defined or POWERTOOLS_SERVICE_NAME is not set'
       );
     }
-    this.environment = options.environment;
+    this.environment = environment;
   }
 
   /**
diff --git a/packages/parameters/src/base/BaseProvider.ts b/packages/parameters/src/base/BaseProvider.ts
@@ -1,6 +1,8 @@
 import {
+  addUserAgentMiddleware,
   isNullOrUndefined,
   isRecord,
+  isSdkClient,
   isString,
 } from '@aws-lambda-powertools/commons';
 import { GetOptions } from './GetOptions';
@@ -36,11 +38,33 @@ import type {
  */
 abstract class BaseProvider implements BaseProviderInterface {
   public envVarsService: EnvironmentVariablesService;
+  protected client: unknown;
   protected store: Map<string, ExpirableValue>;
 
-  public constructor() {
+  public constructor({
+    awsSdkV3Client,
+    clientConfig,
+    proto,
+  }: {
+    awsSdkV3Client?: unknown;
+    clientConfig?: unknown;
+    proto: new (config?: unknown) => unknown;
+  }) {
     this.store = new Map();
     this.envVarsService = new EnvironmentVariablesService();
+    if (awsSdkV3Client) {
+      if (!isSdkClient(awsSdkV3Client)) {
+        console.warn(
+          'awsSdkV3Client is not an AWS SDK v3 client, using default client'
+        );
+        this.client = new proto(clientConfig ?? {});
+      } else {
+        this.client = awsSdkV3Client;
+      }
+    } else {
+      this.client = new proto(clientConfig ?? {});
+    }
+    addUserAgentMiddleware(this.client, 'parameters');
   }
 
   /**
diff --git a/packages/parameters/src/dynamodb/DynamoDBProvider.ts b/packages/parameters/src/dynamodb/DynamoDBProvider.ts
@@ -18,10 +18,6 @@ import type {
 } from '@aws-sdk/client-dynamodb';
 import type { PaginationConfiguration } from '@aws-sdk/types';
 import type { JSONValue } from '@aws-lambda-powertools/commons';
-import {
-  addUserAgentMiddleware,
-  isSdkClient,
-} from '@aws-lambda-powertools/commons';
 
 /**
  * ## Intro
@@ -239,7 +235,7 @@ import {
  * For more usage examples, see [our documentation](https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/parameters/).
  */
 class DynamoDBProvider extends BaseProvider {
-  public client: DynamoDBClient;
+  public client!: DynamoDBClient;
   protected keyAttr = 'id';
   protected sortAttr = 'sk';
   protected tableName: string;
@@ -251,24 +247,17 @@ class DynamoDBProvider extends BaseProvider {
    * @param {DynamoDBProviderOptions} config - The configuration object.
    */
   public constructor(config: DynamoDBProviderOptions) {
-    super();
-
-    this.client = new DynamoDBClient(config?.clientConfig || {});
-    if (config?.awsSdkV3Client) {
-      if (isSdkClient(config.awsSdkV3Client)) {
-        this.client = config.awsSdkV3Client;
-      } else {
-        console.warn(
-          'awsSdkV3Client is not an AWS SDK v3 client, using default client'
-        );
-      }
-    }
-    addUserAgentMiddleware(this.client, 'parameters');
+    super({
+      awsSdkV3Client: config.awsSdkV3Client,
+      clientConfig: config.clientConfig,
+      proto: DynamoDBClient as new (config?: unknown) => DynamoDBClient,
+    });
 
-    this.tableName = config.tableName;
-    if (config.keyAttr) this.keyAttr = config.keyAttr;
-    if (config.sortAttr) this.sortAttr = config.sortAttr;
-    if (config.valueAttr) this.valueAttr = config.valueAttr;
+    const { tableName, keyAttr, sortAttr, valueAttr } = config;
+    this.tableName = tableName;
+    if (keyAttr) this.keyAttr = keyAttr;
+    if (sortAttr) this.sortAttr = sortAttr;
+    if (valueAttr) this.valueAttr = valueAttr;
   }
 
   /**
diff --git a/packages/parameters/src/secrets/SecretsProvider.ts b/packages/parameters/src/secrets/SecretsProvider.ts
@@ -9,10 +9,6 @@ import type {
   SecretsGetOptions,
   SecretsGetOutput,
 } from '../types/SecretsProvider';
-import {
-  addUserAgentMiddleware,
-  isSdkClient,
-} from '@aws-lambda-powertools/commons';
 
 /**
  * ## Intro
@@ -149,27 +145,20 @@ import {
  * @see https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/parameters/
  */
 class SecretsProvider extends BaseProvider {
-  public client: SecretsManagerClient;
+  public client!: SecretsManagerClient;
 
   /**
    * It initializes the SecretsProvider class.
    *
    * @param {SecretsProviderOptions} config - The configuration object.
    */
   public constructor(config?: SecretsProviderOptions) {
-    super();
-
-    this.client = new SecretsManagerClient(config?.clientConfig || {});
-    if (config?.awsSdkV3Client) {
-      if (isSdkClient(config.awsSdkV3Client)) {
-        this.client = config.awsSdkV3Client;
-      } else {
-        console.warn(
-          'awsSdkV3Client is not an AWS SDK v3 client, using default client'
-        );
-      }
-    }
-    addUserAgentMiddleware(this.client, 'parameters');
+    super({
+      proto: SecretsManagerClient as new (
+        config?: unknown
+      ) => SecretsManagerClient,
+      ...(config ?? {}),
+    });
   }
 
   /**
diff --git a/packages/parameters/src/ssm/SSMProvider.ts b/packages/parameters/src/ssm/SSMProvider.ts
@@ -27,10 +27,6 @@ import type {
   SSMGetParametersByNameFromCacheOutputType,
 } from '../types/SSMProvider';
 import type { PaginationConfiguration } from '@aws-sdk/types';
-import {
-  addUserAgentMiddleware,
-  isSdkClient,
-} from '@aws-lambda-powertools/commons';
 
 /**
  * ## Intro
@@ -266,7 +262,7 @@ import {
  * For more usage examples, see [our documentation](https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/parameters/).
  */
 class SSMProvider extends BaseProvider {
-  public client: SSMClient;
+  public client!: SSMClient;
   protected errorsKey = '_errors';
   protected maxGetParametersItems = 10;
 
@@ -276,19 +272,10 @@ class SSMProvider extends BaseProvider {
    * @param {SSMProviderOptions} config - The configuration object.
    */
   public constructor(config?: SSMProviderOptions) {
-    super();
-
-    this.client = new SSMClient(config?.clientConfig || {});
-    if (config?.awsSdkV3Client) {
-      if (isSdkClient(config.awsSdkV3Client)) {
-        this.client = config.awsSdkV3Client;
-      } else {
-        console.warn(
-          'awsSdkV3Client is not an AWS SDK v3 client, using default client'
-        );
-      }
-    }
-    addUserAgentMiddleware(this.client, 'parameters');
+    super({
+      proto: SSMClient as new (config?: unknown) => SSMClient,
+      ...(config ?? {}),
+    });
   }
 
   /**
diff --git a/packages/parameters/tests/unit/BaseProvider.test.ts b/packages/parameters/tests/unit/BaseProvider.test.ts
@@ -9,35 +9,51 @@ import { GetParameterError, TransformParameterError } from '../../src/errors';
 import { toBase64 } from '@aws-sdk/util-base64-node';
 
 const encoder = new TextEncoder();
+jest.mock('@aws-lambda-powertools/commons', () => ({
+  ...jest.requireActual('@aws-lambda-powertools/commons'),
+  addUserAgentMiddleware: jest.fn(),
+}));
+
+class TestProvider extends BaseProvider {
+  public constructor() {
+    super({
+      proto: class {
+        #name = 'TestProvider';
+
+        public hello(): string {
+          return this.#name;
+        }
+      },
+    });
+  }
 
-describe('Class: BaseProvider', () => {
-  afterEach(() => {
-    jest.clearAllMocks();
-  });
-
-  class TestProvider extends BaseProvider {
-    public _add(key: string, value: ExpirableValue): void {
-      this.store.set(key, value);
-    }
+  public _add(key: string, value: ExpirableValue): void {
+    this.store.set(key, value);
+  }
 
-    public _get(_name: string): Promise<string> {
-      throw Error('Not implemented.');
-    }
+  public _get(_name: string): Promise<string> {
+    throw Error('Not implemented.');
+  }
 
-    public _getKeyTest(key: string): ExpirableValue | undefined {
-      return this.store.get(key);
-    }
+  public _getKeyTest(key: string): ExpirableValue | undefined {
+    return this.store.get(key);
+  }
 
-    public _getMultiple(
-      _path: string
-    ): Promise<Record<string, string | undefined>> {
-      throw Error('Not implemented.');
-    }
+  public _getMultiple(
+    _path: string
+  ): Promise<Record<string, string | undefined>> {
+    throw Error('Not implemented.');
+  }
 
-    public _getStoreSize(): number {
-      return this.store.size;
-    }
+  public _getStoreSize(): number {
+    return this.store.size;
   }
+}
+
+describe('Class: BaseProvider', () => {
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
 
   describe('Method: addToCache', () => {
     test('when called with a value and maxAge equal to 0, it skips the cache entirely', () => {
@@ -581,18 +597,6 @@ describe('Class: BaseProvider', () => {
 });
 
 describe('Function: clearCaches', () => {
-  class TestProvider extends BaseProvider {
-    public _get(_name: string): Promise<string> {
-      throw Error('Not implemented.');
-    }
-
-    public _getMultiple(
-      _path: string
-    ): Promise<Record<string, string | undefined>> {
-      throw Error('Not implemented.');
-    }
-  }
-
   test('when called, it clears all the caches', () => {
     // Prepare
     const provider1 = new TestProvider();
