Merge pull request #84 from deliveryhero/PUB-77-misc

deliveryhero · Nov 9, 2021 · 85a151f · 85a151f
2 parents 5253821 + d75a4db
commit 85a151f
Show file tree

Hide file tree

Showing 26 changed files with 245 additions and 197 deletions.
diff --git a/.changeset/three-roses-hammer.md b/.changeset/three-roses-hammer.md
@@ -0,0 +1,5 @@
+---
+'@honestfoodcompany/pubsub': major
+---
+
+Change project id env var to GOOGLE_CLOUD_PROJECT, don't export private methods, refactor internals, and remove ackDeadlineSeconds - PUB-77
diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md
@@ -6,7 +6,7 @@ Create a `.env` file like this:
 
 ```env
 PUBSUB_ROOT_DIR=./examples/typescript
-GOOGLE_CLOUD_PUB_SUB_PROJECT_ID=<your-project-id>
+GOOGLE_CLOUD_PROJECT=<your-project-id>
 GOOGLE_APPLICATION_CREDENTIALS=./creds/<your-project-id-credentials-file>.json
 ```
 

diff --git a/__tests__/pubsub.service.synchronous.test.ts b/__tests__/pubsub.service.synchronous.test.ts
@@ -1,4 +1,5 @@
-import { PubSubService, Subscriber } from '@honestfoodcompany/pubsub';
+import { PubSubService } from '@honestfoodcompany/pubsub';
+import { SubscriberV2 as Subscriber } from '../src/subscriber';
 import TestTopic from './pubsub/topics/test-topic';
 import ExampleSubscriber from './pubsub/subscriptions/test-topic.example.subscription';
 

diff --git a/__tests__/subscriber.test.ts b/__tests__/subscriber.test.ts
@@ -1,10 +1,15 @@
-import { Subscriber, Message } from '@honestfoodcompany/pubsub';
+import { Message } from '@honestfoodcompany/pubsub';
+import { SubscriberV2 as Subscriber } from '../src/subscriber';
 
-Message.from('test message');
-const mockMessage = jest.fn(() => new Message());
+const mockMessage = Message.from('test message');
+mockMessage.ack = jest.fn(() => new Message());
 
