address pr comments

Author: colerogers

spec/v2/providers/alerts/alerts.spec.ts

@@ -1,23 +1,15 @@
 import { expect } from 'chai';
-import * as sinon from 'sinon';
-import * as config from '../../../../src/config';
 import { CloudEvent, CloudFunction } from '../../../../src/v2/core';
 import * as options from '../../../../src/v2/options';
 import * as alerts from '../../../../src/v2/providers/alerts';
-import {
-  BASIC_ENDPOINT,
-  BASIC_OPTIONS,
-  FULL_ENDPOINT,
-  FULL_OPTIONS,
-} from '../helpers';
+import { FULL_ENDPOINT, FULL_OPTIONS } from '../helpers';
 
 const ALERT_TYPE = 'new-alert-type';
 const APPID = '123456789';
 
 function getMockFunction(): CloudFunction<alerts.FirebaseAlertData<String>> {
   const func = (raw: CloudEvent<unknown>) => 42;
   func.run = (event: CloudEvent<alerts.FirebaseAlertData<String>>) => 42;
-  func.__trigger = 'silence the transpiler';
   func.__endpoint = {};
   return func;
 }
@@ -43,15 +35,15 @@ describe('alerts', () => {
     it('should create the function with opts', () => {
       const result = alerts.onAlertPublished(
         {
-          ...BASIC_OPTIONS,
+          ...FULL_OPTIONS,
           alertType: ALERT_TYPE,
           appId: APPID,
         },
         () => 42
       );
 
       expect(result.__endpoint).to.deep.equal({
-        ...BASIC_ENDPOINT,
+        ...FULL_ENDPOINT,
         eventTrigger: {
           eventType: alerts.eventType,
           eventFilters: {
@@ -72,24 +64,20 @@ describe('alerts', () => {
     });
   });
 
-  describe('defineEndpoint', () => {
-    let configStub: sinon.SinonStub;
-
+  describe('getEndpointAnnotation', () => {
     beforeEach(() => {
       process.env.GCLOUD_PROJECT = 'aProject';
-      configStub = sinon.stub(config, 'firebaseConfig');
     });
 
     afterEach(() => {
       options.setGlobalOptions({});
       delete process.env.GCLOUD_PROJECT;
-      configStub.restore();
     });
 
     it('should define the endpoint without appId and opts', () => {
       const func = getMockFunction();
 
-      alerts.defineEndpoint(func, {}, ALERT_TYPE);
+      func.__endpoint = alerts.getEndpointAnnotation({}, ALERT_TYPE);
 
       expect(func.__endpoint).to.deep.equal({
         platform: 'gcfv2',
@@ -104,35 +92,20 @@ describe('alerts', () => {
       });
     });
 
-    it('should define the endpoint without appId, with opts', () => {
-      const func = getMockFunction();
-
-      alerts.defineEndpoint(func, { ...BASIC_OPTIONS }, ALERT_TYPE);
-
-      expect(func.__endpoint).to.deep.equal({
-        ...BASIC_ENDPOINT,
-        eventTrigger: {
-          eventType: alerts.eventType,
-          eventFilters: {
-            alertType: ALERT_TYPE,
-          },
-          retry: false,
-        },
-      });
-    });
-
-    it('should define the endpoint with appId', () => {
+    it('should define a complex endpoint without appId', () => {
       const func = getMockFunction();
 
-      alerts.defineEndpoint(func, { ...BASIC_OPTIONS }, ALERT_TYPE, APPID);
+      func.__endpoint = alerts.getEndpointAnnotation(
+        { ...FULL_OPTIONS },
+        ALERT_TYPE
+      );
 
       expect(func.__endpoint).to.deep.equal({
-        ...BASIC_ENDPOINT,
+        ...FULL_ENDPOINT,
         eventTrigger: {
           eventType: alerts.eventType,
           eventFilters: {
             alertType: ALERT_TYPE,
-            appId: APPID,
           },
           retry: false,
         },
@@ -142,20 +115,19 @@ describe('alerts', () => {
     it('should define a complex endpoint', () => {
       const func = getMockFunction();
 
-      alerts.defineEndpoint(
-        func,
+      func.__endpoint = alerts.getEndpointAnnotation(
         { ...FULL_OPTIONS },
-        'new-alert-type',
-        '123456789'
+        ALERT_TYPE,
+        APPID
       );
 
       expect(func.__endpoint).to.deep.equal({
         ...FULL_ENDPOINT,
         eventTrigger: {
           eventType: alerts.eventType,
           eventFilters: {
-            alertType: 'new-alert-type',
-            appId: '123456789',
+            alertType: ALERT_TYPE,
+            appId: APPID,
           },
           retry: false,
         },
@@ -174,7 +146,11 @@ describe('alerts', () => {
       };
       const func = getMockFunction();
 
-      alerts.defineEndpoint(func, specificOpts, ALERT_TYPE, APPID);
+      func.__endpoint = alerts.getEndpointAnnotation(
+        specificOpts,
+        ALERT_TYPE,
+        APPID
+      );
 
       expect(func.__endpoint).to.deep.equal({
         platform: 'gcfv2',

spec/v2/providers/helpers.ts

@@ -1,20 +1,5 @@
 import * as options from '../../../src/v2/options';
 
-export const BASIC_OPTIONS: options.EventHandlerOptions = {
-  labels: {
-    someKey: 'someValue',
-  },
-  region: 'us-east1',
-};
-
-export const BASIC_ENDPOINT = {
-  platform: 'gcfv2',
-  region: ['us-east1'],
-  labels: {
-    someKey: 'someValue',
-  },
-};
-
 export const FULL_OPTIONS: options.GlobalOptions = {
   region: 'us-west1',
   memory: '512MB',

src/v2/core.ts

@@ -52,7 +52,7 @@ export interface TriggerAnnotation {
  * A CloudEventBase is the base of a cross-platform format for encoding a serverless event.
  * More information can be found in https://github.com/cloudevents/spec
  */
-export interface CloudEventBase<T> {
+interface CloudEventBase<T> {
   /** Version of the CloudEvents spec for this event. */
   readonly specversion: '1.0';
 

src/v2/providers/alerts/alerts.ts

@@ -2,13 +2,27 @@ import { ManifestEndpoint } from '../../../common/manifest';
 import { CloudEvent, CloudFunction } from '../../core';
 import * as options from '../../options';
 
-/** Data */
+/**
+ * The data object that is emitted from Firebase Alerts inside the CloudEvent
+ */
 export interface FirebaseAlertData<T = any> {
   createTime: string;
   endTime: string;
   payload: T;
 }
 
+interface WithAlertTypeAndApp {
+  alertType: string;
+  appId?: string;
+}
+/**
+ * A custom CloudEvent for Firebase Alerts with custom extension attributes defined
+ */
+export type AlertEvent<T> = CloudEvent<
+  FirebaseAlertData<T>,
+  WithAlertTypeAndApp
+>;
+
 /** @internal */
 export const eventType = 'firebase.firebasealerts.alerts.v1.published';
 
@@ -25,24 +39,18 @@ export type AlertType =
   | 'appDistribution.newTesterIosDevice'
   | string;
 
-/** Options */
+/**
+ * Configuration for Firebase Alert functions
+ */
 export interface FirebaseAlertOptions extends options.EventHandlerOptions {
   alertType: AlertType;
   appId?: string;
 }
 
-/** Cloud Event Type */
-interface WithAlertTypeAndApp {
-  alertType: string;
-  appId?: string;
-}
-export type AlertEvent<T> = CloudEvent<
-  FirebaseAlertData<T>,
-  WithAlertTypeAndApp
->;
-
-/** Handlers */
-/** Handle an alert published */
+/**
+ * Declares a function that can handle Firebase Alerts from CloudEvents
+ * @param alertTypeOrOpts the alert type or Firebase Alert function configuration
+ */
 export function onAlertPublished<T extends { ['@type']: string } = any>(
   alertTypeOrOpts: AlertType | FirebaseAlertOptions,
   handler: (event: AlertEvent<T>) => any | Promise<any>
@@ -56,57 +64,48 @@ export function onAlertPublished<T extends { ['@type']: string } = any>(
   };
 
   func.run = handler;
-
-  // TypeScript doesn't recognize defineProperty as adding a property and complains
-  // that __endpoint doesn't exist. We can either cast to any and lose all type safety
-  // or we can just assign a meaningless value before calling defineProperty.
-  func.__trigger = 'silence the transpiler';
-  func.__endpoint = {} as ManifestEndpoint;
-  defineEndpoint(func, opts, alertType, appId);
+  func.__endpoint = getEndpointAnnotation(opts, alertType, appId);
 
   return func;
 }
 
 /**
  * @internal
- * Helper function for defining an endpoint for alert handling functions
+ * Helper function for getting the endpoint annotation used in alert handling functions
  */
-export function defineEndpoint(
-  func: CloudFunction<FirebaseAlertData<any>>,
+export function getEndpointAnnotation(
   opts: options.EventHandlerOptions,
   alertType: string,
   appId?: string
-): void {
-  Object.defineProperty(func, '__endpoint', {
-    get: () => {
-      const baseOpts = options.optionsToEndpoint(options.getGlobalOptions());
-      const specificOpts = options.optionsToEndpoint(opts);
-
-      const endpoint: ManifestEndpoint = {
-        platform: 'gcfv2',
-        ...baseOpts,
-        ...specificOpts,
-        labels: {
-          ...baseOpts?.labels,
-          ...specificOpts?.labels,
-        },
-        eventTrigger: {
-          eventType,
-          eventFilters: {
-            alertType,
-          },
-          retry: false,
-        },
-      };
-      if (appId) {
-        endpoint.eventTrigger.eventFilters.appId = appId;
-      }
-      return endpoint;
+): ManifestEndpoint {
+  const baseOpts = options.optionsToEndpoint(options.getGlobalOptions());
+  const specificOpts = options.optionsToEndpoint(opts);
+  const endpoint: ManifestEndpoint = {
+    platform: 'gcfv2',
+    ...baseOpts,
+    ...specificOpts,
+    labels: {
+      ...baseOpts?.labels,
+      ...specificOpts?.labels,
+    },
+    eventTrigger: {
+      eventType,
+      eventFilters: {
+        alertType,
+      },
+      retry: false,
     },
-  });
+  };
+  if (appId) {
+    endpoint.eventTrigger.eventFilters.appId = appId;
+  }
+  return endpoint;
 }
 
-/** @internal */
+/**
+ * @internal
+ * Helper function to parse the function opts, alert type, and appId
+ */
 export function getOptsAndAlertTypeAndApp(
   alertTypeOrOpts: AlertType | FirebaseAlertOptions
 ): [options.EventHandlerOptions, string, string | undefined] {