-describe('message', () => {
-  const subscriber = new Subscriber();
+describe('Subscriber', () => {
+  const subscriber = new Subscriber({
+    handleMessage: (msg) => msg.ack(),
+    topicName: 'test.topic',
+    subscriptionName: 'test.topic.subscription',
+  });
   it('should be a class', () => {
     expect(subscriber).toBeInstanceOf(Subscriber);
   });
@@ -14,7 +19,7 @@ describe('message', () => {
   });
 
   it('should use handleMessage properly', async () => {
-    await subscriber.handleMessage(mockMessage());
-    expect(mockMessage).toHaveBeenCalled();
+    await subscriber.handleMessage(mockMessage);
+    expect(mockMessage.ack).toHaveBeenCalled();
   });
 });
diff --git a/__tests__/subscription.auto-load.test.ts b/__tests__/subscription.auto-load.test.ts
@@ -1,7 +1,5 @@
-import {
-  SubscriberTuple,
-  SubscriptionService,
-} from '@honestfoodcompany/pubsub';
+import { SubscriptionService } from '@honestfoodcompany/pubsub';
+import { SubscriberTuple } from '../src/subscriber';
 import generateMockMessage from './helpers/generateMockMessage';
 
 const mockPubSub = jest.fn();
@@ -52,9 +50,7 @@ describe('@auto-load subscription tests', () => {
     );
     expect(subscriberTuple).toBeDefined();
     if (!subscriberTuple) throw new Error('Invalid subscriber');
-    const subscriberClass = subscriberTuple[0];
-    const metadata = subscriberTuple[1];
-    const subscriber = new subscriberClass();
+    const [subscriber, metadata] = subscriberTuple;
     expect(() =>
       subscriber.handleMessage(generateMockMessage({ subscriber: metadata })),
     ).not.toThrow();

diff --git a/__tests__/subscription.with-custom-credentials.test.ts b/__tests__/subscription.with-custom-credentials.test.ts
@@ -1,8 +1,5 @@
-import {
-  PubSubService,
-  SubscriptionService,
-  SubscriberTuple,
-} from '@honestfoodcompany/pubsub';
+import { PubSubService, SubscriptionService } from '@honestfoodcompany/pubsub';
+import { SubscriberTuple } from '../src/subscriber';
 import GooglePubSubAdapter from '../src/client/googlePubSub';
 
 process.env.PUBSUB_DRIVER = 'google';
@@ -48,6 +45,19 @@ jest.mock('@google-cloud/pubsub', () => {
   };
 });
 
+jest.mock('@google-cloud/resource', () => {
+  return {
+    __esModule: true,
+    Resource: jest.fn().mockImplementation(() => {
+      return {
+        project: jest.fn(() => ({
+          get: jest.fn(() => Promise.resolve('dummyProjectNum')),
+        })),
+      };
+    }),
+  };
+});
+
 describe('With Project Credentials', (): void => {
   let subscriptions: SubscriberTuple[];
   let subscription: SubscriberTuple;

diff --git a/docs/getting-started/Installation.md b/docs/getting-started/Installation.md
@@ -66,15 +66,15 @@ The framework expects that you've created a pubsub directory in your project wit
 └── package.json
 ```
 
-## Required Environment Variables/CLI Args
+## Environment Variables/CLI Args
 
 The framework expects the following environment variables. They can be added to the `.env` file or passed through CLI args.
 
 | CLI Argument                     | Env Variable                      | Description                                                                                                                        |
 | -------------------------------- | --------------------------------- | -----------------------------------------------------------------------------------------------------------------------------------|
 | `root-dir`                       | `PUBSUB_ROOT_DIR`                 | must be the path to your project's pubsub directory.                                                                               |
 | `google-application-credentials` | `GOOGLE_APPLICATION_CREDENTIALS`  | see <https://cloud.google.com/docs/authentication/getting-started#creating_a_service_account> to generate this                     |
-| `project-id`                     | `GOOGLE_CLOUD_PUB_SUB_PROJECT_ID` | name of the project in Google Cloud Platform                                                                                       |
+| `project-id`                     | `GOOGLE_CLOUD_PROJECT` | the project-id in Google Cloud Platform                                                                                       |
 | `server-port`                    | `PUBSUB_SERVER_PORT`              | PORT at which the pubsub should run the server at                                                                                  |
 | `health-server`                  | `PUBSUB_HEALTH_SERVER`            | If value assigned is true this would run a server showing health state and return 500 if not healthy                               |
 

diff --git a/docs/guides/CLI.md b/docs/guides/CLI.md
@@ -25,7 +25,6 @@ Options:
   --version                         Show version number                                         [boolean]
   --project-id                      Google project id                                           [string]
   --root-dir                        Path where pubsub folder resides                            [string]
-  --project-number                  (Optional) project number for the project                   [string]
   --driver                          Use value `synchronous` or leave it to default              [string]
   --google-application-credentials  Path to exported credentials file                           [string]
   --server-port                     Port at which http health server would run (default:8080)   [number]                                                                      [string]

diff --git a/docs/guides/Drivers.md b/docs/guides/Drivers.md
@@ -8,14 +8,14 @@ sidebar_position: 2
 
 This is the default driver. It uses [Google Cloud PubSub](https://cloud.google.com/pubsub) to send messages.
 
-It requires the env vars `GOOGLE_APPLICATION_CREDENTIALS` and `GOOGLE_CLOUD_PUB_SUB_PROJECT_ID` to function correctly.
+The GCP project-id can be set via the env var and `GOOGLE_CLOUD_PROJECT` and service account via `GOOGLE_APPLICATION_CREDENTIALS` to function correctly. If not provided then the sdk will automatically try to get the values from home directory/service account.
+
+The role assigned to the service account in the credentials should be [`roles/pubsub.admin`](https://cloud.google.com/pubsub/docs/access-control)
 
 :::note
  **NOTE:** Make sure the service account is assigned the correct roles
 :::
 
-The role assigned to the service account in the credentials should be [`roles/pubsub.admin`](https://cloud.google.com/pubsub/docs/access-control)
-
 This is required because we require the below permissions:
 
 | Permission                            | Reason                                                                                                                      |

diff --git a/docs/guides/Migrating to v2.mdx b/docs/guides/Migrating to v2.mdx
@@ -15,13 +15,23 @@ These are the changes made for 2.0.0 that are breaking or just important to know
 
 Minimum supported node version is v12.22.0.
 
-## Native `grpc` module support is removed
+## API Changes
+
+We stopped exporting pubsub internals from the index file, for more details compare the [v1 API](/v1/api/) and [v2 API docs](../api/).
+
+## Configuration Changes
+
+### `GOOGLE_CLOUD_PUB_SUB_PROJECT_ID` is now `GOOGLE_CLOUD_PROJECT`
+
+To follow similar pattern to Google's [automatic injected env vars](https://cloud.google.com/functions/docs/configuring/env-var#runtime_environment_variables_set_automatically).
+
+### Native `grpc` module support is removed
 
 The `PUBSUBS_USE_GRPC` option is removed.
 
-## Throws error on no subscriptions found
+### Throws error on no subscriptions found
 
-`PUBSUB_ROOT_DIR` if is not found or if the directory doesn't have any subscriptions then service will not start and exit with error.
+If `PUBSUB_ROOT_DIR` or the CLI arg (`--root-dir`) is not found or if the directory doesn't have any subscriptions then service will not start and instead exit with error.
 
 ## Publishing
 
@@ -275,6 +285,10 @@ export default: SubscriberObject<Payload> = {
 
 </Tabs>
 
+### `ackDeadlineSeconds` is now `ackDeadline`
+
+Previously we supported both options, but now only `ackDeadline` is accepted.
+
 ## Typescript Changes
 
 Since most of the classes/interfaces now accept generics for Payload types, the typescript workflow is considerably better. This is how the you would reuse types across topics/subscriptions:

diff --git a/docs/subscribing/Internal Error Handling.md b/docs/subscribing/Internal Error Handling.md
@@ -61,11 +61,12 @@ exports.default = {
 
 ## SubscriptionService handleError Examples
 
+You can also setup a global error handler that will act as the default for all subscriptions, this though also receives the subscription metadat for which the error happens.
+
 ### Typescript subscription example
 
 ```ts title="/pubsub/subscription.service.ts"
-import * as PubSub from '@honestfoodcompany/pubsub';
-import { SubscriberOptions } from '@honestfoodcompany/pubsub';
+import { SubscriptionService, SubscriberOptions, SubscriberMetadata } from '@honestfoodcompany/pubsub';
 
 export default class SubscriptionService extends PubSub.SubscriptionService {
   static subscribers = [
@@ -76,9 +77,9 @@ export default class SubscriptionService extends PubSub.SubscriptionService {
 
   static async init(): Promise<void> {}
 
-  static handleError(error: Error): void {
+  static handleError(error: Error, metadata: SubscriberMetadata): void {
     // global default error handling logic for all subscribers
-    console.error(error);
+    console.error({err, metadata}, 'Default error handler');
     // Close DB connections/etc here
     process.exit(1);
   }
@@ -98,9 +99,9 @@ SubscriptionService.defaultSubscriberOptions = {};
 
 SubscriptionService.init = () => {};
 
-SubscriptionService.handleError = (error) => {
+SubscriptionService.handleError = (error, metadata) => {
   // global default error handling logic for all subscribers
-  console.error(error);
+  console.error({err, metadata}, 'Default error handler');
   // Close DB connections/etc here
   process.exit(1);
 };

diff --git a/env-example.txt b/env-example.txt
@@ -1,3 +1,3 @@
 PUBSUB_ROOT_DIR= 
-GOOGLE_CLOUD_PUB_SUB_PROJECT_ID=
+GOOGLE_CLOUD_PROJECT=
 GOOGLE_APPLICATION_CREDENTIALS=
diff --git a/examples/typescript/.env.example b/examples/typescript/.env.example
@@ -1,4 +1,4 @@
 # Rename this file to .env
 PUBSUB_ROOT_DIR=./
-GOOGLE_CLOUD_PUB_SUB_PROJECT_ID=your_project_id_goes_here
+GOOGLE_CLOUD_PROJECT=your_project_id_goes_here
 GOOGLE_APPLICATION_CREDENTIALS=path_to_json_credentials_goes_here
diff --git a/jest.config.js b/jest.config.js
@@ -26,10 +26,10 @@ module.exports = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      statements: 70,
+      statements: 60,
       branches: 55,
-      functions: 65,
-      lines: 70,
+      functions: 60,
+      lines: 60,
     },
   },
 

diff --git a/src/cli/env.setup.ts b/src/cli/env.setup.ts
@@ -1,9 +1,8 @@
 const envMapping: any = {
-  projectId: 'GOOGLE_CLOUD_PUB_SUB_PROJECT_ID',
+  projectId: 'GOOGLE_CLOUD_PROJECT',
+  googleApplicationCredentials: 'GOOGLE_APPLICATION_CREDENTIALS',
   rootDir: 'PUBSUB_ROOT_DIR',
-  projectNumber: 'PROJECT_NUMBER',
   driver: 'PUBSUB_DRIVER',
-  googleApplicationCredentials: 'GOOGLE_APPLICATION_CREDENTIALS',
   healthServer: 'PUBSUB_HEALTH_SERVER',
   serverPort: 'PUBSUB_SERVER_PORT',
 };

diff --git a/src/cli/subscriptions.ts b/src/cli/subscriptions.ts
@@ -14,25 +14,18 @@ require('yargs')
       describe: 'Path where pubsub folder resides',
       type: 'string',
     },
-    'project-number': {
-      describe: '(Optional) project number for the project',
-      type: 'string',
-    },
     driver: {
       describe: 'Use value synchronous or leave it to default async',
       type: 'string',
     },
-
     'google-application-credentials': {
       describe: 'Path to exported credentials file',
       type: 'string',
     },
-
     'server-port': {
       describe: 'Port at which server would run default 8080',
       type: 'string',
     },
-
     'health-server': {
       describe: 'If true runs server telling health state of subscriptions',
       type: 'string',

diff --git a/src/client/eventBus.ts b/src/client/eventBus.ts
@@ -33,8 +33,7 @@ export default class EventBus extends EventEmitter implements PubSubClientV2 {
   }
 
   public async subscribe(subscriber: SubscriberTuple): Promise<void> {
-    const [subscriberClass, metadata] = subscriber;
-    const instance = new subscriberClass();
+    const [instance, metadata] = subscriber;
     await instance.init();
     EventBus.getInstance().addListener(
       metadata.topicName,